Re: [PATCH] checkpatch.pl: Add warning for new __packed additions

[Tom Rini]

On 02/24/2014 05:31 PM, Joe Perches wrote:
> On Mon, 2014-02-24 at 17:20 -0500, Tom Rini wrote:
>> On 02/24/2014 05:08 PM, Joe Perches wrote:
>>> On Mon, 2014-02-24 at 17:04 -0500, Tom Rini wrote:
>>>> I've got this modified to a CHK and only for non-file usage.  Anything
>>>> else we want to talk about before I repost?
>>>
>>> Probably not, but I'm still not convinced it's useful.
>>>
>>> Have you found a case where it's currently specified
>>> but not useful?
>>
>> Well, U-Boot and the kernel both share the dubious to incorrect __packed
>> horror of cros_ec things (see include/linux/mfd/cros_ec_commands.h).
> 
> Are the __packed entries in cros_ec dubious?
> 
> Maybe the ones that don't seem to need them
> because the are naturally 32 bit aligned, but
> the others that are u16 aligned probably _do_
> need __packed.

There's so many unused entries in there that I cannot honestly say if
things need to be packed or not.

>> If this is really not seen as useful for the kernel, that's fine, I'll
>> drop it.  I mainly did this for U-Boot where we do want a bit more loud
>> screaming going on when people add __packed to make sure it's for a good
>> reason.  Wanted to be a good neighbor so to speak and see if upstream
>> wants it too.
> 
> I don't mind keeping checkpatch compatible with U-Boot
> requirements, but probably not on by default.
> 
> Maybe there would be some "UBOOT-<foo>" type specific entries
> that could be default off but enabled with some wildcard or
> a .checkpatch.conf specific file for U-Boot.

My perl is quite limited, so however much effort you're interested in
putting in here is greatly appreciated (even if it's pointing out
something else already in the script to copy and modify).  We already
ship a .checkpatch.conf so having to enable something by default is
fine.  Thanks!

-- 
Tom
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/