| Message ID | 20241008075023.3052370-1-jvetter@kalrayinc.com |
|---|---|
| Headers | show |
| Series | Consolidate IO memcpy functions | expand |
On Tue, 08 Oct 2024 09:50:22 +0200, Julian Vetter wrote: > > When building for the UM arch and neither INDIRECT_IOMEM=y, nor > HAS_IOMEM=y is selected, the build fails because the memcpy_fromio and > memcpy_toio functions are not defined. Fix it here by depending on > HAS_IOMEM or INDIRECT_IOMEM. > > Reviewed-by: Yann Sionneau <ysionneau@kalrayinc.com> > Signed-off-by: Julian Vetter <jvetter@kalrayinc.com> > --- > Changes for v8: > - New patch > --- > sound/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sound/Kconfig b/sound/Kconfig > index 4c036a9a420a..8b40205394fe 100644 > --- a/sound/Kconfig > +++ b/sound/Kconfig > @@ -1,7 +1,7 @@ > # SPDX-License-Identifier: GPL-2.0-only > menuconfig SOUND > tristate "Sound card support" > - depends on HAS_IOMEM || UML > + depends on HAS_IOMEM || INDIRECT_IOMEM > help > If you have a sound card in your computer, i.e. if it can say more > than an occasional beep, say Y. Acked-by: Takashi Iwai <tiwai@suse.de> thanks, Takashi
Hi Julian, jvetter@kalrayinc.com wrote on Tue, 8 Oct 2024 09:50:21 +0200: > The UM arch doesn't have HAS_IOMEM=y, so the build fails because the > functions memcpy_fromio and memcpy_toio are not defined anymore. These > functions are only build for targets which have HAS_IOMEM=y or > INDIRECT_IOMEM=y. So, depend on either of the two. There are many mtd drivers using memcpy_fromio and memcpy_toio, I'm not sure I get why only this subset of drivers would be impacted? Also, from a general standpoint, I don't see with a good eye the proliferation of the use of || INDIRECT_IOMEM just for the um architecture: $ git grep HAS_IOMEM | wc -l 611 $ git grep INDIRECT_IOMEM | wc -l 15 I believe the Kconfig symbol should adapt to reflect the fact that IO operations are fine, regardless of their type ("direct" or "indirect") rather than move the load on the individual drivers. > Reviewed-by: Yann Sionneau <ysionneau@kalrayinc.com> > Signed-off-by: Julian Vetter <jvetter@kalrayinc.com> > --- > Changes for v8: > - New patch > --- > drivers/mtd/chips/Kconfig | 4 ++++ > drivers/mtd/lpddr/Kconfig | 1 + > 2 files changed, 5 insertions(+) > > diff --git a/drivers/mtd/chips/Kconfig b/drivers/mtd/chips/Kconfig > index 19726ebd973d..78afe7ccf005 100644 > --- a/drivers/mtd/chips/Kconfig > +++ b/drivers/mtd/chips/Kconfig > @@ -4,6 +4,7 @@ menu "RAM/ROM/Flash chip drivers" > > config MTD_CFI > tristate "Detect flash chips by Common Flash Interface (CFI) probe" > + depends on HAS_IOMEM || INDIRECT_IOMEM > select MTD_GEN_PROBE > select MTD_CFI_UTIL > help > @@ -16,6 +17,7 @@ config MTD_CFI > > config MTD_JEDECPROBE > tristate "Detect non-CFI AMD/JEDEC-compatible flash chips" > + depends on HAS_IOMEM || INDIRECT_IOMEM > select MTD_GEN_PROBE > select MTD_CFI_UTIL > help > @@ -211,12 +213,14 @@ config MTD_CFI_UTIL > > config MTD_RAM > tristate "Support for RAM chips in bus mapping" > + depends on HAS_IOMEM || INDIRECT_IOMEM > help > This option enables basic support for RAM chips accessed through > a bus mapping driver. > > config MTD_ROM > tristate "Support for ROM chips in bus mapping" > + depends on HAS_IOMEM || INDIRECT_IOMEM > help > This option enables basic support for ROM chips accessed through > a bus mapping driver. > diff --git a/drivers/mtd/lpddr/Kconfig b/drivers/mtd/lpddr/Kconfig > index 0395aa6b68f1..f35dd8052abc 100644 > --- a/drivers/mtd/lpddr/Kconfig > +++ b/drivers/mtd/lpddr/Kconfig > @@ -4,6 +4,7 @@ menu "LPDDR & LPDDR2 PCM memory drivers" > > config MTD_LPDDR > tristate "Support for LPDDR flash chips" > + depends on HAS_IOMEM || INDIRECT_IOMEM > select MTD_QINFO_PROBE > help > This option enables support of LPDDR (Low power double data rate) Thanks, Miquèl
New patch set with all remarks taken into account. Thank you Richard and David. I have masked the int with 0xff for alpha and parisc, and I have replaced the shift operation by the multiplication as you proposed. Thank you Johannes for your remarks on the UM arch. Finally, I have created an UM allyesconfig. I have manually disabled HAS_IOMEM and INDIRECT_IOMEM. This way I was able to identify the drivers which don't guard with 'depends on HAS_IOMEM || INDIRECT_IOMEM'. It was only a small number. I changed them and added the patches to this series (see patch 12 to 14). Thank you Niklas for your feedback. Unfortunately I was not able to simply change the prototypes of the zpci_memcpy functions because they return an int to indicate whether the pci transaction was successful. At the same time they are used as generic memcpy IO functions in the driver code. To resolve this, I implemented three wrapper functions and added defines to overwrite the default. So, on s390 we always use the fast zpci operations and don't fall back to the generic ones. Signed-off-by: Julian Vetter <jvetter@kalrayinc.com> --- Changes for v8: - Dropped the arch/um patch that adds dummy implementations for IO memcpy functions - Added 3 new patches that fix the dependency problem for UM (added dependencies on HAS_IOMEM || INDIRECT_IOMEM) - Added new patch for s390 to internally call the zpci_memcpy functions and not the generic ones from libs/iomap_copy.c - Addressed reviewer comments and replaced 2 or 3 shifts by 'qc *= ~0UL / 0xff;' - Addressed reviewer comments on pasrisc (masking the int value) - Addressed reviewer comments on alpha (masking the int value) Changes for v7: - Added dummy implementations for memcpy_{to,from}io and memset_io on um architecture so drivers that use these functions build for um - Replaced all accesses and checks by long type - Added function prototypes as extern to asm-generic/io.h - Removed '__' from the 3 new function names - Some archs implement their own version of these IO functions with slightly different prototypes. So, I added 3 new patches to align prototypes with new ones in iomap_copy.c + io.h Changes for v6: - Added include of linux/align.h to fix build on arm arch - Replaced compile-time check by ifdef for the CONFIG_64BIT otherwise we get a warning for the 'qc << 32' for archs with 32bit int types - Suffixed arch commits by arch name Changes for v5: - Added functions to iomap_copy.c as proposed by Arndt - Removed again the new io_copy.c and related objects - Removed GENERIC_IO_COPY symbol and instead rely on the existing HAS_IOMEM symbol - Added prototypes of __memcpy_{to,from}io and __memset_io functions to asm-generic/io.h Changes for v4: - Replaced memcpy/memset in asm-generic/io.h by the new __memcpy_{to,from}io and __memset_io, so individual architectures can use it instead of using their own implementation. Changes for v3: - Replaced again 'if(IS_ENABLED(CONFIG_64BIT))' by '#ifdef CONFIG_64BIT' because on 32bit architectures (e.g., csky), __raw_{read,write}q are not defined. So, it leads to compilation errors Changes for v2: - Renamed io.c -> io_copy.c - Updated flag to 'GENERIC_IO_COPY' - Replaced pointer dereferences by 'put_unaligned()'/'get_unaligned()' - Replaced '#ifdef CONFIG_64BIT' by 'if(IS_ENABLED(CONFIG_64BIT))' - Removed '__raw_{read,write}_native' and replaced by 'if(IS_ENABLED(CONFIG_64BIT))' -> '__raw_write{l,q}' --- Julian Vetter (14): Consolidate IO memcpy/memset into iomap_copy.c arm64: Use generic IO memcpy/memset csky: Use generic IO memcpy/memset loongarch: Use generic IO memcpy/memset m68k: Align prototypes of IO memcpy/memset alpha: Align prototypes of IO memcpy/memset parisc: Align prototypes of IO memcpy/memset sh: Align prototypes of IO memcpy/memset arm: Align prototype of IO memset powerpc: Align prototypes of IO memcpy and memset s390: Add wrappers around zpci_memcpy/zpci_memset bus: mhi: ep: Add HAS_IOMEM || INDIRECT_IOMEM dependency mtd: Add HAS_IOMEM || INDIRECT_IOMEM dependency sound: Make CONFIG_SND depend on INDIRECT_IOMEM instead of UML arch/alpha/include/asm/io.h | 6 +- arch/alpha/kernel/io.c | 4 +- arch/arm/include/asm/io.h | 2 +- arch/arm64/include/asm/io.h | 11 --- arch/arm64/kernel/io.c | 87 -------------------- arch/csky/include/asm/io.h | 11 --- arch/csky/kernel/Makefile | 2 +- arch/csky/kernel/io.c | 91 --------------------- arch/loongarch/include/asm/io.h | 10 --- arch/loongarch/kernel/Makefile | 2 +- arch/loongarch/kernel/io.c | 94 --------------------- arch/m68k/include/asm/kmap.h | 8 +- arch/parisc/include/asm/io.h | 3 - arch/parisc/lib/io.c | 12 ++- arch/powerpc/include/asm/io-defs.h | 6 +- arch/powerpc/include/asm/io.h | 6 +- arch/powerpc/kernel/io.c | 6 +- arch/s390/include/asm/io.h | 27 +++++- arch/s390/include/asm/pci_io.h | 6 +- arch/sh/include/asm/io.h | 3 - arch/sh/kernel/io.c | 6 +- drivers/bus/mhi/ep/Kconfig | 1 + drivers/mtd/chips/Kconfig | 4 + drivers/mtd/lpddr/Kconfig | 1 + include/asm-generic/io.h | 58 ++----------- lib/iomap_copy.c | 127 +++++++++++++++++++++++++++++ sound/Kconfig | 2 +- 27 files changed, 197 insertions(+), 399 deletions(-) delete mode 100644 arch/csky/kernel/io.c delete mode 100644 arch/loongarch/kernel/io.c