All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/2] dm: i2c: make request sequence number optional and disable it for UniPhier
@ 2015-02-20 11:38 Masahiro Yamada
  2015-02-20 11:38 ` [U-Boot] [PATCH 1/2] dm: i2c: add DM_I2C_REQ_ALIAS to make sequence number optional Masahiro Yamada
  2015-02-20 11:38 ` [U-Boot] [PATCH 2/2] ARM: UniPhier: disable DM_I2C_REQ_ALIAS to drop I2C aliases Masahiro Yamada
  0 siblings, 2 replies; 6+ messages in thread
From: Masahiro Yamada @ 2015-02-20 11:38 UTC (permalink / raw)
  To: u-boot




Masahiro Yamada (2):
  dm: i2c: make request sequence number optional
  ARM: UniPhier: disable DM_I2C_REQ_ALIAS to drop I2C aliases

 arch/arm/dts/uniphier-ph1-ld4-ref.dts  |  4 ----
 arch/arm/dts/uniphier-ph1-pro4-ref.dts |  6 ------
 arch/arm/dts/uniphier-ph1-sld8-ref.dts |  4 ----
 common/cmd_i2c.c                       |  2 +-
 configs/ph1_ld4_defconfig              |  1 +
 configs/ph1_pro4_defconfig             |  1 +
 configs/ph1_sld8_defconfig             |  1 +
 drivers/i2c/Kconfig                    | 10 ++++++++++
 drivers/i2c/i2c-uclass-compat.c        |  2 +-
 drivers/i2c/i2c-uclass.c               |  4 +++-
 include/i2c.h                          | 17 +++++++++++++++++
 11 files changed, 35 insertions(+), 17 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 1/2] dm: i2c: add DM_I2C_REQ_ALIAS to make sequence number optional
  2015-02-20 11:38 [U-Boot] [PATCH 0/2] dm: i2c: make request sequence number optional and disable it for UniPhier Masahiro Yamada
@ 2015-02-20 11:38 ` Masahiro Yamada
  2015-02-23 18:01   ` Simon Glass
  2015-02-20 11:38 ` [U-Boot] [PATCH 2/2] ARM: UniPhier: disable DM_I2C_REQ_ALIAS to drop I2C aliases Masahiro Yamada
  1 sibling, 1 reply; 6+ messages in thread
From: Masahiro Yamada @ 2015-02-20 11:38 UTC (permalink / raw)
  To: u-boot

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.

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.

Signed-off-by: Masahiro Yamada <yamada.m@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
+
+/**
  * i2c_get_chip() - get a device to use to access a chip on a bus number
  *
  * This returns the device for the given chip address on a particular bus
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 2/2] ARM: UniPhier: disable DM_I2C_REQ_ALIAS to drop I2C aliases
  2015-02-20 11:38 [U-Boot] [PATCH 0/2] dm: i2c: make request sequence number optional and disable it for UniPhier Masahiro Yamada
  2015-02-20 11:38 ` [U-Boot] [PATCH 1/2] dm: i2c: add DM_I2C_REQ_ALIAS to make sequence number optional Masahiro Yamada
@ 2015-02-20 11:38 ` Masahiro Yamada
  1 sibling, 0 replies; 6+ messages in thread
From: Masahiro Yamada @ 2015-02-20 11:38 UTC (permalink / raw)
  To: u-boot

In our use cases, it would not be so troublesome to use the bus
number given by the binding order.

Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
---

 arch/arm/dts/uniphier-ph1-ld4-ref.dts  | 4 ----
 arch/arm/dts/uniphier-ph1-pro4-ref.dts | 6 ------
 arch/arm/dts/uniphier-ph1-sld8-ref.dts | 4 ----
 configs/ph1_ld4_defconfig              | 1 +
 configs/ph1_pro4_defconfig             | 1 +
 configs/ph1_sld8_defconfig             | 1 +
 6 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/arch/arm/dts/uniphier-ph1-ld4-ref.dts b/arch/arm/dts/uniphier-ph1-ld4-ref.dts
index d479be1..9c24bbf 100644
--- a/arch/arm/dts/uniphier-ph1-ld4-ref.dts
+++ b/arch/arm/dts/uniphier-ph1-ld4-ref.dts
@@ -30,10 +30,6 @@
 		serial1 = &uart1;
 		serial2 = &uart2;
 		serial3 = &uart3;
-		i2c0 = &i2c0;
-		i2c1 = &i2c1;
-		i2c2 = &i2c2;
-		i2c3 = &i2c3;
 	};
 };
 
diff --git a/arch/arm/dts/uniphier-ph1-pro4-ref.dts b/arch/arm/dts/uniphier-ph1-pro4-ref.dts
index 685b5a6..c76a837 100644
--- a/arch/arm/dts/uniphier-ph1-pro4-ref.dts
+++ b/arch/arm/dts/uniphier-ph1-pro4-ref.dts
@@ -30,12 +30,6 @@
 		serial1 = &uart1;
 		serial2 = &uart2;
 		serial3 = &uart3;
-		i2c0 = &i2c0;
-		i2c1 = &i2c1;
-		i2c2 = &i2c2;
-		i2c3 = &i2c3;
-		i2c5 = &i2c5;
-		i2c6 = &i2c6;
 	};
 };
 
diff --git a/arch/arm/dts/uniphier-ph1-sld8-ref.dts b/arch/arm/dts/uniphier-ph1-sld8-ref.dts
index 0cb9c47..3399ed2 100644
--- a/arch/arm/dts/uniphier-ph1-sld8-ref.dts
+++ b/arch/arm/dts/uniphier-ph1-sld8-ref.dts
@@ -30,10 +30,6 @@
 		serial1 = &uart1;
 		serial2 = &uart2;
 		serial3 = &uart3;
-		i2c0 = &i2c0;
-		i2c1 = &i2c1;
-		i2c2 = &i2c2;
-		i2c3 = &i2c3;
 	};
 };
 
diff --git a/configs/ph1_ld4_defconfig b/configs/ph1_ld4_defconfig
index 52bbe9b..4d06a2b 100644
--- a/configs/ph1_ld4_defconfig
+++ b/configs/ph1_ld4_defconfig
@@ -36,6 +36,7 @@ CONFIG_NAND_DENALI_SPARE_AREA_SKIP_BYTES=8
 CONFIG_DM_SERIAL=y
 CONFIG_UNIPHIER_SERIAL=y
 CONFIG_DM_I2C=y
+# CONFIG_DM_I2C_SEQ_ALIAS is not set
 CONFIG_USB=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_STORAGE=y
diff --git a/configs/ph1_pro4_defconfig b/configs/ph1_pro4_defconfig
index 1517f3c..e2a7d36 100644
--- a/configs/ph1_pro4_defconfig
+++ b/configs/ph1_pro4_defconfig
@@ -36,6 +36,7 @@ CONFIG_NAND_DENALI_SPARE_AREA_SKIP_BYTES=8
 CONFIG_DM_SERIAL=y
 CONFIG_UNIPHIER_SERIAL=y
 CONFIG_DM_I2C=y
+# CONFIG_DM_I2C_SEQ_ALIAS is not set
 CONFIG_USB=y
 CONFIG_USB_XHCI_HCD=y
 CONFIG_USB_STORAGE=y
diff --git a/configs/ph1_sld8_defconfig b/configs/ph1_sld8_defconfig
index e3ce5cc..394ce71 100644
--- a/configs/ph1_sld8_defconfig
+++ b/configs/ph1_sld8_defconfig
@@ -36,6 +36,7 @@ CONFIG_NAND_DENALI_SPARE_AREA_SKIP_BYTES=8
 CONFIG_DM_SERIAL=y
 CONFIG_UNIPHIER_SERIAL=y
 CONFIG_DM_I2C=y
+# CONFIG_DM_I2C_SEQ_ALIAS is not set
 CONFIG_USB=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_STORAGE=y
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 1/2] dm: i2c: add DM_I2C_REQ_ALIAS to make sequence number optional
  2015-02-20 11:38 ` [U-Boot] [PATCH 1/2] dm: i2c: add DM_I2C_REQ_ALIAS to make sequence number optional Masahiro Yamada
@ 2015-02-23 18:01   ` Simon Glass
  2015-02-24  3:16     ` Masahiro Yamada
  0 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2015-02-23 18:01 UTC (permalink / raw)
  To: u-boot

Hi Masahiro,

On 20 February 2015 at 04:38, Masahiro Yamada <yamada.m@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 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?

>
> Signed-off-by: Masahiro Yamada <yamada.m@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.

> +
> +/**
>   * i2c_get_chip() - get a device to use to access a chip on a bus number
>   *
>   * This returns the device for the given chip address on a particular bus
> --
> 1.9.1
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 1/2] dm: i2c: add DM_I2C_REQ_ALIAS to make sequence number optional
  2015-02-23 18:01   ` Simon Glass
@ 2015-02-24  3:16     ` Masahiro Yamada
  2015-02-24  4:48       ` Simon Glass
  0 siblings, 1 reply; 6+ messages in thread
From: Masahiro Yamada @ 2015-02-24  3:16 UTC (permalink / raw)
  To: u-boot

Hi Simon,



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

> Hi Masahiro,
> 
> On 20 February 2015 at 04:38, Masahiro Yamada <yamada.m@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@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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH 1/2] dm: i2c: add DM_I2C_REQ_ALIAS to make sequence number optional
  2015-02-24  3:16     ` Masahiro Yamada
@ 2015-02-24  4:48       ` Simon Glass
  0 siblings, 0 replies; 6+ messages in thread
From: Simon Glass @ 2015-02-24  4:48 UTC (permalink / raw)
  To: u-boot

Hi Masahiro,
On Feb 23, 2015 8:16 PM, "Masahiro Yamada" <yamada.m@jp.panasonic.com>
wrote:
>
> Hi Simon,
>
>
>
> On Mon, 23 Feb 2015 11:01:02 -0700
> Simon Glass <sjg@chromium.org> wrote:
>
> > Hi Masahiro,
> >
> > On 20 February 2015 at 04:38, Masahiro Yamada <yamada.m@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.

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 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@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.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-02-24  4:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-20 11:38 [U-Boot] [PATCH 0/2] dm: i2c: make request sequence number optional and disable it for UniPhier Masahiro Yamada
2015-02-20 11:38 ` [U-Boot] [PATCH 1/2] dm: i2c: add DM_I2C_REQ_ALIAS to make sequence number optional Masahiro Yamada
2015-02-23 18:01   ` Simon Glass
2015-02-24  3:16     ` Masahiro Yamada
2015-02-24  4:48       ` Simon Glass
2015-02-20 11:38 ` [U-Boot] [PATCH 2/2] ARM: UniPhier: disable DM_I2C_REQ_ALIAS to drop I2C aliases Masahiro Yamada

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.