Messages in this thread | | | Subject | Re: checkpatch: Question regarding asmlinkage and storage class | From | Joe Perches <> | Date | Sun, 19 Mar 2017 01:31:01 -0700 |
| |
On Sat, 2017-03-18 at 13:15 +0100, Paul Menzel wrote: > Dear checkpatch developers, > > > The coreboot project started using checkpatch.pl, and now some effort > is going into fixing issues pointed out by `checkpatch.pl`. > > The file `src/arch/x86/acpi_s3.c` in coreboot contains the code below. > > ``` > 205 void (*acpi_do_wakeup)(uintptr_t vector, u32 backup_source, u32 backup_target, > 206 u32 backup_size) asmlinkage = (void *)WAKEUP_BASE; > ``` > > The warning is > > > WARNING: storage class should be at the beginning of the declaration > > which raised the question below [2]. > > > And I am waiting for someone to answer why checkpatch.pl claims > > asmlinkage as a storage-class in the first place. [] > In coreboot the macro is defined similarly to Linux. > > ``` > #define asmlinkage __attribute__((regparm(0))) > #define alwaysinline inline __attribute__((always_inline)) > ```
Are they similar?
$ git grep -i "define.*ASMLINKAGE\b" include include/linux/linkage.h:#define CPP_ASMLINKAGE extern "C" include/linux/linkage.h:#define CPP_ASMLINKAGE include/linux/linkage.h:#define asmlinkage CPP_ASMLINKAGE
I believe asmlinkage is defined just to avoid possible asm/c++ symbol decorations.
> In Linux, commit 9c0ca6f9 (update checkpatch.pl to version 0.10) seems > to have introduced the check. The commit message contains “asmlinkage > is also a storage type”. > > Furthermore, `checkpatch.pl` doesn’t seem to warn about the code below. > > ``` > void __attribute__((weak)) mainboard_suspend_resume(void) > ``` > > This raises the question below. > > > It appears coreboot proper mostly followed this placement for > > function attributes before. It would be nice if we were consistent, > > specially if checkpatch starts to complaint about these. > > Is there another reason, besides not having that implemented? > > I am looking forward to your answers. > > > Kind regards, > > Paul > > > [1] https://review.coreboot.org/#/c/18865/1/src/arch/x86/acpi_s3.c@205 > [2] https://review.coreboot.org/18865/ > [3] https://review.coreboot.org/#/c/18865/1/src/arch/x86/acpi_s3.c@244
| |