ARM: dts: aspeed: add reg field to pca9541a i2c mux

PATCH Superseded Emily Shaffer 5 participants 10 messages Feb 28, 2017 – Mar 3, 2017

Subsystems (2)

Device tree bindings Arm/Aspeed machine support

lore

Share & tools

lore

Emily Shaffer

author

2017-02-28 21:48 UTC #0 lore raw

PATCH 1/1 ARM: dts: aspeed: add reg field to pca9541a i2c mux

I2C mux pca9541a@70 was incorrectly configured, preventing hotswap@10 LM5066I hotswap controller from being configured at startup. Adding the missing field results in the hotswap controller being present immediately at boot.

Signed-off-by Emily Shaffer <[email protected]>

---
 arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 1 +
 1 file changed, 1 insertion(+)

arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts +1 -0

index e2195a641414..751c79d6dfcf 100644
@@ -246,6 +246,7 @@
 		i2c-arb {
 			#address-cells = <1>;
 			#size-cells = <0>;
+			reg = <0x0>;
 
 			hotswap@10 {
 				compatible = "ti,lm5066";

Signature · 1 line

2.11.0.483.g087da7b7c-goog

Patrick Williams

2017-03-01 20:05 UTC #1 lore raw

Quoted 6 lines · Emily Shaffer

I2C mux pca9541a@70 was incorrectly configured, preventing hotswap@10 LM5066I hotswap controller from being configured at startup. Adding the missing field results in the hotswap controller being present immediately at boot.
Signed-off-by Emily Shaffer <[email protected]>

Emily,

I would recommend referencing Zaius in the title and maybe the commit message body as well. As is, there is no indication as to which machine this change is for. You can 'git log' the file and follow examples of others.

Looks like this is the typical pattern for Zaius-only dts updates.
b40fcf1 ARM: dts: aspeed: zaius: Update EVT2 LED GPIOs 7390d9c ARM: dts: aspeed: zaius: Enable UART1 6fcd70b ARM: dts: aspeed: zaius: Add 1-wire buses

Signature · 1 line

Patrick Williams

Emily Shaffer

author

2017-03-01 22:43 UTC #2 lore raw

PATCH v2 1/1 ARM: dts: aspeed: zaius add reg field to pca9541a i2c mux

I2C mux pca9541a@70 for Zaius was incorrectly configured, preventing hotswap@10 LM5066I hotswap controller from being configured at startup. Adding the missing field results in the hotswap controller being present immediately at boot.

Signed-off-by Emily Shaffer <[email protected]>

---
 arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 1 +
 1 file changed, 1 insertion(+)

arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts +1 -0

index 548ef1e939fe..7015863751e0 100644
@@ -258,6 +258,7 @@
 		i2c-arb {
 			#address-cells = <1>;
 			#size-cells = <0>;
+			reg = <0x0>;
 
 			hotswap@10 {
 				compatible = "ti,lm5066";

Signature · 1 line

2.12.0.rc1.440.g5b76565f74-goog

Xo Wang

Reviewed-by on v2 1/1

2017-03-02 00:22 UTC #3 lore raw

Re: [PATCH v2] ARM: dts: aspeed: zaius add reg field to pca9541a i2c mux

Quoted 23 lines · Emily Shaffer

I2C mux pca9541a@70 for Zaius was incorrectly configured, preventing hotswap@10 LM5066I hotswap controller from being configured at startup. Adding the missing field results in the hotswap controller being present immediately at boot.
Signed-off-by Emily Shaffer <[email protected]>
---
 arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 1 +
 1 file changed, 1 insertion(+)
arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts +1 -1
index 548ef1e939fe..7015863751e0 100644
@@ -258,6 +258,7 @@
                i2c-arb {
                        #address-cells = <1>;
                        #size-cells = <0>;
+                       reg = <0x0>;
 
                        hotswap@10 {
                                compatible = "ti,lm5066";
--

Quoted 2 lines

2.12.0.rc1.440.g5b76565f74-goog

Reviewed-by Xo Wang <[email protected]>

Joel Stanley

maintainer Applied

2017-03-02 00:27 UTC #4 lore raw

Re: [PATCH v2] ARM: dts: aspeed: zaius add reg field to pca9541a i2c mux

In reply to Emily Shaffer

Hey Emily,

Welcome to OpenBMC! I maintain the kernel tree for the project.

When you're sending patches for our fork of Linux I encourage developers to put the project and branch they're targeting in the patch prefix. This helps me out as we use the list for a few things other than Linux:

https://github.com/openbmc/linux/wiki/TipsAndTricks#sending-patches

git format-patch --subject-prefix="PATCH linux dev-4.7"

Quoted 4 lines · Emily Shaffer

I2C mux pca9541a@70 for Zaius was incorrectly configured, preventing hotswap@10 LM5066I hotswap controller from being configured at startup. Adding the missing field results in the hotswap controller being present immediately at boot.

I wasn't sure why this would fix things. I had a chat with Xo and he explained:

09:28 <@shenki> GHF[m]: but i wasn't sure why adding the reg property changed things?
09:28 <@shenki> GHF[m]: was it not binding without the reg? 09:28 <@shenki> there's no device tree bindings that match what zaius is doing, so it's hard to check
10:44 < GHF[m]> shenki: correct, no i2c slave device for the hotswap controller was instantiated and the lm5066 driver did not bind 10:45 < GHF[m]> shenki: actually Emily noted that it did bind if the device name were changed to "lm5066" suggesting that the of_* compatible property match wasn't working
10:46 < GHF[m]> shenki: no idea why the reg property one level up affects that ¯_(ツ)_/¯
10:47 <@shenki> GHF[m]: okay. i will merge it for now

I've applied your patch to dev-4.7 with Xo's review.

Cheers,

Joel

Quoted 20 lines

Signed-off-by Emily Shaffer <[email protected]>

---
 arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 1 +
 1 file changed, 1 insertion(+)

arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts +1 -1

index 548ef1e939fe..7015863751e0 100644
@@ -258,6 +258,7 @@
                i2c-arb {
                        #address-cells = <1>;
                        #size-cells = <0>;
+                       reg = <0x0>;
 
                        hotswap@10 {
                                compatible = "ti,lm5066";
--

2.12.0.rc1.440.g5b76565f74-goog

Joel Stanley

maintainer

2017-03-02 00:30 UTC #5 lore raw

Re: [PATCH v2] ARM: dts: aspeed: zaius add reg field to pca9541a i2c mux

Quoted 35 lines · Joel Stanley

Hey Emily,
Welcome to OpenBMC! I maintain the kernel tree for the project.
When you're sending patches for our fork of Linux I encourage developers to put the project and branch they're targeting in the patch prefix. This helps me out as we use the list for a few things other than Linux:
https://github.com/openbmc/linux/wiki/TipsAndTricks#sending-patches
git format-patch --subject-prefix="PATCH linux dev-4.7"
Quoted 4 lines · Emily Shaffer
I2C mux pca9541a@70 for Zaius was incorrectly configured, preventing hotswap@10 LM5066I hotswap controller from being configured at startup. Adding the missing field results in the hotswap controller being present immediately at boot.
I wasn't sure why this would fix things. I had a chat with Xo and he explained:
09:28 <@shenki> GHF[m]: but i wasn't sure why adding the reg property changed things?
09:28 <@shenki> GHF[m]: was it not binding without the reg? 09:28 <@shenki> there's no device tree bindings that match what zaius is doing, so it's hard to check
10:44 < GHF[m]> shenki: correct, no i2c slave device for the hotswap controller was instantiated and the lm5066 driver did not bind 10:45 < GHF[m]> shenki: actually Emily noted that it did bind if the device name were changed to "lm5066" suggesting that the of_* compatible property match wasn't working
10:46 < GHF[m]> shenki: no idea why the reg property one level up affects that ¯_(ツ)_/¯
10:47 <@shenki> GHF[m]: okay. i will merge it for now

Quoted 1 line

I've applied your patch to dev-4.7 with Xo's review.

I applied it, but then noticed this adds a build warning:

Warning (reg_format): "reg" property in /ahb/apb/i2c@1e78a000/i2c-bus@300/pca9541a@70/i2c-arb has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1) Warning (avoid_default_addr_size): Relying on default #address-cells value for /ahb/apb/i2c@1e78a000/i2c-bus@300/pca9541a@70/i2c-arb Warning (avoid_default_addr_size): Relying on default #size-cells value for /ahb/apb/i2c@1e78a000/i2c-bus@300/pca9541a@70/i2c-arb

Can you please send a v3?

Cheers,

Joel

Xo Wang

2017-03-02 17:58 UTC #6 lore raw

Re: [PATCH v2] ARM: dts: aspeed: zaius add reg field to pca9541a i2c mux

It looks like this issue is fixed in v4.9: https://lkml.org/lkml/2016/8/15/569

The cover letter to that patchset also mentions adding an unused "reg" property as a workaround. I had probably copied and pasted the pca9541 bindings without "reg" to the Zaius dts without reading the rest of the notes. Oops.

Should we backport that patchset and/or wait for the next OpenBMC kernel rebase?

Emily: can you test whether the "reg" property is still necessary if you backport those patches?

cheers
xo

Emily Shaffer

author

2017-03-02 18:52 UTC #7 lore raw

Re: [PATCH v2] ARM: dts: aspeed: zaius add reg field to pca9541a i2c mux

I'm going to give a try with those patches backported, no luck getting rid of the warning.

Incidentally, is there a way to turn on warnings as errors with bitbake? I was unaware of those warnings until Joel pointed them out.

Quoted 17 lines · Xo Wang

It looks like this issue is fixed in v4.9:
https://lkml.org/lkml/2016/8/15/569
The cover letter to that patchset also mentions adding an unused "reg" property as a workaround. I had probably copied and pasted the pca9541 bindings without "reg" to the Zaius dts without reading the rest of the notes. Oops.
Should we backport that patchset and/or wait for the next OpenBMC kernel rebase?
Emily: can you test whether the "reg" property is still necessary if you backport those patches?
cheers
xo

Rick Altherr

2017-03-02 18:57 UTC #8 lore raw

Re: [PATCH v2] ARM: dts: aspeed: zaius add reg field to pca9541a i2c mux

Not that I've found. Best I've been able to work out is grepping the
build log (tmp/work/zaius-openbmc-linux-gnueabi/linux-obmc/4.7+git999-r0/temp/log.do_compile) for "warning".

Quoted 24 lines · Emily Shaffer

I'm going to give a try with those patches backported, no luck getting rid of the warning.
Incidentally, is there a way to turn on warnings as errors with bitbake? I was unaware of those warnings until Joel pointed them out.
Quoted 17 lines · Xo Wang
It looks like this issue is fixed in v4.9:
https://lkml.org/lkml/2016/8/15/569
The cover letter to that patchset also mentions adding an unused "reg" property as a workaround. I had probably copied and pasted the pca9541 bindings without "reg" to the Zaius dts without reading the rest of the notes. Oops.
Should we backport that patchset and/or wait for the next OpenBMC kernel rebase?
Emily: can you test whether the "reg" property is still necessary if you backport those patches?
cheers
xo

Emily Shaffer

author

2017-03-03 23:27 UTC #9 lore raw

Re: [PATCH v2] ARM: dts: aspeed: zaius add reg field to pca9541a i2c mux

Xo, with those backports the LM5066I works out of the box - no code change needed from me.

Joel, can we pull in the patchset Xo linked (
https://lkml.org/lkml/2016/8/15/569)? I didn't have any conflicts locally with this, should be straightforward.

Thanks,
Emily

Quoted 34 lines · Rick Altherr

Not that I've found. Best I've been able to work out is grepping the build log (tmp/work/zaius-openbmc-linux-gnueabi/linux-obmc/4.7+git999-r0/temp/log.do_compile) for "warning".
Quoted 1 line · Emily Shaffer
I'm going to give a try with those patches backported, no luck getting
rid
Quoted 3 lines
of the warning.
Incidentally, is there a way to turn on warnings as errors with bitbake?
I
Quoted 20 lines
was unaware of those warnings until Joel pointed them out.
Quoted 17 lines · Xo Wang
It looks like this issue is fixed in v4.9:
https://lkml.org/lkml/2016/8/15/569
The cover letter to that patchset also mentions adding an unused "reg" property as a workaround. I had probably copied and pasted the pca9541 bindings without "reg" to the Zaius dts without reading the rest of the notes. Oops.
Should we backport that patchset and/or wait for the next OpenBMC kernel rebase?
Emily: can you test whether the "reg" property is still necessary if you backport those patches?
cheers
xo