Re: [PATCH 12/16] Blackfin SPI Driver: get dma working for SPI flashes

[David Brownell]

On Monday 17 November 2008, Bryan Wu wrote:
> From: Mike Frysinger <vapier.adi@xxxxxxxxx>
> 
> When using a BF533-STAMP here with a W25X10 SPI flash. It works fine when
> enable_dma is disabled, but doesn't work at all when turning DMA on.
> We get just 0xff bytes back when trying to read the device.
> 
> If change the code around so that it programs the SPI first and then
> enables DMA, it seems to work a lot better ...
> 
> Signed-off-by: Mike Frysinger <vapier.adi@xxxxxxxxx>
> Signed-off-by: Bryan Wu <cooloney@xxxxxxxxxx>

Acked-by: David Brownell <dbrownell@xxxxxxxxxxxxxxxxxxxxx>

... and I observe some MM patches from you related to bugs
identifying those chips; good to know I wasn't the only
one to trip over those regressions!


> ---
>  drivers/spi/spi_bfin5xx.c |   26 ++++++++++++++++----------
>  1 files changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/spi/spi_bfin5xx.c b/drivers/spi/spi_bfin5xx.c
> index fba77c2..5cd4873 100644
> --- a/drivers/spi/spi_bfin5xx.c
> +++ b/drivers/spi/spi_bfin5xx.c
> @@ -760,11 +760,10 @@ static void pump_transfers(unsigned long data)
>  	if (!full_duplex && drv_data->cur_chip->enable_dma
>  				&& drv_data->len > 6) {
>  
> -		unsigned long dma_start_addr;
> +		unsigned long dma_start_addr, flags;
>  
>  		disable_dma(drv_data->dma_channel);
>  		clear_dma_irqstat(drv_data->dma_channel);
> -		bfin_spi_disable(drv_data);
>  
>  		/* config dma channel */
>  		dev_dbg(&drv_data->pdev->dev, "doing dma transfer\n");
> @@ -795,8 +794,7 @@ static void pump_transfers(unsigned long data)
>  			enable_dma(drv_data->dma_channel);
>  
>  			/* start SPI transfer */
> -			write_CTRL(drv_data,
> -				(cr | BIT_CTL_TIMOD_DMA_TX | BIT_CTL_ENABLE));
> +			write_CTRL(drv_data, cr | BIT_CTL_TIMOD_DMA_TX);
>  
>  			/* just return here, there can only be one transfer
>  			 * in this mode
> @@ -838,14 +836,22 @@ static void pump_transfers(unsigned long data)
>  		} else
>  			BUG();
>  
> -		/* start dma */
> -		dma_enable_irq(drv_data->dma_channel);
> -		set_dma_config(drv_data->dma_channel, dma_config);
> +		/* oh man, here there be monsters ... and i dont mean the
> +		 * fluffy cute ones from pixar, i mean the kind that'll eat
> +		 * your data, kick your dog, and love it all.  do *not* try
> +		 * and change these lines unless you (1) heavily test DMA
> +		 * with SPI flashes on a loaded system (e.g. ping floods),
> +		 * (2) know just how broken the DMA engine interaction with
> +		 * the SPI peripheral is, and (3) have someone else to blame
> +		 * when you screw it all up anyways.
> +		 */
>  		set_dma_start_addr(drv_data->dma_channel, dma_start_addr);
> +		set_dma_config(drv_data->dma_channel, dma_config);
> +		local_irq_save(flags);
>  		enable_dma(drv_data->dma_channel);
> -
> -		/* start SPI transfer */
> -		write_CTRL(drv_data, (cr | BIT_CTL_ENABLE));
> +		write_CTRL(drv_data, cr);
> +		dma_enable_irq(drv_data->dma_channel);
> +		local_irq_restore(flags);
>  
>  	} else {
>  		/* IO mode write then read */
> -- 
> 1.5.6.3
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/