[U-Boot] [PATCH 1/2] dm: i2c: add DM_I2C_REQ_ALIAS to make sequence number optional

Masahiro Yamada yamada.m at jp.panasonic.com
Tue Feb 24 04:16:51 CET 2015


Hi Simon,



On Mon, 23 Feb 2015 11:01:02 -0700
Simon Glass <sjg at chromium.org> wrote:

> Hi Masahiro,
> 
> On 20 February 2015 at 04:38, Masahiro Yamada <yamada.m at jp.panasonic.com> wrote:
> > For I2C (and some other uclasses), all the devices are required to
> > have their own request sequence numbers to use them.  Otherwise,
> > uclass_get_device_by_seq() fails to get them.  To specify those
> > numbers, we have to add aliases to device trees.  As a result, the
> > "aliases" nodes grow bigger and bigger as we support more and more
> > drivers in driver model.
> > (See arch/arm/dts/uniphier-ph1-pro4-ref.dts for example.)
> >
> > It is true that the sequence number is useful for identifying a
> > device with the same number independently of the binding order, but
> > it is painful to add an alias to every device.
> 
> That tends to me the standard. If you look at the .dts files in the
> kernel they have aliases. I didn't see any Uniphier dts files in there
> though.

This is a problem for of my company.
We are still using the locally hacked kernel
that has not been mainlined yet.

(BTW, before me, U-Boot was also locally hacked in an ugly way...)


It would not be a problem to have some aliases,
but I was just wondering if we must have an alias for every device.

If this is the standardized way, that's OK.


> >
> > This commit intends to make this feature optional.
> > If CONFIG_DM_I2C_SEQ_ALIAS is not defined, uclass_get_device() is
> > called instead of uclass_get_device_by_seq().  So we do not have to
> > add aliases of I2C devices.
> >
> > In most cases, this should work well enough because device trees are
> > scanned from top to bottom and we usually describe device nodes in
> > the order we expect for binding.
> 
> As of recently the sequence numbers should be assigned sensibly (0, 1,
> 2...) even without aliases. What do you see when you type 'dm uclass'
> on your board?


The following was displayed.
No sequence number was displayed for I2C.



=> dm uclass
uclass 0: root
- * root_driver @ 9fb52028

Cannot find uclass for id 1: please add the UCLASS_DRIVER() declaration for this UCLASS_... id
Cannot find uclass for id 2: please add the UCLASS_DRIVER() declaration for this UCLASS_... id
Cannot find uclass for id 3: please add the UCLASS_DRIVER() declaration for this UCLASS_... id
Cannot find uclass for id 4: please add the UCLASS_DRIVER() declaration for this UCLASS_... id
Cannot find uclass for id 5: please add the UCLASS_DRIVER() declaration for this UCLASS_... id
Cannot find uclass for id 6: please add the UCLASS_DRIVER() declaration for this UCLASS_... id
uclass 7: simple_bus
- * soc @ 9fb520a0
-   systembus at 00000000 @ 9fb520f8

Cannot find uclass for id 8: please add the UCLASS_DRIVER() declaration for this UCLASS_... id
uclass 9: serial
- * serial at 54006800 @ 9fb52170, 0
-   serial at 54006900 @ 9fb521d8, 1

Cannot find uclass for id 10: please add the UCLASS_DRIVER() declaration for this UCLASS_... id
Cannot find uclass for id 11: please add the UCLASS_DRIVER() declaration for this UCLASS_... id
Cannot find uclass for id 12: please add the UCLASS_DRIVER() declaration for this UCLASS_... id
Cannot find uclass for id 13: please add the UCLASS_DRIVER() declaration for this UCLASS_... id
Cannot find uclass for id 14: please add the UCLASS_DRIVER() declaration for this UCLASS_... id
uclass 15: i2c
-   i2c at 58400000 @ 9fb52260

uclass 16: i2c_generic
uclass 17: i2c_eeprom
-   eeprom @ 9fb522d8

Cannot find uclass for id 18: please add the UCLASS_DRIVER() declaration for this UCLASS_... id



> > Signed-off-by: Masahiro Yamada <yamada.m at jp.panasonic.com>
> > ---
> >
> >  common/cmd_i2c.c                |  2 +-
> >  drivers/i2c/Kconfig             | 10 ++++++++++
> >  drivers/i2c/i2c-uclass-compat.c |  2 +-
> >  drivers/i2c/i2c-uclass.c        |  4 +++-
> >  include/i2c.h                   | 17 +++++++++++++++++
> >  5 files changed, 32 insertions(+), 3 deletions(-)
> >
> > diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c
> > index fe8f77a..5fc927a 100644
> > --- a/common/cmd_i2c.c
> > +++ b/common/cmd_i2c.c
> > @@ -138,7 +138,7 @@ static int cmd_i2c_set_bus_num(unsigned int busnum)
> >         struct udevice *bus;
> >         int ret;
> >
> > -       ret = uclass_get_device_by_seq(UCLASS_I2C, busnum, &bus);
> > +       ret = i2c_get_bus(busnum, &bus);
> >         if (ret) {
> >                 debug("%s: No bus %d\n", __func__, busnum);
> >                 return ret;
> > diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
> > index 2cc776c..5c778ec 100644
> > --- a/drivers/i2c/Kconfig
> > +++ b/drivers/i2c/Kconfig
> > @@ -13,6 +13,16 @@ config DM_I2C
> >           enabled together (it is not possible to use driver model
> >           for one and not the other).
> >
> > +config DM_I2C_SEQ_ALIAS
> > +       bool "Use aliases for numbering I2C buses"
> > +       depends on DM_I2C
> > +       default y
> > +       help
> > +         If this option is enabled, each I2C bus is numbered base on its
> > +         alias in the device tree to identify a device with the same number
> > +         all the time.  Otherwise, I2C buses are numbered as they are scanned
> > +         at binding.
> > +
> >  config SYS_I2C_UNIPHIER
> >         bool "UniPhier I2C driver"
> >         depends on ARCH_UNIPHIER && DM_I2C
> > diff --git a/drivers/i2c/i2c-uclass-compat.c b/drivers/i2c/i2c-uclass-compat.c
> > index 223f238..35736e8 100644
> > --- a/drivers/i2c/i2c-uclass-compat.c
> > +++ b/drivers/i2c/i2c-uclass-compat.c
> > @@ -35,7 +35,7 @@ int i2c_probe(uint8_t chip_addr)
> >         struct udevice *bus, *dev;
> >         int ret;
> >
> > -       ret = uclass_get_device_by_seq(UCLASS_I2C, cur_busnum, &bus);
> > +       ret = i2c_get_bus(UCLASS_I2C, cur_busnum, &bus);
> >         if (ret) {
> >                 debug("Cannot find I2C bus %d: err=%d\n", cur_busnum, ret);
> >                 return ret;
> > diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
> > index a6991bf..0992737 100644
> > --- a/drivers/i2c/i2c-uclass.c
> > +++ b/drivers/i2c/i2c-uclass.c
> > @@ -289,7 +289,7 @@ int i2c_get_chip_for_busnum(int busnum, int chip_addr, uint offset_len,
> >         struct udevice *bus;
> >         int ret;
> >
> > -       ret = uclass_get_device_by_seq(UCLASS_I2C, busnum, &bus);
> > +       ret = i2c_get_bus(busnum, &bus);
> >         if (ret) {
> >                 debug("Cannot find I2C bus %d\n", busnum);
> >                 return ret;
> > @@ -457,7 +457,9 @@ static int i2c_child_post_bind(struct udevice *dev)
> >  UCLASS_DRIVER(i2c) = {
> >         .id             = UCLASS_I2C,
> >         .name           = "i2c",
> > +#ifdef CONFIG_DM_I2C_SEQ_ALIAS
> >         .flags          = DM_UC_FLAG_SEQ_ALIAS,
> > +#endif
> >         .post_bind      = i2c_post_bind,
> >         .post_probe     = i2c_post_probe,
> >         .per_device_auto_alloc_size = sizeof(struct dm_i2c_bus),
> > diff --git a/include/i2c.h b/include/i2c.h
> > index 31b0389..6941df6 100644
> > --- a/include/i2c.h
> > +++ b/include/i2c.h
> > @@ -410,6 +410,23 @@ int i2c_get_chip(struct udevice *bus, uint chip_addr, uint offset_len,
> >                  struct udevice **devp);
> >
> >  /**
> > + * i2c_get_bus() - get an I2C bus
> > + *
> > + * This returns the bus for the given bus number.
> > + *
> > + * @busnum:    Bus number.  If CONFIG_DM_I2C_SEQ_ALIAS is enabled, this is a
> > + *             sequence number.  Otherwise, it is a simple index.
> > + * @devp:      Stores pointer to a new bus if found
> > + */
> > +#ifdef CONFIG_DM_I2C_SEQ_ALIAS
> > +#define i2c_get_bus(busnum, devp) \
> > +                       uclass_get_device_by_seq(UCLASS_I2C, busnum, devp)
> > +#else
> > +#define i2c_get_bus(busnum, devp) \
> > +                       uclass_get_device(UCLASS_I2C, busnum, devp)
> > +#endif
> 
> I would prefer a solution that sits inside driver model itself. Once I
> have a response to the above question we can see if this is possible.
> 

I understand your thought.
I take this series back and I marked it as RFC.


Best Regards
Masahiro Yamada




More information about the U-Boot mailing list