From mboxrd@z Thu Jan  1 00:00:00 1970
From: Rajendra Nayak <rnayak@ti.com>
Subject: RE: [PATCH 3/3] OMAP4: clockdomain: Add wkup/sleep dependency support
Date: Fri, 11 Feb 2011 10:34:34 +0530
Message-ID: <74b7dfbe67e19da4358f7df0bc4dd8f1@mail.gmail.com>
References: <1297084631-27474-1-git-send-email-rnayak@ti.com>
 <1297084631-27474-2-git-send-email-rnayak@ti.com> <1297084631-27474-3-git-send-email-rnayak@ti.com>
 <1297084631-27474-4-git-send-email-rnayak@ti.com> <alpine.DEB.2.00.1102101905160.21991@utopia.booyaka.com>
 <3754d10e94217aa81846aa1d41fe0a9d@mail.gmail.com> <alpine.DEB.2.00.1102102135220.21991@utopia.booyaka.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Return-path: <linux-omap-owner@vger.kernel.org>
Received: from na3sys009aog115.obsmtp.com ([74.125.149.238]:49856 "EHLO
	na3sys009aog115.obsmtp.com" rhost-flags-OK-OK-OK-OK)
	by vger.kernel.org with ESMTP id S1750710Ab1BKFEh (ORCPT
	<rfc822;linux-omap@vger.kernel.org>);
	Fri, 11 Feb 2011 00:04:37 -0500
Received: by mail-fx0-f41.google.com with SMTP id 12so2356468fxm.28
        for <linux-omap@vger.kernel.org>; Thu, 10 Feb 2011 21:04:36 -0800 (PST)
In-Reply-To: <alpine.DEB.2.00.1102102135220.21991@utopia.booyaka.com>
Sender: linux-omap-owner@vger.kernel.org
List-Id: linux-omap@vger.kernel.org
To: Paul Walmsley <paul@pwsan.com>
Cc: linux-omap@vger.kernel.org, Kevin Hilman <khilman@ti.com>, Benoit Cousson <b-cousson@ti.com>, linux-arm-kernel@lists.infradead.org

> -----Original Message-----
> From: Paul Walmsley [mailto:paul@pwsan.com]
> Sent: Friday, February 11, 2011 10:08 AM
> To: Rajendra Nayak
> Cc: linux-omap@vger.kernel.org; Kevin Hilman; Benoit Cousson;
linux-arm-kernel@lists.infradead.org
> 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