| Message ID | 20160718094109.2076-4-mugunthanvnm@ti.com |
|---|---|
| State | Accepted |
| Commit | 5142ac791608a5e166f3f1b76b19adcd01aceabc |
| Delegated to: | Heiko Schocher |
| Headers | show |
Hi Mugunthan, On 18 July 2016 at 03:40, Mugunthan V N <mugunthanvnm@ti.com> wrote: > parse dt parameter of i2c devices only when CONFIG_OF_CONTROL > is enabled. > > Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com> > --- > drivers/i2c/i2c-uclass.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) Reviewed-by: Simon Glass <sjg@chromium.org> Please see below. > > diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c > index 50b99ea..20b30ff 100644 > --- a/drivers/i2c/i2c-uclass.c > +++ b/drivers/i2c/i2c-uclass.c > @@ -467,6 +467,7 @@ int i2c_deblock(struct udevice *bus) > return ops->deblock(bus); > } > > +#if CONFIG_IS_ENABLED(OF_CONTROL) > int i2c_chip_ofdata_to_platdata(const void *blob, int node, > struct dm_i2c_chip *chip) > { > @@ -482,31 +483,44 @@ int i2c_chip_ofdata_to_platdata(const void *blob, int node, > > return 0; > } > +#endif > > static int i2c_post_probe(struct udevice *dev) > { > +#if CONFIG_IS_ENABLED(OF_CONTROL) > struct dm_i2c_bus *i2c = dev_get_uclass_priv(dev); > > i2c->speed_hz = fdtdec_get_int(gd->fdt_blob, dev->of_offset, > "clock-frequency", 100000); The above should be moved into i2c_chip_ofdata_to_platdata(), which will only be called if there is a device tree. > > return dm_i2c_set_bus_speed(dev, i2c->speed_hz); I'm not sure about this one. In principle there should be a value i2c->speed_hz even if OF_CONTROL is not used. But I suppose it's OK to retain this #ifdef. > +#else > + return 0; > +#endif > } > > static int i2c_post_bind(struct udevice *dev) > { > +#if CONFIG_IS_ENABLED(OF_CONTROL) > /* Scan the bus for devices */ > return dm_scan_fdt_node(dev, gd->fdt_blob, dev->of_offset, false); > +#else > + return 0; > +#endif > } > > static int i2c_child_post_bind(struct udevice *dev) > { > +#if CONFIG_IS_ENABLED(OF_CONTROL) > struct dm_i2c_chip *plat = dev_get_parent_platdata(dev); > > if (dev->of_offset == -1) > return 0; > > return i2c_chip_ofdata_to_platdata(gd->fdt_blob, dev->of_offset, plat); > +#else > + return 0; > +#endif > } > > UCLASS_DRIVER(i2c) = { > -- > 2.9.1.200.gb1ec08f > Regards, Simon
On Friday 22 July 2016 08:51 AM, Simon Glass wrote: > Hi Mugunthan, > > On 18 July 2016 at 03:40, Mugunthan V N <mugunthanvnm@ti.com> wrote: >> parse dt parameter of i2c devices only when CONFIG_OF_CONTROL >> is enabled. >> >> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com> >> --- >> drivers/i2c/i2c-uclass.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) > > Reviewed-by: Simon Glass <sjg@chromium.org> > > Please see below. > >> >> diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c >> index 50b99ea..20b30ff 100644 >> --- a/drivers/i2c/i2c-uclass.c >> +++ b/drivers/i2c/i2c-uclass.c >> @@ -467,6 +467,7 @@ int i2c_deblock(struct udevice *bus) >> return ops->deblock(bus); >> } >> >> +#if CONFIG_IS_ENABLED(OF_CONTROL) >> int i2c_chip_ofdata_to_platdata(const void *blob, int node, >> struct dm_i2c_chip *chip) >> { >> @@ -482,31 +483,44 @@ int i2c_chip_ofdata_to_platdata(const void *blob, int node, >> >> return 0; >> } >> +#endif >> >> static int i2c_post_probe(struct udevice *dev) >> { >> +#if CONFIG_IS_ENABLED(OF_CONTROL) >> struct dm_i2c_bus *i2c = dev_get_uclass_priv(dev); >> >> i2c->speed_hz = fdtdec_get_int(gd->fdt_blob, dev->of_offset, >> "clock-frequency", 100000); > > The above should be moved into i2c_chip_ofdata_to_platdata(), which > will only be called if there is a device tree. This cannot be moved to i2c_chip_ofdata_to_platdata() as it is called from post_bind where uclass_priv will not be allocated. uclass_priv will be allocated in device probe. Regards Mugunthan V N > >> >> return dm_i2c_set_bus_speed(dev, i2c->speed_hz); > > I'm not sure about this one. In principle there should be a value > i2c->speed_hz even if OF_CONTROL is not used. But I suppose it's OK to > retain this #ifdef. > >> +#else >> + return 0; >> +#endif >> } >> >> static int i2c_post_bind(struct udevice *dev) >> { >> +#if CONFIG_IS_ENABLED(OF_CONTROL) >> /* Scan the bus for devices */ >> return dm_scan_fdt_node(dev, gd->fdt_blob, dev->of_offset, false); >> +#else >> + return 0; >> +#endif >> } >> >> static int i2c_child_post_bind(struct udevice *dev) >> { >> +#if CONFIG_IS_ENABLED(OF_CONTROL) >> struct dm_i2c_chip *plat = dev_get_parent_platdata(dev); >> >> if (dev->of_offset == -1) >> return 0; >> >> return i2c_chip_ofdata_to_platdata(gd->fdt_blob, dev->of_offset, plat); >> +#else >> + return 0; >> +#endif >> } >> >> UCLASS_DRIVER(i2c) = { >> -- >> 2.9.1.200.gb1ec08f >> > > Regards, > Simon >
Hi Muganthan, On 22 July 2016 at 01:35, Mugunthan V N <mugunthanvnm@ti.com> wrote: > On Friday 22 July 2016 08:51 AM, Simon Glass wrote: >> Hi Mugunthan, >> >> On 18 July 2016 at 03:40, Mugunthan V N <mugunthanvnm@ti.com> wrote: >>> parse dt parameter of i2c devices only when CONFIG_OF_CONTROL >>> is enabled. >>> >>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com> >>> --- >>> drivers/i2c/i2c-uclass.c | 14 ++++++++++++++ >>> 1 file changed, 14 insertions(+) >> >> Reviewed-by: Simon Glass <sjg@chromium.org> >> >> Please see below. >> >>> >>> diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c >>> index 50b99ea..20b30ff 100644 >>> --- a/drivers/i2c/i2c-uclass.c >>> +++ b/drivers/i2c/i2c-uclass.c >>> @@ -467,6 +467,7 @@ int i2c_deblock(struct udevice *bus) >>> return ops->deblock(bus); >>> } >>> >>> +#if CONFIG_IS_ENABLED(OF_CONTROL) >>> int i2c_chip_ofdata_to_platdata(const void *blob, int node, >>> struct dm_i2c_chip *chip) >>> { >>> @@ -482,31 +483,44 @@ int i2c_chip_ofdata_to_platdata(const void *blob, int node, >>> >>> return 0; >>> } >>> +#endif >>> >>> static int i2c_post_probe(struct udevice *dev) >>> { >>> +#if CONFIG_IS_ENABLED(OF_CONTROL) >>> struct dm_i2c_bus *i2c = dev_get_uclass_priv(dev); >>> >>> i2c->speed_hz = fdtdec_get_int(gd->fdt_blob, dev->of_offset, >>> "clock-frequency", 100000); >> >> The above should be moved into i2c_chip_ofdata_to_platdata(), which >> will only be called if there is a device tree. > > This cannot be moved to i2c_chip_ofdata_to_platdata() as it is called > from post_bind where uclass_priv will not be allocated. uclass_priv will > be allocated in device probe. OK I see. Then why do we need to support i2c without OF_CONTROL? Would it not be better to enable OF_CONTROL? Regards, Simon
On Friday 22 July 2016 07:46 PM, Simon Glass wrote: > Hi Muganthan, > > On 22 July 2016 at 01:35, Mugunthan V N <mugunthanvnm@ti.com> wrote: >> On Friday 22 July 2016 08:51 AM, Simon Glass wrote: >>> Hi Mugunthan, >>> >>> On 18 July 2016 at 03:40, Mugunthan V N <mugunthanvnm@ti.com> wrote: >>>> parse dt parameter of i2c devices only when CONFIG_OF_CONTROL >>>> is enabled. >>>> >>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com> >>>> --- >>>> drivers/i2c/i2c-uclass.c | 14 ++++++++++++++ >>>> 1 file changed, 14 insertions(+) >>> >>> Reviewed-by: Simon Glass <sjg@chromium.org> >>> >>> Please see below. >>> >>>> >>>> diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c >>>> index 50b99ea..20b30ff 100644 >>>> --- a/drivers/i2c/i2c-uclass.c >>>> +++ b/drivers/i2c/i2c-uclass.c >>>> @@ -467,6 +467,7 @@ int i2c_deblock(struct udevice *bus) >>>> return ops->deblock(bus); >>>> } >>>> >>>> +#if CONFIG_IS_ENABLED(OF_CONTROL) >>>> int i2c_chip_ofdata_to_platdata(const void *blob, int node, >>>> struct dm_i2c_chip *chip) >>>> { >>>> @@ -482,31 +483,44 @@ int i2c_chip_ofdata_to_platdata(const void *blob, int node, >>>> >>>> return 0; >>>> } >>>> +#endif >>>> >>>> static int i2c_post_probe(struct udevice *dev) >>>> { >>>> +#if CONFIG_IS_ENABLED(OF_CONTROL) >>>> struct dm_i2c_bus *i2c = dev_get_uclass_priv(dev); >>>> >>>> i2c->speed_hz = fdtdec_get_int(gd->fdt_blob, dev->of_offset, >>>> "clock-frequency", 100000); >>> >>> The above should be moved into i2c_chip_ofdata_to_platdata(), which >>> will only be called if there is a device tree. >> >> This cannot be moved to i2c_chip_ofdata_to_platdata() as it is called >> from post_bind where uclass_priv will not be allocated. uclass_priv will >> be allocated in device probe. > > OK I see. Then why do we need to support i2c without OF_CONTROL? Would > it not be better to enable OF_CONTROL? > Due to the memory size issue in OMAP SoCs, enabling OF_CONTROL for spl is not possible. So having an option of enabling i2c uclass without OF_CONTROL will be a good option. Regards Mugunthan V N
Hi Mugunthan, On 25 July 2016 at 08:35, Mugunthan V N <mugunthanvnm@ti.com> wrote: > On Friday 22 July 2016 07:46 PM, Simon Glass wrote: >> Hi Muganthan, >> >> On 22 July 2016 at 01:35, Mugunthan V N <mugunthanvnm@ti.com> wrote: >>> On Friday 22 July 2016 08:51 AM, Simon Glass wrote: >>>> Hi Mugunthan, >>>> >>>> On 18 July 2016 at 03:40, Mugunthan V N <mugunthanvnm@ti.com> wrote: >>>>> parse dt parameter of i2c devices only when CONFIG_OF_CONTROL >>>>> is enabled. >>>>> >>>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com> >>>>> --- >>>>> drivers/i2c/i2c-uclass.c | 14 ++++++++++++++ >>>>> 1 file changed, 14 insertions(+) >>>> >>>> Reviewed-by: Simon Glass <sjg@chromium.org> >>>> >>>> Please see below. >>>> >>>>> >>>>> diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c >>>>> index 50b99ea..20b30ff 100644 >>>>> --- a/drivers/i2c/i2c-uclass.c >>>>> +++ b/drivers/i2c/i2c-uclass.c >>>>> @@ -467,6 +467,7 @@ int i2c_deblock(struct udevice *bus) >>>>> return ops->deblock(bus); >>>>> } >>>>> >>>>> +#if CONFIG_IS_ENABLED(OF_CONTROL) >>>>> int i2c_chip_ofdata_to_platdata(const void *blob, int node, >>>>> struct dm_i2c_chip *chip) >>>>> { >>>>> @@ -482,31 +483,44 @@ int i2c_chip_ofdata_to_platdata(const void *blob, int node, >>>>> >>>>> return 0; >>>>> } >>>>> +#endif >>>>> >>>>> static int i2c_post_probe(struct udevice *dev) >>>>> { >>>>> +#if CONFIG_IS_ENABLED(OF_CONTROL) >>>>> struct dm_i2c_bus *i2c = dev_get_uclass_priv(dev); >>>>> >>>>> i2c->speed_hz = fdtdec_get_int(gd->fdt_blob, dev->of_offset, >>>>> "clock-frequency", 100000); >>>> >>>> The above should be moved into i2c_chip_ofdata_to_platdata(), which >>>> will only be called if there is a device tree. >>> >>> This cannot be moved to i2c_chip_ofdata_to_platdata() as it is called >>> from post_bind where uclass_priv will not be allocated. uclass_priv will >>> be allocated in device probe. >> >> OK I see. Then why do we need to support i2c without OF_CONTROL? Would >> it not be better to enable OF_CONTROL? >> > > Due to the memory size issue in OMAP SoCs, enabling OF_CONTROL for spl > is not possible. So having an option of enabling i2c uclass without > OF_CONTROL will be a good option. How does I2C work without OF_CONTROL? I thought it required it... Regards, Simon
diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c index 50b99ea..20b30ff 100644 --- a/drivers/i2c/i2c-uclass.c +++ b/drivers/i2c/i2c-uclass.c @@ -467,6 +467,7 @@ int i2c_deblock(struct udevice *bus) return ops->deblock(bus); } +#if CONFIG_IS_ENABLED(OF_CONTROL) int i2c_chip_ofdata_to_platdata(const void *blob, int node, struct dm_i2c_chip *chip) { @@ -482,31 +483,44 @@ int i2c_chip_ofdata_to_platdata(const void *blob, int node, return 0; } +#endif static int i2c_post_probe(struct udevice *dev) { +#if CONFIG_IS_ENABLED(OF_CONTROL) struct dm_i2c_bus *i2c = dev_get_uclass_priv(dev); i2c->speed_hz = fdtdec_get_int(gd->fdt_blob, dev->of_offset, "clock-frequency", 100000); return dm_i2c_set_bus_speed(dev, i2c->speed_hz); +#else + return 0; +#endif } static int i2c_post_bind(struct udevice *dev) { +#if CONFIG_IS_ENABLED(OF_CONTROL) /* Scan the bus for devices */ return dm_scan_fdt_node(dev, gd->fdt_blob, dev->of_offset, false); +#else + return 0; +#endif } static int i2c_child_post_bind(struct udevice *dev) { +#if CONFIG_IS_ENABLED(OF_CONTROL) struct dm_i2c_chip *plat = dev_get_parent_platdata(dev); if (dev->of_offset == -1) return 0; return i2c_chip_ofdata_to_platdata(gd->fdt_blob, dev->of_offset, plat); +#else + return 0; +#endif } UCLASS_DRIVER(i2c) = {
parse dt parameter of i2c devices only when CONFIG_OF_CONTROL is enabled. Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com> --- drivers/i2c/i2c-uclass.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)