2017-04-01 22:23:00

by Samuel Ortiz

[permalink] [raw]
Subject: Re: [PATCH v2] nfc: don't be making arch specific unaligned decisions at driver level.

Hi Paul,

On Mon, Jan 09, 2017 at 12:52:22PM -0500, Paul Gortmaker wrote:
> Currently ia64 fails building allmodconfig with variations of:
>
> In file included from drivers/nfc/nxp-nci/i2c.c:39:0:
> ./include/linux/unaligned/access_ok.h:62:29: error: redefinition of ‘put_unaligned_be64’
> static __always_inline void put_unaligned_be64(u64 val, void *p)
> ^~~~~~~~~~~~~~~~~~
> In file included from ./arch/ia64/include/asm/unaligned.h:5:0,
> from ./arch/ia64/include/asm/io.h:22,
> from ./arch/ia64/include/asm/smp.h:20,
> from ./include/linux/smp.h:59,
> from ./include/linux/topology.h:33,
> from ./include/linux/gfp.h:8,
> from ./include/linux/slab.h:14,
> from ./include/linux/resource_ext.h:19,
> from ./include/linux/acpi.h:26,
> from drivers/nfc/nxp-nci/i2c.c:28:
> ./include/linux/unaligned/be_byteshift.h:65:20: note: previous definition of ‘put_unaligned_be64’ was here
> static inline void put_unaligned_be64(u64 val, void *p)
> ^~~~~~~~~~~~~~~~~~
> scripts/Makefile.build:293: recipe for target 'drivers/nfc/nxp-nci/i2c.o' failed
>
> The easiest explanation for this is to look at the non-arch users in
> the following output:
>
> linux$git grep include.*access_ok.h
> arch/arm64/crypto/crc32-arm64.c:#include <linux/unaligned/access_ok.h>
> arch/cris/include/asm/unaligned.h:#include <linux/unaligned/access_ok.h>
> arch/m68k/include/asm/unaligned.h:#include <linux/unaligned/access_ok.h>
> arch/mn10300/include/asm/unaligned.h:#include <linux/unaligned/access_ok.h>
> arch/powerpc/include/asm/unaligned.h:#include <linux/unaligned/access_ok.h>
> arch/s390/include/asm/unaligned.h:#include <linux/unaligned/access_ok.h>
> arch/x86/include/asm/unaligned.h:#include <linux/unaligned/access_ok.h>
> drivers/nfc/nfcmrvl/fw_dnld.c:#include <linux/unaligned/access_ok.h>
> drivers/nfc/nxp-nci/firmware.c:#include <linux/unaligned/access_ok.h>
> drivers/nfc/nxp-nci/i2c.c:#include <linux/unaligned/access_ok.h>
> include/asm-generic/unaligned.h:# include <linux/unaligned/access_ok.h>
>
> Note that nfc is essentially the only non-arch user in the above.
> When it forces use of access_ok.h, it will break any arch that has
> already selected be_byteshift.h (or other conflicting implementations)
> at the arch level.
>
> The decision of what variant for unaligned access to use needs to be
> left to the arch level and not used at the driver level. Since not
> all arch will have sourced asm/unaligned.h already, we need to call
> it out and then the arch can give us just the one definition that
> is needed.
>
> See commit 064106a91be5 ("kernel: add common infrastructure for
> unaligned access") as a reference.
>
> Cc: Lauro Ramos Venancio <[email protected]>
> Cc: Aloisio Almeida Jr <[email protected]>
> Cc: Samuel Ortiz <[email protected]>
> Cc: Tony Luck <[email protected]>
> Cc: Fenghua Yu <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Paul Gortmaker <[email protected]>
> ---
>
> [v2: explicitly include asm/uaccess.h since some arch won't be
> getting any variant of an unaligned access header without it.
> Build test allmodconfig on x86-64, i386, arm64, ia64. ]
>
> drivers/nfc/nfcmrvl/fw_dnld.c | 2 +-
> drivers/nfc/nxp-nci/firmware.c | 2 +-
> drivers/nfc/nxp-nci/i2c.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
This build issue is now fixed in nfc-next with a couple of different
patches.

Cheers,
Samuel.


2017-04-03 17:27:09

by Paul Gortmaker

[permalink] [raw]
Subject: Re: [PATCH v2] nfc: don't be making arch specific unaligned decisions at driver level.

[Re: [PATCH v2] nfc: don't be making arch specific unaligned decisions at driver level.] On 02/04/2017 (Sun 00:22) Samuel Ortiz wrote:

> Hi Paul,
>
> On Mon, Jan 09, 2017 at 12:52:22PM -0500, Paul Gortmaker wrote:
> > Currently ia64 fails building allmodconfig with variations of:
> >

[...]

> >
> > drivers/nfc/nfcmrvl/fw_dnld.c | 2 +-
> > drivers/nfc/nxp-nci/firmware.c | 2 +-
> > drivers/nfc/nxp-nci/i2c.c | 2 +-
> > 3 files changed, 3 insertions(+), 3 deletions(-)
> This build issue is now fixed in nfc-next with a couple of different
> patches.

OK, I'll remove it from my local queue and retest and let you know if I
see any issues still.

Thanks,
Paul.
--

>
> Cheers,
> Samuel.

2017-04-04 01:55:17

by Paul Gortmaker

[permalink] [raw]
Subject: Re: [PATCH v2] nfc: don't be making arch specific unaligned decisions at driver level.

[Re: [PATCH v2] nfc: don't be making arch specific unaligned decisions at driver level.] On 02/04/2017 (Sun 00:22) Samuel Ortiz wrote:

> Hi Paul,
>
> On Mon, Jan 09, 2017 at 12:52:22PM -0500, Paul Gortmaker wrote:
> > Currently ia64 fails building allmodconfig with variations of:
> >
> > In file included from drivers/nfc/nxp-nci/i2c.c:39:0:
> > ./include/linux/unaligned/access_ok.h:62:29: error: redefinition of ‘put_unaligned_be64’

[...]

> >
> > The decision of what variant for unaligned access to use needs to be
> > left to the arch level and not used at the driver level. Since not
> > all arch will have sourced asm/unaligned.h already, we need to call
> > it out and then the arch can give us just the one definition that
> > is needed.
> >
> > See commit 064106a91be5 ("kernel: add common infrastructure for
> > unaligned access") as a reference.
> >

[...]

> This build issue is now fixed in nfc-next with a couple of different
> patches.

I added nfc-next as a remote and saw you pulled similar commits out of
the NFC backlog just recently:

commit 2eee74b7e2a496dea49847c36fd09320505f45b7
Author: Guenter Roeck <[email protected]>
AuthorDate: Sat Aug 1 06:59:29 2015 -0700
Commit: Samuel Ortiz <[email protected]>
CommitDate: Sat Apr 1 23:52:25 2017 +0200

NFC: nxp-nci: Include unaligned.h instead of access_ok.h

...and...

commit d916d923724d59cde99ee588f15eec59dd863bbd
Author: Tobias Klauser <[email protected]>
AuthorDate: Wed Oct 26 11:00:12 2016 +0200
Commit: Samuel Ortiz <[email protected]>
CommitDate: Sat Apr 1 23:45:31 2017 +0200

NFC: nfcmrvl: Include unaligned.h instead of access_ok.h

Thanks for finding those in the backlog and folding them in; presumably
the backlog was related to the MAINTAINERS change on the same branch.

Anyway, they appear to be the same as the change I proposed, and I build
tested nfc-next on ia64 and x86-64 without issue. So it looks good.

One thing I did notice, is that nfc-next doesn't appear to be merged
into the linux-next trees, which explains why I didn't see a conflict
with my patch against your two new commits. Given the recent maintainer
change you added, it seems your nfc-next is now the unconditional source
for NFC changes. So maybe you want to ask Stephen (Cc:'d) to add your
tree to the daily linux-next coverage?

All he needs is the repo and branch, which I assume are:

git://git.kernel.org/pub/scm/linux/kernel/git/sameo/nfc-next.git #master

Well, that and the request for it really should come from you... :)

Thanks,
Paul.
--

>
> Cheers,
> Samuel.