Re: [PM-WIP-OPP][PATCH 1/4] omap3: pm: cpufreq: BUG_ON cleanup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Kevin Hilman had written, on 03/19/2010 01:42 PM, the following:
Felipe Balbi <me@xxxxxxxxxxxxxxx> writes:

On Fri, Mar 19, 2010 at 10:46:54AM -0700, Kevin Hilman wrote:
IMO, Using BUG* macros usually indicates improper or incomplete error
handling rather than a real catastrophic system failure.
on the other hand a kernel oops and system hang will always get
noted. Rather than a WARN() which simply sits in the log buffer.

Of course, but what I'm trying to avoid is making other people deal
with a BUG inserted by a developer when proper error checking and
recovery is what is really needed.

I respect your views. but a few moments of thoughts:
how would the recovery look like? I can think of 2 options here.. do share your views:

Option 1:
if (opp_init_list(OPP_MPU, omap3_opp_def_list[0])) {
	WARN("dsp OPP table registration failed");
	return;
}
if (opp_init_list(OPP_L3, omap3_opp_def_list[1])) {
	WARN("dsp OPP table registration failed");
	return;
}
if (opp_init_list(OPP_DSP, omap3_opp_def_list[2])) {
	WARN("dsp OPP table registration failed");
	return;
}

Option 2:
	if (opp_init_list(OPP_MPU, omap3_opp_def_list[0]))
		return;
	if (opp_init_list(OPP_L3, omap3_opp_def_list[1]))
		goto mpu_disable;
	if (opp_init_list(OPP_DSP, omap3_opp_def_list[2]))
		goto l3_disable;
	return;

l3_disable:
	freq = 0;
	while (!IS_ERR(opp = opp_find_freq_ceil(OPP_L3, &freq)) {
		opp_disable(opp);
		freq++;
	}
mpu_disable:
	freq = 0;
	while (!IS_ERR(opp = opp_find_freq_ceil(OPP_MPU, &freq)) {
		opp_disable(opp);
		freq++;
	}
	WARN("Registration of OPP tables failed!!");
	return;

Option 1 is a bad idea as it leaves the system in an invalid state
Option 2 is the better idea as we dont have a opp_delete option(not required usually).

All that code for something that will almost never happen?	
--
Regards,
Nishanth Menon
--
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

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux