* [PATCH net-next v1 0/5] Microchip DSA Driver Improvements
@ 2023-05-24 12:32 Oleksij Rempel
2023-05-24 12:32 ` [PATCH net-next v1 1/5] net: dsa: microchip: improving error handling for 8-bit register RMW operations Oleksij Rempel
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Oleksij Rempel @ 2023-05-24 12:32 UTC (permalink / raw)
To: David S. Miller, Andrew Lunn, Eric Dumazet, Florian Fainelli,
Jakub Kicinski, Paolo Abeni, Vladimir Oltean, Woojung Huh,
Arun Ramadoss
Cc: Oleksij Rempel, kernel, linux-kernel, netdev, UNGLinuxDriver,
Russell King (Oracle)
I'd like to share a set of patches for the Microchip DSA driver. These
patches were chosen from a bigger set because they are simpler and
should be easier to review. The goal is to make the code easier to read,
get rid of unused code, and handle errors better.
Oleksij Rempel (4):
net: dsa: microchip: improving error handling for 8-bit register RMW
operations
net: dsa: microchip: remove ksz_port:on variable
net: dsa: microchip: ksz8: Prepare ksz8863_smi for regmap register
access validation
net: dsa: microchip: Add register access control for KSZ8873 chip
Vladimir Oltean (1):
net: dsa: microchip: add an enum for regmap widths
drivers/net/dsa/microchip/ksz8795.c | 28 ++-------
drivers/net/dsa/microchip/ksz8863_smi.c | 13 +++-
drivers/net/dsa/microchip/ksz9477.c | 24 ++++----
drivers/net/dsa/microchip/ksz9477_i2c.c | 2 +-
drivers/net/dsa/microchip/ksz_common.c | 47 ++++++++++++++-
drivers/net/dsa/microchip/ksz_common.h | 77 +++++++++++++++++-------
drivers/net/dsa/microchip/ksz_spi.c | 2 +-
drivers/net/dsa/microchip/lan937x_main.c | 8 +--
8 files changed, 135 insertions(+), 66 deletions(-)
--
2.39.2
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH net-next v1 1/5] net: dsa: microchip: improving error handling for 8-bit register RMW operations 2023-05-24 12:32 [PATCH net-next v1 0/5] Microchip DSA Driver Improvements Oleksij Rempel @ 2023-05-24 12:32 ` Oleksij Rempel 2023-05-24 12:39 ` Russell King (Oracle) 2023-05-24 12:32 ` [PATCH net-next v1 2/5] net: dsa: microchip: add an enum for regmap widths Oleksij Rempel ` (3 subsequent siblings) 4 siblings, 1 reply; 13+ messages in thread From: Oleksij Rempel @ 2023-05-24 12:32 UTC (permalink / raw) To: David S. Miller, Andrew Lunn, Eric Dumazet, Florian Fainelli, Jakub Kicinski, Paolo Abeni, Vladimir Oltean, Woojung Huh, Arun Ramadoss Cc: Oleksij Rempel, kernel, linux-kernel, netdev, UNGLinuxDriver, Russell King (Oracle) This patch refines the error handling mechanism for 8-bit register read-modify-write operations. In case of a failure, it now logs an error message detailing the problematic offset. This enhancement aids in debugging by providing more precise information when these operations encounter issues. Furthermore, the ksz_prmw8() function has been updated to return error values rather than void, enabling calling functions to appropriately respond to errors. Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> --- drivers/net/dsa/microchip/ksz_common.h | 28 ++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h index 8abecaf6089e..b86f1e27a0c3 100644 --- a/drivers/net/dsa/microchip/ksz_common.h +++ b/drivers/net/dsa/microchip/ksz_common.h @@ -508,7 +508,14 @@ static inline int ksz_write64(struct ksz_device *dev, u32 reg, u64 value) static inline int ksz_rmw8(struct ksz_device *dev, int offset, u8 mask, u8 val) { - return regmap_update_bits(dev->regmap[0], offset, mask, val); + int ret; + + ret = regmap_update_bits(dev->regmap[0], offset, mask, val); + if (ret) + dev_err(dev->dev, "can't rmw 8bit reg 0x%x: %pe\n", offset, + ERR_PTR(ret)); + + return ret; } static inline int ksz_pread8(struct ksz_device *dev, int port, int offset, @@ -549,12 +556,21 @@ static inline int ksz_pwrite32(struct ksz_device *dev, int port, int offset, data); } -static inline void ksz_prmw8(struct ksz_device *dev, int port, int offset, - u8 mask, u8 val) +static inline int ksz_prmw8(struct ksz_device *dev, int port, int offset, + u8 mask, u8 val) { - regmap_update_bits(dev->regmap[0], - dev->dev_ops->get_port_addr(port, offset), - mask, val); + int ret; + + ret = regmap_update_bits(dev->regmap[0], + dev->dev_ops->get_port_addr(port, offset), + mask, val); + if (ret) + dev_err(dev->dev, "can't rmw 8bit reg 0x%x: %pe\n", + dev->dev_ops->get_port_addr(port, offset), + ERR_PTR(ret)); + + return ret; + } static inline void ksz_regmap_lock(void *__mtx) -- 2.39.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v1 1/5] net: dsa: microchip: improving error handling for 8-bit register RMW operations 2023-05-24 12:32 ` [PATCH net-next v1 1/5] net: dsa: microchip: improving error handling for 8-bit register RMW operations Oleksij Rempel @ 2023-05-24 12:39 ` Russell King (Oracle) 2023-05-24 17:32 ` Oleksij Rempel 0 siblings, 1 reply; 13+ messages in thread From: Russell King (Oracle) @ 2023-05-24 12:39 UTC (permalink / raw) To: Oleksij Rempel Cc: David S. Miller, Andrew Lunn, Eric Dumazet, Florian Fainelli, Jakub Kicinski, Paolo Abeni, Vladimir Oltean, Woojung Huh, Arun Ramadoss, kernel, linux-kernel, netdev, UNGLinuxDriver On Wed, May 24, 2023 at 02:32:16PM +0200, Oleksij Rempel wrote: > This patch refines the error handling mechanism for 8-bit register > read-modify-write operations. In case of a failure, it now logs an error > message detailing the problematic offset. This enhancement aids in > debugging by providing more precise information when these operations > encounter issues. > > Furthermore, the ksz_prmw8() function has been updated to return error > values rather than void, enabling calling functions to appropriately > respond to errors. What happens when there is an error that affects the current and future accesses? Does this PHY driver then begin to flood the kernel log? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v1 1/5] net: dsa: microchip: improving error handling for 8-bit register RMW operations 2023-05-24 12:39 ` Russell King (Oracle) @ 2023-05-24 17:32 ` Oleksij Rempel 0 siblings, 0 replies; 13+ messages in thread From: Oleksij Rempel @ 2023-05-24 17:32 UTC (permalink / raw) To: Russell King (Oracle) Cc: Woojung Huh, Andrew Lunn, Arun Ramadoss, Florian Fainelli, netdev, linux-kernel, UNGLinuxDriver, Eric Dumazet, Vladimir Oltean, kernel, Jakub Kicinski, Paolo Abeni, David S. Miller On Wed, May 24, 2023 at 01:39:53PM +0100, Russell King (Oracle) wrote: > On Wed, May 24, 2023 at 02:32:16PM +0200, Oleksij Rempel wrote: > > This patch refines the error handling mechanism for 8-bit register > > read-modify-write operations. In case of a failure, it now logs an error > > message detailing the problematic offset. This enhancement aids in > > debugging by providing more precise information when these operations > > encounter issues. > > > > Furthermore, the ksz_prmw8() function has been updated to return error > > values rather than void, enabling calling functions to appropriately > > respond to errors. > > What happens when there is an error that affects the current and future > accesses? Does this PHY driver then begin to flood the kernel log? Yes. In the same way as it is done in all ksz_read*/ksz_write* helpers. -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next v1 2/5] net: dsa: microchip: add an enum for regmap widths 2023-05-24 12:32 [PATCH net-next v1 0/5] Microchip DSA Driver Improvements Oleksij Rempel 2023-05-24 12:32 ` [PATCH net-next v1 1/5] net: dsa: microchip: improving error handling for 8-bit register RMW operations Oleksij Rempel @ 2023-05-24 12:32 ` Oleksij Rempel 2023-05-24 17:03 ` Andrew Lunn 2023-05-24 12:32 ` [PATCH net-next v1 3/5] net: dsa: microchip: remove ksz_port:on variable Oleksij Rempel ` (2 subsequent siblings) 4 siblings, 1 reply; 13+ messages in thread From: Oleksij Rempel @ 2023-05-24 12:32 UTC (permalink / raw) To: David S. Miller, Andrew Lunn, Eric Dumazet, Florian Fainelli, Jakub Kicinski, Paolo Abeni, Vladimir Oltean, Woojung Huh, Arun Ramadoss Cc: Vladimir Oltean, Oleksij Rempel, kernel, linux-kernel, netdev, UNGLinuxDriver, Russell King (Oracle) From: Vladimir Oltean <vladimir.oltean@nxp.com> It is not immediately obvious that this driver allocates, via the KSZ_REGMAP_TABLE() macro, 3 regmaps for register access: dev->regmap[0] for 8-bit access, dev->regmap[1] for 16-bit and dev->regmap[2] for 32-bit access. In future changes that add support for reg_fields, each field will have to specify through which of the 3 regmaps it's going to go. Add an enum now, to denote one of the 3 register access widths, and make the code go through some wrapper functions for easier review and further modification. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> --- drivers/net/dsa/microchip/ksz8795.c | 8 ++-- drivers/net/dsa/microchip/ksz8863_smi.c | 2 +- drivers/net/dsa/microchip/ksz9477.c | 24 +++++------ drivers/net/dsa/microchip/ksz9477_i2c.c | 2 +- drivers/net/dsa/microchip/ksz_common.c | 6 +-- drivers/net/dsa/microchip/ksz_common.h | 54 ++++++++++++++++-------- drivers/net/dsa/microchip/ksz_spi.c | 2 +- drivers/net/dsa/microchip/lan937x_main.c | 8 ++-- 8 files changed, 63 insertions(+), 43 deletions(-) diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c index f56fca1b1a22..d2f7fa3fbd27 100644 --- a/drivers/net/dsa/microchip/ksz8795.c +++ b/drivers/net/dsa/microchip/ksz8795.c @@ -28,13 +28,13 @@ static void ksz_cfg(struct ksz_device *dev, u32 addr, u8 bits, bool set) { - regmap_update_bits(dev->regmap[0], addr, bits, set ? bits : 0); + regmap_update_bits(ksz_regmap_8(dev), addr, bits, set ? bits : 0); } static void ksz_port_cfg(struct ksz_device *dev, int port, int offset, u8 bits, bool set) { - regmap_update_bits(dev->regmap[0], PORT_CTRL_ADDR(port, offset), + regmap_update_bits(ksz_regmap_8(dev), PORT_CTRL_ADDR(port, offset), bits, set ? bits : 0); } @@ -1425,14 +1425,14 @@ int ksz8_setup(struct dsa_switch *ds) ksz_cfg(dev, S_LINK_AGING_CTRL, SW_LINK_AUTO_AGING, true); /* Enable aggressive back off algorithm in half duplex mode. */ - regmap_update_bits(dev->regmap[0], REG_SW_CTRL_1, + regmap_update_bits(ksz_regmap_8(dev), REG_SW_CTRL_1, SW_AGGR_BACKOFF, SW_AGGR_BACKOFF); /* * Make sure unicast VLAN boundary is set as default and * enable no excessive collision drop. */ - regmap_update_bits(dev->regmap[0], REG_SW_CTRL_2, + regmap_update_bits(ksz_regmap_8(dev), REG_SW_CTRL_2, UNICAST_VLAN_BOUNDARY | NO_EXC_COLLISION_DROP, UNICAST_VLAN_BOUNDARY | NO_EXC_COLLISION_DROP); diff --git a/drivers/net/dsa/microchip/ksz8863_smi.c b/drivers/net/dsa/microchip/ksz8863_smi.c index 3698112138b7..2af807db0b45 100644 --- a/drivers/net/dsa/microchip/ksz8863_smi.c +++ b/drivers/net/dsa/microchip/ksz8863_smi.c @@ -136,7 +136,7 @@ static int ksz8863_smi_probe(struct mdio_device *mdiodev) if (!dev) return -ENOMEM; - for (i = 0; i < ARRAY_SIZE(ksz8863_regmap_config); i++) { + for (i = 0; i < __KSZ_NUM_REGMAPS; i++) { rc = ksz8863_regmap_config[i]; rc.lock_arg = &dev->regmap_mutex; dev->regmap[i] = devm_regmap_init(&mdiodev->dev, diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c index bf13d47c26cf..3019f54049fc 100644 --- a/drivers/net/dsa/microchip/ksz9477.c +++ b/drivers/net/dsa/microchip/ksz9477.c @@ -21,25 +21,25 @@ static void ksz_cfg(struct ksz_device *dev, u32 addr, u8 bits, bool set) { - regmap_update_bits(dev->regmap[0], addr, bits, set ? bits : 0); + regmap_update_bits(ksz_regmap_8(dev), addr, bits, set ? bits : 0); } static void ksz_port_cfg(struct ksz_device *dev, int port, int offset, u8 bits, bool set) { - regmap_update_bits(dev->regmap[0], PORT_CTRL_ADDR(port, offset), + regmap_update_bits(ksz_regmap_8(dev), PORT_CTRL_ADDR(port, offset), bits, set ? bits : 0); } static void ksz9477_cfg32(struct ksz_device *dev, u32 addr, u32 bits, bool set) { - regmap_update_bits(dev->regmap[2], addr, bits, set ? bits : 0); + regmap_update_bits(ksz_regmap_32(dev), addr, bits, set ? bits : 0); } static void ksz9477_port_cfg32(struct ksz_device *dev, int port, int offset, u32 bits, bool set) { - regmap_update_bits(dev->regmap[2], PORT_CTRL_ADDR(port, offset), + regmap_update_bits(ksz_regmap_32(dev), PORT_CTRL_ADDR(port, offset), bits, set ? bits : 0); } @@ -52,7 +52,7 @@ int ksz9477_change_mtu(struct ksz_device *dev, int port, int mtu) frame_size = mtu + VLAN_ETH_HLEN + ETH_FCS_LEN; - return regmap_update_bits(dev->regmap[1], REG_SW_MTU__2, + return regmap_update_bits(ksz_regmap_16(dev), REG_SW_MTU__2, REG_SW_MTU_MASK, frame_size); } @@ -60,7 +60,7 @@ static int ksz9477_wait_vlan_ctrl_ready(struct ksz_device *dev) { unsigned int val; - return regmap_read_poll_timeout(dev->regmap[0], REG_SW_VLAN_CTRL, + return regmap_read_poll_timeout(ksz_regmap_8(dev), REG_SW_VLAN_CTRL, val, !(val & VLAN_START), 10, 1000); } @@ -147,7 +147,7 @@ static int ksz9477_wait_alu_ready(struct ksz_device *dev) { unsigned int val; - return regmap_read_poll_timeout(dev->regmap[2], REG_SW_ALU_CTRL__4, + return regmap_read_poll_timeout(ksz_regmap_32(dev), REG_SW_ALU_CTRL__4, val, !(val & ALU_START), 10, 1000); } @@ -155,7 +155,7 @@ static int ksz9477_wait_alu_sta_ready(struct ksz_device *dev) { unsigned int val; - return regmap_read_poll_timeout(dev->regmap[2], + return regmap_read_poll_timeout(ksz_regmap_32(dev), REG_SW_ALU_STAT_CTRL__4, val, !(val & ALU_STAT_START), 10, 1000); @@ -170,7 +170,7 @@ int ksz9477_reset_switch(struct ksz_device *dev) ksz_cfg(dev, REG_SW_OPERATION, SW_RESET, true); /* turn off SPI DO Edge select */ - regmap_update_bits(dev->regmap[0], REG_SW_GLOBAL_SERIAL_CTRL_0, + regmap_update_bits(ksz_regmap_8(dev), REG_SW_GLOBAL_SERIAL_CTRL_0, SPI_AUTO_EDGE_DETECTION, 0); /* default configuration */ @@ -213,7 +213,7 @@ void ksz9477_r_mib_cnt(struct ksz_device *dev, int port, u16 addr, u64 *cnt) data |= (addr << MIB_COUNTER_INDEX_S); ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, data); - ret = regmap_read_poll_timeout(dev->regmap[2], + ret = regmap_read_poll_timeout(ksz_regmap_32(dev), PORT_CTRL_ADDR(port, REG_PORT_MIB_CTRL_STAT__4), val, !(val & MIB_COUNTER_READ), 10, 1000); /* failed to read MIB. get out of loop */ @@ -346,7 +346,7 @@ void ksz9477_flush_dyn_mac_table(struct ksz_device *dev, int port) const u16 *regs = dev->info->regs; u8 data; - regmap_update_bits(dev->regmap[0], REG_SW_LUE_CTRL_2, + regmap_update_bits(ksz_regmap_8(dev), REG_SW_LUE_CTRL_2, SW_FLUSH_OPTION_M << SW_FLUSH_OPTION_S, SW_FLUSH_OPTION_DYN_MAC << SW_FLUSH_OPTION_S); @@ -1165,7 +1165,7 @@ int ksz9477_setup(struct dsa_switch *ds) ksz_cfg(dev, REG_SW_MAC_CTRL_1, SW_JUMBO_PACKET, true); /* Now we can configure default MTU value */ - ret = regmap_update_bits(dev->regmap[1], REG_SW_MTU__2, REG_SW_MTU_MASK, + ret = regmap_update_bits(ksz_regmap_16(dev), REG_SW_MTU__2, REG_SW_MTU_MASK, VLAN_ETH_FRAME_LEN + ETH_FCS_LEN); if (ret) return ret; diff --git a/drivers/net/dsa/microchip/ksz9477_i2c.c b/drivers/net/dsa/microchip/ksz9477_i2c.c index 97a317263a2f..497be833f707 100644 --- a/drivers/net/dsa/microchip/ksz9477_i2c.c +++ b/drivers/net/dsa/microchip/ksz9477_i2c.c @@ -24,7 +24,7 @@ static int ksz9477_i2c_probe(struct i2c_client *i2c) if (!dev) return -ENOMEM; - for (i = 0; i < ARRAY_SIZE(ksz9477_regmap_config); i++) { + for (i = 0; i < __KSZ_NUM_REGMAPS; i++) { rc = ksz9477_regmap_config[i]; rc.lock_arg = &dev->regmap_mutex; dev->regmap[i] = devm_regmap_init_i2c(i2c, &rc); diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c index a4428be5f483..53bb7d9712d0 100644 --- a/drivers/net/dsa/microchip/ksz_common.c +++ b/drivers/net/dsa/microchip/ksz_common.c @@ -2095,7 +2095,7 @@ static int ksz_setup(struct dsa_switch *ds) } /* set broadcast storm protection 10% rate */ - regmap_update_bits(dev->regmap[1], regs[S_BROADCAST_CTRL], + regmap_update_bits(ksz_regmap_16(dev), regs[S_BROADCAST_CTRL], BROADCAST_STORM_RATE, (BROADCAST_STORM_VALUE * BROADCAST_STORM_PROT_RATE) / 100); @@ -2106,7 +2106,7 @@ static int ksz_setup(struct dsa_switch *ds) ds->num_tx_queues = dev->info->num_tx_queues; - regmap_update_bits(dev->regmap[0], regs[S_MULTICAST_CTRL], + regmap_update_bits(ksz_regmap_8(dev), regs[S_MULTICAST_CTRL], MULTICAST_STORM_DISABLE, MULTICAST_STORM_DISABLE); ksz_init_mib_timer(dev); @@ -2156,7 +2156,7 @@ static int ksz_setup(struct dsa_switch *ds) } /* start switch */ - regmap_update_bits(dev->regmap[0], regs[S_START_CTRL], + regmap_update_bits(ksz_regmap_8(dev), regs[S_START_CTRL], SW_START, SW_START); return 0; diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h index b86f1e27a0c3..f45f5fe11bde 100644 --- a/drivers/net/dsa/microchip/ksz_common.h +++ b/drivers/net/dsa/microchip/ksz_common.h @@ -22,6 +22,13 @@ struct ksz_device; struct ksz_port; +enum ksz_regmap_width { + KSZ_REGMAP_8, + KSZ_REGMAP_16, + KSZ_REGMAP_32, + __KSZ_NUM_REGMAPS, +}; + struct vlan_table { u32 table[3]; }; @@ -137,7 +144,7 @@ struct ksz_device { const struct ksz_dev_ops *dev_ops; struct device *dev; - struct regmap *regmap[3]; + struct regmap *regmap[__KSZ_NUM_REGMAPS]; void *priv; int irq; @@ -377,11 +384,25 @@ phy_interface_t ksz_get_xmii(struct ksz_device *dev, int port, bool gbit); extern const struct ksz_chip_data ksz_switch_chips[]; /* Common register access functions */ +static inline struct regmap *ksz_regmap_8(struct ksz_device *dev) +{ + return dev->regmap[KSZ_REGMAP_8]; +} + +static inline struct regmap *ksz_regmap_16(struct ksz_device *dev) +{ + return dev->regmap[KSZ_REGMAP_16]; +} + +static inline struct regmap *ksz_regmap_32(struct ksz_device *dev) +{ + return dev->regmap[KSZ_REGMAP_32]; +} static inline int ksz_read8(struct ksz_device *dev, u32 reg, u8 *val) { unsigned int value; - int ret = regmap_read(dev->regmap[0], reg, &value); + int ret = regmap_read(ksz_regmap_8(dev), reg, &value); if (ret) dev_err(dev->dev, "can't read 8bit reg: 0x%x %pe\n", reg, @@ -394,7 +415,7 @@ static inline int ksz_read8(struct ksz_device *dev, u32 reg, u8 *val) static inline int ksz_read16(struct ksz_device *dev, u32 reg, u16 *val) { unsigned int value; - int ret = regmap_read(dev->regmap[1], reg, &value); + int ret = regmap_read(ksz_regmap_16(dev), reg, &value); if (ret) dev_err(dev->dev, "can't read 16bit reg: 0x%x %pe\n", reg, @@ -407,7 +428,7 @@ static inline int ksz_read16(struct ksz_device *dev, u32 reg, u16 *val) static inline int ksz_read32(struct ksz_device *dev, u32 reg, u32 *val) { unsigned int value; - int ret = regmap_read(dev->regmap[2], reg, &value); + int ret = regmap_read(ksz_regmap_32(dev), reg, &value); if (ret) dev_err(dev->dev, "can't read 32bit reg: 0x%x %pe\n", reg, @@ -422,7 +443,7 @@ static inline int ksz_read64(struct ksz_device *dev, u32 reg, u64 *val) u32 value[2]; int ret; - ret = regmap_bulk_read(dev->regmap[2], reg, value, 2); + ret = regmap_bulk_read(ksz_regmap_32(dev), reg, value, 2); if (ret) dev_err(dev->dev, "can't read 64bit reg: 0x%x %pe\n", reg, ERR_PTR(ret)); @@ -436,7 +457,7 @@ static inline int ksz_write8(struct ksz_device *dev, u32 reg, u8 value) { int ret; - ret = regmap_write(dev->regmap[0], reg, value); + ret = regmap_write(ksz_regmap_8(dev), reg, value); if (ret) dev_err(dev->dev, "can't write 8bit reg: 0x%x %pe\n", reg, ERR_PTR(ret)); @@ -448,7 +469,7 @@ static inline int ksz_write16(struct ksz_device *dev, u32 reg, u16 value) { int ret; - ret = regmap_write(dev->regmap[1], reg, value); + ret = regmap_write(ksz_regmap_16(dev), reg, value); if (ret) dev_err(dev->dev, "can't write 16bit reg: 0x%x %pe\n", reg, ERR_PTR(ret)); @@ -460,7 +481,7 @@ static inline int ksz_write32(struct ksz_device *dev, u32 reg, u32 value) { int ret; - ret = regmap_write(dev->regmap[2], reg, value); + ret = regmap_write(ksz_regmap_32(dev), reg, value); if (ret) dev_err(dev->dev, "can't write 32bit reg: 0x%x %pe\n", reg, ERR_PTR(ret)); @@ -473,7 +494,7 @@ static inline int ksz_rmw16(struct ksz_device *dev, u32 reg, u16 mask, { int ret; - ret = regmap_update_bits(dev->regmap[1], reg, mask, value); + ret = regmap_update_bits(ksz_regmap_16(dev), reg, mask, value); if (ret) dev_err(dev->dev, "can't rmw 16bit reg 0x%x: %pe\n", reg, ERR_PTR(ret)); @@ -486,7 +507,7 @@ static inline int ksz_rmw32(struct ksz_device *dev, u32 reg, u32 mask, { int ret; - ret = regmap_update_bits(dev->regmap[2], reg, mask, value); + ret = regmap_update_bits(ksz_regmap_32(dev), reg, mask, value); if (ret) dev_err(dev->dev, "can't rmw 32bit reg 0x%x: %pe\n", reg, ERR_PTR(ret)); @@ -503,14 +524,14 @@ static inline int ksz_write64(struct ksz_device *dev, u32 reg, u64 value) val[0] = swab32(value & 0xffffffffULL); val[1] = swab32(value >> 32ULL); - return regmap_bulk_write(dev->regmap[2], reg, val, 2); + return regmap_bulk_write(ksz_regmap_32(dev), reg, val, 2); } static inline int ksz_rmw8(struct ksz_device *dev, int offset, u8 mask, u8 val) { int ret; - ret = regmap_update_bits(dev->regmap[0], offset, mask, val); + ret = regmap_update_bits(ksz_regmap_8(dev), offset, mask, val); if (ret) dev_err(dev->dev, "can't rmw 8bit reg 0x%x: %pe\n", offset, ERR_PTR(ret)); @@ -561,7 +582,7 @@ static inline int ksz_prmw8(struct ksz_device *dev, int port, int offset, { int ret; - ret = regmap_update_bits(dev->regmap[0], + ret = regmap_update_bits(ksz_regmap_8(dev), dev->dev_ops->get_port_addr(port, offset), mask, val); if (ret) @@ -570,7 +591,6 @@ static inline int ksz_prmw8(struct ksz_device *dev, int port, int offset, ERR_PTR(ret)); return ret; - } static inline void ksz_regmap_lock(void *__mtx) @@ -725,9 +745,9 @@ static inline int is_lan937x(struct ksz_device *dev) #define KSZ_REGMAP_TABLE(ksz, swp, regbits, regpad, regalign) \ static const struct regmap_config ksz##_regmap_config[] = { \ - KSZ_REGMAP_ENTRY(8, swp, (regbits), (regpad), (regalign)), \ - KSZ_REGMAP_ENTRY(16, swp, (regbits), (regpad), (regalign)), \ - KSZ_REGMAP_ENTRY(32, swp, (regbits), (regpad), (regalign)), \ + [KSZ_REGMAP_8] = KSZ_REGMAP_ENTRY(8, swp, (regbits), (regpad), (regalign)), \ + [KSZ_REGMAP_16] = KSZ_REGMAP_ENTRY(16, swp, (regbits), (regpad), (regalign)), \ + [KSZ_REGMAP_32] = KSZ_REGMAP_ENTRY(32, swp, (regbits), (regpad), (regalign)), \ } #endif diff --git a/drivers/net/dsa/microchip/ksz_spi.c b/drivers/net/dsa/microchip/ksz_spi.c index 96c52e8fb51b..279338451621 100644 --- a/drivers/net/dsa/microchip/ksz_spi.c +++ b/drivers/net/dsa/microchip/ksz_spi.c @@ -63,7 +63,7 @@ static int ksz_spi_probe(struct spi_device *spi) else regmap_config = ksz9477_regmap_config; - for (i = 0; i < ARRAY_SIZE(ksz8795_regmap_config); i++) { + for (i = 0; i < __KSZ_NUM_REGMAPS; i++) { rc = regmap_config[i]; rc.lock_arg = &dev->regmap_mutex; rc.wr_table = chip->wr_table; diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c index 399a3905e6ca..b479a628b1ae 100644 --- a/drivers/net/dsa/microchip/lan937x_main.c +++ b/drivers/net/dsa/microchip/lan937x_main.c @@ -20,13 +20,13 @@ static int lan937x_cfg(struct ksz_device *dev, u32 addr, u8 bits, bool set) { - return regmap_update_bits(dev->regmap[0], addr, bits, set ? bits : 0); + return regmap_update_bits(ksz_regmap_8(dev), addr, bits, set ? bits : 0); } static int lan937x_port_cfg(struct ksz_device *dev, int port, int offset, u8 bits, bool set) { - return regmap_update_bits(dev->regmap[0], PORT_CTRL_ADDR(port, offset), + return regmap_update_bits(ksz_regmap_8(dev), PORT_CTRL_ADDR(port, offset), bits, set ? bits : 0); } @@ -86,7 +86,7 @@ static int lan937x_internal_phy_write(struct ksz_device *dev, int addr, int reg, if (ret < 0) return ret; - ret = regmap_read_poll_timeout(dev->regmap[1], REG_VPHY_IND_CTRL__2, + ret = regmap_read_poll_timeout(ksz_regmap_16(dev), REG_VPHY_IND_CTRL__2, value, !(value & VPHY_IND_BUSY), 10, 1000); if (ret < 0) { @@ -116,7 +116,7 @@ static int lan937x_internal_phy_read(struct ksz_device *dev, int addr, int reg, if (ret < 0) return ret; - ret = regmap_read_poll_timeout(dev->regmap[1], REG_VPHY_IND_CTRL__2, + ret = regmap_read_poll_timeout(ksz_regmap_16(dev), REG_VPHY_IND_CTRL__2, value, !(value & VPHY_IND_BUSY), 10, 1000); if (ret < 0) { -- 2.39.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v1 2/5] net: dsa: microchip: add an enum for regmap widths 2023-05-24 12:32 ` [PATCH net-next v1 2/5] net: dsa: microchip: add an enum for regmap widths Oleksij Rempel @ 2023-05-24 17:03 ` Andrew Lunn 2023-05-24 17:53 ` Oleksij Rempel 0 siblings, 1 reply; 13+ messages in thread From: Andrew Lunn @ 2023-05-24 17:03 UTC (permalink / raw) To: Oleksij Rempel Cc: David S. Miller, Eric Dumazet, Florian Fainelli, Jakub Kicinski, Paolo Abeni, Vladimir Oltean, Woojung Huh, Arun Ramadoss, Vladimir Oltean, kernel, linux-kernel, netdev, UNGLinuxDriver, Russell King (Oracle) On Wed, May 24, 2023 at 02:32:17PM +0200, Oleksij Rempel wrote: > From: Vladimir Oltean <vladimir.oltean@nxp.com> > > It is not immediately obvious that this driver allocates, via the > KSZ_REGMAP_TABLE() macro, 3 regmaps for register access: dev->regmap[0] > for 8-bit access, dev->regmap[1] for 16-bit and dev->regmap[2] for > 32-bit access. > > In future changes that add support for reg_fields, each field will have > to specify through which of the 3 regmaps it's going to go. Add an enum > now, to denote one of the 3 register access widths, and make the code go > through some wrapper functions for easier review and further > modification. Given the patches in this series, it is not obvious why the wrapper is needed. dev->regmap[KSZ_REGMAP_8] is just as readable as ksz_regmap_8(dev). Do future changes add extra parameters to ksz_regmap_8()? Andrew ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v1 2/5] net: dsa: microchip: add an enum for regmap widths 2023-05-24 17:03 ` Andrew Lunn @ 2023-05-24 17:53 ` Oleksij Rempel 0 siblings, 0 replies; 13+ messages in thread From: Oleksij Rempel @ 2023-05-24 17:53 UTC (permalink / raw) To: Andrew Lunn Cc: Woojung Huh, Arun Ramadoss, Florian Fainelli, Russell King (Oracle), Vladimir Oltean, linux-kernel, UNGLinuxDriver, Eric Dumazet, Vladimir Oltean, kernel, netdev, Jakub Kicinski, Paolo Abeni, David S. Miller On Wed, May 24, 2023 at 07:03:38PM +0200, Andrew Lunn wrote: > On Wed, May 24, 2023 at 02:32:17PM +0200, Oleksij Rempel wrote: > > From: Vladimir Oltean <vladimir.oltean@nxp.com> > > > > It is not immediately obvious that this driver allocates, via the > > KSZ_REGMAP_TABLE() macro, 3 regmaps for register access: dev->regmap[0] > > for 8-bit access, dev->regmap[1] for 16-bit and dev->regmap[2] for > > 32-bit access. > > > > In future changes that add support for reg_fields, each field will have > > to specify through which of the 3 regmaps it's going to go. Add an enum > > now, to denote one of the 3 register access widths, and make the code go > > through some wrapper functions for easier review and further > > modification. > > Given the patches in this series, it is not obvious why the wrapper is > needed. > > dev->regmap[KSZ_REGMAP_8] is just as readable as ksz_regmap_8(dev). > > Do future changes add extra parameters to ksz_regmap_8()? As for me, it looks short. Less problems with line length restrictions. -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next v1 3/5] net: dsa: microchip: remove ksz_port:on variable 2023-05-24 12:32 [PATCH net-next v1 0/5] Microchip DSA Driver Improvements Oleksij Rempel 2023-05-24 12:32 ` [PATCH net-next v1 1/5] net: dsa: microchip: improving error handling for 8-bit register RMW operations Oleksij Rempel 2023-05-24 12:32 ` [PATCH net-next v1 2/5] net: dsa: microchip: add an enum for regmap widths Oleksij Rempel @ 2023-05-24 12:32 ` Oleksij Rempel 2023-05-24 16:51 ` Andrew Lunn 2023-05-24 12:32 ` [PATCH net-next v1 4/5] net: dsa: microchip: ksz8: Prepare ksz8863_smi for regmap register access validation Oleksij Rempel 2023-05-24 12:32 ` [PATCH net-next v1 5/5] net: dsa: microchip: Add register access control for KSZ8873 chip Oleksij Rempel 4 siblings, 1 reply; 13+ messages in thread From: Oleksij Rempel @ 2023-05-24 12:32 UTC (permalink / raw) To: David S. Miller, Andrew Lunn, Eric Dumazet, Florian Fainelli, Jakub Kicinski, Paolo Abeni, Vladimir Oltean, Woojung Huh, Arun Ramadoss Cc: Oleksij Rempel, kernel, linux-kernel, netdev, UNGLinuxDriver, Russell King (Oracle) The only place where this variable would be set to false is the ksz8_config_cpu_port() function. But it is done in a bogus way: for (i = 0; i < dev->phy_port_cnt; i++) { if (i == dev->phy_port_cnt) <--- will be never executed. break; p->on = 1; So, we never have a situation where p->on = 0. In this case, we can just remove it. Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> --- drivers/net/dsa/microchip/ksz8795.c | 20 +------------------- drivers/net/dsa/microchip/ksz_common.h | 1 - 2 files changed, 1 insertion(+), 20 deletions(-) diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c index d2f7fa3fbd27..84d502589f8e 100644 --- a/drivers/net/dsa/microchip/ksz8795.c +++ b/drivers/net/dsa/microchip/ksz8795.c @@ -941,7 +941,6 @@ void ksz8_flush_dyn_mac_table(struct ksz_device *dev, int port) { u8 learn[DSA_MAX_PORTS]; int first, index, cnt; - struct ksz_port *p; const u16 *regs; regs = dev->info->regs; @@ -955,9 +954,6 @@ void ksz8_flush_dyn_mac_table(struct ksz_device *dev, int port) cnt = dev->info->port_cnt; } for (index = first; index < cnt; index++) { - p = &dev->ports[index]; - if (!p->on) - continue; ksz_pread8(dev, index, regs[P_STP_CTRL], &learn[index]); if (!(learn[index] & PORT_LEARN_DISABLE)) ksz_pwrite8(dev, index, regs[P_STP_CTRL], @@ -965,9 +961,6 @@ void ksz8_flush_dyn_mac_table(struct ksz_device *dev, int port) } ksz_cfg(dev, S_FLUSH_TABLE_CTRL, SW_FLUSH_DYN_MAC_TABLE, true); for (index = first; index < cnt; index++) { - p = &dev->ports[index]; - if (!p->on) - continue; if (!(learn[index] & PORT_LEARN_DISABLE)) ksz_pwrite8(dev, index, regs[P_STP_CTRL], learn[index]); } @@ -1338,25 +1331,14 @@ void ksz8_config_cpu_port(struct dsa_switch *ds) ksz_cfg(dev, regs[S_TAIL_TAG_CTRL], masks[SW_TAIL_TAG_ENABLE], true); - p = &dev->ports[dev->cpu_port]; - p->on = 1; - ksz8_port_setup(dev, dev->cpu_port, true); for (i = 0; i < dev->phy_port_cnt; i++) { - p = &dev->ports[i]; - ksz_port_stp_state_set(ds, i, BR_STATE_DISABLED); - - /* Last port may be disabled. */ - if (i == dev->phy_port_cnt) - break; - p->on = 1; } for (i = 0; i < dev->phy_port_cnt; i++) { p = &dev->ports[i]; - if (!p->on) - continue; + if (!ksz_is_ksz88x3(dev)) { ksz_pread8(dev, i, regs[P_REMOTE_STATUS], &remote); if (remote & KSZ8_PORT_FIBER_MODE) diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h index f45f5fe11bde..5aa58aac3e07 100644 --- a/drivers/net/dsa/microchip/ksz_common.h +++ b/drivers/net/dsa/microchip/ksz_common.h @@ -108,7 +108,6 @@ struct ksz_port { int stp_state; struct phy_device phydev; - u32 on:1; /* port is not disabled by hardware */ u32 fiber:1; /* port is fiber */ u32 force:1; u32 read:1; /* read MIB counters in background */ -- 2.39.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v1 3/5] net: dsa: microchip: remove ksz_port:on variable 2023-05-24 12:32 ` [PATCH net-next v1 3/5] net: dsa: microchip: remove ksz_port:on variable Oleksij Rempel @ 2023-05-24 16:51 ` Andrew Lunn 0 siblings, 0 replies; 13+ messages in thread From: Andrew Lunn @ 2023-05-24 16:51 UTC (permalink / raw) To: Oleksij Rempel Cc: David S. Miller, Eric Dumazet, Florian Fainelli, Jakub Kicinski, Paolo Abeni, Vladimir Oltean, Woojung Huh, Arun Ramadoss, kernel, linux-kernel, netdev, UNGLinuxDriver, Russell King (Oracle) On Wed, May 24, 2023 at 02:32:18PM +0200, Oleksij Rempel wrote: > The only place where this variable would be set to false is the > ksz8_config_cpu_port() function. But it is done in a bogus way: > > for (i = 0; i < dev->phy_port_cnt; i++) { > if (i == dev->phy_port_cnt) <--- will be never executed. > break; > p->on = 1; > > So, we never have a situation where p->on = 0. In this case, we can just > remove it. > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next v1 4/5] net: dsa: microchip: ksz8: Prepare ksz8863_smi for regmap register access validation 2023-05-24 12:32 [PATCH net-next v1 0/5] Microchip DSA Driver Improvements Oleksij Rempel ` (2 preceding siblings ...) 2023-05-24 12:32 ` [PATCH net-next v1 3/5] net: dsa: microchip: remove ksz_port:on variable Oleksij Rempel @ 2023-05-24 12:32 ` Oleksij Rempel 2023-05-24 16:59 ` Andrew Lunn 2023-05-24 12:32 ` [PATCH net-next v1 5/5] net: dsa: microchip: Add register access control for KSZ8873 chip Oleksij Rempel 4 siblings, 1 reply; 13+ messages in thread From: Oleksij Rempel @ 2023-05-24 12:32 UTC (permalink / raw) To: David S. Miller, Andrew Lunn, Eric Dumazet, Florian Fainelli, Jakub Kicinski, Paolo Abeni, Vladimir Oltean, Woojung Huh, Arun Ramadoss Cc: Oleksij Rempel, kernel, linux-kernel, netdev, UNGLinuxDriver, Russell King (Oracle) This patch prepares the ksz8863_smi part of ksz8 driver to utilize the regmap register access validation feature. Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> --- drivers/net/dsa/microchip/ksz8863_smi.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/drivers/net/dsa/microchip/ksz8863_smi.c b/drivers/net/dsa/microchip/ksz8863_smi.c index 2af807db0b45..303a4707c759 100644 --- a/drivers/net/dsa/microchip/ksz8863_smi.c +++ b/drivers/net/dsa/microchip/ksz8863_smi.c @@ -104,6 +104,7 @@ static const struct regmap_config ksz8863_regmap_config[] = { .cache_type = REGCACHE_NONE, .lock = ksz_regmap_lock, .unlock = ksz_regmap_unlock, + .max_register = BIT(8) - 1, }, { .name = "#16", @@ -113,6 +114,7 @@ static const struct regmap_config ksz8863_regmap_config[] = { .cache_type = REGCACHE_NONE, .lock = ksz_regmap_lock, .unlock = ksz_regmap_unlock, + .max_register = BIT(8) - 2, }, { .name = "#32", @@ -122,11 +124,14 @@ static const struct regmap_config ksz8863_regmap_config[] = { .cache_type = REGCACHE_NONE, .lock = ksz_regmap_lock, .unlock = ksz_regmap_unlock, + .max_register = BIT(8) - 4, } }; static int ksz8863_smi_probe(struct mdio_device *mdiodev) { + struct device *ddev = &mdiodev->dev; + const struct ksz_chip_data *chip; struct regmap_config rc; struct ksz_device *dev; int ret; @@ -136,9 +141,15 @@ static int ksz8863_smi_probe(struct mdio_device *mdiodev) if (!dev) return -ENOMEM; + chip = device_get_match_data(ddev); + if (!chip) + return -EINVAL; + for (i = 0; i < __KSZ_NUM_REGMAPS; i++) { rc = ksz8863_regmap_config[i]; rc.lock_arg = &dev->regmap_mutex; + rc.wr_table = chip->wr_table; + rc.rd_table = chip->rd_table; dev->regmap[i] = devm_regmap_init(&mdiodev->dev, ®map_smi[i], dev, &rc); -- 2.39.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v1 4/5] net: dsa: microchip: ksz8: Prepare ksz8863_smi for regmap register access validation 2023-05-24 12:32 ` [PATCH net-next v1 4/5] net: dsa: microchip: ksz8: Prepare ksz8863_smi for regmap register access validation Oleksij Rempel @ 2023-05-24 16:59 ` Andrew Lunn 2023-05-24 17:56 ` Oleksij Rempel 0 siblings, 1 reply; 13+ messages in thread From: Andrew Lunn @ 2023-05-24 16:59 UTC (permalink / raw) To: Oleksij Rempel Cc: David S. Miller, Eric Dumazet, Florian Fainelli, Jakub Kicinski, Paolo Abeni, Vladimir Oltean, Woojung Huh, Arun Ramadoss, kernel, linux-kernel, netdev, UNGLinuxDriver, Russell King (Oracle) On Wed, May 24, 2023 at 02:32:19PM +0200, Oleksij Rempel wrote: > This patch prepares the ksz8863_smi part of ksz8 driver to utilize the > regmap register access validation feature. > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > --- > drivers/net/dsa/microchip/ksz8863_smi.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/net/dsa/microchip/ksz8863_smi.c b/drivers/net/dsa/microchip/ksz8863_smi.c > index 2af807db0b45..303a4707c759 100644 > --- a/drivers/net/dsa/microchip/ksz8863_smi.c > +++ b/drivers/net/dsa/microchip/ksz8863_smi.c > @@ -104,6 +104,7 @@ static const struct regmap_config ksz8863_regmap_config[] = { > .cache_type = REGCACHE_NONE, > .lock = ksz_regmap_lock, > .unlock = ksz_regmap_unlock, > + .max_register = BIT(8) - 1, Maybe SZ_256 - 1 is more readable? > }, > { > .name = "#16", > @@ -113,6 +114,7 @@ static const struct regmap_config ksz8863_regmap_config[] = { > .cache_type = REGCACHE_NONE, > .lock = ksz_regmap_lock, > .unlock = ksz_regmap_unlock, > + .max_register = BIT(8) - 2, - 2? Is this the 16 bit regmap? So it has 1/2 the number of registers of the 8 bit regmap? So i would of thought it should be BIT(7)-1, or SZ_128-1 ? Andrew ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v1 4/5] net: dsa: microchip: ksz8: Prepare ksz8863_smi for regmap register access validation 2023-05-24 16:59 ` Andrew Lunn @ 2023-05-24 17:56 ` Oleksij Rempel 0 siblings, 0 replies; 13+ messages in thread From: Oleksij Rempel @ 2023-05-24 17:56 UTC (permalink / raw) To: Andrew Lunn Cc: Woojung Huh, Arun Ramadoss, Florian Fainelli, Russell King (Oracle), netdev, linux-kernel, UNGLinuxDriver, Eric Dumazet, Vladimir Oltean, kernel, Jakub Kicinski, Paolo Abeni, David S. Miller On Wed, May 24, 2023 at 06:59:28PM +0200, Andrew Lunn wrote: > On Wed, May 24, 2023 at 02:32:19PM +0200, Oleksij Rempel wrote: > > This patch prepares the ksz8863_smi part of ksz8 driver to utilize the > > regmap register access validation feature. > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > --- > > drivers/net/dsa/microchip/ksz8863_smi.c | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/drivers/net/dsa/microchip/ksz8863_smi.c b/drivers/net/dsa/microchip/ksz8863_smi.c > > index 2af807db0b45..303a4707c759 100644 > > --- a/drivers/net/dsa/microchip/ksz8863_smi.c > > +++ b/drivers/net/dsa/microchip/ksz8863_smi.c > > @@ -104,6 +104,7 @@ static const struct regmap_config ksz8863_regmap_config[] = { > > .cache_type = REGCACHE_NONE, > > .lock = ksz_regmap_lock, > > .unlock = ksz_regmap_unlock, > > + .max_register = BIT(8) - 1, > > Maybe SZ_256 - 1 is more readable? It is the same way used in other regmap_config in this driver. As for me, U8_MAX is probably more understandable way, since addressing since is 8bit. > > }, > > { > > .name = "#16", > > @@ -113,6 +114,7 @@ static const struct regmap_config ksz8863_regmap_config[] = { > > .cache_type = REGCACHE_NONE, > > .lock = ksz_regmap_lock, > > .unlock = ksz_regmap_unlock, > > + .max_register = BIT(8) - 2, > > - 2? > > Is this the 16 bit regmap? So it has 1/2 the number of registers of > the 8 bit regmap? So i would of thought it should be BIT(7)-1, or > SZ_128-1 ? Sorry, it is a typo. -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next v1 5/5] net: dsa: microchip: Add register access control for KSZ8873 chip 2023-05-24 12:32 [PATCH net-next v1 0/5] Microchip DSA Driver Improvements Oleksij Rempel ` (3 preceding siblings ...) 2023-05-24 12:32 ` [PATCH net-next v1 4/5] net: dsa: microchip: ksz8: Prepare ksz8863_smi for regmap register access validation Oleksij Rempel @ 2023-05-24 12:32 ` Oleksij Rempel 4 siblings, 0 replies; 13+ messages in thread From: Oleksij Rempel @ 2023-05-24 12:32 UTC (permalink / raw) To: David S. Miller, Andrew Lunn, Eric Dumazet, Florian Fainelli, Jakub Kicinski, Paolo Abeni, Vladimir Oltean, Woojung Huh, Arun Ramadoss Cc: Oleksij Rempel, kernel, linux-kernel, netdev, UNGLinuxDriver, Russell King (Oracle) This update introduces specific register access boundaries for the KSZ8873 and KSZ8863 chips within the DSA Microchip driver. The outlined ranges target global control registers, port registers, and advanced control registers. Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> --- drivers/net/dsa/microchip/ksz_common.c | 41 ++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c index 53bb7d9712d0..768f649d2f40 100644 --- a/drivers/net/dsa/microchip/ksz_common.c +++ b/drivers/net/dsa/microchip/ksz_common.c @@ -1075,6 +1075,45 @@ static const struct regmap_access_table ksz9896_register_set = { .n_yes_ranges = ARRAY_SIZE(ksz9896_valid_regs), }; +static const struct regmap_range ksz8873_valid_regs[] = { + regmap_reg_range(0x00, 0x01), + /* global control register */ + regmap_reg_range(0x02, 0x0f), + + /* port registers */ + regmap_reg_range(0x10, 0x1d), + regmap_reg_range(0x1e, 0x1f), + regmap_reg_range(0x20, 0x2d), + regmap_reg_range(0x2e, 0x2f), + regmap_reg_range(0x30, 0x39), + regmap_reg_range(0x3f, 0x3f), + + /* advanced control registers */ + regmap_reg_range(0x60, 0x6f), + regmap_reg_range(0x70, 0x75), + regmap_reg_range(0x76, 0x78), + regmap_reg_range(0x79, 0x7a), + regmap_reg_range(0x7b, 0x83), + regmap_reg_range(0x8e, 0x99), + regmap_reg_range(0x9a, 0xa5), + regmap_reg_range(0xa6, 0xa6), + regmap_reg_range(0xa7, 0xaa), + regmap_reg_range(0xab, 0xae), + regmap_reg_range(0xaf, 0xba), + regmap_reg_range(0xbb, 0xbc), + regmap_reg_range(0xbd, 0xbd), + regmap_reg_range(0xc0, 0xc0), + regmap_reg_range(0xc2, 0xc2), + regmap_reg_range(0xc3, 0xc3), + regmap_reg_range(0xc4, 0xc4), + regmap_reg_range(0xc6, 0xc6), +}; + +static const struct regmap_access_table ksz8873_register_set = { + .yes_ranges = ksz8873_valid_regs, + .n_yes_ranges = ARRAY_SIZE(ksz8873_valid_regs), +}; + const struct ksz_chip_data ksz_switch_chips[] = { [KSZ8563] = { .chip_id = KSZ8563_CHIP_ID, @@ -1214,6 +1253,8 @@ const struct ksz_chip_data ksz_switch_chips[] = { .supports_mii = {false, false, true}, .supports_rmii = {false, false, true}, .internal_phy = {true, true, false}, + .wr_table = &ksz8873_register_set, + .rd_table = &ksz8873_register_set, }, [KSZ9477] = { -- 2.39.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2023-05-24 17:56 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-05-24 12:32 [PATCH net-next v1 0/5] Microchip DSA Driver Improvements Oleksij Rempel 2023-05-24 12:32 ` [PATCH net-next v1 1/5] net: dsa: microchip: improving error handling for 8-bit register RMW operations Oleksij Rempel 2023-05-24 12:39 ` Russell King (Oracle) 2023-05-24 17:32 ` Oleksij Rempel 2023-05-24 12:32 ` [PATCH net-next v1 2/5] net: dsa: microchip: add an enum for regmap widths Oleksij Rempel 2023-05-24 17:03 ` Andrew Lunn 2023-05-24 17:53 ` Oleksij Rempel 2023-05-24 12:32 ` [PATCH net-next v1 3/5] net: dsa: microchip: remove ksz_port:on variable Oleksij Rempel 2023-05-24 16:51 ` Andrew Lunn 2023-05-24 12:32 ` [PATCH net-next v1 4/5] net: dsa: microchip: ksz8: Prepare ksz8863_smi for regmap register access validation Oleksij Rempel 2023-05-24 16:59 ` Andrew Lunn 2023-05-24 17:56 ` Oleksij Rempel 2023-05-24 12:32 ` [PATCH net-next v1 5/5] net: dsa: microchip: Add register access control for KSZ8873 chip Oleksij Rempel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).