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

[Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2550 bytes --]

* Grant Likely wrote:
> On Tue, 3 Apr 2012 19:55:11 +0200, Thierry Reding <thierry.reding@avionic-design.de> 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?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]