* Tony Lindgren <tony@xxxxxxxxxxx> [131121 12:49]:
> * Tony Lindgren <tony@xxxxxxxxxxx> [131120 17:46]:
> > * Tony Lindgren <tony@xxxxxxxxxxx> [131120 16:06]:
> > >
> > > They at least had interrupts listed looking at commit 3b9b10. Probably
> > > the thing to do for now is to revert those changes, and see if we can
> > > just remove the L3 entries from the .dtsi files.
> >
> > Actually the patch I posted should be able to handle also the
> > ti,hwmods = "l3_main_1", "l3_main_2", "l3_main_3" property in omap4.dtsi,
> > but we're not currently parsing that as we only look at the children
> > and not the properties of the OCP bus. Should be fixable, will take a look
> > tomorrow if this approach makes sense to you.
>
> OK this one seems to do the right thing for me.
No comments? I'll queue the patch below to the fixes, please yell
if you see any issues with that.
> Tony
>
>
> From: Tony Lindgren <tony@xxxxxxxxxxx>
> Date: Wed, 20 Nov 2013 15:46:51 -0800
> Subject: [PATCH] ARM: OMAP2+: Fix overwriting hwmod data with data from device tree
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> We have some device tree properties where the ti,hwmod have multiple
> values:
>
> am33xx.dtsi: ti,hwmods = "tpcc", "tptc0", "tptc1", "tptc2";
> am4372.dtsi: ti,hwmods = "tpcc", "tptc0", "tptc1", "tptc2";
> dra7.dtsi: ti,hwmods = "l3_main_1", "l3_main_2";
> omap3.dtsi: ti,hwmods = "mcbsp2", "mcbsp2_sidetone";
> omap3.dtsi: ti,hwmods = "mcbsp3", "mcbsp3_sidetone";
> omap4.dtsi: ti,hwmods = "l3_main_1", "l3_main_2", "l3_main_3";
> omap5.dtsi: ti,hwmods = "l3_main_1", "l3_main_2", "l3_main_3";
>
> That's not correct way of doing things in this case because these are
> separate devices with their own address space, interrupts, SYSCONFIG
> registers and can set their PM states independently.
>
> So they should all be fixed up to be separate devices in the .dts files.
>
> We also have the related data removed for at least omap4 in commit
> 3b9b10151c68 (ARM: OMAP4: hwmod data: Clean up the data file), so
> that data is wrongly initialized as null data.
>
> So we need to fix two bugs:
>
> 1. We are only checking the first entry of the ti,hwmods property
>
> This means that we're only initializing the first hwmods entry
> instead of the ones listed in the ti,hwmods property.
>
> 2. We are only checking the child nodes, not the nodes themselves
>
> This means that anything listed at OCP level is currently just
> ignored and unitialized and at least the omap4 case, with the
> legacy data missing from the hwmod.
>
> Fix both of the issues by using an index to the ti,hwmods property
> and changing the hwmod lookup function to also check the current node
> for ti,hwmods property instead of just the children.
>
> While at it, let's also add some warnings for the bad data so it's
> easier to fix.
>
> Cc: "Benoît Cousson" <bcousson@xxxxxxxxxxxx>
> Cc: Paul Walmsley <paul@xxxxxxxxx>
> Signed-off-by: Tony Lindgren <tony@xxxxxxxxxxx>
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index e3f0eca..ee655da 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -2326,38 +2326,80 @@ static int _shutdown(struct omap_hwmod *oh)
> return 0;
> }
>
> +static int of_dev_find_hwmod(struct device_node *np,
> + struct omap_hwmod *oh)
> +{
> + int count, i, res;
> + const char *p;
> +
> + count = of_property_count_strings(np, "ti,hwmods");
> + if (count < 1)
> + return -ENODEV;
> +
> + for (i = 0; i < count; i++) {
> + res = of_property_read_string_index(np, "ti,hwmods",
> + i, &p);
> + if (res)
> + continue;
> + if (!strcmp(p, oh->name)) {
> + pr_debug("omap_hwmod: dt %s[%i] uses hwmod %s\n",
> + np->name, i, oh->name);
> + return i;
> + }
> + }
> +
> + return -ENODEV;
> +}
> +
> /**
> * of_dev_hwmod_lookup - look up needed hwmod from dt blob
> * @np: struct device_node *
> * @oh: struct omap_hwmod *
> + * @index: index of the entry found
> + * @found: struct device_node * found or NULL
> *
> * Parse the dt blob and find out needed hwmod. Recursive function is
> * implemented to take care hierarchical dt blob parsing.
> - * Return: The device node on success or NULL on failure.
> + * Return: Returns 0 on success, -ENODEV when not found.
> */
> -static struct device_node *of_dev_hwmod_lookup(struct device_node *np,
> - struct omap_hwmod *oh)
> +static int of_dev_hwmod_lookup(struct device_node *np,
> + struct omap_hwmod *oh,
> + int *index,
> + struct device_node **found)
> {
> - struct device_node *np0 = NULL, *np1 = NULL;
> - const char *p;
> + struct device_node *np0 = NULL;
> + int res;
> +
> + res = of_dev_find_hwmod(np, oh);
> + if (res >= 0) {
> + *found = np;
> + *index = res;
> + return 0;
> + }
>
> for_each_child_of_node(np, np0) {
> - if (of_find_property(np0, "ti,hwmods", NULL)) {
> - p = of_get_property(np0, "ti,hwmods", NULL);
> - if (!strcmp(p, oh->name))
> - return np0;
> - np1 = of_dev_hwmod_lookup(np0, oh);
> - if (np1)
> - return np1;
> + struct device_node *fc;
> + int i;
> +
> + res = of_dev_hwmod_lookup(np0, oh, &i, &fc);
> + if (res == 0) {
> + *found = fc;
> + *index = i;
> + return 0;
> }
> }
> - return NULL;
> +
> + *found = NULL;
> + *index = 0;
> +
> + return -ENODEV;
> }
>
> /**
> * _init_mpu_rt_base - populate the virtual address for a hwmod
> * @oh: struct omap_hwmod * to locate the virtual address
> * @data: (unused, caller should pass NULL)
> + * @index: index of the reg entry iospace in device tree
> * @np: struct device_node * of the IP block's device node in the DT data
> *
> * Cache the virtual address used by the MPU to access this IP block's
> @@ -2368,7 +2410,7 @@ static struct device_node *of_dev_hwmod_lookup(struct device_node *np,
> * -ENXIO on absent or invalid register target address space.
> */
> static int __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data,
> - struct device_node *np)
> + int index, struct device_node *np)
> {
> struct omap_hwmod_addr_space *mem;
> void __iomem *va_start = NULL;
> @@ -2390,13 +2432,17 @@ static int __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data,
> if (!np)
> return -ENXIO;
>
> - va_start = of_iomap(np, oh->mpu_rt_idx);
> + va_start = of_iomap(np, index + oh->mpu_rt_idx);
> } else {
> va_start = ioremap(mem->pa_start, mem->pa_end - mem->pa_start);
> }
>
> if (!va_start) {
> - pr_err("omap_hwmod: %s: Could not ioremap\n", oh->name);
> + if (mem)
> + pr_err("omap_hwmod: %s: Could not ioremap\n", oh->name);
> + else
> + pr_err("omap_hwmod: %s: Missing dt reg%i for %s\n",
> + oh->name, index, np->full_name);
> return -ENXIO;
> }
>
> @@ -2422,17 +2468,29 @@ static int __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data,
> */
> static int __init _init(struct omap_hwmod *oh, void *data)
> {
> - int r;
> + int r, index;
> struct device_node *np = NULL;
>
> if (oh->_state != _HWMOD_STATE_REGISTERED)
> return 0;
>
> - if (of_have_populated_dt())
> - np = of_dev_hwmod_lookup(of_find_node_by_name(NULL, "ocp"), oh);
> + if (of_have_populated_dt()) {
> + struct device_node *bus;
> +
> + bus = of_find_node_by_name(NULL, "ocp");
> + if (!bus)
> + return -ENODEV;
> +
> + r = of_dev_hwmod_lookup(bus, oh, &index, &np);
> + if (r)
> + pr_debug("omap_hwmod: %s missing dt data\n", oh->name);
> + else if (np && index)
> + pr_warn("omap_hwmod: %s using broken dt data from %s\n",
> + oh->name, np->name);
> + }
>
> if (oh->class->sysc) {
> - r = _init_mpu_rt_base(oh, NULL, np);
> + r = _init_mpu_rt_base(oh, NULL, index, np);
> if (r < 0) {
> WARN(1, "omap_hwmod: %s: doesn't have mpu register target base\n",
> oh->name);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
