diff mbox series

[RFC,v1,1/1] mmc: zynq_sdhci: Only evaluate card-stable signal if card was detected

Message ID 20240424082343.212554-2-lukas.funke-oss@weidmueller.com
State RFC
Delegated to: Michal Simek
Headers show
Series mmc: zynq_sdhci: Only evaluate card-stable signal if card was detected | expand

Commit Message

Lukas Funke April 24, 2024, 8:23 a.m. UTC 
From: Lukas Funke <lukas.funke@weidmueller.com>

On ZynqMp there seems to be a dependency between the card-stable bit and
the card-detect bit. The card-stable bit is set *if and only if*
the card-detect bit was set before, indicating that the signal was
stable and reliable during card insertion.

If the card-detect bit is *not* evaluated the corresponding check leads
to a timeout indicating that the card-detect was not stable.

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
---

 drivers/mmc/zynq_sdhci.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Michal Simek May 29, 2024, 3:03 p.m. UTC | #1 
On 4/24/24 10:23, lukas.funke-oss@weidmueller.com wrote:
> From: Lukas Funke <lukas.funke@weidmueller.com>
> 
> On ZynqMp there seems to be a dependency between the card-stable bit and
> the card-detect bit. The card-stable bit is set *if and only if*
> the card-detect bit was set before, indicating that the signal was
> stable and reliable during card insertion.
> 
> If the card-detect bit is *not* evaluated the corresponding check leads
> to a timeout indicating that the card-detect was not stable.
> 
> Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
> ---
> 
>   drivers/mmc/zynq_sdhci.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
> index 935540d171..d0bccd41cc 100644
> --- a/drivers/mmc/zynq_sdhci.c
> +++ b/drivers/mmc/zynq_sdhci.c
> @@ -1168,11 +1168,14 @@ static int arasan_sdhci_probe(struct udevice *dev)
>   	 */
>   	if (IS_ENABLED(CONFIG_ARCH_ZYNQMP) || IS_ENABLED(CONFIG_ARCH_VERSAL)) {
>   		u32 timeout = 1000000;
> +		u32 value;
>   
> -		while (((sdhci_readl(host, SDHCI_PRESENT_STATE) &
> -			 SDHCI_CARD_STATE_STABLE) == 0) && timeout) {
> +		value = sdhci_readl(host, SDHCI_PRESENT_STATE);
> +		while ((value & SDHCI_CARD_PRESENT) &&
> +		       ((value & SDHCI_CARD_STATE_STABLE) == 0) && timeout) {
>   			udelay(1);
>   			timeout--;
> +			value = sdhci_readl(host, SDHCI_PRESENT_STATE);
>   		}
>   		if (!timeout) {
>   			dev_err(dev, "Sdhci card detect state not stable\n");

Venkatesh: Can you please take a look at this?

Thanks,
Michal
Venkatesh Yadav Abbarapu May 30, 2024, 5:39 a.m. UTC | #2 
Hi Lukas,
The polling of card-stable bit alone is enough, as if card-detect bit is not set anyways the card-stable bit won't be set which leads to timeout.
I don’t think it is specific to zynqmp platform.

Could you please let me know if you observe this issue on any of the zynqmp board? 

Thanks
Venkatesh
> -----Original Message-----
> From: Simek, Michal <michal.simek@amd.com>
> Sent: Wednesday, May 29, 2024 8:33 PM
> To: lukas.funke-oss@weidmueller.com; u-boot@lists.denx.de; Abbarapu,
> Venkatesh <venkatesh.abbarapu@amd.com>
> Cc: Stefan Herbrechtsmeier <Stefan.Herbrechtsmeier@weidmueller.com>;
> Ashok Reddy Soma <ashok.reddy.soma@amd.com>; Lukas Funke
> <lukas.funke@weidmueller.com>; Jaehoon Chung
> <jh80.chung@samsung.com>; Johan Jonker <jbx6244@gmail.com>; Peng Fan
> <peng.fan@nxp.com>; Simon Glass <sjg@chromium.org>; Tom Rini
> <trini@konsulko.com>
> Subject: Re: [RFC PATCH v1 1/1] mmc: zynq_sdhci: Only evaluate card-stable
> signal if card was detected
> 
> 
> 
> On 4/24/24 10:23, lukas.funke-oss@weidmueller.com wrote:
> > From: Lukas Funke <lukas.funke@weidmueller.com>
> >
> > On ZynqMp there seems to be a dependency between the card-stable bit
> > and the card-detect bit. The card-stable bit is set *if and only if*
> > the card-detect bit was set before, indicating that the signal was
> > stable and reliable during card insertion.
> >
> > If the card-detect bit is *not* evaluated the corresponding check
> > leads to a timeout indicating that the card-detect was not stable.
> >
> > Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
> > ---
> >
> >   drivers/mmc/zynq_sdhci.c | 7 +++++--
> >   1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c index
> > 935540d171..d0bccd41cc 100644
> > --- a/drivers/mmc/zynq_sdhci.c
> > +++ b/drivers/mmc/zynq_sdhci.c
> > @@ -1168,11 +1168,14 @@ static int arasan_sdhci_probe(struct udevice
> *dev)
> >   	 */
> >   	if (IS_ENABLED(CONFIG_ARCH_ZYNQMP) ||
> IS_ENABLED(CONFIG_ARCH_VERSAL)) {
> >   		u32 timeout = 1000000;
> > +		u32 value;
> >
> > -		while (((sdhci_readl(host, SDHCI_PRESENT_STATE) &
> > -			 SDHCI_CARD_STATE_STABLE) == 0) && timeout) {
> > +		value = sdhci_readl(host, SDHCI_PRESENT_STATE);
> > +		while ((value & SDHCI_CARD_PRESENT) &&
> > +		       ((value & SDHCI_CARD_STATE_STABLE) == 0) && timeout)
> {
> >   			udelay(1);
> >   			timeout--;
> > +			value = sdhci_readl(host, SDHCI_PRESENT_STATE);
> >   		}
> >   		if (!timeout) {
> >   			dev_err(dev, "Sdhci card detect state not stable\n");
> 
> Venkatesh: Can you please take a look at this?
> 
> Thanks,
> Michal
Lukas Funke May 31, 2024, 7:35 a.m. UTC | #3 
Hi Venkatesh,

On 30.05.2024 07:39, Abbarapu, Venkatesh wrote:
> Hi Lukas,
> The polling of card-stable bit alone is enough, as if card-detect bit is not set anyways the card-stable bit won't be set which leads to timeout.
> I don’t think it is specific to zynqmp platform.
> 
> Could you please let me know if you observe this issue on any of the zynqmp board?

We can observe this behaviour on zu2cg and zu3eg. I haven't tried it on 
an eval-board but this would be the next step.

The main problem is that the stable-bit is not set if there is no 
sd-card plugged in from the beginning, but only if the cd-signal changes.

Is there any documentation how this bit should behave then?

Best regards
- Lukas

> 
> Thanks
> Venkatesh
>> -----Original Message-----
>> From: Simek, Michal <michal.simek@amd.com>
>> Sent: Wednesday, May 29, 2024 8:33 PM
>> To: lukas.funke-oss@weidmueller.com; u-boot@lists.denx.de; Abbarapu,
>> Venkatesh <venkatesh.abbarapu@amd.com>
>> Cc: Stefan Herbrechtsmeier <Stefan.Herbrechtsmeier@weidmueller.com>;
>> Ashok Reddy Soma <ashok.reddy.soma@amd.com>; Lukas Funke
>> <lukas.funke@weidmueller.com>; Jaehoon Chung
>> <jh80.chung@samsung.com>; Johan Jonker <jbx6244@gmail.com>; Peng Fan
>> <peng.fan@nxp.com>; Simon Glass <sjg@chromium.org>; Tom Rini
>> <trini@konsulko.com>
>> Subject: Re: [RFC PATCH v1 1/1] mmc: zynq_sdhci: Only evaluate card-stable
>> signal if card was detected
>>
>>
>>
>> On 4/24/24 10:23, lukas.funke-oss@weidmueller.com wrote:
>>> From: Lukas Funke <lukas.funke@weidmueller.com>
>>>
>>> On ZynqMp there seems to be a dependency between the card-stable bit
>>> and the card-detect bit. The card-stable bit is set *if and only if*
>>> the card-detect bit was set before, indicating that the signal was
>>> stable and reliable during card insertion.
>>>
>>> If the card-detect bit is *not* evaluated the corresponding check
>>> leads to a timeout indicating that the card-detect was not stable.
>>>
>>> Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
>>> ---
>>>
>>>    drivers/mmc/zynq_sdhci.c | 7 +++++--
>>>    1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c index
>>> 935540d171..d0bccd41cc 100644
>>> --- a/drivers/mmc/zynq_sdhci.c
>>> +++ b/drivers/mmc/zynq_sdhci.c
>>> @@ -1168,11 +1168,14 @@ static int arasan_sdhci_probe(struct udevice
>> *dev)
>>>    	 */
>>>    	if (IS_ENABLED(CONFIG_ARCH_ZYNQMP) ||
>> IS_ENABLED(CONFIG_ARCH_VERSAL)) {
>>>    		u32 timeout = 1000000;
>>> +		u32 value;
>>>
>>> -		while (((sdhci_readl(host, SDHCI_PRESENT_STATE) &
>>> -			 SDHCI_CARD_STATE_STABLE) == 0) && timeout) {
>>> +		value = sdhci_readl(host, SDHCI_PRESENT_STATE);
>>> +		while ((value & SDHCI_CARD_PRESENT) &&
>>> +		       ((value & SDHCI_CARD_STATE_STABLE) == 0) && timeout)
>> {
>>>    			udelay(1);
>>>    			timeout--;
>>> +			value = sdhci_readl(host, SDHCI_PRESENT_STATE);
>>>    		}
>>>    		if (!timeout) {
>>>    			dev_err(dev, "Sdhci card detect state not stable\n");
>>
>> Venkatesh: Can you please take a look at this?
>>
>> Thanks,
>> Michal
diff mbox series

Patch

diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
index 935540d171..d0bccd41cc 100644
--- a/drivers/mmc/zynq_sdhci.c
+++ b/drivers/mmc/zynq_sdhci.c
@@ -1168,11 +1168,14 @@  static int arasan_sdhci_probe(struct udevice *dev)
 	 */
 	if (IS_ENABLED(CONFIG_ARCH_ZYNQMP) || IS_ENABLED(CONFIG_ARCH_VERSAL)) {
 		u32 timeout = 1000000;
+		u32 value;
 
-		while (((sdhci_readl(host, SDHCI_PRESENT_STATE) &
-			 SDHCI_CARD_STATE_STABLE) == 0) && timeout) {
+		value = sdhci_readl(host, SDHCI_PRESENT_STATE);
+		while ((value & SDHCI_CARD_PRESENT) &&
+		       ((value & SDHCI_CARD_STATE_STABLE) == 0) && timeout) {
 			udelay(1);
 			timeout--;
+			value = sdhci_readl(host, SDHCI_PRESENT_STATE);
 		}
 		if (!timeout) {
 			dev_err(dev, "Sdhci card detect state not stable\n");