Hi Kevin, On Aug 8, 2013, at 12:15 AM, Kevin Hilman wrote: > Pantelis Antoniou <panto@xxxxxxxxxxxxxxxxxxxxxxx> writes: > >> omap hwmod is really sensitive to hwmod misconfiguration. >> Getting a minor clock wrong always ended up in a crash. >> Attempt to be more resilient by not assigning variables with >> error codes and then attempting to use them. >> >> Without this patch, missing a clock ends up with something like this: >> omap_hwmod: ehrpwm0: cannot clk_get opt_clk ehrpwm0_tbclk! > > Definitely agree we should not be crashing when given bad data. > > nit Re: "missing clock". I don't think there will be any crash if a > clock is missing. This looks to me more like the clock name is wrong > (tbclk instead of dbclk?), not missing. > Yes, I'll rephrase. > [...] > >> index 7341eff..42cb7d4 100644 >> --- a/arch/arm/mach-omap2/omap_hwmod.c >> +++ b/arch/arm/mach-omap2/omap_hwmod.c >> @@ -784,7 +784,9 @@ static int _init_interface_clks(struct omap_hwmod *oh) >> if (IS_ERR(c)) { >> pr_warning("omap_hwmod: %s: cannot clk_get interface_clk %s\n", >> oh->name, os->clk); >> - ret = -EINVAL; >> + if (ret == 0) >> + ret = -EINVAL; >> + continue; > > the 'if (ret == 0)' adds confusion IMO. If we don't care additional > failures, errors, then just add a 'break' instead of these 3 lines. > > [...] > I tried to carry on as much as possible even on the presence of errors. The remaining clocks won't be initialized, but that might be OK. > Kevin Regards -- Pantelis -- 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