Message-ID: <1489915463.11604.38.camel@users.sourceforge.net>
Subject: Re: [coreboot] checkpatch: Question regarding asmlinkage and
 storage class
From: Paul Menzel <paulepanter@users.sourceforge.net>
To: Joe Perches <joe@perches.com>, Andy Whitcroft <apw@canonical.com>
Cc: linux-kernel@vger.kernel.org, coreboot@coreboot.org
Date: Sun, 19 Mar 2017 10:24:23 +0100
In-Reply-To: <1489912261.13953.22.camel@perches.com>
References: <1489839338.9183.68.camel@users.sourceforge.net>
         <1489912261.13953.22.camel@perches.com>
Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature";
        boundary="=-Vn+nrgGzHfAjCELyqcfj"
Mime-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3519
Lines: 117


--=-Vn+nrgGzHfAjCELyqcfj
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Dear Joe,


Am Sonntag, den 19.03.2017, 01:31 -0700 schrieb Joe Perches:
> On Sat, 2017-03-18 at 13:15 +0100, Paul Menzel wrote:
> > Dear checkpatch developers,
> >=20
> >=20
> > The coreboot project started using checkpatch.pl, and now some effort
> > is going into fixing issues pointed out by `checkpatch.pl`.
> >=20
> > The file `src/arch/x86/acpi_s3.c` in coreboot contains the code below.
> >=20
> > ```
> > =C2=A0=C2=A0=C2=A0205	void (*acpi_do_wakeup)(uintptr_t vector, u32 back=
up_source, u32 backup_target,
> > =C2=A0=C2=A0=C2=A0206		u32 backup_size) asmlinkage =3D (void *)WAKEUP_B=
ASE;
> > ```
> >=20
> > The warning is
> >=20
> > > WARNING: storage class should be at the beginning of the declaration
> >=20
> > which raised the question below [2].
> >=20
> > > And I am waiting for someone to answer why checkpatch.pl claims
> > > asmlinkage as a storage-class in the first place.
>=20
> []
> > In coreboot the macro is defined similarly to Linux.
> >=20
> > ```
> > #define asmlinkage __attribute__((regparm(0)))
> > #define alwaysinline inline __attribute__((always_inline))
> > ```
>=20
> Are they similar?
>=20
> $ 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

Yes, for x86 (with `CONFIG_X86_32`) they are.

```
$ git grep asmlinkage | grep regparm
arch/x86/include/asm/linkage.h:#def
ine asmlinkage CPP_ASMLINKAGE __attribute__((regparm(0)))
$ nl -ba arch/x86/include/asm/linkage.h | head -11
     1	#ifndef _ASM_X86_LINKAGE_H
     2	#define _ASM_X86_LINKAGE_H
     3=09
     4	#include <linux/stringify.h>
     5=09
     6	#undef notrace
     7	#define notrace __attribute__((no_instrument_function))
     8=09
     9	#ifdef CONFIG_X86_32
    10	#define asmlinkage CPP_ASMLINKAGE __attribute__((regparm(0)))
    11	#endif /* CONFIG_X86_32 */
```

> I believe asmlinkage is defined just to avoid
> possible asm/c++ symbol decorations.
>=20
> > In Linux, commit 9c0ca6f9=C2=A0(update checkpatch.pl to version 0.10) s=
eems
> > to have introduced the check. The commit message contains =E2=80=9Casml=
inkage
> > is also a storage type=E2=80=9D.
> >=20
> > Furthermore, `checkpatch.pl` doesn=E2=80=99t seem to warn about the cod=
e below.
> >=20
> > ```
> > void __attribute__((weak)) mainboard_suspend_resume(void)
> > ```
> >=20
> > This raises the question below.
> >=20
> > > 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.
> >=20
> > Is there another reason, besides not having that implemented?
> >=20
> > 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
--=-Vn+nrgGzHfAjCELyqcfj
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: This is a digitally signed message part
Content-Transfer-Encoding: 7bit

-----BEGIN PGP SIGNATURE-----

iF0EABECAB0WIQQ8+w9d414FAVARIpk9fVorbA4dWAUCWM5OSAAKCRA9fVorbA4d
WKSSAJ46B5ByLCZKtmTjz2n/Fe9LNBmrlQCdHlm2kNAEcO2QBIihd8y86R9TZbw=
=3Pua
-----END PGP SIGNATURE-----

--=-Vn+nrgGzHfAjCELyqcfj--