On 31. 01. 19 11:04, Simon Glass wrote:
> Hi Michal,
>
> On Fri, 18 Jan 2019 at 08:13, Michal Simek <michal.si...@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@ff020000
>> 20: gpio@20, offset len 1, flags 0
>> 21: gpio@21, offset len 1, flags 0
>> 75: i2c-mux@75, offset len 1, flags 0
>> Bus -1: i2c@0
>> Bus -1: i2c@1
>> Bus -1: i2c@2
>> Bus 1: i2c@ff030000
>> 74: i2c-mux@74, offset len 1, flags 0
>> 75: i2c-mux@75, offset len 1, flags 0
>> Bus -1: i2c@0
>> Bus -1: i2c@1
>> Bus -1: i2c@2
>> Bus -1: i2c@3
>> Bus -1: i2c@4
>> Bus -1: i2c@0
>> Bus -1: i2c@1
>> Bus -1: i2c@2
>> Bus -1: i2c@3
>> Bus -1: i2c@4
>> Bus -1: i2c@5
>> Bus -1: i2c@6
>> Bus -1: i2c@7
>>
>> When the patch is applied also i2c-mux busses are listed properly.
>>
>> ZynqMP> i2c bus
>> Bus 0: i2c@ff020000
>> 20: gpio@20, offset len 1, flags 0
>> 21: gpio@21, offset len 1, flags 0
>> 75: i2c-mux@75, offset len 1, flags 0
>> Bus 2: i2c@ff020000->i2c-mux@75->i2c@0
>> Bus 3: i2c@ff020000->i2c-mux@75->i2c@1
>> Bus 4: i2c@ff020000->i2c-mux@75->i2c@2
>> Bus 1: i2c@ff030000 (active 1)
>> 74: i2c-mux@74, offset len 1, flags 0
>> 75: i2c-mux@75, offset len 1, flags 0
>> Bus 5: i2c@ff030000->i2c-mux@74->i2c@0 (active 5)
>> 54: generic_54, offset len 1, flags 0
>> Bus 6: i2c@ff030000->i2c-mux@74->i2c@1
>> Bus 7: i2c@ff030000->i2c-mux@74->i2c@2
>> Bus 8: i2c@ff030000->i2c-mux@74->i2c@3
>> Bus 9: i2c@ff030000->i2c-mux@74->i2c@4
>> Bus 10: i2c@ff030000->i2c-mux@75->i2c@0
>> Bus 11: i2c@ff030000->i2c-mux@75->i2c@1
>> Bus 12: i2c@ff030000->i2c-mux@75->i2c@2
>> Bus 13: i2c@ff030000->i2c-mux@75->i2c@3
>> Bus 14: i2c@ff030000->i2c-mux@75->i2c@4
>> Bus 15: i2c@ff030000->i2c-mux@75->i2c@5
>> Bus 16: i2c@ff030000->i2c-mux@75->i2c@6
>> Bus 17: i2c@ff030000->i2c-mux@75->i2c@7
>>
>> Signed-off-by: Michal Simek <michal.si...@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@0x75:0
>> Bus 2: zynq_0->PCA9544A@0x75:1
>> Bus 3: zynq_0->PCA9544A@0x75:2
>> Bus 4: zynq_1
>> Bus 5: zynq_1->PCA9548@0x74:0
>> Bus 6: zynq_1->PCA9548@0x74:1
>> Bus 7: zynq_1->PCA9548@0x74:2
>> Bus 8: zynq_1->PCA9548@0x74:3
>> Bus 9: zynq_1->PCA9548@0x74:4
>> Bus 10: zynq_1->PCA9548@0x75:0
>> Bus 11: zynq_1->PCA9548@0x75:1
>> Bus 12: zynq_1->PCA9548@0x75:2
>> Bus 13: zynq_1->PCA9548@0x75:3
>> Bus 14: zynq_1->PCA9548@0x75:4
>> Bus 15: zynq_1->PCA9548@0x75:5
>> Bus 16: zynq_1->PCA9548@0x75:6
>> Bus 17: zynq_1->PCA9548@0x75:7
>>
>> When Patch is applied with OF_LIVE - of_alias_get_highest_id() is used
>> ZynqMP> i2c bus
>> Bus 0: i2c@ff020000
>> 75: i2c-mux@75, offset len 1, flags 0
>> Bus 2: i2c@ff020000->i2c-mux@75->i2c@0
>> Bus 3: i2c@ff020000->i2c-mux@75->i2c@1
>> Bus 4: i2c@ff020000->i2c-mux@75->i2c@2
>> Bus 1: i2c@ff030000
>> 74: i2c-mux@74, offset len 1, flags 0
>> 75: i2c-mux@75, offset len 1, flags 0
>> Bus 5: i2c@ff030000->i2c-mux@74->i2c@0
>> Bus 6: i2c@ff030000->i2c-mux@74->i2c@1
>> Bus 7: i2c@ff030000->i2c-mux@74->i2c@2
>> Bus 8: i2c@ff030000->i2c-mux@74->i2c@3
>> Bus 9: i2c@ff030000->i2c-mux@74->i2c@4
>> Bus 10: i2c@ff030000->i2c-mux@75->i2c@0
>> Bus 11: i2c@ff030000->i2c-mux@75->i2c@1
>> Bus 12: i2c@ff030000->i2c-mux@75->i2c@2
>> Bus 13: i2c@ff030000->i2c-mux@75->i2c@3
>> Bus 14: i2c@ff030000->i2c-mux@75->i2c@4
>> Bus 15: i2c@ff030000->i2c-mux@75->i2c@5
>> Bus 16: i2c@ff030000->i2c-mux@75->i2c@6
>> Bus 17: i2c@ff030000->i2c-mux@75->i2c@7
>>
>> For !OF_LIVE - hardcoded number is used
>> ZynqMP> i2c bus
>> Bus 0: i2c@ff020000
>> 75: i2c-mux@75, offset len 1, flags 0
>> Bus 21: i2c@ff020000->i2c-mux@75->i2c@0
>> Bus 22: i2c@ff020000->i2c-mux@75->i2c@1
>> Bus 23: i2c@ff020000->i2c-mux@75->i2c@2
>> Bus 1: i2c@ff030000
>> 74: i2c-mux@74, offset len 1, flags 0
>> 75: i2c-mux@75, offset len 1, flags 0
>> Bus 24: i2c@ff030000->i2c-mux@74->i2c@0
>> Bus 25: i2c@ff030000->i2c-mux@74->i2c@1
>> Bus 26: i2c@ff030000->i2c-mux@74->i2c@2
>> Bus 27: i2c@ff030000->i2c-mux@74->i2c@3
>> Bus 28: i2c@ff030000->i2c-mux@74->i2c@4
>> Bus 29: i2c@ff030000->i2c-mux@75->i2c@0
>> Bus 30: i2c@ff030000->i2c-mux@75->i2c@1
>> Bus 31: i2c@ff030000->i2c-mux@75->i2c@2
>> Bus 32: i2c@ff030000->i2c-mux@75->i2c@3
>> Bus 33: i2c@ff030000->i2c-mux@75->i2c@4
>> Bus 34: i2c@ff030000->i2c-mux@75->i2c@5
>> Bus 35: i2c@ff030000->i2c-mux@75->i2c@6
>> Bus 36: i2c@ff030000->i2c-mux@75->i2c@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@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@1
Bus -1: i2c@2
Bus -1: i2c@3
Bus -1: i2c@4
Bus -1: i2c@0
Bus -1: i2c@1
Bus -1: i2c@2
Bus -1: i2c@3
Bus -1: i2c@4
Bus -1: i2c@5
Bus -1: i2c@6
Bus -1: i2c@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
