Re: [PATCH 1/7] mmc: davinci_mmc: Map the virtual page for PIO

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jan 25, 2024, at 15:37, Linus Walleij wrote:
> Use kmap_local_page() instead of sg_virt() to obtain a page
> from the scatterlist: sg_virt() will not perform bounce
> buffering if the page happens to be located in high memory,
> which the driver may or may not be using.
>
> Suggested-by: Christoph Hellwig <hch@xxxxxx>
> Link: https://lore.kernel.org/linux-mmc/20240122073423.GA25859@xxxxxx/
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
>  drivers/mmc/host/davinci_mmc.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/davinci_mmc.c 
> b/drivers/mmc/host/davinci_mmc.c
> index ee3b1a4e0848..4e9f96b1caf3 100644
> --- a/drivers/mmc/host/davinci_mmc.c
> +++ b/drivers/mmc/host/davinci_mmc.c
> @@ -216,7 +216,7 @@ static irqreturn_t mmc_davinci_irq(int irq, void 
> *dev_id);
>  static void mmc_davinci_sg_to_buf(struct mmc_davinci_host *host)
>  {
>  	host->buffer_bytes_left = sg_dma_len(host->sg);
> -	host->buffer = sg_virt(host->sg);
> +	host->buffer = kmap_local_page(sg_page(host->sg));
>  	if (host->buffer_bytes_left > host->bytes_left)
>  		host->buffer_bytes_left = host->bytes_left;
>  }

I see multiple problems here:

 - you are missing the offset within the page, which you
   get by adding sg->offset

 - kmap_local_page() only maps one page at a time, so
   this will fail if the scatterlist entry spans one or
   more pages.

 - the first call to mmc_davinci_sg_to_buf() may happen
   in mmc_davinci_prepare_data(), while the rest is done
   in the interrupt handler, and you can't hold the
   kmap reference across multiple contexts

 - It looks like you are missing the unmap inside of
   loop when moving to the next sg element.

I think to do this properly, the driver would have to
use struct sg_mapping_iter like the cb710 driver does,
but the conversion is not as simple as your patch here.

       Arnd




[Index of Archives]     [Linux Memonry Technology]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux