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.
Regards,
Marco
