ARM: ux500: Fix SD-card regression by using GPIO regulator
cover ARM: ux500: Fix SD-card regression by using GPIO regulator
This patchset fixes an old stability issue for detection of SD-cards. The DT node for the GPIO regulator which controls the I/O voltage level towards the SD-card was disabled in below commit.
c94a4ab7af3f ("ARM: ux500: Disable the MMCI gpio-regulator by default")
The conseqeunce from that commit, was that the initially chosen voltage level remained, even after I/O voltage switch sequence was performed for UHS-I cards, since no GPIO regulator could be fetched.
Appearantly some SD-cards didn't have any problem to cope with that, but some had.
Recently thorough error handling was introduced for the MMCI mmc host driver, while fetching the regulators for the SD-card. That actually made the driver to continously return -EPROBE_DEFER while probing, which thus meant failing all the times instead of just failing "sometimes".
The commit that introduced the thorough error handling is:9369c97cc7ec ("mmc: mmci: Cascade EPROBE_DEFER from regulators")
Ulf Hansson (3):
ARM: ux500: Move GPIO regulator for SD-card into board DTSs
ARM: ux500: Enable GPIO regulator for SD-card for HREF boards
ARM: ux500: Enable GPIO regulator for SD-card for snowball
arch/arm/boot/dts/ste-dbx5x0.dtsi | 17 ----------------- arch/arm/boot/dts/ste-href.dtsi | 15 +++++++++++++++ arch/arm/boot/dts/ste-snowball.dts | 13 +++++++++++++ 3 files changed, 28 insertions(+), 17 deletions(-)
Signature · 1 line
1.9.1
PATCH 1/3 ARM: ux500: Move GPIO regulator for SD-card into board DTSs
The GPIO regulator for the SD-card isn't a ux500 SOC configuration, but instead it's specific to the board. Move the definition of it, into the board DTSs.
--- arch/arm/boot/dts/ste-dbx5x0.dtsi | 17 ----------------- arch/arm/boot/dts/ste-href.dtsi | 17 +++++++++++++++++ arch/arm/boot/dts/ste-snowball.dts | 15 +++++++++++++++ 3 files changed, 32 insertions(+), 17 deletions(-)
arch/arm/boot/dts/ste-dbx5x0.dtsi +0 -17
@@ -1017,23 +1017,6 @@ status = "disabled"; }; - vmmci: regulator-gpio { - compatible = "regulator-gpio"; - - regulator-min-microvolt = <1800000>; - regulator-max-microvolt = <2900000>; - regulator-name = "mmci-reg"; - regulator-type = "voltage"; - - startup-delay-us = <100>; - enable-active-high; - - states = <1800000 0x1 - 2900000 0x0>; - - status = "disabled"; - }; - mcde at a0350000 { compatible = "stericsson,mcde"; reg = <0xa0350000 0x1000>, /* MCDE */
arch/arm/boot/dts/ste-href.dtsi +17 -0
@@ -111,6 +111,23 @@ pinctrl-1 = <&i2c3_sleep_mode>; }; + vmmci: regulator-gpio { + compatible = "regulator-gpio"; + + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <2900000>; + regulator-name = "mmci-reg"; + regulator-type = "voltage"; + + startup-delay-us = <100>; + enable-active-high; + + states = <1800000 0x1 + 2900000 0x0>; + + status = "disabled"; + }; + // External Micro SD slot sdi0_per1 at 80126000 { arm,primecell-periphid = <0x10480180>;
arch/arm/boot/dts/ste-snowball.dts +15 -0
@@ -146,8 +146,23 @@ }; vmmci: regulator-gpio { + compatible = "regulator-gpio"; + gpios = <&gpio7 4 0x4>; enable-gpio = <&gpio6 25 0x4>; + + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <2900000>; + regulator-name = "mmci-reg"; + regulator-type = "voltage"; + + startup-delay-us = <100>; + enable-active-high; + + states = <1800000 0x1 + 2900000 0x0>; + + status = "disabled"; }; // External Micro SD slot
Signature · 1 line
1.9.1
PATCH 1/3 ARM: ux500: Move GPIO regulator for SD-card into board DTSs
Quoted 3 lines · Ulf Hansson
The GPIO regulator for the SD-card isn't a ux500 SOC configuration, but instead it's specific to the board. Move the definition of it, into the board DTSs.
What makes you think that?
We normally place the common pieces (of which there are many in this node) in the highest level DTSI file, then add the platform specific ones in the DTS files.
Quoted 92 lines
Fixes c94a4ab7af3f ("ARM: ux500: Disable the MMCI gpio-regulator by default")Signed-off-by Ulf Hansson <[email protected]>--- arch/arm/boot/dts/ste-dbx5x0.dtsi | 17 ----------------- arch/arm/boot/dts/ste-href.dtsi | 17 +++++++++++++++++ arch/arm/boot/dts/ste-snowball.dts | 15 +++++++++++++++ 3 files changed, 32 insertions(+), 17 deletions(-)arch/arm/boot/dts/ste-dbx5x0.dtsi +0
-17@@ -1017,23 +1017,6 @@ status = "disabled"; }; - vmmci: regulator-gpio { - compatible = "regulator-gpio"; - - regulator-min-microvolt = <1800000>; - regulator-max-microvolt = <2900000>; - regulator-name = "mmci-reg"; - regulator-type = "voltage"; - - startup-delay-us = <100>; - enable-active-high; - - states = <1800000 0x1 - 2900000 0x0>; - - status = "disabled"; - }; - mcde at a0350000 { compatible = "stericsson,mcde"; reg = <0xa0350000 0x1000>, /* MCDE */arch/arm/boot/dts/ste-href.dtsi +17
-0@@ -111,6 +111,23 @@ pinctrl-1 = <&i2c3_sleep_mode>; }; + vmmci: regulator-gpio { + compatible = "regulator-gpio"; + + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <2900000>; + regulator-name = "mmci-reg"; + regulator-type = "voltage"; + + startup-delay-us = <100>; + enable-active-high; + + states = <1800000 0x1 + 2900000 0x0>; + + status = "disabled"; + }; + // External Micro SD slot sdi0_per1 at 80126000 { arm,primecell-periphid = <0x10480180>;arch/arm/boot/dts/ste-snowball.dts +15
-0@@ -146,8 +146,23 @@ }; vmmci: regulator-gpio { + compatible = "regulator-gpio"; + gpios = <&gpio7 4 0x4>; enable-gpio = <&gpio6 25 0x4>; + + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <2900000>; + regulator-name = "mmci-reg"; + regulator-type = "voltage"; + + startup-delay-us = <100>; + enable-active-high; + + states = <1800000 0x1 + 2900000 0x0>; + + status = "disabled"; }; // External Micro SD slot
PATCH 1/3 ARM: ux500: Move GPIO regulator for SD-card into board DTSs
Quoted 7 lines · Lee Jones
Quoted 3 lines · Ulf Hansson
The GPIO regulator for the SD-card isn't a ux500 SOC configuration, but instead it's specific to the board. Move the definition of it, into the board DTSs.
What makes you think that?
Because of how it was structured today.
ste-dbx5x0.dtsi - common for all ux500 boards, thus I considered this as the SoC configuration.
Then below are board configs which uses the above dtsi:ste-href.dtsi - common for href boards (used by ste-hrefprev60.dtsi and ste-hrefv60plus.dtsi), have vmmciste-snowball.dts, have vmmciste-ccu8540.dts, don't have vmmciste-ccu9540.dts, don't have vmmci
Quoted 4 lines
We normally place the common pieces (of which there are many in this node) in the highest level DTSI file, then add the platform specific ones in the DTS files.
Okay, so maybe it's due to the naming of ste-dbx5x0.dtsi, that I thought this was intended to cover the SoC configuration and not any of the platform specific stuff.
So what your advise of doing this?
Kind regards
Uffe
Quoted 93 lines
Quoted 93 lines
Fixes c94a4ab7af3f ("ARM: ux500: Disable the MMCI gpio-regulator by default")Signed-off-by Ulf Hansson <[email protected]>--- arch/arm/boot/dts/ste-dbx5x0.dtsi | 17 ----------------- arch/arm/boot/dts/ste-href.dtsi | 17 +++++++++++++++++ arch/arm/boot/dts/ste-snowball.dts | 15 +++++++++++++++ 3 files changed, 32 insertions(+), 17 deletions(-)arch/arm/boot/dts/ste-dbx5x0.dtsi +0
-17@@ -1017,23 +1017,6 @@ status = "disabled"; }; - vmmci: regulator-gpio { - compatible = "regulator-gpio"; - - regulator-min-microvolt = <1800000>; - regulator-max-microvolt = <2900000>; - regulator-name = "mmci-reg"; - regulator-type = "voltage"; - - startup-delay-us = <100>; - enable-active-high; - - states = <1800000 0x1 - 2900000 0x0>; - - status = "disabled"; - }; - mcde at a0350000 { compatible = "stericsson,mcde"; reg = <0xa0350000 0x1000>, /* MCDE */arch/arm/boot/dts/ste-href.dtsi +17
-0@@ -111,6 +111,23 @@ pinctrl-1 = <&i2c3_sleep_mode>; }; + vmmci: regulator-gpio { + compatible = "regulator-gpio"; + + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <2900000>; + regulator-name = "mmci-reg"; + regulator-type = "voltage"; + + startup-delay-us = <100>; + enable-active-high; + + states = <1800000 0x1 + 2900000 0x0>; + + status = "disabled"; + }; + // External Micro SD slot sdi0_per1 at 80126000 { arm,primecell-periphid = <0x10480180>;arch/arm/boot/dts/ste-snowball.dts +15
-0@@ -146,8 +146,23 @@ }; vmmci: regulator-gpio { + compatible = "regulator-gpio"; + gpios = <&gpio7 4 0x4>; enable-gpio = <&gpio6 25 0x4>; + + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <2900000>; + regulator-name = "mmci-reg"; + regulator-type = "voltage"; + + startup-delay-us = <100>; + enable-active-high; + + states = <1800000 0x1 + 2900000 0x0>; + + status = "disabled"; }; // External Micro SD slot
PATCH 1/3 ARM: ux500: Move GPIO regulator for SD-card into board DTSs
Quoted 7 lines · Ulf Hansson
Quoted 6 lines · Lee Jones
Quoted 3 lines · Ulf Hansson
The GPIO regulator for the SD-card isn't a ux500 SOC configuration, but instead it's specific to the board. Move the definition of it, into the board DTSs.
Quoted 6 lines
Quoted 1 line
What makes you think that?
Because of how it was structured today.
ste-dbx5x0.dtsi- common for all ux500 boards, thus I considered this as the SoC configuration.
ste-dbx5x0.dtsi is common for all ux500 and ux540 boards.
Quoted 6 lines
Then below are board configs which uses the above dtsi:
ste-href.dtsi- common for href boards (used byste-hrefprev60.dtsiandste-hrefv60plus.dtsi), have vmmciste-snowball.dts, have vmmciste-ccu8540.dts, don't have vmmciste-ccu9540.dts, don't have vmmci
Ah, got you. In which case it doesn't belong in ste-dbx5x0.dtsi.
Quoted 7 lines
Quoted 3 lines
We normally place the common pieces (of which there are many in this node) in the highest level DTSI file, then add the platform specific ones in the DTS files.
Okay, so maybe it's due to the naming of
ste-dbx5x0.dtsi, that I thought this was intended to cover the SoC configuration and not any of the platform specific stuff.
ste-dbx5x0.dtsi should cover all pieces which are common to all ux500 and ux540 devices. Then the lower level file ste-href-ab8500.dtsi should cover all pieces which are common to ux500 devices and finally the DTS files should add board specific information. Duplicate nodes and properties are frowned upon.
Quoted 1 line
So what your advise of doing this?
So the file which covers the x500 boards is ste-href-ab8500.dtsi. I would move the node into there instead. Keep it disabled and enable the associated nodes in the 2 DTS files.
Quoted 92 lines
Quoted 92 lines
Quoted 92 lines
Fixes c94a4ab7af3f ("ARM: ux500: Disable the MMCI gpio-regulator by default")Signed-off-by Ulf Hansson <[email protected]>--- arch/arm/boot/dts/ste-dbx5x0.dtsi | 17 ----------------- arch/arm/boot/dts/ste-href.dtsi | 17 +++++++++++++++++ arch/arm/boot/dts/ste-snowball.dts | 15 +++++++++++++++ 3 files changed, 32 insertions(+), 17 deletions(-)arch/arm/boot/dts/ste-dbx5x0.dtsi +0
-17@@ -1017,23 +1017,6 @@ status = "disabled"; }; - vmmci: regulator-gpio { - compatible = "regulator-gpio"; - - regulator-min-microvolt = <1800000>; - regulator-max-microvolt = <2900000>; - regulator-name = "mmci-reg"; - regulator-type = "voltage"; - - startup-delay-us = <100>; - enable-active-high; - - states = <1800000 0x1 - 2900000 0x0>; - - status = "disabled"; - }; - mcde at a0350000 { compatible = "stericsson,mcde"; reg = <0xa0350000 0x1000>, /* MCDE */arch/arm/boot/dts/ste-href.dtsi +17
-0@@ -111,6 +111,23 @@ pinctrl-1 = <&i2c3_sleep_mode>; }; + vmmci: regulator-gpio { + compatible = "regulator-gpio"; + + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <2900000>; + regulator-name = "mmci-reg"; + regulator-type = "voltage"; + + startup-delay-us = <100>; + enable-active-high; + + states = <1800000 0x1 + 2900000 0x0>; + + status = "disabled"; + }; + // External Micro SD slot sdi0_per1 at 80126000 { arm,primecell-periphid = <0x10480180>;arch/arm/boot/dts/ste-snowball.dts +15
-0@@ -146,8 +146,23 @@ }; vmmci: regulator-gpio { + compatible = "regulator-gpio"; + gpios = <&gpio7 4 0x4>; enable-gpio = <&gpio6 25 0x4>; + + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <2900000>; + regulator-name = "mmci-reg"; + regulator-type = "voltage"; + + startup-delay-us = <100>; + enable-active-high; + + states = <1800000 0x1 + 2900000 0x0>; + + status = "disabled"; }; // External Micro SD slot
PATCH 1/3 ARM: ux500: Move GPIO regulator for SD-card into board DTSs
Quoted 41 lines · Lee Jones
Quoted 7 lines · Ulf Hansson
Quoted 6 lines · Lee Jones
Quoted 3 lines · Ulf Hansson
The GPIO regulator for the SD-card isn't a ux500 SOC configuration, but instead it's specific to the board. Move the definition of it, into the board DTSs.
Quoted 6 lines
Quoted 1 line
What makes you think that?
Because of how it was structured today.
ste-dbx5x0.dtsi- common for all ux500 boards, thus I considered this as the SoC configuration.
ste-dbx5x0.dtsiis common for all ux500 and ux540 boards.Quoted 6 lines
Then below are board configs which uses the above dtsi:
ste-href.dtsi- common for href boards (used byste-hrefprev60.dtsiandste-hrefv60plus.dtsi), have vmmciste-snowball.dts, have vmmciste-ccu8540.dts, don't have vmmciste-ccu9540.dts, don't have vmmciAh, got you. In which case it doesn't belong in
ste-dbx5x0.dtsi.Quoted 7 lines
Quoted 3 lines
We normally place the common pieces (of which there are many in this node) in the highest level DTSI file, then add the platform specific ones in the DTS files.
Okay, so maybe it's due to the naming of
ste-dbx5x0.dtsi, that I thought this was intended to cover the SoC configuration and not any of the platform specific stuff.
ste-dbx5x0.dtsishould cover all pieces which are common to all ux500 and ux540 devices. Then the lower level fileste-href-ab8500.dtsishould cover all pieces which are common to ux500 devices and finally the DTS files should add board specific information. Duplicate nodes and properties are frowned upon.
Quoted 5 lines
Quoted 1 line
So what your advise of doing this?
So the file which covers the x500 boards is
ste-href-ab8500.dtsi. I would move the node into there instead. Keep it disabled and enable the associated nodes in the 2 DTS files.
Why ste-href-ab8500.dtsi? Isn't that suppose to cover configurations common to the ab8500 subsystem?
The vmmci models a board specific mounted circuit (aka level-shifter). Thus it exist on some boards but not on others.
Kind regards
Uffe
PATCH 1/3 ARM: ux500: Move GPIO regulator for SD-card into board DTSs
Quoted 42 lines · Ulf Hansson
Quoted 41 lines · Lee Jones
Quoted 7 lines · Ulf Hansson
Quoted 6 lines · Lee Jones
Quoted 3 lines · Ulf Hansson
The GPIO regulator for the SD-card isn't a ux500 SOC configuration, but instead it's specific to the board. Move the definition of it, into the board DTSs.
Quoted 6 lines
Quoted 1 line
What makes you think that?
Because of how it was structured today.
ste-dbx5x0.dtsi- common for all ux500 boards, thus I considered this as the SoC configuration.
ste-dbx5x0.dtsiis common for all ux500 and ux540 boards.Quoted 6 lines
Then below are board configs which uses the above dtsi:
ste-href.dtsi- common for href boards (used byste-hrefprev60.dtsiandste-hrefv60plus.dtsi), have vmmciste-snowball.dts, have vmmciste-ccu8540.dts, don't have vmmciste-ccu9540.dts, don't have vmmciAh, got you. In which case it doesn't belong in
ste-dbx5x0.dtsi.Quoted 7 lines
Quoted 3 lines
We normally place the common pieces (of which there are many in this node) in the highest level DTSI file, then add the platform specific ones in the DTS files.
Okay, so maybe it's due to the naming of
ste-dbx5x0.dtsi, that I thought this was intended to cover the SoC configuration and not any of the platform specific stuff.
ste-dbx5x0.dtsishould cover all pieces which are common to all ux500 and ux540 devices. Then the lower level fileste-href-ab8500.dtsishould cover all pieces which are common to ux500 devices and finally the DTS files should add board specific information. Duplicate nodes and properties are frowned upon.
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
So what your advise of doing this?
So the file which covers the x500 boards is
ste-href-ab8500.dtsi. I would move the node into there instead. Keep it disabled and enable the associated nodes in the 2 DTS files.Why
ste-href-ab8500.dtsi? Isn't that suppose to cover configurations common to the ab8500 subsystem?
Only because up until now that has been what is a) different from the abx5{40,05} platforms and b) common on abx500 ones.
However, the point of the DTS(I) hierarchy is to prevent duplication. Lower level DTSI files contain nodes which are similar to a sub-set of platforms, whereas the highest level DTSI files contain nodes which are shared between all associated platforms.
Quoted 2 lines
The vmmci models a board specific mounted circuit (aka level-shifter). Thus it exist on some boards but not on others.
Many of the peripherals we use on the boards are 'off-chip'. That does not preclude them from DTSI files if they are shared among various platforms.
Signature · 4 lines
Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org ? Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
PATCH 2/3 ARM: ux500: Enable GPIO regulator for SD-card for HREF boards
In reply to Ulf Hansson--- arch/arm/boot/dts/ste-href.dtsi | 2 -- 1 file changed, 2 deletions(-)
arch/arm/boot/dts/ste-href.dtsi +0 -2
@@ -124,8 +124,6 @@ states = <1800000 0x1 2900000 0x0>; - - status = "disabled"; }; // External Micro SD slot
Signature · 1 line
1.9.1
PATCH 2/3 ARM: ux500: Enable GPIO regulator for SD-card for HREF boards
Quoted 11 lines · Ulf Hansson
Fixes c94a4ab7af3f ("ARM: ux500: Disable the MMCI gpio-regulator by default")Signed-off-by Ulf Hansson <[email protected]>--- arch/arm/boot/dts/ste-href.dtsi | 2 -- 1 file changed, 2 deletions(-)arch/arm/boot/dts/ste-href.dtsi +0
-0
arch/arm/boot/dts/ste-href.dtsi line 127
Quoted 6 lines
arch/arm/boot/dts/ste-href.dtsi +0
-2@@ -124,8 +124,6 @@ states = <1800000 0x1 2900000 0x0>; - - status = "disabled"; };
Seems weird to do this in a separate patch.
Quoted 1 line
arch/arm/boot/dts/ste-href.dtsi +0
-0// External Micro SD slot
PATCH 3/3 ARM: ux500: Enable GPIO regulator for SD-card for snowball
In reply to Ulf Hansson--- arch/arm/boot/dts/ste-snowball.dts | 2 -- 1 file changed, 2 deletions(-)
arch/arm/boot/dts/ste-snowball.dts +0 -2
@@ -161,8 +161,6 @@ states = <1800000 0x1 2900000 0x0>; - - status = "disabled"; }; // External Micro SD slot
Signature · 1 line
1.9.1
cover ARM: ux500: Fix SD-card regression by using GPIO regulator
In reply to Ulf HanssonQuoted 4 lines · Ulf Hansson
This patchset fixes an old stability issue for detection of SD-cards. The DT node for the GPIO regulator which controls the I/O voltage level towards the SD-card was disabled in below commit.
[..]
Quoted 3 lines
The commit that introduced the thorough error handling is:
9369c97cc7ec("mmc: mmci: CascadeEPROBE_DEFERfrom regulators")
Glad I could help :)
Quoted 6 lines
Ulf Hansson (3):
ARM: ux500: Move GPIO regulator for SD-card into board DTSs
ARM: ux500: Enable GPIO regulator for SD-card for HREF boards
ARM: ux500: Enable GPIO regulator for SD-card for snowball
Looks good
Regards,
Bjorn
cover ARM: ux500: Fix SD-card regression by using GPIO regulator
In reply to Ulf HanssonWhere did you get this email address from?
Quoted 32 lines
This patchset fixes an old stability issue for detection of SD-cards. The DT node for the GPIO regulator which controls the I/O voltage level towards the SD-card was disabled in below commit.
c94a4ab7af3f("ARM: ux500: Disable the MMCI gpio-regulator by default")The conseqeunce from that commit, was that the initially chosen voltage level remained, even after I/O voltage switch sequence was performed for UHS-I cards, since no GPIO regulator could be fetched.
Appearantly some SD-cards didn't have any problem to cope with that, but some had.
Recently thorough error handling was introduced for the MMCI mmc host driver, while fetching the regulators for the SD-card. That actually made the driver to continously return -EPROBE_DEFER while probing, which thus meant failing all the times instead of just failing "sometimes".
The commit that introduced the thorough error handling is:
9369c97cc7ec("mmc: mmci: CascadeEPROBE_DEFERfrom regulators")Ulf Hansson (3):
ARM: ux500: Move GPIO regulator for SD-card into board DTSs
ARM: ux500: Enable GPIO regulator for SD-card for HREF boards
ARM: ux500: Enable GPIO regulator for SD-card for snowballarch/arm/boot/dts/ste-dbx5x0.dtsi | 17 ----------------- arch/arm/boot/dts/ste-href.dtsi | 15 +++++++++++++++ arch/arm/boot/dts/ste-snowball.dts | 13 +++++++++++++ 3 files changed, 28 insertions(+), 17 deletions(-)