[RFT][PATCH 0/7] net: dsa: microchip: Convert to regmap

[RFT][PATCH 0/7] net: dsa: microchip: Convert to regmap

[Marek Vasut]

This patchset converts KSZ9477 switch driver to regmap.

This was only compile-tested as I don't own a device with
KSZ9477, but is implemented in hope that it will help with
proper regmap conversion.

Note that the first 5 patches might just go in as cleanups,
the 6/7 and 7/7 probably need more debugging and/or work.

Tristram, I hope this helps you get started with the regmap.
Please test on the KSZ9477 and let me know how it works, or
ideally provide fixes :)

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 (7):
  net: dsa: microchip: Remove ksz_{read,write}24()
  net: dsa: microchip: Remove ksz_{get,set}24()
  net: dsa: microchip: Inline ksz_spi.h
  net: dsa: microchip: Remove dev->txbuf
  net: dsa: microchip: Factor out register access opcode generation
  net: dsa: microchip: Initial SPI regmap support
  net: dsa: microchip: Dispose of ksz_io_ops

 drivers/net/dsa/microchip/Kconfig       |   1 +
 drivers/net/dsa/microchip/ksz9477_spi.c | 106 ++++--------------------
 drivers/net/dsa/microchip/ksz_common.c  |   6 +-
 drivers/net/dsa/microchip/ksz_common.h  |  96 ++++-----------------
 drivers/net/dsa/microchip/ksz_priv.h    |  23 +----
 drivers/net/dsa/microchip/ksz_spi.h     |  69 ---------------
 6 files changed, 38 insertions(+), 263 deletions(-)
 delete mode 100644 drivers/net/dsa/microchip/ksz_spi.h

-- 
2.19.2

[RFT][PATCH 1/7] 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>
---
 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 d757ba151cb1..695def784933 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 2dd832de0d52..5f2206a9b3e0 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -56,17 +56,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;
@@ -100,17 +89,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 60b49010904b..db16002b6b6a 100644
--- a/drivers/net/dsa/microchip/ksz_priv.h
+++ b/drivers/net/dsa/microchip/ksz_priv.h
@@ -104,11 +104,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.19.2

[RFT][PATCH 2/7] 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>
---
 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 695def784933..2d5262b01c77 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 5f2206a9b3e0..8611122a51d6 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -100,30 +100,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 db16002b6b6a..c3a272505af1 100644
--- a/drivers/net/dsa/microchip/ksz_priv.h
+++ b/drivers/net/dsa/microchip/ksz_priv.h
@@ -108,8 +108,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.19.2

[RFT][PATCH 3/7] 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>
---
 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 2d5262b01c77..9ca150a472ea 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.19.2

[RFT][PATCH 4/7] net: dsa: microchip: Remove dev->txbuf

[Marek Vasut]

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. This wastes 256 bytes for no reason and can be
replaced with 8-byte stack allocated buffer, which is what this patch
does. This is an intermediate step though, which will go away after
regmap conversion.

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>
---
 drivers/net/dsa/microchip/ksz9477_spi.c | 10 ++++------
 drivers/net/dsa/microchip/ksz_priv.h    |  2 --
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477_spi.c b/drivers/net/dsa/microchip/ksz9477_spi.c
index 9ca150a472ea..69baf9677def 100644
--- a/drivers/net/dsa/microchip/ksz9477_spi.c
+++ b/drivers/net/dsa/microchip/ksz9477_spi.c
@@ -65,11 +65,11 @@ static int ksz_spi_write(struct ksz_device *dev, u32 reg, void *data,
 			 unsigned int len)
 {
 	struct spi_device *spi = dev->priv;
+	u8 txbuf[8];
 
-	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);
+	memcpy(txbuf + 4, data, len);
+
+	return ksz9477_spi_write_reg(spi, reg, txbuf, len);
 }
 
 static int ksz_spi_read8(struct ksz_device *dev, u32 reg, u8 *val)
@@ -135,8 +135,6 @@ static int ksz9477_spi_probe(struct spi_device *spi)
 	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 c3a272505af1..3ab14ee0e36b 100644
--- a/drivers/net/dsa/microchip/ksz_priv.h
+++ b/drivers/net/dsa/microchip/ksz_priv.h
@@ -81,8 +81,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;
-- 
2.19.2

Re: [RFT][PATCH 4/7] net: dsa: microchip: Remove dev->txbuf

[Florian Fainelli]

On 12/19/18 5:06 PM, Marek Vasut wrote:
> 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. This wastes 256 bytes for no reason and can be
> replaced with 8-byte stack allocated buffer, which is what this patch
> does. This is an intermediate step though, which will go away after
> regmap conversion.

dev is a kmalloc'd buffer, so dev->txbuf is a DMA-able buffer, that
could be presumably why it was used in the first place, DMA from the
stack is not something safe, but I did not check the core SPI layer or
the SPI bus master driver to see whether they do take care of that already.
-- 
Florian

Re: [RFT][PATCH 4/7] net: dsa: microchip: Remove dev->txbuf

[Andrew Lunn]

On Wed, Dec 19, 2018 at 05:20:33PM -0800, Florian Fainelli wrote:
> On 12/19/18 5:06 PM, Marek Vasut wrote:
> > 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. This wastes 256 bytes for no reason and can be
> > replaced with 8-byte stack allocated buffer, which is what this patch
> > does. This is an intermediate step though, which will go away after
> > regmap conversion.
> 
> dev is a kmalloc'd buffer, so dev->txbuf is a DMA-able buffer, that
> could be presumably why it was used in the first place, DMA from the
> stack is not something safe, but I did not check the core SPI layer or
> the SPI bus master driver to see whether they do take care of that already.

Hi Marek

This is also something i requested for the i2c layering. An i2c bus
master might use DMA. So you cannot use stack space, or the i2c core
needs to copy the data to somewhere which is DMA'able.

Please could you check what regmap offers. If regmap does the copies,
than this buffer can be removed, but only after the change to regmap
is performed. You should not remove it before swapping to regmap,
otherwise you can introducing a regression, which somebody might hit
during a git bisect.

Thanks
       Andrew

Re: [RFT][PATCH 4/7] net: dsa: microchip: Remove dev->txbuf

[Marek Vasut]

On 12/20/2018 10:41 AM, Andrew Lunn wrote:
> On Wed, Dec 19, 2018 at 05:20:33PM -0800, Florian Fainelli wrote:
>> On 12/19/18 5:06 PM, Marek Vasut wrote:
>>> 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. This wastes 256 bytes for no reason and can be
>>> replaced with 8-byte stack allocated buffer, which is what this patch
>>> does. This is an intermediate step though, which will go away after
>>> regmap conversion.
>>
>> dev is a kmalloc'd buffer, so dev->txbuf is a DMA-able buffer, that
>> could be presumably why it was used in the first place, DMA from the
>> stack is not something safe, but I did not check the core SPI layer or
>> the SPI bus master driver to see whether they do take care of that already.
> 
> Hi Marek
> 
> This is also something i requested for the i2c layering. An i2c bus
> master might use DMA. So you cannot use stack space, or the i2c core
> needs to copy the data to somewhere which is DMA'able.
> 
> Please could you check what regmap offers. If regmap does the copies,
> than this buffer can be removed, but only after the change to regmap
> is performed. You should not remove it before swapping to regmap,
> otherwise you can introducing a regression, which somebody might hit
> during a git bisect.

This code goes away with the regmap completely, so I'll just reorder the
patches and remove it in 6/7 . The 6/7 will need rework anyway, I talked
to Mark Brown about this crazy register layout of this switch and it
seems we'll need a regmap per register width (sigh).

-- 
Best regards,
Marek Vasut

RE: [RFT][PATCH 4/7] net: dsa: microchip: Remove dev->txbuf

[Tristram.Ha]

> On 12/20/2018 10:41 AM, Andrew Lunn wrote:
> > On Wed, Dec 19, 2018 at 05:20:33PM -0800, Florian Fainelli wrote:
> >> On 12/19/18 5:06 PM, Marek Vasut wrote:
> >>> 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. This wastes 256 bytes for no reason and can be
> >>> replaced with 8-byte stack allocated buffer, which is what this patch
> >>> does. This is an intermediate step though, which will go away after
> >>> regmap conversion.

The switch uses auto address increment and can return the whole register
space in 1 transfer.  Most switches have only 256 registers, so I just stopped
at that number for KSZ9477.

This read operation is mostly done by users to dump the registers for debugging.
The write operation can be used to setup registers from a file.

I mentioned before the driver can use the standard Linux register access API for
users to read the chip.  Maybe you also want to try that for debugging.

Re: [RFT][PATCH 4/7] net: dsa: microchip: Remove dev->txbuf

[Marek Vasut]

On 12/21/2018 02:02 AM, Tristram.Ha@microchip.com wrote:
>> On 12/20/2018 10:41 AM, Andrew Lunn wrote:
>>> On Wed, Dec 19, 2018 at 05:20:33PM -0800, Florian Fainelli wrote:
>>>> On 12/19/18 5:06 PM, Marek Vasut wrote:
>>>>> 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. This wastes 256 bytes for no reason and can be
>>>>> replaced with 8-byte stack allocated buffer, which is what this patch
>>>>> does. This is an intermediate step though, which will go away after
>>>>> regmap conversion.
> 
> The switch uses auto address increment and can return the whole register
> space in 1 transfer.  Most switches have only 256 registers, so I just stopped
> at that number for KSZ9477.
> 
> This read operation is mostly done by users to dump the registers for debugging.
> The write operation can be used to setup registers from a file.
> 
> I mentioned before the driver can use the standard Linux register access API for
> users to read the chip.  Maybe you also want to try that for debugging.

The bulk IO is never used in the driver, ever, so that's not really a
concern. Although, you can use the regmap to read (and write, if you
enable it) registers via debugfs.

-- 
Best regards,
Marek Vasut

Re: [RFT][PATCH 4/7] net: dsa: microchip: Remove dev->txbuf

[Andrew Lunn]

On Fri, Dec 21, 2018 at 01:02:23AM +0000, Tristram.Ha@microchip.com wrote:
> > On 12/20/2018 10:41 AM, Andrew Lunn wrote:
> > > On Wed, Dec 19, 2018 at 05:20:33PM -0800, Florian Fainelli wrote:
> > >> On 12/19/18 5:06 PM, Marek Vasut wrote:
> > >>> 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. This wastes 256 bytes for no reason and can be
> > >>> replaced with 8-byte stack allocated buffer, which is what this patch
> > >>> does. This is an intermediate step though, which will go away after
> > >>> regmap conversion.
> 
> The switch uses auto address increment and can return the whole register
> space in 1 transfer.  Most switches have only 256 registers, so I just stopped
> at that number for KSZ9477.
> 
> This read operation is mostly done by users to dump the registers for debugging.
> The write operation can be used to setup registers from a file.
> 
> I mentioned before the driver can use the standard Linux register access API for
> users to read the chip.  Maybe you also want to try that for debugging.

Hi Tristram

You might want to implement the register dump per port, and connect it
to ethtool --register-dump. The core DSA code has what is needed, you
just need driver specific code. Vivien also sent patches for ethtool
to pritty print the values for the mv88e6xxx. You can do the same for
your chipsets.

     Andrew

Re: [RFT][PATCH 4/7] net: dsa: microchip: Remove dev->txbuf

[Marek Vasut]

On 12/20/2018 02:20 AM, Florian Fainelli wrote:
> On 12/19/18 5:06 PM, Marek Vasut wrote:
>> 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. This wastes 256 bytes for no reason and can be
>> replaced with 8-byte stack allocated buffer, which is what this patch
>> does. This is an intermediate step though, which will go away after
>> regmap conversion.
> 
> dev is a kmalloc'd buffer, so dev->txbuf is a DMA-able buffer, that
> could be presumably why it was used in the first place, DMA from the
> stack is not something safe, but I did not check the core SPI layer or
> the SPI bus master driver to see whether they do take care of that already.

Well, I can just squash this and 6/7 and let regmap handle these details.

-- 
Best regards,
Marek Vasut

[RFT][PATCH 5/7] 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>
---
 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 69baf9677def..4fc36dc2195a 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.19.2

[RFT][PATCH 6/7] net: dsa: microchip: Initial SPI regmap support

[Marek Vasut]

Add basic SPI regmap support into the driver.

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>
---
 drivers/net/dsa/microchip/Kconfig       |  1 +
 drivers/net/dsa/microchip/ksz9477_spi.c | 85 ++++++++-----------------
 drivers/net/dsa/microchip/ksz_priv.h    |  1 +
 3 files changed, 30 insertions(+), 57 deletions(-)

diff --git a/drivers/net/dsa/microchip/Kconfig b/drivers/net/dsa/microchip/Kconfig
index bea29fde9f3d..3a00cb1b372a 100644
--- a/drivers/net/dsa/microchip/Kconfig
+++ b/drivers/net/dsa/microchip/Kconfig
@@ -1,4 +1,5 @@
 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 4fc36dc2195a..3cf3c8143e98 100644
--- a/drivers/net/dsa/microchip/ksz9477_spi.c
+++ b/drivers/net/dsa/microchip/ksz9477_spi.c
@@ -10,6 +10,7 @@
 #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"
@@ -22,66 +23,29 @@
 #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 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;
-	u8 txbuf[8];
-
-	memcpy(txbuf + 4, data, len);
-
-	return ksz9477_spi_write_reg(spi, reg, txbuf, len);
-}
+static const struct regmap_config ksz9477_regmap_config = {
+	.reg_bits = 32,
+	.val_bits = 8,
+	.max_register = 0x100,
+	.cache_type = REGCACHE_RBTREE,
+	.read_flag_mask = KS_SPIOP_RD << SPI_ADDR_SHIFT,
+	.write_flag_mask = KS_SPIOP_WR << SPI_ADDR_SHIFT,
+	.reg_format_endian = REGMAP_ENDIAN_BIG,
+	.val_format_endian = REGMAP_ENDIAN_BIG,
+};
 
 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 +55,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 +65,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 = {
@@ -134,6 +98,13 @@ static int ksz9477_spi_probe(struct spi_device *spi)
 	if (!dev)
 		return -ENOMEM;
 
+	dev->regmap = devm_regmap_init_spi(spi, &ksz9477_regmap_config);
+	if (IS_ERR(dev->regmap)) {
+		ret = PTR_ERR(dev->regmap);
+		dev_err(&spi->dev, "Failed to initialize regmap: %d\n", ret);
+		return ret;
+	}
+
 	if (spi->dev.platform_data)
 		dev->pdata = spi->dev.platform_data;
 
diff --git a/drivers/net/dsa/microchip/ksz_priv.h b/drivers/net/dsa/microchip/ksz_priv.h
index 3ab14ee0e36b..f9429e1fc7fe 100644
--- a/drivers/net/dsa/microchip/ksz_priv.h
+++ b/drivers/net/dsa/microchip/ksz_priv.h
@@ -56,6 +56,7 @@ struct ksz_device {
 	const struct ksz_dev_ops *dev_ops;
 
 	struct device *dev;
+	struct regmap *regmap;
 
 	void *priv;
 
-- 
2.19.2

RE: [RFT][PATCH 6/7] net: dsa: microchip: Initial SPI regmap support

[Tristram.Ha]

> +static const struct regmap_config ksz9477_regmap_config = {
> +	.reg_bits = 32,
> +	.val_bits = 8,
> +	.max_register = 0x100,
> +	.cache_type = REGCACHE_RBTREE,
> +	.read_flag_mask = KS_SPIOP_RD << SPI_ADDR_SHIFT,
> +	.write_flag_mask = KS_SPIOP_WR << SPI_ADDR_SHIFT,
> +	.reg_format_endian = REGMAP_ENDIAN_BIG,
> +	.val_format_endian = REGMAP_ENDIAN_BIG,
> +};

As you have KSZ8795 can you verify regmap works?

I think you need to add reg_shift.

Re: [RFT][PATCH 6/7] net: dsa: microchip: Initial SPI regmap support

[Marek Vasut]

On 12/21/2018 02:30 AM, Tristram.Ha@microchip.com wrote:
>> +static const struct regmap_config ksz9477_regmap_config = {
>> +	.reg_bits = 32,
>> +	.val_bits = 8,
>> +	.max_register = 0x100,
>> +	.cache_type = REGCACHE_RBTREE,
>> +	.read_flag_mask = KS_SPIOP_RD << SPI_ADDR_SHIFT,
>> +	.write_flag_mask = KS_SPIOP_WR << SPI_ADDR_SHIFT,
>> +	.reg_format_endian = REGMAP_ENDIAN_BIG,
>> +	.val_format_endian = REGMAP_ENDIAN_BIG,
>> +};
> 
> As you have KSZ8795 can you verify regmap works?

I just sent V2 that works on KSZ8795 , give it a go.

> I think you need to add reg_shift.
> 


-- 
Best regards,
Marek Vasut

[RFT][PATCH 7/7] 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>
---
 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, 19 insertions(+), 110 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477_spi.c b/drivers/net/dsa/microchip/ksz9477_spi.c
index 3cf3c8143e98..e28547315211 100644
--- a/drivers/net/dsa/microchip/ksz9477_spi.c
+++ b/drivers/net/dsa/microchip/ksz9477_spi.c
@@ -34,67 +34,12 @@ static const struct regmap_config ksz9477_regmap_config = {
 	.val_format_endian = REGMAP_ENDIAN_BIG,
 };
 
-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 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 3b12e2dcff31..c78afd5e3d4b 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -262,9 +262,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, void *priv)
 {
 	struct dsa_switch *ds;
 	struct ksz_device *swdev;
@@ -282,7 +280,6 @@ struct ksz_device *ksz_switch_alloc(struct device *base,
 
 	swdev->ds = ds;
 	swdev->priv = priv;
-	swdev->ops = ops;
 
 	return swdev;
 }
@@ -307,7 +304,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 8611122a51d6..ae5e8707d718 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_update_port_member(struct ksz_device *dev, int port);
 
 /* Common DSA access functions */
@@ -36,68 +38,48 @@ void ksz_disable_port(struct dsa_switch *ds, int port, struct phy_device *phy);
 
 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, reg, &value);
 
+	*val = value;
 	return ret;
 }
 
 static inline int ksz_read16(struct ksz_device *dev, u32 reg, u16 *val)
 {
-	int ret;
+	int ret = regmap_bulk_read(dev->regmap, reg, val, 2);
 
-	mutex_lock(&dev->reg_mutex);
-	ret = dev->ops->read16(dev, reg, val);
-	mutex_unlock(&dev->reg_mutex);
+	if (!ret)
+		*val = be16_to_cpu(*val);
 
 	return ret;
 }
 
 static inline int ksz_read32(struct ksz_device *dev, u32 reg, u32 *val)
 {
-	int ret;
+	int ret = regmap_bulk_read(dev->regmap, reg, val, 4);
 
-	mutex_lock(&dev->reg_mutex);
-	ret = dev->ops->read32(dev, reg, val);
-	mutex_unlock(&dev->reg_mutex);
+	if (!ret)
+		*val = be32_to_cpu(*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_write(dev->regmap, 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;
+	value = cpu_to_be16(value);
+	return regmap_bulk_write(dev->regmap, reg, &value, 2);
 }
 
 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_bulk_write(dev->regmap, reg, &value, 4);
 }
 
 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 f9429e1fc7fe..f1401ae5e7a2 100644
--- a/drivers/net/dsa/microchip/ksz_priv.h
+++ b/drivers/net/dsa/microchip/ksz_priv.h
@@ -16,8 +16,6 @@
 
 #include "ksz9477_reg.h"
 
-struct ksz_io_ops;
-
 struct vlan_table {
 	u32 table[3];
 };
@@ -48,11 +46,9 @@ struct ksz_device {
 	struct ksz_platform_data *pdata;
 	const char *name;
 
-	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;
@@ -100,15 +96,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;
@@ -153,8 +140,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.19.2