Message-ID: <6d0f2f31-7d9f-5b6b-b244-d976c4d3cd0d@kernel.org>
Date:	Sun, 26 Jun 2016 15:53:14 +0100
From:	Jonathan Cameron <jic23@...nel.org>
To:	Florian Vaussard <florian.vaussard@...g-vd.ch>,
	Peter Rosin <peda@...ntia.se>
Cc:	devicetree@...r.kernel.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
Subject: Re: [PATCH 3/3] iio: potentiometer: mcp4531: Add device tree binding

On 23/06/16 09:50, Florian Vaussard wrote:
> 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.
The original 'quirk' of matching in i2c and spi based on the part number
alone has been the source of much grief.  Unfortunately it is there
and there are numerous device trees out there using it.

The preferred option is a nice conventional table as you added in patch 2.

We just haven't gotten round to updating all the drivers yet and as it
'works' right now there is little motivation to do it unless someone is
working on the driver as you are.
> 
> 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.
Exactly and it's already happened I believe... With two totally different parts
having the same part number rather than the similar ones case.
> 
> 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
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Powered by blists - more mailing lists