On 11/26/24 12:48 PM, Jonathan Cameron wrote:
> On Thu, 21 Nov 2024 10:18:31 +0000
> Guillaume Stols <gstols@xxxxxxxxxxxx> wrote:
>
>> Adds the logic for effectively enabling the software mode for the
>> iio-backend, i.e enabling the software mode channel configuration and
>> implementing the register writing functions.
>>
>> Signed-off-by: Guillaume Stols <gstols@xxxxxxxxxxxx>
>
> A few comments inline, but basically looks fine to me.
>
> Thanks,
>
> Jonathan
>
>> diff --git a/drivers/iio/adc/ad7606_par.c b/drivers/iio/adc/ad7606_par.c
>> index a25182a3daa7..0c1177f436f3 100644
>> --- a/drivers/iio/adc/ad7606_par.c
>> +++ b/drivers/iio/adc/ad7606_par.c
>
>> static int ad7606_bi_update_scan_mode(struct iio_dev *indio_dev, const unsigned long *scan_mask)
>> {
>> struct ad7606_state *st = iio_priv(indio_dev);
>> @@ -70,7 +83,7 @@ static int ad7606_bi_setup_iio_backend(struct device *dev, struct iio_dev *indio
>> if (ret)
>> return ret;
>>
>> - ret = devm_iio_backend_enable(dev, st->back);
>> + ret = devm_iio_backend_enable(st->dev, st->back);
>
> Is that a different dev? That's not obvious...
>
>> if (ret)
>> return ret;
>>
>> @@ -86,9 +99,52 @@ static int ad7606_bi_setup_iio_backend(struct device *dev, struct iio_dev *indio
>> return 0;
>> }
>>
>> +static int ad7606_bi_reg_read(struct iio_dev *indio_dev, unsigned int addr)
>> +{
>> + struct ad7606_state *st = iio_priv(indio_dev);
>> + int val, ret;
>> + struct ad7606_platform_data *pdata = st->dev->platform_data;
>> +
>> + iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
>> + ret = pdata->bus_reg_read(st->back,
>> + addr,
>> + &val);
>
> As below.
>
>> + }
>> + if (ret < 0)
>> + return ret;
>> +
>> + return val;
>> +}
>> +
>> +static int ad7606_bi_reg_write(struct iio_dev *indio_dev,
>> + unsigned int addr,
>> + unsigned int val)
>> +{
>> + struct ad7606_state *st = iio_priv(indio_dev);
>> + struct ad7606_platform_data *pdata = st->dev->platform_data;
>> + int ret;
>> +
>> + iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
>
> Given David's if_not_cond_guard() should land shortly I'd prefer
> to use that going forwards for cases like this.
Well, Torvalds wasn't happy with the patch and suggested we should
give up on trying to do conditional guards altogether in cleanup.h.
[1]: https://lore.kernel.org/all/CAHk-=whn07tnDosPfn+UcAtWHBcLg=KqA16SHVv0GV4t8P1fHw@xxxxxxxxxxxxxx/
So I'm tempted to just revert the if_not_cond_guard() patch rather
than trying to fix it.
>
>> + ret = pdata->bus_reg_write(st->back,
>> + addr,
>> + val);
> Put parameters all on one line.
> + return here (which needs the new if_not_cond_guard() to avoid
> confusing the compiler).
>
>> + }
>> + return ret;
>> +}
