Re: [PATCH 3/3] iio: potentiometer: mcp4531: Add device tree binding

[Florian Vaussard]

Hello Peter,

On 06/22/2016 09:06 AM, Peter Rosin wrote:
> On 2016-06-22 08:22, Florian Vaussard wrote:
>> Hello Peter,
>>
>> Le 21. 06. 16 à 09:51, Peter Rosin a écrit :
>>> That is, if you need this patch at all, see my reply to 2/3...
>>>
>>
>> This seems necessary in order to have the vendor ID in the compatible string.
> 
> Hmm, I don't think so. The way I read the response from Rob was that
> *my* device tree snippet should not assume that the i2c subsystem
> ignores the vendor. So, I think that even w/o this patch a DT entry
> like
> 
> 		mcp4651-104@28 {
> 			compatible = "microchip,mcp4651-104";
> 			reg = <0x28>;
> 		};
> 
> will work, precisely since i2c ignores the microchip, part (and thus
> allows you to omit/misspell it). I.e. I think that Rob is concerned
> with how the DT is documented/defined, and not so much about how it
> is then implemented in Linux.
> 

The way that I read Rob's reply is reverse: you should not rely on the matching
done by i2c subsystem and provide a proper of_device_id table instead.

I think that ignoring the vendor when matching the compatible, like what is
currently done by the i2c subsystem, is bad. What happens if you have two chips
from two companies with the same name (yes it happens)? You may need different
data to address the small specificities of each chip, or even worse a different
driver at all. This implies that we should explicitly match the vendor as well,
otherwise we would be unable to cope with such cases.

Moreover, if we look at the current i2c drivers, a whole bunch of them already
have a of_device_id table:

(on v4.6)

$ git grep -l i2c_device_id | wc -l
636
$ git grep -l i2c_device_id | xargs grep -l 'of_device_id' | wc -l
197

Currently 30% of all i2c drivers explicitly declare DT compatible strings. And I
am sure that this number will keep increasing as drivers are converted to DT.

Regards,
Florian