[PATCH] iio: adc: ad7606: fix serial register access

Angelo Dureghello posted 1 patch 9 months, 3 weeks ago
There is a newer version of this series
drivers/iio/adc/ad7606_spi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] iio: adc: ad7606: fix serial register access
Posted by Angelo Dureghello 9 months, 3 weeks ago
From: Angelo Dureghello <adureghello@baylibre.com>

Fix register read/write routine as per datasheet.

When reading multiple consecutive registers, only the first one is read
properly. This is due to missing chip select between first and second
16bit transfer.

Add chip select between the two 16bit transfers as per datasheet
AD7606C-16, rev 0, figure 110.

Fixes: f2a22e1e172f ("iio: adc: ad7606: Add support for software mode for ad7616")
Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
---
 drivers/iio/adc/ad7606_spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ad7606_spi.c b/drivers/iio/adc/ad7606_spi.c
index 997be483ebb93293481b922e13ece4edb47e940a..bd05357a542cf7234d5bb6b718829d6b720262cd 100644
--- a/drivers/iio/adc/ad7606_spi.c
+++ b/drivers/iio/adc/ad7606_spi.c
@@ -103,7 +103,7 @@ static int ad7606_spi_reg_read(struct ad7606_state *st, unsigned int addr)
 		{
 			.tx_buf = &st->d16[0],
 			.len = 2,
-			.cs_change = 0,
+			.cs_change = 1,
 		}, {
 			.rx_buf = &st->d16[1],
 			.len = 2,

---
base-commit: 8dc6b228d746b1a900bed28568defb2266fa4c43
change-id: 20250417-wip-bl-ad7606-fix-reg-access-729c21478d1f

Best regards,
-- 
Angelo Dureghello <adureghello@baylibre.com>
Re: [PATCH] iio: adc: ad7606: fix serial register access
Posted by Jonathan Cameron 9 months, 3 weeks ago
On Thu, 17 Apr 2025 23:42:51 +0200
Angelo Dureghello <adureghello@baylibre.com> wrote:

> From: Angelo Dureghello <adureghello@baylibre.com>
> 
> Fix register read/write routine as per datasheet.
> 
> When reading multiple consecutive registers, only the first one is read
> properly. This is due to missing chip select between first and second
> 16bit transfer.
In what sense of missing? Given code you mean missing being unselected
briefly between transfers I think.

chip select itself is always set in current code and hence the 'missing'
description had me confused!

Anyhow, looks fine to me but I'd like this on list a little longer before
I pick it up.

Thanks,

Jonathan

> Add chip select between the two 16bit transfers as per datasheet
> AD7606C-16, rev 0, figure 110.
> 
> Fixes: f2a22e1e172f ("iio: adc: ad7606: Add support for software mode for ad7616")
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
> ---
>  drivers/iio/adc/ad7606_spi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/ad7606_spi.c b/drivers/iio/adc/ad7606_spi.c
> index 997be483ebb93293481b922e13ece4edb47e940a..bd05357a542cf7234d5bb6b718829d6b720262cd 100644
> --- a/drivers/iio/adc/ad7606_spi.c
> +++ b/drivers/iio/adc/ad7606_spi.c
> @@ -103,7 +103,7 @@ static int ad7606_spi_reg_read(struct ad7606_state *st, unsigned int addr)
>  		{
>  			.tx_buf = &st->d16[0],
>  			.len = 2,
> -			.cs_change = 0,
> +			.cs_change = 1,
>  		}, {
>  			.rx_buf = &st->d16[1],
>  			.len = 2,
> 
> ---
> base-commit: 8dc6b228d746b1a900bed28568defb2266fa4c43
> change-id: 20250417-wip-bl-ad7606-fix-reg-access-729c21478d1f
> 
> Best regards,
Re: [PATCH] iio: adc: ad7606: fix serial register access
Posted by David Lechner 9 months, 3 weeks ago
On 4/18/25 10:56 AM, Jonathan Cameron wrote:
> On Thu, 17 Apr 2025 23:42:51 +0200
> Angelo Dureghello <adureghello@baylibre.com> wrote:
> 
>> From: Angelo Dureghello <adureghello@baylibre.com>
>>
>> Fix register read/write routine as per datasheet.
>>
>> When reading multiple consecutive registers, only the first one is read
>> properly. This is due to missing chip select between first and second
>> 16bit transfer.
> In what sense of missing? Given code you mean missing being unselected
> briefly between transfers I think.
> 
> chip select itself is always set in current code and hence the 'missing'
> description had me confused!

Agree it would be better to describe this as missing the momentary chip select
deassert between the transfers.

Reviewed-by: David Lechner <dlechner@baylibre.com>