On Tue, 2024-10-08 at 09:50 +0200, Julian Vetter wrote:
> Add wrapper functions around zpci_memcpy_{from,to}io and zpci_memset_io,
> which have aligned prototypes with the ones from iomap_copy.c. These
> wrappers are necessary because the prototypes of the zpci_ functions
> can't be changed. In s390 arch code they are directly being called and
> the return value is checked, At the same time they serve as generic
> memcpy_{from,to}io and memset_io functions, without a return value.
>
> Reviewed-by: Yann Sionneau <ysionneau@xxxxxxxxxxxxx>
> Signed-off-by: Julian Vetter <jvetter@xxxxxxxxxxxxx>
> ---
> Changes for v8:
> - New patch
> ---
> arch/s390/include/asm/io.h | 27 +++++++++++++++++++++++----
> arch/s390/include/asm/pci_io.h | 6 +++---
> 2 files changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/arch/s390/include/asm/io.h b/arch/s390/include/asm/io.h
> index 0fbc992d7a5e..f3ef6d4130b3 100644
> --- a/arch/s390/include/asm/io.h
> +++ b/arch/s390/include/asm/io.h
> @@ -58,10 +58,6 @@ static inline void ioport_unmap(void __iomem *p)
> #define pci_iomap_wc pci_iomap_wc
> #define pci_iomap_wc_range pci_iomap_wc_range
>
> -#define memcpy_fromio(dst, src, count) zpci_memcpy_fromio(dst, src, count)
> -#define memcpy_toio(dst, src, count) zpci_memcpy_toio(dst, src, count)
> -#define memset_io(dst, val, count) zpci_memset_io(dst, val, count)
> -
> #define mmiowb() zpci_barrier()
>
> #define __raw_readb zpci_read_u8
> @@ -73,6 +69,10 @@ static inline void ioport_unmap(void __iomem *p)
> #define __raw_writel zpci_write_u32
> #define __raw_writeq zpci_write_u64
>
> +#define memcpy_fromio memcpy_fromio
> +#define memcpy_toio memcpy_toio
> +#define memset_io memset_io
> +
> /* combine single writes by using store-block insn */
> static inline void __iowrite32_copy(void __iomem *to, const void *from,
> size_t count)
> @@ -88,6 +88,25 @@ static inline void __iowrite64_copy(void __iomem *to, const void *from,
> }
> #define __iowrite64_copy __iowrite64_copy
>
> +static inline void memcpy_fromio(void *dst, const volatile void __iomem *src,
> + size_t n)
> +{
> + zpci_memcpy_fromio(dst, src, n);
> +}
> +
> +static inline void memcpy_toio(volatile void __iomem *dst,
> + const void *src, size_t n)
> +{
> + zpci_memcpy_toio(dst, src, n);
> +}
> +
> +static inline void memset_io(volatile void __iomem *dst,
> + int val, size_t count)
> +{
> + zpci_memset_io(dst, val, count);
> +}
> +
> +
> #endif /* CONFIG_PCI */
>
> #include <asm-generic/io.h>
> diff --git a/arch/s390/include/asm/pci_io.h b/arch/s390/include/asm/pci_io.h
> index 2686bee800e3..43a5ea4ee20f 100644
> --- a/arch/s390/include/asm/pci_io.h
> +++ b/arch/s390/include/asm/pci_io.h
> @@ -143,7 +143,7 @@ static inline int zpci_get_max_io_size(u64 src, u64 dst, int len, int max)
>
> static inline int zpci_memcpy_fromio(void *dst,
> const volatile void __iomem *src,
> - unsigned long n)
> + size_t n)
> {
> int size, rc = 0;
>
> @@ -162,7 +162,7 @@ static inline int zpci_memcpy_fromio(void *dst,
> }
>
> static inline int zpci_memcpy_toio(volatile void __iomem *dst,
> - const void *src, unsigned long n)
> + const void *src, size_t n)
> {
> int size, rc = 0;
>
> @@ -187,7 +187,7 @@ static inline int zpci_memcpy_toio(volatile void __iomem *dst,
> }
>
> static inline int zpci_memset_io(volatile void __iomem *dst,
> - unsigned char val, size_t count)
> + int val, size_t count)
> {
> u8 *src = kmalloc(count, GFP_KERNEL);
> int rc;
The code makes sense to me. If I understand Arnd's comment correctly
the wrappers may not be necessary if you don't have the global
prototypes though, right? In this case I think we might want to still
align the parameter types as this seems generally cleaner and the
internal callers already use size_t anyway.
Either way I gave the whole series a quick test on s390x with PCI
devices and all looks good. So feel free to add:
Acked-by: Niklas Schnelle <schnelle@xxxxxxxxxxxxx>
Tested-by: Niklas Schnelle <schnelle@xxxxxxxxxxxxx>
