* Re: [RFC PATCH net-next v3 05/20] net: dsa: qca8k: handle error with qca8k_read operation
@ 2021-05-05 6:26 kernel test robot
0 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2021-05-05 6:26 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 4405 bytes --]
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20210504222915.17206-5-ansuelsmth@gmail.com>
References: <20210504222915.17206-5-ansuelsmth@gmail.com>
TO: Ansuel Smith <ansuelsmth@gmail.com>
Hi Ansuel,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on robh/for-next]
[also build test WARNING on v5.12]
[cannot apply to net-next/master net/master linus/master next-20210505]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Ansuel-Smith/net-mdio-ipq8064-clean-whitespaces-in-define/20210505-063401
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
:::::: branch date: 8 hours ago
:::::: commit date: 8 hours ago
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Julia Lawall <julia.lawall@lip6.fr>
cocci warnings: (new ones prefixed by >>)
>> drivers/net/dsa/qca8k.c:496:2-8: preceding lock on line 489
drivers/net/dsa/qca8k.c:529:2-8: preceding lock on line 522
vim +496 drivers/net/dsa/qca8k.c
69462fe6a39048 Jonathan McDowell 2020-08-01 475
69462fe6a39048 Jonathan McDowell 2020-08-01 476 static int
69462fe6a39048 Jonathan McDowell 2020-08-01 477 qca8k_vlan_add(struct qca8k_priv *priv, u8 port, u16 vid, bool untagged)
69462fe6a39048 Jonathan McDowell 2020-08-01 478 {
69462fe6a39048 Jonathan McDowell 2020-08-01 479 u32 reg;
69462fe6a39048 Jonathan McDowell 2020-08-01 480 int ret;
69462fe6a39048 Jonathan McDowell 2020-08-01 481
69462fe6a39048 Jonathan McDowell 2020-08-01 482 /*
69462fe6a39048 Jonathan McDowell 2020-08-01 483 We do the right thing with VLAN 0 and treat it as untagged while
69462fe6a39048 Jonathan McDowell 2020-08-01 484 preserving the tag on egress.
69462fe6a39048 Jonathan McDowell 2020-08-01 485 */
69462fe6a39048 Jonathan McDowell 2020-08-01 486 if (vid == 0)
69462fe6a39048 Jonathan McDowell 2020-08-01 487 return 0;
69462fe6a39048 Jonathan McDowell 2020-08-01 488
69462fe6a39048 Jonathan McDowell 2020-08-01 @489 mutex_lock(&priv->reg_mutex);
69462fe6a39048 Jonathan McDowell 2020-08-01 490 ret = qca8k_vlan_access(priv, QCA8K_VLAN_READ, vid);
69462fe6a39048 Jonathan McDowell 2020-08-01 491 if (ret < 0)
69462fe6a39048 Jonathan McDowell 2020-08-01 492 goto out;
69462fe6a39048 Jonathan McDowell 2020-08-01 493
69462fe6a39048 Jonathan McDowell 2020-08-01 494 reg = qca8k_read(priv, QCA8K_REG_VTU_FUNC0);
e0bc1bb1e57e47 Ansuel Smith 2021-05-05 495 if (reg < 0)
e0bc1bb1e57e47 Ansuel Smith 2021-05-05 @496 return reg;
69462fe6a39048 Jonathan McDowell 2020-08-01 497 reg |= QCA8K_VTU_FUNC0_VALID | QCA8K_VTU_FUNC0_IVL_EN;
69462fe6a39048 Jonathan McDowell 2020-08-01 498 reg &= ~(QCA8K_VTU_FUNC0_EG_MODE_MASK << QCA8K_VTU_FUNC0_EG_MODE_S(port));
69462fe6a39048 Jonathan McDowell 2020-08-01 499 if (untagged)
69462fe6a39048 Jonathan McDowell 2020-08-01 500 reg |= QCA8K_VTU_FUNC0_EG_MODE_UNTAG <<
69462fe6a39048 Jonathan McDowell 2020-08-01 501 QCA8K_VTU_FUNC0_EG_MODE_S(port);
69462fe6a39048 Jonathan McDowell 2020-08-01 502 else
69462fe6a39048 Jonathan McDowell 2020-08-01 503 reg |= QCA8K_VTU_FUNC0_EG_MODE_TAG <<
69462fe6a39048 Jonathan McDowell 2020-08-01 504 QCA8K_VTU_FUNC0_EG_MODE_S(port);
69462fe6a39048 Jonathan McDowell 2020-08-01 505
69462fe6a39048 Jonathan McDowell 2020-08-01 506 qca8k_write(priv, QCA8K_REG_VTU_FUNC0, reg);
69462fe6a39048 Jonathan McDowell 2020-08-01 507 ret = qca8k_vlan_access(priv, QCA8K_VLAN_LOAD, vid);
69462fe6a39048 Jonathan McDowell 2020-08-01 508
69462fe6a39048 Jonathan McDowell 2020-08-01 509 out:
69462fe6a39048 Jonathan McDowell 2020-08-01 510 mutex_unlock(&priv->reg_mutex);
69462fe6a39048 Jonathan McDowell 2020-08-01 511
69462fe6a39048 Jonathan McDowell 2020-08-01 512 return ret;
69462fe6a39048 Jonathan McDowell 2020-08-01 513 }
69462fe6a39048 Jonathan McDowell 2020-08-01 514
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 65455 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread* [RFC PATCH net-next v3 01/20] net: mdio: ipq8064: clean whitespaces in define @ 2021-05-04 22:28 Ansuel Smith 2021-05-04 22:28 ` [RFC PATCH net-next v3 05/20] net: dsa: qca8k: handle error with qca8k_read operation Ansuel Smith 0 siblings, 1 reply; 4+ messages in thread From: Ansuel Smith @ 2021-05-04 22:28 UTC (permalink / raw) To: Florian Fainelli Cc: Ansuel Smith, Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller, Jakub Kicinski, netdev, linux-kernel Fix mixed whitespace and tab for define spacing. Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> --- drivers/net/mdio/mdio-ipq8064.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/drivers/net/mdio/mdio-ipq8064.c b/drivers/net/mdio/mdio-ipq8064.c index 1bd18857e1c5..fb1614242e13 100644 --- a/drivers/net/mdio/mdio-ipq8064.c +++ b/drivers/net/mdio/mdio-ipq8064.c @@ -15,25 +15,26 @@ #include <linux/mfd/syscon.h> /* MII address register definitions */ -#define MII_ADDR_REG_ADDR 0x10 -#define MII_BUSY BIT(0) -#define MII_WRITE BIT(1) -#define MII_CLKRANGE_60_100M (0 << 2) -#define MII_CLKRANGE_100_150M (1 << 2) -#define MII_CLKRANGE_20_35M (2 << 2) -#define MII_CLKRANGE_35_60M (3 << 2) -#define MII_CLKRANGE_150_250M (4 << 2) -#define MII_CLKRANGE_250_300M (5 << 2) +#define MII_ADDR_REG_ADDR 0x10 +#define MII_BUSY BIT(0) +#define MII_WRITE BIT(1) +#define MII_CLKRANGE(x) ((x) << 2) +#define MII_CLKRANGE_60_100M MII_CLKRANGE(0) +#define MII_CLKRANGE_100_150M MII_CLKRANGE(1) +#define MII_CLKRANGE_20_35M MII_CLKRANGE(2) +#define MII_CLKRANGE_35_60M MII_CLKRANGE(3) +#define MII_CLKRANGE_150_250M MII_CLKRANGE(4) +#define MII_CLKRANGE_250_300M MII_CLKRANGE(5) #define MII_CLKRANGE_MASK GENMASK(4, 2) #define MII_REG_SHIFT 6 #define MII_REG_MASK GENMASK(10, 6) #define MII_ADDR_SHIFT 11 #define MII_ADDR_MASK GENMASK(15, 11) -#define MII_DATA_REG_ADDR 0x14 +#define MII_DATA_REG_ADDR 0x14 -#define MII_MDIO_DELAY_USEC (1000) -#define MII_MDIO_RETRY_MSEC (10) +#define MII_MDIO_DELAY_USEC (1000) +#define MII_MDIO_RETRY_MSEC (10) struct ipq8064_mdio { struct regmap *base; /* NSS_GMAC0_BASE */ -- 2.30.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [RFC PATCH net-next v3 05/20] net: dsa: qca8k: handle error with qca8k_read operation 2021-05-04 22:28 [RFC PATCH net-next v3 01/20] net: mdio: ipq8064: clean whitespaces in define Ansuel Smith @ 2021-05-04 22:28 ` Ansuel Smith 2021-05-05 0:36 ` Andrew Lunn 0 siblings, 1 reply; 4+ messages in thread From: Ansuel Smith @ 2021-05-04 22:28 UTC (permalink / raw) To: Florian Fainelli Cc: Ansuel Smith, Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S. Miller, Jakub Kicinski, Russell King, netdev, linux-kernel qca8k_read can fail. Rework any user to handle error values and correctly return. Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> --- drivers/net/dsa/qca8k.c | 90 +++++++++++++++++++++++++++++++---------- 1 file changed, 69 insertions(+), 21 deletions(-) diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c index 411b42d38819..cde68ed6856b 100644 --- a/drivers/net/dsa/qca8k.c +++ b/drivers/net/dsa/qca8k.c @@ -146,12 +146,13 @@ qca8k_set_page(struct mii_bus *bus, u16 page) static u32 qca8k_read(struct qca8k_priv *priv, u32 reg) { + struct mii_bus *bus = priv->bus; u16 r1, r2, page; u32 val; qca8k_split_addr(reg, &r1, &r2, &page); - mutex_lock_nested(&priv->bus->mdio_lock, MDIO_MUTEX_NESTED); + mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED); val = qca8k_set_page(priv->bus, page); if (val < 0) @@ -160,8 +161,7 @@ qca8k_read(struct qca8k_priv *priv, u32 reg) val = qca8k_mii_read32(priv->bus, 0x10 | r2, r1); exit: - mutex_unlock(&priv->bus->mdio_lock); - + mutex_unlock(&bus->mdio_lock); return val; } @@ -226,8 +226,13 @@ static int qca8k_regmap_read(void *ctx, uint32_t reg, uint32_t *val) { struct qca8k_priv *priv = (struct qca8k_priv *)ctx; + int ret; - *val = qca8k_read(priv, reg); + ret = qca8k_read(priv, reg); + if (ret < 0) + return ret; + + *val = ret; return 0; } @@ -280,15 +285,17 @@ static int qca8k_busy_wait(struct qca8k_priv *priv, u32 reg, u32 mask) { unsigned long timeout; + u32 val; timeout = jiffies + msecs_to_jiffies(20); /* loop until the busy flag has cleared */ do { - u32 val = qca8k_read(priv, reg); - int busy = val & mask; + val = qca8k_read(priv, reg); + if (val < 0) + continue; - if (!busy) + if (!(val & mask)) break; cond_resched(); } while (!time_after_eq(jiffies, timeout)); @@ -296,15 +303,20 @@ qca8k_busy_wait(struct qca8k_priv *priv, u32 reg, u32 mask) return time_after_eq(jiffies, timeout); } -static void +static int qca8k_fdb_read(struct qca8k_priv *priv, struct qca8k_fdb *fdb) { - u32 reg[4]; + u32 reg[4], val; int i; /* load the ARL table into an array */ - for (i = 0; i < 4; i++) - reg[i] = qca8k_read(priv, QCA8K_REG_ATU_DATA0 + (i * 4)); + for (i = 0; i < 4; i++) { + val = qca8k_read(priv, QCA8K_REG_ATU_DATA0 + (i * 4)); + if (val < 0) + return val; + + reg[i] = val; + } /* vid - 83:72 */ fdb->vid = (reg[2] >> QCA8K_ATU_VID_S) & QCA8K_ATU_VID_M; @@ -319,6 +331,8 @@ qca8k_fdb_read(struct qca8k_priv *priv, struct qca8k_fdb *fdb) fdb->mac[3] = (reg[0] >> QCA8K_ATU_ADDR3_S) & 0xff; fdb->mac[4] = (reg[0] >> QCA8K_ATU_ADDR4_S) & 0xff; fdb->mac[5] = reg[0] & 0xff; + + return 0; } static void @@ -370,6 +384,8 @@ qca8k_fdb_access(struct qca8k_priv *priv, enum qca8k_fdb_cmd cmd, int port) /* Check for table full violation when adding an entry */ if (cmd == QCA8K_FDB_LOAD) { reg = qca8k_read(priv, QCA8K_REG_ATU_FUNC); + if (reg < 0) + return reg; if (reg & QCA8K_ATU_FUNC_FULL) return -1; } @@ -380,12 +396,15 @@ qca8k_fdb_access(struct qca8k_priv *priv, enum qca8k_fdb_cmd cmd, int port) static int qca8k_fdb_next(struct qca8k_priv *priv, struct qca8k_fdb *fdb, int port) { - int ret; + int ret, ret_read; qca8k_fdb_write(priv, fdb->vid, fdb->port_mask, fdb->mac, fdb->aging); ret = qca8k_fdb_access(priv, QCA8K_FDB_NEXT, port); - if (ret >= 0) - qca8k_fdb_read(priv, fdb); + if (ret >= 0) { + ret_read = qca8k_fdb_read(priv, fdb); + if (ret_read < 0) + return ret_read; + } return ret; } @@ -445,6 +464,8 @@ qca8k_vlan_access(struct qca8k_priv *priv, enum qca8k_vlan_cmd cmd, u16 vid) /* Check for table full violation when adding an entry */ if (cmd == QCA8K_VLAN_LOAD) { reg = qca8k_read(priv, QCA8K_REG_VTU_FUNC1); + if (reg < 0) + return reg; if (reg & QCA8K_VTU_FUNC1_FULL) return -ENOMEM; } @@ -471,6 +492,8 @@ qca8k_vlan_add(struct qca8k_priv *priv, u8 port, u16 vid, bool untagged) goto out; reg = qca8k_read(priv, QCA8K_REG_VTU_FUNC0); + if (reg < 0) + return reg; reg |= QCA8K_VTU_FUNC0_VALID | QCA8K_VTU_FUNC0_IVL_EN; reg &= ~(QCA8K_VTU_FUNC0_EG_MODE_MASK << QCA8K_VTU_FUNC0_EG_MODE_S(port)); if (untagged) @@ -502,6 +525,8 @@ qca8k_vlan_del(struct qca8k_priv *priv, u8 port, u16 vid) goto out; reg = qca8k_read(priv, QCA8K_REG_VTU_FUNC0); + if (reg < 0) + return reg; reg &= ~(3 << QCA8K_VTU_FUNC0_EG_MODE_S(port)); reg |= QCA8K_VTU_FUNC0_EG_MODE_NOT << QCA8K_VTU_FUNC0_EG_MODE_S(port); @@ -617,8 +642,11 @@ qca8k_mdio_read(struct qca8k_priv *priv, int port, u32 regnum) QCA8K_MDIO_MASTER_BUSY)) return -ETIMEDOUT; - val = (qca8k_read(priv, QCA8K_MDIO_MASTER_CTRL) & - QCA8K_MDIO_MASTER_DATA_MASK); + val = qca8k_read(priv, QCA8K_MDIO_MASTER_CTRL); + if (val < 0) + return val; + + val &= QCA8K_MDIO_MASTER_DATA_MASK; return val; } @@ -974,6 +1002,8 @@ qca8k_phylink_mac_link_state(struct dsa_switch *ds, int port, u32 reg; reg = qca8k_read(priv, QCA8K_REG_PORT_STATUS(port)); + if (reg < 0) + return reg; state->link = !!(reg & QCA8K_PORT_STATUS_LINK_UP); state->an_complete = state->link; @@ -1074,18 +1104,26 @@ qca8k_get_ethtool_stats(struct dsa_switch *ds, int port, { struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv; const struct qca8k_mib_desc *mib; - u32 reg, i; + u32 reg, i, val; u64 hi; for (i = 0; i < ARRAY_SIZE(ar8327_mib); i++) { mib = &ar8327_mib[i]; reg = QCA8K_PORT_MIB_COUNTER(port) + mib->offset; - data[i] = qca8k_read(priv, reg); + val = qca8k_read(priv, reg); + if (val < 0) + continue; + if (mib->size == 2) { hi = qca8k_read(priv, reg + 4); - data[i] |= hi << 32; + if (hi < 0) + continue; } + + data[i] = val; + if (mib->size == 2) + data[i] |= hi << 32; } } @@ -1103,18 +1141,25 @@ qca8k_set_mac_eee(struct dsa_switch *ds, int port, struct ethtool_eee *eee) { struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv; u32 lpi_en = QCA8K_REG_EEE_CTRL_LPI_EN(port); + int ret = 0; u32 reg; mutex_lock(&priv->reg_mutex); reg = qca8k_read(priv, QCA8K_REG_EEE_CTRL); + if (reg < 0) { + ret = reg; + goto exit; + } + if (eee->eee_enabled) reg |= lpi_en; else reg &= ~lpi_en; qca8k_write(priv, QCA8K_REG_EEE_CTRL, reg); - mutex_unlock(&priv->reg_mutex); - return 0; +exit: + mutex_unlock(&priv->reg_mutex); + return ret; } static int @@ -1439,6 +1484,9 @@ qca8k_sw_probe(struct mdio_device *mdiodev) /* read the switches ID register */ id = qca8k_read(priv, QCA8K_REG_MASK_CTRL); + if (id < 0) + return id; + id >>= QCA8K_MASK_CTRL_ID_S; id &= QCA8K_MASK_CTRL_ID_M; if (id != QCA8K_ID_QCA8337) -- 2.30.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [RFC PATCH net-next v3 05/20] net: dsa: qca8k: handle error with qca8k_read operation 2021-05-04 22:28 ` [RFC PATCH net-next v3 05/20] net: dsa: qca8k: handle error with qca8k_read operation Ansuel Smith @ 2021-05-05 0:36 ` Andrew Lunn 2021-05-05 0:44 ` Ansuel Smith 0 siblings, 1 reply; 4+ messages in thread From: Andrew Lunn @ 2021-05-05 0:36 UTC (permalink / raw) To: Ansuel Smith Cc: Florian Fainelli, Vivien Didelot, Vladimir Oltean, David S. Miller, Jakub Kicinski, Russell King, netdev, linux-kernel On Wed, May 05, 2021 at 12:28:59AM +0200, Ansuel Smith wrote: > qca8k_read can fail. Rework any user to handle error values and > correctly return. > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> > --- > drivers/net/dsa/qca8k.c | 90 +++++++++++++++++++++++++++++++---------- > 1 file changed, 69 insertions(+), 21 deletions(-) > > diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c > index 411b42d38819..cde68ed6856b 100644 > --- a/drivers/net/dsa/qca8k.c > +++ b/drivers/net/dsa/qca8k.c > @@ -146,12 +146,13 @@ qca8k_set_page(struct mii_bus *bus, u16 page) > static u32 > qca8k_read(struct qca8k_priv *priv, u32 reg) > { > + struct mii_bus *bus = priv->bus; > u16 r1, r2, page; > u32 val; > > qca8k_split_addr(reg, &r1, &r2, &page); > > - mutex_lock_nested(&priv->bus->mdio_lock, MDIO_MUTEX_NESTED); > + mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED); > > val = qca8k_set_page(priv->bus, page); > if (val < 0) > @@ -160,8 +161,7 @@ qca8k_read(struct qca8k_priv *priv, u32 reg) > val = qca8k_mii_read32(priv->bus, 0x10 | r2, r1); > > exit: > - mutex_unlock(&priv->bus->mdio_lock); > - > + mutex_unlock(&bus->mdio_lock); > return val; This change does not have anything to do with the commit message. > } > > @@ -226,8 +226,13 @@ static int > qca8k_regmap_read(void *ctx, uint32_t reg, uint32_t *val) > { > struct qca8k_priv *priv = (struct qca8k_priv *)ctx; > + int ret; > > - *val = qca8k_read(priv, reg); > + ret = qca8k_read(priv, reg); > + if (ret < 0) > + return ret; > + > + *val = ret; > > return 0; > } > @@ -280,15 +285,17 @@ static int > qca8k_busy_wait(struct qca8k_priv *priv, u32 reg, u32 mask) > { > unsigned long timeout; > + u32 val; > > timeout = jiffies + msecs_to_jiffies(20); > > /* loop until the busy flag has cleared */ > do { > - u32 val = qca8k_read(priv, reg); > - int busy = val & mask; > + val = qca8k_read(priv, reg); > + if (val < 0) > + continue; > > - if (!busy) > + if (!(val & mask)) > break; > cond_resched(); Maybe there is a patch doing this already, but it would be good to make use of include/linux/iopoll.h > qca8k_fdb_next(struct qca8k_priv *priv, struct qca8k_fdb *fdb, int port) > { > - int ret; > + int ret, ret_read; > > qca8k_fdb_write(priv, fdb->vid, fdb->port_mask, fdb->mac, fdb->aging); > ret = qca8k_fdb_access(priv, QCA8K_FDB_NEXT, port); > - if (ret >= 0) > - qca8k_fdb_read(priv, fdb); > + if (ret >= 0) { > + ret_read = qca8k_fdb_read(priv, fdb); > + if (ret_read < 0) > + return ret_read; > + } > > return ret; > } This is oddly structured. Why not: qca8k_fdb_next(struct qca8k_priv *priv, struct qca8k_fdb *fdb, int port) { int ret; qca8k_fdb_write(priv, fdb->vid, fdb->port_mask, fdb->mac, fdb->aging); ret = qca8k_fdb_access(priv, QCA8K_FDB_NEXT, port); if (ret < 0) return ret; return qca8k_fdb_read(priv, fdb); } Andrew ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH net-next v3 05/20] net: dsa: qca8k: handle error with qca8k_read operation 2021-05-05 0:36 ` Andrew Lunn @ 2021-05-05 0:44 ` Ansuel Smith 0 siblings, 0 replies; 4+ messages in thread From: Ansuel Smith @ 2021-05-05 0:44 UTC (permalink / raw) To: Andrew Lunn Cc: Florian Fainelli, Vivien Didelot, Vladimir Oltean, David S. Miller, Jakub Kicinski, Russell King, netdev, linux-kernel On Wed, May 05, 2021 at 02:36:15AM +0200, Andrew Lunn wrote: > On Wed, May 05, 2021 at 12:28:59AM +0200, Ansuel Smith wrote: > > qca8k_read can fail. Rework any user to handle error values and > > correctly return. > > > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> > > --- > > drivers/net/dsa/qca8k.c | 90 +++++++++++++++++++++++++++++++---------- > > 1 file changed, 69 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c > > index 411b42d38819..cde68ed6856b 100644 > > --- a/drivers/net/dsa/qca8k.c > > +++ b/drivers/net/dsa/qca8k.c > > @@ -146,12 +146,13 @@ qca8k_set_page(struct mii_bus *bus, u16 page) > > static u32 > > qca8k_read(struct qca8k_priv *priv, u32 reg) > > { > > + struct mii_bus *bus = priv->bus; > > u16 r1, r2, page; > > u32 val; > > > > qca8k_split_addr(reg, &r1, &r2, &page); > > > > - mutex_lock_nested(&priv->bus->mdio_lock, MDIO_MUTEX_NESTED); > > + mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED); > > > > val = qca8k_set_page(priv->bus, page); > > if (val < 0) > > @@ -160,8 +161,7 @@ qca8k_read(struct qca8k_priv *priv, u32 reg) > > val = qca8k_mii_read32(priv->bus, 0x10 | r2, r1); > > > > exit: > > - mutex_unlock(&priv->bus->mdio_lock); > > - > > + mutex_unlock(&bus->mdio_lock); > > return val; > > This change does not have anything to do with the commit message. > > > } Will split in another patch. > > > > @@ -226,8 +226,13 @@ static int > > qca8k_regmap_read(void *ctx, uint32_t reg, uint32_t *val) > > { > > struct qca8k_priv *priv = (struct qca8k_priv *)ctx; > > + int ret; > > > > - *val = qca8k_read(priv, reg); > > + ret = qca8k_read(priv, reg); > > + if (ret < 0) > > + return ret; > > + > > + *val = ret; > > > > return 0; > > } > > @@ -280,15 +285,17 @@ static int > > qca8k_busy_wait(struct qca8k_priv *priv, u32 reg, u32 mask) > > { > > unsigned long timeout; > > + u32 val; > > > > timeout = jiffies + msecs_to_jiffies(20); > > > > /* loop until the busy flag has cleared */ > > do { > > - u32 val = qca8k_read(priv, reg); > > - int busy = val & mask; > > + val = qca8k_read(priv, reg); > > + if (val < 0) > > + continue; > > > > - if (!busy) > > + if (!(val & mask)) > > break; > > cond_resched(); > > Maybe there is a patch doing this already, but it would be good to > make use of include/linux/iopoll.h > Will check if I can find something to replace this. > > qca8k_fdb_next(struct qca8k_priv *priv, struct qca8k_fdb *fdb, int port) > > { > > - int ret; > > + int ret, ret_read; > > > > qca8k_fdb_write(priv, fdb->vid, fdb->port_mask, fdb->mac, fdb->aging); > > ret = qca8k_fdb_access(priv, QCA8K_FDB_NEXT, port); > > - if (ret >= 0) > > - qca8k_fdb_read(priv, fdb); > > + if (ret >= 0) { > > + ret_read = qca8k_fdb_read(priv, fdb); > > + if (ret_read < 0) > > + return ret_read; > > + } > > > > return ret; > > } > > This is oddly structured. Why not: > > qca8k_fdb_next(struct qca8k_priv *priv, struct qca8k_fdb *fdb, int port) > { > int ret; > > qca8k_fdb_write(priv, fdb->vid, fdb->port_mask, fdb->mac, fdb->aging); > > ret = qca8k_fdb_access(priv, QCA8K_FDB_NEXT, port); > if (ret < 0) > return ret; > > return qca8k_fdb_read(priv, fdb); > } > It's late here and I could be wrong... Doesn't your suggested code change the original function return value? In the original function we returned qca8k_fdb_access, isn't wrong to return qca8k_fdb_read on success? Or the function was wrong from the start? > Andrew ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-05-05 6:26 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-05 6:26 [RFC PATCH net-next v3 05/20] net: dsa: qca8k: handle error with qca8k_read operation kernel test robot -- strict thread matches above, loose matches on Subject: below -- 2021-05-04 22:28 [RFC PATCH net-next v3 01/20] net: mdio: ipq8064: clean whitespaces in define Ansuel Smith 2021-05-04 22:28 ` [RFC PATCH net-next v3 05/20] net: dsa: qca8k: handle error with qca8k_read operation Ansuel Smith 2021-05-05 0:36 ` Andrew Lunn 2021-05-05 0:44 ` Ansuel Smith
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.