RFC: arch: shall we have generic readl_be()/writel_be()/... or in_be32()/out_be32() ?

RFC: arch: shall we have generic readl_be()/writel_be()/... or in_be32()/out_be32() ?

[Enrico Weigelt, metux IT consult]

Hello folks,


while trying to make some more drivers compile-test'able, i've
discovered some arch specific calls in here, eg.:


In file included from
/home/nekrad/src/apu2-dev/pkg/kernel.apu2.git/drivers/usb/host/ehci-spear.c:23:
/home/nekrad/src/apu2-dev/pkg/kernel.apu2.git/drivers/usb/host/ehci.h:
In function 'ehci_readl':
/home/nekrad/src/apu2-dev/pkg/kernel.apu2.git/drivers/usb/host/ehci.h:743:3:
error: implicit declaration of function 'readl_be'; did you mean
'readsb'? [-Werror=implicit-function-declaration]
  743 |   readl_be(regs) :
      |   ^~~~~~~~
      |   readsb
/home/nekrad/src/apu2-dev/pkg/kernel.apu2.git/drivers/usb/host/ehci.h:
In function 'ehci_writel':
/home/nekrad/src/apu2-dev/pkg/kernel.apu2.git/drivers/usb/host/ehci.h:767:3:
error: implicit declaration of function 'writel_be'; did you mean
'writesb'? [-Werror=implicit-function-declaration]
  767 |   writel_be(val, regs) :
      |   ^~~~~~~~~
      |   writesb
In file included from
/home/nekrad/src/apu2-dev/pkg/kernel.apu2.git/drivers/usb/host/ehci-hcd.c:97:
/home/nekrad/src/apu2-dev/pkg/kernel.apu2.git/drivers/usb/host/ehci.h:
In function 'ehci_readl':
/home/nekrad/src/apu2-dev/pkg/kernel.apu2.git/drivers/usb/host/ehci.h:743:3:
error: implicit declaration of function 'readl_be'; did you mean
'readsb'? [-Werror=implicit-function-declaration]
  743 |   readl_be(regs) :
      |   ^~~~~~~~
      |   readsb
/home/nekrad/src/apu2-dev/pkg/kernel.apu2.git/drivers/usb/host/ehci.h:
In function 'ehci_writel':
/home/nekrad/src/apu2-dev/pkg/kernel.apu2.git/drivers/usb/host/ehci.h:767:3:
error: implicit declaration of function 'writel_be'; did you mean
'writesb'? [-Werror=implicit-function-declaration]
  767 |   writel_be(val, regs) :
      |   ^~~~~~~~~
      |   writesb


It seems that only few archs (microblaze, ppc, sparc) define them.

Also drivers/usb/host/ehci.h defines them, but only for one particular
arch/subarch.

IIRC, these funcs are for accessing hw registers that are in BEs, so
BE cpus can do direct access, while LE cpus need to do a conversion.

OTOH, we also have in_be32() / out_be32. They seem to do quite the same
thing, referenced much more often, but also just defined on a few archs.


I believe we should have generic functions, that all archs implement
(possibly doing automatic conversion, if necessary), which are used
by everybody else.

What's your oppionion on that ?


thanks,

--mtx

-- 
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

Re: RFC: arch: shall we have generic readl_be()/writel_be()/... or in_be32()/out_be32() ?

[Alan Stern]

On Wed, Dec 09, 2020 at 12:08:51PM +0100, Enrico Weigelt, metux IT consult wrote:
> Hello folks,
> 
> 
> while trying to make some more drivers compile-test'able, i've
> discovered some arch specific calls in here, eg.:
> 
> 
> In file included from
> /home/nekrad/src/apu2-dev/pkg/kernel.apu2.git/drivers/usb/host/ehci-spear.c:23:
> /home/nekrad/src/apu2-dev/pkg/kernel.apu2.git/drivers/usb/host/ehci.h:
> In function 'ehci_readl':
> /home/nekrad/src/apu2-dev/pkg/kernel.apu2.git/drivers/usb/host/ehci.h:743:3:
> error: implicit declaration of function 'readl_be'; did you mean
> 'readsb'? [-Werror=implicit-function-declaration]
>   743 |   readl_be(regs) :
>       |   ^~~~~~~~
>       |   readsb
> /home/nekrad/src/apu2-dev/pkg/kernel.apu2.git/drivers/usb/host/ehci.h:
> In function 'ehci_writel':
> /home/nekrad/src/apu2-dev/pkg/kernel.apu2.git/drivers/usb/host/ehci.h:767:3:
> error: implicit declaration of function 'writel_be'; did you mean
> 'writesb'? [-Werror=implicit-function-declaration]
>   767 |   writel_be(val, regs) :
>       |   ^~~~~~~~~
>       |   writesb
> In file included from
> /home/nekrad/src/apu2-dev/pkg/kernel.apu2.git/drivers/usb/host/ehci-hcd.c:97:
> /home/nekrad/src/apu2-dev/pkg/kernel.apu2.git/drivers/usb/host/ehci.h:
> In function 'ehci_readl':
> /home/nekrad/src/apu2-dev/pkg/kernel.apu2.git/drivers/usb/host/ehci.h:743:3:
> error: implicit declaration of function 'readl_be'; did you mean
> 'readsb'? [-Werror=implicit-function-declaration]
>   743 |   readl_be(regs) :
>       |   ^~~~~~~~
>       |   readsb
> /home/nekrad/src/apu2-dev/pkg/kernel.apu2.git/drivers/usb/host/ehci.h:
> In function 'ehci_writel':
> /home/nekrad/src/apu2-dev/pkg/kernel.apu2.git/drivers/usb/host/ehci.h:767:3:
> error: implicit declaration of function 'writel_be'; did you mean
> 'writesb'? [-Werror=implicit-function-declaration]
>   767 |   writel_be(val, regs) :
>       |   ^~~~~~~~~
>       |   writesb
> 
> 
> It seems that only few archs (microblaze, ppc, sparc) define them.
> 
> Also drivers/usb/host/ehci.h defines them, but only for one particular
> arch/subarch.
> 
> IIRC, these funcs are for accessing hw registers that are in BEs, so
> BE cpus can do direct access, while LE cpus need to do a conversion.
> 
> OTOH, we also have in_be32() / out_be32. They seem to do quite the same
> thing, referenced much more often, but also just defined on a few archs.
> 
> 
> I believe we should have generic functions, that all archs implement
> (possibly doing automatic conversion, if necessary), which are used
> by everybody else.
> 
> What's your oppionion on that ?

It certainly seems reasonable.  Another possibility, less stringent, is 
to require that definitions exist on all architectures that can have 
big-endian MMIO (or port-based IO).  For example, any architecture 
which might select CONFIG_EHCI_BIG_ENDIAN_MMIO, as used in ehci.h.

Otherwise we're left in the unfortunate position of having to provide 
definitions for these functions, but _only_ on architectures that don't 
already make their own definitions -- basically an impossible task.

Alan Stern

Re: RFC: arch: shall we have generic readl_be()/writel_be()/... or in_be32()/out_be32() ?

[Maciej W. Rozycki]

On Wed, 9 Dec 2020, Alan Stern wrote:

> > I believe we should have generic functions, that all archs implement
> > (possibly doing automatic conversion, if necessary), which are used
> > by everybody else.
> > 
> > What's your oppionion on that ?
> 
> It certainly seems reasonable.  Another possibility, less stringent, is 
> to require that definitions exist on all architectures that can have 
> big-endian MMIO (or port-based IO).  For example, any architecture 
> which might select CONFIG_EHCI_BIG_ENDIAN_MMIO, as used in ehci.h.

 Lane swapping is a complex matter where there is an endianness mismatch 
between buses.  A bus bridge may implement the byte lane match policy or 
the bit lane match policy, or even both to choose from, perhaps on a 
case-by-case basis for individual accesses (e.g. with a pair of address 
windows decoded to the other bus according to a different policy each; I 
actually have such a system).

 Consequently not only data transferred may have to be transformed, but so 
may have the address used.  Also the transformation will be different 
depending on whether data accessed is to be interpreted numerically (where 
the bit lane match policy is more suitable) such as with CSR access, or as 
a byte stream (where the byte lane match policy is) such as with PIO data 
moves.

 See arch/mips/include/asm/io.h and arch/mips/include/asm/*/mangle-port.h 
for an example where we take care of different cases.

 Building infrastructure for doing this all in a generic manner would I 
think be a good idea, but then a major effort as well, and you'd have to 
coordinate it with all the arch maintainers.

  Maciej