[PATCH RFC 0/4] Convert KSZ9477 SPI driver to use regmap

[PATCH RFC 0/4] Convert KSZ9477 SPI driver to use regmap

[Tristram.Ha]

From: Tristram Ha <Tristram.Ha@microchip.com>

Convert KSZ9477 SPI driver to use regmap mechanism so that an I2C
driver can be easily added.

Original regmap implementation was submitted by Marek Vasut.  Modified
and verified the implementation on real hardware.

Tristram Ha (4):
  net: dsa: microchip: convert KSZ9477 SPI driver to use regmap
  net: dsa: microchip: Use regmap_update_bits
  net: dsa: microchip: remove ksz9477_get_port_addr
  net: dsa: microchip: remove ksz_spi.h

 drivers/net/dsa/microchip/Kconfig       |   1 +
 drivers/net/dsa/microchip/ksz9477.c     |  34 +--------
 drivers/net/dsa/microchip/ksz9477_spi.c | 131 ++++++++++----------------------
 drivers/net/dsa/microchip/ksz_common.c  |   9 +--
 drivers/net/dsa/microchip/ksz_common.h  | 113 ++++++++-------------------
 drivers/net/dsa/microchip/ksz_priv.h    |  29 +------
 drivers/net/dsa/microchip/ksz_spi.h     |  69 -----------------
 7 files changed, 84 insertions(+), 302 deletions(-)
 delete mode 100644 drivers/net/dsa/microchip/ksz_spi.h

-- 
1.9.1

[PATCH RFC 1/4] net: dsa: microchip: convert KSZ9477 SPI driver to use regmap

[Tristram.Ha]

From: Tristram Ha <Tristram.Ha@microchip.com>

Convert KSZ9477 SPI driver to use regmap mechanism so that an I2C driver
can be easily added.

KSZ9477 SPI driver uses a 32-bit SPI command containing a 24-bit address
to access registers in 8-bit.  The address is automatically increased to
next register.  In theory all registers can be read in one transfer as
long as the buffer is enough.  Therefore the regmap_raw_read and
regmap_raw_write functions are used for basic 8-bit access.  Two more
regmap configurations are used for 16-bit and 32-bit accesses just that
regmap_update_bits can be used.

All variables and functions associated with SPI access are removed.

Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
---
 drivers/net/dsa/microchip/Kconfig       |   1 +
 drivers/net/dsa/microchip/ksz9477_spi.c | 131 ++++++++++----------------------
 drivers/net/dsa/microchip/ksz_common.c  |   9 +--
 drivers/net/dsa/microchip/ksz_common.h  | 113 ++++++++-------------------
 drivers/net/dsa/microchip/ksz_priv.h    |  28 +------
 5 files changed, 80 insertions(+), 202 deletions(-)

diff --git a/drivers/net/dsa/microchip/Kconfig b/drivers/net/dsa/microchip/Kconfig
index bea29fd..385b93f 100644
--- a/drivers/net/dsa/microchip/Kconfig
+++ b/drivers/net/dsa/microchip/Kconfig
@@ -12,5 +12,6 @@ menuconfig NET_DSA_MICROCHIP_KSZ9477
 config NET_DSA_MICROCHIP_KSZ9477_SPI
 	tristate "KSZ9477 series SPI connected switch driver"
 	depends on NET_DSA_MICROCHIP_KSZ9477 && SPI
+	select REGMAP_SPI
 	help
 	  Select to enable support for registering switches configured through SPI.
diff --git a/drivers/net/dsa/microchip/ksz9477_spi.c b/drivers/net/dsa/microchip/ksz9477_spi.c
index d757ba1..d5f0aaa 100644
--- a/drivers/net/dsa/microchip/ksz9477_spi.c
+++ b/drivers/net/dsa/microchip/ksz9477_spi.c
@@ -2,18 +2,14 @@
 /*
  * Microchip KSZ9477 series register access through SPI
  *
- * Copyright (C) 2017-2018 Microchip Technology Inc.
+ * Copyright (C) 2017-2019 Microchip Technology Inc.
  */
 
-#include <asm/unaligned.h>
-
-#include <linux/delay.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/spi/spi.h>
 
 #include "ksz_priv.h"
-#include "ksz_spi.h"
 
 /* SPI frame opcodes */
 #define KS_SPIOP_RD			3
@@ -23,106 +19,61 @@
 #define SPI_ADDR_MASK			(BIT(SPI_ADDR_SHIFT) - 1)
 #define SPI_TURNAROUND_SHIFT		5
 
-/* 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)
-{
-	u32 txbuf;
-	int ret;
-
-	txbuf = reg & SPI_ADDR_MASK;
-	txbuf |= KS_SPIOP_RD << SPI_ADDR_SHIFT;
-	txbuf <<= SPI_TURNAROUND_SHIFT;
-	txbuf = cpu_to_be32(txbuf);
-
-	ret = spi_write_then_read(spi, &txbuf, 4, val, len);
-	return ret;
-}
-
-static int ksz9477_spi_write_reg(struct spi_device *spi, u32 reg, u8 *val,
-				 unsigned int len)
-{
-	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);
-
-	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;
-
-	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);
-}
-
-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);
+#define SPI_CMD_LEN			4
+#define REG_SIZE			0x8000
+
+#define SPI_REGMAP_PAD			SPI_TURNAROUND_SHIFT
+#define SPI_REGMAP_VAL			8
+#define SPI_REGMAP_REG			\
+	(SPI_CMD_LEN * SPI_REGMAP_VAL - SPI_TURNAROUND_SHIFT)
+#define SPI_REGMAP_MASK_S		\
+	(SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT - \
+	(SPI_CMD_LEN * SPI_REGMAP_VAL - 8))
+
+#define KSZ_REGMAP_COMMON(n, width)					\
+{									\
+	.name			= n,					\
+	.max_register		= REG_SIZE - (width),			\
+	.reg_bits		= SPI_REGMAP_REG,			\
+	.val_bits		= SPI_REGMAP_VAL * (width),		\
+	.pad_bits		= SPI_REGMAP_PAD,			\
+	.reg_stride		= (width),				\
+	.read_flag_mask		= KS_SPIOP_RD << SPI_REGMAP_MASK_S,	\
+	.write_flag_mask	= KS_SPIOP_WR << SPI_REGMAP_MASK_S,	\
+	.reg_format_endian	= REGMAP_ENDIAN_BIG,			\
+	.val_format_endian	= REGMAP_ENDIAN_BIG,			\
 }
 
-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,
+static const struct regmap_config ksz9477_regmap_cfg[] = {
+	KSZ_REGMAP_COMMON("8", 1),
+	KSZ_REGMAP_COMMON("16", 2),
+	KSZ_REGMAP_COMMON("32", 4),
 };
 
 static int ksz9477_spi_probe(struct spi_device *spi)
 {
 	struct ksz_device *dev;
+	int i;
 	int ret;
 
-	dev = ksz_switch_alloc(&spi->dev, &ksz9477_spi_ops, spi);
+	dev = ksz_switch_alloc(&spi->dev);
 	if (!dev)
 		return -ENOMEM;
 
+	for (i = 0; i < ARRAY_SIZE(ksz9477_regmap_cfg); i++) {
+		dev->regmap[i] = devm_regmap_init_spi(spi,
+						      &ksz9477_regmap_cfg[i]);
+		if (IS_ERR(dev->regmap[i])) {
+			ret = PTR_ERR(dev->regmap[i]);
+			dev_err(&spi->dev, "Failed to initialize regmap: %d\n",
+				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_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 8a5111f..d50a194 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2,7 +2,7 @@
 /*
  * Microchip switch driver main logic
  *
- * Copyright (C) 2017-2018 Microchip Technology Inc.
+ * Copyright (C) 2017-2019 Microchip Technology Inc.
  */
 
 #include <linux/delay.h>
@@ -260,9 +260,7 @@ void ksz_disable_port(struct dsa_switch *ds, int port, struct phy_device *phy)
 }
 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)
 {
 	struct dsa_switch *ds;
 	struct ksz_device *swdev;
@@ -279,8 +277,6 @@ struct ksz_device *ksz_switch_alloc(struct device *base,
 	swdev->dev = base;
 
 	swdev->ds = ds;
-	swdev->priv = priv;
-	swdev->ops = ops;
 
 	return swdev;
 }
@@ -305,7 +301,6 @@ int ksz_switch_register(struct ksz_device *dev,
 		gpiod_set_value(dev->reset_gpio, 0);
 	}
 
-	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 2dd832d..2dfab32 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -1,7 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0
  * Microchip switch driver common header
  *
- * Copyright (C) 2017-2018 Microchip Technology Inc.
+ * Copyright (C) 2017-2019 Microchip Technology Inc.
  */
 
 #ifndef __KSZ_COMMON_H
@@ -38,20 +38,18 @@ 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);
+	ret = regmap_raw_read(dev->regmap[0], reg, val, 1);
 
 	return ret;
 }
 
 static inline int ksz_read16(struct ksz_device *dev, u32 reg, u16 *val)
 {
+	unsigned int data;
 	int ret;
 
-	mutex_lock(&dev->reg_mutex);
-	ret = dev->ops->read16(dev, reg, val);
-	mutex_unlock(&dev->reg_mutex);
+	ret = regmap_read(dev->regmap[1], reg, &data);
+	*val = (u16)data;
 
 	return ret;
 }
@@ -60,9 +58,12 @@ 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);
+	ret = regmap_raw_read(dev->regmap[0], reg, val, 3);
+	if (!ret) {
+		*val = be32_to_cpu(*val);
+		/* convert to 24bit */
+		*val >>= 8;
+	}
 
 	return ret;
 }
@@ -71,144 +72,94 @@ 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);
+	ret = regmap_read(dev->regmap[2], reg, val);
 
 	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_raw_write(dev->regmap[0], reg, &value, 1);
 }
 
 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;
+	value = cpu_to_be16(value);
+	return regmap_raw_write(dev->regmap[0], reg, &value, 2);
 }
 
 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;
+	/* make it to big endian 24bit from MSB */
+	value <<= 8;
+	value = cpu_to_be32(value);
+	return regmap_raw_write(dev->regmap[0], reg, &value, 3);
 }
 
 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;
+	value = cpu_to_be32(value);
+	return regmap_raw_write(dev->regmap[0], reg, &value, 4);
 }
 
 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;
+	return regmap_raw_read(dev->regmap[0], reg, data, len);
 }
 
 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;
+	return regmap_raw_write(dev->regmap[0], reg, data, len);
 }
 
 static inline void ksz_pread8(struct ksz_device *dev, int port, int offset,
 			      u8 *data)
 {
-	ksz_read8(dev, dev->dev_ops->get_port_addr(port, offset), data);
+	ksz_read8(dev, PORT_CTRL_ADDR(port, offset), data);
 }
 
 static inline void ksz_pread16(struct ksz_device *dev, int port, int offset,
 			       u16 *data)
 {
-	ksz_read16(dev, dev->dev_ops->get_port_addr(port, offset), data);
+	ksz_read16(dev, PORT_CTRL_ADDR(port, offset), data);
 }
 
 static inline void ksz_pread32(struct ksz_device *dev, int port, int offset,
 			       u32 *data)
 {
-	ksz_read32(dev, dev->dev_ops->get_port_addr(port, offset), data);
+	ksz_read32(dev, PORT_CTRL_ADDR(port, offset), data);
 }
 
 static inline void ksz_pwrite8(struct ksz_device *dev, int port, int offset,
 			       u8 data)
 {
-	ksz_write8(dev, dev->dev_ops->get_port_addr(port, offset), data);
+	ksz_write8(dev, PORT_CTRL_ADDR(port, offset), data);
 }
 
 static inline void ksz_pwrite16(struct ksz_device *dev, int port, int offset,
 				u16 data)
 {
-	ksz_write16(dev, dev->dev_ops->get_port_addr(port, offset), data);
+	ksz_write16(dev, PORT_CTRL_ADDR(port, offset), data);
 }
 
 static inline void ksz_pwrite32(struct ksz_device *dev, int port, int offset,
 				u32 data)
 {
-	ksz_write32(dev, dev->dev_ops->get_port_addr(port, offset), data);
+	ksz_write32(dev, PORT_CTRL_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);
+	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 = 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);
+	regmap_update_bits(dev->regmap[0], PORT_CTRL_ADDR(port, offset), bits,
+			   set ? bits : 0);
 }
 
 #endif
diff --git a/drivers/net/dsa/microchip/ksz_priv.h b/drivers/net/dsa/microchip/ksz_priv.h
index 60b4901..526cd0f 100644
--- a/drivers/net/dsa/microchip/ksz_priv.h
+++ b/drivers/net/dsa/microchip/ksz_priv.h
@@ -2,7 +2,7 @@
  *
  * Microchip KSZ series switch common definitions
  *
- * Copyright (C) 2017-2018 Microchip Technology Inc.
+ * Copyright (C) 2017-2019 Microchip Technology Inc.
  */
 
 #ifndef __KSZ_PRIV_H
@@ -11,13 +11,12 @@
 #include <linux/kernel.h>
 #include <linux/mutex.h>
 #include <linux/phy.h>
+#include <linux/regmap.h>
 #include <linux/etherdevice.h>
 #include <net/dsa.h>
 
 #include "ksz9477_reg.h"
 
-struct ksz_io_ops;
-
 struct vlan_table {
 	u32 table[3];
 };
@@ -47,18 +46,15 @@ struct ksz_device {
 	struct dsa_switch *ds;
 	struct ksz_platform_data *pdata;
 	const char *name;
+	struct regmap *regmap[3];
 
-	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;
 
-	void *priv;
-
 	struct gpio_desc *reset_gpio;	/* Optional reset GPIO */
 
 	/* chip specific data */
@@ -81,8 +77,6 @@ struct ksz_device {
 
 	u64 mib_value[TOTAL_SWITCH_COUNTER_NUM];
 
-	u8 *txbuf;
-
 	struct ksz_port *ports;
 	struct timer_list mib_read_timer;
 	struct work_struct mib_read;
@@ -101,19 +95,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 (*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);
-};
-
 struct alu_struct {
 	/* entry 1 */
 	u8	is_static:1;
@@ -158,8 +139,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);
 int ksz_switch_register(struct ksz_device *dev,
 			const struct ksz_dev_ops *ops);
 void ksz_switch_remove(struct ksz_device *dev);
-- 
1.9.1

[PATCH RFC 2/4] net: dsa: microchip: Use regmap_update_bits

[Tristram.Ha]

From: Tristram Ha <Tristram.Ha@microchip.com>

Use regmap_update_bits function for bit manipulation in 32-bit access.

Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
---
 drivers/net/dsa/microchip/ksz9477.c | 23 +++--------------------
 1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 89ed059..14b6371 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -65,31 +65,14 @@
 
 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,
-- 
1.9.1

[PATCH RFC 3/4] net: dsa: microchip: remove ksz9477_get_port_addr

[Tristram.Ha]

From: Tristram Ha <Tristram.Ha@microchip.com>

Remove ksz9477_get_port_addr as it is considered too slow and the macro
PORT_CTRL_ADDR can be used directly.

Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
---
 drivers/net/dsa/microchip/ksz9477.c  | 11 +----------
 drivers/net/dsa/microchip/ksz_priv.h |  1 -
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 14b6371..3cf8ad7 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -2,12 +2,9 @@
 /*
  * Microchip KSZ9477 switch driver main logic
  *
- * Copyright (C) 2017-2018 Microchip Technology Inc.
+ * Copyright (C) 2017-2019 Microchip Technology Inc.
  */
 
-#include <linux/delay.h>
-#include <linux/export.h>
-#include <linux/gpio.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/platform_data/microchip-ksz.h>
@@ -1157,11 +1154,6 @@ static int ksz9477_setup(struct dsa_switch *ds)
 	.port_mirror_del	= ksz9477_port_mirror_del,
 };
 
-static u32 ksz9477_get_port_addr(int port, int offset)
-{
-	return PORT_CTRL_ADDR(port, offset);
-}
-
 static int ksz9477_switch_detect(struct ksz_device *dev)
 {
 	u8 data8;
@@ -1278,7 +1270,6 @@ static void ksz9477_switch_exit(struct ksz_device *dev)
 }
 
 static const struct ksz_dev_ops ksz9477_dev_ops = {
-	.get_port_addr = ksz9477_get_port_addr,
 	.cfg_port_member = ksz9477_cfg_port_member,
 	.flush_dyn_mac_table = ksz9477_flush_dyn_mac_table,
 	.port_setup = ksz9477_port_setup,
diff --git a/drivers/net/dsa/microchip/ksz_priv.h b/drivers/net/dsa/microchip/ksz_priv.h
index 526cd0f..00ab370 100644
--- a/drivers/net/dsa/microchip/ksz_priv.h
+++ b/drivers/net/dsa/microchip/ksz_priv.h
@@ -115,7 +115,6 @@ struct alu_struct {
 };
 
 struct ksz_dev_ops {
-	u32 (*get_port_addr)(int port, int offset);
 	void (*cfg_port_member)(struct ksz_device *dev, int port, u8 member);
 	void (*flush_dyn_mac_table)(struct ksz_device *dev, int port);
 	void (*port_setup)(struct ksz_device *dev, int port, bool cpu_port);
-- 
1.9.1

[PATCH RFC 4/4] net: dsa: microchip: remove ksz_spi.h

[Tristram.Ha]

From: Tristram Ha <Tristram.Ha@microchip.com>

Remove ksz_spi.h as it is not used anymore.

Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
---
 drivers/net/dsa/microchip/ksz_spi.h | 69 -------------------------------------
 1 file changed, 69 deletions(-)
 delete mode 100644 drivers/net/dsa/microchip/ksz_spi.h

diff --git a/drivers/net/dsa/microchip/ksz_spi.h b/drivers/net/dsa/microchip/ksz_spi.h
deleted file mode 100644
index 427811b..0000000
--- a/drivers/net/dsa/microchip/ksz_spi.h
+++ /dev/null
@@ -1,69 +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);
-}
-
-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
-- 
1.9.1

Re: [PATCH RFC 1/4] net: dsa: microchip: convert KSZ9477 SPI driver to use regmap

[Marek Vasut]

On 1/17/19 4:22 AM, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
> 
> Convert KSZ9477 SPI driver to use regmap mechanism so that an I2C driver
> can be easily added.
> 
> KSZ9477 SPI driver uses a 32-bit SPI command containing a 24-bit address
> to access registers in 8-bit.  The address is automatically increased to
> next register.  In theory all registers can be read in one transfer as
> long as the buffer is enough.  Therefore the regmap_raw_read and
> regmap_raw_write functions are used for basic 8-bit access.  Two more
> regmap configurations are used for 16-bit and 32-bit accesses just that
> regmap_update_bits can be used.
> 
> All variables and functions associated with SPI access are removed.
> 
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>

These extremely patches look similar to what I posted here before:

https://patchwork.ozlabs.org/cover/1017222/

But the authorship has changed. Why ?

> ---
>  drivers/net/dsa/microchip/Kconfig       |   1 +
>  drivers/net/dsa/microchip/ksz9477_spi.c | 131 ++++++++++----------------------
>  drivers/net/dsa/microchip/ksz_common.c  |   9 +--
>  drivers/net/dsa/microchip/ksz_common.h  | 113 ++++++++-------------------
>  drivers/net/dsa/microchip/ksz_priv.h    |  28 +------
>  5 files changed, 80 insertions(+), 202 deletions(-)
> 
> diff --git a/drivers/net/dsa/microchip/Kconfig b/drivers/net/dsa/microchip/Kconfig
> index bea29fd..385b93f 100644
> --- a/drivers/net/dsa/microchip/Kconfig
> +++ b/drivers/net/dsa/microchip/Kconfig
> @@ -12,5 +12,6 @@ menuconfig NET_DSA_MICROCHIP_KSZ9477
>  config NET_DSA_MICROCHIP_KSZ9477_SPI
>  	tristate "KSZ9477 series SPI connected switch driver"
>  	depends on NET_DSA_MICROCHIP_KSZ9477 && SPI
> +	select REGMAP_SPI
>  	help
>  	  Select to enable support for registering switches configured through SPI.
> diff --git a/drivers/net/dsa/microchip/ksz9477_spi.c b/drivers/net/dsa/microchip/ksz9477_spi.c
> index d757ba1..d5f0aaa 100644
> --- a/drivers/net/dsa/microchip/ksz9477_spi.c
> +++ b/drivers/net/dsa/microchip/ksz9477_spi.c
> @@ -2,18 +2,14 @@
>  /*
>   * Microchip KSZ9477 series register access through SPI
>   *
> - * Copyright (C) 2017-2018 Microchip Technology Inc.
> + * Copyright (C) 2017-2019 Microchip Technology Inc.
>   */
>  
> -#include <asm/unaligned.h>
> -
> -#include <linux/delay.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/spi/spi.h>
>  
>  #include "ksz_priv.h"
> -#include "ksz_spi.h"
>  
>  /* SPI frame opcodes */
>  #define KS_SPIOP_RD			3
> @@ -23,106 +19,61 @@
>  #define SPI_ADDR_MASK			(BIT(SPI_ADDR_SHIFT) - 1)
>  #define SPI_TURNAROUND_SHIFT		5
>  
> -/* 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)
> -{
> -	u32 txbuf;
> -	int ret;
> -
> -	txbuf = reg & SPI_ADDR_MASK;
> -	txbuf |= KS_SPIOP_RD << SPI_ADDR_SHIFT;
> -	txbuf <<= SPI_TURNAROUND_SHIFT;
> -	txbuf = cpu_to_be32(txbuf);
> -
> -	ret = spi_write_then_read(spi, &txbuf, 4, val, len);
> -	return ret;
> -}
> -
> -static int ksz9477_spi_write_reg(struct spi_device *spi, u32 reg, u8 *val,
> -				 unsigned int len)
> -{
> -	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);
> -
> -	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;
> -
> -	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);
> -}
> -
> -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);
> +#define SPI_CMD_LEN			4
> +#define REG_SIZE			0x8000
> +
> +#define SPI_REGMAP_PAD			SPI_TURNAROUND_SHIFT
> +#define SPI_REGMAP_VAL			8
> +#define SPI_REGMAP_REG			\
> +	(SPI_CMD_LEN * SPI_REGMAP_VAL - SPI_TURNAROUND_SHIFT)
> +#define SPI_REGMAP_MASK_S		\
> +	(SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT - \
> +	(SPI_CMD_LEN * SPI_REGMAP_VAL - 8))
> +
> +#define KSZ_REGMAP_COMMON(n, width)					\
> +{									\
> +	.name			= n,					\
> +	.max_register		= REG_SIZE - (width),			\
> +	.reg_bits		= SPI_REGMAP_REG,			\
> +	.val_bits		= SPI_REGMAP_VAL * (width),		\
> +	.pad_bits		= SPI_REGMAP_PAD,			\
> +	.reg_stride		= (width),				\
> +	.read_flag_mask		= KS_SPIOP_RD << SPI_REGMAP_MASK_S,	\
> +	.write_flag_mask	= KS_SPIOP_WR << SPI_REGMAP_MASK_S,	\
> +	.reg_format_endian	= REGMAP_ENDIAN_BIG,			\
> +	.val_format_endian	= REGMAP_ENDIAN_BIG,			\
>  }
>  
> -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,
> +static const struct regmap_config ksz9477_regmap_cfg[] = {
> +	KSZ_REGMAP_COMMON("8", 1),
> +	KSZ_REGMAP_COMMON("16", 2),
> +	KSZ_REGMAP_COMMON("32", 4),
>  };
>  
>  static int ksz9477_spi_probe(struct spi_device *spi)
>  {
>  	struct ksz_device *dev;
> +	int i;
>  	int ret;
>  
> -	dev = ksz_switch_alloc(&spi->dev, &ksz9477_spi_ops, spi);
> +	dev = ksz_switch_alloc(&spi->dev);
>  	if (!dev)
>  		return -ENOMEM;
>  
> +	for (i = 0; i < ARRAY_SIZE(ksz9477_regmap_cfg); i++) {
> +		dev->regmap[i] = devm_regmap_init_spi(spi,
> +						      &ksz9477_regmap_cfg[i]);
> +		if (IS_ERR(dev->regmap[i])) {
> +			ret = PTR_ERR(dev->regmap[i]);
> +			dev_err(&spi->dev, "Failed to initialize regmap: %d\n",
> +				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_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 8a5111f..d50a194 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -2,7 +2,7 @@
>  /*
>   * Microchip switch driver main logic
>   *
> - * Copyright (C) 2017-2018 Microchip Technology Inc.
> + * Copyright (C) 2017-2019 Microchip Technology Inc.
>   */
>  
>  #include <linux/delay.h>
> @@ -260,9 +260,7 @@ void ksz_disable_port(struct dsa_switch *ds, int port, struct phy_device *phy)
>  }
>  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)
>  {
>  	struct dsa_switch *ds;
>  	struct ksz_device *swdev;
> @@ -279,8 +277,6 @@ struct ksz_device *ksz_switch_alloc(struct device *base,
>  	swdev->dev = base;
>  
>  	swdev->ds = ds;
> -	swdev->priv = priv;
> -	swdev->ops = ops;
>  
>  	return swdev;
>  }
> @@ -305,7 +301,6 @@ int ksz_switch_register(struct ksz_device *dev,
>  		gpiod_set_value(dev->reset_gpio, 0);
>  	}
>  
> -	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 2dd832d..2dfab32 100644
> --- a/drivers/net/dsa/microchip/ksz_common.h
> +++ b/drivers/net/dsa/microchip/ksz_common.h
> @@ -1,7 +1,7 @@
>  /* SPDX-License-Identifier: GPL-2.0
>   * Microchip switch driver common header
>   *
> - * Copyright (C) 2017-2018 Microchip Technology Inc.
> + * Copyright (C) 2017-2019 Microchip Technology Inc.
>   */
>  
>  #ifndef __KSZ_COMMON_H
> @@ -38,20 +38,18 @@ 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);
> +	ret = regmap_raw_read(dev->regmap[0], reg, val, 1);
>  
>  	return ret;
>  }
>  
>  static inline int ksz_read16(struct ksz_device *dev, u32 reg, u16 *val)
>  {
> +	unsigned int data;
>  	int ret;
>  
> -	mutex_lock(&dev->reg_mutex);
> -	ret = dev->ops->read16(dev, reg, val);
> -	mutex_unlock(&dev->reg_mutex);
> +	ret = regmap_read(dev->regmap[1], reg, &data);
> +	*val = (u16)data;
>  
>  	return ret;
>  }
> @@ -60,9 +58,12 @@ 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);
> +	ret = regmap_raw_read(dev->regmap[0], reg, val, 3);
> +	if (!ret) {
> +		*val = be32_to_cpu(*val);
> +		/* convert to 24bit */
> +		*val >>= 8;
> +	}
>  
>  	return ret;
>  }
> @@ -71,144 +72,94 @@ 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);
> +	ret = regmap_read(dev->regmap[2], reg, val);
>  
>  	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_raw_write(dev->regmap[0], reg, &value, 1);
>  }
>  
>  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;
> +	value = cpu_to_be16(value);
> +	return regmap_raw_write(dev->regmap[0], reg, &value, 2);
>  }
>  
>  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;
> +	/* make it to big endian 24bit from MSB */
> +	value <<= 8;
> +	value = cpu_to_be32(value);
> +	return regmap_raw_write(dev->regmap[0], reg, &value, 3);
>  }
>  
>  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;
> +	value = cpu_to_be32(value);
> +	return regmap_raw_write(dev->regmap[0], reg, &value, 4);
>  }
>  
>  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;
> +	return regmap_raw_read(dev->regmap[0], reg, data, len);
>  }
>  
>  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;
> +	return regmap_raw_write(dev->regmap[0], reg, data, len);
>  }
>  
>  static inline void ksz_pread8(struct ksz_device *dev, int port, int offset,
>  			      u8 *data)
>  {
> -	ksz_read8(dev, dev->dev_ops->get_port_addr(port, offset), data);
> +	ksz_read8(dev, PORT_CTRL_ADDR(port, offset), data);
>  }
>  
>  static inline void ksz_pread16(struct ksz_device *dev, int port, int offset,
>  			       u16 *data)
>  {
> -	ksz_read16(dev, dev->dev_ops->get_port_addr(port, offset), data);
> +	ksz_read16(dev, PORT_CTRL_ADDR(port, offset), data);
>  }
>  
>  static inline void ksz_pread32(struct ksz_device *dev, int port, int offset,
>  			       u32 *data)
>  {
> -	ksz_read32(dev, dev->dev_ops->get_port_addr(port, offset), data);
> +	ksz_read32(dev, PORT_CTRL_ADDR(port, offset), data);
>  }
>  
>  static inline void ksz_pwrite8(struct ksz_device *dev, int port, int offset,
>  			       u8 data)
>  {
> -	ksz_write8(dev, dev->dev_ops->get_port_addr(port, offset), data);
> +	ksz_write8(dev, PORT_CTRL_ADDR(port, offset), data);
>  }
>  
>  static inline void ksz_pwrite16(struct ksz_device *dev, int port, int offset,
>  				u16 data)
>  {
> -	ksz_write16(dev, dev->dev_ops->get_port_addr(port, offset), data);
> +	ksz_write16(dev, PORT_CTRL_ADDR(port, offset), data);
>  }
>  
>  static inline void ksz_pwrite32(struct ksz_device *dev, int port, int offset,
>  				u32 data)
>  {
> -	ksz_write32(dev, dev->dev_ops->get_port_addr(port, offset), data);
> +	ksz_write32(dev, PORT_CTRL_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);
> +	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 = 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);
> +	regmap_update_bits(dev->regmap[0], PORT_CTRL_ADDR(port, offset), bits,
> +			   set ? bits : 0);
>  }
>  
>  #endif
> diff --git a/drivers/net/dsa/microchip/ksz_priv.h b/drivers/net/dsa/microchip/ksz_priv.h
> index 60b4901..526cd0f 100644
> --- a/drivers/net/dsa/microchip/ksz_priv.h
> +++ b/drivers/net/dsa/microchip/ksz_priv.h
> @@ -2,7 +2,7 @@
>   *
>   * Microchip KSZ series switch common definitions
>   *
> - * Copyright (C) 2017-2018 Microchip Technology Inc.
> + * Copyright (C) 2017-2019 Microchip Technology Inc.
>   */
>  
>  #ifndef __KSZ_PRIV_H
> @@ -11,13 +11,12 @@
>  #include <linux/kernel.h>
>  #include <linux/mutex.h>
>  #include <linux/phy.h>
> +#include <linux/regmap.h>
>  #include <linux/etherdevice.h>
>  #include <net/dsa.h>
>  
>  #include "ksz9477_reg.h"
>  
> -struct ksz_io_ops;
> -
>  struct vlan_table {
>  	u32 table[3];
>  };
> @@ -47,18 +46,15 @@ struct ksz_device {
>  	struct dsa_switch *ds;
>  	struct ksz_platform_data *pdata;
>  	const char *name;
> +	struct regmap *regmap[3];
>  
> -	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;
>  
> -	void *priv;
> -
>  	struct gpio_desc *reset_gpio;	/* Optional reset GPIO */
>  
>  	/* chip specific data */
> @@ -81,8 +77,6 @@ struct ksz_device {
>  
>  	u64 mib_value[TOTAL_SWITCH_COUNTER_NUM];
>  
> -	u8 *txbuf;
> -
>  	struct ksz_port *ports;
>  	struct timer_list mib_read_timer;
>  	struct work_struct mib_read;
> @@ -101,19 +95,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 (*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);
> -};
> -
>  struct alu_struct {
>  	/* entry 1 */
>  	u8	is_static:1;
> @@ -158,8 +139,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);
>  int ksz_switch_register(struct ksz_device *dev,
>  			const struct ksz_dev_ops *ops);
>  void ksz_switch_remove(struct ksz_device *dev);
> 


-- 
Best regards,
Marek Vasut

Re: [PATCH RFC 1/4] net: dsa: microchip: convert KSZ9477 SPI driver to use regmap

[Pavel Machek]

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

On Fri 2019-01-18 07:29:35, Marek Vasut wrote:
> On 1/17/19 4:22 AM, Tristram.Ha@microchip.com wrote:
> > From: Tristram Ha <Tristram.Ha@microchip.com>
> > 
> > Convert KSZ9477 SPI driver to use regmap mechanism so that an I2C driver
> > can be easily added.
> > 
> > KSZ9477 SPI driver uses a 32-bit SPI command containing a 24-bit address
> > to access registers in 8-bit.  The address is automatically increased to
> > next register.  In theory all registers can be read in one transfer as
> > long as the buffer is enough.  Therefore the regmap_raw_read and
> > regmap_raw_write functions are used for basic 8-bit access.  Two more
> > regmap configurations are used for 16-bit and 32-bit accesses just that
> > regmap_update_bits can be used.
> > 
> > All variables and functions associated with SPI access are removed.
> > 
> > Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> 
> These extremely patches look similar to what I posted here before:
> 
> https://patchwork.ozlabs.org/cover/1017222/
> 
> But the authorship has changed. Why ?

There seems to be explanation in 0/4.

Tristram: if you started from Marek's patches as you describe in 0/4,
you should mark respective patches as From: and Signed-off-by:
Marek...

Best regards,
								Pavel
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

RE: [PATCH RFC 1/4] net: dsa: microchip: convert KSZ9477 SPI driver to use regmap

[Tristram.Ha]

> > These extremely patches look similar to what I posted here before:
> >
> > https://patchwork.ozlabs.org/cover/1017222/
> >
> > But the authorship has changed. Why ?
> 
> There seems to be explanation in 0/4.
> 
> Tristram: if you started from Marek's patches as you describe in 0/4,
> you should mark respective patches as From: and Signed-off-by:
> Marek...

Sorry, do not know much about pulling out the patches and applying them and there are several versions of Marek's patchset.

This is a RFC so please suggest more changes.

Re: [PATCH RFC 1/4] net: dsa: microchip: convert KSZ9477 SPI driver to use regmap

[Marek Vasut]

On 1/18/19 9:32 PM, Tristram.Ha@microchip.com wrote:
>>> These extremely patches look similar to what I posted here before:
>>>
>>> https://patchwork.ozlabs.org/cover/1017222/
>>>
>>> But the authorship has changed. Why ?
>>
>> There seems to be explanation in 0/4.
>>
>> Tristram: if you started from Marek's patches as you describe in 0/4,
>> you should mark respective patches as From: and Signed-off-by:
>> Marek...
> 
> Sorry, do not know much about pulling out the patches and applying them and there are several versions of Marek's patchset.
> 
> This is a RFC so please suggest more changes.

My suggestion is that you take the RFC V2 patchset I posted, adjust it
so it works with the KSZ9477 and post those changes. Or post V3 of that
patchset, but make sure to retain authorship.

-- 
Best regards,
Marek Vasut

Re: [PATCH RFC 1/4] net: dsa: microchip: convert KSZ9477 SPI driver to use regmap

[Pavel Machek]

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

On Fri 2019-01-18 20:32:17, Tristram.Ha@microchip.com wrote:
> > > These extremely patches look similar to what I posted here before:
> > >
> > > https://patchwork.ozlabs.org/cover/1017222/
> > >
> > > But the authorship has changed. Why ?
> > 
> > There seems to be explanation in 0/4.
> > 
> > Tristram: if you started from Marek's patches as you describe in 0/4,
> > you should mark respective patches as From: and Signed-off-by:
> > Marek...
> 
> Sorry, do not know much about pulling out the patches and applying them and there are several versions of Marek's patchset.
> 

For patches that are originally from Marek, you should have:

From: Marek Vasut <marex@denx.de>

and

Signed-off-by: Marek Vasut <marex@denx.de>

That is important even for RFC patches. See
Documentation/process/submitting-patches.rst  for details.

Best regards,
								Pavel

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]