Re: [PATCH] mmc: Fix force card detect in sdhci

[Aubin Constans <aubin.constans@microchip.com>]

On 23/09/2023 16:23, Mathieu Moneyron wrote:
> Hi Aubin,
> 
> Thanks for your feedback.
> I'll try my best to give clear explanations.
> 
> On Sat, Sep 23, 2023 at 2:44 PM Aubin Constans
> <aubin.constans@microchip.com> wrote:
>>
>> On 04/09/2023 09:38, Adrian Hunter wrote:
>>> + Eugen Hristev
>>> On 30/08/23 12:23, mathieu wrote:
>>>> On the ATMEL at91 when using the non-removable flag in device tree and not
>>>> using the card-detect pin inside the device-tree pinctrl, the card detect
>>>> pin is physically still used which can cause unknown behaviour when this
>>>> pin is used for other purposes.
>> Hi Mathieu,
>>
>> On which SoC(s) exactly, has this behaviour been observed?
> 
> This has been observed on SAMA5D27.
>>
>>
>> Also, has this issue been discussed in any separate support request, in such
>> case we could retrieve some background from it?
>>
>> By "unknown behaviour", do you mean "the card insertion status would
>> follow whatever electrical level is seen on the card-detect pin"?
> 
> For instance our board design has the PA13 (card detect) pin wired to drive a
> 3v3 power supply.
> The fixed regulator driver configure this pin as output (described in the device
> tree).
> But what I observed is that the sdmmc driver still want to use PA13 as the card
> detect signal which is then configured as an input. This cause the 3v3 power to
> drop.

In the device tree, has PIN_PA13__SDMMC0_CD been removed from the 
pinctrl for sdmmc0?

In case the above behaviour was due to hardware, it would likely point 
out a flaw in the Parallel Input/Output Controller (PIO) rather than in 
the SDMMC Controller.

Would you read back the relevant PIO_CFGRx register, with only "PA13" 
set in PIO_MSKRx, and provide me with the value read?
Assuming PA13 is Non-Secure; if PA13 is configured as Secure, 
S_PIO_CFGRx and S_PIO_MSKRx should be accessed instead.

> By "unknown behavior" I mean the configuration of this pin as card detect sets
> unwanted electrical levels to what's connected to this pin.
>>
>>
>>>> From my interpretation this seems to be caused by a hardware design flaw
>>>> and the real hardware is not working as intended by the documentation. >>>>
>>>> Signed-off-by: Mathieu Moneyron <mathieu.moneyron@gmail.com>
>>>>
>>>> ---
>>>>   drivers/mmc/host/sdhci-of-at91.c | 5 +++++
>>>>   1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/drivers/mmc/host/sdhci-of-at91.c
>>>> b/drivers/mmc/host/sdhci-of-at91.c
>>>> index 69fef88e7..4fd6bfbf6 100644
>>>> --- a/drivers/mmc/host/sdhci-of-at91.c
>>>> +++ b/drivers/mmc/host/sdhci-of-at91.c
>>>> @@ -51,10 +51,15 @@ struct sdhci_at91_priv {
>>>>   static void sdhci_at91_set_force_card_detect(struct sdhci_host *host)
>>>>   {
>>>>       u8 mc1r;
>>>> +    u8 ctrl;
>>>>
>>>>       mc1r = readb(host->ioaddr + SDMMC_MC1R);
>>>>       mc1r |= SDMMC_MC1R_FCD;
>>>>       writeb(mc1r, host->ioaddr + SDMMC_MC1R);
>>>> +
>>>> +    ctrl = readb(host->ioaddr + SDHCI_HOST_CONTROL);
>>>> +    ctrl |= SDHCI_CTRL_CDTEST_INS | SDHCI_CTRL_CDTEST_EN;
>>>> +    writeb(ctrl, host->ioaddr + SDHCI_HOST_CONTROL);
>>>>   }
>>>>
>>>>   static void sdhci_at91_set_clock(struct sdhci_host *host, unsigned
>>>> int clock)
>>
>> Kind regards,
>> Aubin
>>
> Kind regards,
> Mathieu

Thank you,
Aubin