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

[Simon Glass]

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.

Regards,
Simon