Re: [PATCH resend] sdhci: work around broken dma boundary behaviour

[Wolfram Sang <w.sang@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 3068 bytes --]

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 #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]