On 22/02/24 02:00, Linus Walleij wrote:
> The sg_miter used to loop over the returned sglist from a
> transfer in the esdhc subdriver for SDHCI needs to know if
> it is being used in process or atomic context.
>
> This creates a problem because we cannot unconditionally
> add SG_MITER_ATOMIC to the miter, as that can create
> scheduling while atomic dmesg splats.
>
> Bit the bullet and make the .request_done() callback in
> struct sdhci_ops aware of whether it is called from atomic
> context or not, and only add the flag when actually called
> from atomic context.
>
> sdhci_request_done() is always called from process context,
> either as a work or as part of the threaded interrupt handler,
> so this will pass false for is_atomic, and the one case when
> we are actually calling .request_done() from an atomic context
> is in sdhci_irq().
>
> Fixes: e8a167b84886 ("mmc: sdhci-esdhc-mcf: Use sg_miter for swapping")
I notice, in fact, that the driver is already using a bounce
buffer always:
static int sdhci_esdhc_mcf_probe(struct platform_device *pdev)
...
if (!host->bounce_buffer) {
dev_err(&pdev->dev, "bounce buffer not allocated");
err = -ENOMEM;
goto cleanup;
}
...
Doesn't that mean the original patch is not needed?
> Reported-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> drivers/mmc/host/sdhci-esdhc-mcf.c | 8 ++++++--
> drivers/mmc/host/sdhci-pxav2.c | 2 +-
> drivers/mmc/host/sdhci-sprd.c | 3 ++-
> drivers/mmc/host/sdhci.c | 4 ++--
> drivers/mmc/host/sdhci.h | 3 ++-
> 5 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-mcf.c b/drivers/mmc/host/sdhci-esdhc-mcf.c
> index 1909a11fd065..7b0686701c6e 100644
> --- a/drivers/mmc/host/sdhci-esdhc-mcf.c
> +++ b/drivers/mmc/host/sdhci-esdhc-mcf.c
> @@ -297,8 +297,10 @@ static void esdhc_mcf_pltfm_set_bus_width(struct sdhci_host *host, int width)
> }
>
> static void esdhc_mcf_request_done(struct sdhci_host *host,
> - struct mmc_request *mrq)
> + struct mmc_request *mrq,
> + bool is_atomic)
> {
> + unsigned int miter_flags = SG_MITER_TO_SG | SG_MITER_FROM_SG;
> struct sg_mapping_iter sgm;
> u32 *buffer;
>
> @@ -312,8 +314,10 @@ static void esdhc_mcf_request_done(struct sdhci_host *host,
> * On mcf5441x there is no hw sdma option/flag to select the dma
> * transfer endiannes. A swap after the transfer is needed.
> */
> + if (is_atomic)
> + miter_flags |= SG_MITER_ATOMIC;
> sg_miter_start(&sgm, mrq->data->sg, mrq->data->sg_len,
> - SG_MITER_TO_SG | SG_MITER_FROM_SG);
> + miter_flags);
> while (sg_miter_next(&sgm)) {
> buffer = sgm.addr;
> esdhc_mcf_buffer_swap32(buffer, sgm.length);
> diff --git a/drivers/mmc/host/sdhci-pxav2.c b/drivers/mmc/host/sdhci-pxav2.c
> index b75cbea88b40..c1b49c2feea8 100644
> --- a/drivers/mmc/host/sdhci-pxav2.c
> +++ b/drivers/mmc/host/sdhci-pxav2.c
> @@ -120,7 +120,7 @@ static u32 pxav1_irq(struct sdhci_host *host, u32 intmask)
> return intmask;
> }
>
> -static void pxav1_request_done(struct sdhci_host *host, struct mmc_request *mrq)
> +static void pxav1_request_done(struct sdhci_host *host, struct mmc_request *mrq, bool is_atomic)
> {
> u16 tmp;
> struct sdhci_pxav2_host *pxav2_host;
> diff --git a/drivers/mmc/host/sdhci-sprd.c b/drivers/mmc/host/sdhci-sprd.c
> index bed57a1c64b5..a2b244e13451 100644
> --- a/drivers/mmc/host/sdhci-sprd.c
> +++ b/drivers/mmc/host/sdhci-sprd.c
> @@ -411,7 +411,8 @@ static unsigned int sdhci_sprd_get_ro(struct sdhci_host *host)
> }
>
> static void sdhci_sprd_request_done(struct sdhci_host *host,
> - struct mmc_request *mrq)
> + struct mmc_request *mrq,
> + bool is_atomic)
> {
> /* Validate if the request was from software queue firstly. */
> if (mmc_hsq_finalize_request(host->mmc, mrq))
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index c79f73459915..3c6a9ba573cc 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -3184,7 +3184,7 @@ static bool sdhci_request_done(struct sdhci_host *host)
> spin_unlock_irqrestore(&host->lock, flags);
>
> if (host->ops->request_done)
> - host->ops->request_done(host, mrq);
> + host->ops->request_done(host, mrq, false);
> else
> mmc_request_done(host->mmc, mrq);
>
> @@ -3639,7 +3639,7 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
> continue;
>
> if (host->ops->request_done)
> - host->ops->request_done(host, mrqs_done[i]);
> + host->ops->request_done(host, mrqs_done[i], true);
> else
> mmc_request_done(host->mmc, mrqs_done[i]);
> }
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index a20864fc0641..c2ff327e2c96 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -666,7 +666,8 @@ struct sdhci_ops {
> struct mmc_data *data,
> unsigned int length);
> void (*request_done)(struct sdhci_host *host,
> - struct mmc_request *mrq);
> + struct mmc_request *mrq,
> + bool is_atomic);
> void (*dump_vendor_regs)(struct sdhci_host *host);
> };
>
>
> ---
> base-commit: 2d5c7b7eb345249cb34d42cbc2b97b4c57ea944e
> change-id: 20240222-fix-sdhci-esdhc-mcf-aa062c072800
>
> Best regards,
