[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20240226145500.00007783@Huawei.com>
Date: Mon, 26 Feb 2024 14:55:00 +0000
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Jonathan Cameron <jic23@...nel.org>
CC: Dumitru Ceclan <mitrutzceclan@...il.com>, Krzysztof Kozlowski
<krzysztof.kozlowski+dt@...aro.org>, <linus.walleij@...aro.org>,
<brgl@...ev.pl>, <andy@...nel.org>, <linux-gpio@...r.kernel.org>, "Lars-Peter
Clausen" <lars@...afoo.de>, Rob Herring <robh+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Michael Walle <michael@...le.cc>, Andy Shevchenko
<andy.shevchenko@...il.com>, Arnd Bergmann <arnd@...db.de>, ChiaEn Wu
<chiaen_wu@...htek.com>, Niklas Schnelle <schnelle@...ux.ibm.com>, Leonard
Göhrs <l.goehrs@...gutronix.de>, Mike Looijmans
<mike.looijmans@...ic.nl>, Haibo Chen <haibo.chen@....com>, Hugo Villeneuve
<hvilleneuve@...onoff.com>, David Lechner <dlechner@...libre.com>, "Ceclan
Dumitru" <dumitru.ceclan@...log.com>, <linux-iio@...r.kernel.org>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>, Conor Dooley
<conor.dooley@...rochip.com>
Subject: Re: [PATCH v15 1/3] dt-bindings: adc: add AD7173
On Sat, 24 Feb 2024 17:30:55 +0000
Jonathan Cameron <jic23@...nel.org> wrote:
> On Fri, 23 Feb 2024 15:37:28 +0200
> Dumitru Ceclan <mitrutzceclan@...il.com> 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@...rochip.com>
> > Signed-off-by: Dumitru Ceclan <mitrutzceclan@...il.com>
>
> 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
>
Powered by blists - more mailing lists