From: Tom Rini <trini@ti.com>
To: Joe Perches <joe@perches.com>, Josh Triplett <josh@joshtriplett.org>
Cc: <linux-kernel@vger.kernel.org>,
Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH] checkpatch.pl: Add warning for new __packed additions
Date: Mon, 24 Feb 2014 16:52:16 -0500 [thread overview]
Message-ID: <530BBF10.6050601@ti.com> (raw)
In-Reply-To: <1393277304.11020.64.camel@joe-AO722>
On 02/24/2014 04:28 PM, Joe Perches wrote:
> On Mon, 2014-02-24 at 16:11 -0500, Tom Rini wrote:
>> On 02/24/2014 04:00 PM, Joe Perches wrote:
>>> On Mon, 2014-02-24 at 15:38 -0500, Tom Rini wrote:
>>>> While there are valid reasons to use __packed, often the answer is that
>>>> you should be doing something else here instead.
> []
>>> How often is this actually a problem?
>>
>> I think the first line answers the second one, honestly. If one wants
>> to get pedantic about things and really investigate there's probably
>> some unneeded usages scattered about, and that's generally the type of
>> thing one wants to address when checking whole files, right?
>
> Maybe not.
>
> That entirely depends on the correct and necessary uses of
> packed vs the incorrect usage rates.
>
> I think almost all packed uses are correct and there might
> be a lot of patches submitted to remove them by over-zealous
> advocates of checkpatch -f.
To try and also answer Josh's feedback as well, I've been lead to
believe that most cases now people should be using regmap instead, which
just leaves the case of having to match on-disk formats or similar cases
I believe as the things that must stay __packed.
>>> This may be better as
>>> "Using 'packed' can impact performance\n"
>>> and only tested when not in --file mode.
>>
>> I can also make this change, sure, just point me off-list for an example
>> to crib from and test?
>
> Look at the FSF mailing address test as an example:
>
> my $msg_type = \&ERROR;
> $msg_type = \&CHK if ($file);
> &{$msg_type}("FSF_MAILING_ADDRESS",
OK, thanks, I'll make something happen, and drop it to a CHK too.
--
Tom
next prev parent reply other threads:[~2014-02-24 21:52 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-24 20:38 [PATCH] checkpatch.pl: Add warning for new __packed additions Tom Rini
2014-02-24 21:00 ` Joe Perches
2014-02-24 21:11 ` Tom Rini
2014-02-24 21:28 ` Joe Perches
2014-02-24 21:52 ` Tom Rini [this message]
2014-02-24 22:02 ` Joe Perches
2014-02-24 22:04 ` Tom Rini
2014-02-24 22:08 ` Joe Perches
2014-02-24 22:20 ` Tom Rini
2014-02-24 22:31 ` Joe Perches
2014-02-24 22:43 ` Tom Rini
2014-02-25 5:23 ` Joe Perches
2014-02-25 12:30 ` Tom Rini
2014-02-26 22:04 ` Joe Perches
2014-02-27 20:33 ` Tom Rini
2014-02-25 8:56 ` Heiko Carstens
2014-02-24 21:31 ` josh
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=530BBF10.6050601@ti.com \
--to=trini@ti.com \
--cc=akpm@linux-foundation.org \
--cc=joe@perches.com \
--cc=josh@joshtriplett.org \
--cc=linux-kernel@vger.kernel.org \
/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