2019-01-01 22:07:27

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH 0/1] mmc: fix boards with CD GPIO and "cd-inverted"

As I explained in my original mail [0] I observed that some of my boards
were not detecting their SD card anymore:
- Meson8b Odroid-C1: the one one Kernel CI and another one on my desk
- Meson8b EC-100: on my desk
- Meson GXBB Odroid-C2: on Kernel CI

git bisect pointed to the following commit:
89a5e15bcba87d ("gpio/mmc/of: Respect polarity in the device tree")

The one-line change from this series is supposed to fix this issue for
v4.21 (mainline git master).
As explained in the patch description the MMC core code which manages
the GPIO inversion can be cleaned up once no driver depends on it
anymore (it seems that only one driver is left, but I don't have
hardware for it so I can't test it). More details can also be found
in my discussion with Linus on this topic: [1]


[0] https://marc.info/?l=linux-gpio&m=154609359306731&w=3
[1] https://marc.info/?l=linux-mmc&m=154626524506451&w=3


Martin Blumenstingl (1):
mmc: core: don't override the CD GPIO level when "cd-inverted" is set

drivers/mmc/core/host.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

--
2.20.1



2019-01-01 22:07:27

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH 1/1] mmc: core: don't override the CD GPIO level when "cd-inverted" is set

Since commit 89a5e15bcba87d ("gpio/mmc/of: Respect polarity in the device
tree") gpiolib-of parses the "cd-gpios" property and flips the polarity
if "cd-inverted" is also set. This results in the "cd-inverted" property
being evaluated twice, which effectively makes it a no-op:
- first in drivers/gpio/gpiolib-of.c (of_xlate_and_get_gpiod_flags) when
setting up the CD GPIO
- then again in drivers/mmc/core/slot-gpio.c (mmc_gpio_get_cd) when
reading the CD GPIO value at runtime

On boards which are using device-tree with the "cd-inverted" property
being set any inserted card are not detected anymore. This is due to the
MMC core treating the CD GPIO with the wrong polarity.

Disable "override_cd_active_level" for the card detection GPIO which is
parsed using mmc_of_parse. This fixes SD card detection on the boards
which are currently using the "cd-inverted" device-tree property (tested
on Meson8b Odroid-C1 and Meson8b EC-100).

This does not remove the CD GPIO inversion logic from the MMC core
because there's at least one driver (sdhci-pci-core for Intel BayTrail
based boards) which still passes "override_cd_active_level = true" to
mmc_gpiod_request_cd(). Due to lack of hardware for testing this is left
untouched.
In the future the GPIO inversion logic for both, card and read-only
detection can be removed once no driver is using it anymore.

Fixes: 89a5e15bcba87d ("gpio/mmc/of: Respect polarity in the device tree")
Signed-off-by: Martin Blumenstingl <[email protected]>
---
drivers/mmc/core/host.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index f57f5de54206..cf58ccaf22d5 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -234,7 +234,7 @@ int mmc_of_parse(struct mmc_host *host)
if (device_property_read_bool(dev, "broken-cd"))
host->caps |= MMC_CAP_NEEDS_POLL;

- ret = mmc_gpiod_request_cd(host, "cd", 0, true,
+ ret = mmc_gpiod_request_cd(host, "cd", 0, false,
cd_debounce_delay_ms * 1000,
&cd_gpio_invert);
if (!ret)
--
2.20.1


2019-01-04 03:24:15

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 1/1] mmc: core: don't override the CD GPIO level when "cd-inverted" is set

On Tue, Jan 1, 2019 at 8:44 PM Martin Blumenstingl
<[email protected]> wrote:

> Since commit 89a5e15bcba87d ("gpio/mmc/of: Respect polarity in the device
> tree") gpiolib-of parses the "cd-gpios" property and flips the polarity
> if "cd-inverted" is also set. This results in the "cd-inverted" property
> being evaluated twice, which effectively makes it a no-op:
> - first in drivers/gpio/gpiolib-of.c (of_xlate_and_get_gpiod_flags) when
> setting up the CD GPIO
> - then again in drivers/mmc/core/slot-gpio.c (mmc_gpio_get_cd) when
> reading the CD GPIO value at runtime
>
> On boards which are using device-tree with the "cd-inverted" property
> being set any inserted card are not detected anymore. This is due to the
> MMC core treating the CD GPIO with the wrong polarity.
>
> Disable "override_cd_active_level" for the card detection GPIO which is
> parsed using mmc_of_parse. This fixes SD card detection on the boards
> which are currently using the "cd-inverted" device-tree property (tested
> on Meson8b Odroid-C1 and Meson8b EC-100).
>
> This does not remove the CD GPIO inversion logic from the MMC core
> because there's at least one driver (sdhci-pci-core for Intel BayTrail
> based boards) which still passes "override_cd_active_level = true" to
> mmc_gpiod_request_cd(). Due to lack of hardware for testing this is left
> untouched.
> In the future the GPIO inversion logic for both, card and read-only
> detection can be removed once no driver is using it anymore.
>
> Fixes: 89a5e15bcba87d ("gpio/mmc/of: Respect polarity in the device tree")
> Signed-off-by: Martin Blumenstingl <[email protected]>

OK two steps forward, one step back, that's a good fix for now, I
can fix it properly for v4.22.
Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2019-01-08 22:59:12

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: [PATCH 1/1] mmc: core: don't override the CD GPIO level when "cd-inverted" is set

Hi Linus,

On Thu, Jan 3, 2019 at 11:06 PM Linus Walleij <[email protected]> wrote:
>
> On Tue, Jan 1, 2019 at 8:44 PM Martin Blumenstingl
> <[email protected]> wrote:
>
> > Since commit 89a5e15bcba87d ("gpio/mmc/of: Respect polarity in the device
> > tree") gpiolib-of parses the "cd-gpios" property and flips the polarity
> > if "cd-inverted" is also set. This results in the "cd-inverted" property
> > being evaluated twice, which effectively makes it a no-op:
> > - first in drivers/gpio/gpiolib-of.c (of_xlate_and_get_gpiod_flags) when
> > setting up the CD GPIO
> > - then again in drivers/mmc/core/slot-gpio.c (mmc_gpio_get_cd) when
> > reading the CD GPIO value at runtime
> >
> > On boards which are using device-tree with the "cd-inverted" property
> > being set any inserted card are not detected anymore. This is due to the
> > MMC core treating the CD GPIO with the wrong polarity.
> >
> > Disable "override_cd_active_level" for the card detection GPIO which is
> > parsed using mmc_of_parse. This fixes SD card detection on the boards
> > which are currently using the "cd-inverted" device-tree property (tested
> > on Meson8b Odroid-C1 and Meson8b EC-100).
> >
> > This does not remove the CD GPIO inversion logic from the MMC core
> > because there's at least one driver (sdhci-pci-core for Intel BayTrail
> > based boards) which still passes "override_cd_active_level = true" to
> > mmc_gpiod_request_cd(). Due to lack of hardware for testing this is left
> > untouched.
> > In the future the GPIO inversion logic for both, card and read-only
> > detection can be removed once no driver is using it anymore.
> >
> > Fixes: 89a5e15bcba87d ("gpio/mmc/of: Respect polarity in the device tree")
> > Signed-off-by: Martin Blumenstingl <[email protected]>
>
> OK two steps forward, one step back, that's a good fix for now, I
> can fix it properly for v4.22.
> Reviewed-by: Linus Walleij <[email protected]>
great, thanks for reviewing this!

feel free to keep me CC'ed on the "cleanup patches". I can review them
if you don't mind that I usually need a few days to do that


Regards
Martin

2019-01-09 20:51:38

by Anand Moon

[permalink] [raw]
Subject: Re: [PATCH 1/1] mmc: core: don't override the CD GPIO level when "cd-inverted" is set

Hi Martin,

On Wed, 2 Jan 2019 at 01:14, Martin Blumenstingl
<[email protected]> wrote:
>
> Since commit 89a5e15bcba87d ("gpio/mmc/of: Respect polarity in the device
> tree") gpiolib-of parses the "cd-gpios" property and flips the polarity
> if "cd-inverted" is also set. This results in the "cd-inverted" property
> being evaluated twice, which effectively makes it a no-op:
> - first in drivers/gpio/gpiolib-of.c (of_xlate_and_get_gpiod_flags) when
> setting up the CD GPIO
> - then again in drivers/mmc/core/slot-gpio.c (mmc_gpio_get_cd) when
> reading the CD GPIO value at runtime
>
> On boards which are using device-tree with the "cd-inverted" property
> being set any inserted card are not detected anymore. This is due to the
> MMC core treating the CD GPIO with the wrong polarity.
>
> Disable "override_cd_active_level" for the card detection GPIO which is
> parsed using mmc_of_parse. This fixes SD card detection on the boards
> which are currently using the "cd-inverted" device-tree property (tested
> on Meson8b Odroid-C1 and Meson8b EC-100).
>
> This does not remove the CD GPIO inversion logic from the MMC core
> because there's at least one driver (sdhci-pci-core for Intel BayTrail
> based boards) which still passes "override_cd_active_level = true" to
> mmc_gpiod_request_cd(). Due to lack of hardware for testing this is left
> untouched.
> In the future the GPIO inversion logic for both, card and read-only
> detection can be removed once no driver is using it anymore.
>
> Fixes: 89a5e15bcba87d ("gpio/mmc/of: Respect polarity in the device tree")
> Signed-off-by: Martin Blumenstingl <[email protected]>
> ---
> drivers/mmc/core/host.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index f57f5de54206..cf58ccaf22d5 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -234,7 +234,7 @@ int mmc_of_parse(struct mmc_host *host)
> if (device_property_read_bool(dev, "broken-cd"))
> host->caps |= MMC_CAP_NEEDS_POLL;
>
> - ret = mmc_gpiod_request_cd(host, "cd", 0, true,
> + ret = mmc_gpiod_request_cd(host, "cd", 0, false,
> cd_debounce_delay_ms * 1000,
> &cd_gpio_invert);
> if (!ret)
> --
> 2.20.1
>

Please add my.
Tested on Odroidc1+ and Odroidc2 both booted up fine.

Tested-by: Anand Moon <[email protected]>

>
> _______________________________________________
> linux-amlogic mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

2019-01-10 11:36:09

by Loys Ollivier

[permalink] [raw]
Subject: Re: [PATCH 1/1] mmc: core: don't override the CD GPIO level when "cd-inverted" is set

Hi Martin,

On Tue 01 Jan 2019 at 19:44, Martin Blumenstingl wrote:

> Since commit 89a5e15bcba87d ("gpio/mmc/of: Respect polarity in the device
> tree") gpiolib-of parses the "cd-gpios" property and flips the polarity
> if "cd-inverted" is also set. This results in the "cd-inverted" property
> being evaluated twice, which effectively makes it a no-op:
> - first in drivers/gpio/gpiolib-of.c (of_xlate_and_get_gpiod_flags) when
> setting up the CD GPIO
> - then again in drivers/mmc/core/slot-gpio.c (mmc_gpio_get_cd) when
> reading the CD GPIO value at runtime
>
> On boards which are using device-tree with the "cd-inverted" property
> being set any inserted card are not detected anymore. This is due to the
> MMC core treating the CD GPIO with the wrong polarity.
>
> Disable "override_cd_active_level" for the card detection GPIO which is
> parsed using mmc_of_parse. This fixes SD card detection on the boards
> which are currently using the "cd-inverted" device-tree property (tested
> on Meson8b Odroid-C1 and Meson8b EC-100).
>
> This does not remove the CD GPIO inversion logic from the MMC core
> because there's at least one driver (sdhci-pci-core for Intel BayTrail
> based boards) which still passes "override_cd_active_level = true" to
> mmc_gpiod_request_cd(). Due to lack of hardware for testing this is left
> untouched.
> In the future the GPIO inversion logic for both, card and read-only
> detection can be removed once no driver is using it anymore.
>
> Fixes: 89a5e15bcba87d ("gpio/mmc/of: Respect polarity in the device tree")
> Signed-off-by: Martin Blumenstingl <[email protected]>
> ---
> drivers/mmc/core/host.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index f57f5de54206..cf58ccaf22d5 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -234,7 +234,7 @@ int mmc_of_parse(struct mmc_host *host)
> if (device_property_read_bool(dev, "broken-cd"))
> host->caps |= MMC_CAP_NEEDS_POLL;
>
> - ret = mmc_gpiod_request_cd(host, "cd", 0, true,
> + ret = mmc_gpiod_request_cd(host, "cd", 0, false,
> cd_debounce_delay_ms * 1000,
> &cd_gpio_invert);
> if (!ret)

Test on libretech-cc, sd is fixed with that patch.

Tested-by: Loys Ollivier <[email protected]>
--
-L

2019-01-11 19:18:49

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 1/1] mmc: core: don't override the CD GPIO level when "cd-inverted" is set

On Tue, Jan 1, 2019 at 8:44 PM Martin Blumenstingl
<[email protected]> wrote:

> Since commit 89a5e15bcba87d ("gpio/mmc/of: Respect polarity in the device
> tree") gpiolib-of parses the "cd-gpios" property and flips the polarity
> if "cd-inverted" is also set. This results in the "cd-inverted" property
> being evaluated twice, which effectively makes it a no-op:
> - first in drivers/gpio/gpiolib-of.c (of_xlate_and_get_gpiod_flags) when
> setting up the CD GPIO
> - then again in drivers/mmc/core/slot-gpio.c (mmc_gpio_get_cd) when
> reading the CD GPIO value at runtime
>
> On boards which are using device-tree with the "cd-inverted" property
> being set any inserted card are not detected anymore. This is due to the
> MMC core treating the CD GPIO with the wrong polarity.
>
> Disable "override_cd_active_level" for the card detection GPIO which is
> parsed using mmc_of_parse. This fixes SD card detection on the boards
> which are currently using the "cd-inverted" device-tree property (tested
> on Meson8b Odroid-C1 and Meson8b EC-100).
>
> This does not remove the CD GPIO inversion logic from the MMC core
> because there's at least one driver (sdhci-pci-core for Intel BayTrail
> based boards) which still passes "override_cd_active_level = true" to
> mmc_gpiod_request_cd(). Due to lack of hardware for testing this is left
> untouched.
> In the future the GPIO inversion logic for both, card and read-only
> detection can be removed once no driver is using it anymore.
>
> Fixes: 89a5e15bcba87d ("gpio/mmc/of: Respect polarity in the device tree")
> Signed-off-by: Martin Blumenstingl <[email protected]>

I applied this to the GPIO fixes with the ACKs.

Ulf is on vacation but I managed to get his ACK over chat,
so I will try to get this upstream ASAP.

Yours,
Linus Walleij