Re: [PATCH 13/15] staging:iio:ad7746: Do not store the transfer buffer on the stack

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

 



On 11/30/13 11:13, Jonathan Cameron wrote:
> On 11/25/13 12:42, Lars-Peter Clausen wrote:
>> Some I2C controllers might not be able to handle transfer buffers that are
>> stored on stack.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
> Applied to the togreg branch of iio.git
> 
> Thanks,
Hi Lars,

I let this one through without quite enough scrutiny.  It's a valid cleanup, but the statement
that some i2c controllers migh tnot be able to handle transfer buffers that are stored on the
stack is missleading, seeing as the relevant i2c calls do an explicit memcpy - (curriously to
another buffer that is also on the stack...)  I guess this might change in future.  In the pull
request I've mentioned that it isn't a problem right now.

So I should have picked up on the commit message content!

oops.

I'm not rebasing for a missleading bit of comment though so hopefully no one upstream will care
too much!
>> ---
>>  drivers/staging/iio/cdc/ad7746.c | 14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
>> index 01e15e2..cbb1588 100644
>> --- a/drivers/staging/iio/cdc/ad7746.c
>> +++ b/drivers/staging/iio/cdc/ad7746.c
>> @@ -105,6 +105,11 @@ struct ad7746_chip_info {
>>  	u8	vt_setup;
>>  	u8	capdac[2][2];
>>  	s8	capdac_set;
>> +
>> +	union {
>> +		__be32 d32;
>> +		u8 d8[4];
>> +	} data ____cacheline_aligned;
>>  };
>>  
>>  enum ad7746_chan {
>> @@ -566,11 +571,6 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
>>  	int ret, delay;
>>  	u8 regval, reg;
>>  
>> -	union {
>> -		__be32 d32;
>> -		u8 d8[4];
>> -	} data;
>> -
>>  	mutex_lock(&indio_dev->mlock);
>>  
>>  	switch (mask) {
>> @@ -591,12 +591,12 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
>>  		/* Now read the actual register */
>>  
>>  		ret = i2c_smbus_read_i2c_block_data(chip->client,
>> -			chan->address >> 8, 3, &data.d8[1]);
>> +			chan->address >> 8, 3, &chip->data.d8[1]);
>>  
>>  		if (ret < 0)
>>  			goto out;
>>  
>> -		*val = (be32_to_cpu(data.d32) & 0xFFFFFF) - 0x800000;
>> +		*val = (be32_to_cpu(chip->data.d32) & 0xFFFFFF) - 0x800000;
>>  
>>  		switch (chan->type) {
>>  		case IIO_TEMP:
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux