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

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

[Moritz Fischer]

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@ettus.com>
Cc: Heiko Schocher <hs@denx.de>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Michal Simek <michal.simek@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 */
+	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)
-- 
2.7.4

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

[Michal Simek]

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@ettus.com>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Michal Simek <michal.simek@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.

> +	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.

Thanks,
Michal

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

[Moritz Fischer]

Hi Michal,

On Mon, Jan 2, 2017 at 6:24 AM, Michal Simek <michal.simek@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@ettus.com>
>> Cc: Heiko Schocher <hs@denx.de>
>> Cc: Bin Meng <bmeng.cn@gmail.com>
>> Cc: Simon Glass <sjg@chromium.org>
>> Cc: Michal Simek <michal.simek@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.

Thanks for the review, will resubmit

Moritz

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

[Michal Simek]

On 2.1.2017 20:20, Moritz Fischer wrote:
> Hi Michal,
> 
> On Mon, Jan 2, 2017 at 6:24 AM, Michal Simek <michal.simek@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@ettus.com>
>>> Cc: Heiko Schocher <hs@denx.de>
>>> Cc: Bin Meng <bmeng.cn@gmail.com>
>>> Cc: Simon Glass <sjg@chromium.org>
>>> Cc: Michal Simek <michal.simek@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.
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.

I think that if you detect mux with 8 ports you can simply use unique
busid to be able to address them.

Thanks,
Michal

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

[Moritz Fischer]

Hi Michal,

On Tue, Jan 3, 2017 at 1:22 AM, Michal Simek <michal.simek@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@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@ettus.com>
>>>> Cc: Heiko Schocher <hs@denx.de>
>>>> Cc: Bin Meng <bmeng.cn@gmail.com>
>>>> Cc: Simon Glass <sjg@chromium.org>
>>>> Cc: Michal Simek <michal.simek@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.

> 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.

> 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).
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.

Thanks,

Moritz

[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@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@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@ettus.com>
>>>>> Cc: Heiko Schocher <hs@denx.de>
>>>>> Cc: Bin Meng <bmeng.cn@gmail.com>
>>>>> Cc: Simon Glass <sjg@chromium.org>
>>>>> Cc: Michal Simek <michal.simek@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

[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@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@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@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@ettus.com>
> >>>>> Cc: Heiko Schocher <hs@denx.de>
> >>>>> Cc: Bin Meng <bmeng.cn@gmail.com>
> >>>>> Cc: Simon Glass <sjg@chromium.org>
> >>>>> Cc: Michal Simek <michal.simek@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

[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@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@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@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@ettus.com>
>>>>>>> Cc: Heiko Schocher <hs@denx.de>
>>>>>>> Cc: Bin Meng <bmeng.cn@gmail.com>
>>>>>>> Cc: Simon Glass <sjg@chromium.org>
>>>>>>> Cc: Michal Simek <michal.simek@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