Re: [PATCH 2/2] iio: adc: ad717x: add AD717X driver

[Jonathan Cameron]

On Thu, 10 Aug 2023 18:36:36 +0300
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:

> On Thu, Aug 10, 2023 at 01:57:02PM +0200, Nuno Sá wrote:
> > On Thu, 2023-08-10 at 12:33 +0300, Dumitru Ceclan wrote:  
> 
> ...
> 
> > Is ad717x_gpio_cleanup() being used anywhere? Moreover I would maybe just get rid of
> > the #ifdef wrapper and just select GPIOLIB. How often will it be disabled anyways?  
> 
> The agreement is that users are depend on and not selecting GPIOLIB.
> Any news in these agreement terms?
> 
> ...
> 
> > > +       id &= AD717X_ID_MASK;
> > > +       if (id != st->info->id)
> > > +               dev_warn(&st->sd.spi->dev, "Unexpected device id: %x, expected:
> > > %x\n",
> > > +                                           id, st->info->id);
> > > +  
> > 
> > Shouldn't we error out?  
> 
> It seems a new way of thinking about unsupported CHIP ID. Dunno if hw vendors
> won't ever do a dirty trick that new ID must be programmed differently and
> otherwise burn hardware to a smoke...
> 
> I'm with you here, unknown chips mustn't be supported.

Some discussions with DT maintainers on this led me to change my mind...
They are very strongly of the view that if a DT firmware claims a device is compatible
then a device ID register that has an unknown value should be ignored. It gets
more tricky if we have a known wrong value (in which case we assume that it
is the value the hardware is claiming whatever DT says).

Argument is that lots of new versions of devices that are fully compatible with
exception of ID registers are released and if you have an old kernel but a new
device tree (typically running a distro that hasn't caught up yet) then the
fallback compatible is there to make things work.

I changed the way we handle this case in IIO to follow this policy - with
the slight tweak that we print a message whenever such a mismatch occurs.

Jonathan