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

[Pantelis Antoniou <panto@antoniou-consulting.com>]

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