From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Ansuel Smith <ansuelsmth@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
Vivien Didelot <vivien.didelot@gmail.com>,
Florian Fainelli <f.fainelli@gmail.com>,
Vladimir Oltean <olteanv@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
Rob Herring <robh+dt@kernel.org>,
Heiner Kallweit <hkallweit1@gmail.com>,
netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v6 05/25] net: dsa: qca8k: handle error with qca8k_read operation
Date: Sat, 15 May 2021 00:01:52 +0100 [thread overview]
Message-ID: <20210514230152.GL12395@shell.armlinux.org.uk> (raw)
In-Reply-To: <20210514210015.18142-6-ansuelsmth@gmail.com>
On Fri, May 14, 2021 at 10:59:55PM +0200, Ansuel Smith wrote:
> -static void
> +static int
> qca8k_fdb_read(struct qca8k_priv *priv, struct qca8k_fdb *fdb)
> {
> - u32 reg[4];
> + u32 reg[4], val;
val is unsigned.
> 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;
So this return statement will never be reached.
> @@ -374,6 +386,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;
"reg" here is also a u32, and therefore unsigned, so this will have no
effect.
> if (reg & QCA8K_ATU_FUNC_FULL)
> return -1;
> }
> @@ -388,10 +402,10 @@ qca8k_fdb_next(struct qca8k_priv *priv, struct qca8k_fdb *fdb, int port)
>
> 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)
> + return ret;
This looks fine to me.
>
> - return ret;
> + return qca8k_fdb_read(priv, fdb);
> }
>
> static int
> @@ -449,6 +463,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;
reg is unsigned... unreachable.
> if (reg & QCA8K_VTU_FUNC1_FULL)
> return -ENOMEM;
> }
> @@ -475,6 +491,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 is unsigned... unreachable.
> 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)
> @@ -506,6 +524,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 is unsigned... unreachable.
> reg &= ~(3 << QCA8K_VTU_FUNC0_EG_MODE_S(port));
> reg |= QCA8K_VTU_FUNC0_EG_MODE_NOT <<
> QCA8K_VTU_FUNC0_EG_MODE_S(port);
> @@ -621,8 +641,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 is unsigned... unreachable.
> +
> + val &= QCA8K_MDIO_MASTER_DATA_MASK;
>
> return val;
> }
> @@ -978,6 +1001,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;
reg is unsigned... unreachable.
>
> state->link = !!(reg & QCA8K_PORT_STATUS_LINK_UP);
> state->an_complete = state->link;
> @@ -1078,18 +1103,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;
val is unsigned... unreachable....
> +
> if (mib->size == 2) {
> hi = qca8k_read(priv, reg + 4);
> - data[i] |= hi << 32;
> + if (hi < 0)
> + continue;
hi is a u64, so this condition is always false.
> }
> +
> + data[i] = val;
> + if (mib->size == 2)
> + data[i] |= hi << 32;
> }
> }
>
> @@ -1107,18 +1140,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;
No need to zero-initialise this.
> 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
> @@ -1443,6 +1483,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 is unsigned ...
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
next prev parent reply other threads:[~2021-05-14 23:01 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-14 20:59 [PATCH net-next v6 00/25] Multiple improvement to qca8k stability Ansuel Smith
2021-05-14 20:59 ` [PATCH net-next v6 01/25] net: dsa: qca8k: change simple print to dev variant Ansuel Smith
2021-05-14 22:43 ` Russell King (Oracle)
2021-05-14 20:59 ` [PATCH net-next v6 02/25] net: dsa: qca8k: use iopoll macro for qca8k_busy_wait Ansuel Smith
2021-05-14 22:52 ` Russell King (Oracle)
2021-05-14 23:07 ` Ansuel Smith
2021-05-14 20:59 ` [PATCH net-next v6 03/25] net: dsa: qca8k: improve qca8k read/write/rmw bus access Ansuel Smith
2021-05-14 22:53 ` Russell King (Oracle)
2021-05-14 20:59 ` [PATCH net-next v6 04/25] net: dsa: qca8k: handle qca8k_set_page errors Ansuel Smith
2021-05-14 22:55 ` Russell King (Oracle)
2021-05-14 20:59 ` [PATCH net-next v6 05/25] net: dsa: qca8k: handle error with qca8k_read operation Ansuel Smith
2021-05-14 23:01 ` Russell King (Oracle) [this message]
2021-05-14 20:59 ` [PATCH net-next v6 06/25] net: dsa: qca8k: handle error with qca8k_write operation Ansuel Smith
2021-05-14 20:59 ` [PATCH net-next v6 07/25] net: dsa: qca8k: handle error with qca8k_rmw operation Ansuel Smith
2021-05-14 20:59 ` [PATCH net-next v6 08/25] net: dsa: qca8k: handle error from qca8k_busy_wait Ansuel Smith
2021-05-14 20:59 ` [PATCH net-next v6 09/25] net: dsa: qca8k: add support for qca8327 switch Ansuel Smith
2021-05-14 21:00 ` [PATCH net-next v6 10/25] devicetree: net: dsa: qca8k: Document new compatible qca8327 Ansuel Smith
2021-05-14 21:00 ` [PATCH net-next v6 11/25] net: dsa: qca8k: add priority tweak to qca8337 switch Ansuel Smith
2021-05-14 21:00 ` [PATCH net-next v6 12/25] net: dsa: qca8k: limit port5 delay to qca8337 Ansuel Smith
2021-05-14 21:00 ` [PATCH net-next v6 13/25] net: dsa: qca8k: add GLOBAL_FC settings needed for qca8327 Ansuel Smith
2021-05-14 21:00 ` [PATCH net-next v6 14/25] net: dsa: qca8k: add support for switch rev Ansuel Smith
2021-05-14 21:00 ` [PATCH net-next v6 15/25] net: dsa: qca8k: add ethernet-ports fallback to setup_mdio_bus Ansuel Smith
2021-05-14 21:00 ` [PATCH net-next v6 16/25] net: dsa: qca8k: make rgmii delay configurable Ansuel Smith
2021-05-14 21:00 ` [PATCH net-next v6 17/25] net: dsa: qca8k: clear MASTER_EN after phy read/write Ansuel Smith
2021-05-14 21:00 ` [PATCH net-next v6 18/25] net: dsa: qca8k: dsa: qca8k: protect MASTER busy_wait with mdio mutex Ansuel Smith
2021-05-14 21:00 ` [PATCH net-next v6 19/25] net: dsa: qca8k: enlarge mdio delay and timeout Ansuel Smith
2021-05-14 21:00 ` [PATCH net-next v6 20/25] net: dsa: qca8k: add support for internal phy and internal mdio Ansuel Smith
2021-05-14 21:00 ` [PATCH net-next v6 21/25] devicetree: bindings: dsa: qca8k: Document internal mdio definition Ansuel Smith
2021-05-14 21:00 ` [PATCH net-next v6 22/25] net: dsa: qca8k: improve internal mdio read/write bus access Ansuel Smith
2021-05-14 21:00 ` [PATCH net-next v6 23/25] net: dsa: qca8k: pass switch_revision info to phy dev_flags Ansuel Smith
2021-05-14 21:00 ` [PATCH net-next v6 24/25] net: phy: at803x: clean whitespace errors Ansuel Smith
2021-05-14 21:00 ` [PATCH net-next v6 25/25] net: phy: add support for qca8k switch internal PHY in at803x Ansuel Smith
2021-05-14 22:40 ` [PATCH net-next v6 00/25] Multiple improvement to qca8k stability patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210514230152.GL12395@shell.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=andrew@lunn.ch \
--cc=ansuelsmth@gmail.com \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=f.fainelli@gmail.com \
--cc=hkallweit1@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=olteanv@gmail.com \
--cc=robh+dt@kernel.org \
--cc=vivien.didelot@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).