Re: [PATCH] hwmon: Driver for as1531, Austria-Microsystem Analog to Digital Converter.

[Jonathan Cameron <jic23@cam.ac.uk>]

On 05/18/11 14:09, Jonathan Cameron wrote:
> On 05/16/11 14:39, fabien.marteau@armadeus.com wrote:
>> From: Fabien Marteau <fabien.marteau@armadeus.com>
>>
> Had a few minutes so done a quick port to IIO with minimal code changes.
> Now looking at cleaning things up so actually reading the code rather
> than cut and paste.  The message is somewhat 'novel'. 
> 
> ...
> 
> So we have a spi device that only talks in 1 bit words?
> 1) I'm amazed any spi controllers support that.
> 2) Doesn't look necessary from the data sheet google furnished me with.
> 
> Looks like it will quite happily work with 8 bit transfers. Suggested replacement code below.
>> +static int as1531_message(struct spi_device *spi, int cmd, int *ret_value)
>> +{
>> +	struct spi_message	message;
>> +	struct spi_transfer	x[1];
>> +	int status, i;
>> +	u8	cmd_send;
>> +	unsigned char buf[64];
>> +	unsigned char buf_read[64];
>> +
>> +	cmd_send = cmd;
>> +
>> +	spi_message_init(&message);
>> +	memset(x, 0, sizeof x);
>> +	memset(buf, 0, sizeof(buf));
>> +	memset(buf_read, 0, sizeof(buf_read));
>> +
>> +	for (i = 0; i < 8; i++) {
>> +		buf[i] = ((cmd_send & 0x80)>>7);
>> +		cmd_send = cmd_send << 1;
>> +	}
>> +
>> +	x[0].tx_buf = buf;
>> +	x[0].len = 24;
>> +	x[0].rx_buf = buf_read;
>> +	x[0].speed_hz = AS1531_SPI_SPEED;
>> +	x[0].bits_per_word = 1;
>> +	spi_message_add_tail(&x[0], &message);
>> +
>> +	status = spi_sync(spi, &message);
>> +	if (status < 0)
>> +		return status;
>> +
>> +	*ret_value = buf_read[11] & 0x01;
>> +	for (i = 12; i < 23 ; i++) {
>> +		*ret_value = *ret_value << 1;
>> +		*ret_value = *ret_value | (buf_read[i]&0x01);
>> +	}
>> +
>> +	return 0;
>> +}

Or skip the below and use spi_w8r16. This isn't a fast capture route anyway.
> static int as1531_message(struct spi_device *spi, int cmd, u16 *ret_value)
> {
> 	int status;
> 	u8 cmd_send = cmd;
> 	struct spi_message message;
> 	struct spi_transfer x[2] = {
> 		{
> 			.len = 1,
> 			/* this should be default anyway - so could drop */
> 			.bits_per_word = 8,
> 			/* This should be set in board config really */
> 			.speed_hz = AS1531_SPI_SPEED,
> 			.rx_buf = &cmd_send,
> 		}, {
> 			.len = 2,
> 			.bits_per_word = 8,
> 			.speed_hz = AS1531_SPI_SPEED,
> 			.tx_buf = ret_value,
> 		}
> 	};
> 
> 	spi_message_init(&message);
> 	spi_message_add_tail(&x[0], &message);
> 	spi_message_add_tail(&x[1], &message);
> 
> 	status = spi_sync(spi, &message);
> 	if (status < 0)
> 		return status;
> 
> 	*ret_value = (be16_to_cpup(ret_value) >> 2) & 0xFFF;
> 
> 	return 0;
> }
> 
> Obviously also requires changing the type of the element passed
> as ret_value.  Could set the bits_per_word of the second transfer
> to 16 and avoid the endianness conversion, but I'm not sure how
> many spi masters support that.
> 
> Jonathan