Re: [PATCH] staging: iio: ad9834: fix probe error handling and comment typo

["Maxwell Doose" <m32285159@gmail.com>]

On Sun May 3, 2026 at 3:53 AM CDT, Joshua Crofts wrote:
> On Sat, 2 May 2026 at 21:54, Maxwell Doose <m32285159@gmail.com> wrote:
>>
>> Hi Angus,
>>
>> On Fri May 1, 2026 at 9:18 PM CDT, Angus Gardner <angusg778@gmail.com> wrote:
>> > Use dev_err_probe() for the master clock error path instead of open-coding
>> > dev_err() + PTR_ERR(), consistent with the avdd regulator error path above
>> > it and with the equivalent driver ad9832.c.
>
> I found another dev_err() call in the same function, maybe change this one also?
> https://elixir.bootlin.com/linux/v7.0.1/source/drivers/staging/iio/frequency/ad9834.c#L452
>

Shoot, I seem to have missed that in my review. Nice catch.

>
>> > Simplify the -ENOMEM return after devm_iio_device_alloc() to a direct
>> > return rather than bouncing through a local variable.
>> >
>> > Fix a copy-paste typo in two comments that referred to 'AD9843' instead
>> > of the correct chip name 'AD9834'.
>> >
>> > Signed-off-by: Angus Gardner <angusg778@gmail.com>
>> > ---
>> >  drivers/staging/iio/frequency/ad9834.c | 17 +++++++----------
>> >  1 file changed, 7 insertions(+), 10 deletions(-)
>> >
>>
>> Patch itself looks good but maybe rephrase subject to say "Use probe
>> error handling and fix comment typo" instead? "Fix" seems a bit generic
>> for my tastes, and we're not really fixing anything anyways.
>>
>> I'm not the final authority on naming so its at the discretion of
>> either Andy or Jonathan, and maybe Greg. So either way,
>> Reviewed-by: Maxwell Doose <m32285159@gmail.com>
>
> You're mixing different logical changes into one patch, therefore it's
> harder to review and harder to keep track of the changes after merging
> (e.g. running git blame). Instead, you should have a series (named
> something like "ad9834: driver cleanup") and three patches under it:
> 1. simplify -ENOMEM errno return
> 2. use dev_err_probe()
> 3. fix typos in comments
>

Of course this is undeniable but I think the other changes are so
miniscule that it wouldn't matter as much, but that's just my personal
opinion. Angus, you should probably split this into a patch series.

Either way, I still stand by my reviewed-by, at least based on technical
correctness, but make sure you at least add the missing dev_err_probe()
call that Joshua wanted Angus :)

best regards,
maxwell