On Tue, 2010-10-05 at 08:01 -0700, ext Kevin Hilman wrote:
> Felipe Balbi <balbi@xxxxxx> writes:
>
> > Hi,
> >
> > On Tue, Oct 05, 2010 at 03:42:10AM -0500, Evgeny Kuznetsov wrote:
> >>+ if (!isr_reg) {
> >>+ printk(KERN_ERR "FATAL: Incorrect GPIO method %i\n",
> >>+ bank->method);
> >>+ BUG();
> >>+ }
> >
> > this could be simply:
> >
> > BUG_ON(!isr_reg);
>
> WARN_ON is better.
>
> A BUG() will panic the kernel and stop everything. This is not an error
> condition that should prevent the entire kernel from running.
'isr_reg' is dereferenced later in code:
...
isr_saved = isr = __raw_readl(isr_reg) & enabled;
...
So this will stop kernel anyway.
I just hoped to help in understanding of issue by log line. WARN_ON
could be used for this.
As a variant compilation error could be added, to prevent situation when
kernel is incorrectly configured.
E.g.:
#if !defined(CONFIG_ARCH_OMAP1) &&
!defined(CONFIG_ARCH_OMAP15XX) &&
!defined(CONFIG_ARCH_OMAP16XX) &&
!defined(CONFIG_ARCH_OMAP730) &&
!defined(CONFIG_ARCH_OMAP850) &&
!defined(CONFIG_ARCH_OMAP2) &&
!defined(CONFIG_ARCH_OMAP3) &&
!defined(CONFIG_ARCH_OMAP4)
#error "Incorrect arch configuration"
#endif
But there are still cases when 'isr_reg' could have NULL value (if
'bank->method' is not equal to configured one).
Regards,
Evgeny
>
> From asm-generic/bug.h:
>
> /*
> * Don't use BUG() or BUG_ON() unless there's really no way out; one
> * example might be detecting data structure corruption in the middle
> * of an operation that can't be backed out of. If the (sub)system
> * can somehow continue operating, perhaps with reduced functionality,
> * it's probably not BUG-worthy.
> *
> * If you're tempted to BUG(), think again: is completely giving up
> * really the *only* solution? There are usually better options, where
> * users don't need to reboot ASAP and can mostly shut down cleanly.
> */
--
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
