2016-04-01 21:35:17

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH] of/platform: Allow secondary compatible match in of_dev_lookup

We currently try to match of_dev_auxdata based on compatible,
IO address, and device name. But in some cases we have multiple
instances of drivers that can use the same auxdata.

Let's add an additional secondary lookup for generic compatible
match for auxdata if no device specific match is found. This does
not change the existing matching, and still allows adding device
specific auxdata.

This simplifies things as specifying the IO address and device
name is prone errors as it requires maintaining an in kernel
database for each SoC.

To specify a generic match, all that is needed is to add a
OF_DEV_AUXDATA entry with no device instance specified:

OF_DEV_AUXDATA("pinctrl-single", 0, NULL, &pcs_pdata),

As the auxdata is already initialized only for the booted SoC,
there's not much of a chance of getting things wrong.

Let's also fix two checkpatch warnings while at it to add a
space before parenthesis in the for loop, and remove a comparison
to NULL by using !auxdata->compatible.

Signed-off-by: Tony Lindgren <[email protected]>
---
drivers/of/platform.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 8d103e4..16e8daf 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -297,19 +297,37 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
static const struct of_dev_auxdata *of_dev_lookup(const struct of_dev_auxdata *lookup,
struct device_node *np)
{
+ const struct of_dev_auxdata *auxdata;
struct resource res;
+ int compatible = 0;

if (!lookup)
return NULL;

- for(; lookup->compatible != NULL; lookup++) {
- if (!of_device_is_compatible(np, lookup->compatible))
+ auxdata = lookup;
+ for (; auxdata->compatible; auxdata++) {
+ if (!of_device_is_compatible(np, auxdata->compatible))
continue;
+ compatible++;
if (!of_address_to_resource(np, 0, &res))
- if (res.start != lookup->phys_addr)
+ if (res.start != auxdata->phys_addr)
continue;
- pr_debug("%s: devname=%s\n", np->full_name, lookup->name);
- return lookup;
+ pr_debug("%s: devname=%s\n", np->full_name, auxdata->name);
+ return auxdata;
+ }
+
+ if (!compatible)
+ return NULL;
+
+ /* Try compatible match if no phys_addr and name are specified */
+ auxdata = lookup;
+ for (; auxdata->compatible; auxdata++) {
+ if (!of_device_is_compatible(np, auxdata->compatible))
+ continue;
+ if (!auxdata->phys_addr && !auxdata->name) {
+ pr_debug("%s: compatible match\n", np->full_name);
+ return auxdata;
+ }
}

return NULL;
--
2.7.0


2016-04-01 21:40:59

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH] of/platform: Allow secondary compatible match in of_dev_lookup

* Tony Lindgren <[email protected]> [160401 14:37]:
> We currently try to match of_dev_auxdata based on compatible,
> IO address, and device name. But in some cases we have multiple
> instances of drivers that can use the same auxdata.
>
> Let's add an additional secondary lookup for generic compatible
> match for auxdata if no device specific match is found. This does
> not change the existing matching, and still allows adding device
> specific auxdata.
>
> This simplifies things as specifying the IO address and device
> name is prone errors as it requires maintaining an in kernel
> database for each SoC.

And here's what I can apply later on to get rid of some
ifdeffery.

I'm also planning to move some of the legacy omap hwmod
functionality into proper device drivers, so can generic
pdata for that too.

Cheers,

Tony

8< -----------------------
From: Tony Lindgren <[email protected]>
Date: Fri, 1 Apr 2016 13:57:49 -0700
Subject: [PATCH] ARM: OMAP2+: Simplify auxdata by using the generic match

We can now just use the compatible if there's no need to have
device instance specific auxdata.

Signed-off-by: Tony Lindgren <[email protected]>
---
arch/arm/mach-omap2/pdata-quirks.c | 17 ++---------------
1 file changed, 2 insertions(+), 15 deletions(-)

--- a/arch/arm/mach-omap2/pdata-quirks.c
+++ b/arch/arm/mach-omap2/pdata-quirks.c
@@ -492,9 +492,6 @@ static struct of_dev_auxdata omap_auxdata_lookup[] __initdata = {
OF_DEV_AUXDATA("tlv320aic3x", 0x18, "2-0018", &n810_aic33_data),
#endif
#ifdef CONFIG_ARCH_OMAP3
- OF_DEV_AUXDATA("ti,omap3-padconf", 0x48002030, "48002030.pinmux", &pcs_pdata),
- OF_DEV_AUXDATA("ti,omap3-padconf", 0x480025a0, "480025a0.pinmux", &pcs_pdata),
- OF_DEV_AUXDATA("ti,omap3-padconf", 0x48002a00, "48002a00.pinmux", &pcs_pdata),
OF_DEV_AUXDATA("ti,omap2-iommu", 0x5d000000, "5d000000.mmu",
&omap3_iommu_pdata),
/* Only on am3517 */
@@ -506,19 +503,7 @@ static struct of_dev_auxdata omap_auxdata_lookup[] __initdata = {
OF_DEV_AUXDATA("ti,am3352-wkup-m3", 0x44d00000, "44d00000.wkup_m3",
&wkup_m3_data),
#endif
-#ifdef CONFIG_ARCH_OMAP4
- OF_DEV_AUXDATA("ti,omap4-padconf", 0x4a100040, "4a100040.pinmux", &pcs_pdata),
- OF_DEV_AUXDATA("ti,omap4-padconf", 0x4a31e040, "4a31e040.pinmux", &pcs_pdata),
-#endif
-#ifdef CONFIG_SOC_OMAP5
- OF_DEV_AUXDATA("ti,omap5-padconf", 0x4a002840, "4a002840.pinmux", &pcs_pdata),
- OF_DEV_AUXDATA("ti,omap5-padconf", 0x4ae0c840, "4ae0c840.pinmux", &pcs_pdata),
-#endif
-#ifdef CONFIG_SOC_DRA7XX
- OF_DEV_AUXDATA("ti,dra7-padconf", 0x4a003400, "4a003400.pinmux", &pcs_pdata),
-#endif
#ifdef CONFIG_SOC_AM43XX
- OF_DEV_AUXDATA("ti,am437-padconf", 0x44e10800, "44e10800.pinmux", &pcs_pdata),
OF_DEV_AUXDATA("ti,am4372-wkup-m3", 0x44d00000, "44d00000.wkup_m3",
&wkup_m3_data),
#endif
@@ -531,6 +516,8 @@ static struct of_dev_auxdata omap_auxdata_lookup[] __initdata = {
OF_DEV_AUXDATA("ti,omap4-iommu", 0x55082000, "55082000.mmu",
&omap4_iommu_pdata),
#endif
+ /* Common auxdata */
+ OF_DEV_AUXDATA("pinctrl-single", 0, NULL, &pcs_pdata),
{ /* sentinel */ },
};

--
2.7.0

2016-04-07 04:52:49

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH] of/platform: Allow secondary compatible match in of_dev_lookup

On Fri, Apr 1, 2016 at 4:40 PM, Tony Lindgren <[email protected]> wrote:
> * Tony Lindgren <[email protected]> [160401 14:37]:
>> We currently try to match of_dev_auxdata based on compatible,
>> IO address, and device name. But in some cases we have multiple
>> instances of drivers that can use the same auxdata.
>>
>> Let's add an additional secondary lookup for generic compatible
>> match for auxdata if no device specific match is found. This does
>> not change the existing matching, and still allows adding device
>> specific auxdata.
>>
>> This simplifies things as specifying the IO address and device
>> name is prone errors as it requires maintaining an in kernel
>> database for each SoC.
>
> And here's what I can apply later on to get rid of some
> ifdeffery.
>
> I'm also planning to move some of the legacy omap hwmod
> functionality into proper device drivers, so can generic
> pdata for that too.

Why can't the platform data be moved into the driver given that it
appears to be only SoC family specific? Auxdata was somewhat intended
to be temporary. It appears there is already some per compatible match
data for these OMAP parts in the driver.

Rob

2016-04-07 10:21:11

by Grygorii Strashko

[permalink] [raw]
Subject: Re: [PATCH] of/platform: Allow secondary compatible match in of_dev_lookup

On 04/07/2016 07:52 AM, Rob Herring wrote:
> On Fri, Apr 1, 2016 at 4:40 PM, Tony Lindgren <[email protected]> wrote:
>> * Tony Lindgren <[email protected]> [160401 14:37]:
>>> We currently try to match of_dev_auxdata based on compatible,
>>> IO address, and device name. But in some cases we have multiple
>>> instances of drivers that can use the same auxdata.
>>>
>>> Let's add an additional secondary lookup for generic compatible
>>> match for auxdata if no device specific match is found. This does
>>> not change the existing matching, and still allows adding device
>>> specific auxdata.
>>>
>>> This simplifies things as specifying the IO address and device
>>> name is prone errors as it requires maintaining an in kernel
>>> database for each SoC.
>>
>> And here's what I can apply later on to get rid of some
>> ifdeffery.
>>
>> I'm also planning to move some of the legacy omap hwmod
>> functionality into proper device drivers, so can generic
>> pdata for that too.
>
> Why can't the platform data be moved into the driver given that it
> appears to be only SoC family specific? Auxdata was somewhat intended
> to be temporary. It appears there is already some per compatible match
> data for these OMAP parts in the driver.
>

Most probably this is required to pass some data from parent device to
children when parent dev instantiate children from DT, at least I've experimented with
this in mostly similar way (I've not added second pass and did break in the first
if !phys_addr, but Tony's patch is more correct).

For example,
- USB dwc3 platform/integration layer dev creates DWC3-core device (of_platform_populate())
- DWC3-core device creates xhci device
- USB dwc3 platform/integration layer dev can dynamically get rev info
from HW and identify limitation/erratas/quirks which need to be applied to
to its children.
- DWC3-core can dynamically get rev info ...

I was not able to find any other proper way to pass this (platfrom)data to children, except
using Auxdata. And, as per Felipe Balbi, It was the major reason why now
DWC3-core device creates xhci device manually.

--
regards,
-grygorii

2016-04-07 11:17:56

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH] of/platform: Allow secondary compatible match in of_dev_lookup


Hi,

Grygorii Strashko <[email protected]> writes:
> On 04/07/2016 07:52 AM, Rob Herring wrote:
>> On Fri, Apr 1, 2016 at 4:40 PM, Tony Lindgren <[email protected]> wrote:
>>> * Tony Lindgren <[email protected]> [160401 14:37]:
>>>> We currently try to match of_dev_auxdata based on compatible,
>>>> IO address, and device name. But in some cases we have multiple
>>>> instances of drivers that can use the same auxdata.
>>>>
>>>> Let's add an additional secondary lookup for generic compatible
>>>> match for auxdata if no device specific match is found. This does
>>>> not change the existing matching, and still allows adding device
>>>> specific auxdata.
>>>>
>>>> This simplifies things as specifying the IO address and device
>>>> name is prone errors as it requires maintaining an in kernel
>>>> database for each SoC.
>>>
>>> And here's what I can apply later on to get rid of some
>>> ifdeffery.
>>>
>>> I'm also planning to move some of the legacy omap hwmod
>>> functionality into proper device drivers, so can generic
>>> pdata for that too.
>>
>> Why can't the platform data be moved into the driver given that it
>> appears to be only SoC family specific? Auxdata was somewhat intended
>> to be temporary. It appears there is already some per compatible match
>> data for these OMAP parts in the driver.
>>
>
> Most probably this is required to pass some data from parent device to
> children when parent dev instantiate children from DT, at least I've experimented with
> this in mostly similar way (I've not added second pass and did break in the first
> if !phys_addr, but Tony's patch is more correct).
>
> For example,
> - USB dwc3 platform/integration layer dev creates DWC3-core device (of_platform_populate())
> - DWC3-core device creates xhci device
> - USB dwc3 platform/integration layer dev can dynamically get rev info
> from HW and identify limitation/erratas/quirks which need to be applied to
> to its children.
> - DWC3-core can dynamically get rev info ...
>
> I was not able to find any other proper way to pass this (platfrom)data to children, except
> using Auxdata. And, as per Felipe Balbi, It was the major reason why now
> DWC3-core device creates xhci device manually.

doesn't device_property() solve that ? I'm just waiting for some patches
from Heikki (in Cc) before we rip pdata from dwc3 completely. At that
point, we can look at building xhci from DT.

The only thing that comes to mind is how this will look for PCI-only
systems if we rip manual xHCI device creation completely.

--
balbi


Attachments:
signature.asc (818.00 B)

2016-04-07 16:58:50

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH] of/platform: Allow secondary compatible match in of_dev_lookup

* Rob Herring <[email protected]> [160406 21:53]:
> On Fri, Apr 1, 2016 at 4:40 PM, Tony Lindgren <[email protected]> wrote:
> > * Tony Lindgren <[email protected]> [160401 14:37]:
> >> We currently try to match of_dev_auxdata based on compatible,
> >> IO address, and device name. But in some cases we have multiple
> >> instances of drivers that can use the same auxdata.
> >>
> >> Let's add an additional secondary lookup for generic compatible
> >> match for auxdata if no device specific match is found. This does
> >> not change the existing matching, and still allows adding device
> >> specific auxdata.
> >>
> >> This simplifies things as specifying the IO address and device
> >> name is prone errors as it requires maintaining an in kernel
> >> database for each SoC.
> >
> > And here's what I can apply later on to get rid of some
> > ifdeffery.
> >
> > I'm also planning to move some of the legacy omap hwmod
> > functionality into proper device drivers, so can generic
> > pdata for that too.
>
> Why can't the platform data be moved into the driver given that it
> appears to be only SoC family specific? Auxdata was somewhat intended
> to be temporary. It appears there is already some per compatible match
> data for these OMAP parts in the driver.

There are just too many dependencies to move legacy code into drivers
directly. Especially when moving the omap hwmod code into drivers,
we still to use hwmod callbacks at least for clockdomain configuration,
wake-up dependencies and clock autogating configuration.

When we have Linux generic frameworks available for all this we no longer
need the auxdata. But meanwhile, removing the depenencies by using
auxdata already allows moving big chunks of the hwmod code into regular
device drivers.

Regards,

Tony

2016-04-12 15:13:33

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH] of/platform: Allow secondary compatible match in of_dev_lookup

* Felipe Balbi <[email protected]> [160407 04:19]:
> Grygorii Strashko <[email protected]> writes:
> > Most probably this is required to pass some data from parent device to
> > children when parent dev instantiate children from DT, at least I've experimented with
> > this in mostly similar way (I've not added second pass and did break in the first
> > if !phys_addr, but Tony's patch is more correct).

OK that's a different use case I have, but good to know it helps
with that too.

> > For example,
> > - USB dwc3 platform/integration layer dev creates DWC3-core device (of_platform_populate())
> > - DWC3-core device creates xhci device
> > - USB dwc3 platform/integration layer dev can dynamically get rev info
> > from HW and identify limitation/erratas/quirks which need to be applied to
> > to its children.
> > - DWC3-core can dynamically get rev info ...
> >
> > I was not able to find any other proper way to pass this (platfrom)data to children, except
> > using Auxdata. And, as per Felipe Balbi, It was the major reason why now
> > DWC3-core device creates xhci device manually.
>
> doesn't device_property() solve that ? I'm just waiting for some patches
> from Heikki (in Cc) before we rip pdata from dwc3 completely. At that
> point, we can look at building xhci from DT.
>
> The only thing that comes to mind is how this will look for PCI-only
> systems if we rip manual xHCI device creation completely.

Sounds like this dwc3 thread is a bit separate from the $subject patch
except for the legacy usage.

Regards,

Tony

2016-04-12 15:56:46

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH] of/platform: Allow secondary compatible match in of_dev_lookup

On Thu, Apr 7, 2016 at 11:58 AM, Tony Lindgren <[email protected]> wrote:
> * Rob Herring <[email protected]> [160406 21:53]:
>> On Fri, Apr 1, 2016 at 4:40 PM, Tony Lindgren <[email protected]> wrote:
>> > * Tony Lindgren <[email protected]> [160401 14:37]:
>> >> We currently try to match of_dev_auxdata based on compatible,
>> >> IO address, and device name. But in some cases we have multiple
>> >> instances of drivers that can use the same auxdata.
>> >>
>> >> Let's add an additional secondary lookup for generic compatible
>> >> match for auxdata if no device specific match is found. This does
>> >> not change the existing matching, and still allows adding device
>> >> specific auxdata.
>> >>
>> >> This simplifies things as specifying the IO address and device
>> >> name is prone errors as it requires maintaining an in kernel
>> >> database for each SoC.
>> >
>> > And here's what I can apply later on to get rid of some
>> > ifdeffery.
>> >
>> > I'm also planning to move some of the legacy omap hwmod
>> > functionality into proper device drivers, so can generic
>> > pdata for that too.
>>
>> Why can't the platform data be moved into the driver given that it
>> appears to be only SoC family specific? Auxdata was somewhat intended
>> to be temporary. It appears there is already some per compatible match
>> data for these OMAP parts in the driver.
>
> There are just too many dependencies to move legacy code into drivers
> directly. Especially when moving the omap hwmod code into drivers,
> we still to use hwmod callbacks at least for clockdomain configuration,
> wake-up dependencies and clock autogating configuration.
>
> When we have Linux generic frameworks available for all this we no longer
> need the auxdata. But meanwhile, removing the depenencies by using
> auxdata already allows moving big chunks of the hwmod code into regular
> device drivers.

Okay. In that case:

Acked-by: Rob Herring <[email protected]>

Apply it when you have actual users depending on it.

Rob

2016-04-12 16:32:04

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH] of/platform: Allow secondary compatible match in of_dev_lookup

* Rob Herring <[email protected]> [160412 08:57]:
> On Thu, Apr 7, 2016 at 11:58 AM, Tony Lindgren <[email protected]> wrote:
> > * Rob Herring <[email protected]> [160406 21:53]:
> >> On Fri, Apr 1, 2016 at 4:40 PM, Tony Lindgren <[email protected]> wrote:
> >> > * Tony Lindgren <[email protected]> [160401 14:37]:
> >> >> We currently try to match of_dev_auxdata based on compatible,
> >> >> IO address, and device name. But in some cases we have multiple
> >> >> instances of drivers that can use the same auxdata.
> >> >>
> >> >> Let's add an additional secondary lookup for generic compatible
> >> >> match for auxdata if no device specific match is found. This does
> >> >> not change the existing matching, and still allows adding device
> >> >> specific auxdata.
> >> >>
> >> >> This simplifies things as specifying the IO address and device
> >> >> name is prone errors as it requires maintaining an in kernel
> >> >> database for each SoC.
> >> >
> >> > And here's what I can apply later on to get rid of some
> >> > ifdeffery.
> >> >
> >> > I'm also planning to move some of the legacy omap hwmod
> >> > functionality into proper device drivers, so can generic
> >> > pdata for that too.
> >>
> >> Why can't the platform data be moved into the driver given that it
> >> appears to be only SoC family specific? Auxdata was somewhat intended
> >> to be temporary. It appears there is already some per compatible match
> >> data for these OMAP parts in the driver.
> >
> > There are just too many dependencies to move legacy code into drivers
> > directly. Especially when moving the omap hwmod code into drivers,
> > we still to use hwmod callbacks at least for clockdomain configuration,
> > wake-up dependencies and clock autogating configuration.
> >
> > When we have Linux generic frameworks available for all this we no longer
> > need the auxdata. But meanwhile, removing the depenencies by using
> > auxdata already allows moving big chunks of the hwmod code into regular
> > device drivers.
>
> Okay. In that case:
>
> Acked-by: Rob Herring <[email protected]>
>
> Apply it when you have actual users depending on it.

OK thanks will do.

Regards,

Tony