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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <576BA2DD.2060501@heig-vd.ch>
Date:	Thu, 23 Jun 2016 10:50:37 +0200
From:	Florian Vaussard <florian.vaussard@...g-vd.ch>
To:	Peter Rosin <peda@...ntia.se>
CC:	<devicetree@...r.kernel.org>, Jonathan Cameron <jic23@...nel.org>,
	Hartmut Knaack <knaack.h@....de>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Peter Meerwald-Stadler <pmeerw@...erw.net>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Slawomir Stepien <sst@...zta.fm>,
	Joachim Eastwood <manabian@...il.com>,
	Matt Ranostay <mranostay@...il.com>,
	Cristina Moraru <cristina.moraru09@...il.com>,
	<linux-iio@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	Vaussard Florian <florian.vaussard@...g-vd.ch>
Subject: Re: [PATCH 3/3] iio: potentiometer: mcp4531: Add device tree binding

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