linux-kernel - Re: [PATCH] mmc: mmci: stm32: add SDIO in-band interrupt mode

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]

Date:   Mon, 4 Sep 2023 14:21:35 +0200
From:   Ulf Hansson <ulf.hansson@...aro.org>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     Yann Gautier <yann.gautier@...s.st.com>,
        Russell King <linux@...linux.org.uk>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        Christophe Kerello <christophe.kerello@...s.st.com>,
        Yang Yingliang <yangyingliang@...wei.com>,
        Rob Herring <robh@...nel.org>, linux-mmc@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        linux-stm32@...md-mailman.stormreply.com,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH] mmc: mmci: stm32: add SDIO in-band interrupt mode

On Fri, 1 Sept 2023 at 16:10, Linus Walleij <linus.walleij@...aro.org> wrote:
>
> Hi Yann/Christophe,
>
> thanks for your patch!
>
> On Fri, Sep 1, 2023 at 2:08 PM Yann Gautier <yann.gautier@...s.st.com> wrote:
>
> > From: Christophe Kerello <christophe.kerello@...s.st.com>
> >
> > Add the support of SDIO in-band interrupt mode for STM32 variant.
> > It allows the SD I/O card to interrupt the host on SDMMC_D1 data line.
> >
> > Signed-off-by: Christophe Kerello <christophe.kerello@...s.st.com>
> > Signed-off-by: Yann Gautier <yann.gautier@...s.st.com>
> (...)
> > +++ b/drivers/mmc/host/mmci.h
> > @@ -332,6 +332,7 @@ enum mmci_busy_state {
> >   * @opendrain: bitmask identifying the OPENDRAIN bit inside MMCIPOWER register
> >   * @dma_lli: true if variant has dma link list feature.
> >   * @stm32_idmabsize_mask: stm32 sdmmc idma buffer size.
> > + * @use_sdio_irq: allow SD I/O card to interrupt the host
>
> The documentation tag should be one line up (compare to the members...)
>
> > @@ -376,6 +377,7 @@ struct variant_data {
> >         u32                     start_err;
> >         u32                     opendrain;
> >         u8                      dma_lli:1;
> > +       u8                      use_sdio_irq:1;
>
> 1. bool use_sdio_irq;
>
> 2. supports_sdio_irq is more to the point don't you think?
>     Especially since it activates these two callbacks:
>
> > +       void (*enable_sdio_irq)(struct mmci_host *host, int enable);
> > +       void (*sdio_irq)(struct mmci_host *host, u32 status);
>
> Further: all the Ux500 variants support this (bit 22) as well, so enable those
> too in their vendor data. All I have is out-of-band signaling with an GPIO IRQ
> on my Broadcom chips but I think it works (maybe Ulf has tested it in the
> far past).

For the ux500 variant there is a HW problem. After running some stress
tests, we may end up being stuck waiting for an SDIO IRQ to be
delivered. Even if the SDIO irqs should be considered level triggered,
it looked like it was implemented in the HW as an edge triggered IRQ.

The downstream workaround consisted of re-routing the DAT1 to a GPIO
at runtime suspend (we wanted that for optimal power save support
anyway) - and manually checking if the DAT1 line was asserted, before
enabling the GPIO line for an irq. This worked perfectly fine as a
workaround, with the limitation that one may observe a little bit of
hick-up in the traffic occasionally.

That said, the out-of-band IRQs is what works best for the ux500 variants.

Kind regards
Uffe