On Mon, 2025-01-20 at 15:07 +0100, Uwe Kleine-König wrote:
> The key objective in ad7124_disable_one() is clearing the
> AD7124_CHANNEL_EN_MSK bit in the channel register. However there is no
> advantage to keep the other bits in that register because when the
> channel is used next time, all fields are rewritten anyhow. So instead
> of using ad7124_spi_write_mask() (which is a register read plus a
> register write) use a simple register write clearing the complete
> register.
>
> Also do the same in the .disable_all() callback by using the
> .disable_one() callback there.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxx>
> ---
Reviewed-by: Nuno Sa <nuno.sa@xxxxxxxxxx>
> Hello,
>
> in (implicit) v1 I only adapted ad7124_disable_one() which resulted in
> the very legitimate question why this wasn't done for .disable_all(). I
> haven't checked because I wrongly assumed that .disable_all() used
> .disable_one(). This v2 now makes the latter true and so .disable_all()
> now also benefits from the micro optimisation.
>
> Best regards
> Uwe
>
> drivers/iio/adc/ad7124.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
> index 6ae27cdd3250..2fdeb3247952 100644
> --- a/drivers/iio/adc/ad7124.c
> +++ b/drivers/iio/adc/ad7124.c
> @@ -540,6 +540,14 @@ static int ad7124_append_status(struct ad_sigma_delta
> *sd, bool append)
> return 0;
> }
>
> +static int ad7124_disable_one(struct ad_sigma_delta *sd, unsigned int chan)
> +{
> + struct ad7124_state *st = container_of(sd, struct ad7124_state, sd);
> +
> + /* The relevant thing here is that AD7124_CHANNEL_EN_MSK is cleared.
> */
> + return ad_sd_write_reg(&st->sd, AD7124_CHANNEL(chan), 2, 0);
> +}
> +
> static int ad7124_disable_all(struct ad_sigma_delta *sd)
> {
> struct ad7124_state *st = container_of(sd, struct ad7124_state, sd);
> @@ -547,7 +555,7 @@ static int ad7124_disable_all(struct ad_sigma_delta *sd)
> int i;
>
> for (i = 0; i < st->num_channels; i++) {
> - ret = ad7124_spi_write_mask(st, AD7124_CHANNEL(i),
> AD7124_CHANNEL_EN_MSK, 0, 2);
> + ret = ad7124_disable_one(sd, i);
> if (ret < 0)
> return ret;
> }
> @@ -555,13 +563,6 @@ static int ad7124_disable_all(struct ad_sigma_delta *sd)
> return 0;
> }
>
> -static int ad7124_disable_one(struct ad_sigma_delta *sd, unsigned int chan)
> -{
> - struct ad7124_state *st = container_of(sd, struct ad7124_state, sd);
> -
> - return ad7124_spi_write_mask(st, AD7124_CHANNEL(chan),
> AD7124_CHANNEL_EN_MSK, 0, 2);
> -}
> -
> static const struct ad_sigma_delta_info ad7124_sigma_delta_info = {
> .set_channel = ad7124_set_channel,
> .append_status = ad7124_append_status,
>
> base-commit: b323d8e7bc03d27dec646bfdccb7d1a92411f189
