> -----Original Message-----
> From: Paul Walmsley [mailto:[email protected]]
> Sent: Friday, February 11, 2011 10:08 AM
> To: Rajendra Nayak
> Cc: [email protected]; Kevin Hilman; Benoit Cousson;
[email protected]
> Subject: RE: [PATCH 3/3] OMAP4: clockdomain: Add wkup/sleep dependency
support
>
> On Fri, 11 Feb 2011, Rajendra Nayak wrote:
>
> > My initial version actually did have a check for cd->clkdm_name
instead
> > of cd->clkdm, and then I ran into aborts when a clkdm, though
belonging
> > to the right chip version, failed lookup (in clkdm_init) and left the
> > cd->clkdm pointer NULL. This however was due to the fact that the
> > clkdm_name populated was'nt matching the actual name,
>
> So those aborts were due to clockdomain or clockdomain dependency data
> that had errors that caused it not to have referential integrity?
Yes, I specifically found it when my script updates were actually
generating some non-matching (and hence wrong) clkdm_names.
The aborts actually helped me fix it...
>
> > Would it make sense to add an additional check here to avoid
> > an abort in case of mismatches in clkdm_name populated and
> > lookup's failing in clkdm_init?
> >
> > Something like...
> >
> > If (cd->clkdm) {
> > |= 1 << cd->clkdm->dep_bit;
> > atomic_set(&cd->wkdep_usecount, 0);
> > }
>
> That is going to fail silently. If I'm understanding the problem
> that you're referring to correctly, it seems to me that in these
> circumstances, we want to fail loudly. Especially now that all that
data
> is supposed to be autogenerated. It is a symptom of a more profound
> problem that the end user should never see, no?
... so you are right. Failing silently is going to make it more difficult
to identify and fix. Maybe a WARN in else?
if (cd->clkdm) {
...
} else
WARN()
>
>
> - Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
