Re: [Linux-stm32] [PATCH 1/3] iio: adc: stm32-adc: warn if dt uses legacy channel config

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 30 Mar 2023 17:30:32 +0200
Fabrice Gasnier <fabrice.gasnier@foss.st.com> wrote:

> On 3/27/23 10:34, Sean Nyekjaer wrote:
> > Since nearly all stm32 dt's are using the legacy adc channel config,
> > we should warn users about using it.
> > 
> > Signed-off-by: Sean Nyekjaer <sean@geanix.com>
> > ---
> >  drivers/iio/adc/stm32-adc.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> > index 1aadb2ad2cab..d8e755d0cc52 100644
> > --- a/drivers/iio/adc/stm32-adc.c
> > +++ b/drivers/iio/adc/stm32-adc.c
> > @@ -1993,6 +1993,8 @@ static int stm32_adc_get_legacy_chan_count(struct iio_dev *indio_dev, struct stm
> >  	const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
> >  	int num_channels = 0, ret;
> >  
> > +	dev_warn(&indio_dev->dev, "using legacy channel config\n");
> > +  
> 
> Hi Sean,
> 
> I'd recommend to avoid adding a dev_warn() on a per driver basis, for
> depreacted DT properties. Still I'm not sure if there's some policy in
> this area.
> 
> IMHO, deprecated properties should be checked by using dt tools
> (dt_binding_check / dtbs_check or other mean?). But I have no idea if
> this is going to report warnings and when. Purpose would be to avoid
> introducing no dts files with these. As commented by Olivier on Patch 3,
> we've some downstream patches to adopt the generic bindings (not
> upstream 'yet').
> 
> Another downside is regarding backward compatibility. In case an old dtb
> is used to boot the kernel, as long as there's no functionality loss,
> I'd advise not to use any warning here. That's a valid use of an old dt.
> 
> In all case, thanks for pointing issues (e.g. Patch 2 & 3), regarding
> this patch, I'd nack adding this warning. Please drop this change if you
> re-submit or turn this into a dev_dbg().
> 

Agreed. Better to change to dev_dbg().

Other two patches look good to me, but will leave a bit more time for others
to comment before I pick them up.  As a small side note. They are fixes and
this first patch is not, so they should have been before it in the series
so I can route them to mainline faster than the non fix.

Jonathan

> Best Regards,
> Fabrice
> 
> >  	ret = device_property_count_u32(dev, "st,adc-channels");
> >  	if (ret > adc_info->max_channels) {
> >  		dev_err(&indio_dev->dev, "Bad st,adc-channels?\n");