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

[Michal Simek]

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

Thanks,
Michal