Re: [PATCH v5 09/16] pwm: tegra: Add device tree support

[Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>]

On 04/03/2012 11:00 PM, Thierry Reding wrote:
> * Grant Likely wrote:
>> On Tue, 3 Apr 2012 19:55:11 +0200, Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org> wrote:
>>> * Stephen Warren wrote:
>>>> On 04/02/2012 02:37 AM, Thierry Reding wrote:
>>>>> * Stephen Warren wrote:
>>>>>> On 03/28/2012 08:33 AM, Thierry Reding wrote:
>>>>>>> Add auxdata to instantiate the PWFM controller from a device tree,
>>>>>>> include the corresponding nodes in the dtsi files for Tegra 20 and
>>>>>>> Tegra 30 and add binding documentation.
>>>>>>>
>>>>>>> Signed-off-by: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>
>>>>>>> Acked-by: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
>>>>>>
>>>>>>> diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
>>>>>> ...
>>>>>>> +#ifdef CONFIG_OF
>>>>>>> +static struct of_device_id tegra_pwm_of_match[] = {
>>>>>>> +	{ .compatible = "nvidia,tegra20-pwm" },
>>>>>>> +	{ .compatible = "nvidia,tegra30-pwm" },
>>>>>>
>>>>>> Could you swap those two lines, so that tegra30-pwm matches first. It
>>>>>> makes no difference at present, but might in the future if the driver
>>>>>> actually has to differentiate the two SoCs.
>>>>>
>>>>> I thought the matching order was determined by the compatible property in the
>>>>> device tree, not the OF match table of the driver.
>>>>
>>>> At least logically, yes. However, of_match_device() appears to iterate
>>>> over each match table entry, checking whether it matches any string in
>>>> the compatible flag. Perhaps this could be considered a bug?
>>>
>>> It certainly is counter-intuitive. Maybe Grant or Rob can comment?
>>
>> Yes, it is a bug.  The order of of_device_id should be entirely
>> irrelevant, and the order in the DT compatible property should
>> determine which match entry is returned.
> 
> I've had a look at the code and it looks like a fix will not be entirely
> trivial. I think moving out the compatible check out of the while loop in
> of_match_node() and separately iterate over all strings in the compatible
> property would be the easiest. That will also prioritize the compatible
> match over matches by name and type but I think that's exactly what we
> want. From a quick look it certainly seems like the large majority of
> drivers match by compatible anyway.
> 
> Do you want me to prepare a patch or can you take care of it?
> 
> Stephen: Can I assume that you're fine with this Tegra PWM patch if such
> a change to the matching function is committed?

Yes.