[U-Boot] [PATCH] i2c: mux: Allow muxes to work as children of i2c bus without i2c-parent
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@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)
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@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
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@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
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@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
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@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@70 { compatible = "ti,pca9548", "nxp,pca9548"; #address-cells = <1>; #size-cells = <0>; reg = <0x70>; status = "okay";
i2c0_70_0: i2c@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
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@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@70 { compatible = "ti,pca9548", "nxp,pca9548"; #address-cells = <1>; #size-cells = <0>; reg = <0x70>; status = "okay";
i2c0_70_0: i2c@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
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@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@70 { compatible = "ti,pca9548", "nxp,pca9548"; #address-cells = <1>; #size-cells = <0>; reg = <0x70>; status = "okay";
i2c0_70_0: i2c@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
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@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@70 { compatible = "ti,pca9548", "nxp,pca9548"; #address-cells = <1>; #size-cells = <0>; reg = <0x70>; status = "okay";
i2c0_70_0: i2c@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
participants (3)
-
Michal Simek -
Moritz Fischer -
Simon Glass