Hi Yann/Christophe, just a quick note: On Fri, Sep 1, 2023 at 2:08 PM Yann Gautier <yann.gautier@xxxxxxxxxxx> wrote: > +static void sdmmc_enable_sdio_irq(struct mmci_host *host, int enable) > +{ > + void __iomem *base = host->base; > + u32 mask = readl_relaxed(base + MMCIMASK0); > + > + if (enable) > + writel_relaxed(mask | MCI_ST_SDIOITMASK, base + MMCIMASK0); > + else > + writel_relaxed(mask & ~MCI_ST_SDIOITMASK, base + MMCIMASK0); > +} > + > +static void sdmmc_sdio_irq(struct mmci_host *host, u32 status) > +{ > + if (status & MCI_ST_SDIOIT) { > + sdmmc_enable_sdio_irq(host, 0); > + sdio_signal_irq(host->mmc); > + } > +} You need to move these to mmci and rename them since Ux500 will use the same callbacks. > static struct mmci_host_ops sdmmc_variant_ops = { > .validate_data = sdmmc_idma_validate_data, (...) > + .enable_sdio_irq = sdmmc_enable_sdio_irq, > + .sdio_irq = sdmmc_sdio_irq, > }; What about dropping the per-variant callbacks and just inline this into mmci_enable_sdio_irq()/mmci_ack_sdio_irq() since so many variants have the same scheme? I haven't looked at the Qualcomm variant though, maybe it is completely different... Yours, Linus Walleij