> -----Original Message----- > From: Paul Walmsley [mailto:paul@xxxxxxxxx] > Sent: Friday, February 11, 2011 10:08 AM > To: Rajendra Nayak > Cc: linux-omap@xxxxxxxxxxxxxxx; Kevin Hilman; Benoit Cousson; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
