[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