[coreboot] What is the best way to treat warnings reported by checkpatch.pl

[Zeh, Werner]

Hi community.

I recently ran into a blocked commit due to warnings reported by checkpatch.pl.
In this particular case I wanted to add the "ATSR" structure to the DMAR tables. To do that, I need to define a new structure
which reflects the per specification needed layout of this table entry.
I did that in the same style how other structures are defined in the same file (src/arch/x86/include/arch/acpi.h):

typedef struct dmar_atsr_entry {
            u16 type;
            u16 length;
            u8 flags;
            u8 reserved;
            u16 segment;
} __attribute__ ((packed)) dmar_atsr_entry_t;

This change resulted in two warnings caused by util/lint/checkpatch.pl when I was trying to do a git commit:

WARNING: do not add new typedefs
#34: FILE: src/arch/x86/include/arch/acpi.h:288:
+typedef struct dmar_atsr_entry {

WARNING: __packed is preferred over __attribute__((packed))
#40: FILE: src/arch/x86/include/arch/acpi.h:294:
+} __attribute__ ((packed)) dmar_atsr_entry_t;

So the first warning tells me that I should not use "typedef" in my code.

The second one tells me not to use __attribute__ ((packed) but instead __packed!
If one take a closer look to the coreboot tree, one will not find a macro definition called __packed at all.

After a short discussion on IRC with Stefan it was clear that although we have this checkpatch.pl script in the tree and the git hook "pre-commit"
activates it, the script does not perfectly match current coreboot coding style.

So in the end there needs to be a change somewhere:

1.     We can adapt the script to match current coreboot coding style

2.     We can start to refactor the coreboot tree to match the demands of checkpatch.pl

3.     We can align only new patches to the demands of the script

4.     We can show warnings from the script but prevent them from aborting the git commit

While 1. would be the best, straight forward approach for the project, it might cause high effort for a person who knows the code base very well.
2. will lead to a lot of changes in the tree while the reason/goal is still questionable (at least to me).
3. will lead to different code style all over the tree (even in the same file) and in the end will result in fragmented code. I personally would not like it.
4. can be a short term solution while we are seeking for the best way to deal with this issue.

There may be other ways to go, too, which I have overseen now.

I just want to start this discussion officially as this is not the first time I ran into issues with checkpatch.pl.
What do you think about it, what would be the best way to handle this case?

Your ideas and thoughts are appreciated.

Werner