* Tony Lindgren <[email protected]> [131121 12:49]: > * Tony Lindgren <[email protected]> [131120 17:46]: > > * Tony Lindgren <[email protected]> [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 <[email protected]> > 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" <[email protected]> > Cc: Paul Walmsley <[email protected]> > Signed-off-by: Tony Lindgren <[email protected]> > > 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 [email protected] > 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
