Re: [PATCH v4 10/13] iio: adc: ad4134: Support SPI 4-wire mode

[Nuno Sá]

On Tue, 06 Oct 2026 15:19:00 -0300, Marcelo Schmitt <marcelo.schmitt@xxxxxxxxxx> wrote:
> diff --git a/drivers/iio/adc/ad4134.c b/drivers/iio/adc/ad4134.c
> index b5207f16054e..debc2ff02705 100644
> --- a/drivers/iio/adc/ad4134.c
> +++ b/drivers/iio/adc/ad4134.c
> @@ -308,14 +423,18 @@ static int ad4134_register_read(struct ad4134_state *st, unsigned int reg,
>  		.len = st->crc_en ? AD4134_SPI_MAX_XFER_LEN : 2,
>  	};
>  	unsigned int inst;
> -	int ret;
> +	int access_ret, ret;
> +
> +	ret = ad4134_claim_reg_access(st);
> +	if (ret)
> +		return ret;

Do we really need to claim/release for every register read? Can't we
just select register mode at probe and leave it in that position by
default? Then when starting to buffer, we deselect + select sample mode
and when exiting do the reverse. Seems even more appropriate when the dt
mode property does not want to use DOUT at all which means that all this
select + deselect is really unneeded.

> @@ -594,10 +728,64 @@ static int ad4134_probe(struct spi_device *spi)
> [ ... skip 16 lines ... ]
>  		return dev_err_probe(dev, ret,
> -				     "failed to setup minimum I/O mode\n");
> +				     "getting adi,spi-mode property failed\n");
> +	} else {
> +		st->spi_mode = ret;
> +	}

Andy has a better idea but I think the way we want to handle the above
in IIO nowadays is like:

if (device_property_present()) {
	ret = device_property_match_property_string();
	if (ret < 0)
		return ret;
	
	...

} else {
	st->spi_mode = AD4134_SPI_MODE_NO_CS
}

> +
> +	if (st->spi_mode == AD4134_SPI_MODE_NO_CS) {
> +		/*
> +		 * The support hardware for AD4134 may have a multiplexer for
> +		 * selecting between AD4134 SDO and AD4134 DOUT0. If that mux
> +		 * is set but the user still wants to run AD4134 in minimum I/O
> +		 * mode, then DOUT0 is not used and the multiplexer OUTSIDE OF
> +		 * AD4134 SILICON must be set to select AD4134 SDO. See AD4134
> +		 * IIO documentation for details.
> +		 */
> +		st->mux_st[AD4134_SDO_INPUT] =
> +			devm_mux_state_get_optional_selected(dev, "reg_access");

Seems like an issue indeed. But with I propose above, stops being one I
think.

-- 
Nuno Sá <nuno.sa@xxxxxxxxxx>