On Thu, 30 Mar 2023 17:30:32 +0200
Fabrice Gasnier <fabrice.gasnier@xxxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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");
