Hi Masahiro, On Feb 23, 2015 8:16 PM, "Masahiro Yamada" <[email protected]> wrote: > > Hi Simon, > > > > On Mon, 23 Feb 2015 11:01:02 -0700 > Simon Glass <[email protected]> wrote: > > > Hi Masahiro, > > > > On 20 February 2015 at 04:38, Masahiro Yamada <[email protected]> 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.
OK, this command displays ->req_seq. Can you check ->seq after you probe the device? > > > > => 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@00000000 @ 9fb520f8 > > Cannot find uclass for id 8: please add the UCLASS_DRIVER() declaration for this UCLASS_... id > uclass 9: serial > - * serial@54006800 @ 9fb52170, 0 > - serial@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@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 <[email protected]> > > > --- > > > > > > 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. Sorry I can't test right now. But I wonder if we could/should build your logic into a new function ? I need to think about it too. Regards, Simon _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
