* [U-Boot] [PATCH 0/4] Align U-Boot I2C DM bus ID handling with Linux
@ 2019-01-18 15:13 Michal Simek
2019-01-18 15:13 ` [U-Boot] [PATCH 1/4] dm: core: Add of_alias_get_highest_id() Michal Simek
` (4 more replies)
0 siblings, 5 replies; 19+ messages in thread
From: Michal Simek @ 2019-01-18 15:13 UTC (permalink / raw)
To: u-boot
U-Boot with I2C_DM enabled is not capable to list i2c busses connected
to i2c mux. For getting this work there is a need to find out highest
alias ID and use this uniq number for new buses connected to I2C mux.
This series is making this happen.
There is only one missing piece which is that also i2c controllers which
are not listed in DT are not using this feature.
For getting this work it would be necessary to setup dev->req_seq in
pre_bind function.
int i2c_pre_bind(struct udevice *dev)
{
if (dev->req_seq == -1) {
dev->req_seq = ++i2c_highest_id;
}
return 0;
}
It can be done also in post_bind but it looks quite weird that
controller has highest ID then i2c-mux sub-buses connected to it.
For example:
ZynqMP> i2c bus
Bus 3: i2c at ff020000
20: gpio at 20, offset len 1, flags 0
21: gpio at 21, offset len 1, flags 0
75: i2c-mux at 75, offset len 1, flags 0
Bus 0: i2c at ff020000->i2c-mux at 75->i2c at 0
Bus 1: i2c at ff020000->i2c-mux at 75->i2c at 1
Bus 2: i2c at ff020000->i2c-mux at 75->i2c at 2
Bus 17: i2c at ff030000
74: i2c-mux at 74, offset len 1, flags 0
75: i2c-mux at 75, offset len 1, flags 0
Bus 4: i2c at ff030000->i2c-mux at 74->i2c at 0
Bus 5: i2c at ff030000->i2c-mux at 74->i2c at 1
Bus 6: i2c at ff030000->i2c-mux at 74->i2c at 2
Bus 7: i2c at ff030000->i2c-mux at 74->i2c at 3
...
That's why I think it would be better to introduce pre_bind hooks and
call it in device_bind_common() before binding childs.
Thanks,
Michal
Michal Simek (4):
dm: core: Add of_alias_get_highest_id()
fdt: Introduce fdtdec_get_alias_highest_id()
i2c: dm: Record maximum id of devices before probing devices
i2c: mux: Add support for not listed sub-buses
drivers/core/of_access.c | 18 ++++++++++++++++++
drivers/i2c/i2c-uclass.c | 19 +++++++++++++++++++
drivers/i2c/muxes/i2c-mux-uclass.c | 35 +++++++++++++++++++++++++++++++++--
include/dm/of_access.h | 9 +++++++++
include/fdtdec.h | 13 +++++++++++++
include/i2c.h | 3 +++
lib/fdtdec.c | 33 +++++++++++++++++++++++++++++++++
7 files changed, 128 insertions(+), 2 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH 1/4] dm: core: Add of_alias_get_highest_id()
2019-01-18 15:13 [U-Boot] [PATCH 0/4] Align U-Boot I2C DM bus ID handling with Linux Michal Simek
@ 2019-01-18 15:13 ` Michal Simek
2019-01-28 12:35 ` Michal Simek
2019-01-31 10:04 ` Simon Glass
2019-01-18 15:13 ` [U-Boot] [PATCH 2/4] fdt: Introduce fdtdec_get_alias_highest_id() Michal Simek
` (3 subsequent siblings)
4 siblings, 2 replies; 19+ messages in thread
From: Michal Simek @ 2019-01-18 15:13 UTC (permalink / raw)
To: u-boot
The same functionality was added to Linux for i2c bus registration with this
commit message:
"
of: base: add function to get highest id of an alias stem
I2C supports adding adapters using either a dynamic or fixed id. The
latter is provided by aliases in the DT case. To prevent id collisions
of those two types, install this function which gives us the highest
fixed id, so we can then let the dynamically created ones come after
this highest number.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
"
Add it also to U-Boot for DM I2C support.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
drivers/core/of_access.c | 18 ++++++++++++++++++
include/dm/of_access.h | 9 +++++++++
2 files changed, 27 insertions(+)
diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c
index 14c020a687b7..7c2df2354109 100644
--- a/drivers/core/of_access.c
+++ b/drivers/core/of_access.c
@@ -812,6 +812,24 @@ int of_alias_get_id(const struct device_node *np, const char *stem)
return id;
}
+int of_alias_get_highest_id(const char *stem)
+{
+ struct alias_prop *app;
+ int id = -ENODEV;
+
+ mutex_lock(&of_mutex);
+ list_for_each_entry(app, &aliases_lookup, link) {
+ if (strcmp(app->stem, stem) != 0)
+ continue;
+
+ if (app->id > id)
+ id = app->id;
+ }
+ mutex_unlock(&of_mutex);
+
+ return id;
+}
+
struct device_node *of_get_stdout(void)
{
return of_stdout;
diff --git a/include/dm/of_access.h b/include/dm/of_access.h
index 5ed1a0cdb427..5cbfd220bfd4 100644
--- a/include/dm/of_access.h
+++ b/include/dm/of_access.h
@@ -425,6 +425,15 @@ int of_alias_scan(void);
int of_alias_get_id(const struct device_node *np, const char *stem);
/**
+ * of_alias_get_highest_id - Get highest alias id for the given stem
+ * @stem: Alias stem to be examined
+ *
+ * The function travels the lookup table to get the highest alias id for the
+ * given alias stem. It returns the alias id if found.
+ */
+int of_alias_get_highest_id(const char *stem);
+
+/**
* of_get_stdout() - Get node to use for stdout
*
* @return node referred to by stdout-path alias, or NULL if none
--
1.9.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH 2/4] fdt: Introduce fdtdec_get_alias_highest_id()
2019-01-18 15:13 [U-Boot] [PATCH 0/4] Align U-Boot I2C DM bus ID handling with Linux Michal Simek
2019-01-18 15:13 ` [U-Boot] [PATCH 1/4] dm: core: Add of_alias_get_highest_id() Michal Simek
@ 2019-01-18 15:13 ` Michal Simek
2019-01-31 10:04 ` Simon Glass
2019-01-18 15:13 ` [U-Boot] [PATCH 3/4] i2c: dm: Record maximum id of devices before probing devices Michal Simek
` (2 subsequent siblings)
4 siblings, 1 reply; 19+ messages in thread
From: Michal Simek @ 2019-01-18 15:13 UTC (permalink / raw)
To: u-boot
Find out the highest alias ID used for certain subsystem.
This call will be used for alocating IDs for i2c buses which are not
described in DT.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
include/fdtdec.h | 13 +++++++++++++
lib/fdtdec.c | 33 +++++++++++++++++++++++++++++++++
2 files changed, 46 insertions(+)
diff --git a/include/fdtdec.h b/include/fdtdec.h
index f1bcbf837ffb..c2dd87ede226 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -626,6 +626,19 @@ int fdtdec_get_alias_seq(const void *blob, const char *base, int node,
int *seqp);
/**
+ * Get the highest alias number for susbystem.
+ *
+ * It parses all aliases and find out highest recorded alias for subsystem.
+ * Aliases are of the form <base><num> where <num> is the sequence number.
+ *
+ * @param blob Device tree blob (if NULL, then error is returned)
+ * @param base Base name for alias susbystem (before the number)
+ *
+ * @return 0 highest alias ID, -1 if not found
+ */
+int fdtdec_get_alias_highest_id(const void *blob, const char *base);
+
+/**
* Get a property from the /chosen node
*
* @param blob Device tree blob (if NULL, then NULL is returned)
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 18663ce6bdac..55811975ef54 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -549,6 +549,39 @@ int fdtdec_get_alias_seq(const void *blob, const char *base, int offset,
return -ENOENT;
}
+int fdtdec_get_alias_highest_id(const void *blob, const char *base)
+{
+ int base_len = strlen(base);
+ int prop_offset;
+ int aliases;
+ int max = -1;
+
+ debug("Looking for highest alias id for '%s'\n", base);
+
+ aliases = fdt_path_offset(blob, "/aliases");
+ for (prop_offset = fdt_first_property_offset(blob, aliases);
+ prop_offset > 0;
+ prop_offset = fdt_next_property_offset(blob, prop_offset)) {
+ const char *prop;
+ const char *name;
+ int len, val;
+
+ prop = fdt_getprop_by_offset(blob, prop_offset, &name, &len);
+ debug(" - %s, %s\n", name, prop);
+ if (*prop != '/' || prop[len - 1] ||
+ strncmp(name, base, base_len))
+ continue;
+
+ val = trailing_strtol(name);
+ if (val > max) {
+ debug("Found seq %d\n", val);
+ max = val;
+ }
+ }
+
+ return max;
+}
+
const char *fdtdec_get_chosen_prop(const void *blob, const char *name)
{
int chosen_node;
--
1.9.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH 3/4] i2c: dm: Record maximum id of devices before probing devices
2019-01-18 15:13 [U-Boot] [PATCH 0/4] Align U-Boot I2C DM bus ID handling with Linux Michal Simek
2019-01-18 15:13 ` [U-Boot] [PATCH 1/4] dm: core: Add of_alias_get_highest_id() Michal Simek
2019-01-18 15:13 ` [U-Boot] [PATCH 2/4] fdt: Introduce fdtdec_get_alias_highest_id() Michal Simek
@ 2019-01-18 15:13 ` Michal Simek
2019-01-31 10:04 ` Simon Glass
2019-01-18 15:13 ` [U-Boot] [PATCH 4/4] i2c: mux: Add support for not listed sub-buses Michal Simek
2019-01-18 15:24 ` [U-Boot] [PATCH 0/4] Align U-Boot I2C DM bus ID handling with Linux Michal Simek
4 siblings, 1 reply; 19+ messages in thread
From: Michal Simek @ 2019-01-18 15:13 UTC (permalink / raw)
To: u-boot
There is a need to find out the first free i2c ID which can be used for
i2s buses (including i2c buses connected to i2c mux). Do it early in
init and share this variable with other i2c classes for uniq bus
identification.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
drivers/i2c/i2c-uclass.c | 19 +++++++++++++++++++
include/i2c.h | 3 +++
2 files changed, 22 insertions(+)
diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
index 975318e5f254..aaece115f02f 100644
--- a/drivers/i2c/i2c-uclass.c
+++ b/drivers/i2c/i2c-uclass.c
@@ -23,6 +23,8 @@ enum {
PIN_COUNT,
};
+int i2c_highest_id = -1;
+
/* Useful debugging function */
void i2c_dump_msgs(struct i2c_msg *msg, int nmsgs)
{
@@ -619,10 +621,27 @@ static int i2c_child_post_bind(struct udevice *dev)
#endif
}
+int i2c_uclass_init(struct uclass *class)
+{
+#ifdef CONFIG_OF_LIVE
+ int ret;
+
+ ret = of_alias_get_highest_id("i2c");
+ if (ret >= 0)
+ i2c_highest_id = ret;
+#else
+ i2c_highest_id = fdtdec_get_alias_highest_id(gd->fdt_blob, "i2c");
+#endif
+ debug("%s: i2c highest_id %d\n", __func__, i2c_highest_id);
+
+ return 0;
+}
+
UCLASS_DRIVER(i2c) = {
.id = UCLASS_I2C,
.name = "i2c",
.flags = DM_UC_FLAG_SEQ_ALIAS,
+ .init = i2c_uclass_init,
#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
.post_bind = dm_scan_fdt_dev,
#endif
diff --git a/include/i2c.h b/include/i2c.h
index ccffc1955275..c69e26deaeed 100644
--- a/include/i2c.h
+++ b/include/i2c.h
@@ -16,6 +16,9 @@
#ifndef _I2C_H_
#define _I2C_H_
+/* Stores maximum highest i2c alias number */
+extern int i2c_highest_id;
+
/*
* For now there are essentially two parts to this file - driver model
* here at the top, and the older code below (with CONFIG_SYS_I2C being
--
1.9.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH 4/4] i2c: mux: Add support for not listed sub-buses
2019-01-18 15:13 [U-Boot] [PATCH 0/4] Align U-Boot I2C DM bus ID handling with Linux Michal Simek
` (2 preceding siblings ...)
2019-01-18 15:13 ` [U-Boot] [PATCH 3/4] i2c: dm: Record maximum id of devices before probing devices Michal Simek
@ 2019-01-18 15:13 ` Michal Simek
2019-01-24 8:27 ` Michal Simek
2019-01-31 10:04 ` Simon Glass
2019-01-18 15:24 ` [U-Boot] [PATCH 0/4] Align U-Boot I2C DM bus ID handling with Linux Michal Simek
4 siblings, 2 replies; 19+ messages in thread
From: Michal Simek @ 2019-01-18 15:13 UTC (permalink / raw)
To: u-boot
Before this patch is applied all i2c sub-buses are using number -1 and
they can't be addresses(switch to). If all busses are listed in DT alias
they will get proper numbers and U-Boot can work with them.
In Linux buses which are not listed in DT alias get uniq number which
starts from the first highest free ID.
This is the behavior on ZynqMP zcu100-revA before this patch is applied.
Bus 0: i2c at ff020000
20: gpio at 20, offset len 1, flags 0
21: gpio at 21, offset len 1, flags 0
75: i2c-mux at 75, offset len 1, flags 0
Bus -1: i2c at 0
Bus -1: i2c at 1
Bus -1: i2c at 2
Bus 1: i2c at ff030000
74: i2c-mux at 74, offset len 1, flags 0
75: i2c-mux at 75, offset len 1, flags 0
Bus -1: i2c at 0
Bus -1: i2c at 1
Bus -1: i2c at 2
Bus -1: i2c at 3
Bus -1: i2c at 4
Bus -1: i2c at 0
Bus -1: i2c at 1
Bus -1: i2c at 2
Bus -1: i2c at 3
Bus -1: i2c at 4
Bus -1: i2c at 5
Bus -1: i2c at 6
Bus -1: i2c at 7
When the patch is applied also i2c-mux busses are listed properly.
ZynqMP> i2c bus
Bus 0: i2c at ff020000
20: gpio at 20, offset len 1, flags 0
21: gpio at 21, offset len 1, flags 0
75: i2c-mux at 75, offset len 1, flags 0
Bus 2: i2c at ff020000->i2c-mux at 75->i2c at 0
Bus 3: i2c at ff020000->i2c-mux at 75->i2c at 1
Bus 4: i2c at ff020000->i2c-mux at 75->i2c at 2
Bus 1: i2c at ff030000 (active 1)
74: i2c-mux at 74, offset len 1, flags 0
75: i2c-mux at 75, offset len 1, flags 0
Bus 5: i2c at ff030000->i2c-mux at 74->i2c at 0 (active 5)
54: generic_54, offset len 1, flags 0
Bus 6: i2c at ff030000->i2c-mux at 74->i2c at 1
Bus 7: i2c at ff030000->i2c-mux at 74->i2c at 2
Bus 8: i2c at ff030000->i2c-mux at 74->i2c at 3
Bus 9: i2c at ff030000->i2c-mux at 74->i2c at 4
Bus 10: i2c at ff030000->i2c-mux at 75->i2c at 0
Bus 11: i2c at ff030000->i2c-mux at 75->i2c at 1
Bus 12: i2c at ff030000->i2c-mux at 75->i2c at 2
Bus 13: i2c at ff030000->i2c-mux at 75->i2c at 3
Bus 14: i2c at ff030000->i2c-mux at 75->i2c at 4
Bus 15: i2c at ff030000->i2c-mux at 75->i2c at 5
Bus 16: i2c at ff030000->i2c-mux at 75->i2c at 6
Bus 17: i2c at ff030000->i2c-mux at 75->i2c at 7
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
zcu102-revA
Before this patch applied for !DM case with static description
ZynqMP> i2c bus
Bus 0: zynq_0
Bus 1: zynq_0->PCA9544A at 0x75:0
Bus 2: zynq_0->PCA9544A at 0x75:1
Bus 3: zynq_0->PCA9544A at 0x75:2
Bus 4: zynq_1
Bus 5: zynq_1->PCA9548 at 0x74:0
Bus 6: zynq_1->PCA9548 at 0x74:1
Bus 7: zynq_1->PCA9548 at 0x74:2
Bus 8: zynq_1->PCA9548 at 0x74:3
Bus 9: zynq_1->PCA9548 at 0x74:4
Bus 10: zynq_1->PCA9548 at 0x75:0
Bus 11: zynq_1->PCA9548 at 0x75:1
Bus 12: zynq_1->PCA9548 at 0x75:2
Bus 13: zynq_1->PCA9548 at 0x75:3
Bus 14: zynq_1->PCA9548 at 0x75:4
Bus 15: zynq_1->PCA9548 at 0x75:5
Bus 16: zynq_1->PCA9548 at 0x75:6
Bus 17: zynq_1->PCA9548 at 0x75:7
When Patch is applied with OF_LIVE - of_alias_get_highest_id() is used
ZynqMP> i2c bus
Bus 0: i2c at ff020000
75: i2c-mux at 75, offset len 1, flags 0
Bus 2: i2c at ff020000->i2c-mux at 75->i2c at 0
Bus 3: i2c at ff020000->i2c-mux at 75->i2c at 1
Bus 4: i2c at ff020000->i2c-mux at 75->i2c at 2
Bus 1: i2c at ff030000
74: i2c-mux at 74, offset len 1, flags 0
75: i2c-mux at 75, offset len 1, flags 0
Bus 5: i2c at ff030000->i2c-mux at 74->i2c at 0
Bus 6: i2c at ff030000->i2c-mux at 74->i2c at 1
Bus 7: i2c at ff030000->i2c-mux at 74->i2c at 2
Bus 8: i2c at ff030000->i2c-mux at 74->i2c at 3
Bus 9: i2c at ff030000->i2c-mux at 74->i2c at 4
Bus 10: i2c at ff030000->i2c-mux at 75->i2c at 0
Bus 11: i2c at ff030000->i2c-mux at 75->i2c at 1
Bus 12: i2c at ff030000->i2c-mux at 75->i2c at 2
Bus 13: i2c at ff030000->i2c-mux at 75->i2c at 3
Bus 14: i2c at ff030000->i2c-mux at 75->i2c at 4
Bus 15: i2c at ff030000->i2c-mux at 75->i2c at 5
Bus 16: i2c at ff030000->i2c-mux at 75->i2c at 6
Bus 17: i2c at ff030000->i2c-mux at 75->i2c at 7
For !OF_LIVE - hardcoded number is used
ZynqMP> i2c bus
Bus 0: i2c at ff020000
75: i2c-mux at 75, offset len 1, flags 0
Bus 21: i2c at ff020000->i2c-mux at 75->i2c at 0
Bus 22: i2c at ff020000->i2c-mux at 75->i2c at 1
Bus 23: i2c at ff020000->i2c-mux at 75->i2c at 2
Bus 1: i2c at ff030000
74: i2c-mux at 74, offset len 1, flags 0
75: i2c-mux at 75, offset len 1, flags 0
Bus 24: i2c at ff030000->i2c-mux at 74->i2c at 0
Bus 25: i2c at ff030000->i2c-mux at 74->i2c at 1
Bus 26: i2c at ff030000->i2c-mux at 74->i2c at 2
Bus 27: i2c at ff030000->i2c-mux at 74->i2c at 3
Bus 28: i2c at ff030000->i2c-mux at 74->i2c at 4
Bus 29: i2c at ff030000->i2c-mux at 75->i2c at 0
Bus 30: i2c at ff030000->i2c-mux at 75->i2c at 1
Bus 31: i2c at ff030000->i2c-mux at 75->i2c at 2
Bus 32: i2c at ff030000->i2c-mux at 75->i2c at 3
Bus 33: i2c at ff030000->i2c-mux at 75->i2c at 4
Bus 34: i2c at ff030000->i2c-mux at 75->i2c at 5
Bus 35: i2c at ff030000->i2c-mux at 75->i2c at 6
Bus 36: i2c at ff030000->i2c-mux at 75->i2c at 7
---
drivers/i2c/muxes/i2c-mux-uclass.c | 35 +++++++++++++++++++++++++++++++++--
1 file changed, 33 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-mux-uclass.c b/drivers/i2c/muxes/i2c-mux-uclass.c
index a680ee176253..cb69d053fd59 100644
--- a/drivers/i2c/muxes/i2c-mux-uclass.c
+++ b/drivers/i2c/muxes/i2c-mux-uclass.c
@@ -9,6 +9,7 @@
#include <errno.h>
#include <i2c.h>
#include <dm/lists.h>
+#include <dm/of_access.h>
#include <dm/root.h>
/**
@@ -59,13 +60,42 @@ static int i2c_mux_post_bind(struct udevice *mux)
dev_for_each_subnode(node, mux) {
struct udevice *dev;
const char *name;
+ const char *arrow = "->";
+ char *full_name;
+ int parent_name_len, arrow_len, mux_name_len, name_len;
name = ofnode_get_name(node);
- ret = device_bind_driver_to_node(mux, "i2c_mux_bus_drv", name,
- node, &dev);
+
+ /* Calculate lenghts of strings */
+ parent_name_len = strlen(mux->parent->name);
+ arrow_len = strlen(arrow);
+ mux_name_len = strlen(mux->name);
+ name_len = strlen(name);
+
+ full_name = calloc(1, parent_name_len + arrow_len +
+ mux_name_len + arrow_len + name_len + 1);
+ if (!full_name)
+ return -ENOMEM;
+
+ /* Compose bus name */
+ strcat(full_name, mux->parent->name);
+ strcat(full_name, arrow);
+ strcat(full_name, mux->name);
+ strcat(full_name, arrow);
+ strcat(full_name, name);
+
+ ret = device_bind_driver_to_node(mux, "i2c_mux_bus_drv",
+ full_name, node, &dev);
debug(" - bind ret=%d, %s\n", ret, dev ? dev->name : NULL);
if (ret)
return ret;
+
+ /* If dt alias is not found start to allocate new IDs */
+ if (dev->req_seq == -1)
+ dev->req_seq = ++i2c_highest_id;
+
+ debug("%s: MUX bus %d: %s\n", __func__, dev->req_seq,
+ full_name);
}
return 0;
@@ -192,6 +222,7 @@ U_BOOT_DRIVER(i2c_mux_bus) = {
UCLASS_DRIVER(i2c_mux) = {
.id = UCLASS_I2C_MUX,
+ .flags = DM_UC_FLAG_SEQ_ALIAS,
.name = "i2c_mux",
.post_bind = i2c_mux_post_bind,
.post_probe = i2c_mux_post_probe,
--
1.9.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH 0/4] Align U-Boot I2C DM bus ID handling with Linux
2019-01-18 15:13 [U-Boot] [PATCH 0/4] Align U-Boot I2C DM bus ID handling with Linux Michal Simek
` (3 preceding siblings ...)
2019-01-18 15:13 ` [U-Boot] [PATCH 4/4] i2c: mux: Add support for not listed sub-buses Michal Simek
@ 2019-01-18 15:24 ` Michal Simek
4 siblings, 0 replies; 19+ messages in thread
From: Michal Simek @ 2019-01-18 15:24 UTC (permalink / raw)
To: u-boot
On 18. 01. 19 16:13, Michal Simek wrote:
> U-Boot with I2C_DM enabled is not capable to list i2c busses connected
> to i2c mux. For getting this work there is a need to find out highest
> alias ID and use this uniq number for new buses connected to I2C mux.
> This series is making this happen.
>
> There is only one missing piece which is that also i2c controllers which
> are not listed in DT are not using this feature.
> For getting this work it would be necessary to setup dev->req_seq in
> pre_bind function.
>
> int i2c_pre_bind(struct udevice *dev)
> {
> if (dev->req_seq == -1) {
> dev->req_seq = ++i2c_highest_id;
> }
> return 0;
> }
>
> It can be done also in post_bind but it looks quite weird that
> controller has highest ID then i2c-mux sub-buses connected to it.
> For example:
> ZynqMP> i2c bus
> Bus 3: i2c at ff020000
> 20: gpio at 20, offset len 1, flags 0
> 21: gpio at 21, offset len 1, flags 0
> 75: i2c-mux at 75, offset len 1, flags 0
> Bus 0: i2c at ff020000->i2c-mux at 75->i2c at 0
> Bus 1: i2c at ff020000->i2c-mux at 75->i2c at 1
> Bus 2: i2c at ff020000->i2c-mux at 75->i2c at 2
> Bus 17: i2c at ff030000
> 74: i2c-mux at 74, offset len 1, flags 0
> 75: i2c-mux at 75, offset len 1, flags 0
> Bus 4: i2c at ff030000->i2c-mux at 74->i2c at 0
> Bus 5: i2c at ff030000->i2c-mux at 74->i2c at 1
> Bus 6: i2c at ff030000->i2c-mux at 74->i2c at 2
> Bus 7: i2c at ff030000->i2c-mux at 74->i2c at 3
> ...
>
> That's why I think it would be better to introduce pre_bind hooks and
> call it in device_bind_common() before binding childs.
I have retested again and this is working properly.
This is the patch.
diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
index aaece115f02f..8430a23eece7 100644
--- a/drivers/i2c/i2c-uclass.c
+++ b/drivers/i2c/i2c-uclass.c
@@ -637,14 +637,28 @@ int i2c_uclass_init(struct uclass *class)
return 0;
}
+static int i2c_post_bind(struct udevice *dev)
+{
+ int ret = 0;
+
+ debug("%s %d\n", __func__, dev->req_seq);
+
+ if (dev->req_seq == -1)
+ dev->req_seq = ++i2c_highest_id;
+
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
+ ret = dm_scan_fdt_dev(dev);
+#endif
+
+ return ret;
+}
+
UCLASS_DRIVER(i2c) = {
.id = UCLASS_I2C,
.name = "i2c",
.flags = DM_UC_FLAG_SEQ_ALIAS,
.init = i2c_uclass_init,
-#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
- .post_bind = dm_scan_fdt_dev,
-#endif
+ .post_bind = i2c_post_bind,
.post_probe = i2c_post_probe,
.per_device_auto_alloc_size = sizeof(struct dm_i2c_bus),
.per_child_platdata_auto_alloc_size = sizeof(struct dm_i2c_chip),
And this is the output
ZynqMP> i2c bus
Bus 0: i2c at ff020000
20: gpio at 20, offset len 1, flags 0
21: gpio at 21, offset len 1, flags 0
75: i2c-mux at 75, offset len 1, flags 0
Bus 1: i2c at ff020000->i2c-mux at 75->i2c at 0
Bus 2: i2c at ff020000->i2c-mux at 75->i2c at 1
Bus 3: i2c at ff020000->i2c-mux at 75->i2c at 2
Bus 4: i2c at ff030000
74: i2c-mux at 74, offset len 1, flags 0
75: i2c-mux at 75, offset len 1, flags 0
Bus 5: i2c at ff030000->i2c-mux at 74->i2c at 0
Bus 6: i2c at ff030000->i2c-mux at 74->i2c at 1
Bus 7: i2c at ff030000->i2c-mux at 74->i2c at 2
Bus 8: i2c at ff030000->i2c-mux at 74->i2c at 3
Bus 9: i2c at ff030000->i2c-mux at 74->i2c at 4
Bus 10: i2c at ff030000->i2c-mux at 75->i2c at 0
Bus 11: i2c at ff030000->i2c-mux at 75->i2c at 1
Bus 12: i2c at ff030000->i2c-mux at 75->i2c at 2
Bus 13: i2c at ff030000->i2c-mux at 75->i2c at 3
Bus 14: i2c at ff030000->i2c-mux at 75->i2c at 4
Bus 15: i2c at ff030000->i2c-mux at 75->i2c at 5
Bus 16: i2c at ff030000->i2c-mux at 75->i2c at 6
Bus 17: i2c at ff030000->i2c-mux at 75->i2c at 7
Without this fix i2c controllers have only -1 because they are not
listed in DT.
I will send this as a patch when this is all reviewed.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190118/e0fdccdf/attachment.sig>
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH 4/4] i2c: mux: Add support for not listed sub-buses
2019-01-18 15:13 ` [U-Boot] [PATCH 4/4] i2c: mux: Add support for not listed sub-buses Michal Simek
@ 2019-01-24 8:27 ` Michal Simek
2019-01-31 10:04 ` Simon Glass
1 sibling, 0 replies; 19+ messages in thread
From: Michal Simek @ 2019-01-24 8:27 UTC (permalink / raw)
To: u-boot
Hi Heiko,
On 18. 01. 19 16:13, Michal Simek wrote:
> Before this patch is applied all i2c sub-buses are using number -1 and
> they can't be addresses(switch to). If all busses are listed in DT alias
> they will get proper numbers and U-Boot can work with them.
> In Linux buses which are not listed in DT alias get uniq number which
> starts from the first highest free ID.
>
> This is the behavior on ZynqMP zcu100-revA before this patch is applied.
>
> Bus 0: i2c at ff020000
> 20: gpio at 20, offset len 1, flags 0
> 21: gpio at 21, offset len 1, flags 0
> 75: i2c-mux at 75, offset len 1, flags 0
> Bus -1: i2c at 0
> Bus -1: i2c at 1
> Bus -1: i2c at 2
> Bus 1: i2c at ff030000
> 74: i2c-mux at 74, offset len 1, flags 0
> 75: i2c-mux at 75, offset len 1, flags 0
> Bus -1: i2c at 0
> Bus -1: i2c at 1
> Bus -1: i2c at 2
> Bus -1: i2c at 3
> Bus -1: i2c at 4
> Bus -1: i2c at 0
> Bus -1: i2c at 1
> Bus -1: i2c at 2
> Bus -1: i2c at 3
> Bus -1: i2c at 4
> Bus -1: i2c at 5
> Bus -1: i2c at 6
> Bus -1: i2c at 7
>
> When the patch is applied also i2c-mux busses are listed properly.
>
> ZynqMP> i2c bus
> Bus 0: i2c at ff020000
> 20: gpio at 20, offset len 1, flags 0
> 21: gpio at 21, offset len 1, flags 0
> 75: i2c-mux at 75, offset len 1, flags 0
> Bus 2: i2c at ff020000->i2c-mux at 75->i2c at 0
> Bus 3: i2c at ff020000->i2c-mux at 75->i2c at 1
> Bus 4: i2c at ff020000->i2c-mux at 75->i2c at 2
> Bus 1: i2c at ff030000 (active 1)
> 74: i2c-mux at 74, offset len 1, flags 0
> 75: i2c-mux at 75, offset len 1, flags 0
> Bus 5: i2c at ff030000->i2c-mux at 74->i2c at 0 (active 5)
> 54: generic_54, offset len 1, flags 0
> Bus 6: i2c at ff030000->i2c-mux at 74->i2c at 1
> Bus 7: i2c at ff030000->i2c-mux at 74->i2c at 2
> Bus 8: i2c at ff030000->i2c-mux at 74->i2c at 3
> Bus 9: i2c at ff030000->i2c-mux at 74->i2c at 4
> Bus 10: i2c at ff030000->i2c-mux at 75->i2c at 0
> Bus 11: i2c at ff030000->i2c-mux at 75->i2c at 1
> Bus 12: i2c at ff030000->i2c-mux at 75->i2c at 2
> Bus 13: i2c at ff030000->i2c-mux at 75->i2c at 3
> Bus 14: i2c at ff030000->i2c-mux at 75->i2c at 4
> Bus 15: i2c at ff030000->i2c-mux at 75->i2c at 5
> Bus 16: i2c at ff030000->i2c-mux at 75->i2c at 6
> Bus 17: i2c at ff030000->i2c-mux at 75->i2c at 7
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
> zcu102-revA
>
> Before this patch applied for !DM case with static description
> ZynqMP> i2c bus
> Bus 0: zynq_0
> Bus 1: zynq_0->PCA9544A at 0x75:0
> Bus 2: zynq_0->PCA9544A at 0x75:1
> Bus 3: zynq_0->PCA9544A at 0x75:2
> Bus 4: zynq_1
> Bus 5: zynq_1->PCA9548 at 0x74:0
> Bus 6: zynq_1->PCA9548 at 0x74:1
> Bus 7: zynq_1->PCA9548 at 0x74:2
> Bus 8: zynq_1->PCA9548 at 0x74:3
> Bus 9: zynq_1->PCA9548 at 0x74:4
> Bus 10: zynq_1->PCA9548 at 0x75:0
> Bus 11: zynq_1->PCA9548 at 0x75:1
> Bus 12: zynq_1->PCA9548 at 0x75:2
> Bus 13: zynq_1->PCA9548 at 0x75:3
> Bus 14: zynq_1->PCA9548 at 0x75:4
> Bus 15: zynq_1->PCA9548 at 0x75:5
> Bus 16: zynq_1->PCA9548 at 0x75:6
> Bus 17: zynq_1->PCA9548 at 0x75:7
>
> When Patch is applied with OF_LIVE - of_alias_get_highest_id() is used
> ZynqMP> i2c bus
> Bus 0: i2c at ff020000
> 75: i2c-mux at 75, offset len 1, flags 0
> Bus 2: i2c at ff020000->i2c-mux at 75->i2c at 0
> Bus 3: i2c at ff020000->i2c-mux at 75->i2c at 1
> Bus 4: i2c at ff020000->i2c-mux at 75->i2c at 2
> Bus 1: i2c at ff030000
> 74: i2c-mux at 74, offset len 1, flags 0
> 75: i2c-mux at 75, offset len 1, flags 0
> Bus 5: i2c at ff030000->i2c-mux at 74->i2c at 0
> Bus 6: i2c at ff030000->i2c-mux at 74->i2c at 1
> Bus 7: i2c at ff030000->i2c-mux at 74->i2c at 2
> Bus 8: i2c at ff030000->i2c-mux at 74->i2c at 3
> Bus 9: i2c at ff030000->i2c-mux at 74->i2c at 4
> Bus 10: i2c at ff030000->i2c-mux at 75->i2c at 0
> Bus 11: i2c at ff030000->i2c-mux at 75->i2c at 1
> Bus 12: i2c at ff030000->i2c-mux at 75->i2c at 2
> Bus 13: i2c at ff030000->i2c-mux at 75->i2c at 3
> Bus 14: i2c at ff030000->i2c-mux at 75->i2c at 4
> Bus 15: i2c at ff030000->i2c-mux at 75->i2c at 5
> Bus 16: i2c at ff030000->i2c-mux at 75->i2c at 6
> Bus 17: i2c at ff030000->i2c-mux at 75->i2c at 7
>
> For !OF_LIVE - hardcoded number is used
> ZynqMP> i2c bus
> Bus 0: i2c at ff020000
> 75: i2c-mux at 75, offset len 1, flags 0
> Bus 21: i2c at ff020000->i2c-mux at 75->i2c at 0
> Bus 22: i2c at ff020000->i2c-mux at 75->i2c at 1
> Bus 23: i2c at ff020000->i2c-mux at 75->i2c at 2
> Bus 1: i2c at ff030000
> 74: i2c-mux at 74, offset len 1, flags 0
> 75: i2c-mux at 75, offset len 1, flags 0
> Bus 24: i2c at ff030000->i2c-mux at 74->i2c at 0
> Bus 25: i2c at ff030000->i2c-mux at 74->i2c at 1
> Bus 26: i2c at ff030000->i2c-mux at 74->i2c at 2
> Bus 27: i2c at ff030000->i2c-mux at 74->i2c at 3
> Bus 28: i2c at ff030000->i2c-mux at 74->i2c at 4
> Bus 29: i2c at ff030000->i2c-mux at 75->i2c at 0
> Bus 30: i2c at ff030000->i2c-mux at 75->i2c at 1
> Bus 31: i2c at ff030000->i2c-mux at 75->i2c at 2
> Bus 32: i2c at ff030000->i2c-mux at 75->i2c at 3
> Bus 33: i2c at ff030000->i2c-mux at 75->i2c at 4
> Bus 34: i2c at ff030000->i2c-mux at 75->i2c at 5
> Bus 35: i2c at ff030000->i2c-mux at 75->i2c at 6
> Bus 36: i2c at ff030000->i2c-mux at 75->i2c at 7
>
> ---
> drivers/i2c/muxes/i2c-mux-uclass.c | 35 +++++++++++++++++++++++++++++++++--
> 1 file changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/muxes/i2c-mux-uclass.c b/drivers/i2c/muxes/i2c-mux-uclass.c
> index a680ee176253..cb69d053fd59 100644
> --- a/drivers/i2c/muxes/i2c-mux-uclass.c
> +++ b/drivers/i2c/muxes/i2c-mux-uclass.c
> @@ -9,6 +9,7 @@
> #include <errno.h>
> #include <i2c.h>
> #include <dm/lists.h>
> +#include <dm/of_access.h>
> #include <dm/root.h>
>
> /**
> @@ -59,13 +60,42 @@ static int i2c_mux_post_bind(struct udevice *mux)
> dev_for_each_subnode(node, mux) {
> struct udevice *dev;
> const char *name;
> + const char *arrow = "->";
> + char *full_name;
> + int parent_name_len, arrow_len, mux_name_len, name_len;
>
> name = ofnode_get_name(node);
> - ret = device_bind_driver_to_node(mux, "i2c_mux_bus_drv", name,
> - node, &dev);
> +
> + /* Calculate lenghts of strings */
> + parent_name_len = strlen(mux->parent->name);
> + arrow_len = strlen(arrow);
> + mux_name_len = strlen(mux->name);
> + name_len = strlen(name);
> +
> + full_name = calloc(1, parent_name_len + arrow_len +
> + mux_name_len + arrow_len + name_len + 1);
> + if (!full_name)
> + return -ENOMEM;
> +
> + /* Compose bus name */
> + strcat(full_name, mux->parent->name);
> + strcat(full_name, arrow);
> + strcat(full_name, mux->name);
> + strcat(full_name, arrow);
> + strcat(full_name, name);
> +
> + ret = device_bind_driver_to_node(mux, "i2c_mux_bus_drv",
> + full_name, node, &dev);
> debug(" - bind ret=%d, %s\n", ret, dev ? dev->name : NULL);
> if (ret)
> return ret;
> +
> + /* If dt alias is not found start to allocate new IDs */
> + if (dev->req_seq == -1)
> + dev->req_seq = ++i2c_highest_id;
> +
> + debug("%s: MUX bus %d: %s\n", __func__, dev->req_seq,
> + full_name);
> }
>
> return 0;
> @@ -192,6 +222,7 @@ U_BOOT_DRIVER(i2c_mux_bus) = {
>
> UCLASS_DRIVER(i2c_mux) = {
> .id = UCLASS_I2C_MUX,
> + .flags = DM_UC_FLAG_SEQ_ALIAS,
> .name = "i2c_mux",
> .post_bind = i2c_mux_post_bind,
> .post_probe = i2c_mux_post_probe,
>
Can you please take a look at this series?
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190124/0260e2a4/attachment.sig>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH 1/4] dm: core: Add of_alias_get_highest_id()
2019-01-18 15:13 ` [U-Boot] [PATCH 1/4] dm: core: Add of_alias_get_highest_id() Michal Simek
@ 2019-01-28 12:35 ` Michal Simek
2019-01-31 10:04 ` Simon Glass
1 sibling, 0 replies; 19+ messages in thread
From: Michal Simek @ 2019-01-28 12:35 UTC (permalink / raw)
To: u-boot
Hi Simon,
On 18. 01. 19 16:13, Michal Simek wrote:
> The same functionality was added to Linux for i2c bus registration with this
> commit message:
>
> "
> of: base: add function to get highest id of an alias stem
>
> I2C supports adding adapters using either a dynamic or fixed id. The
> latter is provided by aliases in the DT case. To prevent id collisions
> of those two types, install this function which gives us the highest
> fixed id, so we can then let the dynamically created ones come after
> this highest number.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> "
>
> Add it also to U-Boot for DM I2C support.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
> drivers/core/of_access.c | 18 ++++++++++++++++++
> include/dm/of_access.h | 9 +++++++++
> 2 files changed, 27 insertions(+)
>
> diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c
> index 14c020a687b7..7c2df2354109 100644
> --- a/drivers/core/of_access.c
> +++ b/drivers/core/of_access.c
> @@ -812,6 +812,24 @@ int of_alias_get_id(const struct device_node *np, const char *stem)
> return id;
> }
>
> +int of_alias_get_highest_id(const char *stem)
> +{
> + struct alias_prop *app;
> + int id = -ENODEV;
> +
> + mutex_lock(&of_mutex);
> + list_for_each_entry(app, &aliases_lookup, link) {
> + if (strcmp(app->stem, stem) != 0)
> + continue;
> +
> + if (app->id > id)
> + id = app->id;
> + }
> + mutex_unlock(&of_mutex);
> +
> + return id;
> +}
> +
> struct device_node *of_get_stdout(void)
> {
> return of_stdout;
> diff --git a/include/dm/of_access.h b/include/dm/of_access.h
> index 5ed1a0cdb427..5cbfd220bfd4 100644
> --- a/include/dm/of_access.h
> +++ b/include/dm/of_access.h
> @@ -425,6 +425,15 @@ int of_alias_scan(void);
> int of_alias_get_id(const struct device_node *np, const char *stem);
>
> /**
> + * of_alias_get_highest_id - Get highest alias id for the given stem
> + * @stem: Alias stem to be examined
> + *
> + * The function travels the lookup table to get the highest alias id for the
> + * given alias stem. It returns the alias id if found.
> + */
> +int of_alias_get_highest_id(const char *stem);
> +
> +/**
> * of_get_stdout() - Get node to use for stdout
> *
> * @return node referred to by stdout-path alias, or NULL if none
>
Can you please review 1/4 and 2/4 patches?
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190128/b9f01382/attachment.sig>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH 1/4] dm: core: Add of_alias_get_highest_id()
2019-01-18 15:13 ` [U-Boot] [PATCH 1/4] dm: core: Add of_alias_get_highest_id() Michal Simek
2019-01-28 12:35 ` Michal Simek
@ 2019-01-31 10:04 ` Simon Glass
2019-01-31 10:19 ` Michal Simek
1 sibling, 1 reply; 19+ messages in thread
From: Simon Glass @ 2019-01-31 10:04 UTC (permalink / raw)
To: u-boot
Hi Michal,
On Fri, 18 Jan 2019 at 08:13, Michal Simek <michal.simek@xilinx.com> wrote:
>
> The same functionality was added to Linux for i2c bus registration with this
> commit message:
>
> "
> of: base: add function to get highest id of an alias stem
>
> I2C supports adding adapters using either a dynamic or fixed id. The
> latter is provided by aliases in the DT case. To prevent id collisions
> of those two types, install this function which gives us the highest
> fixed id, so we can then let the dynamically created ones come after
> this highest number.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> "
>
> Add it also to U-Boot for DM I2C support.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
> drivers/core/of_access.c | 18 ++++++++++++++++++
> include/dm/of_access.h | 9 +++++++++
> 2 files changed, 27 insertions(+)
>
> diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c
> index 14c020a687b7..7c2df2354109 100644
> --- a/drivers/core/of_access.c
> +++ b/drivers/core/of_access.c
> @@ -812,6 +812,24 @@ int of_alias_get_id(const struct device_node *np, const char *stem)
> return id;
> }
>
> +int of_alias_get_highest_id(const char *stem)
> +{
> + struct alias_prop *app;
> + int id = -ENODEV;
> +
> + mutex_lock(&of_mutex);
> + list_for_each_entry(app, &aliases_lookup, link) {
> + if (strcmp(app->stem, stem) != 0)
> + continue;
> +
> + if (app->id > id)
> + id = app->id;
> + }
> + mutex_unlock(&of_mutex);
> +
> + return id;
> +}
> +
> struct device_node *of_get_stdout(void)
> {
> return of_stdout;
> diff --git a/include/dm/of_access.h b/include/dm/of_access.h
> index 5ed1a0cdb427..5cbfd220bfd4 100644
> --- a/include/dm/of_access.h
> +++ b/include/dm/of_access.h
> @@ -425,6 +425,15 @@ int of_alias_scan(void);
> int of_alias_get_id(const struct device_node *np, const char *stem);
>
> /**
> + * of_alias_get_highest_id - Get highest alias id for the given stem
> + * @stem: Alias stem to be examined
> + *
> + * The function travels the lookup table to get the highest alias id for the
> + * given alias stem. It returns the alias id if found.
@return
> + */
> +int of_alias_get_highest_id(const char *stem);
> +
> +/**
> * of_get_stdout() - Get node to use for stdout
> *
> * @return node referred to by stdout-path alias, or NULL if none
> --
> 1.9.1
>
Can we place have a test that calls this for a few values?
Also see uclass_find_next_free_req_seq() and uclass_resolve_seq()
which is what U-Boot does today. It is first in, first served. This is
supposed to mean that devices without an alias don't get a req_seq
value, meaning that the seq value is set to the next available value.
I suspect it is OK to change the behaviour though. It might affect
some boards which rely on not having aliases but still getting bus
numbers.
Regards,
Simon
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH 2/4] fdt: Introduce fdtdec_get_alias_highest_id()
2019-01-18 15:13 ` [U-Boot] [PATCH 2/4] fdt: Introduce fdtdec_get_alias_highest_id() Michal Simek
@ 2019-01-31 10:04 ` Simon Glass
2019-01-31 10:14 ` Michal Simek
0 siblings, 1 reply; 19+ messages in thread
From: Simon Glass @ 2019-01-31 10:04 UTC (permalink / raw)
To: u-boot
Hi Michal,
On Fri, 18 Jan 2019 at 08:13, Michal Simek <michal.simek@xilinx.com> wrote:
>
> Find out the highest alias ID used for certain subsystem.
> This call will be used for alocating IDs for i2c buses which are not
> described in DT.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
> include/fdtdec.h | 13 +++++++++++++
> lib/fdtdec.c | 33 +++++++++++++++++++++++++++++++++
> 2 files changed, 46 insertions(+)
>
> diff --git a/include/fdtdec.h b/include/fdtdec.h
> index f1bcbf837ffb..c2dd87ede226 100644
> --- a/include/fdtdec.h
> +++ b/include/fdtdec.h
> @@ -626,6 +626,19 @@ int fdtdec_get_alias_seq(const void *blob, const char *base, int node,
> int *seqp);
>
> /**
> + * Get the highest alias number for susbystem.
> + *
> + * It parses all aliases and find out highest recorded alias for subsystem.
> + * Aliases are of the form <base><num> where <num> is the sequence number.
> + *
> + * @param blob Device tree blob (if NULL, then error is returned)
> + * @param base Base name for alias susbystem (before the number)
> + *
> + * @return 0 highest alias ID, -1 if not found
> + */
> +int fdtdec_get_alias_highest_id(const void *blob, const char *base);
> +
> +/**
> * Get a property from the /chosen node
> *
> * @param blob Device tree blob (if NULL, then NULL is returned)
> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> index 18663ce6bdac..55811975ef54 100644
> --- a/lib/fdtdec.c
> +++ b/lib/fdtdec.c
> @@ -549,6 +549,39 @@ int fdtdec_get_alias_seq(const void *blob, const char *base, int offset,
> return -ENOENT;
> }
>
> +int fdtdec_get_alias_highest_id(const void *blob, const char *base)
> +{
> + int base_len = strlen(base);
> + int prop_offset;
> + int aliases;
> + int max = -1;
> +
> + debug("Looking for highest alias id for '%s'\n", base);
> +
> + aliases = fdt_path_offset(blob, "/aliases");
> + for (prop_offset = fdt_first_property_offset(blob, aliases);
> + prop_offset > 0;
> + prop_offset = fdt_next_property_offset(blob, prop_offset)) {
> + const char *prop;
> + const char *name;
> + int len, val;
> +
> + prop = fdt_getprop_by_offset(blob, prop_offset, &name, &len);
> + debug(" - %s, %s\n", name, prop);
> + if (*prop != '/' || prop[len - 1] ||
> + strncmp(name, base, base_len))
> + continue;
> +
> + val = trailing_strtol(name);
> + if (val > max) {
> + debug("Found seq %d\n", val);
> + max = val;
> + }
> + }
> +
> + return max;
This looks right to me. Can you please add a test that calls this for
a few sandbox aliases?
> +}
> +
> const char *fdtdec_get_chosen_prop(const void *blob, const char *name)
> {
> int chosen_node;
> --
> 1.9.1
>
Regards,
Simon
On Fri, 18 Jan 2019 at 08:13, Michal Simek <michal.simek@xilinx.com> wrote:
>
> Find out the highest alias ID used for certain subsystem.
> This call will be used for alocating IDs for i2c buses which are not
> described in DT.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
> include/fdtdec.h | 13 +++++++++++++
> lib/fdtdec.c | 33 +++++++++++++++++++++++++++++++++
> 2 files changed, 46 insertions(+)
>
> diff --git a/include/fdtdec.h b/include/fdtdec.h
> index f1bcbf837ffb..c2dd87ede226 100644
> --- a/include/fdtdec.h
> +++ b/include/fdtdec.h
> @@ -626,6 +626,19 @@ int fdtdec_get_alias_seq(const void *blob, const char *base, int node,
> int *seqp);
>
> /**
> + * Get the highest alias number for susbystem.
> + *
> + * It parses all aliases and find out highest recorded alias for subsystem.
> + * Aliases are of the form <base><num> where <num> is the sequence number.
> + *
> + * @param blob Device tree blob (if NULL, then error is returned)
> + * @param base Base name for alias susbystem (before the number)
> + *
> + * @return 0 highest alias ID, -1 if not found
> + */
> +int fdtdec_get_alias_highest_id(const void *blob, const char *base);
> +
> +/**
> * Get a property from the /chosen node
> *
> * @param blob Device tree blob (if NULL, then NULL is returned)
> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> index 18663ce6bdac..55811975ef54 100644
> --- a/lib/fdtdec.c
> +++ b/lib/fdtdec.c
> @@ -549,6 +549,39 @@ int fdtdec_get_alias_seq(const void *blob, const char *base, int offset,
> return -ENOENT;
> }
>
> +int fdtdec_get_alias_highest_id(const void *blob, const char *base)
> +{
> + int base_len = strlen(base);
> + int prop_offset;
> + int aliases;
> + int max = -1;
> +
> + debug("Looking for highest alias id for '%s'\n", base);
> +
> + aliases = fdt_path_offset(blob, "/aliases");
> + for (prop_offset = fdt_first_property_offset(blob, aliases);
> + prop_offset > 0;
> + prop_offset = fdt_next_property_offset(blob, prop_offset)) {
> + const char *prop;
> + const char *name;
> + int len, val;
> +
> + prop = fdt_getprop_by_offset(blob, prop_offset, &name, &len);
> + debug(" - %s, %s\n", name, prop);
> + if (*prop != '/' || prop[len - 1] ||
> + strncmp(name, base, base_len))
> + continue;
> +
> + val = trailing_strtol(name);
> + if (val > max) {
> + debug("Found seq %d\n", val);
> + max = val;
> + }
> + }
> +
> + return max;
> +}
> +
> const char *fdtdec_get_chosen_prop(const void *blob, const char *name)
> {
> int chosen_node;
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH 3/4] i2c: dm: Record maximum id of devices before probing devices
2019-01-18 15:13 ` [U-Boot] [PATCH 3/4] i2c: dm: Record maximum id of devices before probing devices Michal Simek
@ 2019-01-31 10:04 ` Simon Glass
2019-01-31 11:03 ` Michal Simek
0 siblings, 1 reply; 19+ messages in thread
From: Simon Glass @ 2019-01-31 10:04 UTC (permalink / raw)
To: u-boot
Hi Michal,
On Fri, 18 Jan 2019 at 08:13, Michal Simek <michal.simek@xilinx.com> wrote:
>
> There is a need to find out the first free i2c ID which can be used for
> i2s buses (including i2c buses connected to i2c mux). Do it early in
> init and share this variable with other i2c classes for uniq bus
> identification.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
> drivers/i2c/i2c-uclass.c | 19 +++++++++++++++++++
> include/i2c.h | 3 +++
> 2 files changed, 22 insertions(+)
>
> diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
> index 975318e5f254..aaece115f02f 100644
> --- a/drivers/i2c/i2c-uclass.c
> +++ b/drivers/i2c/i2c-uclass.c
> @@ -23,6 +23,8 @@ enum {
> PIN_COUNT,
> };
>
> +int i2c_highest_id = -1;
> +
> /* Useful debugging function */
> void i2c_dump_msgs(struct i2c_msg *msg, int nmsgs)
> {
> @@ -619,10 +621,27 @@ static int i2c_child_post_bind(struct udevice *dev)
> #endif
> }
>
> +int i2c_uclass_init(struct uclass *class)
> +{
> +#ifdef CONFIG_OF_LIVE
> + int ret;
> +
> + ret = of_alias_get_highest_id("i2c");
> + if (ret >= 0)
> + i2c_highest_id = ret;
> +#else
> + i2c_highest_id = fdtdec_get_alias_highest_id(gd->fdt_blob, "i2c");
> +#endif
This should call dev_read_alias_highest_id() or similar, which does
the logic you have here.
> + debug("%s: i2c highest_id %d\n", __func__, i2c_highest_id);
> +
> + return 0;
> +}
> +
> UCLASS_DRIVER(i2c) = {
> .id = UCLASS_I2C,
> .name = "i2c",
> .flags = DM_UC_FLAG_SEQ_ALIAS,
> + .init = i2c_uclass_init,
> #if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
> .post_bind = dm_scan_fdt_dev,
> #endif
> diff --git a/include/i2c.h b/include/i2c.h
> index ccffc1955275..c69e26deaeed 100644
> --- a/include/i2c.h
> +++ b/include/i2c.h
> @@ -16,6 +16,9 @@
> #ifndef _I2C_H_
> #define _I2C_H_
>
> +/* Stores maximum highest i2c alias number */
> +extern int i2c_highest_id;
This should be in uclass-private data, not a global.
But also I feel that the behaviour needs to be consistent for all
U-Boot buses, not just I2C. So can we modify the existing core DM code
instead? I think a change to uclass_resolve_seq() might be enough.
> +
> /*
> * For now there are essentially two parts to this file - driver model
> * here at the top, and the older code below (with CONFIG_SYS_I2C being
> --
> 1.9.1
>
Regards,
Simon
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH 4/4] i2c: mux: Add support for not listed sub-buses
2019-01-18 15:13 ` [U-Boot] [PATCH 4/4] i2c: mux: Add support for not listed sub-buses Michal Simek
2019-01-24 8:27 ` Michal Simek
@ 2019-01-31 10:04 ` Simon Glass
2019-01-31 15:05 ` Michal Simek
1 sibling, 1 reply; 19+ messages in thread
From: Simon Glass @ 2019-01-31 10:04 UTC (permalink / raw)
To: u-boot
Hi Michal,
On Fri, 18 Jan 2019 at 08:13, Michal Simek <michal.simek@xilinx.com> wrote:
>
> Before this patch is applied all i2c sub-buses are using number -1 and
> they can't be addresses(switch to). If all busses are listed in DT alias
> they will get proper numbers and U-Boot can work with them.
> In Linux buses which are not listed in DT alias get uniq number which
> starts from the first highest free ID.
>
> This is the behavior on ZynqMP zcu100-revA before this patch is applied.
>
> Bus 0: i2c at ff020000
> 20: gpio at 20, offset len 1, flags 0
> 21: gpio at 21, offset len 1, flags 0
> 75: i2c-mux at 75, offset len 1, flags 0
> Bus -1: i2c at 0
> Bus -1: i2c at 1
> Bus -1: i2c at 2
> Bus 1: i2c at ff030000
> 74: i2c-mux at 74, offset len 1, flags 0
> 75: i2c-mux at 75, offset len 1, flags 0
> Bus -1: i2c at 0
> Bus -1: i2c at 1
> Bus -1: i2c at 2
> Bus -1: i2c at 3
> Bus -1: i2c at 4
> Bus -1: i2c at 0
> Bus -1: i2c at 1
> Bus -1: i2c at 2
> Bus -1: i2c at 3
> Bus -1: i2c at 4
> Bus -1: i2c at 5
> Bus -1: i2c at 6
> Bus -1: i2c at 7
>
> When the patch is applied also i2c-mux busses are listed properly.
>
> ZynqMP> i2c bus
> Bus 0: i2c at ff020000
> 20: gpio at 20, offset len 1, flags 0
> 21: gpio at 21, offset len 1, flags 0
> 75: i2c-mux at 75, offset len 1, flags 0
> Bus 2: i2c at ff020000->i2c-mux at 75->i2c at 0
> Bus 3: i2c at ff020000->i2c-mux at 75->i2c at 1
> Bus 4: i2c at ff020000->i2c-mux at 75->i2c at 2
> Bus 1: i2c at ff030000 (active 1)
> 74: i2c-mux at 74, offset len 1, flags 0
> 75: i2c-mux at 75, offset len 1, flags 0
> Bus 5: i2c at ff030000->i2c-mux at 74->i2c at 0 (active 5)
> 54: generic_54, offset len 1, flags 0
> Bus 6: i2c at ff030000->i2c-mux at 74->i2c at 1
> Bus 7: i2c at ff030000->i2c-mux at 74->i2c at 2
> Bus 8: i2c at ff030000->i2c-mux at 74->i2c at 3
> Bus 9: i2c at ff030000->i2c-mux at 74->i2c at 4
> Bus 10: i2c at ff030000->i2c-mux at 75->i2c at 0
> Bus 11: i2c at ff030000->i2c-mux at 75->i2c at 1
> Bus 12: i2c at ff030000->i2c-mux at 75->i2c at 2
> Bus 13: i2c at ff030000->i2c-mux at 75->i2c at 3
> Bus 14: i2c at ff030000->i2c-mux at 75->i2c at 4
> Bus 15: i2c at ff030000->i2c-mux at 75->i2c at 5
> Bus 16: i2c at ff030000->i2c-mux at 75->i2c at 6
> Bus 17: i2c at ff030000->i2c-mux at 75->i2c at 7
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
> zcu102-revA
>
> Before this patch applied for !DM case with static description
> ZynqMP> i2c bus
> Bus 0: zynq_0
> Bus 1: zynq_0->PCA9544A at 0x75:0
> Bus 2: zynq_0->PCA9544A at 0x75:1
> Bus 3: zynq_0->PCA9544A at 0x75:2
> Bus 4: zynq_1
> Bus 5: zynq_1->PCA9548 at 0x74:0
> Bus 6: zynq_1->PCA9548 at 0x74:1
> Bus 7: zynq_1->PCA9548 at 0x74:2
> Bus 8: zynq_1->PCA9548 at 0x74:3
> Bus 9: zynq_1->PCA9548 at 0x74:4
> Bus 10: zynq_1->PCA9548 at 0x75:0
> Bus 11: zynq_1->PCA9548 at 0x75:1
> Bus 12: zynq_1->PCA9548 at 0x75:2
> Bus 13: zynq_1->PCA9548 at 0x75:3
> Bus 14: zynq_1->PCA9548 at 0x75:4
> Bus 15: zynq_1->PCA9548 at 0x75:5
> Bus 16: zynq_1->PCA9548 at 0x75:6
> Bus 17: zynq_1->PCA9548 at 0x75:7
>
> When Patch is applied with OF_LIVE - of_alias_get_highest_id() is used
> ZynqMP> i2c bus
> Bus 0: i2c at ff020000
> 75: i2c-mux at 75, offset len 1, flags 0
> Bus 2: i2c at ff020000->i2c-mux at 75->i2c at 0
> Bus 3: i2c at ff020000->i2c-mux at 75->i2c at 1
> Bus 4: i2c at ff020000->i2c-mux at 75->i2c at 2
> Bus 1: i2c at ff030000
> 74: i2c-mux at 74, offset len 1, flags 0
> 75: i2c-mux at 75, offset len 1, flags 0
> Bus 5: i2c at ff030000->i2c-mux at 74->i2c at 0
> Bus 6: i2c at ff030000->i2c-mux at 74->i2c at 1
> Bus 7: i2c at ff030000->i2c-mux at 74->i2c at 2
> Bus 8: i2c at ff030000->i2c-mux at 74->i2c at 3
> Bus 9: i2c at ff030000->i2c-mux at 74->i2c at 4
> Bus 10: i2c at ff030000->i2c-mux at 75->i2c at 0
> Bus 11: i2c at ff030000->i2c-mux at 75->i2c at 1
> Bus 12: i2c at ff030000->i2c-mux at 75->i2c at 2
> Bus 13: i2c at ff030000->i2c-mux at 75->i2c at 3
> Bus 14: i2c at ff030000->i2c-mux at 75->i2c at 4
> Bus 15: i2c at ff030000->i2c-mux at 75->i2c at 5
> Bus 16: i2c at ff030000->i2c-mux at 75->i2c at 6
> Bus 17: i2c at ff030000->i2c-mux at 75->i2c at 7
>
> For !OF_LIVE - hardcoded number is used
> ZynqMP> i2c bus
> Bus 0: i2c at ff020000
> 75: i2c-mux at 75, offset len 1, flags 0
> Bus 21: i2c at ff020000->i2c-mux at 75->i2c at 0
> Bus 22: i2c at ff020000->i2c-mux at 75->i2c at 1
> Bus 23: i2c at ff020000->i2c-mux at 75->i2c at 2
> Bus 1: i2c at ff030000
> 74: i2c-mux at 74, offset len 1, flags 0
> 75: i2c-mux at 75, offset len 1, flags 0
> Bus 24: i2c at ff030000->i2c-mux at 74->i2c at 0
> Bus 25: i2c at ff030000->i2c-mux at 74->i2c at 1
> Bus 26: i2c at ff030000->i2c-mux at 74->i2c at 2
> Bus 27: i2c at ff030000->i2c-mux at 74->i2c at 3
> Bus 28: i2c at ff030000->i2c-mux at 74->i2c at 4
> Bus 29: i2c at ff030000->i2c-mux at 75->i2c at 0
> Bus 30: i2c at ff030000->i2c-mux at 75->i2c at 1
> Bus 31: i2c at ff030000->i2c-mux at 75->i2c at 2
> Bus 32: i2c at ff030000->i2c-mux at 75->i2c at 3
> Bus 33: i2c at ff030000->i2c-mux at 75->i2c at 4
> Bus 34: i2c at ff030000->i2c-mux at 75->i2c at 5
> Bus 35: i2c at ff030000->i2c-mux at 75->i2c at 6
> Bus 36: i2c at ff030000->i2c-mux at 75->i2c at 7
>
> ---
> drivers/i2c/muxes/i2c-mux-uclass.c | 35 +++++++++++++++++++++++++++++++++--
> 1 file changed, 33 insertions(+), 2 deletions(-)
This is quite a complicated issue...
>
> diff --git a/drivers/i2c/muxes/i2c-mux-uclass.c b/drivers/i2c/muxes/i2c-mux-uclass.c
> index a680ee176253..cb69d053fd59 100644
> --- a/drivers/i2c/muxes/i2c-mux-uclass.c
> +++ b/drivers/i2c/muxes/i2c-mux-uclass.c
> @@ -9,6 +9,7 @@
> #include <errno.h>
> #include <i2c.h>
> #include <dm/lists.h>
> +#include <dm/of_access.h>
> #include <dm/root.h>
>
> /**
> @@ -59,13 +60,42 @@ static int i2c_mux_post_bind(struct udevice *mux)
> dev_for_each_subnode(node, mux) {
> struct udevice *dev;
> const char *name;
> + const char *arrow = "->";
> + char *full_name;
> + int parent_name_len, arrow_len, mux_name_len, name_len;
>
> name = ofnode_get_name(node);
> - ret = device_bind_driver_to_node(mux, "i2c_mux_bus_drv", name,
> - node, &dev);
> +
> + /* Calculate lenghts of strings */
> + parent_name_len = strlen(mux->parent->name);
> + arrow_len = strlen(arrow);
> + mux_name_len = strlen(mux->name);
> + name_len = strlen(name);
> +
> + full_name = calloc(1, parent_name_len + arrow_len +
> + mux_name_len + arrow_len + name_len + 1);
> + if (!full_name)
> + return -ENOMEM;
> +
> + /* Compose bus name */
> + strcat(full_name, mux->parent->name);
> + strcat(full_name, arrow);
> + strcat(full_name, mux->name);
> + strcat(full_name, arrow);
> + strcat(full_name, name);
> +
> + ret = device_bind_driver_to_node(mux, "i2c_mux_bus_drv",
> + full_name, node, &dev);
Why do we use the full name here? We can create this by looking at the
parents if needed.
> debug(" - bind ret=%d, %s\n", ret, dev ? dev->name : NULL);
> if (ret)
> return ret;
> +
> + /* If dt alias is not found start to allocate new IDs */
> + if (dev->req_seq == -1)
> + dev->req_seq = ++i2c_highest_id;
As in the other patch, if we change this, I think we should do it in
DM core, by adjusting uclass_resolve_seq(). Would that work?
> +
> + debug("%s: MUX bus %d: %s\n", __func__, dev->req_seq,
> + full_name);
> }
>
> return 0;
> @@ -192,6 +222,7 @@ U_BOOT_DRIVER(i2c_mux_bus) = {
>
> UCLASS_DRIVER(i2c_mux) = {
> .id = UCLASS_I2C_MUX,
> + .flags = DM_UC_FLAG_SEQ_ALIAS,
> .name = "i2c_mux",
> .post_bind = i2c_mux_post_bind,
> .post_probe = i2c_mux_post_probe,
> --
> 1.9.1
>
Also we have no i2c mux tests by the look of it, and should add a test
to make sure this works on sandbox.
Regards,
Simon
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH 2/4] fdt: Introduce fdtdec_get_alias_highest_id()
2019-01-31 10:04 ` Simon Glass
@ 2019-01-31 10:14 ` Michal Simek
0 siblings, 0 replies; 19+ messages in thread
From: Michal Simek @ 2019-01-31 10:14 UTC (permalink / raw)
To: u-boot
On 31. 01. 19 11:04, Simon Glass wrote:
> Hi Michal,
>
> On Fri, 18 Jan 2019 at 08:13, Michal Simek <michal.simek@xilinx.com> wrote:
>>
>> Find out the highest alias ID used for certain subsystem.
>> This call will be used for alocating IDs for i2c buses which are not
>> described in DT.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>> include/fdtdec.h | 13 +++++++++++++
>> lib/fdtdec.c | 33 +++++++++++++++++++++++++++++++++
>> 2 files changed, 46 insertions(+)
>>
>> diff --git a/include/fdtdec.h b/include/fdtdec.h
>> index f1bcbf837ffb..c2dd87ede226 100644
>> --- a/include/fdtdec.h
>> +++ b/include/fdtdec.h
>> @@ -626,6 +626,19 @@ int fdtdec_get_alias_seq(const void *blob, const char *base, int node,
>> int *seqp);
>>
>> /**
>> + * Get the highest alias number for susbystem.
>> + *
>> + * It parses all aliases and find out highest recorded alias for subsystem.
>> + * Aliases are of the form <base><num> where <num> is the sequence number.
>> + *
>> + * @param blob Device tree blob (if NULL, then error is returned)
>> + * @param base Base name for alias susbystem (before the number)
>> + *
>> + * @return 0 highest alias ID, -1 if not found
>> + */
>> +int fdtdec_get_alias_highest_id(const void *blob, const char *base);
>> +
>> +/**
>> * Get a property from the /chosen node
>> *
>> * @param blob Device tree blob (if NULL, then NULL is returned)
>> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
>> index 18663ce6bdac..55811975ef54 100644
>> --- a/lib/fdtdec.c
>> +++ b/lib/fdtdec.c
>> @@ -549,6 +549,39 @@ int fdtdec_get_alias_seq(const void *blob, const char *base, int offset,
>> return -ENOENT;
>> }
>>
>> +int fdtdec_get_alias_highest_id(const void *blob, const char *base)
>> +{
>> + int base_len = strlen(base);
>> + int prop_offset;
>> + int aliases;
>> + int max = -1;
>> +
>> + debug("Looking for highest alias id for '%s'\n", base);
>> +
>> + aliases = fdt_path_offset(blob, "/aliases");
>> + for (prop_offset = fdt_first_property_offset(blob, aliases);
>> + prop_offset > 0;
>> + prop_offset = fdt_next_property_offset(blob, prop_offset)) {
>> + const char *prop;
>> + const char *name;
>> + int len, val;
>> +
>> + prop = fdt_getprop_by_offset(blob, prop_offset, &name, &len);
>> + debug(" - %s, %s\n", name, prop);
>> + if (*prop != '/' || prop[len - 1] ||
>> + strncmp(name, base, base_len))
>> + continue;
>> +
>> + val = trailing_strtol(name);
>> + if (val > max) {
>> + debug("Found seq %d\n", val);
>> + max = val;
>> + }
>> + }
>> +
>> + return max;
>
> This looks right to me. Can you please add a test that calls this for
> a few sandbox aliases?
let's see how this can be done.
M
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH 1/4] dm: core: Add of_alias_get_highest_id()
2019-01-31 10:04 ` Simon Glass
@ 2019-01-31 10:19 ` Michal Simek
2019-02-02 6:05 ` Simon Glass
0 siblings, 1 reply; 19+ messages in thread
From: Michal Simek @ 2019-01-31 10:19 UTC (permalink / raw)
To: u-boot
On 31. 01. 19 11:04, Simon Glass wrote:
> Hi Michal,
>
> On Fri, 18 Jan 2019 at 08:13, Michal Simek <michal.simek@xilinx.com> wrote:
>>
>> The same functionality was added to Linux for i2c bus registration with this
>> commit message:
>>
>> "
>> of: base: add function to get highest id of an alias stem
>>
>> I2C supports adding adapters using either a dynamic or fixed id. The
>> latter is provided by aliases in the DT case. To prevent id collisions
>> of those two types, install this function which gives us the highest
>> fixed id, so we can then let the dynamically created ones come after
>> this highest number.
>>
>> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>> Acked-by: Rob Herring <robh@kernel.org>
>> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
>> "
>>
>> Add it also to U-Boot for DM I2C support.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>> drivers/core/of_access.c | 18 ++++++++++++++++++
>> include/dm/of_access.h | 9 +++++++++
>> 2 files changed, 27 insertions(+)
>>
>> diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c
>> index 14c020a687b7..7c2df2354109 100644
>> --- a/drivers/core/of_access.c
>> +++ b/drivers/core/of_access.c
>> @@ -812,6 +812,24 @@ int of_alias_get_id(const struct device_node *np, const char *stem)
>> return id;
>> }
>>
>> +int of_alias_get_highest_id(const char *stem)
>> +{
>> + struct alias_prop *app;
>> + int id = -ENODEV;
>> +
>> + mutex_lock(&of_mutex);
>> + list_for_each_entry(app, &aliases_lookup, link) {
>> + if (strcmp(app->stem, stem) != 0)
>> + continue;
>> +
>> + if (app->id > id)
>> + id = app->id;
>> + }
>> + mutex_unlock(&of_mutex);
>> +
>> + return id;
>> +}
>> +
>> struct device_node *of_get_stdout(void)
>> {
>> return of_stdout;
>> diff --git a/include/dm/of_access.h b/include/dm/of_access.h
>> index 5ed1a0cdb427..5cbfd220bfd4 100644
>> --- a/include/dm/of_access.h
>> +++ b/include/dm/of_access.h
>> @@ -425,6 +425,15 @@ int of_alias_scan(void);
>> int of_alias_get_id(const struct device_node *np, const char *stem);
>>
>> /**
>> + * of_alias_get_highest_id - Get highest alias id for the given stem
>> + * @stem: Alias stem to be examined
>> + *
>> + * The function travels the lookup table to get the highest alias id for the
>> + * given alias stem. It returns the alias id if found.
>
> @return
>
>> + */
>> +int of_alias_get_highest_id(const char *stem);
>> +
>> +/**
>> * of_get_stdout() - Get node to use for stdout
>> *
>> * @return node referred to by stdout-path alias, or NULL if none
>> --
>> 1.9.1
>>
>
> Can we place have a test that calls this for a few values?
>
> Also see uclass_find_next_free_req_seq() and uclass_resolve_seq()
> which is what U-Boot does today. It is first in, first served. This is
> supposed to mean that devices without an alias don't get a req_seq
> value, meaning that the seq value is set to the next available value.
>
Let me play with it and see that calling sequence.
> I suspect it is OK to change the behaviour though. It might affect
> some boards which rely on not having aliases but still getting bus
> numbers.
Do you mean with I2C? Or different buses?
I don't think there is any affected board with DM_I2C and OF_CONTROL
because unlisted aliases are using req_seq -1 now.
Thanks,
Michal
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH 3/4] i2c: dm: Record maximum id of devices before probing devices
2019-01-31 10:04 ` Simon Glass
@ 2019-01-31 11:03 ` Michal Simek
2019-02-02 6:05 ` Simon Glass
0 siblings, 1 reply; 19+ messages in thread
From: Michal Simek @ 2019-01-31 11:03 UTC (permalink / raw)
To: u-boot
On 31. 01. 19 11:04, Simon Glass wrote:
> Hi Michal,
>
> On Fri, 18 Jan 2019 at 08:13, Michal Simek <michal.simek@xilinx.com> wrote:
>>
>> There is a need to find out the first free i2c ID which can be used for
>> i2s buses (including i2c buses connected to i2c mux). Do it early in
>> init and share this variable with other i2c classes for uniq bus
>> identification.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>> drivers/i2c/i2c-uclass.c | 19 +++++++++++++++++++
>> include/i2c.h | 3 +++
>> 2 files changed, 22 insertions(+)
>>
>> diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
>> index 975318e5f254..aaece115f02f 100644
>> --- a/drivers/i2c/i2c-uclass.c
>> +++ b/drivers/i2c/i2c-uclass.c
>> @@ -23,6 +23,8 @@ enum {
>> PIN_COUNT,
>> };
>>
>> +int i2c_highest_id = -1;
>> +
>> /* Useful debugging function */
>> void i2c_dump_msgs(struct i2c_msg *msg, int nmsgs)
>> {
>> @@ -619,10 +621,27 @@ static int i2c_child_post_bind(struct udevice *dev)
>> #endif
>> }
>>
>> +int i2c_uclass_init(struct uclass *class)
>> +{
>> +#ifdef CONFIG_OF_LIVE
>> + int ret;
>> +
>> + ret = of_alias_get_highest_id("i2c");
>> + if (ret >= 0)
>> + i2c_highest_id = ret;
>> +#else
>> + i2c_highest_id = fdtdec_get_alias_highest_id(gd->fdt_blob, "i2c");
>> +#endif
>
> This should call dev_read_alias_highest_id() or similar, which does
> the logic you have here.
not a problem with this.
>
>> + debug("%s: i2c highest_id %d\n", __func__, i2c_highest_id);
>> +
>> + return 0;
>> +}
>> +
>> UCLASS_DRIVER(i2c) = {
>> .id = UCLASS_I2C,
>> .name = "i2c",
>> .flags = DM_UC_FLAG_SEQ_ALIAS,
>> + .init = i2c_uclass_init,
>> #if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
>> .post_bind = dm_scan_fdt_dev,
>> #endif
>> diff --git a/include/i2c.h b/include/i2c.h
>> index ccffc1955275..c69e26deaeed 100644
>> --- a/include/i2c.h
>> +++ b/include/i2c.h
>> @@ -16,6 +16,9 @@
>> #ifndef _I2C_H_
>> #define _I2C_H_
>>
>> +/* Stores maximum highest i2c alias number */
>> +extern int i2c_highest_id;
>
> This should be in uclass-private data, not a global.
In this case there are two uclasses. UCLASS_I2C and UCLASS_I2C_MUX which
has to share the same numbers. I would expect that private data will be
private just for one not for both.
Reading
>
> But also I feel that the behaviour needs to be consistent for all
> U-Boot buses, not just I2C. So can we modify the existing core DM code
> instead? I think a change to uclass_resolve_seq() might be enough.
I need to check calling sequence to see if this is properly timed or not.
Thanks,
Michal
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH 4/4] i2c: mux: Add support for not listed sub-buses
2019-01-31 10:04 ` Simon Glass
@ 2019-01-31 15:05 ` Michal Simek
0 siblings, 0 replies; 19+ messages in thread
From: Michal Simek @ 2019-01-31 15:05 UTC (permalink / raw)
To: u-boot
On 31. 01. 19 11:04, Simon Glass wrote:
> Hi Michal,
>
> On Fri, 18 Jan 2019 at 08:13, Michal Simek <michal.simek@xilinx.com> wrote:
>>
>> Before this patch is applied all i2c sub-buses are using number -1 and
>> they can't be addresses(switch to). If all busses are listed in DT alias
>> they will get proper numbers and U-Boot can work with them.
>> In Linux buses which are not listed in DT alias get uniq number which
>> starts from the first highest free ID.
>>
>> This is the behavior on ZynqMP zcu100-revA before this patch is applied.
>>
>> Bus 0: i2c at ff020000
>> 20: gpio at 20, offset len 1, flags 0
>> 21: gpio at 21, offset len 1, flags 0
>> 75: i2c-mux at 75, offset len 1, flags 0
>> Bus -1: i2c at 0
>> Bus -1: i2c at 1
>> Bus -1: i2c at 2
>> Bus 1: i2c at ff030000
>> 74: i2c-mux at 74, offset len 1, flags 0
>> 75: i2c-mux at 75, offset len 1, flags 0
>> Bus -1: i2c at 0
>> Bus -1: i2c at 1
>> Bus -1: i2c at 2
>> Bus -1: i2c at 3
>> Bus -1: i2c at 4
>> Bus -1: i2c at 0
>> Bus -1: i2c at 1
>> Bus -1: i2c at 2
>> Bus -1: i2c at 3
>> Bus -1: i2c at 4
>> Bus -1: i2c at 5
>> Bus -1: i2c at 6
>> Bus -1: i2c at 7
>>
>> When the patch is applied also i2c-mux busses are listed properly.
>>
>> ZynqMP> i2c bus
>> Bus 0: i2c at ff020000
>> 20: gpio at 20, offset len 1, flags 0
>> 21: gpio at 21, offset len 1, flags 0
>> 75: i2c-mux at 75, offset len 1, flags 0
>> Bus 2: i2c at ff020000->i2c-mux at 75->i2c at 0
>> Bus 3: i2c at ff020000->i2c-mux at 75->i2c at 1
>> Bus 4: i2c at ff020000->i2c-mux at 75->i2c at 2
>> Bus 1: i2c at ff030000 (active 1)
>> 74: i2c-mux at 74, offset len 1, flags 0
>> 75: i2c-mux at 75, offset len 1, flags 0
>> Bus 5: i2c at ff030000->i2c-mux at 74->i2c at 0 (active 5)
>> 54: generic_54, offset len 1, flags 0
>> Bus 6: i2c at ff030000->i2c-mux at 74->i2c at 1
>> Bus 7: i2c at ff030000->i2c-mux at 74->i2c at 2
>> Bus 8: i2c at ff030000->i2c-mux at 74->i2c at 3
>> Bus 9: i2c at ff030000->i2c-mux at 74->i2c at 4
>> Bus 10: i2c at ff030000->i2c-mux at 75->i2c at 0
>> Bus 11: i2c at ff030000->i2c-mux at 75->i2c at 1
>> Bus 12: i2c at ff030000->i2c-mux at 75->i2c at 2
>> Bus 13: i2c at ff030000->i2c-mux at 75->i2c at 3
>> Bus 14: i2c at ff030000->i2c-mux at 75->i2c at 4
>> Bus 15: i2c at ff030000->i2c-mux at 75->i2c at 5
>> Bus 16: i2c at ff030000->i2c-mux at 75->i2c at 6
>> Bus 17: i2c at ff030000->i2c-mux at 75->i2c at 7
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>> zcu102-revA
>>
>> Before this patch applied for !DM case with static description
>> ZynqMP> i2c bus
>> Bus 0: zynq_0
>> Bus 1: zynq_0->PCA9544A at 0x75:0
>> Bus 2: zynq_0->PCA9544A at 0x75:1
>> Bus 3: zynq_0->PCA9544A at 0x75:2
>> Bus 4: zynq_1
>> Bus 5: zynq_1->PCA9548 at 0x74:0
>> Bus 6: zynq_1->PCA9548 at 0x74:1
>> Bus 7: zynq_1->PCA9548 at 0x74:2
>> Bus 8: zynq_1->PCA9548 at 0x74:3
>> Bus 9: zynq_1->PCA9548 at 0x74:4
>> Bus 10: zynq_1->PCA9548 at 0x75:0
>> Bus 11: zynq_1->PCA9548 at 0x75:1
>> Bus 12: zynq_1->PCA9548 at 0x75:2
>> Bus 13: zynq_1->PCA9548 at 0x75:3
>> Bus 14: zynq_1->PCA9548 at 0x75:4
>> Bus 15: zynq_1->PCA9548 at 0x75:5
>> Bus 16: zynq_1->PCA9548 at 0x75:6
>> Bus 17: zynq_1->PCA9548 at 0x75:7
>>
>> When Patch is applied with OF_LIVE - of_alias_get_highest_id() is used
>> ZynqMP> i2c bus
>> Bus 0: i2c at ff020000
>> 75: i2c-mux at 75, offset len 1, flags 0
>> Bus 2: i2c at ff020000->i2c-mux at 75->i2c at 0
>> Bus 3: i2c at ff020000->i2c-mux at 75->i2c at 1
>> Bus 4: i2c at ff020000->i2c-mux at 75->i2c at 2
>> Bus 1: i2c at ff030000
>> 74: i2c-mux at 74, offset len 1, flags 0
>> 75: i2c-mux at 75, offset len 1, flags 0
>> Bus 5: i2c at ff030000->i2c-mux at 74->i2c at 0
>> Bus 6: i2c at ff030000->i2c-mux at 74->i2c at 1
>> Bus 7: i2c at ff030000->i2c-mux at 74->i2c at 2
>> Bus 8: i2c at ff030000->i2c-mux at 74->i2c at 3
>> Bus 9: i2c at ff030000->i2c-mux at 74->i2c at 4
>> Bus 10: i2c at ff030000->i2c-mux at 75->i2c at 0
>> Bus 11: i2c at ff030000->i2c-mux at 75->i2c at 1
>> Bus 12: i2c at ff030000->i2c-mux at 75->i2c at 2
>> Bus 13: i2c at ff030000->i2c-mux at 75->i2c at 3
>> Bus 14: i2c at ff030000->i2c-mux at 75->i2c at 4
>> Bus 15: i2c at ff030000->i2c-mux at 75->i2c at 5
>> Bus 16: i2c at ff030000->i2c-mux at 75->i2c at 6
>> Bus 17: i2c at ff030000->i2c-mux at 75->i2c at 7
>>
>> For !OF_LIVE - hardcoded number is used
>> ZynqMP> i2c bus
>> Bus 0: i2c at ff020000
>> 75: i2c-mux at 75, offset len 1, flags 0
>> Bus 21: i2c at ff020000->i2c-mux at 75->i2c at 0
>> Bus 22: i2c at ff020000->i2c-mux at 75->i2c at 1
>> Bus 23: i2c at ff020000->i2c-mux at 75->i2c at 2
>> Bus 1: i2c at ff030000
>> 74: i2c-mux at 74, offset len 1, flags 0
>> 75: i2c-mux at 75, offset len 1, flags 0
>> Bus 24: i2c at ff030000->i2c-mux at 74->i2c at 0
>> Bus 25: i2c at ff030000->i2c-mux at 74->i2c at 1
>> Bus 26: i2c at ff030000->i2c-mux at 74->i2c at 2
>> Bus 27: i2c at ff030000->i2c-mux at 74->i2c at 3
>> Bus 28: i2c at ff030000->i2c-mux at 74->i2c at 4
>> Bus 29: i2c at ff030000->i2c-mux at 75->i2c at 0
>> Bus 30: i2c at ff030000->i2c-mux at 75->i2c at 1
>> Bus 31: i2c at ff030000->i2c-mux at 75->i2c at 2
>> Bus 32: i2c at ff030000->i2c-mux at 75->i2c at 3
>> Bus 33: i2c at ff030000->i2c-mux at 75->i2c at 4
>> Bus 34: i2c at ff030000->i2c-mux at 75->i2c at 5
>> Bus 35: i2c at ff030000->i2c-mux at 75->i2c at 6
>> Bus 36: i2c at ff030000->i2c-mux at 75->i2c at 7
>>
>> ---
>> drivers/i2c/muxes/i2c-mux-uclass.c | 35 +++++++++++++++++++++++++++++++++--
>> 1 file changed, 33 insertions(+), 2 deletions(-)
>
> This is quite a complicated issue...
and it is time to talk about it.
>
>>
>> diff --git a/drivers/i2c/muxes/i2c-mux-uclass.c b/drivers/i2c/muxes/i2c-mux-uclass.c
>> index a680ee176253..cb69d053fd59 100644
>> --- a/drivers/i2c/muxes/i2c-mux-uclass.c
>> +++ b/drivers/i2c/muxes/i2c-mux-uclass.c
>> @@ -9,6 +9,7 @@
>> #include <errno.h>
>> #include <i2c.h>
>> #include <dm/lists.h>
>> +#include <dm/of_access.h>
>> #include <dm/root.h>
>>
>> /**
>> @@ -59,13 +60,42 @@ static int i2c_mux_post_bind(struct udevice *mux)
>> dev_for_each_subnode(node, mux) {
>> struct udevice *dev;
>> const char *name;
>> + const char *arrow = "->";
>> + char *full_name;
>> + int parent_name_len, arrow_len, mux_name_len, name_len;
>>
>> name = ofnode_get_name(node);
>> - ret = device_bind_driver_to_node(mux, "i2c_mux_bus_drv", name,
>> - node, &dev);
>> +
>> + /* Calculate lenghts of strings */
>> + parent_name_len = strlen(mux->parent->name);
>> + arrow_len = strlen(arrow);
>> + mux_name_len = strlen(mux->name);
>> + name_len = strlen(name);
>> +
>> + full_name = calloc(1, parent_name_len + arrow_len +
>> + mux_name_len + arrow_len + name_len + 1);
>> + if (!full_name)
>> + return -ENOMEM;
>> +
>> + /* Compose bus name */
>> + strcat(full_name, mux->parent->name);
>> + strcat(full_name, arrow);
>> + strcat(full_name, mux->name);
>> + strcat(full_name, arrow);
>> + strcat(full_name, name);
>> +
>> + ret = device_bind_driver_to_node(mux, "i2c_mux_bus_drv",
>> + full_name, node, &dev);
>
> Why do we use the full name here? We can create this by looking at the
> parents if needed.
If you look at i2c mux description and you have 3-4 i2c muxes with
proper sub bus node name like i2c at 0 for all first busses you need to
distinguish that.
Linux is doing that too.
Because from this - you have no idea which bus is which.
Bus -1: i2c at 1
Bus -1: i2c at 2
Bus -1: i2c at 3
Bus -1: i2c at 4
Bus -1: i2c at 0
Bus -1: i2c at 1
Bus -1: i2c at 2
Bus -1: i2c at 3
Bus -1: i2c at 4
Bus -1: i2c at 5
Bus -1: i2c at 6
Bus -1: i2c at 7
That code above is just composing bus name by looking at parent + actual
node name. If you see better way how this can be done please let me know.
>
>> debug(" - bind ret=%d, %s\n", ret, dev ? dev->name : NULL);
>> if (ret)
>> return ret;
>> +
>> + /* If dt alias is not found start to allocate new IDs */
>> + if (dev->req_seq == -1)
>> + dev->req_seq = ++i2c_highest_id;
>
> As in the other patch, if we change this, I think we should do it in
> DM core, by adjusting uclass_resolve_seq(). Would that work?
I have played with it and uclass_resolve_seq as is now is called when
device is probed. But IMHO you need to have bus number before probe to
have it listed to know where you should move by i2c dev command.
It means setting up req->seq should be done in bind phase.
Let me send you v2 of this where I fix some stuff based on your comments.
Thanks,
Michal
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH 1/4] dm: core: Add of_alias_get_highest_id()
2019-01-31 10:19 ` Michal Simek
@ 2019-02-02 6:05 ` Simon Glass
0 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2019-02-02 6:05 UTC (permalink / raw)
To: u-boot
Hi Michal,
On Thu, 31 Jan 2019 at 03:19, Michal Simek <michal.simek@xilinx.com> wrote:
>
> On 31. 01. 19 11:04, Simon Glass wrote:
> > Hi Michal,
> >
> > On Fri, 18 Jan 2019 at 08:13, Michal Simek <michal.simek@xilinx.com>
wrote:
> >>
> >> The same functionality was added to Linux for i2c bus registration
with this
> >> commit message:
> >>
> >> "
> >> of: base: add function to get highest id of an alias stem
> >>
> >> I2C supports adding adapters using either a dynamic or fixed id. The
> >> latter is provided by aliases in the DT case. To prevent id collisions
> >> of those two types, install this function which gives us the highest
> >> fixed id, so we can then let the dynamically created ones come after
> >> this highest number.
> >>
> >> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >> Acked-by: Rob Herring <robh@kernel.org>
> >> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> >> "
> >>
> >> Add it also to U-Boot for DM I2C support.
> >>
> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> >> ---
> >>
> >> drivers/core/of_access.c | 18 ++++++++++++++++++
> >> include/dm/of_access.h | 9 +++++++++
> >> 2 files changed, 27 insertions(+)
> >>
> >> diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c
> >> index 14c020a687b7..7c2df2354109 100644
> >> --- a/drivers/core/of_access.c
> >> +++ b/drivers/core/of_access.c
> >> @@ -812,6 +812,24 @@ int of_alias_get_id(const struct device_node *np,
const char *stem)
> >> return id;
> >> }
> >>
> >> +int of_alias_get_highest_id(const char *stem)
> >> +{
> >> + struct alias_prop *app;
> >> + int id = -ENODEV;
> >> +
> >> + mutex_lock(&of_mutex);
> >> + list_for_each_entry(app, &aliases_lookup, link) {
> >> + if (strcmp(app->stem, stem) != 0)
> >> + continue;
> >> +
> >> + if (app->id > id)
> >> + id = app->id;
> >> + }
> >> + mutex_unlock(&of_mutex);
> >> +
> >> + return id;
> >> +}
> >> +
> >> struct device_node *of_get_stdout(void)
> >> {
> >> return of_stdout;
> >> diff --git a/include/dm/of_access.h b/include/dm/of_access.h
> >> index 5ed1a0cdb427..5cbfd220bfd4 100644
> >> --- a/include/dm/of_access.h
> >> +++ b/include/dm/of_access.h
> >> @@ -425,6 +425,15 @@ int of_alias_scan(void);
> >> int of_alias_get_id(const struct device_node *np, const char *stem);
> >>
> >> /**
> >> + * of_alias_get_highest_id - Get highest alias id for the given stem
> >> + * @stem: Alias stem to be examined
> >> + *
> >> + * The function travels the lookup table to get the highest alias id
for the
> >> + * given alias stem. It returns the alias id if found.
> >
> > @return
> >
> >> + */
> >> +int of_alias_get_highest_id(const char *stem);
> >> +
> >> +/**
> >> * of_get_stdout() - Get node to use for stdout
> >> *
> >> * @return node referred to by stdout-path alias, or NULL if none
> >> --
> >> 1.9.1
> >>
> >
> > Can we place have a test that calls this for a few values?
> >
> > Also see uclass_find_next_free_req_seq() and uclass_resolve_seq()
> > which is what U-Boot does today. It is first in, first served. This is
> > supposed to mean that devices without an alias don't get a req_seq
> > value, meaning that the seq value is set to the next available value.
> >
>
> Let me play with it and see that calling sequence.
>
>
> > I suspect it is OK to change the behaviour though. It might affect
> > some boards which rely on not having aliases but still getting bus
> > numbers.
>
> Do you mean with I2C? Or different buses?
>
> I don't think there is any affected board with DM_I2C and OF_CONTROL
> because unlisted aliases are using req_seq -1 now.
Yes I think that is right, actually.
Regards,
SImon
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH 3/4] i2c: dm: Record maximum id of devices before probing devices
2019-01-31 11:03 ` Michal Simek
@ 2019-02-02 6:05 ` Simon Glass
2019-02-02 6:22 ` Michal Simek
0 siblings, 1 reply; 19+ messages in thread
From: Simon Glass @ 2019-02-02 6:05 UTC (permalink / raw)
To: u-boot
Hi Michal,
On Thu, 31 Jan 2019 at 04:03, Michal Simek <michal.simek@xilinx.com> wrote:
>
> On 31. 01. 19 11:04, Simon Glass wrote:
> > Hi Michal,
> >
> > On Fri, 18 Jan 2019 at 08:13, Michal Simek <michal.simek@xilinx.com>
wrote:
> >>
> >> There is a need to find out the first free i2c ID which can be used for
> >> i2s buses (including i2c buses connected to i2c mux). Do it early in
> >> init and share this variable with other i2c classes for uniq bus
> >> identification.
> >>
> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> >> ---
> >>
> >> drivers/i2c/i2c-uclass.c | 19 +++++++++++++++++++
> >> include/i2c.h | 3 +++
> >> 2 files changed, 22 insertions(+)
> >>
> >> diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
> >> index 975318e5f254..aaece115f02f 100644
> >> --- a/drivers/i2c/i2c-uclass.c
> >> +++ b/drivers/i2c/i2c-uclass.c
> >> @@ -23,6 +23,8 @@ enum {
> >> PIN_COUNT,
> >> };
> >>
> >> +int i2c_highest_id = -1;
> >> +
> >> /* Useful debugging function */
> >> void i2c_dump_msgs(struct i2c_msg *msg, int nmsgs)
> >> {
> >> @@ -619,10 +621,27 @@ static int i2c_child_post_bind(struct udevice
*dev)
> >> #endif
> >> }
> >>
> >> +int i2c_uclass_init(struct uclass *class)
> >> +{
> >> +#ifdef CONFIG_OF_LIVE
> >> + int ret;
> >> +
> >> + ret = of_alias_get_highest_id("i2c");
> >> + if (ret >= 0)
> >> + i2c_highest_id = ret;
> >> +#else
> >> + i2c_highest_id = fdtdec_get_alias_highest_id(gd->fdt_blob,
"i2c");
> >> +#endif
> >
> > This should call dev_read_alias_highest_id() or similar, which does
> > the logic you have here.
>
> not a problem with this.
>
> >
> >> + debug("%s: i2c highest_id %d\n", __func__, i2c_highest_id);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> UCLASS_DRIVER(i2c) = {
> >> .id = UCLASS_I2C,
> >> .name = "i2c",
> >> .flags = DM_UC_FLAG_SEQ_ALIAS,
> >> + .init = i2c_uclass_init,
> >> #if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
> >> .post_bind = dm_scan_fdt_dev,
> >> #endif
> >> diff --git a/include/i2c.h b/include/i2c.h
> >> index ccffc1955275..c69e26deaeed 100644
> >> --- a/include/i2c.h
> >> +++ b/include/i2c.h
> >> @@ -16,6 +16,9 @@
> >> #ifndef _I2C_H_
> >> #define _I2C_H_
> >>
> >> +/* Stores maximum highest i2c alias number */
> >> +extern int i2c_highest_id;
> >
> > This should be in uclass-private data, not a global.
>
> In this case there are two uclasses. UCLASS_I2C and UCLASS_I2C_MUX which
> has to share the same numbers. I would expect that private data will be
> private just for one not for both.
> Reading
Hmm I hadn't realised that. Perhaps you can wrap it in a function which
looks up the I2C uclass and the private data. We should try to avoid using
BSS so that I2C can be used before relocation, etc.
>
> >
> > But also I feel that the behaviour needs to be consistent for all
> > U-Boot buses, not just I2C. So can we modify the existing core DM code
> > instead? I think a change to uclass_resolve_seq() might be enough.
>
> I need to check calling sequence to see if this is properly timed or not.
OK.
Regards,
Simon
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH 3/4] i2c: dm: Record maximum id of devices before probing devices
2019-02-02 6:05 ` Simon Glass
@ 2019-02-02 6:22 ` Michal Simek
0 siblings, 0 replies; 19+ messages in thread
From: Michal Simek @ 2019-02-02 6:22 UTC (permalink / raw)
To: u-boot
so, 2. úno 2019 7:09 odesílatel Simon Glass <sjg@chromium.org> napsal:
> Hi Michal,
>
> On Thu, 31 Jan 2019 at 04:03, Michal Simek <michal.simek@xilinx.com>
> wrote:
> >
> > On 31. 01. 19 11:04, Simon Glass wrote:
> > > Hi Michal,
> > >
> > > On Fri, 18 Jan 2019 at 08:13, Michal Simek <michal.simek@xilinx.com>
> wrote:
> > >>
> > >> There is a need to find out the first free i2c ID which can be used
> for
> > >> i2s buses (including i2c buses connected to i2c mux). Do it early in
> > >> init and share this variable with other i2c classes for uniq bus
> > >> identification.
> > >>
> > >> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> > >> ---
> > >>
> > >> drivers/i2c/i2c-uclass.c | 19 +++++++++++++++++++
> > >> include/i2c.h | 3 +++
> > >> 2 files changed, 22 insertions(+)
> > >>
> > >> diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
> > >> index 975318e5f254..aaece115f02f 100644
> > >> --- a/drivers/i2c/i2c-uclass.c
> > >> +++ b/drivers/i2c/i2c-uclass.c
> > >> @@ -23,6 +23,8 @@ enum {
> > >> PIN_COUNT,
> > >> };
> > >>
> > >> +int i2c_highest_id = -1;
> > >> +
> > >> /* Useful debugging function */
> > >> void i2c_dump_msgs(struct i2c_msg *msg, int nmsgs)
> > >> {
> > >> @@ -619,10 +621,27 @@ static int i2c_child_post_bind(struct udevice
> *dev)
> > >> #endif
> > >> }
> > >>
> > >> +int i2c_uclass_init(struct uclass *class)
> > >> +{
> > >> +#ifdef CONFIG_OF_LIVE
> > >> + int ret;
> > >> +
> > >> + ret = of_alias_get_highest_id("i2c");
> > >> + if (ret >= 0)
> > >> + i2c_highest_id = ret;
> > >> +#else
> > >> + i2c_highest_id = fdtdec_get_alias_highest_id(gd->fdt_blob,
> "i2c");
> > >> +#endif
> > >
> > > This should call dev_read_alias_highest_id() or similar, which does
> > > the logic you have here.
> >
> > not a problem with this.
> >
> > >
> > >> + debug("%s: i2c highest_id %d\n", __func__, i2c_highest_id);
> > >> +
> > >> + return 0;
> > >> +}
> > >> +
> > >> UCLASS_DRIVER(i2c) = {
> > >> .id = UCLASS_I2C,
> > >> .name = "i2c",
> > >> .flags = DM_UC_FLAG_SEQ_ALIAS,
> > >> + .init = i2c_uclass_init,
> > >> #if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
> > >> .post_bind = dm_scan_fdt_dev,
> > >> #endif
> > >> diff --git a/include/i2c.h b/include/i2c.h
> > >> index ccffc1955275..c69e26deaeed 100644
> > >> --- a/include/i2c.h
> > >> +++ b/include/i2c.h
> > >> @@ -16,6 +16,9 @@
> > >> #ifndef _I2C_H_
> > >> #define _I2C_H_
> > >>
> > >> +/* Stores maximum highest i2c alias number */
> > >> +extern int i2c_highest_id;
> > >
> > > This should be in uclass-private data, not a global.
> >
> > In this case there are two uclasses. UCLASS_I2C and UCLASS_I2C_MUX which
> > has to share the same numbers. I would expect that private data will be
> > private just for one not for both.
> > Reading
>
> Hmm I hadn't realised that. Perhaps you can wrap it in a function which
> looks up the I2C uclass and the private data. We should try to avoid using
> BSS so that I2C can be used before relocation, etc.
>
I was wrong on this one. Také a look at v2.
M
> >
> > >
> > > But also I feel that the behaviour needs to be consistent for all
> > > U-Boot buses, not just I2C. So can we modify the existing core DM code
> > > instead? I think a change to uclass_resolve_seq() might be enough.
> >
> > I need to check calling sequence to see if this is properly timed or not.
>
> OK.
>
> Regards,
> Simon
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2019-02-02 6:22 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-18 15:13 [U-Boot] [PATCH 0/4] Align U-Boot I2C DM bus ID handling with Linux Michal Simek
2019-01-18 15:13 ` [U-Boot] [PATCH 1/4] dm: core: Add of_alias_get_highest_id() Michal Simek
2019-01-28 12:35 ` Michal Simek
2019-01-31 10:04 ` Simon Glass
2019-01-31 10:19 ` Michal Simek
2019-02-02 6:05 ` Simon Glass
2019-01-18 15:13 ` [U-Boot] [PATCH 2/4] fdt: Introduce fdtdec_get_alias_highest_id() Michal Simek
2019-01-31 10:04 ` Simon Glass
2019-01-31 10:14 ` Michal Simek
2019-01-18 15:13 ` [U-Boot] [PATCH 3/4] i2c: dm: Record maximum id of devices before probing devices Michal Simek
2019-01-31 10:04 ` Simon Glass
2019-01-31 11:03 ` Michal Simek
2019-02-02 6:05 ` Simon Glass
2019-02-02 6:22 ` Michal Simek
2019-01-18 15:13 ` [U-Boot] [PATCH 4/4] i2c: mux: Add support for not listed sub-buses Michal Simek
2019-01-24 8:27 ` Michal Simek
2019-01-31 10:04 ` Simon Glass
2019-01-31 15:05 ` Michal Simek
2019-01-18 15:24 ` [U-Boot] [PATCH 0/4] Align U-Boot I2C DM bus ID handling with Linux Michal Simek
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.