On Sat, 24 Feb 2024 17:30:55 +0000
Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> On Fri, 23 Feb 2024 15:37:28 +0200
> Dumitru Ceclan <mitrutzceclan@xxxxxxxxx> wrote:
>
> > The AD7173 family offer a complete integrated Sigma-Delta ADC solution
> > which can be used in high precision, low noise single channel applications
> > or higher speed multiplexed applications. The Sigma-Delta ADC is intended
> > primarily for measurement of signals close to DC but also delivers
> > outstanding performance with input bandwidths out to ~10kHz.
> >
> > Reviewed-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> > Signed-off-by: Dumitru Ceclan <mitrutzceclan@xxxxxxxxx>
>
> Ok, in the interests of perfect not being the enemy of good enough.
> I'll leave the supplies for now. There are lots of existing drivers
> where we don't list them as required (because my understanding of this
> changed in more recent times).
>
> It's been on my list of jobs for a really boring Friday afternoon
> to bring them all inline with the convention of if it needs power
> on the pin, it's required, so what's one more? :)
>
> As Nuno pointed out, patch 2 clashed with work already upstream to
> allow firmware to have the final say on interrupt types. I think
> I've resolved that correctly.
>
> I tidied up the docs ordering issue Andy noted.
>
> Also, ad_sigma_delta is namespaced. So added
> MODULE_IMPORT_NS(IIO_AD_SIGMA_DELTA).
>
> Make sure you test your patches with a modular build
> on a more recent tree - that change was early last in 2022!
>
> A few lines in the driver were too long.
> I don't mind them going over 80 for readability reasons, but
> not over 100.
>
> Anyhow, with those changes (and please check I didn't mess things up!)
> applied to the togreg branch of iio.git and pushed for now as testing
> for 0-day to get a look in.
Not good news. There are 2 issues.
>> drivers/iio/adc/ad7173.c:854:3: warning: variable 'chan_arr' is uninitialized when used here [-Wuninitialized]
854 | chan_arr[chan_index] = ad7173_temp_iio_channel_template;
| ^~~~~~~~
drivers/iio/adc/ad7173.c:848:32: note: initialize the variable 'chan_arr' to silence this warning
848 | struct iio_chan_spec *chan_arr, *chan;
| ^
| = NULL
>> drivers/iio/adc/ad7173.c:855:19: warning: variable 'chans_st_arr' is uninitialized when used here [-Wuninitialized]
855 | chan_st_priv = &chans_st_arr[chan_index];
| ^~~~~~~~~~~~
drivers/iio/adc/ad7173.c:845:37: note: initialize the variable 'chans_st_arr' to silence this warning
845 | struct ad7173_channel *chans_st_arr, *chan_st_priv;
| ^
| = NULL
+ if you build with !CONFIG_GPIOLIB
ad7173_gpio_init() isn't defined. That needs a stub.
I'll back this driver out for now as fixing the first issue is a little fiddly because
indio_dev->channels is const so the code should allocate and fill the array via a local pointer
before assigning it to indio_dev.
Please send a new version with these resolved + make sure you run some build tests.
Thanks,
Jonathan
>
> Thanks,
>
> Jonathan
>
