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

Hi Kevin,

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

Pantelis Antoniou [off-list ref] writes:

quoted

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.

[...]

quoted

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