Re: [PATCH v2 3/4] iio: temperature: tmp117: add TI TMP116 support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux