[PATCH v2 0/2] spi: realtek-rtl: two small improvements

[PATCH v2 0/2] spi: realtek-rtl: two small improvements

[Martin Blumenstingl]

This is a successor to my single patch [0] "spi: realtek-rtl: Fix
clearing some register bits"

Changes since v1 at [0]:
- Added new patch to enable compile testing for the spi-realtek-rtl
  driver. This makes it easier to verify coding style changes (such as
  patch #2 in this series) where the binary result is not supposed to
  change.
- Updated the original patch to drop the RTL_SPI_SFCSR_CS change as
  it's not fully clear why that code is written as-is (and due to
  lack of hardware I am unable to runtime test the changes). Also
  move the ~ operator from the RTL_SPI_SFCSR_LEN_MASK definition to
  the place where it's used. Thanks to Sander for spotting the mistake
  in my initial patch!


[0] https://lore.kernel.org/lkml/78673dc7438a03bf9ed0103a935abf0172f146ce.camel@svanheule.net/T/


Martin Blumenstingl (2):
  spi: realtek-rtl: Add compile testing support
  spi: realtek-rtl: Improve readability when clearing the size mask

 drivers/spi/Kconfig           | 9 +++++++++
 drivers/spi/Makefile          | 2 +-
 drivers/spi/spi-realtek-rtl.c | 4 ++--
 3 files changed, 12 insertions(+), 3 deletions(-)

-- 
2.37.1

[PATCH v2 1/2] spi: realtek-rtl: Add compile testing support

[Martin Blumenstingl]

Add support for compile testing the spi-realtek-rtl driver to increase
build testing coverage.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/spi/Kconfig  | 9 +++++++++
 drivers/spi/Makefile | 2 +-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 8e550269d488..d8b55efb05d5 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -969,6 +969,15 @@ config SPI_MXS
 	help
 	  SPI driver for Freescale MXS devices.
 
+config SPI_REALTEK_RTL
+	tristate "Realtek RTL SPI controller"
+	depends on HAS_IOMEM
+	depends on MACH_REALTEK_RTL || COMPILE_TEST
+	default MACH_REALTEK_RTL
+	help
+	  SPI driver for the hardware found on Realtek RLTL8380, RTL8382,
+	  RTL8391, RTL8392 and RTL8393 SoCs.
+
 config SPI_TEGRA210_QUAD
 	tristate "NVIDIA Tegra QSPI Controller"
 	depends on ARCH_TEGRA || COMPILE_TEST
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 18bf3d8c6df7..9fe4a2aa0fd7 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -105,7 +105,7 @@ obj-$(CONFIG_SPI_QUP)			+= spi-qup.o
 obj-$(CONFIG_SPI_ROCKCHIP)		+= spi-rockchip.o
 obj-$(CONFIG_SPI_ROCKCHIP_SFC)		+= spi-rockchip-sfc.o
 obj-$(CONFIG_SPI_RB4XX)			+= spi-rb4xx.o
-obj-$(CONFIG_MACH_REALTEK_RTL)		+= spi-realtek-rtl.o
+obj-$(CONFIG_SPI_REALTEK_RTL)		+= spi-realtek-rtl.o
 obj-$(CONFIG_SPI_RPCIF)			+= spi-rpc-if.o
 obj-$(CONFIG_SPI_RSPI)			+= spi-rspi.o
 obj-$(CONFIG_SPI_S3C24XX)		+= spi-s3c24xx-hw.o
-- 
2.37.1

Re: [PATCH v2 1/2] spi: realtek-rtl: Add compile testing support

[Sander Vanheule]

Hi Martin,

On Thu, 2022-07-28 at 17:21 +0200, Martin Blumenstingl wrote:
> Add support for compile testing the spi-realtek-rtl driver to increase
> build testing coverage.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  drivers/spi/Kconfig  | 9 +++++++++
>  drivers/spi/Makefile | 2 +-
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 8e550269d488..d8b55efb05d5 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -969,6 +969,15 @@ config SPI_MXS
>         help
>           SPI driver for Freescale MXS devices.
>  
> +config SPI_REALTEK_RTL
> +       tristate "Realtek RTL SPI controller"
> +       depends on HAS_IOMEM

I think this is already an implied dependency, by virtue of this symbol being
available only when CONFIG_SPI=y.

> +       depends on MACH_REALTEK_RTL || COMPILE_TEST
> +       default MACH_REALTEK_RTL
> +       help
> +         SPI driver for the hardware found on Realtek RLTL8380, RTL8382,
> +         RTL8391, RTL8392 and RTL8393 SoCs.
> +

Could you add how the module will be named be to the help text? See for example
CONFIG_SPI_DLN2.

Best,
Sander

>  config SPI_TEGRA210_QUAD
>         tristate "NVIDIA Tegra QSPI Controller"
>         depends on ARCH_TEGRA || COMPILE_TEST
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index 18bf3d8c6df7..9fe4a2aa0fd7 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -105,7 +105,7 @@ obj-$(CONFIG_SPI_QUP)                       += spi-qup.o
>  obj-$(CONFIG_SPI_ROCKCHIP)             += spi-rockchip.o
>  obj-$(CONFIG_SPI_ROCKCHIP_SFC)         += spi-rockchip-sfc.o
>  obj-$(CONFIG_SPI_RB4XX)                        += spi-rb4xx.o
> -obj-$(CONFIG_MACH_REALTEK_RTL)         += spi-realtek-rtl.o
> +obj-$(CONFIG_SPI_REALTEK_RTL)          += spi-realtek-rtl.o
>  obj-$(CONFIG_SPI_RPCIF)                        += spi-rpc-if.o
>  obj-$(CONFIG_SPI_RSPI)                 += spi-rspi.o
>  obj-$(CONFIG_SPI_S3C24XX)              += spi-s3c24xx-hw.o

[PATCH v2 2/2] spi: realtek-rtl: Improve readability when clearing the size mask

[Martin Blumenstingl]

Define the bitmask RTL_SPI_SFCSR_LEN_MASK so it only sets the bits of
this specific part of the register instead of setting all bits except
the relevant ones. This makes it consistent with single bit macros in
the spi-realtek-rtl driver as well as with the approach that many other
drivers use.

Suggested-by: Sander Vanheule <sander@svanheule.net>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/spi/spi-realtek-rtl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-realtek-rtl.c b/drivers/spi/spi-realtek-rtl.c
index 866b0477dbd7..0371d44cbfbd 100644
--- a/drivers/spi/spi-realtek-rtl.c
+++ b/drivers/spi/spi-realtek-rtl.c
@@ -20,7 +20,7 @@ struct rtspi {
 #define RTL_SPI_SFCSR_CSB1		BIT(30)
 #define RTL_SPI_SFCSR_RDY		BIT(27)
 #define RTL_SPI_SFCSR_CS		BIT(24)
-#define RTL_SPI_SFCSR_LEN_MASK		~(0x03 << 28)
+#define RTL_SPI_SFCSR_LEN_MASK		(0x03 << 28)
 #define RTL_SPI_SFCSR_LEN1		(0x00 << 28)
 #define RTL_SPI_SFCSR_LEN4		(0x03 << 28)
 
@@ -49,7 +49,7 @@ static void set_size(struct rtspi *rtspi, int size)
 	u32 value;
 
 	value = readl(REG(RTL_SPI_SFCSR));
-	value &= RTL_SPI_SFCSR_LEN_MASK;
+	value &= ~RTL_SPI_SFCSR_LEN_MASK;
 	if (size == 4)
 		value |= RTL_SPI_SFCSR_LEN4;
 	else if (size == 1)
-- 
2.37.1

Re: [PATCH v2 2/2] spi: realtek-rtl: Improve readability when clearing the size mask

[Sander Vanheule]

On Thu, 2022-07-28 at 17:21 +0200, Martin Blumenstingl wrote:
> Define the bitmask RTL_SPI_SFCSR_LEN_MASK so it only sets the bits of
> this specific part of the register instead of setting all bits except
> the relevant ones. This makes it consistent with single bit macros in
> the spi-realtek-rtl driver as well as with the approach that many other
> drivers use.
> 
> Suggested-by: Sander Vanheule <sander@svanheule.net>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

FWIW
Tested-by: Sander Vanheule <sander@svanheule.net>

on my Netgear GS110TPP v1.

> ---
>  drivers/spi/spi-realtek-rtl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/spi-realtek-rtl.c b/drivers/spi/spi-realtek-rtl.c
> index 866b0477dbd7..0371d44cbfbd 100644
> --- a/drivers/spi/spi-realtek-rtl.c
> +++ b/drivers/spi/spi-realtek-rtl.c
> @@ -20,7 +20,7 @@ struct rtspi {
>  #define RTL_SPI_SFCSR_CSB1             BIT(30)
>  #define RTL_SPI_SFCSR_RDY              BIT(27)
>  #define RTL_SPI_SFCSR_CS               BIT(24)
> -#define RTL_SPI_SFCSR_LEN_MASK         ~(0x03 << 28)
> +#define RTL_SPI_SFCSR_LEN_MASK         (0x03 << 28)
>  #define RTL_SPI_SFCSR_LEN1             (0x00 << 28)
>  #define RTL_SPI_SFCSR_LEN4             (0x03 << 28)
>  
> @@ -49,7 +49,7 @@ static void set_size(struct rtspi *rtspi, int size)
>         u32 value;
>  
>         value = readl(REG(RTL_SPI_SFCSR));
> -       value &= RTL_SPI_SFCSR_LEN_MASK;
> +       value &= ~RTL_SPI_SFCSR_LEN_MASK;
>         if (size == 4)
>                 value |= RTL_SPI_SFCSR_LEN4;
>         else if (size == 1)