public inbox for linux-omap@vger.kernel.org
 help / color / mirror / Atom feed
From: "Cousson, Benoit" <b-cousson@ti.com>
To: "Nayak, Rajendra" <rnayak@ti.com>, Paul Walmsley <paul@pwsan.com>
Cc: "linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 0/3] OMAP2+ hwmod fixes
Date: Tue, 1 Mar 2011 17:57:45 +0100	[thread overview]
Message-ID: <4D6D2589.3090505@ti.com> (raw)
In-Reply-To: <19850e063c924a8a30c69ac02621a659@mail.gmail.com>

Hi Paul,

On 2/23/2011 11:05 AM, Nayak, Rajendra wrote:
> Hi Paul,
>
>> From: Paul Walmsley [mailto:paul@pwsan.com]
>> Sent: Wednesday, February 23, 2011 12:40 AM
>>
>> Hi Rajendra
>>
>> On Tue, 22 Feb 2011, Rajendra Nayak wrote:
>>
>>> The original behavior of the iterators, to terminate upon
>>> encountering an error, seems fine to me. The only problem
>>> I faced was that they fail silently and go undetected, unless
>>> their user catches the return value and WARN's, which I found
>>> was not the case with most users, mainly those of
>>> omap_hwmod_for_each_by_class.
>>> I was thinking of keeping the behaviour of these iterators
>>> same for now and add WARN's in these iterators itself upon
>>> an error, so its seen even if the user fails to catch it.
>>
>> What's your opinion on adding the pr_err() or WARN() into the code that
>> the iterator calls for each hwmod?  That code should know why something
>> fails, so it should be able to provide a more detailed error message.
> Of
>> course, it is not as general a solution...
>
> I agree, if the callback functions are written with proper errors
> or WARN's, they are the right place where most of the details'
> exist. So maybe we don't need these in the iterator's after all.

So to conclude, I will drop the #3 and just push #1 and #2.

#1 is fine with addition of the WARN.
#2 does return an error but does not print anything, but since each call 
(_init_main_clk, _init_interface_clks, _init_opt_clks) does report
some pr_warn in case of error, this is fine.


Regards,
Benoit


  reply	other threads:[~2011-03-01 16:57 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-16 12:11 [PATCH 0/3] OMAP2+ hwmod fixes Rajendra Nayak
2011-02-16 12:11 ` [PATCH 1/3] OMAP2+: hwmod: Avoid setup if clock lookup failed Rajendra Nayak
2011-02-16 12:11   ` [PATCH 2/3] OMAP2+: hwmod: Fix what _init_clock returns Rajendra Nayak
2011-02-16 12:11     ` [PATCH 3/3] OMAP2+: hwmod: Do not break iterator fn's if one fails Rajendra Nayak
2011-02-18 14:51     ` [PATCH 2/3] OMAP2+: hwmod: Fix what _init_clock returns Cousson, Benoit
2011-02-16 12:35   ` [PATCH 1/3] OMAP2+: hwmod: Avoid setup if clock lookup failed Sergei Shtylyov
2011-02-18 17:34   ` Cousson, Benoit
2011-02-16 13:07 ` [PATCH 0/3] OMAP2+ hwmod fixes Cousson, Benoit
2011-02-16 13:43   ` Rajendra Nayak
2011-02-18 14:44     ` Cousson, Benoit
2011-02-21 16:55       ` Paul Walmsley
2011-02-22 13:11         ` Rajendra Nayak
2011-02-22 19:09           ` Paul Walmsley
2011-02-23 10:05             ` Rajendra Nayak
2011-03-01 16:57               ` Cousson, Benoit [this message]
2011-03-03  6:08                 ` Paul Walmsley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4D6D2589.3090505@ti.com \
    --to=b-cousson@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=paul@pwsan.com \
    --cc=rnayak@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox