arm64: dts: imx8mm-kontron-n801x-som: Fix the SPI chipselect polarity
Subsystems (2)
Fabio Estevam
reviewerPATCH 1/2 arm64: dts: imx8mm-kontron-n801x-som: Fix the SPI chipselect polarity
The conversion of the spi-imx driver to use GPIO descriptors in commit 8cdcd8aeee28 ("spi: imx/fsl-lpspi: Convert to GPIO descriptors") helped to detect the following SPI chipselect polarity mismatch on an imx6q-sabresd for example:
[ 4.854337] m25p80@0 enforce active low on chipselect handle
Prior to the above commit, the chipselect polarity passed via cs-gpios property was ignored and considered active-low.
The reason for such mismatch is clearly explained in the comments inside drivers/gpio/gpiolib-of.c:
* SPI children have active low chip selects
* by default. This can be specified negatively
* by just omitting "spi-cs-high" in the
* device node, or actively by tagging on
* GPIO_ACTIVE_LOW as flag in the device
* tree. If the line is simultaneously
* tagged as active low in the device tree
* and has the "spi-cs-high" set, we get a
* conflict and the "spi-cs-high" flag will
* take precedence.
To properly represent the SPI chipselect polarity, change it to active-low when the "spi-cs-high" property is absent.
--- arch/arm64/boot/dts/freescale/imx8mm-kontron-n801x-som.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/boot/dts/freescale/imx8mm-kontron-n801x-som.dtsi +1 -1
@@ -63,7 +63,7 @@ opp-750M { &ecspi1 { pinctrl-names = "default"; pinctrl-0 = <&pinctrl_ecspi1>; - cs-gpios = <&gpio5 9 GPIO_ACTIVE_HIGH>; + cs-gpios = <&gpio5 9 GPIO_ACTIVE_LOW>; status = "okay"; spi-flash@0 {
Signature · 7 lines
2.25.1 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Fabio Estevam
reviewerPATCH 2/2 arm64: dts: imx8mm-venice: Fix the SPI chipselect polarity
The conversion of the spi-imx driver to use GPIO descriptors in commit 8cdcd8aeee28 ("spi: imx/fsl-lpspi: Convert to GPIO descriptors") helped to detect the following SPI chipselect polarity mismatch on an imx6q-sabresd for example:
[ 4.854337] m25p80@0 enforce active low on chipselect handle
Prior to the above commit, the chipselect polarity passed via cs-gpios property was ignored and considered active-low.
The reason for such mismatch is clearly explained in the comments inside drivers/gpio/gpiolib-of.c:
* SPI children have active low chip selects
* by default. This can be specified negatively
* by just omitting "spi-cs-high" in the
* device node, or actively by tagging on
* GPIO_ACTIVE_LOW as flag in the device
* tree. If the line is simultaneously
* tagged as active low in the device tree
* and has the "spi-cs-high" set, we get a
* conflict and the "spi-cs-high" flag will
* take precedence.
To properly represent the SPI chipselect polarity, change it to active-low when the "spi-cs-high" property is absent.
--- arch/arm64/boot/dts/freescale/imx8mm-venice-gw71xx.dtsi | 2 +- arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx.dtsi | 2 +- arch/arm64/boot/dts/freescale/imx8mm-venice-gw73xx.dtsi | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
arch/arm64/boot/dts/freescale/imx8mm-venice-gw71xx.dtsi +1 -1
@@ -57,7 +57,7 @@ reg_usb_otg1_vbus: regulator-usb-otg1 { &ecspi2 { pinctrl-names = "default"; pinctrl-0 = <&pinctrl_spi2>; - cs-gpios = <&gpio5 13 GPIO_ACTIVE_HIGH>; + cs-gpios = <&gpio5 13 GPIO_ACTIVE_LOW>; status = "okay"; };
arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx.dtsi +1 -1
@@ -76,7 +76,7 @@ reg_usb_otg2_vbus: regulator-usb-otg2 { &ecspi2 { pinctrl-names = "default"; pinctrl-0 = <&pinctrl_spi2>; - cs-gpios = <&gpio5 13 GPIO_ACTIVE_HIGH>; + cs-gpios = <&gpio5 13 GPIO_ACTIVE_LOW>; status = "okay"; };
arch/arm64/boot/dts/freescale/imx8mm-venice-gw73xx.dtsi +1 -1
@@ -96,7 +96,7 @@ reg_wifi_en: regulator-wifi-en { &ecspi2 { pinctrl-names = "default"; pinctrl-0 = <&pinctrl_spi2>; - cs-gpios = <&gpio5 13 GPIO_ACTIVE_HIGH>; + cs-gpios = <&gpio5 13 GPIO_ACTIVE_LOW>; status = "okay"; };
Signature · 7 lines
2.25.1 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH 2/2] arm64: dts: imx8mm-venice: Fix the SPI chipselect polarity
Quoted 75 lines · Fabio Estevam
The conversion of the spi-imx driver to use GPIO descriptors in commit
8cdcd8aeee28("spi: imx/fsl-lpspi: Convert to GPIO descriptors") helped to detect the following SPI chipselect polarity mismatch on an imx6q-sabresd for example:[ 4.854337] m25p80@0 enforce active low on chipselect handlePrior to the above commit, the chipselect polarity passed via cs-gpios property was ignored and considered active-low.
The reason for such mismatch is clearly explained in the comments inside
drivers/gpio/gpiolib-of.c:* SPI children have active low chip selects
* by default. This can be specified negatively
* by just omitting "spi-cs-high" in the
* device node, or actively by tagging on
*GPIO_ACTIVE_LOWas flag in the device
* tree. If the line is simultaneously
* tagged as active low in the device tree
* and has the "spi-cs-high" set, we get a
* conflict and the "spi-cs-high" flag will
* take precedence.To properly represent the SPI chipselect polarity, change it to active-low when the "spi-cs-high" property is absent.
Signed-off-by Fabio Estevam <[email protected]>--- arch/arm64/boot/dts/freescale/imx8mm-venice-gw71xx.dtsi | 2 +- arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx.dtsi | 2 +- arch/arm64/boot/dts/freescale/imx8mm-venice-gw73xx.dtsi | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)arch/arm64/boot/dts/freescale/imx8mm-venice-gw71xx.dtsi +1
-1@@ -57,7 +57,7 @@ reg_usb_otg1_vbus: regulator-usb-otg1 { &ecspi2 { pinctrl-names = "default"; pinctrl-0 = <&pinctrl_spi2>; - cs-gpios = <&gpio5 13 GPIO_ACTIVE_HIGH>; + cs-gpios = <&gpio5 13 GPIO_ACTIVE_LOW>; status = "okay"; };arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx.dtsi +1
-1@@ -76,7 +76,7 @@ reg_usb_otg2_vbus: regulator-usb-otg2 { &ecspi2 { pinctrl-names = "default"; pinctrl-0 = <&pinctrl_spi2>; - cs-gpios = <&gpio5 13 GPIO_ACTIVE_HIGH>; + cs-gpios = <&gpio5 13 GPIO_ACTIVE_LOW>; status = "okay"; };arch/arm64/boot/dts/freescale/imx8mm-venice-gw73xx.dtsi +1
-2@@ -96,7 +96,7 @@ reg_wifi_en: regulator-wifi-en { &ecspi2 { pinctrl-names = "default"; pinctrl-0 = <&pinctrl_spi2>; - cs-gpios = <&gpio5 13 GPIO_ACTIVE_HIGH>; + cs-gpios = <&gpio5 13 GPIO_ACTIVE_LOW>; status = "okay"; }; --
Quoted 2 lines
2.25.1
Fabio,
Thanks for finding this and submitting the patch!
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 24 lines · Fabio Estevam
The conversion of the spi-imx driver to use GPIO descriptors in commit
8cdcd8aeee28("spi: imx/fsl-lpspi: Convert to GPIO descriptors") helped to detect the following SPI chipselect polarity mismatch on an imx6q-sabresd for example:[ 4.854337] m25p80@0 enforce active low on chipselect handlePrior to the above commit, the chipselect polarity passed via cs-gpios property was ignored and considered active-low.
The reason for such mismatch is clearly explained in the comments inside
drivers/gpio/gpiolib-of.c:* SPI children have active low chip selects
* by default. This can be specified negatively
* by just omitting "spi-cs-high" in the
* device node, or actively by tagging on
*GPIO_ACTIVE_LOWas flag in the device
* tree. If the line is simultaneously
* tagged as active low in the device tree
* and has the "spi-cs-high" set, we get a
* conflict and the "spi-cs-high" flag will
* take precedence.
Quoted 4 lines
To properly represent the SPI chipselect polarity, change it to active-low when the "spi-cs-high" property is absent.
Signed-off-by Fabio Estevam <[email protected]>
Thanks for the fix!
Shouldn't we consider to add the following tag, as this is clearly a bug in the original implementation of the DTS and the commit that uncovers this is already in multiple stable releases?
Quoted 10 lines
--- arch/arm64/boot/dts/freescale/imx8mm-kontron-n801x-som.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/arm64/boot/dts/freescale/imx8mm-kontron-n801x-som.dtsi +0
-0
arch/arm64/boot/dts/freescale/imx8mm-kontron-n801x-som.dtsi opp-750M { · line 69
Quoted 8 lines
arch/arm64/boot/dts/freescale/imx8mm-kontron-n801x-som.dtsi +1
-1@@ -63,7 +63,7 @@ opp-750M { &ecspi1 { pinctrl-names = "default"; pinctrl-0 = <&pinctrl_ecspi1>; - cs-gpios = <&gpio5 9 GPIO_ACTIVE_HIGH>; + cs-gpios = <&gpio5 9 GPIO_ACTIVE_LOW>; status = "okay"; spi-flash@0 {
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Fabio Estevam
reviewerHi Shawn,
Quoted 7 lines · Frieder Schrempf
Thanks for the fix!
Reviewed-by Frieder Schrempf <[email protected]>Shouldn't we consider to add the following tag, as this is clearly a bug in the original implementation of the DTS and the commit that uncovers this is already in multiple stable releases?
Fixes 8668d8b2e67f ("arm64: dts: Add the Kontron i.MX8M Mini SoMs and baseboards")
Can you add the Fixes tag, or should I resend it?
Thanks
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 7 lines · Fabio Estevam
Hi Shawn,
Quoted 2 lines · Frieder Schrempf
Thanks for the fix!
Quoted 7 lines
Quoted 5 lines
Reviewed-by Frieder Schrempf <[email protected]>Shouldn't we consider to add the following tag, as this is clearly a bug in the original implementation of the DTS and the commit that uncovers this is already in multiple stable releases?
Fixes 8668d8b2e67f ("arm64: dts: Add the Kontron i.MX8M Mini SoMs and baseboards")Can you add the Fixes tag, or should I resend it?
Resend, please.
If this one needs a Fixes tag, the other one does too, right?
Shawn
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Shawn Guo
AppliedQuoted 24 lines · Fabio Estevam
The conversion of the spi-imx driver to use GPIO descriptors in commit
8cdcd8aeee28("spi: imx/fsl-lpspi: Convert to GPIO descriptors") helped to detect the following SPI chipselect polarity mismatch on an imx6q-sabresd for example:[ 4.854337] m25p80@0 enforce active low on chipselect handlePrior to the above commit, the chipselect polarity passed via cs-gpios property was ignored and considered active-low.
The reason for such mismatch is clearly explained in the comments inside
drivers/gpio/gpiolib-of.c:* SPI children have active low chip selects
* by default. This can be specified negatively
* by just omitting "spi-cs-high" in the
* device node, or actively by tagging on
*GPIO_ACTIVE_LOWas flag in the device
* tree. If the line is simultaneously
* tagged as active low in the device tree
* and has the "spi-cs-high" set, we get a
* conflict and the "spi-cs-high" flag will
* take precedence.
Quoted 4 lines
To properly represent the SPI chipselect polarity, change it to active-low when the "spi-cs-high" property is absent.
Signed-off-by Fabio Estevam <[email protected]>
Applied both, thanks!
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel