| Message ID | 20240125145007.748295-26-tudor.ambarus@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | spi: s3c64xx: winter cleanup and gs101 support | expand |
On Thu, Jan 25, 2024 at 02:50:03PM +0000, Tudor Ambarus wrote: > This will allow devices that require 32 bits register accesses to write > data in chunks of 8 or 16 bits. > > One SoC that requires 32 bit register accesses is the google gs101. A > typical use case is SPI, where the clients can request transfers in words > of 8 bits. Might be good to CC this one to linux-arch if reposting.
On Thu, Jan 25, 2024, at 15:50, Tudor Ambarus wrote: > This will allow devices that require 32 bits register accesses to write > data in chunks of 8 or 16 bits. > > One SoC that requires 32 bit register accesses is the google gs101. A > typical use case is SPI, where the clients can request transfers in words > of 8 bits. > > Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> My feeling is that this operation is rare enough that I'd prefer it to be open-coded in the driver than made generic here. Making it work for all corner cases is possible but probably not worth it. > +#ifndef writesb_l > +#define writesb_l writesb_l > +static inline void writesb_l(volatile void __iomem *addr, const void > *buffer, > + unsigned int count) > +{ > + if (count) { > + const u8 *buf = buffer; > + > + do { > + __raw_writel(*buf++, addr); > + } while (--count); > + } > +} > +#endif There are architectures where writesb() requires an extra barrier before and/or after the loop. I think there are others that get the endianess wrong in the generic version you have here. > +#ifndef iowrite8_32_rep > +#define iowrite8_32_rep iowrite8_32_rep > +static inline void iowrite8_32_rep(volatile void __iomem *addr, > + const void *buffer, > + unsigned int count) > +{ > + writesb_l(addr, buffer, count); > +} > +#endif This one is wrong for architectures that have a custom inl() helper and need to multiplex between inl() and writel() in iowrite32(), notably x86. For completeness you would need to add the out-of-line version in lib/iomap.c for those, plus the corresponding insb_32() and possibly the respective big-endian versions of those. If you keep the helper in a driver that is only used on regular architectures like arm64, it will work reliably. Arnd
On 1/25/24 21:23, Arnd Bergmann wrote: > My feeling is that this operation is rare enough that I'd prefer > it to be open-coded in the driver than made generic here. Making > it work for all corner cases is possible but probably not worth > it. Thanks for all the explanations, Arnd. I'll open-code the op in the SPI driver for now. Cheers, ta
diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h index bac63e874c7b..1e224d1ccc98 100644 --- a/include/asm-generic/io.h +++ b/include/asm-generic/io.h @@ -476,6 +476,21 @@ static inline void writesb(volatile void __iomem *addr, const void *buffer, } #endif +#ifndef writesb_l +#define writesb_l writesb_l +static inline void writesb_l(volatile void __iomem *addr, const void *buffer, + unsigned int count) +{ + if (count) { + const u8 *buf = buffer; + + do { + __raw_writel(*buf++, addr); + } while (--count); + } +} +#endif + #ifndef writesw #define writesw writesw static inline void writesw(volatile void __iomem *addr, const void *buffer, @@ -491,6 +506,21 @@ static inline void writesw(volatile void __iomem *addr, const void *buffer, } #endif +#ifndef writesw_l +#define writesw_l writesw_l +static inline void writesw_l(volatile void __iomem *addr, const void *buffer, + unsigned int count) +{ + if (count) { + const u16 *buf = buffer; + + do { + __raw_writel(*buf++, addr); + } while (--count); + } +} +#endif + #ifndef writesl #define writesl writesl static inline void writesl(volatile void __iomem *addr, const void *buffer, @@ -956,6 +986,16 @@ static inline void iowrite8_rep(volatile void __iomem *addr, } #endif +#ifndef iowrite8_32_rep +#define iowrite8_32_rep iowrite8_32_rep +static inline void iowrite8_32_rep(volatile void __iomem *addr, + const void *buffer, + unsigned int count) +{ + writesb_l(addr, buffer, count); +} +#endif + #ifndef iowrite16_rep #define iowrite16_rep iowrite16_rep static inline void iowrite16_rep(volatile void __iomem *addr, @@ -966,6 +1006,16 @@ static inline void iowrite16_rep(volatile void __iomem *addr, } #endif +#ifndef iowrite16_32_rep +#define iowrite16_32_rep iowrite16_32_rep +static inline void iowrite16_32_rep(volatile void __iomem *addr, + const void *buffer, + unsigned int count) +{ + writesw_l(addr, buffer, count); +} +#endif + #ifndef iowrite32_rep #define iowrite32_rep iowrite32_rep static inline void iowrite32_rep(volatile void __iomem *addr, diff --git a/include/asm-generic/iomap.h b/include/asm-generic/iomap.h index 196087a8126e..9d63f9adf2db 100644 --- a/include/asm-generic/iomap.h +++ b/include/asm-generic/iomap.h @@ -84,7 +84,9 @@ extern void ioread16_rep(const void __iomem *port, void *buf, unsigned long coun extern void ioread32_rep(const void __iomem *port, void *buf, unsigned long count); extern void iowrite8_rep(void __iomem *port, const void *buf, unsigned long count); +extern void iowrite8_32_rep(void __iomem *port, const void *buf, unsigned long count); extern void iowrite16_rep(void __iomem *port, const void *buf, unsigned long count); +extern void iowrite16_32_rep(void __iomem *port, const void *buf, unsigned long count); extern void iowrite32_rep(void __iomem *port, const void *buf, unsigned long count); #ifdef CONFIG_HAS_IOPORT_MAP
This will allow devices that require 32 bits register accesses to write data in chunks of 8 or 16 bits. One SoC that requires 32 bit register accesses is the google gs101. A typical use case is SPI, where the clients can request transfers in words of 8 bits. Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> --- include/asm-generic/io.h | 50 +++++++++++++++++++++++++++++++++++++ include/asm-generic/iomap.h | 2 ++ 2 files changed, 52 insertions(+)