From: Stephen Boyd <sboyd@codeaurora.org>
To: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Liam Girdwood <lrg@ti.com>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] regulator: Fix and simplify debugfs support
Date: Sun, 19 Feb 2012 23:46:40 -0800 [thread overview]
Message-ID: <4F41FA60.7090503@codeaurora.org> (raw)
In-Reply-To: <20120220021157.GF3194@opensource.wolfsonmicro.com>
On 2/19/2012 6:12 PM, Mark Brown wrote:
> On Sat, Feb 18, 2012 at 11:28:25PM -0800, Stephen Boyd wrote:
>> If CONFIG_DEBUG_FS=y debugfs functions will never return an
>> ERR_PTR. Instead they'll return NULL. The intent is to remove
>> ifdefs in calling code.
>> Instead of checking for an ERR_PTR check for NULL. This simplifies
>> the code and also fixes an error check that would never have
>> worked otherwise. While we're here modernize the code to use
>> S_IRUGO instead of 0444.
> This was actually a deliberate decision to make the code more robust
> against change - the IS_ERR_OR_NULL doesn't make the code any bigger but
> it means it's less likely to break in the face of changes.
More robust? The debugfs code in the regulator core looks confused on
what the return value is. Sometimes it's IS_ERR, sometimes it's NULL,
sometimes it's both. Might as well clean it up to be consistent and proper.
>> If we're willing to sacrifice a pointer per rdev we can remove the
>> ifdefs and the compiler should be able to optimize away the dead
>> code.
> Personally I'd be happy to do that, the only reason I put the ifdefs in
> there was that it appears to be idiomatic to do so but I'm not really a
> big fan of it. Then again I never build kernls without debugfs support
> in them myself...
Ok. I'll send a patch to do that too.
How about a two part series with the erroneous error check fix in one
and then the other stuff that isn't critical in another?
next prev parent reply other threads:[~2012-02-20 7:46 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-19 7:28 [PATCH] regulator: Fix and simplify debugfs support Stephen Boyd
2012-02-19 7:51 ` Hillf Danton
2012-02-20 2:12 ` Mark Brown
2012-02-20 7:46 ` Stephen Boyd [this message]
2012-02-20 20:57 ` Mark Brown
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=4F41FA60.7090503@codeaurora.org \
--to=sboyd@codeaurora.org \
--cc=broonie@opensource.wolfsonmicro.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lrg@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;
as well as URLs for NNTP newsgroup(s).