Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1446923

Re: [PATCH] mtd: spi-nor: don't build Cadence QuadSPI on non-ARM

Show all headers | View raw

On Wed, Jul 20, 2016 at 04:58:08AM +0200, Marek Vasut wrote:
> On 07/20/2016 04:50 AM, Brian Norris wrote:
> > On Wed, Jul 20, 2016 at 03:50:27AM +0200, Marek Vasut wrote:
> >> On 07/19/2016 10:05 PM, Brian Norris wrote:
> >>> On Tue, Jul 19, 2016 at 08:03:00AM +0200, Stefan Roese wrote:
> >>>> On 18.07.2016 22:20, Brian Norris wrote:
> >>>>> Hmm, does x86 not define readsl()/writesl()? I can never tell what
> >>>>> accessors are supposed to be "standard" across architectures.
> >>>>>
> >>>>> Either we need to drop the COMPILE_TEST or maybe make it (!X86 &&
> >>>>> COMPILE_TEST).
> >>>>
> >>>> iowrite32_rep() etc should work for x86 as well.
> >>>
> >>> Looks like it might. I'm not sure the original submitter can retest
> >>> right now (travel), so I'd probably rather just take the easy fix for
> >>> now, and we can widen to COMPILE_TEST later if desired.
> >>
> >> Isn't there a generic readsl() and writesl() implementation in
> >> include/asm-generic/io.h ?
> > 
> > Yes, but somehow x86 has managed to avoid that. I guess it's optional
> > for arch/<foo>/include/asm/io.h to include <asm-generic/io.h>? At any
> > rate, I double-checked myself by adding '#error "blah"' to
> > include/asm-generic/io.h, and x86 still seemed to build fine (at least
> > for the modules I was checking, like cadence-quadspi.o).
> 
> Yep, I just checked the same and it's not included from
> arch/x86/include/asm/io.h for whatever reason. Maybe this needs to be
> fixed on x86 level?

Maybe. That's why I added the x86 maintainers. Maybe they'd respond
better^Wmore loudly if I just sent a patch to do that :)

But seriously, doing the above really breaks things, even if I stick the
include at the end of asm/io.h. There's plenty of stuff that the
asm-generic version includes based on #ifndef some_accessor, except x86
uses a static inline for their definition. So it seems it's not trivial
to get an architecture to fall back gracefully to asm-generic; you have
to put in some work. It also may not be all that desirable to have some
allegedly generic version generate something that may not be safe on a
given architecture (and I don't purport to understand the x86 memory
model).

Additionally, it looks like asm-generic/io.h is actually only included
in 14 of 33 arch'es, so it seems like that's really not a designated
goal. It does make it awfully difficult to figure out what I/O accessors
are *actually* portable though...

Brian

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: [mtd-next:master 30/33]  drivers/mtd/spi-nor/cadence-quadspi.c:529:4: error: implicit declaration of  function 'readsl' Brian Norris <computersforpeace@gmail.com> - 2016-07-18 22:30 +0200
  Re: [mtd-next:master 30/33]  drivers/mtd/spi-nor/cadence-quadspi.c:529:4: error: implicit declaration of  function 'readsl' Stefan Roese <sr@denx.de> - 2016-07-19 08:10 +0200
    [PATCH] mtd: spi-nor: don't build Cadence QuadSPI on non-ARM Brian Norris <computersforpeace@gmail.com> - 2016-07-19 22:10 +0200
      Re: [PATCH] mtd: spi-nor: don't build Cadence QuadSPI on non-ARM Marek Vasut <marex@denx.de> - 2016-07-20 04:00 +0200
        Re: [PATCH] mtd: spi-nor: don't build Cadence QuadSPI on non-ARM Marek Vasut <marex@denx.de> - 2016-07-20 05:00 +0200
          Re: [PATCH] mtd: spi-nor: don't build Cadence QuadSPI on non-ARM Brian Norris <computersforpeace@gmail.com> - 2016-07-20 05:30 +0200
            Re: [PATCH] mtd: spi-nor: don't build Cadence QuadSPI on non-ARM Marek Vasut <marex@denx.de> - 2016-07-20 08:10 +0200
        Re: [PATCH] mtd: spi-nor: don't build Cadence QuadSPI on non-ARM Brian Norris <computersforpeace@gmail.com> - 2016-07-20 05:00 +0200

csiph-web