[U-Boot] [PATCH 4/4] i2c: mux: Add support for not listed sub-buses
Michal Simek
michal.simek at xilinx.com
Thu Jan 31 15:05:13 UTC 2019
On 31. 01. 19 11:04, Simon Glass wrote:
> Hi Michal,
>
> On Fri, 18 Jan 2019 at 08:13, Michal Simek <michal.simek at 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 at 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
More information about the U-Boot
mailing list