On Fri, 23 Dec 2022 17:13:59 +0100
Marco Felsch <m.felsch@xxxxxxxxxxxxxx> wrote:
> On 22-12-23, Jonathan Cameron wrote:
>
> ...
>
> > > > > @@ -118,27 +127,28 @@ static int tmp117_identify(struct i2c_client *client)
> > > > > int dev_id;
> > > > >
> > > > > dev_id = i2c_smbus_read_word_swapped(client, TMP117_REG_DEVICE_ID);
> > > > > - if (dev_id < 0)
> > > >
> > > > Keep this handling of the smbus read returning an error.
> > > > Otherwise, you end up replacing the error code with -ENODEV rather than
> > > > returning what actually happened.
> > > >
> > > > if (dev_id < 0)
> > > > return dev_id;
> > >
> > > You're right, I will change this thanks.
> > >
> > > > switch (dev_id) {
> > > > ...
> > > >
> > > > > + switch (dev_id) {
> > > > > + case TMP116_DEVICE_ID:
> > > > > + case TMP117_DEVICE_ID:
> > > > > return dev_id;
> > > > > - if (dev_id != TMP117_DEVICE_ID) {
> > > > > - dev_err(&client->dev, "TMP117 not found\n");
> > > > > + default:
> > > > > + dev_err(&client->dev, "TMP116/117 not found\n");
> > > > > return -ENODEV;
> >
> > As per the other branch of this thread. This isn't an error.
> > If we want fallback compatibles to work in their role of allowing
> > for newer devices that are actually compatible, the most we should
> > do here is warn.
> >
> > Say a new tmp117b device is released. It's fully backwards compatible
> > with the exception of an ID - or supports only new features + backwards
> > compatibility then that would have a fallback to tmp117 and we need
> > it to work.
>
> This isn't part of this patchset and IMHO implementing something which
> may happen in the future is not the way we should go.
I held a similar view, but the response I got from the DT maintainers was
that a driver should not reject a DTS that says it is compatible based
on an unknown ID - because it prevents that case of an old kernel working
absolutely fine with a completely compatible newer part.
Jonathan
>
> Regards,
> Marco
