Re: [PATCH 3/6] iio: adc: mcp320x: Speed up readout of single-channel ADCs

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 22 Aug 2017 15:33:00 +0200
Lukas Wunner <lukas@wunner.de> wrote:

> Single-channel converters such as mcp3001, mcp3201, mcp3301 and the
> upcoming mcp3550/1/3 lack a MOSI pin, so there's no need to call
> mcp320x_channel_to_tx_data() for them.
> 
> Moreover, instead of calling spi_read() for these converters, which
> generates an spi_message and spi_transfer on the stack on every readout,
> it's more efficient to use the spi_message and spi_transfer[] included
> in struct mcp320x (as we do for multi-channel ADCs), but initialize the
> spi_message only with the receive transfer.
> 
> Signed-off-by: Lukas Wunner <lukas@wunner.de>

Looks good, but I'll hold this for now as the fixes will not show up
in my upstream tree for a least 3-4 weeks.

Please give me a bump if it looks like I have forgotten it, or include
it in updated versions of the rest of the series as appropriate.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/mcp320x.c | 31 +++++++++++++------------------
>  1 file changed, 13 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
> index 071dd23a33d9..790d3e857c80 100644
> --- a/drivers/iio/adc/mcp320x.c
> +++ b/drivers/iio/adc/mcp320x.c
> @@ -78,10 +78,6 @@ static int mcp320x_channel_to_tx_data(int device_index,
>  	int start_bit = 1;
>  
>  	switch (device_index) {
> -	case mcp3001:
> -	case mcp3201:
> -	case mcp3301:
> -		return 0;
>  	case mcp3002:
>  	case mcp3202:
>  		return ((start_bit << 4) | (!differential << 3) |
> @@ -104,18 +100,13 @@ static int mcp320x_adc_conversion(struct mcp320x *adc, u8 channel,
>  
>  	adc->rx_buf[0] = 0;
>  	adc->rx_buf[1] = 0;
> -	adc->tx_buf = mcp320x_channel_to_tx_data(device_index,
> -						channel, differential);
> +	if (adc->chip_info->num_channels > 1)
> +		adc->tx_buf = mcp320x_channel_to_tx_data(device_index, channel,
> +							 differential);
>  
> -	if (device_index != mcp3001 && device_index != mcp3201 && device_index != mcp3301) {
> -		ret = spi_sync(adc->spi, &adc->msg);
> -		if (ret < 0)
> -			return ret;
> -	} else {
> -		ret = spi_read(adc->spi, &adc->rx_buf, sizeof(adc->rx_buf));
> -		if (ret < 0)
> -			return ret;
> -	}
> +	ret = spi_sync(adc->spi, &adc->msg);
> +	if (ret < 0)
> +		return ret;
>  
>  	switch (device_index) {
>  	case mcp3001:
> @@ -330,9 +321,13 @@ static int mcp320x_probe(struct spi_device *spi)
>  	adc->transfer[0].len = sizeof(adc->tx_buf);
>  	adc->transfer[1].rx_buf = adc->rx_buf;
>  	adc->transfer[1].len = sizeof(adc->rx_buf);
> -
> -	spi_message_init_with_transfers(&adc->msg, adc->transfer,
> -					ARRAY_SIZE(adc->transfer));
> +	if (chip_info->num_channels == 1)
> +		/* single-channel converters are rx only (no MOSI pin) */
> +		spi_message_init_with_transfers(&adc->msg,
> +						&adc->transfer[1], 1);
> +	else
> +		spi_message_init_with_transfers(&adc->msg, adc->transfer,
> +						ARRAY_SIZE(adc->transfer));
>  
>  	adc->reg = devm_regulator_get(&spi->dev, "vref");
>  	if (IS_ERR(adc->reg))