On 9/4/23 14:21, Ulf Hansson wrote:
On Fri, 1 Sept 2023 at 16:10, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
Hi Yann/Christophe,
thanks for your patch!
On Fri, Sep 1, 2023 at 2:08 PM Yann Gautier <yann.gautier@xxxxxxxxxxx> wrote:
From: Christophe Kerello <christophe.kerello@xxxxxxxxxxx>
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@xxxxxxxxxxx>
Signed-off-by: Yann Gautier <yann.gautier@xxxxxxxxxxx>
(...)
+++ 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;
Hi,
Should it really be changed to a bool?
Other boolean likes in the structure are u8:1.
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.
What I understand here is that in-band interrupts are not properly
working on ux500, and then the feature shouldn't be enabled for this
platform.
Am I correct?
If this is the case, the v2 will consist in changing the use_sdio_irq to
use_sdio_irq, and update the comment of the struct.
And depending on the answer, maybe change the field to bool.
Best regards,
Yann
Kind regards
Uffe
