Re: [1/2] i2c: ismt: 16-byte align the DMA buffer address

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

 



On Fri, Aug 18, 2017 at 05:01:27PM +0100, Radu Rendec wrote:
> Use only a portion of the data buffer for DMA transfers, which is always
> 16-byte aligned. This makes the DMA buffer address 16-byte aligned and
> compensates for spurious hardware parity errors that may appear when the
> DMA buffer address is not 16-byte aligned.
> 
> The data buffer is enlarged in order to accommodate any possible 16-byte
> alignment offset and changes the DMA code to only use a portion of the
> data buffer, which is 16-byte aligned.
> 
> The symptom of the hardware issue is the same as the one addressed in
> v3.12-rc2-5-gbf41691 and manifests by transfers failing with EIO, with
> bit 9 being set in the ERRSTS register.
> 
> Signed-off-by: Radu Rendec <radu.rendec@xxxxxxxxx>
Why not just use the alligned attribute here for buffer? 

https://gcc.gnu.org/onlinedocs/gcc-3.3/gcc/Type-Attributes.html

That would save you over allocation when possible and keep you from needing to
create a private aligned variable.

Neil

> ---
>  drivers/i2c/busses/i2c-ismt.c | 37 +++++++++++++++++++------------------
>  1 file changed, 19 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-ismt.c b/drivers/i2c/busses/i2c-ismt.c
> index e98e44e..ccce0ca 100644
> --- a/drivers/i2c/busses/i2c-ismt.c
> +++ b/drivers/i2c/busses/i2c-ismt.c
> @@ -172,7 +172,7 @@ struct ismt_priv {
>  	dma_addr_t io_rng_dma;			/* descriptor HW base addr */
>  	u8 head;				/* ring buffer head pointer */
>  	struct completion cmp;			/* interrupt completion */
> -	u8 dma_buffer[I2C_SMBUS_BLOCK_MAX + 1];	/* temp R/W data buffer */
> +	u8 buffer[I2C_SMBUS_BLOCK_MAX + 16];	/* temp R/W data buffer */
>  };
>  
>  /**
> @@ -320,7 +320,7 @@ static int ismt_process_desc(const struct ismt_desc *desc,
>  			     struct ismt_priv *priv, int size,
>  			     char read_write)
>  {
> -	u8 *dma_buffer = priv->dma_buffer;
> +	u8 *dma_buffer = (u8*)ALIGN((unsigned long)&priv->buffer[0], 16);
>  
>  	dev_dbg(&priv->pci_dev->dev, "Processing completed descriptor\n");
>  	__ismt_desc_dump(&priv->pci_dev->dev, desc);
> @@ -388,11 +388,12 @@ static int ismt_access(struct i2c_adapter *adap, u16 addr,
>  	struct ismt_desc *desc;
>  	struct ismt_priv *priv = i2c_get_adapdata(adap);
>  	struct device *dev = &priv->pci_dev->dev;
> +	u8 *dma_buffer = (u8*)ALIGN((unsigned long)&priv->buffer[0], 16);
>  
>  	desc = &priv->hw[priv->head];
>  
>  	/* Initialize the DMA buffer */
> -	memset(priv->dma_buffer, 0, sizeof(priv->dma_buffer));
> +	memset(priv->buffer, 0, sizeof(priv->buffer));
>  
>  	/* Initialize the descriptor */
>  	memset(desc, 0, sizeof(struct ismt_desc));
> @@ -441,8 +442,8 @@ static int ismt_access(struct i2c_adapter *adap, u16 addr,
>  			desc->wr_len_cmd = 2;
>  			dma_size = 2;
>  			dma_direction = DMA_TO_DEVICE;
> -			priv->dma_buffer[0] = command;
> -			priv->dma_buffer[1] = data->byte;
> +			dma_buffer[0] = command;
> +			dma_buffer[1] = data->byte;
>  		} else {
>  			/* Read Byte */
>  			dev_dbg(dev, "I2C_SMBUS_BYTE_DATA:  READ\n");
> @@ -461,9 +462,9 @@ static int ismt_access(struct i2c_adapter *adap, u16 addr,
>  			desc->wr_len_cmd = 3;
>  			dma_size = 3;
>  			dma_direction = DMA_TO_DEVICE;
> -			priv->dma_buffer[0] = command;
> -			priv->dma_buffer[1] = data->word & 0xff;
> -			priv->dma_buffer[2] = data->word >> 8;
> +			dma_buffer[0] = command;
> +			dma_buffer[1] = data->word & 0xff;
> +			dma_buffer[2] = data->word >> 8;
>  		} else {
>  			/* Read Word */
>  			dev_dbg(dev, "I2C_SMBUS_WORD_DATA:  READ\n");
> @@ -481,9 +482,9 @@ static int ismt_access(struct i2c_adapter *adap, u16 addr,
>  		desc->rd_len = 2;
>  		dma_size = 3;
>  		dma_direction = DMA_BIDIRECTIONAL;
> -		priv->dma_buffer[0] = command;
> -		priv->dma_buffer[1] = data->word & 0xff;
> -		priv->dma_buffer[2] = data->word >> 8;
> +		dma_buffer[0] = command;
> +		dma_buffer[1] = data->word & 0xff;
> +		dma_buffer[2] = data->word >> 8;
>  		break;
>  
>  	case I2C_SMBUS_BLOCK_DATA:
> @@ -494,8 +495,8 @@ static int ismt_access(struct i2c_adapter *adap, u16 addr,
>  			dma_direction = DMA_TO_DEVICE;
>  			desc->wr_len_cmd = dma_size;
>  			desc->control |= ISMT_DESC_BLK;
> -			priv->dma_buffer[0] = command;
> -			memcpy(&priv->dma_buffer[1], &data->block[1], dma_size - 1);
> +			dma_buffer[0] = command;
> +			memcpy(&dma_buffer[1], &data->block[1], dma_size - 1);
>  		} else {
>  			/* Block Read */
>  			dev_dbg(dev, "I2C_SMBUS_BLOCK_DATA:  READ\n");
> @@ -522,8 +523,8 @@ static int ismt_access(struct i2c_adapter *adap, u16 addr,
>  			dma_direction = DMA_TO_DEVICE;
>  			desc->wr_len_cmd = dma_size;
>  			desc->control |= ISMT_DESC_I2C;
> -			priv->dma_buffer[0] = command;
> -			memcpy(&priv->dma_buffer[1], &data->block[1], dma_size - 1);
> +			dma_buffer[0] = command;
> +			memcpy(&dma_buffer[1], &data->block[1], dma_size - 1);
>  		} else {
>  			/* i2c Block Read */
>  			dev_dbg(dev, "I2C_SMBUS_I2C_BLOCK_DATA:  READ\n");
> @@ -552,18 +553,18 @@ static int ismt_access(struct i2c_adapter *adap, u16 addr,
>  	if (dma_size != 0) {
>  		dev_dbg(dev, " dev=%p\n", dev);
>  		dev_dbg(dev, " data=%p\n", data);
> -		dev_dbg(dev, " dma_buffer=%p\n", priv->dma_buffer);
> +		dev_dbg(dev, " dma_buffer=%p\n", dma_buffer);
>  		dev_dbg(dev, " dma_size=%d\n", dma_size);
>  		dev_dbg(dev, " dma_direction=%d\n", dma_direction);
>  
>  		dma_addr = dma_map_single(dev,
> -				      priv->dma_buffer,
> +				      dma_buffer,
>  				      dma_size,
>  				      dma_direction);
>  
>  		if (dma_mapping_error(dev, dma_addr)) {
>  			dev_err(dev, "Error in mapping dma buffer %p\n",
> -				priv->dma_buffer);
> +				dma_buffer);
>  			return -EIO;
>  		}
>  




[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux