Re: [PATCH 4/5] omap: Avoid crashes in the case of hwmod misconfiguration

Pantelis Antoniou
2013-08-08T09:29:08Z

Hi Kevin,

On Aug 8, 2013, at 12:15 AM, Kevin Hilman wrote:

> Pantelis Antoniou <panto@antoniou-consulting.com> 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