On 1/17/19 4:22 AM, tristram...@microchip.com wrote: > From: Tristram Ha <tristram...@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...@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
