lkml.org 
[lkml]   [2019]   [Jun]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH] mips: Remove q-accessors from non-64bit platforms
    On Thu, Jun 20, 2019 at 8:19 PM Maciej W. Rozycki <macro@linux-mips.org> wrote:
    >
    > On Thu, 20 Jun 2019, Paul Burton wrote:
    >
    > > So this seems pretty reasonable. Build testing all our defconfigs only
    > > showed up one issue for decstation_defconfig & decstation_r4k_defconfig:
    > >
    > > drivers/net/fddi/defza.c: In function 'fza_reads':
    > > drivers/net/fddi/defza.c:88:17: error: implicit declaration of
    > > function 'readq_relaxed'; did you mean 'readw_relaxed'?
    > > [-Werror=implicit-function-declaration]
    > > #define readq_u readq_relaxed
    > > ^~~~~~~~~~~~~
    > > drivers/net/fddi/defza.c:126:13: note: in expansion of macro 'readq_u'
    > > *dst++ = readq_u(src++);
    > > ^~~~~~~
    > > drivers/net/fddi/defza.c: In function 'fza_writes':
    > > drivers/net/fddi/defza.c:92:18: error: implicit declaration of
    > > function 'writeq_relaxed'; did you mean 'writel_relaxed'?
    > > [-Werror=implicit-function-declaration]
    > > #define writeq_u writeq_relaxed
    > > ^~~~~~~~~~~~~~
    > > drivers/net/fddi/defza.c:151:4: note: in expansion of macro 'writeq_u'
    > > writeq_u(*src++, dst++);
    > > ^~~~~~~~
    > > CC net/core/scm.o
    > > cc1: some warnings being treated as errors
    > > make[4]: *** [scripts/Makefile.build:279: drivers/net/fddi/defza.o] Error 1
    > >
    > > These uses of readq_relaxed & writeq_relaxed are both conditional upon
    > > sizeof(unsigned long) == 8, ie. upon CONFIG_64BIT=y so they're not going
    > > to present a runtime issue but we need to provide some implementation of
    > > the *q accessors to keep the compiler happy.
    > >
    > > I see a few options:
    > >
    > > 1) We could just have defza.c include <io-64-nonatomic-lo-hi.h> to get
    > > the appropriate declarations, which should then get optimized away by
    > > the compiler anyway & never actually be used.
    >
    > This, definitely.

    The compiler should generally not be allowed to combine two adjacent
    readl_relaxed() back into a 64-bit load. Only __raw_readl() can be
    combined or split up. If the mips version of the *_relaxed() accessors
    allows the compiler to do this, I would consider that a bug.

    > > 2) We could have defza.h #define its readq_u & writeq_u macros
    > > differently for CONFIG_32BIT=y kernels, perhaps using
    > > __compiletime_error to catch any bogus use of them.
    > >
    > > 3) We could do the same in a generic header, though if nobody else has
    > > needed it so far & this is the only place we need it then maybe it's
    > > not worth it.
    > >
    > > So I'm thinking option 2 might be best, as below. Having said that I
    > > don't mind option 1 either - it's simple. Maciej do you have any
    > > preference?
    >
    > The use of 64-bit operations to access option's packet memory, which is
    > true SRAM, i.e. no side effects, is to improve throughput only and there's
    > no need for atomicity here nor also any kind of barriers, except at the
    > conclusion. Splitting 64-bit accesses into 32-bit halves in software
    > would not be a functional error here.

    The other property of packet memory and similar things is that you
    basically want memcpy()-behavior with no byteswaps. This is one
    of the few cases in which __raw_readq() is actually the right accessor
    in (mostly) portable code.

    Arnd

    \
     
     \ /
      Last update: 2019-06-21 11:27    [W:3.722 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site