[U-Boot] [PATCH] i2c: mux: Allow muxes to work as children of i2c bus without i2c-parent

[Michal Simek]

On 18.1.2017 22:42, Simon Glass wrote:
> Hi Michal,
> 
> On 4 January 2017 at 02:40, Michal Simek <michal.simek at xilinx.com> wrote:
>>
>> Hi,
>>
>> On 3.1.2017 17:15, Moritz Fischer wrote:
>>> Hi Michal,
>>>
>>> On Tue, Jan 3, 2017 at 1:22 AM, Michal Simek <michal.simek at xilinx.com> wrote:
>>>> On 2.1.2017 20:20, Moritz Fischer wrote:
>>>>> Hi Michal,
>>>>>
>>>>> On Mon, Jan 2, 2017 at 6:24 AM, Michal Simek <michal.simek at xilinx.com> wrote:
>>>>>> On 29.12.2016 23:50, Moritz Fischer wrote:
>>>>>>> For mux check if the parent is already a device of UCLASS_I2C and if yes
>>>>>>> just use that. Otherwise see if someone specified an i2c-parent phandle.
>>>>>>> This mimics the behavior found in the Kernel, as it removes the
>>>>>>> requirement to explicitly specify a i2c-parent phandle.
>>>>>>>
>>>>>>> Signed-off-by: Moritz Fischer <moritz.fischer at ettus.com>
>>>>>>> Cc: Heiko Schocher <hs at denx.de>
>>>>>>> Cc: Bin Meng <bmeng.cn at gmail.com>
>>>>>>> Cc: Simon Glass <sjg at chromium.org>
>>>>>>> Cc: Michal Simek <michal.simek at xilinx.com>
>>>>>>> Cc: u-boot at lists.denx.de
>>>>>>> ---
>>>>>>>  drivers/i2c/muxes/i2c-mux-uclass.c | 9 +++++++++
>>>>>>>  1 file changed, 9 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/i2c/muxes/i2c-mux-uclass.c b/drivers/i2c/muxes/i2c-mux-uclass.c
>>>>>>> index 7a698b6..e01b773 100644
>>>>>>> --- a/drivers/i2c/muxes/i2c-mux-uclass.c
>>>>>>> +++ b/drivers/i2c/muxes/i2c-mux-uclass.c
>>>>>>> @@ -86,6 +86,15 @@ static int i2c_mux_post_probe(struct udevice *mux)
>>>>>>>       debug("%s: %s\n", __func__, mux->name);
>>>>>>>       priv->selected = -1;
>>>>>>>
>>>>>>> +     /* if parent is of i2c uclass already, we'll take that, otherwise
>>>>>>> +      * look if we find an i2c-parent phandle */
>>>>>>
>>>>>> Incorrect comment style.
>>>>>
>>>>> Yeah, wasn't flagged by checkpatch .... will fix.
>>>>>>
>>>>>>> +     if (UCLASS_I2C == device_get_uclass_id(mux->parent)) {
>>>>>>> +             priv->i2c_bus = dev_get_parent(mux);
>>>>>>> +             debug("%s: bus=%p/%s\n", __func__, priv->i2c_bus,
>>>>>>> +                   priv->i2c_bus->name);
>>>>>>> +             return 0;
>>>>>>> +     }
>>>>>>> +
>>>>>>>       ret = uclass_get_device_by_phandle(UCLASS_I2C, mux, "i2c-parent",
>>>>>>>                                          &priv->i2c_bus);
>>>>>>>       if (ret)
>>>>>>>
>>>>>>
>>>>>> The part of this will be good to also handle
>>>>>> req_seq for mux busses. But at least this should solved part of the
>>>>>> problems.
>>>>>
>>>>> I'm not sure I understand this comment.
>>>>
>>>> AFAIK using i2c muxes requires two changes in DTS file.
>>>> First change is this to setup i2c-parent in DTS file which is something
>>>> what Linux doesn't need.
>>>
>>> Yeah this part is addressed in this patch.
>>
>> yep
>>
>>>
>>>> The next change is that you have to extend i2c aliases to point to i2c
>>>> mux sub busses which is the second thing what Linux doesn't need.
>>>> I expect that this change you have in your dts file.
>>>
>>> Yeah, thanks for clarifying. In my dts I have aliases for each of the
>>> mux channels,
>>> which, I don't have a good idea on how to solve differently. In Linux
>>> I think I don't need that.
>>
>> yes and this is not required by Linux kernel.
>> IIRC Linux simply assign free ID to that bus.
>> It means starting from 0 and asking if there is alias or not and then
>> +1, etc should work.
>>
>>
>>>> I think that if you detect mux with 8 ports you can simply use unique
>>>> busid to be able to address them.
>>>
>>> In Linux? I'll have to take another look at that. Currently I get
>>> busses like 700,701,702 etc (which are aliases I defined).
>>
>> When I was playing with this if you define other aliases like 1700,
>> 1701, 1702 etc u-boot will mess it up.
>>
>>> Each of them points to a mux channel.
>>>
>>> <snip>
>>> aliases { ...
>>>                 i2c0 = &i2c0;
>>>                 i2c0700 = &i2c0_70_0;
>>>                 i2c0701 = &i2c0_70_1;
>>>                 i2c0702 = &i2c0_70_2;
>>>                 i2c0703 = &i2c0_70_3;
>>> };
>>>
>>> ...
>>>         i2cswitch at 70 {
>>>                 compatible = "ti,pca9548", "nxp,pca9548";
>>>                 #address-cells = <1>;
>>>                 #size-cells = <0>;
>>>                 reg = <0x70>;
>>>                 status = "okay";
>>>
>>>                 i2c0_70_0: i2c at 0 {
>>>                         reg = <0x0>;
>>>                         #address-cells = <1>;
>>>                         #size-cells = <0>;
>>>
>>>                         status = "okay";
>>>                 };
>>>                 ...
>>>           };
>>>
>>> </snip>
>>>
>>> I Will resubmit a v2 for the other changes above
>>>
>>> If you or Simon have ideas on how to correctly solve the alias issue,
>>> I can take another stab at that.
>>
>> Look at this.
>> http://lists.denx.de/pipermail/u-boot/2016-April/251769.html
> 
> That patch was never applied but it would be good to get this problem resolved.

Definitely but TBH I won't have any time for looking at this at least
for 2 months.

Thanks,
Michal