[PATCH net-next 07/12] net: dsa: qca8k: Pass error code from reply decoder to requester
From: Luke Howard <hidden>
Date: 2026-07-03 07:31:10
Also in:
lkml
Subsystem:
networking drivers, networking [dsa], networking [general], the rest · Maintainers:
Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Vladimir Oltean, Linus Torvalds
From: Andrew Lunn <andrew@lunn.ch> The code which decodes the frame and signals the complete can detect error within the reply, such as fields have unexpected values. Pass an error code between the completer and the function waiting on the complete. This simplifies the error handling, since all errors are combined into one place. Signed-off-by: Andrew Lunn <andrew@lunn.ch> --- v2: Remove EPROTO if the sequence numbers don't match, drop the reply --- drivers/net/dsa/qca/qca8k-8xxx.c | 70 +++++++++++++--------------------------- drivers/net/dsa/qca/qca8k.h | 2 -- include/net/dsa.h | 13 +++++++- net/dsa/dsa.c | 33 +++++++++++++++++-- 4 files changed, 65 insertions(+), 53 deletions(-)
diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index 499cc8ef5ddea..ffa3b1ba23bed 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c@@ -167,6 +167,8 @@ static void qca8k_rw_reg_ack_handler(struct dsa_switch *ds, struct sk_buff *skb) struct qca_mgmt_ethhdr *mgmt_ethhdr; u32 command; u8 len, cmd; + u32 data[4]; + int err = 0; int i; mgmt_ethhdr = (struct qca_mgmt_ethhdr *)skb_mac_header(skb);
@@ -191,7 +193,7 @@ static void qca8k_rw_reg_ack_handler(struct dsa_switch *ds, struct sk_buff *skb) return; if (cmd == MDIO_READ) { - u32 *val = mgmt_eth_data->data; + u32 *val = &data[0]; *val = get_unaligned_le32(&mgmt_ethhdr->mdio_data);
@@ -213,7 +215,7 @@ static void qca8k_rw_reg_ack_handler(struct dsa_switch *ds, struct sk_buff *skb) } } - dsa_inband_complete(&mgmt_eth_data->inband); + dsa_inband_complete(&mgmt_eth_data->inband, &data, sizeof(data), err); } static struct sk_buff *qca8k_alloc_mdio_header(enum mdio_cmd cmd, u32 reg, u32 *val,
@@ -314,7 +316,7 @@ static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len) { struct qca8k_mgmt_eth_data *mgmt_eth_data = &priv->mgmt_eth_data; struct sk_buff *skb; - int err; + u32 data[4]; int ret; skb = qca8k_alloc_mdio_header(MDIO_READ, reg, NULL,
@@ -335,30 +337,25 @@ static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len) ret = dsa_inband_request(&mgmt_eth_data->inband, skb, qca8k_mdio_header_fill_seq_num, + data, sizeof(data), QCA8K_ETHERNET_TIMEOUT); + if (ret < 0) + goto out; - *val = mgmt_eth_data->data[0]; + *val = data[0]; if (len > QCA_HDR_MGMT_DATA1_LEN) - memcpy(val + 1, mgmt_eth_data->data + 1, len - QCA_HDR_MGMT_DATA1_LEN); - - err = mgmt_eth_data->err; + memcpy(val + 1, &data[1], len - QCA_HDR_MGMT_DATA1_LEN); +out: mutex_unlock(&mgmt_eth_data->mutex); - if (ret) - return ret; - - if (err) - return err; - - return 0; + return ret; } static int qca8k_write_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len) { struct qca8k_mgmt_eth_data *mgmt_eth_data = &priv->mgmt_eth_data; struct sk_buff *skb; - int err; int ret; skb = qca8k_alloc_mdio_header(MDIO_WRITE, reg, val,
@@ -379,19 +376,12 @@ static int qca8k_write_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len) ret = dsa_inband_request(&mgmt_eth_data->inband, skb, qca8k_mdio_header_fill_seq_num, + NULL, 0, QCA8K_ETHERNET_TIMEOUT); - err = mgmt_eth_data->err; - mutex_unlock(&mgmt_eth_data->mutex); - if (ret) - return ret; - - if (err) - return err; - - return 0; + return ret; } static int
@@ -573,7 +563,7 @@ qca8k_phy_eth_busy_wait(struct qca8k_mgmt_eth_data *mgmt_eth_data, struct sk_buff *read_skb, u32 *val) { struct sk_buff *skb = skb_copy(read_skb, GFP_KERNEL); - int err; + u32 data[4]; int ret; if (!skb)
@@ -581,17 +571,13 @@ qca8k_phy_eth_busy_wait(struct qca8k_mgmt_eth_data *mgmt_eth_data, ret = dsa_inband_request(&mgmt_eth_data->inband, skb, qca8k_mdio_header_fill_seq_num, + data, sizeof(data), QCA8K_ETHERNET_TIMEOUT); - err = mgmt_eth_data->err; - if (ret) return ret; - if (err) - return err; - - *val = mgmt_eth_data->data[0]; + *val = data[0]; return 0; }
@@ -604,8 +590,8 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy, struct qca8k_mgmt_eth_data *mgmt_eth_data; u32 write_val, clear_val = 0, val; struct net_device *mgmt_conduit; + u32 resp_data[4]; int ret, ret1; - int err; if (regnum >= QCA8K_MDIO_MASTER_MAX_REG) return -EINVAL;
@@ -675,21 +661,14 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy, ret = dsa_inband_request(&mgmt_eth_data->inband, write_skb, qca8k_mdio_header_fill_seq_num, + NULL, 0, QCA8K_ETHERNET_TIMEOUT); - err = mgmt_eth_data->err; - if (ret) { kfree_skb(read_skb); goto exit; } - if (err) { - ret = err; - kfree_skb(read_skb); - goto exit; - } - ret = read_poll_timeout(qca8k_phy_eth_busy_wait, ret1, !(val & QCA8K_MDIO_MASTER_BUSY), 0, QCA8K_BUSY_WAIT_TIMEOUT * USEC_PER_MSEC, false,
@@ -703,19 +682,13 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy, if (read) { ret = dsa_inband_request(&mgmt_eth_data->inband, read_skb, qca8k_mdio_header_fill_seq_num, + resp_data, sizeof(resp_data), QCA8K_ETHERNET_TIMEOUT); - err = mgmt_eth_data->err; - if (ret) goto exit; - if (err) { - ret = err; - goto exit; - } - - ret = mgmt_eth_data->data[0] & QCA8K_MDIO_MASTER_DATA_MASK; + ret = resp_data[0] & QCA8K_MDIO_MASTER_DATA_MASK; } else { kfree_skb(read_skb); }
@@ -724,6 +697,7 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy, /* This is expected to fail sometimes, so don't check return value. */ dsa_inband_request(&mgmt_eth_data->inband, clear_skb, qca8k_mdio_header_fill_seq_num, + NULL, 0, QCA8K_ETHERNET_TIMEOUT); mutex_unlock(&mgmt_eth_data->mutex);
diff --git a/drivers/net/dsa/qca/qca8k.h b/drivers/net/dsa/qca/qca8k.h
index 4c055aef674c2..db89025b4243a 100644
--- a/drivers/net/dsa/qca/qca8k.h
+++ b/drivers/net/dsa/qca/qca8k.h@@ -394,8 +394,6 @@ enum { struct qca8k_mgmt_eth_data { struct dsa_inband inband; struct mutex mutex; /* Enforce one mdio read/write at time */ - int err; - u32 data[4]; }; struct qca8k_mib_eth_data {
diff --git a/include/net/dsa.h b/include/net/dsa.h
index cdf2487397c2b..50ac6f0aa2e67 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h@@ -1350,17 +1350,28 @@ int dsa_port_simple_hsr_leave(struct dsa_switch *ds, int port, /* Perform operations on a switch by sending it request in Ethernet * frames and expecting a response in a frame. + * + * resp_lock protects resp and resp_len to ensure they are consistent. + * If there is a thread waiting for the response, resp will point to a + * buffer to copy the response to. If the thread has given up waiting, + * resp will be a NULL pointer. */ struct dsa_inband { struct completion completion; u32 seqno; u32 seqno_mask; + int err; + spinlock_t resp_lock; + void *resp; + unsigned int resp_len; }; void dsa_inband_init(struct dsa_inband *inband, u32 seqno_mask); -void dsa_inband_complete(struct dsa_inband *inband); +void dsa_inband_complete(struct dsa_inband *inband, + void *resp, unsigned int resp_len, int err); int dsa_inband_request(struct dsa_inband *inband, struct sk_buff *skb, void (*insert_seqno)(struct sk_buff *skb, u32 seqno), + void *resp, unsigned int resp_len, int timeout_ms); int dsa_inband_wait_for_completion(struct dsa_inband *inband, int timeout_ms); u32 dsa_inband_seqno(struct dsa_inband *inband);
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 98a75ee08f011..3e480770854ca 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c@@ -1837,13 +1837,27 @@ EXPORT_SYMBOL_GPL(dsa_port_simple_hsr_leave); void dsa_inband_init(struct dsa_inband *inband, u32 seqno_mask) { init_completion(&inband->completion); + spin_lock_init(&inband->resp_lock); inband->seqno_mask = seqno_mask; inband->seqno = 0; } EXPORT_SYMBOL_GPL(dsa_inband_init); -void dsa_inband_complete(struct dsa_inband *inband) +void dsa_inband_complete(struct dsa_inband *inband, + void *resp, unsigned int resp_len, + int err) { + inband->err = err; + + if (!err) { + spin_lock_bh(&inband->resp_lock); + resp_len = min(inband->resp_len, resp_len); + if (inband->resp && resp) + memcpy(inband->resp, resp, resp_len); + spin_unlock_bh(&inband->resp_lock); + inband->err = resp_len; + } + complete(&inband->completion); } EXPORT_SYMBOL_GPL(dsa_inband_complete);
@@ -1878,11 +1892,19 @@ EXPORT_SYMBOL_GPL(dsa_inband_wait_for_completion); */ int dsa_inband_request(struct dsa_inband *inband, struct sk_buff *skb, void (*insert_seqno)(struct sk_buff *skb, u32 seqno), + void *resp, unsigned int resp_len, int timeout_ms) { unsigned long jiffies = msecs_to_jiffies(timeout_ms); int ret; + inband->err = 0; + + spin_lock_bh(&inband->resp_lock); + inband->resp = resp; + inband->resp_len = resp_len; + spin_unlock_bh(&inband->resp_lock); + if (insert_seqno) { WRITE_ONCE(inband->seqno, inband->seqno + 1); insert_seqno(skb, inband->seqno & inband->seqno_mask);
@@ -1893,9 +1915,16 @@ int dsa_inband_request(struct dsa_inband *inband, struct sk_buff *skb, dev_queue_xmit(skb); ret = wait_for_completion_timeout(&inband->completion, jiffies); + + spin_lock_bh(&inband->resp_lock); + inband->resp = NULL; + inband->resp_len = 0; + spin_unlock_bh(&inband->resp_lock); + if (ret == 0) return -ETIMEDOUT; - return 0; + + return inband->err; } EXPORT_SYMBOL_GPL(dsa_inband_request);
--
2.43.0