[PATCH V3 00/10] net: dsa: microchip: Convert to regmap

[PATCH V3 00/10] net: dsa: microchip: Convert to regmap

[Marek Vasut]

This patchset converts KSZ9477 switch driver to regmap.

This was tested with extra patches on KSZ8795. This was also tested
on KSZ9477 on Microchip KSZ9477EVB board, which I now have.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>

Marek Vasut (10):
  net: dsa: microchip: Remove ksz_{read,write}24()
  net: dsa: microchip: Remove ksz_{get,set}()
  net: dsa: microchip: Inline ksz_spi.h
  net: dsa: microchip: Move ksz_cfg and ksz_port_cfg to ksz9477.c
  net: dsa: microchip: Use PORT_CTRL_ADDR() instead of indirect function
    call
  net: dsa: microchip: Factor out register access opcode generation
  net: dsa: microchip: Initial SPI regmap support
  net: dsa: microchip: Dispose of ksz_io_ops
  net: dsa: microchip: Factor out regmap config generation into common
    header
  net: dsa: microchip: Replace ad-hoc bit manipulation with regmap

 drivers/net/dsa/microchip/Kconfig       |   1 +
 drivers/net/dsa/microchip/ksz9477.c     |  35 +++---
 drivers/net/dsa/microchip/ksz9477_spi.c | 114 +++--------------
 drivers/net/dsa/microchip/ksz_common.c  |   6 +-
 drivers/net/dsa/microchip/ksz_common.h  | 157 +++++++-----------------
 drivers/net/dsa/microchip/ksz_priv.h    |  23 +---
 drivers/net/dsa/microchip/ksz_spi.h     |  69 -----------
 7 files changed, 84 insertions(+), 321 deletions(-)
 delete mode 100644 drivers/net/dsa/microchip/ksz_spi.h

-- 
2.20.1

[PATCH V3 01/10] net: dsa: microchip: Remove ksz_{read,write}24()

[Marek Vasut]

These functions and callbacks are never used, remove them.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
---
V2: No change
V3: - Rebase on next/master
    - Test on KSZ9477EVB
---
 drivers/net/dsa/microchip/ksz9477_spi.c | 25 -------------------------
 drivers/net/dsa/microchip/ksz_common.h  | 22 ----------------------
 drivers/net/dsa/microchip/ksz_priv.h    |  2 --
 3 files changed, 49 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477_spi.c b/drivers/net/dsa/microchip/ksz9477_spi.c
index 75178624d3f5..e7118319c192 100644
--- a/drivers/net/dsa/microchip/ksz9477_spi.c
+++ b/drivers/net/dsa/microchip/ksz9477_spi.c
@@ -73,37 +73,12 @@ static int ksz_spi_write(struct ksz_device *dev, u32 reg, void *data,
 	return ksz9477_spi_write_reg(spi, reg, dev->txbuf, len);
 }
 
-static int ksz_spi_read24(struct ksz_device *dev, u32 reg, u32 *val)
-{
-	int ret;
-
-	*val = 0;
-	ret = ksz_spi_read(dev, reg, (u8 *)val, 3);
-	if (!ret) {
-		*val = be32_to_cpu(*val);
-		/* convert to 24bit */
-		*val >>= 8;
-	}
-
-	return ret;
-}
-
-static int ksz_spi_write24(struct ksz_device *dev, u32 reg, u32 value)
-{
-	/* make it to big endian 24bit from MSB */
-	value <<= 8;
-	value = cpu_to_be32(value);
-	return ksz_spi_write(dev, reg, &value, 3);
-}
-
 static const struct ksz_io_ops ksz9477_spi_ops = {
 	.read8 = ksz_spi_read8,
 	.read16 = ksz_spi_read16,
-	.read24 = ksz_spi_read24,
 	.read32 = ksz_spi_read32,
 	.write8 = ksz_spi_write8,
 	.write16 = ksz_spi_write16,
-	.write24 = ksz_spi_write24,
 	.write32 = ksz_spi_write32,
 	.get = ksz_spi_get,
 	.set = ksz_spi_set,
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 21cd794e18f1..1781539c3a81 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -61,17 +61,6 @@ static inline int ksz_read16(struct ksz_device *dev, u32 reg, u16 *val)
 	return ret;
 }
 
-static inline int ksz_read24(struct ksz_device *dev, u32 reg, u32 *val)
-{
-	int ret;
-
-	mutex_lock(&dev->reg_mutex);
-	ret = dev->ops->read24(dev, reg, val);
-	mutex_unlock(&dev->reg_mutex);
-
-	return ret;
-}
-
 static inline int ksz_read32(struct ksz_device *dev, u32 reg, u32 *val)
 {
 	int ret;
@@ -105,17 +94,6 @@ static inline int ksz_write16(struct ksz_device *dev, u32 reg, u16 value)
 	return ret;
 }
 
-static inline int ksz_write24(struct ksz_device *dev, u32 reg, u32 value)
-{
-	int ret;
-
-	mutex_lock(&dev->reg_mutex);
-	ret = dev->ops->write24(dev, reg, value);
-	mutex_unlock(&dev->reg_mutex);
-
-	return ret;
-}
-
 static inline int ksz_write32(struct ksz_device *dev, u32 reg, u32 value)
 {
 	int ret;
diff --git a/drivers/net/dsa/microchip/ksz_priv.h b/drivers/net/dsa/microchip/ksz_priv.h
index c615d2a81dd5..5ef6153bd2cc 100644
--- a/drivers/net/dsa/microchip/ksz_priv.h
+++ b/drivers/net/dsa/microchip/ksz_priv.h
@@ -105,11 +105,9 @@ struct ksz_device {
 struct ksz_io_ops {
 	int (*read8)(struct ksz_device *dev, u32 reg, u8 *value);
 	int (*read16)(struct ksz_device *dev, u32 reg, u16 *value);
-	int (*read24)(struct ksz_device *dev, u32 reg, u32 *value);
 	int (*read32)(struct ksz_device *dev, u32 reg, u32 *value);
 	int (*write8)(struct ksz_device *dev, u32 reg, u8 value);
 	int (*write16)(struct ksz_device *dev, u32 reg, u16 value);
-	int (*write24)(struct ksz_device *dev, u32 reg, u32 value);
 	int (*write32)(struct ksz_device *dev, u32 reg, u32 value);
 	int (*get)(struct ksz_device *dev, u32 reg, void *data, size_t len);
 	int (*set)(struct ksz_device *dev, u32 reg, void *data, size_t len);
-- 
2.20.1

[PATCH V3 02/10] net: dsa: microchip: Remove ksz_{get,set}()

[Marek Vasut]

These functions and callbacks are never used, remove them.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
---
V2: No change
V3: - Rebase on next/master
    - Test on KSZ9477EVB
---
 drivers/net/dsa/microchip/ksz9477_spi.c |  2 --
 drivers/net/dsa/microchip/ksz_common.h  | 24 ------------------------
 drivers/net/dsa/microchip/ksz_priv.h    |  2 --
 drivers/net/dsa/microchip/ksz_spi.h     | 10 ----------
 4 files changed, 38 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477_spi.c b/drivers/net/dsa/microchip/ksz9477_spi.c
index e7118319c192..86d12d48a2a9 100644
--- a/drivers/net/dsa/microchip/ksz9477_spi.c
+++ b/drivers/net/dsa/microchip/ksz9477_spi.c
@@ -80,8 +80,6 @@ static const struct ksz_io_ops ksz9477_spi_ops = {
 	.write8 = ksz_spi_write8,
 	.write16 = ksz_spi_write16,
 	.write32 = ksz_spi_write32,
-	.get = ksz_spi_get,
-	.set = ksz_spi_set,
 };
 
 static int ksz9477_spi_probe(struct spi_device *spi)
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 1781539c3a81..c15b49528bad 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -105,30 +105,6 @@ static inline int ksz_write32(struct ksz_device *dev, u32 reg, u32 value)
 	return ret;
 }
 
-static inline int ksz_get(struct ksz_device *dev, u32 reg, void *data,
-			  size_t len)
-{
-	int ret;
-
-	mutex_lock(&dev->reg_mutex);
-	ret = dev->ops->get(dev, reg, data, len);
-	mutex_unlock(&dev->reg_mutex);
-
-	return ret;
-}
-
-static inline int ksz_set(struct ksz_device *dev, u32 reg, void *data,
-			  size_t len)
-{
-	int ret;
-
-	mutex_lock(&dev->reg_mutex);
-	ret = dev->ops->set(dev, reg, data, len);
-	mutex_unlock(&dev->reg_mutex);
-
-	return ret;
-}
-
 static inline void ksz_pread8(struct ksz_device *dev, int port, int offset,
 			      u8 *data)
 {
diff --git a/drivers/net/dsa/microchip/ksz_priv.h b/drivers/net/dsa/microchip/ksz_priv.h
index 5ef6153bd2cc..d3ddf98156bb 100644
--- a/drivers/net/dsa/microchip/ksz_priv.h
+++ b/drivers/net/dsa/microchip/ksz_priv.h
@@ -109,8 +109,6 @@ struct ksz_io_ops {
 	int (*write8)(struct ksz_device *dev, u32 reg, u8 value);
 	int (*write16)(struct ksz_device *dev, u32 reg, u16 value);
 	int (*write32)(struct ksz_device *dev, u32 reg, u32 value);
-	int (*get)(struct ksz_device *dev, u32 reg, void *data, size_t len);
-	int (*set)(struct ksz_device *dev, u32 reg, void *data, size_t len);
 };
 
 struct alu_struct {
diff --git a/drivers/net/dsa/microchip/ksz_spi.h b/drivers/net/dsa/microchip/ksz_spi.h
index 427811bd60b3..976bace31f37 100644
--- a/drivers/net/dsa/microchip/ksz_spi.h
+++ b/drivers/net/dsa/microchip/ksz_spi.h
@@ -56,14 +56,4 @@ static int ksz_spi_write32(struct ksz_device *dev, u32 reg, u32 value)
 	return ksz_spi_write(dev, reg, &value, 4);
 }
 
-static int ksz_spi_get(struct ksz_device *dev, u32 reg, void *data, size_t len)
-{
-	return ksz_spi_read(dev, reg, data, len);
-}
-
-static int ksz_spi_set(struct ksz_device *dev, u32 reg, void *data, size_t len)
-{
-	return ksz_spi_write(dev, reg, data, len);
-}
-
 #endif
-- 
2.20.1

[PATCH V3 03/10] net: dsa: microchip: Inline ksz_spi.h

[Marek Vasut]

The functions in the header file are static, and the header file is
included from single C file, just inline the code into the C file.
The bonus is that it's easier to spot further content to clean up.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
---
V2: No change
V3: - Rebase on next/master
    - Test on KSZ9477EVB
---
 drivers/net/dsa/microchip/ksz9477_spi.c | 43 +++++++++++++++++-
 drivers/net/dsa/microchip/ksz_spi.h     | 59 -------------------------
 2 files changed, 42 insertions(+), 60 deletions(-)
 delete mode 100644 drivers/net/dsa/microchip/ksz_spi.h

diff --git a/drivers/net/dsa/microchip/ksz9477_spi.c b/drivers/net/dsa/microchip/ksz9477_spi.c
index 86d12d48a2a9..a34e66eccbcd 100644
--- a/drivers/net/dsa/microchip/ksz9477_spi.c
+++ b/drivers/net/dsa/microchip/ksz9477_spi.c
@@ -13,7 +13,6 @@
 #include <linux/spi/spi.h>
 
 #include "ksz_priv.h"
-#include "ksz_spi.h"
 
 /* SPI frame opcodes */
 #define KS_SPIOP_RD			3
@@ -73,6 +72,48 @@ static int ksz_spi_write(struct ksz_device *dev, u32 reg, void *data,
 	return ksz9477_spi_write_reg(spi, reg, dev->txbuf, len);
 }
 
+static int ksz_spi_read8(struct ksz_device *dev, u32 reg, u8 *val)
+{
+	return ksz_spi_read(dev, reg, val, 1);
+}
+
+static int ksz_spi_read16(struct ksz_device *dev, u32 reg, u16 *val)
+{
+	int ret = ksz_spi_read(dev, reg, (u8 *)val, 2);
+
+	if (!ret)
+		*val = be16_to_cpu(*val);
+
+	return ret;
+}
+
+static int ksz_spi_read32(struct ksz_device *dev, u32 reg, u32 *val)
+{
+	int ret = ksz_spi_read(dev, reg, (u8 *)val, 4);
+
+	if (!ret)
+		*val = be32_to_cpu(*val);
+
+	return ret;
+}
+
+static int ksz_spi_write8(struct ksz_device *dev, u32 reg, u8 value)
+{
+	return ksz_spi_write(dev, reg, &value, 1);
+}
+
+static int ksz_spi_write16(struct ksz_device *dev, u32 reg, u16 value)
+{
+	value = cpu_to_be16(value);
+	return ksz_spi_write(dev, reg, &value, 2);
+}
+
+static int ksz_spi_write32(struct ksz_device *dev, u32 reg, u32 value)
+{
+	value = cpu_to_be32(value);
+	return ksz_spi_write(dev, reg, &value, 4);
+}
+
 static const struct ksz_io_ops ksz9477_spi_ops = {
 	.read8 = ksz_spi_read8,
 	.read16 = ksz_spi_read16,
diff --git a/drivers/net/dsa/microchip/ksz_spi.h b/drivers/net/dsa/microchip/ksz_spi.h
deleted file mode 100644
index 976bace31f37..000000000000
--- a/drivers/net/dsa/microchip/ksz_spi.h
+++ /dev/null
@@ -1,59 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0
- * Microchip KSZ series SPI access common header
- *
- * Copyright (C) 2017-2018 Microchip Technology Inc.
- *	Tristram Ha <Tristram.Ha@microchip.com>
- */
-
-#ifndef __KSZ_SPI_H
-#define __KSZ_SPI_H
-
-/* Chip dependent SPI access */
-static int ksz_spi_read(struct ksz_device *dev, u32 reg, u8 *data,
-			unsigned int len);
-static int ksz_spi_write(struct ksz_device *dev, u32 reg, void *data,
-			 unsigned int len);
-
-static int ksz_spi_read8(struct ksz_device *dev, u32 reg, u8 *val)
-{
-	return ksz_spi_read(dev, reg, val, 1);
-}
-
-static int ksz_spi_read16(struct ksz_device *dev, u32 reg, u16 *val)
-{
-	int ret = ksz_spi_read(dev, reg, (u8 *)val, 2);
-
-	if (!ret)
-		*val = be16_to_cpu(*val);
-
-	return ret;
-}
-
-static int ksz_spi_read32(struct ksz_device *dev, u32 reg, u32 *val)
-{
-	int ret = ksz_spi_read(dev, reg, (u8 *)val, 4);
-
-	if (!ret)
-		*val = be32_to_cpu(*val);
-
-	return ret;
-}
-
-static int ksz_spi_write8(struct ksz_device *dev, u32 reg, u8 value)
-{
-	return ksz_spi_write(dev, reg, &value, 1);
-}
-
-static int ksz_spi_write16(struct ksz_device *dev, u32 reg, u16 value)
-{
-	value = cpu_to_be16(value);
-	return ksz_spi_write(dev, reg, &value, 2);
-}
-
-static int ksz_spi_write32(struct ksz_device *dev, u32 reg, u32 value)
-{
-	value = cpu_to_be32(value);
-	return ksz_spi_write(dev, reg, &value, 4);
-}
-
-#endif
-- 
2.20.1

[PATCH V3 04/10] net: dsa: microchip: Move ksz_cfg and ksz_port_cfg to ksz9477.c

[Marek Vasut]

These functions are only used by the KSZ9477 code, move them from
the header into that code. Note that these functions will be soon
replaced by regmap equivalents.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
---
V2: New patch
V3: - Rebase on next/master
    - Test on KSZ9477EVB
---
 drivers/net/dsa/microchip/ksz9477.c    | 29 ++++++++++++++++++++++++++
 drivers/net/dsa/microchip/ksz_common.h | 29 --------------------------
 2 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 508380f80875..e8b96566abd9 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -65,6 +65,35 @@ static const struct {
 	{ 0x83, "tx_discards" },
 };
 
+static void ksz_cfg(struct ksz_device *dev, u32 addr, u8 bits, bool set)
+{
+	u8 data;
+
+	ksz_read8(dev, addr, &data);
+	if (set)
+		data |= bits;
+	else
+		data &= ~bits;
+	ksz_write8(dev, addr, data);
+}
+
+static void ksz_port_cfg(struct ksz_device *dev, int port, int offset, u8 bits,
+			 bool set)
+{
+	u32 addr;
+	u8 data;
+
+	addr = dev->dev_ops->get_port_addr(port, offset);
+	ksz_read8(dev, addr, &data);
+
+	if (set)
+		data |= bits;
+	else
+		data &= ~bits;
+
+	ksz_write8(dev, addr, data);
+}
+
 static void ksz9477_cfg32(struct ksz_device *dev, u32 addr, u32 bits, bool set)
 {
 	u32 data;
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index c15b49528bad..fe576a00facf 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -141,35 +141,6 @@ static inline void ksz_pwrite32(struct ksz_device *dev, int port, int offset,
 	ksz_write32(dev, dev->dev_ops->get_port_addr(port, offset), data);
 }
 
-static void ksz_cfg(struct ksz_device *dev, u32 addr, u8 bits, bool set)
-{
-	u8 data;
-
-	ksz_read8(dev, addr, &data);
-	if (set)
-		data |= bits;
-	else
-		data &= ~bits;
-	ksz_write8(dev, addr, data);
-}
-
-static void ksz_port_cfg(struct ksz_device *dev, int port, int offset, u8 bits,
-			 bool set)
-{
-	u32 addr;
-	u8 data;
-
-	addr = dev->dev_ops->get_port_addr(port, offset);
-	ksz_read8(dev, addr, &data);
-
-	if (set)
-		data |= bits;
-	else
-		data &= ~bits;
-
-	ksz_write8(dev, addr, data);
-}
-
 struct ksz_poll_ctx {
 	struct ksz_device *dev;
 	int port;
-- 
2.20.1

[PATCH V3 05/10] net: dsa: microchip: Use PORT_CTRL_ADDR() instead of indirect function call

[Marek Vasut]

The indirect function call to dev->dev_ops->get_port_addr() is expensive
especially if called for every single register access, and only returns
the value of PORT_CTRL_ADDR() macro. Use PORT_CTRL_ADDR() macro directly
instead.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
---
V2: New patch
V3: - Rebase on next/master
    - Test on KSZ9477EVB
---
 drivers/net/dsa/microchip/ksz9477.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index e8b96566abd9..7d209fd9f26f 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -83,7 +83,7 @@ static void ksz_port_cfg(struct ksz_device *dev, int port, int offset, u8 bits,
 	u32 addr;
 	u8 data;
 
-	addr = dev->dev_ops->get_port_addr(port, offset);
+	addr = PORT_CTRL_ADDR(port, offset);
 	ksz_read8(dev, addr, &data);
 
 	if (set)
-- 
2.20.1

[PATCH V3 06/10] net: dsa: microchip: Factor out register access opcode generation

[Marek Vasut]

Factor out the code which sends out the register read/write opcodes
to the switch, since the code differs in single bit between read and
write.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
---
V2: New patch
V3: - Rebase on next/master
    - Test on KSZ9477EVB
---
 drivers/net/dsa/microchip/ksz9477_spi.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477_spi.c b/drivers/net/dsa/microchip/ksz9477_spi.c
index a34e66eccbcd..49aeb92d36fc 100644
--- a/drivers/net/dsa/microchip/ksz9477_spi.c
+++ b/drivers/net/dsa/microchip/ksz9477_spi.c
@@ -25,19 +25,24 @@
 /* Enough to read all switch port registers. */
 #define SPI_TX_BUF_LEN			0x100
 
-static int ksz9477_spi_read_reg(struct spi_device *spi, u32 reg, u8 *val,
-				unsigned int len)
+static u32 ksz9477_spi_cmd(u32 reg, bool read)
 {
 	u32 txbuf;
-	int ret;
 
 	txbuf = reg & SPI_ADDR_MASK;
-	txbuf |= KS_SPIOP_RD << SPI_ADDR_SHIFT;
+	txbuf |= (read ? KS_SPIOP_RD : KS_SPIOP_WR) << SPI_ADDR_SHIFT;
 	txbuf <<= SPI_TURNAROUND_SHIFT;
 	txbuf = cpu_to_be32(txbuf);
 
-	ret = spi_write_then_read(spi, &txbuf, 4, val, len);
-	return ret;
+	return txbuf;
+}
+
+static int ksz9477_spi_read_reg(struct spi_device *spi, u32 reg, u8 *val,
+				unsigned int len)
+{
+	u32 txbuf = ksz9477_spi_cmd(reg, true);
+
+	return spi_write_then_read(spi, &txbuf, 4, val, len);
 }
 
 static int ksz9477_spi_write_reg(struct spi_device *spi, u32 reg, u8 *val,
@@ -45,10 +50,7 @@ static int ksz9477_spi_write_reg(struct spi_device *spi, u32 reg, u8 *val,
 {
 	u32 *txbuf = (u32 *)val;
 
-	*txbuf = reg & SPI_ADDR_MASK;
-	*txbuf |= (KS_SPIOP_WR << SPI_ADDR_SHIFT);
-	*txbuf <<= SPI_TURNAROUND_SHIFT;
-	*txbuf = cpu_to_be32(*txbuf);
+	*txbuf = ksz9477_spi_cmd(reg, false);
 
 	return spi_write(spi, txbuf, 4 + len);
 }
-- 
2.20.1

[PATCH V3 07/10] net: dsa: microchip: Initial SPI regmap support

[Marek Vasut]

Add basic SPI regmap support into the driver.

Previous patches unconver that ksz_spi_write() is always ever called
with len = 1, 2 or 4. We can thus drop the if (len > SPI_TX_BUF_LEN)
check and we can also drop the allocation of the txbuf which is part
of the driver data and wastes 256 bytes for no reason. Regmap covers
the whole thing now.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
---
V2: - Squash with "net: dsa: microchip: Remove dev->txbuf"
    - Use separate regmaps for 8/16/32bit registers
    - Increase the regmap size to 0xd00 to cover the entire register area
V3: - Rebase on next/master
    - Test on KSZ9477EVB
    - Increase regmap max register, to cover all switch registers
    - Correct regmap reg_bits value to match the hardware
    - Use cpu_to_be32() instead of cpu_to_le16() in register masks, since
      the KSZ9477 has some 32bit registers.
---
 drivers/net/dsa/microchip/Kconfig       |   1 +
 drivers/net/dsa/microchip/ksz9477_spi.c | 114 +++++++++++-------------
 drivers/net/dsa/microchip/ksz_priv.h    |   3 +-
 3 files changed, 52 insertions(+), 66 deletions(-)

diff --git a/drivers/net/dsa/microchip/Kconfig b/drivers/net/dsa/microchip/Kconfig
index 2c3a6751bdaf..56790d544eb2 100644
--- a/drivers/net/dsa/microchip/Kconfig
+++ b/drivers/net/dsa/microchip/Kconfig
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config NET_DSA_MICROCHIP_KSZ_COMMON
+	select REGMAP_SPI
 	tristate
 
 menuconfig NET_DSA_MICROCHIP_KSZ9477
diff --git a/drivers/net/dsa/microchip/ksz9477_spi.c b/drivers/net/dsa/microchip/ksz9477_spi.c
index 49aeb92d36fc..77e3cb100eae 100644
--- a/drivers/net/dsa/microchip/ksz9477_spi.c
+++ b/drivers/net/dsa/microchip/ksz9477_spi.c
@@ -10,78 +10,54 @@
 #include <linux/delay.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/regmap.h>
 #include <linux/spi/spi.h>
 
 #include "ksz_priv.h"
 
-/* SPI frame opcodes */
-#define KS_SPIOP_RD			3
-#define KS_SPIOP_WR			2
-
 #define SPI_ADDR_SHIFT			24
-#define SPI_ADDR_MASK			(BIT(SPI_ADDR_SHIFT) - 1)
+#define SPI_ADDR_ALIGN			3
 #define SPI_TURNAROUND_SHIFT		5
 
-/* Enough to read all switch port registers. */
-#define SPI_TX_BUF_LEN			0x100
-
-static u32 ksz9477_spi_cmd(u32 reg, bool read)
-{
-	u32 txbuf;
-
-	txbuf = reg & SPI_ADDR_MASK;
-	txbuf |= (read ? KS_SPIOP_RD : KS_SPIOP_WR) << SPI_ADDR_SHIFT;
-	txbuf <<= SPI_TURNAROUND_SHIFT;
-	txbuf = cpu_to_be32(txbuf);
-
-	return txbuf;
-}
-
-static int ksz9477_spi_read_reg(struct spi_device *spi, u32 reg, u8 *val,
-				unsigned int len)
-{
-	u32 txbuf = ksz9477_spi_cmd(reg, true);
-
-	return spi_write_then_read(spi, &txbuf, 4, val, len);
-}
-
-static int ksz9477_spi_write_reg(struct spi_device *spi, u32 reg, u8 *val,
-				 unsigned int len)
-{
-	u32 *txbuf = (u32 *)val;
-
-	*txbuf = ksz9477_spi_cmd(reg, false);
-
-	return spi_write(spi, txbuf, 4 + len);
-}
-
-static int ksz_spi_read(struct ksz_device *dev, u32 reg, u8 *data,
-			unsigned int len)
-{
-	struct spi_device *spi = dev->priv;
-
-	return ksz9477_spi_read_reg(spi, reg, data, len);
-}
-
-static int ksz_spi_write(struct ksz_device *dev, u32 reg, void *data,
-			 unsigned int len)
-{
-	struct spi_device *spi = dev->priv;
+/* SPI frame opcodes */
+#define KS_SPIOP_RD			3
+#define KS_SPIOP_WR			2
 
-	if (len > SPI_TX_BUF_LEN)
-		len = SPI_TX_BUF_LEN;
-	memcpy(&dev->txbuf[4], data, len);
-	return ksz9477_spi_write_reg(spi, reg, dev->txbuf, len);
-}
+#define KS_SPIOP_FLAG_MASK(opcode)		\
+	cpu_to_be32((opcode) << (SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT))
+
+#define KSZ_REGMAP_COMMON(width)					\
+	{								\
+		.val_bits = (width),					\
+		.reg_stride = (width) / 8,				\
+		.reg_bits = SPI_ADDR_SHIFT + SPI_ADDR_ALIGN,		\
+		.pad_bits = SPI_TURNAROUND_SHIFT,			\
+		.max_register = BIT(SPI_ADDR_SHIFT) - 1,		\
+		.cache_type = REGCACHE_NONE,				\
+		.read_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_RD),	\
+		.write_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_WR),	\
+		.reg_format_endian = REGMAP_ENDIAN_BIG,			\
+		.val_format_endian = REGMAP_ENDIAN_BIG			\
+	}
+
+static const struct regmap_config ksz9477_regmap_config[] = {
+	KSZ_REGMAP_COMMON(8),
+	KSZ_REGMAP_COMMON(16),
+	KSZ_REGMAP_COMMON(32),
+};
 
 static int ksz_spi_read8(struct ksz_device *dev, u32 reg, u8 *val)
 {
-	return ksz_spi_read(dev, reg, val, 1);
+	unsigned int value;
+	int ret = regmap_read(dev->regmap, reg, &value);
+
+	*val = value;
+	return ret;
 }
 
 static int ksz_spi_read16(struct ksz_device *dev, u32 reg, u16 *val)
 {
-	int ret = ksz_spi_read(dev, reg, (u8 *)val, 2);
+	int ret = regmap_bulk_read(dev->regmap, reg, val, 2);
 
 	if (!ret)
 		*val = be16_to_cpu(*val);
@@ -91,7 +67,7 @@ static int ksz_spi_read16(struct ksz_device *dev, u32 reg, u16 *val)
 
 static int ksz_spi_read32(struct ksz_device *dev, u32 reg, u32 *val)
 {
-	int ret = ksz_spi_read(dev, reg, (u8 *)val, 4);
+	int ret = regmap_bulk_read(dev->regmap, reg, val, 4);
 
 	if (!ret)
 		*val = be32_to_cpu(*val);
@@ -101,19 +77,19 @@ static int ksz_spi_read32(struct ksz_device *dev, u32 reg, u32 *val)
 
 static int ksz_spi_write8(struct ksz_device *dev, u32 reg, u8 value)
 {
-	return ksz_spi_write(dev, reg, &value, 1);
+	return regmap_write(dev->regmap, reg, value);
 }
 
 static int ksz_spi_write16(struct ksz_device *dev, u32 reg, u16 value)
 {
 	value = cpu_to_be16(value);
-	return ksz_spi_write(dev, reg, &value, 2);
+	return regmap_bulk_write(dev->regmap, reg, &value, 2);
 }
 
 static int ksz_spi_write32(struct ksz_device *dev, u32 reg, u32 value)
 {
 	value = cpu_to_be32(value);
-	return ksz_spi_write(dev, reg, &value, 4);
+	return regmap_bulk_write(dev->regmap, reg, &value, 4);
 }
 
 static const struct ksz_io_ops ksz9477_spi_ops = {
@@ -128,17 +104,27 @@ static const struct ksz_io_ops ksz9477_spi_ops = {
 static int ksz9477_spi_probe(struct spi_device *spi)
 {
 	struct ksz_device *dev;
-	int ret;
+	int i, ret;
 
 	dev = ksz_switch_alloc(&spi->dev, &ksz9477_spi_ops, spi);
 	if (!dev)
 		return -ENOMEM;
 
+	for (i = 0; i < ARRAY_SIZE(ksz9477_regmap_config); i++) {
+		dev->regmap[i] = devm_regmap_init_spi(spi,
+					&ksz9477_regmap_config[i]);
+		if (IS_ERR(dev->regmap[i])) {
+			ret = PTR_ERR(dev->regmap[i]);
+			dev_err(&spi->dev,
+				"Failed to initialize regmap%i: %d\n",
+				ksz9477_regmap_config[i].val_bits, ret);
+			return ret;
+		}
+	}
+
 	if (spi->dev.platform_data)
 		dev->pdata = spi->dev.platform_data;
 
-	dev->txbuf = devm_kzalloc(dev->dev, 4 + SPI_TX_BUF_LEN, GFP_KERNEL);
-
 	ret = ksz9477_switch_register(dev);
 
 	/* Main DSA driver may not be started yet. */
diff --git a/drivers/net/dsa/microchip/ksz_priv.h b/drivers/net/dsa/microchip/ksz_priv.h
index d3ddf98156bb..5ccc633fc766 100644
--- a/drivers/net/dsa/microchip/ksz_priv.h
+++ b/drivers/net/dsa/microchip/ksz_priv.h
@@ -57,6 +57,7 @@ struct ksz_device {
 	const struct ksz_dev_ops *dev_ops;
 
 	struct device *dev;
+	struct regmap *regmap[3];
 
 	void *priv;
 
@@ -82,8 +83,6 @@ struct ksz_device {
 
 	struct vlan_table *vlan_cache;
 
-	u8 *txbuf;
-
 	struct ksz_port *ports;
 	struct timer_list mib_read_timer;
 	struct work_struct mib_read;
-- 
2.20.1

[PATCH V3 08/10] net: dsa: microchip: Dispose of ksz_io_ops

[Marek Vasut]

Since the driver now uses regmap , get rid of ad-hoc ksz_io_ops
abstraction, which no longer has any meaning. Moreover, since regmap
has it's own locking, get rid of the register access mutex.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
---
V2: Use separate regmaps for 8/16/32bit registers
V3: - Rebase on next/master
    - Test on KSZ9477EVB
---
 drivers/net/dsa/microchip/ksz9477_spi.c | 57 +------------------------
 drivers/net/dsa/microchip/ksz_common.c  |  6 +--
 drivers/net/dsa/microchip/ksz_common.h  | 50 ++++++----------------
 drivers/net/dsa/microchip/ksz_priv.h    | 16 +------
 4 files changed, 17 insertions(+), 112 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477_spi.c b/drivers/net/dsa/microchip/ksz9477_spi.c
index 77e3cb100eae..8c8bf3237013 100644
--- a/drivers/net/dsa/microchip/ksz9477_spi.c
+++ b/drivers/net/dsa/microchip/ksz9477_spi.c
@@ -46,67 +46,12 @@ static const struct regmap_config ksz9477_regmap_config[] = {
 	KSZ_REGMAP_COMMON(32),
 };
 
-static int ksz_spi_read8(struct ksz_device *dev, u32 reg, u8 *val)
-{
-	unsigned int value;
-	int ret = regmap_read(dev->regmap, reg, &value);
-
-	*val = value;
-	return ret;
-}
-
-static int ksz_spi_read16(struct ksz_device *dev, u32 reg, u16 *val)
-{
-	int ret = regmap_bulk_read(dev->regmap, reg, val, 2);
-
-	if (!ret)
-		*val = be16_to_cpu(*val);
-
-	return ret;
-}
-
-static int ksz_spi_read32(struct ksz_device *dev, u32 reg, u32 *val)
-{
-	int ret = regmap_bulk_read(dev->regmap, reg, val, 4);
-
-	if (!ret)
-		*val = be32_to_cpu(*val);
-
-	return ret;
-}
-
-static int ksz_spi_write8(struct ksz_device *dev, u32 reg, u8 value)
-{
-	return regmap_write(dev->regmap, reg, value);
-}
-
-static int ksz_spi_write16(struct ksz_device *dev, u32 reg, u16 value)
-{
-	value = cpu_to_be16(value);
-	return regmap_bulk_write(dev->regmap, reg, &value, 2);
-}
-
-static int ksz_spi_write32(struct ksz_device *dev, u32 reg, u32 value)
-{
-	value = cpu_to_be32(value);
-	return regmap_bulk_write(dev->regmap, reg, &value, 4);
-}
-
-static const struct ksz_io_ops ksz9477_spi_ops = {
-	.read8 = ksz_spi_read8,
-	.read16 = ksz_spi_read16,
-	.read32 = ksz_spi_read32,
-	.write8 = ksz_spi_write8,
-	.write16 = ksz_spi_write16,
-	.write32 = ksz_spi_write32,
-};
-
 static int ksz9477_spi_probe(struct spi_device *spi)
 {
 	struct ksz_device *dev;
 	int i, ret;
 
-	dev = ksz_switch_alloc(&spi->dev, &ksz9477_spi_ops, spi);
+	dev = ksz_switch_alloc(&spi->dev, spi);
 	if (!dev)
 		return -ENOMEM;
 
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 978c59aa8efb..a3d2d67894bd 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -396,9 +396,7 @@ void ksz_disable_port(struct dsa_switch *ds, int port)
 }
 EXPORT_SYMBOL_GPL(ksz_disable_port);
 
-struct ksz_device *ksz_switch_alloc(struct device *base,
-				    const struct ksz_io_ops *ops,
-				    void *priv)
+struct ksz_device *ksz_switch_alloc(struct device *base, void *priv)
 {
 	struct dsa_switch *ds;
 	struct ksz_device *swdev;
@@ -416,7 +414,6 @@ struct ksz_device *ksz_switch_alloc(struct device *base,
 
 	swdev->ds = ds;
 	swdev->priv = priv;
-	swdev->ops = ops;
 
 	return swdev;
 }
@@ -442,7 +439,6 @@ int ksz_switch_register(struct ksz_device *dev,
 	}
 
 	mutex_init(&dev->dev_mutex);
-	mutex_init(&dev->reg_mutex);
 	mutex_init(&dev->stats_mutex);
 	mutex_init(&dev->alu_mutex);
 	mutex_init(&dev->vlan_mutex);
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index fe576a00facf..c3871ed9b097 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -7,6 +7,8 @@
 #ifndef __KSZ_COMMON_H
 #define __KSZ_COMMON_H
 
+#include <linux/regmap.h>
+
 void ksz_port_cleanup(struct ksz_device *dev, int port);
 void ksz_update_port_member(struct ksz_device *dev, int port);
 void ksz_init_mib_timer(struct ksz_device *dev);
@@ -41,68 +43,44 @@ void ksz_disable_port(struct dsa_switch *ds, int port);
 
 static inline int ksz_read8(struct ksz_device *dev, u32 reg, u8 *val)
 {
-	int ret;
-
-	mutex_lock(&dev->reg_mutex);
-	ret = dev->ops->read8(dev, reg, val);
-	mutex_unlock(&dev->reg_mutex);
+	unsigned int value;
+	int ret = regmap_read(dev->regmap[0], reg, &value);
 
+	*val = value;
 	return ret;
 }
 
 static inline int ksz_read16(struct ksz_device *dev, u32 reg, u16 *val)
 {
-	int ret;
-
-	mutex_lock(&dev->reg_mutex);
-	ret = dev->ops->read16(dev, reg, val);
-	mutex_unlock(&dev->reg_mutex);
+	unsigned int value;
+	int ret = regmap_read(dev->regmap[1], reg, &value);
 
+	*val = value;
 	return ret;
 }
 
 static inline int ksz_read32(struct ksz_device *dev, u32 reg, u32 *val)
 {
-	int ret;
-
-	mutex_lock(&dev->reg_mutex);
-	ret = dev->ops->read32(dev, reg, val);
-	mutex_unlock(&dev->reg_mutex);
+	unsigned int value;
+	int ret = regmap_read(dev->regmap[2], reg, &value);
 
+	*val = value;
 	return ret;
 }
 
 static inline int ksz_write8(struct ksz_device *dev, u32 reg, u8 value)
 {
-	int ret;
-
-	mutex_lock(&dev->reg_mutex);
-	ret = dev->ops->write8(dev, reg, value);
-	mutex_unlock(&dev->reg_mutex);
-
-	return ret;
+	return regmap_write(dev->regmap[0], reg, value);
 }
 
 static inline int ksz_write16(struct ksz_device *dev, u32 reg, u16 value)
 {
-	int ret;
-
-	mutex_lock(&dev->reg_mutex);
-	ret = dev->ops->write16(dev, reg, value);
-	mutex_unlock(&dev->reg_mutex);
-
-	return ret;
+	return regmap_write(dev->regmap[1], reg, value);
 }
 
 static inline int ksz_write32(struct ksz_device *dev, u32 reg, u32 value)
 {
-	int ret;
-
-	mutex_lock(&dev->reg_mutex);
-	ret = dev->ops->write32(dev, reg, value);
-	mutex_unlock(&dev->reg_mutex);
-
-	return ret;
+	return regmap_write(dev->regmap[2], reg, value);
 }
 
 static inline void ksz_pread8(struct ksz_device *dev, int port, int offset,
diff --git a/drivers/net/dsa/microchip/ksz_priv.h b/drivers/net/dsa/microchip/ksz_priv.h
index 5ccc633fc766..beacf0e40f42 100644
--- a/drivers/net/dsa/microchip/ksz_priv.h
+++ b/drivers/net/dsa/microchip/ksz_priv.h
@@ -14,8 +14,6 @@
 #include <linux/etherdevice.h>
 #include <net/dsa.h>
 
-struct ksz_io_ops;
-
 struct vlan_table {
 	u32 table[3];
 };
@@ -49,11 +47,9 @@ struct ksz_device {
 	const char *name;
 
 	struct mutex dev_mutex;		/* device access */
-	struct mutex reg_mutex;		/* register access */
 	struct mutex stats_mutex;	/* status access */
 	struct mutex alu_mutex;		/* ALU access */
 	struct mutex vlan_mutex;	/* vlan access */
-	const struct ksz_io_ops *ops;
 	const struct ksz_dev_ops *dev_ops;
 
 	struct device *dev;
@@ -101,15 +97,6 @@ struct ksz_device {
 	u16 port_mask;
 };
 
-struct ksz_io_ops {
-	int (*read8)(struct ksz_device *dev, u32 reg, u8 *value);
-	int (*read16)(struct ksz_device *dev, u32 reg, u16 *value);
-	int (*read32)(struct ksz_device *dev, u32 reg, u32 *value);
-	int (*write8)(struct ksz_device *dev, u32 reg, u8 value);
-	int (*write16)(struct ksz_device *dev, u32 reg, u16 value);
-	int (*write32)(struct ksz_device *dev, u32 reg, u32 value);
-};
-
 struct alu_struct {
 	/* entry 1 */
 	u8	is_static:1;
@@ -158,8 +145,7 @@ struct ksz_dev_ops {
 	void (*exit)(struct ksz_device *dev);
 };
 
-struct ksz_device *ksz_switch_alloc(struct device *base,
-				    const struct ksz_io_ops *ops, void *priv);
+struct ksz_device *ksz_switch_alloc(struct device *base, void *priv);
 int ksz_switch_register(struct ksz_device *dev,
 			const struct ksz_dev_ops *ops);
 void ksz_switch_remove(struct ksz_device *dev);
-- 
2.20.1

[PATCH V3 09/10] net: dsa: microchip: Factor out regmap config generation into common header

[Marek Vasut]

The regmap config tables are rather similar for various generations of
the KSZ8xxx/KSZ9xxx switches. Introduce a macro which allows generating
those tables without duplication. Note that $regalign parameter is not
used right now, but will be used in KSZ87xx series switches.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
---
V2: New patch
V3: - Rebase on next/master
    - Test on KSZ9477EVB
    - Increase regmap max register, to cover all switch registers
    - Make register swabbing configurable, to allow handling switches
      with only 16bit registers as well as switches with some 32bit ones
---
 drivers/net/dsa/microchip/ksz9477_spi.c | 29 +++-------------------
 drivers/net/dsa/microchip/ksz_common.h  | 32 +++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 26 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477_spi.c b/drivers/net/dsa/microchip/ksz9477_spi.c
index 8c8bf3237013..5a9e27b337a8 100644
--- a/drivers/net/dsa/microchip/ksz9477_spi.c
+++ b/drivers/net/dsa/microchip/ksz9477_spi.c
@@ -14,37 +14,14 @@
 #include <linux/spi/spi.h>
 
 #include "ksz_priv.h"
+#include "ksz_common.h"
 
 #define SPI_ADDR_SHIFT			24
 #define SPI_ADDR_ALIGN			3
 #define SPI_TURNAROUND_SHIFT		5
 
-/* SPI frame opcodes */
-#define KS_SPIOP_RD			3
-#define KS_SPIOP_WR			2
-
-#define KS_SPIOP_FLAG_MASK(opcode)		\
-	cpu_to_be32((opcode) << (SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT))
-
-#define KSZ_REGMAP_COMMON(width)					\
-	{								\
-		.val_bits = (width),					\
-		.reg_stride = (width) / 8,				\
-		.reg_bits = SPI_ADDR_SHIFT + SPI_ADDR_ALIGN,		\
-		.pad_bits = SPI_TURNAROUND_SHIFT,			\
-		.max_register = BIT(SPI_ADDR_SHIFT) - 1,		\
-		.cache_type = REGCACHE_NONE,				\
-		.read_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_RD),	\
-		.write_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_WR),	\
-		.reg_format_endian = REGMAP_ENDIAN_BIG,			\
-		.val_format_endian = REGMAP_ENDIAN_BIG			\
-	}
-
-static const struct regmap_config ksz9477_regmap_config[] = {
-	KSZ_REGMAP_COMMON(8),
-	KSZ_REGMAP_COMMON(16),
-	KSZ_REGMAP_COMMON(32),
-};
+KSZ_REGMAP_TABLE(ksz9477, 32, SPI_ADDR_SHIFT,
+		 SPI_TURNAROUND_SHIFT, SPI_ADDR_ALIGN);
 
 static int ksz9477_spi_probe(struct spi_device *spi)
 {
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index c3871ed9b097..78b5ab7db403 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -133,4 +133,36 @@ static inline u32 ksz_pread32_poll(struct ksz_poll_ctx *ctx)
 	return data;
 }
 
+/* Regmap tables generation */
+#define KSZ_SPI_OP_RD		3
+#define KSZ_SPI_OP_WR		2
+
+#define KSZ_SPI_OP_FLAG_MASK(opcode, swp, regbits, regpad)		\
+	cpu_to_be##swp((opcode) << ((regbits) + (regpad)))
+
+#define KSZ_REGMAP_ENTRY(width, swp, regbits, regpad, regalign)		\
+	{								\
+		.val_bits = (width),					\
+		.reg_stride = (width) / 8,				\
+		.reg_bits = (regbits) + (regalign),			\
+		.pad_bits = (regpad),					\
+		.max_register = BIT(regbits) - 1,			\
+		.cache_type = REGCACHE_NONE,				\
+		.read_flag_mask =					\
+			KSZ_SPI_OP_FLAG_MASK(KSZ_SPI_OP_RD, swp,	\
+					     regbits, regpad),		\
+		.write_flag_mask =					\
+			KSZ_SPI_OP_FLAG_MASK(KSZ_SPI_OP_WR, swp,	\
+					     regbits, regpad),		\
+		.reg_format_endian = REGMAP_ENDIAN_BIG,			\
+		.val_format_endian = REGMAP_ENDIAN_BIG			\
+	}
+
+#define KSZ_REGMAP_TABLE(ksz, swp, regbits, regpad, regalign)		\
+	static const struct regmap_config ksz##_regmap_config[] = {	\
+		KSZ_REGMAP_ENTRY(8, swp, (regbits), (regpad), (regalign)), \
+		KSZ_REGMAP_ENTRY(16, swp, (regbits), (regpad), (regalign)), \
+		KSZ_REGMAP_ENTRY(32, swp, (regbits), (regpad), (regalign)), \
+	}
+
 #endif
-- 
2.20.1

[PATCH V3 10/10] net: dsa: microchip: Replace ad-hoc bit manipulation with regmap

[Marek Vasut]

Regmap provides bit manipulation functions to set/clear bits, use those
insted of reimplementing them.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
---
V2: New patch
V3: - Rebase on next/master
    - Test on KSZ9477EVB
---
 drivers/net/dsa/microchip/ksz9477.c | 46 ++++-------------------------
 1 file changed, 6 insertions(+), 40 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 7d209fd9f26f..8f13dcc05a10 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -67,60 +67,26 @@ static const struct {
 
 static void ksz_cfg(struct ksz_device *dev, u32 addr, u8 bits, bool set)
 {
-	u8 data;
-
-	ksz_read8(dev, addr, &data);
-	if (set)
-		data |= bits;
-	else
-		data &= ~bits;
-	ksz_write8(dev, addr, data);
+	regmap_update_bits(dev->regmap[0], addr, bits, set ? bits : 0);
 }
 
 static void ksz_port_cfg(struct ksz_device *dev, int port, int offset, u8 bits,
 			 bool set)
 {
-	u32 addr;
-	u8 data;
-
-	addr = PORT_CTRL_ADDR(port, offset);
-	ksz_read8(dev, addr, &data);
-
-	if (set)
-		data |= bits;
-	else
-		data &= ~bits;
-
-	ksz_write8(dev, addr, data);
+	regmap_update_bits(dev->regmap[0], PORT_CTRL_ADDR(port, offset),
+			   bits, set ? bits : 0);
 }
 
 static void ksz9477_cfg32(struct ksz_device *dev, u32 addr, u32 bits, bool set)
 {
-	u32 data;
-
-	ksz_read32(dev, addr, &data);
-	if (set)
-		data |= bits;
-	else
-		data &= ~bits;
-	ksz_write32(dev, addr, data);
+	regmap_update_bits(dev->regmap[2], addr, bits, set ? bits : 0);
 }
 
 static void ksz9477_port_cfg32(struct ksz_device *dev, int port, int offset,
 			       u32 bits, bool set)
 {
-	u32 addr;
-	u32 data;
-
-	addr = PORT_CTRL_ADDR(port, offset);
-	ksz_read32(dev, addr, &data);
-
-	if (set)
-		data |= bits;
-	else
-		data &= ~bits;
-
-	ksz_write32(dev, addr, data);
+	regmap_update_bits(dev->regmap[2], PORT_CTRL_ADDR(port, offset),
+			   bits, set ? bits : 0);
 }
 
 static int ksz9477_wait_vlan_ctrl_ready(struct ksz_device *dev, u32 waiton,
-- 
2.20.1

Re: [PATCH V3 01/10] net: dsa: microchip: Remove ksz_{read,write}24()

[Andrew Lunn]

On Mon, Jun 24, 2019 at 12:34:59AM +0200, Marek Vasut wrote:
> These functions and callbacks are never used, remove them.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH V3 02/10] net: dsa: microchip: Remove ksz_{get,set}()

[Andrew Lunn]

On Mon, Jun 24, 2019 at 12:35:00AM +0200, Marek Vasut wrote:
> These functions and callbacks are never used, remove them.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH V3 03/10] net: dsa: microchip: Inline ksz_spi.h

[Andrew Lunn]

On Mon, Jun 24, 2019 at 12:35:01AM +0200, Marek Vasut wrote:
> The functions in the header file are static, and the header file is
> included from single C file, just inline the code into the C file.
> The bonus is that it's easier to spot further content to clean up.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH V3 04/10] net: dsa: microchip: Move ksz_cfg and ksz_port_cfg to ksz9477.c

[Andrew Lunn]

On Mon, Jun 24, 2019 at 12:35:02AM +0200, Marek Vasut wrote:
> These functions are only used by the KSZ9477 code, move them from
> the header into that code. Note that these functions will be soon
> replaced by regmap equivalents.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH V3 05/10] net: dsa: microchip: Use PORT_CTRL_ADDR() instead of indirect function call

[Andrew Lunn]

On Mon, Jun 24, 2019 at 12:35:03AM +0200, Marek Vasut wrote:
> The indirect function call to dev->dev_ops->get_port_addr() is expensive
> especially if called for every single register access, and only returns
> the value of PORT_CTRL_ADDR() macro. Use PORT_CTRL_ADDR() macro directly
> instead.

Hi Marek

Rather than change just one instance, it would be better to change
them all. And then remove dev_ops->get_port_addr().

     Andrew

Re: [PATCH V3 06/10] net: dsa: microchip: Factor out register access opcode generation

[Andrew Lunn]

On Mon, Jun 24, 2019 at 12:35:04AM +0200, Marek Vasut wrote:
> Factor out the code which sends out the register read/write opcodes
> to the switch, since the code differs in single bit between read and
> write.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH V3 07/10] net: dsa: microchip: Initial SPI regmap support

[kbuild test robot]

Hi Marek,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net/master]
[also build test WARNING on v5.2-rc6 next-20190621]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Marek-Vasut/net-dsa-microchip-Convert-to-regmap/20190625-021215
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-rc1-7-g2b96cd8-dirty
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

>> drivers/net/dsa/microchip/ksz9477_spi.c:44:9: sparse: sparse: incorrect type in initializer (different base types) @@    expected unsigned long read_flag_mask @@    got restricted __beunsigned long read_flag_mask @@
>> drivers/net/dsa/microchip/ksz9477_spi.c:44:9: sparse:    expected unsigned long read_flag_mask
   drivers/net/dsa/microchip/ksz9477_spi.c:44:9: sparse:    got restricted __be32 [usertype]
>> drivers/net/dsa/microchip/ksz9477_spi.c:44:9: sparse: sparse: incorrect type in initializer (different base types) @@    expected unsigned long write_flag_mask @@    got restricted __beunsigned long write_flag_mask @@
>> drivers/net/dsa/microchip/ksz9477_spi.c:44:9: sparse:    expected unsigned long write_flag_mask
   drivers/net/dsa/microchip/ksz9477_spi.c:44:9: sparse:    got restricted __be32 [usertype]
   drivers/net/dsa/microchip/ksz9477_spi.c:45:9: sparse: sparse: incorrect type in initializer (different base types) @@    expected unsigned long read_flag_mask @@    got restricted __beunsigned long read_flag_mask @@
   drivers/net/dsa/microchip/ksz9477_spi.c:45:9: sparse:    expected unsigned long read_flag_mask
   drivers/net/dsa/microchip/ksz9477_spi.c:45:9: sparse:    got restricted __be32 [usertype]
   drivers/net/dsa/microchip/ksz9477_spi.c:45:9: sparse: sparse: incorrect type in initializer (different base types) @@    expected unsigned long write_flag_mask @@    got restricted __beunsigned long write_flag_mask @@
   drivers/net/dsa/microchip/ksz9477_spi.c:45:9: sparse:    expected unsigned long write_flag_mask
   drivers/net/dsa/microchip/ksz9477_spi.c:45:9: sparse:    got restricted __be32 [usertype]
   drivers/net/dsa/microchip/ksz9477_spi.c:46:9: sparse: sparse: incorrect type in initializer (different base types) @@    expected unsigned long read_flag_mask @@    got restricted __beunsigned long read_flag_mask @@
   drivers/net/dsa/microchip/ksz9477_spi.c:46:9: sparse:    expected unsigned long read_flag_mask
   drivers/net/dsa/microchip/ksz9477_spi.c:46:9: sparse:    got restricted __be32 [usertype]
   drivers/net/dsa/microchip/ksz9477_spi.c:46:9: sparse: sparse: incorrect type in initializer (different base types) @@    expected unsigned long write_flag_mask @@    got restricted __beunsigned long write_flag_mask @@
   drivers/net/dsa/microchip/ksz9477_spi.c:46:9: sparse:    expected unsigned long write_flag_mask
   drivers/net/dsa/microchip/ksz9477_spi.c:46:9: sparse:    got restricted __be32 [usertype]
   drivers/net/dsa/microchip/ksz9477_spi.c:52:31: sparse: sparse: incorrect type in argument 1 (different base types) @@    expected struct regmap *map @@    got sstruct regmap *map @@
   drivers/net/dsa/microchip/ksz9477_spi.c:52:31: sparse:    expected struct regmap *map
   drivers/net/dsa/microchip/ksz9477_spi.c:52:31: sparse:    got struct regmap **
   drivers/net/dsa/microchip/ksz9477_spi.c:60:36: sparse: sparse: incorrect type in argument 1 (different base types) @@    expected struct regmap *map @@    got sstruct regmap *map @@
   drivers/net/dsa/microchip/ksz9477_spi.c:60:36: sparse:    expected struct regmap *map
   drivers/net/dsa/microchip/ksz9477_spi.c:60:36: sparse:    got struct regmap **
   drivers/net/dsa/microchip/ksz9477_spi.c:63:24: sparse: sparse: cast to restricted __be16
   drivers/net/dsa/microchip/ksz9477_spi.c:63:24: sparse: sparse: cast to restricted __be16
   drivers/net/dsa/microchip/ksz9477_spi.c:63:24: sparse: sparse: cast to restricted __be16
   drivers/net/dsa/microchip/ksz9477_spi.c:63:24: sparse: sparse: cast to restricted __be16
   drivers/net/dsa/microchip/ksz9477_spi.c:70:36: sparse: sparse: incorrect type in argument 1 (different base types) @@    expected struct regmap *map @@    got sstruct regmap *map @@
   drivers/net/dsa/microchip/ksz9477_spi.c:70:36: sparse:    expected struct regmap *map
   drivers/net/dsa/microchip/ksz9477_spi.c:70:36: sparse:    got struct regmap **
   drivers/net/dsa/microchip/ksz9477_spi.c:73:24: sparse: sparse: cast to restricted __be32
   drivers/net/dsa/microchip/ksz9477_spi.c:73:24: sparse: sparse: cast to restricted __be32
   drivers/net/dsa/microchip/ksz9477_spi.c:73:24: sparse: sparse: cast to restricted __be32
   drivers/net/dsa/microchip/ksz9477_spi.c:73:24: sparse: sparse: cast to restricted __be32
   drivers/net/dsa/microchip/ksz9477_spi.c:73:24: sparse: sparse: cast to restricted __be32
   drivers/net/dsa/microchip/ksz9477_spi.c:73:24: sparse: sparse: cast to restricted __be32
   drivers/net/dsa/microchip/ksz9477_spi.c:80:29: sparse: sparse: incorrect type in argument 1 (different base types) @@    expected struct regmap *map @@    got sstruct regmap *map @@
   drivers/net/dsa/microchip/ksz9477_spi.c:80:29: sparse:    expected struct regmap *map
   drivers/net/dsa/microchip/ksz9477_spi.c:80:29: sparse:    got struct regmap **
   drivers/net/dsa/microchip/ksz9477_spi.c:85:15: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned short [usertype] value @@    got resunsigned short [usertype] value @@
   drivers/net/dsa/microchip/ksz9477_spi.c:85:15: sparse:    expected unsigned short [usertype] value
   drivers/net/dsa/microchip/ksz9477_spi.c:85:15: sparse:    got restricted __be16 [usertype]
   drivers/net/dsa/microchip/ksz9477_spi.c:86:34: sparse: sparse: incorrect type in argument 1 (different base types) @@    expected struct regmap *map @@    got sstruct regmap *map @@
   drivers/net/dsa/microchip/ksz9477_spi.c:86:34: sparse:    expected struct regmap *map
   drivers/net/dsa/microchip/ksz9477_spi.c:86:34: sparse:    got struct regmap **
   drivers/net/dsa/microchip/ksz9477_spi.c:91:15: sparse: sparse: incorrect type in assignment (different base types) @@    expected unsigned int [usertype] value @@    got restrunsigned int [usertype] value @@
   drivers/net/dsa/microchip/ksz9477_spi.c:91:15: sparse:    expected unsigned int [usertype] value
   drivers/net/dsa/microchip/ksz9477_spi.c:91:15: sparse:    got restricted __be32 [usertype]
   drivers/net/dsa/microchip/ksz9477_spi.c:92:34: sparse: sparse: incorrect type in argument 1 (different base types) @@    expected struct regmap *map @@    got sstruct regmap *map @@
   drivers/net/dsa/microchip/ksz9477_spi.c:92:34: sparse:    expected struct regmap *map
   drivers/net/dsa/microchip/ksz9477_spi.c:92:34: sparse:    got struct regmap **

vim +44 drivers/net/dsa/microchip/ksz9477_spi.c

    25	
    26	#define KS_SPIOP_FLAG_MASK(opcode)		\
    27		cpu_to_be32((opcode) << (SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT))
    28	
    29	#define KSZ_REGMAP_COMMON(width)					\
    30		{								\
    31			.val_bits = (width),					\
    32			.reg_stride = (width) / 8,				\
    33			.reg_bits = SPI_ADDR_SHIFT + SPI_ADDR_ALIGN,		\
    34			.pad_bits = SPI_TURNAROUND_SHIFT,			\
    35			.max_register = BIT(SPI_ADDR_SHIFT) - 1,		\
    36			.cache_type = REGCACHE_NONE,				\
    37			.read_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_RD),	\
    38			.write_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_WR),	\
    39			.reg_format_endian = REGMAP_ENDIAN_BIG,			\
    40			.val_format_endian = REGMAP_ENDIAN_BIG			\
    41		}
    42	
    43	static const struct regmap_config ksz9477_regmap_config[] = {
  > 44		KSZ_REGMAP_COMMON(8),
    45		KSZ_REGMAP_COMMON(16),
    46		KSZ_REGMAP_COMMON(32),
    47	};
    48	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH V3 07/10] net: dsa: microchip: Initial SPI regmap support

[Marek Vasut]

On 6/24/19 12:35 AM, Marek Vasut wrote:
> Add basic SPI regmap support into the driver.
> 
> Previous patches unconver that ksz_spi_write() is always ever called
> with len = 1, 2 or 4. We can thus drop the if (len > SPI_TX_BUF_LEN)
> check and we can also drop the allocation of the txbuf which is part
> of the driver data and wastes 256 bytes for no reason. Regmap covers
> the whole thing now.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Tristram Ha <Tristram.Ha@microchip.com>
> Cc: Woojung Huh <Woojung.Huh@microchip.com>

[...]

> +#define KS_SPIOP_FLAG_MASK(opcode)		\
> +	cpu_to_be32((opcode) << (SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT))

So the robot is complaining about this. I believe this is correct, as
the mask should be in native endianness on the register and NOT the
native endianness of the CPU.

I think a cast would help here, e.g.:
-	cpu_to_be32((opcode) << (SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT))
-	(__force unsigned long)cpu_to_be32((opcode) << (SPI_ADDR_SHIFT +
SPI_TURNAROUND_SHIFT))

Does this make sense ?

> +#define KSZ_REGMAP_COMMON(width)					\
> +	{								\
> +		.val_bits = (width),					\
> +		.reg_stride = (width) / 8,				\
> +		.reg_bits = SPI_ADDR_SHIFT + SPI_ADDR_ALIGN,		\
> +		.pad_bits = SPI_TURNAROUND_SHIFT,			\
> +		.max_register = BIT(SPI_ADDR_SHIFT) - 1,		\
> +		.cache_type = REGCACHE_NONE,				\
> +		.read_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_RD),	\
> +		.write_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_WR),	\

[...]

-- 
Best regards,
Marek Vasut

Re: [PATCH V3 05/10] net: dsa: microchip: Use PORT_CTRL_ADDR() instead of indirect function call

[Marek Vasut]

On 6/24/19 5:20 AM, Andrew Lunn wrote:
> On Mon, Jun 24, 2019 at 12:35:03AM +0200, Marek Vasut wrote:
>> The indirect function call to dev->dev_ops->get_port_addr() is expensive
>> especially if called for every single register access, and only returns
>> the value of PORT_CTRL_ADDR() macro. Use PORT_CTRL_ADDR() macro directly
>> instead.
> 
> Hi Marek
> 
> Rather than change just one instance, it would be better to change
> them all. And then remove dev_ops->get_port_addr().

So that actually doesn't work. The rest of the calls are in common code
(ksz_common.h) and I plan to add the KSZ8795, which has different
spacing between the ports, so those have to stay.

Although, depending on how things look, I will do more regmap cleanups,
the driver needs it. Since I have the KSZ9477 devkit, that also makes it
much easier to test the changes. I think inlining those custom accessors
would be high on the list, because they are just a mess. But that's for
another series.

-- 
Best regards,
Marek Vasut

Re: [PATCH V3 07/10] net: dsa: microchip: Initial SPI regmap support

[Vladimir Oltean]

On Tue, 25 Jun 2019 at 01:17, Marek Vasut <marex@denx.de> wrote:
>
> On 6/24/19 12:35 AM, Marek Vasut wrote:
> > Add basic SPI regmap support into the driver.
> >
> > Previous patches unconver that ksz_spi_write() is always ever called
> > with len = 1, 2 or 4. We can thus drop the if (len > SPI_TX_BUF_LEN)
> > check and we can also drop the allocation of the txbuf which is part
> > of the driver data and wastes 256 bytes for no reason. Regmap covers
> > the whole thing now.
> >
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Andrew Lunn <andrew@lunn.ch>
> > Cc: Florian Fainelli <f.fainelli@gmail.com>
> > Cc: Tristram Ha <Tristram.Ha@microchip.com>
> > Cc: Woojung Huh <Woojung.Huh@microchip.com>
>
> [...]
>
> > +#define KS_SPIOP_FLAG_MASK(opcode)           \
> > +     cpu_to_be32((opcode) << (SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT))
>
> So the robot is complaining about this. I believe this is correct, as
> the mask should be in native endianness on the register and NOT the
> native endianness of the CPU.
>
> I think a cast would help here, e.g.:
> -       cpu_to_be32((opcode) << (SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT))
> -       (__force unsigned long)cpu_to_be32((opcode) << (SPI_ADDR_SHIFT +
> SPI_TURNAROUND_SHIFT))
>
> Does this make sense ?
>
> > +#define KSZ_REGMAP_COMMON(width)                                     \
> > +     {                                                               \
> > +             .val_bits = (width),                                    \
> > +             .reg_stride = (width) / 8,                              \
> > +             .reg_bits = SPI_ADDR_SHIFT + SPI_ADDR_ALIGN,            \
> > +             .pad_bits = SPI_TURNAROUND_SHIFT,                       \
> > +             .max_register = BIT(SPI_ADDR_SHIFT) - 1,                \
> > +             .cache_type = REGCACHE_NONE,                            \
> > +             .read_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_RD),      \
> > +             .write_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_WR),     \
>
> [...]
>
> --
> Best regards,
> Marek Vasut

Hi Marek,

I saw SPI buffers and endianness and got triggered :)
Would it make sense to take a look at CONFIG_PACKING for the KSZ9477 driver?
I don't know how bad the field alignment issue is on that hardware,
but on SJA1105 it was such a disaster that I couldn't have managed it
any other way.

Regards,
-Vladimir

Re: [PATCH V3 07/10] net: dsa: microchip: Initial SPI regmap support

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 6104 bytes --]

Hi Marek,

I love your patch! Yet something to improve:

[auto build test ERROR on net/master]
[also build test ERROR on v5.2-rc6 next-20190621]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Marek-Vasut/net-dsa-microchip-Convert-to-regmap/20190625-021215
config: x86_64-randconfig-ne0-06250702 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: drivers/base/regmap/regmap-spi.o: in function `regmap_spi_read':
>> drivers/base/regmap/regmap-spi.c:98: undefined reference to `spi_write_then_read'
   ld: drivers/base/regmap/regmap-spi.o: in function `regmap_spi_async_write':
>> drivers/base/regmap/regmap-spi.c:77: undefined reference to `spi_async'
   ld: drivers/base/regmap/regmap-spi.o: in function `spi_sync_transfer':
   include/linux/spi/spi.h:1087: undefined reference to `spi_sync'
   ld: drivers/base/regmap/regmap-spi.o: in function `regmap_spi_gather_write':
>> drivers/base/regmap/regmap-spi.c:50: undefined reference to `spi_sync'

vim +98 drivers/base/regmap/regmap-spi.c

a676f083 Mark Brown     2011-05-12   35  
0135bbcc Stephen Warren 2012-04-04   36  static int regmap_spi_gather_write(void *context,
a676f083 Mark Brown     2011-05-12   37  				   const void *reg, size_t reg_len,
a676f083 Mark Brown     2011-05-12   38  				   const void *val, size_t val_len)
a676f083 Mark Brown     2011-05-12   39  {
0135bbcc Stephen Warren 2012-04-04   40  	struct device *dev = context;
a676f083 Mark Brown     2011-05-12   41  	struct spi_device *spi = to_spi_device(dev);
a676f083 Mark Brown     2011-05-12   42  	struct spi_message m;
a676f083 Mark Brown     2011-05-12   43  	struct spi_transfer t[2] = { { .tx_buf = reg, .len = reg_len, },
a676f083 Mark Brown     2011-05-12   44  				     { .tx_buf = val, .len = val_len, }, };
a676f083 Mark Brown     2011-05-12   45  
a676f083 Mark Brown     2011-05-12   46  	spi_message_init(&m);
a676f083 Mark Brown     2011-05-12   47  	spi_message_add_tail(&t[0], &m);
a676f083 Mark Brown     2011-05-12   48  	spi_message_add_tail(&t[1], &m);
a676f083 Mark Brown     2011-05-12   49  
a676f083 Mark Brown     2011-05-12  @50  	return spi_sync(spi, &m);
a676f083 Mark Brown     2011-05-12   51  }
a676f083 Mark Brown     2011-05-12   52  
e0356dfe Mark Brown     2013-01-29   53  static int regmap_spi_async_write(void *context,
e0356dfe Mark Brown     2013-01-29   54  				  const void *reg, size_t reg_len,
e0356dfe Mark Brown     2013-01-29   55  				  const void *val, size_t val_len,
e0356dfe Mark Brown     2013-01-29   56  				  struct regmap_async *a)
e0356dfe Mark Brown     2013-01-29   57  {
e0356dfe Mark Brown     2013-01-29   58  	struct regmap_async_spi *async = container_of(a,
e0356dfe Mark Brown     2013-01-29   59  						      struct regmap_async_spi,
e0356dfe Mark Brown     2013-01-29   60  						      core);
e0356dfe Mark Brown     2013-01-29   61  	struct device *dev = context;
e0356dfe Mark Brown     2013-01-29   62  	struct spi_device *spi = to_spi_device(dev);
e0356dfe Mark Brown     2013-01-29   63  
e0356dfe Mark Brown     2013-01-29   64  	async->t[0].tx_buf = reg;
e0356dfe Mark Brown     2013-01-29   65  	async->t[0].len = reg_len;
e0356dfe Mark Brown     2013-01-29   66  	async->t[1].tx_buf = val;
e0356dfe Mark Brown     2013-01-29   67  	async->t[1].len = val_len;
e0356dfe Mark Brown     2013-01-29   68  
e0356dfe Mark Brown     2013-01-29   69  	spi_message_init(&async->m);
e0356dfe Mark Brown     2013-01-29   70  	spi_message_add_tail(&async->t[0], &async->m);
cd1b9dd0 Mark Brown     2013-10-10   71  	if (val)
e0356dfe Mark Brown     2013-01-29   72  		spi_message_add_tail(&async->t[1], &async->m);
e0356dfe Mark Brown     2013-01-29   73  
e0356dfe Mark Brown     2013-01-29   74  	async->m.complete = regmap_spi_complete;
e0356dfe Mark Brown     2013-01-29   75  	async->m.context = async;
e0356dfe Mark Brown     2013-01-29   76  
e0356dfe Mark Brown     2013-01-29  @77  	return spi_async(spi, &async->m);
e0356dfe Mark Brown     2013-01-29   78  }
e0356dfe Mark Brown     2013-01-29   79  
e0356dfe Mark Brown     2013-01-29   80  static struct regmap_async *regmap_spi_async_alloc(void)
e0356dfe Mark Brown     2013-01-29   81  {
e0356dfe Mark Brown     2013-01-29   82  	struct regmap_async_spi *async_spi;
e0356dfe Mark Brown     2013-01-29   83  
e0356dfe Mark Brown     2013-01-29   84  	async_spi = kzalloc(sizeof(*async_spi), GFP_KERNEL);
95601d65 Mark Brown     2013-02-05   85  	if (!async_spi)
95601d65 Mark Brown     2013-02-05   86  		return NULL;
e0356dfe Mark Brown     2013-01-29   87  
e0356dfe Mark Brown     2013-01-29   88  	return &async_spi->core;
e0356dfe Mark Brown     2013-01-29   89  }
e0356dfe Mark Brown     2013-01-29   90  
0135bbcc Stephen Warren 2012-04-04   91  static int regmap_spi_read(void *context,
a676f083 Mark Brown     2011-05-12   92  			   const void *reg, size_t reg_size,
a676f083 Mark Brown     2011-05-12   93  			   void *val, size_t val_size)
a676f083 Mark Brown     2011-05-12   94  {
0135bbcc Stephen Warren 2012-04-04   95  	struct device *dev = context;
a676f083 Mark Brown     2011-05-12   96  	struct spi_device *spi = to_spi_device(dev);
a676f083 Mark Brown     2011-05-12   97  
a676f083 Mark Brown     2011-05-12  @98  	return spi_write_then_read(spi, reg, reg_size, val, val_size);
a676f083 Mark Brown     2011-05-12   99  }
a676f083 Mark Brown     2011-05-12  100  

:::::: The code at line 98 was first introduced by commit
:::::: a676f083068b08e676c557279effbd7f4d590812 regmap: Add SPI bus support

:::::: TO: Mark Brown <broonie@opensource.wolfsonmicro.com>
:::::: CC: Mark Brown <broonie@opensource.wolfsonmicro.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36625 bytes --]

Re: [PATCH V3 07/10] net: dsa: microchip: Initial SPI regmap support

[Marek Vasut]

On 6/25/19 1:59 AM, Vladimir Oltean wrote:
> On Tue, 25 Jun 2019 at 01:17, Marek Vasut <marex@denx.de> wrote:
>>
>> On 6/24/19 12:35 AM, Marek Vasut wrote:
>>> Add basic SPI regmap support into the driver.
>>>
>>> Previous patches unconver that ksz_spi_write() is always ever called
>>> with len = 1, 2 or 4. We can thus drop the if (len > SPI_TX_BUF_LEN)
>>> check and we can also drop the allocation of the txbuf which is part
>>> of the driver data and wastes 256 bytes for no reason. Regmap covers
>>> the whole thing now.
>>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> Cc: Andrew Lunn <andrew@lunn.ch>
>>> Cc: Florian Fainelli <f.fainelli@gmail.com>
>>> Cc: Tristram Ha <Tristram.Ha@microchip.com>
>>> Cc: Woojung Huh <Woojung.Huh@microchip.com>
>>
>> [...]
>>
>>> +#define KS_SPIOP_FLAG_MASK(opcode)           \
>>> +     cpu_to_be32((opcode) << (SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT))
>>
>> So the robot is complaining about this. I believe this is correct, as
>> the mask should be in native endianness on the register and NOT the
>> native endianness of the CPU.
>>
>> I think a cast would help here, e.g.:
>> -       cpu_to_be32((opcode) << (SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT))
>> -       (__force unsigned long)cpu_to_be32((opcode) << (SPI_ADDR_SHIFT +
>> SPI_TURNAROUND_SHIFT))
>>
>> Does this make sense ?
>>
>>> +#define KSZ_REGMAP_COMMON(width)                                     \
>>> +     {                                                               \
>>> +             .val_bits = (width),                                    \
>>> +             .reg_stride = (width) / 8,                              \
>>> +             .reg_bits = SPI_ADDR_SHIFT + SPI_ADDR_ALIGN,            \
>>> +             .pad_bits = SPI_TURNAROUND_SHIFT,                       \
>>> +             .max_register = BIT(SPI_ADDR_SHIFT) - 1,                \
>>> +             .cache_type = REGCACHE_NONE,                            \
>>> +             .read_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_RD),      \
>>> +             .write_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_WR),     \
>>
>> [...]
>>
>> --
>> Best regards,
>> Marek Vasut
> 
> Hi Marek,
> 
> I saw SPI buffers and endianness and got triggered :)
> Would it make sense to take a look at CONFIG_PACKING for the KSZ9477 driver?
> I don't know how bad the field alignment issue is on that hardware,
> but on SJA1105 it was such a disaster that I couldn't have managed it
> any other way.

How does that help me here ? All I really need is a static constant mask
for the register/flags , 32bit for KSZ9xxx and 16bit for KSZ87xx. I
don't need any dynamic stuff, luckily.

But I'm glad to see TJA1105 driver mainline :)

-- 
Best regards,
Marek Vasut

Re: [PATCH V3 07/10] net: dsa: microchip: Initial SPI regmap support

[Vladimir Oltean]

On Tue, 25 Jun 2019 at 15:06, Marek Vasut <marex@denx.de> wrote:
>
> On 6/25/19 1:59 AM, Vladimir Oltean wrote:
> > On Tue, 25 Jun 2019 at 01:17, Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 6/24/19 12:35 AM, Marek Vasut wrote:
> >>> Add basic SPI regmap support into the driver.
> >>>
> >>> Previous patches unconver that ksz_spi_write() is always ever called
> >>> with len = 1, 2 or 4. We can thus drop the if (len > SPI_TX_BUF_LEN)
> >>> check and we can also drop the allocation of the txbuf which is part
> >>> of the driver data and wastes 256 bytes for no reason. Regmap covers
> >>> the whole thing now.
> >>>
> >>> Signed-off-by: Marek Vasut <marex@denx.de>
> >>> Cc: Andrew Lunn <andrew@lunn.ch>
> >>> Cc: Florian Fainelli <f.fainelli@gmail.com>
> >>> Cc: Tristram Ha <Tristram.Ha@microchip.com>
> >>> Cc: Woojung Huh <Woojung.Huh@microchip.com>
> >>
> >> [...]
> >>
> >>> +#define KS_SPIOP_FLAG_MASK(opcode)           \
> >>> +     cpu_to_be32((opcode) << (SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT))
> >>
> >> So the robot is complaining about this. I believe this is correct, as
> >> the mask should be in native endianness on the register and NOT the
> >> native endianness of the CPU.
> >>
> >> I think a cast would help here, e.g.:
> >> -       cpu_to_be32((opcode) << (SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT))
> >> -       (__force unsigned long)cpu_to_be32((opcode) << (SPI_ADDR_SHIFT +
> >> SPI_TURNAROUND_SHIFT))
> >>
> >> Does this make sense ?
> >>
> >>> +#define KSZ_REGMAP_COMMON(width)                                     \
> >>> +     {                                                               \
> >>> +             .val_bits = (width),                                    \
> >>> +             .reg_stride = (width) / 8,                              \
> >>> +             .reg_bits = SPI_ADDR_SHIFT + SPI_ADDR_ALIGN,            \
> >>> +             .pad_bits = SPI_TURNAROUND_SHIFT,                       \
> >>> +             .max_register = BIT(SPI_ADDR_SHIFT) - 1,                \
> >>> +             .cache_type = REGCACHE_NONE,                            \
> >>> +             .read_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_RD),      \
> >>> +             .write_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_WR),     \
> >>
> >> [...]
> >>
> >> --
> >> Best regards,
> >> Marek Vasut
> >
> > Hi Marek,
> >
> > I saw SPI buffers and endianness and got triggered :)
> > Would it make sense to take a look at CONFIG_PACKING for the KSZ9477 driver?
> > I don't know how bad the field alignment issue is on that hardware,
> > but on SJA1105 it was such a disaster that I couldn't have managed it
> > any other way.
>
> How does that help me here ? All I really need is a static constant mask
> for the register/flags , 32bit for KSZ9xxx and 16bit for KSZ87xx. I
> don't need any dynamic stuff, luckily.
>

Ok. I was thinking *instead of* regmap.
On the SJA1105 I couldn't use a spi regmap because:
* the enum regmap_endian wasn't enough to describe the hardware's bit
layout (it is big endian, but high-order and low-order 32 bit words
are swapped)
* it doesn't really expose a well-defined register map, but rather,
you're supposed to dynamically construct a TLV-type buffer and write
it starting with a fixed address.
I just thought you were facing some of these problems as well with
regmap. If not, that's perfectly fine!

> But I'm glad to see TJA1105 driver mainline :)
>
> --
> Best regards,
> Marek Vasut

Regards,
-Vladimir

Re: [PATCH V3 07/10] net: dsa: microchip: Initial SPI regmap support

[Marek Vasut]

On 6/25/19 2:40 PM, Vladimir Oltean wrote:
> On Tue, 25 Jun 2019 at 15:06, Marek Vasut <marex@denx.de> wrote:
>>
>> On 6/25/19 1:59 AM, Vladimir Oltean wrote:
>>> On Tue, 25 Jun 2019 at 01:17, Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> On 6/24/19 12:35 AM, Marek Vasut wrote:
>>>>> Add basic SPI regmap support into the driver.
>>>>>
>>>>> Previous patches unconver that ksz_spi_write() is always ever called
>>>>> with len = 1, 2 or 4. We can thus drop the if (len > SPI_TX_BUF_LEN)
>>>>> check and we can also drop the allocation of the txbuf which is part
>>>>> of the driver data and wastes 256 bytes for no reason. Regmap covers
>>>>> the whole thing now.
>>>>>
>>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>>> Cc: Andrew Lunn <andrew@lunn.ch>
>>>>> Cc: Florian Fainelli <f.fainelli@gmail.com>
>>>>> Cc: Tristram Ha <Tristram.Ha@microchip.com>
>>>>> Cc: Woojung Huh <Woojung.Huh@microchip.com>
>>>>
>>>> [...]
>>>>
>>>>> +#define KS_SPIOP_FLAG_MASK(opcode)           \
>>>>> +     cpu_to_be32((opcode) << (SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT))
>>>>
>>>> So the robot is complaining about this. I believe this is correct, as
>>>> the mask should be in native endianness on the register and NOT the
>>>> native endianness of the CPU.
>>>>
>>>> I think a cast would help here, e.g.:
>>>> -       cpu_to_be32((opcode) << (SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT))
>>>> -       (__force unsigned long)cpu_to_be32((opcode) << (SPI_ADDR_SHIFT +
>>>> SPI_TURNAROUND_SHIFT))
>>>>
>>>> Does this make sense ?
>>>>
>>>>> +#define KSZ_REGMAP_COMMON(width)                                     \
>>>>> +     {                                                               \
>>>>> +             .val_bits = (width),                                    \
>>>>> +             .reg_stride = (width) / 8,                              \
>>>>> +             .reg_bits = SPI_ADDR_SHIFT + SPI_ADDR_ALIGN,            \
>>>>> +             .pad_bits = SPI_TURNAROUND_SHIFT,                       \
>>>>> +             .max_register = BIT(SPI_ADDR_SHIFT) - 1,                \
>>>>> +             .cache_type = REGCACHE_NONE,                            \
>>>>> +             .read_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_RD),      \
>>>>> +             .write_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_WR),     \
>>>>
>>>> [...]
>>>>
>>>> --
>>>> Best regards,
>>>> Marek Vasut
>>>
>>> Hi Marek,
>>>
>>> I saw SPI buffers and endianness and got triggered :)
>>> Would it make sense to take a look at CONFIG_PACKING for the KSZ9477 driver?
>>> I don't know how bad the field alignment issue is on that hardware,
>>> but on SJA1105 it was such a disaster that I couldn't have managed it
>>> any other way.
>>
>> How does that help me here ? All I really need is a static constant mask
>> for the register/flags , 32bit for KSZ9xxx and 16bit for KSZ87xx. I
>> don't need any dynamic stuff, luckily.
>>
> 
> Ok. I was thinking *instead of* regmap.
> On the SJA1105 I couldn't use a spi regmap because:
> * the enum regmap_endian wasn't enough to describe the hardware's bit
> layout (it is big endian, but high-order and low-order 32 bit words
> are swapped)

Oooooof. Is the switch development some competition in register layout
braindeath ? What's it gonna be next ... I was gonna suggest something,
but then reconsidered, better not give them any wild ideas :-)

> * it doesn't really expose a well-defined register map, but rather,
> you're supposed to dynamically construct a TLV-type buffer and write
> it starting with a fixed address.
> I just thought you were facing some of these problems as well with
> regmap. If not, that's perfectly fine!

All right, that's not the case here, whew.

-- 
Best regards,
Marek Vasut