[PATCH] drivers/soc/litex: remove 8-bit subregister option

* [PATCH] drivers/soc/litex: remove 8-bit subregister option
@ 2021-05-21 18:36 Gabriel Somlo
  2021-05-21 23:44 ` Stafford Horne
  0 siblings, 1 reply; 3+ messages in thread
From: Gabriel Somlo @ 2021-05-21 18:36 UTC (permalink / raw)
  To: linux-kernel
  Cc: shorne, kgugala, mholenko, pczarnecki, davidgow, florent, joel

Since upstream LiteX recommends that Linux support be limited to
designs configured with 32-bit CSR subregisters (see commit a2b71fde
in upstream LiteX, https://github.com/enjoy-digital/litex), remove
the option to select 8-bit subregisters, significantly reducing the
complexity of LiteX CSR (MMIO register) accessor methods.

NOTE: for details on the underlying mechanics of LiteX CSR registers,
see https://github.com/enjoy-digital/litex/wiki/CSR-Bus or the original
LiteX accessors (litex/soc/software/include/hw/common.h in the upstream
repository).

Signed-off-by: Gabriel Somlo <gsomlo@gmail.com>
Cc: Stafford Horne <shorne@gmail.com>
Cc: Florent Kermarrec <florent@enjoy-digital.fr>
Cc: Mateusz Holenko <mholenko@antmicro.com>
Cc: Joel Stanley <joel@jms.id.au>

---
 drivers/soc/litex/Kconfig |  12 -----
 include/linux/litex.h     | 100 +++++++-------------------------------
 2 files changed, 17 insertions(+), 95 deletions(-)

diff --git a/drivers/soc/litex/Kconfig b/drivers/soc/litex/Kconfig
index e7011d665b15..e6ba3573a772 100644
--- a/drivers/soc/litex/Kconfig
+++ b/drivers/soc/litex/Kconfig
@@ -17,16 +17,4 @@ config LITEX_SOC_CONTROLLER
 	  All drivers that use functions from litex.h must depend on
 	  LITEX.
 
-config LITEX_SUBREG_SIZE
-	int "Size of a LiteX CSR subregister, in bytes"
-	depends on LITEX
-	range 1 4
-	default 4
-	help
-	LiteX MMIO registers (referred to as Configuration and Status
-	registers, or CSRs) are spread across adjacent 8- or 32-bit
-	subregisters, located at 32-bit aligned MMIO addresses. Use
-	this to select the appropriate size (1 or 4 bytes) matching
-	your particular LiteX build.
-
 endmenu
diff --git a/include/linux/litex.h b/include/linux/litex.h
index 5ea9ccf5cce4..aef6ca8af949 100644
--- a/include/linux/litex.h
+++ b/include/linux/litex.h
@@ -11,13 +11,8 @@
 
 #include <linux/io.h>
 
-/* LiteX SoCs support 8- or 32-bit CSR Bus data width (i.e., subreg. size) */
-#if defined(CONFIG_LITEX_SUBREG_SIZE) && \
-	(CONFIG_LITEX_SUBREG_SIZE == 1 || CONFIG_LITEX_SUBREG_SIZE == 4)
-#define LITEX_SUBREG_SIZE      CONFIG_LITEX_SUBREG_SIZE
-#else
-#error LiteX subregister size (LITEX_SUBREG_SIZE) must be 4 or 1!
-#endif
+/* Linux-capable LiteX SoCs support 32-bit CSR Bus data width (subreg. size) */
+#define LITEX_SUBREG_SIZE	  0x4
 #define LITEX_SUBREG_SIZE_BIT	 (LITEX_SUBREG_SIZE * 8)
 
 /* LiteX subregisters of any width are always aligned on a 4-byte boundary */
@@ -42,115 +37,54 @@ static inline u32 _read_litex_subregister(void __iomem *addr)
  * 32-bit wide logical CSR will be laid out as four 32-bit physical
  * subregisters, each one containing one byte of meaningful data.
  *
+ * For Linux support, upstream LiteX enforces a 32-bit wide CSR bus, which
+ * means that only larger-than-32-bit CSRs will be split across multiple
+ * subregisters (e.g., a 64-bit CSR will be spread across two consecutive
+ * 32-bit subregisters).
+ *
  * For details see: https://github.com/enjoy-digital/litex/wiki/CSR-Bus
  */
 
-/* number of LiteX subregisters needed to store a register of given reg_size */
-#define _litex_num_subregs(reg_size) \
-	(((reg_size) - 1) / LITEX_SUBREG_SIZE + 1)
-
-/*
- * since the number of 4-byte aligned subregisters required to store a single
- * LiteX CSR (MMIO) register varies with LITEX_SUBREG_SIZE, the offset of the
- * next adjacent LiteX CSR register w.r.t. the offset of the current one also
- * depends on how many subregisters the latter is spread across
- */
-#define _next_reg_off(off, size) \
-	((off) + _litex_num_subregs(size) * LITEX_SUBREG_ALIGN)
-
-/*
- * The purpose of `_litex_[set|get]_reg()` is to implement the logic of
- * writing to/reading from the LiteX CSR in a single place that can be then
- * reused by all LiteX drivers via the `litex_[write|read][8|16|32|64]()`
- * accessors for the appropriate data width.
- * NOTE: direct use of `_litex_[set|get]_reg()` by LiteX drivers is strongly
- * discouraged, as they perform no error checking on the requested data width!
- */
-
-/**
- * _litex_set_reg() - Writes a value to the LiteX CSR (Control&Status Register)
- * @reg: Address of the CSR
- * @reg_size: The width of the CSR expressed in the number of bytes
- * @val: Value to be written to the CSR
- *
- * This function splits a single (possibly multi-byte) LiteX CSR write into
- * a series of subregister writes with a proper offset.
- * NOTE: caller is responsible for ensuring (0 < reg_size <= sizeof(u64)).
- */
-static inline void _litex_set_reg(void __iomem *reg, size_t reg_size, u64 val)
-{
-	u8 shift = _litex_num_subregs(reg_size) * LITEX_SUBREG_SIZE_BIT;
-
-	while (shift > 0) {
-		shift -= LITEX_SUBREG_SIZE_BIT;
-		_write_litex_subregister(val >> shift, reg);
-		reg += LITEX_SUBREG_ALIGN;
-	}
-}
-
-/**
- * _litex_get_reg() - Reads a value of the LiteX CSR (Control&Status Register)
- * @reg: Address of the CSR
- * @reg_size: The width of the CSR expressed in the number of bytes
- *
- * Return: Value read from the CSR
- *
- * This function generates a series of subregister reads with a proper offset
- * and joins their results into a single (possibly multi-byte) LiteX CSR value.
- * NOTE: caller is responsible for ensuring (0 < reg_size <= sizeof(u64)).
- */
-static inline u64 _litex_get_reg(void __iomem *reg, size_t reg_size)
-{
-	u64 r;
-	u8 i;
-
-	r = _read_litex_subregister(reg);
-	for (i = 1; i < _litex_num_subregs(reg_size); i++) {
-		r <<= LITEX_SUBREG_SIZE_BIT;
-		reg += LITEX_SUBREG_ALIGN;
-		r |= _read_litex_subregister(reg);
-	}
-	return r;
-}
-
 static inline void litex_write8(void __iomem *reg, u8 val)
 {
-	_litex_set_reg(reg, sizeof(u8), val);
+	_write_litex_subregister(val, reg);
 }
 
 static inline void litex_write16(void __iomem *reg, u16 val)
 {
-	_litex_set_reg(reg, sizeof(u16), val);
+	_write_litex_subregister(val, reg);
 }
 
 static inline void litex_write32(void __iomem *reg, u32 val)
 {
-	_litex_set_reg(reg, sizeof(u32), val);
+	_write_litex_subregister(val, reg);
 }
 
 static inline void litex_write64(void __iomem *reg, u64 val)
 {
-	_litex_set_reg(reg, sizeof(u64), val);
+	_write_litex_subregister(val >> LITEX_SUBREG_SIZE_BIT, reg);
+	_write_litex_subregister(val, reg + LITEX_SUBREG_ALIGN);
 }
 
 static inline u8 litex_read8(void __iomem *reg)
 {
-	return _litex_get_reg(reg, sizeof(u8));
+	return _read_litex_subregister(reg);
 }
 
 static inline u16 litex_read16(void __iomem *reg)
 {
-	return _litex_get_reg(reg, sizeof(u16));
+	return _read_litex_subregister(reg);
 }
 
 static inline u32 litex_read32(void __iomem *reg)
 {
-	return _litex_get_reg(reg, sizeof(u32));
+	return _read_litex_subregister(reg);
 }
 
 static inline u64 litex_read64(void __iomem *reg)
 {
-	return _litex_get_reg(reg, sizeof(u64));
+	return ((u64)_read_litex_subregister(reg) << LITEX_SUBREG_SIZE_BIT) |
+		_read_litex_subregister(reg + LITEX_SUBREG_ALIGN);
 }
 
 #endif /* _LINUX_LITEX_H */
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread