Hi,

I finally found some time.

On Mon, Mar 07, 2011 at 10:40:57PM +0200, Mikko Vinni wrote:
> Some SD host controllers (noticed on an integrated JMicron SD reader
> on an HP Pavilion dv5-1250eo laptop) don't update the dma address
> register before signaling a dma interrupt due to a dma boundary.
> Detect this and update the register to the next 512KB boundary,
> at which the transfer presumably stopped.
> 
> As long as each transfer is at most 512KB in size (on this hardware
> the max seems to be 65536 bytes), this fix is needed at most once
> per transfer.

But we can't guarantee that. Transfer could be up to 65535 * 2K.

> Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=28462
> 
> Signed-off-by: Mikko Vinni <mmvinni <at> yahoo.com>

Proper EMail please.

> 
> ---
> Sent on 2011-02-21 21:23:32 GMT
> (http://thread.gmane.org/gmane.linux.kernel.mmc/5568/focus=6145)
> 
> Hoping to be able to drop this patch eventually from my own
> repo and have the hardware just work with mainline code.
> Maybe first in -next if nobody sees any serious problems
> straight away?
> 
> This patch should not break anything for anybody whose
> hardware isn't already broken.
> 
>  drivers/mmc/host/sdhci.c |   24 +++++++++++++++++++++---
>  1 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index a25db42..8651731 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1537,9 +1537,27 @@ static void sdhci_data_irq(struct sdhci_host *host, 
> u32 intmask)
>                * boundaries, but as we can't disable the feature
>                * we need to at least restart the transfer.
>                */
> -             if (intmask & SDHCI_INT_DMA_END)
> -                     sdhci_writel(host, sdhci_readl(host, SDHCI_DMA_ADDRESS),
> -                             SDHCI_DMA_ADDRESS);
> +             if (intmask & SDHCI_INT_DMA_END) {
> +                     u32 dmastart, dmanow;
> +                     dmastart = sg_dma_address(host->data->sg);

This will only work for the first 512K, right?

> +                     dmanow = sdhci_readl(host, SDHCI_DMA_ADDRESS);
> +                     if (dmanow == dmastart) {
> +                             /*
> +                              * HW failed to increase the address.
> +                              * Update to the next 512KB block boundary.
> +                              */
> +                             dmanow = (dmanow & ~0x7ffff) + 0x80000;

Hmm, hardcoding these values is probably not a good idea. They should be
dependent on what is written to MAKE_BLKSIZE. Maybe a common define?

> +                             if (dmanow > dmastart + host->data->blksz *
> +                                                     host->data->blocks) {
> +                                     WARN_ON(1);
> +                                     dmanow = dmastart;
> +                             }

Did this happen?

> +                             DBG("%s: next DMA address forced "
> +                                 "from 0x%08x to 0x%08x\n",
> +                                 mmc_hostname(host->mmc), dmastart, dmanow);
> +                     }
> +                     sdhci_writel(host, dmanow, SDHCI_DMA_ADDRESS);
> +             }
>  
>               if (intmask & SDHCI_INT_DATA_END) {
>                       if (host->cmd) {
> -- 
> 1.7.4.1

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Attachment: signature.asc
Description: Digital signature

Reply via email to