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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c8178e61-31c1-4eb3-9d04-7aa70ed18634@microchip.com>
Date:   Fri, 10 Nov 2023 20:57:10 +0100
From:   Aubin Constans <aubin.constans@...rochip.com>
To:     Mathieu Moneyron <mathieu.moneyron@...il.com>
CC:     Adrian Hunter <adrian.hunter@...el.com>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Eugen Hristev <eugen.hristev@...labora.com>,
        Ludovic Desroches <ludovic.desroches@...rochip.com>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        <linux-mmc@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mmc: Fix force card detect in sdhci

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@...rochip.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@...il.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