ARM: bcm2835: assign pins to sdhci pinctrl
Subsystems (3)
cover ARM: bcm2835: assign pins to sdhci pinctrl
This patch series fixes the pinctrl of the sdhci pins for all Raspberry Pis.
The pin configuration based on the proprietary DTS [1].
[1] - https://www.raspberrypi.org/documentation/configuration/images/dt-blob.dts
Stefan Wahren (4):
ARM: bcm2835: remove sdhci pins from GPIO pinctrl
ARM: bcm2835: rename sdhci pin group
ARM: bcm2835: specify card detect pin for RPi B
ARM: bcm2835: assign pins to sdhci pinctrl
arch/arm/boot/dts/bcm2835-rpi-a-plus.dts | 6 +++++- arch/arm/boot/dts/bcm2835-rpi-b-plus.dts | 6 +++++- arch/arm/boot/dts/bcm2835-rpi-b-rev2.dts | 6 +++++- arch/arm/boot/dts/bcm2835-rpi-b.dts | 11 ++++++++++- arch/arm/boot/dts/bcm2835-rpi.dtsi | 2 +- 5 files changed, 26 insertions(+), 5 deletions(-)
Signature · 1 line
1.7.9.5
RFC PATCH 1/4 ARM: bcm2835: remove sdhci pins from GPIO pinctrl
Currently the pins alt3 (sdhci) are assigned to GPIO pinctrl. This is bad because a user could export it to sysfs and break sdhci. In order to avoid that remove those pins from GPIO pintrl.
--- arch/arm/boot/dts/bcm2835-rpi-a-plus.dts | 2 +- arch/arm/boot/dts/bcm2835-rpi-b-plus.dts | 2 +- arch/arm/boot/dts/bcm2835-rpi-b-rev2.dts | 2 +- arch/arm/boot/dts/bcm2835-rpi-b.dts | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-)
arch/arm/boot/dts/bcm2835-rpi-a-plus.dts +1 -1
@@ -20,7 +20,7 @@ }; &gpio { - pinctrl-0 = <&gpioout &alt0 &i2s_alt0 &alt3>; + pinctrl-0 = <&gpioout &alt0 &i2s_alt0>; /* I2S interface */ i2s_alt0: i2s_alt0 {
arch/arm/boot/dts/bcm2835-rpi-b-plus.dts +1 -1
@@ -20,7 +20,7 @@ }; &gpio { - pinctrl-0 = <&gpioout &alt0 &i2s_alt0 &alt3>; + pinctrl-0 = <&gpioout &alt0 &i2s_alt0>; /* I2S interface */ i2s_alt0: i2s_alt0 {
arch/arm/boot/dts/bcm2835-rpi-b-rev2.dts +1 -1
@@ -13,7 +13,7 @@ }; &gpio { - pinctrl-0 = <&gpioout &alt0 &i2s_alt2 &alt3>; + pinctrl-0 = <&gpioout &alt0 &i2s_alt2>; /* I2S interface */ i2s_alt2: i2s_alt2 {
arch/arm/boot/dts/bcm2835-rpi-b.dts +1 -1
@@ -13,5 +13,5 @@ }; &gpio { - pinctrl-0 = <&gpioout &alt0 &alt3>; + pinctrl-0 = <&gpioout &alt0>; };
Signature · 1 line
1.7.9.5
RFC PATCH 1/4 ARM: bcm2835: remove sdhci pins from GPIO pinctrl
arch/arm/boot/dts/bcm2835-rpi-a-plus.dts line 23
Quoted 9 lines · Stefan Wahren
Currently the pins alt3 (sdhci) are assigned to GPIO pinctrl. This is bad because a user could export it to sysfs and break sdhci. In order to avoid that remove those pins from GPIO pintrl.
arch/arm/boot/dts/bcm2835-rpi-a-plus.dts +1
-1&gpio { - pinctrl-0 = <&gpioout &alt0 &i2s_alt0 &alt3>; + pinctrl-0 = <&gpioout &alt0 &i2s_alt0>;
This doesn't make sense. The current DT content is configuring those pins as SDHCI, not as GPIO. Admitedly this is redundant since the firmware and/or bootloader already did this in order to boot the system, but irrespective, the current DT causes no issues. Removing the pinctrl setting should not influence whether the pins can be exported via GPIO sysfs either.
RFC PATCH 1/4 ARM: bcm2835: remove sdhci pins from GPIO pinctrl
Quoted 17 lines
Stephen Warren <[email protected]> hat am 2. Dezember 2015 um 04:40 geschrieben:
Quoted 10 lines · Stefan Wahren
Currently the pins alt3 (sdhci) are assigned to GPIO pinctrl. This is bad because a user could export it to sysfs and break sdhci. In order to avoid that remove those pins from GPIO pintrl.
arch/arm/boot/dts/bcm2835-rpi-a-plus.dts +0
-0&gpio {
- pinctrl-0 = <&gpioout &alt0&i2s_alt0&alt3>;
+ pinctrl-0 = <&gpioout &alt0&i2s_alt0>;
Quoted 6 lines
This doesn't make sense. The current DT content is configuring those pins as SDHCI, not as GPIO. Admitedly this is redundant since the firmware and/or bootloader already did this in order to boot the system, but irrespective, the current DT causes no issues. Removing the pinctrl setting should not influence whether the pins can be exported via GPIO sysfs either.
You are right.
Is it generally possible to avoid the GPIO sysfs export for SDHCI pins? Is it an issue of pinctrl-bcm2835?
RFC PATCH 1/4 ARM: bcm2835: remove sdhci pins from GPIO pinctrl
Quoted 24 lines · Stefan Wahren
Quoted 17 lines
Stephen Warren <[email protected]> hat am 2. Dezember 2015 um 04:40 geschrieben:
Quoted 10 lines · Stefan Wahren
Currently the pins alt3 (sdhci) are assigned to GPIO pinctrl. This is bad because a user could export it to sysfs and break sdhci. In order to avoid that remove those pins from GPIO pintrl.
arch/arm/boot/dts/bcm2835-rpi-a-plus.dts +0
-0&gpio {
- pinctrl-0 = <&gpioout &alt0&i2s_alt0&alt3>;
+ pinctrl-0 = <&gpioout &alt0&i2s_alt0>;Quoted 6 lines
This doesn't make sense. The current DT content is configuring those pins as SDHCI, not as GPIO. Admitedly this is redundant since the firmware and/or bootloader already did this in order to boot the system, but irrespective, the current DT causes no issues. Removing the pinctrl setting should not influence whether the pins can be exported via GPIO sysfs either.
Quoted 4 lines
You are right.
Is it generally possible to avoid the GPIO sysfs export for SDHCI pins? Is it an issue of pinctrl-bcm2835?
I believe this same issue exists on all platforms where GPIO pins can be mux'd onto the same pins as other functions.
RFC PATCH 2/4 ARM: bcm2835: rename sdhci pin group
In reply to Stefan WahrenThe node name of the sdhci pin group doesn't explain it's real function. So rename it.
--- arch/arm/boot/dts/bcm2835-rpi.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/boot/dts/bcm2835-rpi.dtsi +1 -1
@@ -36,7 +36,7 @@ brcm,function = <BCM2835_FSEL_ALT0>; }; - alt3: alt3 { + sdhci_alt3: sdhci_alt3 { brcm,pins = <48 49 50 51 52 53>; brcm,function = <BCM2835_FSEL_ALT3>; };
Signature · 1 line
1.7.9.5
RFC PATCH 2/4 ARM: bcm2835: rename sdhci pin group
Quoted 2 lines · Stefan Wahren
The node name of the sdhci pin group doesn't explain it's real function. So rename it.
The real function of this node is not to configure SDHCI pins, but to set pins to alt3, as the current name states. Admittedly it's possible that currently the only pins that need to be set to ALT3 are SDHCI related, but that's incidental.
RFC PATCH 2/4 ARM: bcm2835: rename sdhci pin group
Quoted 5 lines
Stephen Warren <[email protected]> hat am 2. Dezember 2015 um 04:42 geschrieben:
Quoted 8 lines
Quoted 2 lines · Stefan Wahren
The node name of the sdhci pin group doesn't explain it's real function. So rename it.
The real function of this node is not to configure SDHCI pins, but to set pins to alt3, as the current name states. Admittedly it's possible that currently the only pins that need to be set to ALT3 are SDHCI related, but that's incidental.
Yes, i understand the original intension to assign every pin to the available mux functions ( gpio_in, gpio_out, alt* ).
But 3f37169fb3 ("ARM: bcm2835: dt: Add Raspberry Pi Model B rev2") introduce a better self-describing pin group naming for I2S. So my idea was to adapt it according to sdhci first and go on.
So here is a possible vision for bcm2835-rpi.dtsi
&gpio {
pinctrl-names = "default";
act_gpio: gpio {
brcm,pins = <6>;
brcm,function = <BCM2835_FSEL_GPIO_OUT>;
};
i2c0_alt0: i2c {
brcm,pins = <0 1>;
brcm,function = <BCM2835_FSEL_ALT0>;
};
i2c1_alt0: i2c {
brcm,pins = <2 3>;
brcm,function = <BCM2835_FSEL_ALT0>;
};
gpclk_alt0: gpclk {
brcm,pins = <4 5>;
brcm,function = <BCM2835_FSEL_ALT0>;
};
spi0_alt0: spi {
brcm,pins = <7 8 9 10 11>;
brcm,function = <BCM2835_FSEL_ALT0>;
};
uart0_alt0: spi {
brcm,pins = <14 15>;
brcm,function = <BCM2835_FSEL_ALT0>;
};
pwm0_alt0: pwm {
brcm,pins = <40>;
brcm,function = <BCM2835_FSEL_ALT0>;
};
pwm1_alt0: pwm {
brcm,pins = <45>;
brcm,function = <BCM2835_FSEL_ALT0>;
};
sdhci_alt3: sdhci_alt3 {
brcm,pins = <48 49 50 51 52 53>;
brcm,function = <BCM2835_FSEL_ALT3>;
};
};Best regards
Stefan
RFC PATCH 2/4 ARM: bcm2835: rename sdhci pin group
Quoted 14 lines · Stefan Wahren
Quoted 5 lines
Stephen Warren <[email protected]> hat am 2. Dezember 2015 um 04:42 geschrieben:
Quoted 8 lines
Quoted 2 lines · Stefan Wahren
The node name of the sdhci pin group doesn't explain it's real function. So rename it.
The real function of this node is not to configure SDHCI pins, but to set pins to alt3, as the current name states. Admittedly it's possible that currently the only pins that need to be set to ALT3 are SDHCI related, but that's incidental.
Quoted 4 lines
Yes, i understand the original intension to assign every pin to the available mux functions (
gpio_in,gpio_out, alt* ).But
3f37169fb3("ARM: bcm2835: dt: Add Raspberry Pi Model B rev2") introduce a
FYI I don't have that yet and git fetch is being very slow right now:-(
Quoted 2 lines
better self-describing pin group naming for I2S. So my idea was to adapt it according to sdhci first and go on.
OK. I'd suggest explaining that directly in the commit description then. The commit description above has a quite different semantic meaning.
Quoted 11 lines
So here is a possible vision for
bcm2835-rpi.dtsi&gpio { pinctrl-names = "default"; act_gpio: gpio { brcm,pins = <6>; brcm,function = <BCM2835_FSEL_GPIO_OUT>; }; i2c0_alt0: i2c {
Quoted 8 lines
brcm,pins = <0 1>;
brcm,function = <BCM2835_FSEL_ALT0>;
};
i2c1_alt0: i2c {
brcm,pins = <2 3>;
brcm,function = <BCM2835_FSEL_ALT0>;
};...
OK, I guess that can work; I imagine it would make DT overlays easier by making the pinctrl setup more fine-grained.
RFC PATCH 3/4 ARM: bcm2835: specify card detect pin for RPi B
In reply to Stefan WahrenOnly the Raspberry Pi B has a card detect pin. Specify it on board level because it's not free to use.
--- arch/arm/boot/dts/bcm2835-rpi-b.dts | 5 +++++ 1 file changed, 5 insertions(+)
arch/arm/boot/dts/bcm2835-rpi-b.dts +5 -0
@@ -14,4 +14,9 @@ &gpio { pinctrl-0 = <&gpioout &alt0>; + + sdhci_cd: sdhci_cd { + brcm,pins = <47>; + brcm,function = <BCM2835_FSEL_GPIO_IN>; + }; };
Signature · 1 line
1.7.9.5
RFC PATCH 3/4 ARM: bcm2835: specify card detect pin for RPi B
Quoted 2 lines · Stefan Wahren
Only the Raspberry Pi B has a card detect pin. Specify it on board level because it's not free to use.
This seems fine, but it should have no effect in practice; when the SD controller driver gpio_get()s the GPIO, the same setting will be programmed into HW. There's a requirement to use the pinctrl bindings to configure pins to non-GPIO mux settings, but no actual requirement to do so for GPIOs.
However, the new node sdhci_cd should be added to some pinctrl-0 property or it won't be used.
RFC PATCH 3/4 ARM: bcm2835: specify card detect pin for RPi B
Am 02.12.2015 um 04:44 schrieb Stephen Warren:
Quoted 11 lines · Stefan Wahren
Quoted 2 lines
Only the Raspberry Pi B has a card detect pin. Specify it on board level because it's not free to use.
This seems fine, but it should have no effect in practice; when the SD controller driver gpio_get()s the GPIO, the same setting will be programmed into HW. There's a requirement to use the pinctrl bindings to configure pins to non-GPIO mux settings, but no actual requirement to do so for GPIOs.
However, the new node
sdhci_cdshould be added to some pinctrl-0 property or it won't be used.
This is done in Patch 4
RFC PATCH 4/4 ARM: bcm2835: assign pins to sdhci pinctrl
In reply to Stefan WahrenFinally this patch assign all relevant pins to the sdhci pinctrl.
--- arch/arm/boot/dts/bcm2835-rpi-a-plus.dts | 4 ++++ arch/arm/boot/dts/bcm2835-rpi-b-plus.dts | 4 ++++ arch/arm/boot/dts/bcm2835-rpi-b-rev2.dts | 4 ++++ arch/arm/boot/dts/bcm2835-rpi-b.dts | 4 ++++ 4 files changed, 16 insertions(+)
arch/arm/boot/dts/bcm2835-rpi-a-plus.dts +4 -0
@@ -28,3 +28,7 @@ brcm,function = <BCM2835_FSEL_ALT0>; }; }; + +&sdhci { + pinctrl-0 = <&sdhci_alt3>; +};
arch/arm/boot/dts/bcm2835-rpi-b-plus.dts +4 -0
@@ -28,3 +28,7 @@ brcm,function = <BCM2835_FSEL_ALT0>; }; }; + +&sdhci { + pinctrl-0 = <&sdhci_alt3>; +};
arch/arm/boot/dts/bcm2835-rpi-b-rev2.dts +4 -0
@@ -21,3 +21,7 @@ brcm,function = <BCM2835_FSEL_ALT2>; }; }; + +&sdhci { + pinctrl-0 = <&sdhci_alt3>; +};
arch/arm/boot/dts/bcm2835-rpi-b.dts +4 -0
@@ -20,3 +20,7 @@ brcm,function = <BCM2835_FSEL_GPIO_IN>; }; }; + +&sdhci { + pinctrl-0 = <&sdhci_alt3 &sdhci_cd>; +};
Signature · 1 line
1.7.9.5
RFC PATCH 4/4 ARM: bcm2835: assign pins to sdhci pinctrl
Quoted 15 lines · Stefan Wahren
Finally this patch assign all relevant pins to the sdhci pinctrl.
Signed-off-by Stefan Wahren <[email protected]>--- arch/arm/boot/dts/bcm2835-rpi-a-plus.dts | 4 ++++ arch/arm/boot/dts/bcm2835-rpi-b-plus.dts | 4 ++++ arch/arm/boot/dts/bcm2835-rpi-b-rev2.dts | 4 ++++ arch/arm/boot/dts/bcm2835-rpi-b.dts | 4 ++++ 4 files changed, 16 insertions(+)arch/arm/boot/dts/bcm2835-rpi-a-plus.dts +0
-0
arch/arm/boot/dts/bcm2835-rpi-a-plus.dts line 34
Quoted 7 lines
arch/arm/boot/dts/bcm2835-rpi-a-plus.dts +4
-0@@ -28,3 +28,7 @@ brcm,function = <BCM2835_FSEL_ALT0>; }; }; + +&sdhci { + pinctrl-0 = <&sdhci_alt3>; +};
[...]
What effect does this have? Does this change any pinmuxing at boot? If so, is it just resetting things from what the firmware has set to what we expect it to be? Does it enable card detection on the Pi B?
As far as the whole series, I really like the improved description in
patch 2, and patch 3 seems like a good idea, too.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151201/432834ea/attachment.sig>