[PATCH] sdhci: Fix SD card detection issue

[PATCH] sdhci: Fix SD card detection issue

[Richard Clark]

The mmc_gpio_get_cd(...) will return 0 called from sdhci_get_cd(...), which means
the card is not present. Actually, the card detection pin is active low by default
according to the SDHCI psec, thus the card detection result is not correct, more
specificly below if condition is true in mmc_rescan(...):
	...
	if (mmc_card_is_removable(host) && host->ops->get_cd &&
		host->ops->get_cd(host) == 0) {
		...
		goto out;
	}
The SD card device will have no chance to be created.

This commit fixes this detection issue via the MMC_CAP2_CD_ACTIVE_HIGH cap2 flag,
parsed from the 'cd-inverted' property of DT.

Signed-off-by: Richard Clark <richard.xnu.clark@gmail.com>
---
 drivers/mmc/host/sdhci.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c79f73459915..79f33a161ca8 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2483,6 +2483,9 @@ static int sdhci_get_cd(struct mmc_host *mmc)
 	 * Try slot gpio detect, if defined it take precedence
 	 * over build in controller functionality
 	 */
+	if (!(mmc->caps2 & MMC_CAP2_CD_ACTIVE_HIGH))
+		gpio_cd = !gpio_cd;
+
 	if (gpio_cd >= 0)
 		return !!gpio_cd;
 
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] sdhci: Fix SD card detection issue

[Adrian Hunter]

On 15/04/24 10:06, Richard Clark wrote:
> The mmc_gpio_get_cd(...) will return 0 called from sdhci_get_cd(...), which means
> the card is not present. Actually, the card detection pin is active low by default
> according to the SDHCI psec, thus the card detection result is not correct, more

SDHCI spec covers the SDHCI lines.  GPIO is separate.

> specificly below if condition is true in mmc_rescan(...):
> 	...
> 	if (mmc_card_is_removable(host) && host->ops->get_cd &&
> 		host->ops->get_cd(host) == 0) {
> 		...
> 		goto out;
> 	}
> The SD card device will have no chance to be created.
> 
> This commit fixes this detection issue via the MMC_CAP2_CD_ACTIVE_HIGH cap2 flag,
> parsed from the 'cd-inverted' property of DT.

What hardware / driver is it?

> 
> Signed-off-by: Richard Clark <richard.xnu.clark@gmail.com>
> ---
>  drivers/mmc/host/sdhci.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index c79f73459915..79f33a161ca8 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2483,6 +2483,9 @@ static int sdhci_get_cd(struct mmc_host *mmc)
>  	 * Try slot gpio detect, if defined it take precedence
>  	 * over build in controller functionality
>  	 */
> +	if (!(mmc->caps2 & MMC_CAP2_CD_ACTIVE_HIGH))
> +		gpio_cd = !gpio_cd;

MMC_CAP2_CD_ACTIVE_HIGH is already handled in 
mmc_gpiod_request_cd(), and this turns an error (gpio_cd < 0)
into 0, which is not right.

> +
>  	if (gpio_cd >= 0)
>  		return !!gpio_cd;
>  


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] sdhci: Fix SD card detection issue

[Russell King (Oracle)]

On Mon, Apr 15, 2024 at 10:18:39AM +0300, Adrian Hunter wrote:
> On 15/04/24 10:06, Richard Clark wrote:
> > The mmc_gpio_get_cd(...) will return 0 called from sdhci_get_cd(...), which means
> > the card is not present. Actually, the card detection pin is active low by default
> > according to the SDHCI psec, thus the card detection result is not correct, more
> 
> SDHCI spec covers the SDHCI lines.  GPIO is separate.

... and the key bit of information that should be mentioned is in the
case of a GPIO, the GPIO library can be told if a GPIO is active-high
or active-low in either firmware or via the GPIO lookup data, and this
should be used instead of drivers inventing their own "quirking".

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] sdhci: Fix SD card detection issue

[richard clark]

On Mon, Apr 15, 2024 at 3:22 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Mon, Apr 15, 2024 at 10:18:39AM +0300, Adrian Hunter wrote:
> > On 15/04/24 10:06, Richard Clark wrote:
> > > The mmc_gpio_get_cd(...) will return 0 called from sdhci_get_cd(...), which means
> > > the card is not present. Actually, the card detection pin is active low by default
> > > according to the SDHCI psec, thus the card detection result is not correct, more
> >
> > SDHCI spec covers the SDHCI lines.  GPIO is separate.
>
> ... and the key bit of information that should be mentioned is in the
> case of a GPIO, the GPIO library can be told if a GPIO is active-high
> or active-low in either firmware or via the GPIO lookup data, and this
> should be used instead of drivers inventing their own "quirking".
>
Agree! But unfortunately, it seems I can't find the right place to
handle this from either firmware or via the GPIO lookup data. Will be
appreciated if any suggestion about that?!

> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] sdhci: Fix SD card detection issue

[Russell King (Oracle)]

On Mon, Apr 15, 2024 at 04:17:14PM +0800, richard clark wrote:
> On Mon, Apr 15, 2024 at 3:22 PM Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> >
> > On Mon, Apr 15, 2024 at 10:18:39AM +0300, Adrian Hunter wrote:
> > > On 15/04/24 10:06, Richard Clark wrote:
> > > > The mmc_gpio_get_cd(...) will return 0 called from sdhci_get_cd(...), which means
> > > > the card is not present. Actually, the card detection pin is active low by default
> > > > according to the SDHCI psec, thus the card detection result is not correct, more
> > >
> > > SDHCI spec covers the SDHCI lines.  GPIO is separate.
> >
> > ... and the key bit of information that should be mentioned is in the
> > case of a GPIO, the GPIO library can be told if a GPIO is active-high
> > or active-low in either firmware or via the GPIO lookup data, and this
> > should be used instead of drivers inventing their own "quirking".
> >
> Agree! But unfortunately, it seems I can't find the right place to
> handle this from either firmware or via the GPIO lookup data. Will be
> appreciated if any suggestion about that?!

If you're using DT, then, for example:

        cd-gpios = <&gpio1 4 GPIO_ACTIVE_LOW>;

is all it takes. If you are using firmware then GPIO lookup data isn't
what you should be using. I'm afraid I don't know the ACPI bindings for
SDHCI.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] sdhci: Fix SD card detection issue

[richard clark]

On Mon, Apr 15, 2024 at 4:23 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Mon, Apr 15, 2024 at 04:17:14PM +0800, richard clark wrote:
> > On Mon, Apr 15, 2024 at 3:22 PM Russell King (Oracle)
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Mon, Apr 15, 2024 at 10:18:39AM +0300, Adrian Hunter wrote:
> > > > On 15/04/24 10:06, Richard Clark wrote:
> > > > > The mmc_gpio_get_cd(...) will return 0 called from sdhci_get_cd(...), which means
> > > > > the card is not present. Actually, the card detection pin is active low by default
> > > > > according to the SDHCI psec, thus the card detection result is not correct, more
> > > >
> > > > SDHCI spec covers the SDHCI lines.  GPIO is separate.
> > >
> > > ... and the key bit of information that should be mentioned is in the
> > > case of a GPIO, the GPIO library can be told if a GPIO is active-high
> > > or active-low in either firmware or via the GPIO lookup data, and this
> > > should be used instead of drivers inventing their own "quirking".
> > >
> > Agree! But unfortunately, it seems I can't find the right place to
> > handle this from either firmware or via the GPIO lookup data. Will be
> > appreciated if any suggestion about that?!
>
> If you're using DT, then, for example:
>
>         cd-gpios = <&gpio1 4 GPIO_ACTIVE_LOW>;
>
> is all it takes. If you are using firmware then GPIO lookup data isn't
> what you should be using. I'm afraid I don't know the ACPI bindings for
> SDHCI.
>
Ah, this seems to be a bug of the Nvidia DT, its cd-gpios=<... 0x00>
meaning the GPIO_ACTIVE_HIGH, but the CD gpio value is 0 when the card
is inserted. In the kernel v5.10, the sdhci-tegra use below logic as
the card present indicator:

if (!host->mmc->cd_cap_invert)
        host->mmc->rem_card_present = (mmc_gpio_get_cd(host->mmc) == 0);

But the newer version kernel removes the 'rem_card_present', and the
CD gpio still value 0 will be interpreted as the card is not present,
thus the issue happens...

> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] sdhci: Fix SD card detection issue

[richard clark]

On Mon, Apr 15, 2024 at 3:18 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> On 15/04/24 10:06, Richard Clark wrote:
> > The mmc_gpio_get_cd(...) will return 0 called from sdhci_get_cd(...), which means
> > the card is not present. Actually, the card detection pin is active low by default
> > according to the SDHCI psec, thus the card detection result is not correct, more
>
> SDHCI spec covers the SDHCI lines.  GPIO is separate.
>
> > specificly below if condition is true in mmc_rescan(...):
> >       ...
> >       if (mmc_card_is_removable(host) && host->ops->get_cd &&
> >               host->ops->get_cd(host) == 0) {
> >               ...
> >               goto out;
> >       }
> > The SD card device will have no chance to be created.
> >
> > This commit fixes this detection issue via the MMC_CAP2_CD_ACTIVE_HIGH cap2 flag,
> > parsed from the 'cd-inverted' property of DT.
>
> What hardware / driver is it?
sdhci-tegra on Orin.
> >
> > Signed-off-by: Richard Clark <richard.xnu.clark@gmail.com>
> > ---
> >  drivers/mmc/host/sdhci.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > index c79f73459915..79f33a161ca8 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -2483,6 +2483,9 @@ static int sdhci_get_cd(struct mmc_host *mmc)
> >        * Try slot gpio detect, if defined it take precedence
> >        * over build in controller functionality
> >        */
> > +     if (!(mmc->caps2 & MMC_CAP2_CD_ACTIVE_HIGH))
> > +             gpio_cd = !gpio_cd;
>
> MMC_CAP2_CD_ACTIVE_HIGH is already handled in
> mmc_gpiod_request_cd(), and this turns an error (gpio_cd < 0)
> into 0, which is not right.

But in case of 'cd-inverted' is not specified, the gpio CD pin return
0 which will be explained as card is not present.
>
> > +
> >       if (gpio_cd >= 0)
> >               return !!gpio_cd;
> >
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] sdhci: Fix SD card detection issue

[Adrian Hunter]

+Nvidia guys

On 15/04/24 11:11, richard clark wrote:
> On Mon, Apr 15, 2024 at 3:18 PM Adrian Hunter <adrian.hunter@intel.com> wrote:
>>
>> On 15/04/24 10:06, Richard Clark wrote:
>>> The mmc_gpio_get_cd(...) will return 0 called from sdhci_get_cd(...), which means
>>> the card is not present. Actually, the card detection pin is active low by default
>>> according to the SDHCI psec, thus the card detection result is not correct, more
>>
>> SDHCI spec covers the SDHCI lines.  GPIO is separate.
>>
>>> specificly below if condition is true in mmc_rescan(...):
>>>       ...
>>>       if (mmc_card_is_removable(host) && host->ops->get_cd &&
>>>               host->ops->get_cd(host) == 0) {
>>>               ...
>>>               goto out;
>>>       }
>>> The SD card device will have no chance to be created.
>>>
>>> This commit fixes this detection issue via the MMC_CAP2_CD_ACTIVE_HIGH cap2 flag,
>>> parsed from the 'cd-inverted' property of DT.
>>
>> What hardware / driver is it?
> sdhci-tegra on Orin.
>>>
>>> Signed-off-by: Richard Clark <richard.xnu.clark@gmail.com>
>>> ---
>>>  drivers/mmc/host/sdhci.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index c79f73459915..79f33a161ca8 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -2483,6 +2483,9 @@ static int sdhci_get_cd(struct mmc_host *mmc)
>>>        * Try slot gpio detect, if defined it take precedence
>>>        * over build in controller functionality
>>>        */
>>> +     if (!(mmc->caps2 & MMC_CAP2_CD_ACTIVE_HIGH))
>>> +             gpio_cd = !gpio_cd;
>>
>> MMC_CAP2_CD_ACTIVE_HIGH is already handled in
>> mmc_gpiod_request_cd(), and this turns an error (gpio_cd < 0)
>> into 0, which is not right.
> 
> But in case of 'cd-inverted' is not specified, the gpio CD pin return
> 0 which will be explained as card is not present.
>>
>>> +
>>>       if (gpio_cd >= 0)
>>>               return !!gpio_cd;
>>>
>>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel