diff mbox series

[PATCHv3,5/7] ARM: dts: exynos5422: use wp-gpio method to detect sd-card

Message ID 20180920175826.1284-6-linux.amoon@gmail.com (mailing list archive)
State New, archived
Headers show
Series Add support for UHS-I on Exynos5422 odroid boards | expand

Commit Message

Anand Moon Sept. 20, 2018, 5:58 p.m. UTC
add the Write Protect GPIO property for sdcard, to fix
the warning message during boot time.
of_get_named_gpiod_flags: can't parse 'wp-gpios' property of node '/soc/mmc@12220000[0]'

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
New patch to this series.
If we keep the wp-gpio GPIO_ACTIVE_LOW sdcard boot into RO mode.
---
 arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 1 +
 1 file changed, 1 insertion(+)

Comments

Krzysztof Kozlowski Sept. 21, 2018, 11:12 a.m. UTC | #1
On Thu, 20 Sep 2018 at 19:59, Anand Moon <linux.amoon@gmail.com> wrote:
>
> add the Write Protect GPIO property for sdcard, to fix
> the warning message during boot time.
> of_get_named_gpiod_flags: can't parse 'wp-gpios' property of node '/soc/mmc@12220000[0]'
>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> New patch to this series.
> If we keep the wp-gpio GPIO_ACTIVE_LOW sdcard boot into RO mode.
> ---
>  arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
> index 03fe02ceae66..475c38bbc99e 100644
> --- a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
> +++ b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
> @@ -370,6 +370,7 @@
>         cap-sd-highspeed;
>         max-frequency = <200000000>;
>         cd-gpios = <&gpc2 2 GPIO_ACTIVE_LOW>;
> +       wp-gpios = <&gpc4 0 GPIO_ACTIVE_HIGH>;

Two issues here:
1. You need also proper pinctrl setting.
2. On schematics the WP is not connected... so how do you know that it
is active high? How did you test it? How do you want to test it on
microSD?

Best regards,
Krzysztof

>         vmmc-supply = <&ldo19_reg>;
>         vqmmc-supply = <&ldo13_reg>;
>         sd-uhs-sdr50;
> --
> 2.17.1
>
Anand Moon Sept. 21, 2018, 12:31 p.m. UTC | #2
Hi Krzysztof,

On Fri, 21 Sep 2018 at 16:42, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Thu, 20 Sep 2018 at 19:59, Anand Moon <linux.amoon@gmail.com> wrote:
> >
> > add the Write Protect GPIO property for sdcard, to fix
> > the warning message during boot time.
> > of_get_named_gpiod_flags: can't parse 'wp-gpios' property of node '/soc/mmc@12220000[0]'
> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> > New patch to this series.
> > If we keep the wp-gpio GPIO_ACTIVE_LOW sdcard boot into RO mode.
> > ---
> >  arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
> > index 03fe02ceae66..475c38bbc99e 100644
> > --- a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
> > +++ b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
> > @@ -370,6 +370,7 @@
> >         cap-sd-highspeed;
> >         max-frequency = <200000000>;
> >         cd-gpios = <&gpc2 2 GPIO_ACTIVE_LOW>;
> > +       wp-gpios = <&gpc4 0 GPIO_ACTIVE_HIGH>;
>
> Two issues here:
> 1. You need also proper pinctrl setting.

Thanks for pointing out. Yes proper pinctrl is needed to support this setting.
probably pinctrl sd2_wp is needed to support this feature.

> 2. On schematics the WP is not connected... so how do you know that it
From XU4_MAIN_REV0.1 schematics page 7 "Micro Socket" their is no
connection to WP pin.
but below schematics page "EXYNOS 5422 MMC UFS" it's shows
XMMC2WP/GPC4_0/EXT_INT13_0.

> is active high? How did you test it? How do you want to test it on
> microSD?

I just tested this with the DTS change to GPIO_ACTIVE_LOW which
resulted the microSD card booted into RO mode.
So I set this to GPIO_ACTIVE_HIGH just to avoid warning.

I will study the changes requires and then post proper separate patch
to support this feature.

Best Regards
-Anand
>
> Best regards,
> Krzysztof
>
> >         vmmc-supply = <&ldo19_reg>;
> >         vqmmc-supply = <&ldo13_reg>;
> >         sd-uhs-sdr50;
> > --
> > 2.17.1
> >
Krzysztof Kozlowski Sept. 21, 2018, 9:09 p.m. UTC | #3
On Fri, Sep 21, 2018 at 06:01:07PM +0530, Anand Moon wrote:
> Hi Krzysztof,
> 
> On Fri, 21 Sep 2018 at 16:42, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > On Thu, 20 Sep 2018 at 19:59, Anand Moon <linux.amoon@gmail.com> wrote:
> > >
> > > add the Write Protect GPIO property for sdcard, to fix
> > > the warning message during boot time.
> > > of_get_named_gpiod_flags: can't parse 'wp-gpios' property of node '/soc/mmc@12220000[0]'
> > >
> > > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > > ---
> > > New patch to this series.
> > > If we keep the wp-gpio GPIO_ACTIVE_LOW sdcard boot into RO mode.
> > > ---
> > >  arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
> > > index 03fe02ceae66..475c38bbc99e 100644
> > > --- a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
> > > +++ b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
> > > @@ -370,6 +370,7 @@
> > >         cap-sd-highspeed;
> > >         max-frequency = <200000000>;
> > >         cd-gpios = <&gpc2 2 GPIO_ACTIVE_LOW>;
> > > +       wp-gpios = <&gpc4 0 GPIO_ACTIVE_HIGH>;
> >
> > Two issues here:
> > 1. You need also proper pinctrl setting.
> 
> Thanks for pointing out. Yes proper pinctrl is needed to support this setting.
> probably pinctrl sd2_wp is needed to support this feature.
> 
> > 2. On schematics the WP is not connected... so how do you know that it
> From XU4_MAIN_REV0.1 schematics page 7 "Micro Socket" their is no
> connection to WP pin.
> but below schematics page "EXYNOS 5422 MMC UFS" it's shows
> XMMC2WP/GPC4_0/EXT_INT13_0.
> 
> > is active high? How did you test it? How do you want to test it on
> > microSD?
> 
> I just tested this with the DTS change to GPIO_ACTIVE_LOW which
> resulted the microSD card booted into RO mode.
> So I set this to GPIO_ACTIVE_HIGH just to avoid warning.
> 
> I will study the changes requires and then post proper separate patch
> to support this feature.

Please squash it with cd-gpios change and send it only for the purpose
of describing the hardware. They do not have any meaningful change from
functional point of view (maybe except slower code as now it goes
through gpio subsystem instead of just reading dw-mmc registers).

Please use full sentences in commit msg (so start with capital letter)
and remove reference for warning because there is no warning.
You can follow up on my change for XU:
https://patchwork.kernel.org/patch/10610985/

Best regards,
Krzysztof
Anand Moon Sept. 22, 2018, 7:55 a.m. UTC | #4
Hi Krzysztof,
On Sat, 22 Sep 2018 at 02:39, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Fri, Sep 21, 2018 at 06:01:07PM +0530, Anand Moon wrote:
> > Hi Krzysztof,
> >
> > On Fri, 21 Sep 2018 at 16:42, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > >
> > > On Thu, 20 Sep 2018 at 19:59, Anand Moon <linux.amoon@gmail.com> wrote:
> > > >
> > > > add the Write Protect GPIO property for sdcard, to fix
> > > > the warning message during boot time.
> > > > of_get_named_gpiod_flags: can't parse 'wp-gpios' property of node '/soc/mmc@12220000[0]'
> > > >
> > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > > > ---
> > > > New patch to this series.
> > > > If we keep the wp-gpio GPIO_ACTIVE_LOW sdcard boot into RO mode.
> > > > ---
> > > >  arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
> > > > index 03fe02ceae66..475c38bbc99e 100644
> > > > --- a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
> > > > +++ b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
> > > > @@ -370,6 +370,7 @@
> > > >         cap-sd-highspeed;
> > > >         max-frequency = <200000000>;
> > > >         cd-gpios = <&gpc2 2 GPIO_ACTIVE_LOW>;
> > > > +       wp-gpios = <&gpc4 0 GPIO_ACTIVE_HIGH>;
> > >
> > > Two issues here:
> > > 1. You need also proper pinctrl setting.
> >
> > Thanks for pointing out. Yes proper pinctrl is needed to support this setting.
> > probably pinctrl sd2_wp is needed to support this feature.
> >
> > > 2. On schematics the WP is not connected... so how do you know that it
> > From XU4_MAIN_REV0.1 schematics page 7 "Micro Socket" their is no
> > connection to WP pin.
> > but below schematics page "EXYNOS 5422 MMC UFS" it's shows
> > XMMC2WP/GPC4_0/EXT_INT13_0.
> >
> > > is active high? How did you test it? How do you want to test it on
> > > microSD?
> >
> > I just tested this with the DTS change to GPIO_ACTIVE_LOW which
> > resulted the microSD card booted into RO mode.
> > So I set this to GPIO_ACTIVE_HIGH just to avoid warning.
> >
> > I will study the changes requires and then post proper separate patch
> > to support this feature.
>
> Please squash it with cd-gpios change and send it only for the purpose
> of describing the hardware. They do not have any meaningful change from
> functional point of view (maybe except slower code as now it goes
> through gpio subsystem instead of just reading dw-mmc registers).
>
> Please use full sentences in commit msg (so start with capital letter)
> and remove reference for warning because there is no warning.
> You can follow up on my change for XU:
> https://patchwork.kernel.org/patch/10610985/
>
Ok I have tried to squash the changes into single patch.

> Best regards,
> Krzysztof

Best Regards
-Anand
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
index 03fe02ceae66..475c38bbc99e 100644
--- a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
+++ b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
@@ -370,6 +370,7 @@ 
 	cap-sd-highspeed;
 	max-frequency = <200000000>;
 	cd-gpios = <&gpc2 2 GPIO_ACTIVE_LOW>;
+	wp-gpios = <&gpc4 0 GPIO_ACTIVE_HIGH>;
 	vmmc-supply = <&ldo19_reg>;
 	vqmmc-supply = <&ldo13_reg>;
 	sd-uhs-sdr50;