From: Christian Marangi Date: Tue, 3 Jan 2023 21:42:45 +0000 (+0100) Subject: generic: 5.15: backport qca8k fixup for mgmt and mdio read/write X-Git-Url: http://git.cdn.openwrt.org/?a=commitdiff_plain;h=f4d949b404855c7a643f810dbca0ad85088bd185;p=openwrt%2Fstaging%2Fdedeckeh.git generic: 5.15: backport qca8k fixup for mgmt and mdio read/write Backport qca8k fixup for mgmt and mdio read/write for kernel 5.15 fixup port dropping and configuration issue. Signed-off-by: Christian Marangi Reviewed-by: Robert Marko --- diff --git a/target/linux/generic/backport-5.15/777-v6.2-01-net-dsa-qca8k-fix-wrong-length-value-for-mgmt-eth-pa.patch b/target/linux/generic/backport-5.15/777-v6.2-01-net-dsa-qca8k-fix-wrong-length-value-for-mgmt-eth-pa.patch new file mode 100644 index 0000000000..b61e7ede49 --- /dev/null +++ b/target/linux/generic/backport-5.15/777-v6.2-01-net-dsa-qca8k-fix-wrong-length-value-for-mgmt-eth-pa.patch @@ -0,0 +1,102 @@ +From 9807ae69746196ee4bbffe7d22d22ab2b61c6ed0 Mon Sep 17 00:00:00 2001 +From: Christian Marangi +Date: Thu, 29 Dec 2022 17:33:32 +0100 +Subject: [PATCH 1/5] net: dsa: qca8k: fix wrong length value for mgmt eth + packet + +The assumption that Documentation was right about how this value work was +wrong. It was discovered that the length value of the mgmt header is in +step of word size. + +As an example to process 4 byte of data the correct length to set is 2. +To process 8 byte 4, 12 byte 6, 16 byte 8... + +Odd values will always return the next size on the ack packet. +(length of 3 (6 byte) will always return 8 bytes of data) + +This means that a value of 15 (0xf) actually means reading/writing 32 bytes +of data instead of 16 bytes. This behaviour is totally absent and not +documented in the switch Documentation. + +In fact from Documentation the max value that mgmt eth can process is +16 byte of data while in reality it can process 32 bytes at once. + +To handle this we always round up the length after deviding it for word +size. We check if the result is odd and we round another time to align +to what the switch will provide in the ack packet. +The workaround for the length limit of 15 is still needed as the length +reg max value is 0xf(15) + +Reported-by: Ronald Wahl +Tested-by: Ronald Wahl +Fixes: 90386223f44e ("net: dsa: qca8k: add support for larger read/write size with mgmt Ethernet") +Signed-off-by: Christian Marangi +Cc: stable@vger.kernel.org # v5.18+ +Signed-off-by: David S. Miller +--- + drivers/net/dsa/qca/qca8k-8xxx.c | 45 +++++++++++++++++++++++++------- + 1 file changed, 35 insertions(+), 10 deletions(-) + +--- a/drivers/net/dsa/qca/qca8k-8xxx.c ++++ b/drivers/net/dsa/qca/qca8k-8xxx.c +@@ -146,7 +146,16 @@ static void qca8k_rw_reg_ack_handler(str + + command = get_unaligned_le32(&mgmt_ethhdr->command); + cmd = FIELD_GET(QCA_HDR_MGMT_CMD, command); ++ + len = FIELD_GET(QCA_HDR_MGMT_LENGTH, command); ++ /* Special case for len of 15 as this is the max value for len and needs to ++ * be increased before converting it from word to dword. ++ */ ++ if (len == 15) ++ len++; ++ ++ /* We can ignore odd value, we always round up them in the alloc function. */ ++ len *= sizeof(u16); + + /* Make sure the seq match the requested packet */ + if (get_unaligned_le32(&mgmt_ethhdr->seq) == mgmt_eth_data->seq) +@@ -193,17 +202,33 @@ static struct sk_buff *qca8k_alloc_mdio_ + if (!skb) + return NULL; + +- /* Max value for len reg is 15 (0xf) but the switch actually return 16 byte +- * Actually for some reason the steps are: +- * 0: nothing +- * 1-4: first 4 byte +- * 5-6: first 12 byte +- * 7-15: all 16 byte ++ /* Hdr mgmt length value is in step of word size. ++ * As an example to process 4 byte of data the correct length to set is 2. ++ * To process 8 byte 4, 12 byte 6, 16 byte 8... ++ * ++ * Odd values will always return the next size on the ack packet. ++ * (length of 3 (6 byte) will always return 8 bytes of data) ++ * ++ * This means that a value of 15 (0xf) actually means reading/writing 32 bytes ++ * of data. ++ * ++ * To correctly calculate the length we devide the requested len by word and ++ * round up. ++ * On the ack function we can skip the odd check as we already handle the ++ * case here. + */ +- if (len == 16) +- real_len = 15; +- else +- real_len = len; ++ real_len = DIV_ROUND_UP(len, sizeof(u16)); ++ ++ /* We check if the result len is odd and we round up another time to ++ * the next size. (length of 3 will be increased to 4 as switch will always ++ * return 8 bytes) ++ */ ++ if (real_len % sizeof(u16) != 0) ++ real_len++; ++ ++ /* Max reg value is 0xf(15) but switch will always return the next size (32 byte) */ ++ if (real_len == 16) ++ real_len--; + + skb_reset_mac_header(skb); + skb_set_network_header(skb, skb->len); diff --git a/target/linux/generic/backport-5.15/777-v6.2-02-net-dsa-tag_qca-fix-wrong-MGMT_DATA2-size.patch b/target/linux/generic/backport-5.15/777-v6.2-02-net-dsa-tag_qca-fix-wrong-MGMT_DATA2-size.patch new file mode 100644 index 0000000000..55ecb1eb42 --- /dev/null +++ b/target/linux/generic/backport-5.15/777-v6.2-02-net-dsa-tag_qca-fix-wrong-MGMT_DATA2-size.patch @@ -0,0 +1,34 @@ +From d9dba91be71f03cc75bcf39fc0d5d99ff33f1ae0 Mon Sep 17 00:00:00 2001 +From: Christian Marangi +Date: Thu, 29 Dec 2022 17:33:33 +0100 +Subject: [PATCH 2/5] net: dsa: tag_qca: fix wrong MGMT_DATA2 size + +It was discovered that MGMT_DATA2 can contain up to 28 bytes of data +instead of the 12 bytes written in the Documentation by accounting the +limit of 16 bytes declared in Documentation subtracting the first 4 byte +in the packet header. + +Update the define with the real world value. + +Tested-by: Ronald Wahl +Fixes: c2ee8181fddb ("net: dsa: tag_qca: add define for handling mgmt Ethernet packet") +Signed-off-by: Christian Marangi +Cc: stable@vger.kernel.org # v5.18+ +Signed-off-by: David S. Miller +--- + include/linux/dsa/tag_qca.h | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +--- a/include/linux/dsa/tag_qca.h ++++ b/include/linux/dsa/tag_qca.h +@@ -40,8 +40,8 @@ + QCA_HDR_MGMT_COMMAND_LEN + \ + QCA_HDR_MGMT_DATA1_LEN) + +-#define QCA_HDR_MGMT_DATA2_LEN 12 /* Other 12 byte for the mdio data */ +-#define QCA_HDR_MGMT_PADDING_LEN 34 /* Padding to reach the min Ethernet packet */ ++#define QCA_HDR_MGMT_DATA2_LEN 28 /* Other 28 byte for the mdio data */ ++#define QCA_HDR_MGMT_PADDING_LEN 18 /* Padding to reach the min Ethernet packet */ + + #define QCA_HDR_MGMT_PKT_LEN (QCA_HDR_MGMT_HEADER_LEN + \ + QCA_HDR_LEN + \ diff --git a/target/linux/generic/backport-5.15/777-v6.2-03-Revert-net-dsa-qca8k-cache-lo-and-hi-for-mdio-write.patch b/target/linux/generic/backport-5.15/777-v6.2-03-Revert-net-dsa-qca8k-cache-lo-and-hi-for-mdio-write.patch new file mode 100644 index 0000000000..c8e22fd1b8 --- /dev/null +++ b/target/linux/generic/backport-5.15/777-v6.2-03-Revert-net-dsa-qca8k-cache-lo-and-hi-for-mdio-write.patch @@ -0,0 +1,172 @@ +From 03cb9e6d0b32b768e3d9d473c5c4ca1100877664 Mon Sep 17 00:00:00 2001 +From: Christian Marangi +Date: Thu, 29 Dec 2022 17:33:34 +0100 +Subject: [PATCH 3/5] Revert "net: dsa: qca8k: cache lo and hi for mdio write" + +This reverts commit 2481d206fae7884cd07014fd1318e63af35e99eb. + +The Documentation is very confusing about the topic. +The cache logic for hi and lo is wrong and actually miss some regs to be +actually written. + +What the Documentation actually intended was that it's possible to skip +writing hi OR lo if half of the reg is not needed to be written or read. + +Revert the change in favor of a better and correct implementation. + +Reported-by: Ronald Wahl +Signed-off-by: Christian Marangi +Cc: stable@vger.kernel.org # v5.18+ +Signed-off-by: David S. Miller +--- + drivers/net/dsa/qca/qca8k-8xxx.c | 61 +++++++------------------------- + drivers/net/dsa/qca/qca8k.h | 5 --- + 2 files changed, 12 insertions(+), 54 deletions(-) + +--- a/drivers/net/dsa/qca/qca8k-8xxx.c ++++ b/drivers/net/dsa/qca/qca8k-8xxx.c +@@ -37,44 +37,6 @@ qca8k_split_addr(u32 regaddr, u16 *r1, u + } + + static int +-qca8k_set_lo(struct qca8k_priv *priv, int phy_id, u32 regnum, u16 lo) +-{ +- u16 *cached_lo = &priv->mdio_cache.lo; +- struct mii_bus *bus = priv->bus; +- int ret; +- +- if (lo == *cached_lo) +- return 0; +- +- ret = bus->write(bus, phy_id, regnum, lo); +- if (ret < 0) +- dev_err_ratelimited(&bus->dev, +- "failed to write qca8k 32bit lo register\n"); +- +- *cached_lo = lo; +- return 0; +-} +- +-static int +-qca8k_set_hi(struct qca8k_priv *priv, int phy_id, u32 regnum, u16 hi) +-{ +- u16 *cached_hi = &priv->mdio_cache.hi; +- struct mii_bus *bus = priv->bus; +- int ret; +- +- if (hi == *cached_hi) +- return 0; +- +- ret = bus->write(bus, phy_id, regnum, hi); +- if (ret < 0) +- dev_err_ratelimited(&bus->dev, +- "failed to write qca8k 32bit hi register\n"); +- +- *cached_hi = hi; +- return 0; +-} +- +-static int + qca8k_mii_read32(struct mii_bus *bus, int phy_id, u32 regnum, u32 *val) + { + int ret; +@@ -97,7 +59,7 @@ qca8k_mii_read32(struct mii_bus *bus, in + } + + static void +-qca8k_mii_write32(struct qca8k_priv *priv, int phy_id, u32 regnum, u32 val) ++qca8k_mii_write32(struct mii_bus *bus, int phy_id, u32 regnum, u32 val) + { + u16 lo, hi; + int ret; +@@ -105,9 +67,12 @@ qca8k_mii_write32(struct qca8k_priv *pri + lo = val & 0xffff; + hi = (u16)(val >> 16); + +- ret = qca8k_set_lo(priv, phy_id, regnum, lo); ++ ret = bus->write(bus, phy_id, regnum, lo); + if (ret >= 0) +- ret = qca8k_set_hi(priv, phy_id, regnum + 1, hi); ++ ret = bus->write(bus, phy_id, regnum + 1, hi); ++ if (ret < 0) ++ dev_err_ratelimited(&bus->dev, ++ "failed to write qca8k 32bit register\n"); + } + + static int +@@ -442,7 +407,7 @@ qca8k_regmap_write(void *ctx, uint32_t r + if (ret < 0) + goto exit; + +- qca8k_mii_write32(priv, 0x10 | r2, r1, val); ++ qca8k_mii_write32(bus, 0x10 | r2, r1, val); + + exit: + mutex_unlock(&bus->mdio_lock); +@@ -475,7 +440,7 @@ qca8k_regmap_update_bits(void *ctx, uint + + val &= ~mask; + val |= write_val; +- qca8k_mii_write32(priv, 0x10 | r2, r1, val); ++ qca8k_mii_write32(bus, 0x10 | r2, r1, val); + + exit: + mutex_unlock(&bus->mdio_lock); +@@ -750,14 +715,14 @@ qca8k_mdio_write(struct qca8k_priv *priv + if (ret) + goto exit; + +- qca8k_mii_write32(priv, 0x10 | r2, r1, val); ++ qca8k_mii_write32(bus, 0x10 | r2, r1, val); + + ret = qca8k_mdio_busy_wait(bus, QCA8K_MDIO_MASTER_CTRL, + QCA8K_MDIO_MASTER_BUSY); + + exit: + /* even if the busy_wait timeouts try to clear the MASTER_EN */ +- qca8k_mii_write32(priv, 0x10 | r2, r1, 0); ++ qca8k_mii_write32(bus, 0x10 | r2, r1, 0); + + mutex_unlock(&bus->mdio_lock); + +@@ -787,7 +752,7 @@ qca8k_mdio_read(struct qca8k_priv *priv, + if (ret) + goto exit; + +- qca8k_mii_write32(priv, 0x10 | r2, r1, val); ++ qca8k_mii_write32(bus, 0x10 | r2, r1, val); + + ret = qca8k_mdio_busy_wait(bus, QCA8K_MDIO_MASTER_CTRL, + QCA8K_MDIO_MASTER_BUSY); +@@ -798,7 +763,7 @@ qca8k_mdio_read(struct qca8k_priv *priv, + + exit: + /* even if the busy_wait timeouts try to clear the MASTER_EN */ +- qca8k_mii_write32(priv, 0x10 | r2, r1, 0); ++ qca8k_mii_write32(bus, 0x10 | r2, r1, 0); + + mutex_unlock(&bus->mdio_lock); + +@@ -1914,8 +1879,6 @@ qca8k_sw_probe(struct mdio_device *mdiod + } + + priv->mdio_cache.page = 0xffff; +- priv->mdio_cache.lo = 0xffff; +- priv->mdio_cache.hi = 0xffff; + + /* Check the detected switch id */ + ret = qca8k_read_switch_id(priv); +--- a/drivers/net/dsa/qca/qca8k.h ++++ b/drivers/net/dsa/qca/qca8k.h +@@ -375,11 +375,6 @@ struct qca8k_mdio_cache { + * mdio writes + */ + u16 page; +-/* lo and hi can also be cached and from Documentation we can skip one +- * extra mdio write if lo or hi is didn't change. +- */ +- u16 lo; +- u16 hi; + }; + + struct qca8k_priv { diff --git a/target/linux/generic/backport-5.15/777-v6.2-04-net-dsa-qca8k-introduce-single-mii-read-write-lo-hi.patch b/target/linux/generic/backport-5.15/777-v6.2-04-net-dsa-qca8k-introduce-single-mii-read-write-lo-hi.patch new file mode 100644 index 0000000000..c0320ad6f9 --- /dev/null +++ b/target/linux/generic/backport-5.15/777-v6.2-04-net-dsa-qca8k-introduce-single-mii-read-write-lo-hi.patch @@ -0,0 +1,150 @@ +From cfbd6de588ef659c198083205dc954a6d3ed2aec Mon Sep 17 00:00:00 2001 +From: Christian Marangi +Date: Thu, 29 Dec 2022 17:33:35 +0100 +Subject: [PATCH 4/5] net: dsa: qca8k: introduce single mii read/write lo/hi + +It may be useful to read/write just the lo or hi half of a reg. + +This is especially useful for phy poll with the use of mdio master. +The mdio master reg is composed by the first 16 bit related to setup and +the other half with the returned data or data to write. + +Refactor the mii function to permit single mii read/write of lo or hi +half of the reg. + +Tested-by: Ronald Wahl +Signed-off-by: Christian Marangi +Signed-off-by: David S. Miller +--- + drivers/net/dsa/qca/qca8k-8xxx.c | 106 ++++++++++++++++++++++++------- + 1 file changed, 84 insertions(+), 22 deletions(-) + +--- a/drivers/net/dsa/qca/qca8k-8xxx.c ++++ b/drivers/net/dsa/qca/qca8k-8xxx.c +@@ -37,42 +37,104 @@ qca8k_split_addr(u32 regaddr, u16 *r1, u + } + + static int +-qca8k_mii_read32(struct mii_bus *bus, int phy_id, u32 regnum, u32 *val) ++qca8k_mii_write_lo(struct mii_bus *bus, int phy_id, u32 regnum, u32 val) + { + int ret; ++ u16 lo; + +- ret = bus->read(bus, phy_id, regnum); +- if (ret >= 0) { +- *val = ret; +- ret = bus->read(bus, phy_id, regnum + 1); +- *val |= ret << 16; +- } ++ lo = val & 0xffff; ++ ret = bus->write(bus, phy_id, regnum, lo); ++ if (ret < 0) ++ dev_err_ratelimited(&bus->dev, ++ "failed to write qca8k 32bit lo register\n"); ++ ++ return ret; ++} + +- if (ret < 0) { ++static int ++qca8k_mii_write_hi(struct mii_bus *bus, int phy_id, u32 regnum, u32 val) ++{ ++ int ret; ++ u16 hi; ++ ++ hi = (u16)(val >> 16); ++ ret = bus->write(bus, phy_id, regnum, hi); ++ if (ret < 0) + dev_err_ratelimited(&bus->dev, +- "failed to read qca8k 32bit register\n"); +- *val = 0; +- return ret; +- } ++ "failed to write qca8k 32bit hi register\n"); + ++ return ret; ++} ++ ++static int ++qca8k_mii_read_lo(struct mii_bus *bus, int phy_id, u32 regnum, u32 *val) ++{ ++ int ret; ++ ++ ret = bus->read(bus, phy_id, regnum); ++ if (ret < 0) ++ goto err; ++ ++ *val = ret & 0xffff; + return 0; ++ ++err: ++ dev_err_ratelimited(&bus->dev, ++ "failed to read qca8k 32bit lo register\n"); ++ *val = 0; ++ ++ return ret; + } + +-static void +-qca8k_mii_write32(struct mii_bus *bus, int phy_id, u32 regnum, u32 val) ++static int ++qca8k_mii_read_hi(struct mii_bus *bus, int phy_id, u32 regnum, u32 *val) + { +- u16 lo, hi; + int ret; + +- lo = val & 0xffff; +- hi = (u16)(val >> 16); ++ ret = bus->read(bus, phy_id, regnum); ++ if (ret < 0) ++ goto err; + +- ret = bus->write(bus, phy_id, regnum, lo); +- if (ret >= 0) +- ret = bus->write(bus, phy_id, regnum + 1, hi); ++ *val = ret << 16; ++ return 0; ++ ++err: ++ dev_err_ratelimited(&bus->dev, ++ "failed to read qca8k 32bit hi register\n"); ++ *val = 0; ++ ++ return ret; ++} ++ ++static int ++qca8k_mii_read32(struct mii_bus *bus, int phy_id, u32 regnum, u32 *val) ++{ ++ u32 hi, lo; ++ int ret; ++ ++ *val = 0; ++ ++ ret = qca8k_mii_read_lo(bus, phy_id, regnum, &lo); + if (ret < 0) +- dev_err_ratelimited(&bus->dev, +- "failed to write qca8k 32bit register\n"); ++ goto err; ++ ++ ret = qca8k_mii_read_hi(bus, phy_id, regnum + 1, &hi); ++ if (ret < 0) ++ goto err; ++ ++ *val = lo | hi; ++ ++err: ++ return ret; ++} ++ ++static void ++qca8k_mii_write32(struct mii_bus *bus, int phy_id, u32 regnum, u32 val) ++{ ++ if (qca8k_mii_write_lo(bus, phy_id, regnum, val) < 0) ++ return; ++ ++ qca8k_mii_write_hi(bus, phy_id, regnum + 1, val); + } + + static int diff --git a/target/linux/generic/backport-5.15/777-v6.2-05-net-dsa-qca8k-improve-mdio-master-read-write-by-usin.patch b/target/linux/generic/backport-5.15/777-v6.2-05-net-dsa-qca8k-improve-mdio-master-read-write-by-usin.patch new file mode 100644 index 0000000000..4cbb66cf35 --- /dev/null +++ b/target/linux/generic/backport-5.15/777-v6.2-05-net-dsa-qca8k-improve-mdio-master-read-write-by-usin.patch @@ -0,0 +1,73 @@ +From a4165830ca237f2b3318faf62562bce8ce12a389 Mon Sep 17 00:00:00 2001 +From: Christian Marangi +Date: Thu, 29 Dec 2022 17:33:36 +0100 +Subject: [PATCH 5/5] net: dsa: qca8k: improve mdio master read/write by using + single lo/hi + +Improve mdio master read/write by using singe mii read/write lo/hi. + +In a read and write we need to poll the mdio master regs in a busy loop +to check for a specific bit present in the upper half of the reg. We can +ignore the other half since it won't contain useful data. This will save +an additional useless read for each read and write operation. + +In a read operation the returned data is present in the mdio master reg +lower half. We can ignore the other half since it won't contain useful +data. This will save an additional useless read for each read operation. + +In a read operation it's needed to just set the hi half of the mdio +master reg as the lo half will be replaced by the result. This will save +an additional useless write for each read operation. + +Tested-by: Ronald Wahl +Signed-off-by: Christian Marangi +Signed-off-by: David S. Miller +--- + drivers/net/dsa/qca/qca8k-8xxx.c | 12 ++++++------ + 1 file changed, 6 insertions(+), 6 deletions(-) + +--- a/drivers/net/dsa/qca/qca8k-8xxx.c ++++ b/drivers/net/dsa/qca/qca8k-8xxx.c +@@ -740,9 +740,9 @@ qca8k_mdio_busy_wait(struct mii_bus *bus + + qca8k_split_addr(reg, &r1, &r2, &page); + +- ret = read_poll_timeout(qca8k_mii_read32, ret1, !(val & mask), 0, ++ ret = read_poll_timeout(qca8k_mii_read_hi, ret1, !(val & mask), 0, + QCA8K_BUSY_WAIT_TIMEOUT * USEC_PER_MSEC, false, +- bus, 0x10 | r2, r1, &val); ++ bus, 0x10 | r2, r1 + 1, &val); + + /* Check if qca8k_read has failed for a different reason + * before returnting -ETIMEDOUT +@@ -784,7 +784,7 @@ qca8k_mdio_write(struct qca8k_priv *priv + + exit: + /* even if the busy_wait timeouts try to clear the MASTER_EN */ +- qca8k_mii_write32(bus, 0x10 | r2, r1, 0); ++ qca8k_mii_write_hi(bus, 0x10 | r2, r1 + 1, 0); + + mutex_unlock(&bus->mdio_lock); + +@@ -814,18 +814,18 @@ qca8k_mdio_read(struct qca8k_priv *priv, + if (ret) + goto exit; + +- qca8k_mii_write32(bus, 0x10 | r2, r1, val); ++ qca8k_mii_write_hi(bus, 0x10 | r2, r1 + 1, val); + + ret = qca8k_mdio_busy_wait(bus, QCA8K_MDIO_MASTER_CTRL, + QCA8K_MDIO_MASTER_BUSY); + if (ret) + goto exit; + +- ret = qca8k_mii_read32(bus, 0x10 | r2, r1, &val); ++ ret = qca8k_mii_read_lo(bus, 0x10 | r2, r1, &val); + + exit: + /* even if the busy_wait timeouts try to clear the MASTER_EN */ +- qca8k_mii_write32(bus, 0x10 | r2, r1, 0); ++ qca8k_mii_write_hi(bus, 0x10 | r2, r1 + 1, 0); + + mutex_unlock(&bus->mdio_lock); +