[PATCH] sparc64: Fix the generic IO helpers

[PATCH] sparc64: Fix the generic IO helpers

[Linus Walleij]

This enables the Sparc to use <asm-generic/io.h> to fill in the
missing (undefined) [read|write]sq I/O accessor functions.

This is needed if Sparc[64] ever wants to uses CONFIG_REGMAP_MMIO
which has been patches to use accelerated _noinc accessors
such as readsq/writesq that Sparc64, while being a 64bit platform,
as of now not yet provide.

This comes with the requirement that everything the architecture
already provides needs to be defined, rather than just being,
say, static inline functions.

Bite the bullet and just provide the definitions and make it work.
Compile-tested on sparc64.

Reported-by: kernel test robot <lkp@intel.com>
Link: https://lore.kernel.org/linux-arm-kernel/202208201639.HXye3ke4-lkp@intel.com/
Cc: David S. Miller <davem@davemloft.net>
Cc: sparclinux@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: Mark Brown <broonie@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/sparc/include/asm/io.h | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/sparc/include/asm/io.h b/arch/sparc/include/asm/io.h
index 2eefa526b38f..88da27165c01 100644
--- a/arch/sparc/include/asm/io.h
+++ b/arch/sparc/include/asm/io.h
@@ -19,4 +19,35 @@
 #define writel_be(__w, __addr)	__raw_writel(__w, __addr)
 #define writew_be(__l, __addr)	__raw_writew(__l, __addr)
 
+/*
+ * These defines are necessary to use the generic io.h for filling in
+ * the missing parts of the API contract. This is because the platform
+ * uses (inline) functions rather than defines and the generic helper
+ * fills in the undefined.
+ */
+/* These are static inlines on 64BIT only */
+#if defined(__sparc__) && defined(__arch64__)
+#define memset_io memset_io
+#define memcpy_fromio memcpy_fromio
+#define memcpy_toio memcpy_toio
+#endif
+#define pci_iomap pci_iomap
+#define pci_iounmap pci_iounmap
+#define ioremap_np ioremap_np
+#define ioport_map ioport_map
+#define ioport_unmap ioport_unmap
+#define readsb readsb
+#define readsw readsw
+#define readsl readsl
+#define writesb writesb
+#define writesw writesw
+#define writesl writesl
+#define insb insb
+#define insw insw
+#define insl insl
+#define outsb outsb
+#define outsw outsw
+#define outsl outsl
+#include <asm-generic/io.h>
+
 #endif
-- 
2.37.2

Re: [PATCH] sparc64: Fix the generic IO helpers

[Randy Dunlap]

Hi--

On 8/23/22 01:50, Linus Walleij wrote:
> This enables the Sparc to use <asm-generic/io.h> to fill in the
> missing (undefined) [read|write]sq I/O accessor functions.
> 
> This is needed if Sparc[64] ever wants to uses CONFIG_REGMAP_MMIO
> which has been patches to use accelerated _noinc accessors
> such as readsq/writesq that Sparc64, while being a 64bit platform,
> as of now not yet provide.
> 
> This comes with the requirement that everything the architecture
> already provides needs to be defined, rather than just being,
> say, static inline functions.
> 
> Bite the bullet and just provide the definitions and make it work.
> Compile-tested on sparc64.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Link: https://lore.kernel.org/linux-arm-kernel/202208201639.HXye3ke4-lkp@intel.com/
> Cc: David S. Miller <davem@davemloft.net>
> Cc: sparclinux@vger.kernel.org
> Cc: linux-arch@vger.kernel.org
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Both alpha & parisc (32 and 64 bits) need this fix also.

Is it always safe to do this?

> ---
>  arch/sparc/include/asm/io.h | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/arch/sparc/include/asm/io.h b/arch/sparc/include/asm/io.h
> index 2eefa526b38f..88da27165c01 100644
> --- a/arch/sparc/include/asm/io.h
> +++ b/arch/sparc/include/asm/io.h
> @@ -19,4 +19,35 @@
>  #define writel_be(__w, __addr)	__raw_writel(__w, __addr)
>  #define writew_be(__l, __addr)	__raw_writew(__l, __addr)
>  
> +/*
> + * These defines are necessary to use the generic io.h for filling in
> + * the missing parts of the API contract. This is because the platform
> + * uses (inline) functions rather than defines and the generic helper
> + * fills in the undefined.
> + */
> +/* These are static inlines on 64BIT only */
> +#if defined(__sparc__) && defined(__arch64__)
> +#define memset_io memset_io
> +#define memcpy_fromio memcpy_fromio
> +#define memcpy_toio memcpy_toio
> +#endif
> +#define pci_iomap pci_iomap
> +#define pci_iounmap pci_iounmap
> +#define ioremap_np ioremap_np
> +#define ioport_map ioport_map
> +#define ioport_unmap ioport_unmap
> +#define readsb readsb
> +#define readsw readsw
> +#define readsl readsl
> +#define writesb writesb
> +#define writesw writesw
> +#define writesl writesl
> +#define insb insb
> +#define insw insw
> +#define insl insl
> +#define outsb outsb
> +#define outsw outsw
> +#define outsl outsl
> +#include <asm-generic/io.h>
> +
>  #endif

-- 
~Randy

Re: [PATCH] sparc64: Fix the generic IO helpers

[Linus Walleij]

On Thu, Aug 25, 2022 at 12:15 AM Randy Dunlap <rdunlap@infradead.org> wrote:

> Both alpha

https://lore.kernel.org/linux-arch/20220818092059.103884-1-linus.walleij@linaro.org/

>  & parisc (32 and 64 bits) need this fix also.

Yeah I'll try to send something for them too. Also fixed Hexagon.
The problem is installing all the cross compilers just to test this
stuff.... It's a lot of work.

> Is it always safe to do this?

I think the arch part is fine, i.e. I don't think it will fix something
broken.

Then it is the matter of testing the new accessors.

That is done by a usecase, in this case, a usecase using
a fixed address in 64bit registers write under regmap MMIO.

Not sure someone will ever have a usecase for that to test
with. The people making use of it need to test it when they
use it.

As mentioned in the alpha patch, the alternative is to leave
regmap-mmio broken for the arch, until someone uses is.
It feels wrong to develop generic code for all archs that cannot
even compile on all of them, so I prefer to fix it to the point
of compiling at least. The generic helpers try their best to
be, well generic helpers.

Yours,
Linus Walleij

Re: [PATCH] sparc64: Fix the generic IO helpers

[Sam Ravnborg]

Hi Linus,

On Tue, Aug 23, 2022 at 10:50:14AM +0200, Linus Walleij wrote:
> This enables the Sparc to use <asm-generic/io.h> to fill in the
> missing (undefined) [read|write]sq I/O accessor functions.
> 
> This is needed if Sparc[64] ever wants to uses CONFIG_REGMAP_MMIO
> which has been patches to use accelerated _noinc accessors
> such as readsq/writesq that Sparc64, while being a 64bit platform,
> as of now not yet provide.
> 
> This comes with the requirement that everything the architecture
> already provides needs to be defined, rather than just being,
> say, static inline functions.
> 
> Bite the bullet and just provide the definitions and make it work.
> Compile-tested on sparc64.
This file is shared with sparc32 - so you need to build test it here as
well.

> 
> Reported-by: kernel test robot <lkp@intel.com>
> Link: https://lore.kernel.org/linux-arm-kernel/202208201639.HXye3ke4-lkp@intel.com/
> Cc: David S. Miller <davem@davemloft.net>
> Cc: sparclinux@vger.kernel.org
> Cc: linux-arch@vger.kernel.org
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  arch/sparc/include/asm/io.h | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/arch/sparc/include/asm/io.h b/arch/sparc/include/asm/io.h
> index 2eefa526b38f..88da27165c01 100644
> --- a/arch/sparc/include/asm/io.h
> +++ b/arch/sparc/include/asm/io.h
> @@ -19,4 +19,35 @@
>  #define writel_be(__w, __addr)	__raw_writel(__w, __addr)
>  #define writew_be(__l, __addr)	__raw_writew(__l, __addr)
>  
> +/*
> + * These defines are necessary to use the generic io.h for filling in
> + * the missing parts of the API contract. This is because the platform
> + * uses (inline) functions rather than defines and the generic helper
> + * fills in the undefined.
> + */
> +/* These are static inlines on 64BIT only */
> +#if defined(__sparc__) && defined(__arch64__)
> +#define memset_io memset_io
> +#define memcpy_fromio memcpy_fromio
> +#define memcpy_toio memcpy_toio
> +#endif
> +#define pci_iomap pci_iomap
> +#define pci_iounmap pci_iounmap
> +#define ioremap_np ioremap_np
> +#define ioport_map ioport_map
> +#define ioport_unmap ioport_unmap
> +#define readsb readsb
> +#define readsw readsw
> +#define readsl readsl
> +#define writesb writesb
> +#define writesw writesw
> +#define writesl writesl
> +#define insb insb
> +#define insw insw
> +#define insl insl
> +#define outsb outsb
> +#define outsw outsw
> +#define outsl outsl
> +#include <asm-generic/io.h>

The normal practice is to stick the define near where it makes sense,
and not an ureadable list in the bottom.
Also "readsb" looks wrong as sparc32 do not provice it.
I guess most needs to be pushed to io_64.h as io_32.h already includes
asm-generic/io.h.

I gave up migrating sparc64 to the generiv io.h when I migrated sparc32 -
but I fail to remember why. Most likely lack of motivation.

	Sam