Commit d02b8256 authored by David S. Miller's avatar David S. Miller

Merge branch 'dsa-qca8k-fixes'

Christian Marangi says:

====================
net: dsa: qca8k: multiple fix on mdio read/write

Due to some problems in reading the Documentation and elaborating it
some wrong assumption were done. The error was reported and notice only
now due to how things are setup in the code flow.

First 2 patch fix mgmt eth where the lenght calculation is very
confusing and in step of word size. (the related commit description have
an extensive description about how this mess works)

Last 3 patch revert the broken mdio cache and apply a correct version
that should still save some extra mdio in phy poll secnario.

These 5 patch fix each related problem and apply what the Documentation
actually say.

Changes v2:
- Add cover letter
- Fix typo in revert patch
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents d0395358 a4165830
...@@ -37,77 +37,104 @@ qca8k_split_addr(u32 regaddr, u16 *r1, u16 *r2, u16 *page) ...@@ -37,77 +37,104 @@ qca8k_split_addr(u32 regaddr, u16 *r1, u16 *r2, u16 *page)
} }
static int static int
qca8k_set_lo(struct qca8k_priv *priv, int phy_id, u32 regnum, u16 lo) qca8k_mii_write_lo(struct mii_bus *bus, int phy_id, u32 regnum, u32 val)
{ {
u16 *cached_lo = &priv->mdio_cache.lo;
struct mii_bus *bus = priv->bus;
int ret; int ret;
u16 lo;
if (lo == *cached_lo) lo = val & 0xffff;
return 0;
ret = bus->write(bus, phy_id, regnum, lo); ret = bus->write(bus, phy_id, regnum, lo);
if (ret < 0) if (ret < 0)
dev_err_ratelimited(&bus->dev, dev_err_ratelimited(&bus->dev,
"failed to write qca8k 32bit lo register\n"); "failed to write qca8k 32bit lo register\n");
*cached_lo = lo; return ret;
return 0;
} }
static int static int
qca8k_set_hi(struct qca8k_priv *priv, int phy_id, u32 regnum, u16 hi) qca8k_mii_write_hi(struct mii_bus *bus, int phy_id, u32 regnum, u32 val)
{ {
u16 *cached_hi = &priv->mdio_cache.hi;
struct mii_bus *bus = priv->bus;
int ret; int ret;
u16 hi;
if (hi == *cached_hi) hi = (u16)(val >> 16);
return 0;
ret = bus->write(bus, phy_id, regnum, hi); ret = bus->write(bus, phy_id, regnum, hi);
if (ret < 0) if (ret < 0)
dev_err_ratelimited(&bus->dev, dev_err_ratelimited(&bus->dev,
"failed to write qca8k 32bit hi register\n"); "failed to write qca8k 32bit hi register\n");
*cached_hi = hi; return ret;
return 0;
} }
static int static int
qca8k_mii_read32(struct mii_bus *bus, int phy_id, u32 regnum, u32 *val) qca8k_mii_read_lo(struct mii_bus *bus, int phy_id, u32 regnum, u32 *val)
{ {
int ret; int ret;
ret = bus->read(bus, phy_id, regnum); ret = bus->read(bus, phy_id, regnum);
if (ret >= 0) { if (ret < 0)
*val = ret; goto err;
ret = bus->read(bus, phy_id, regnum + 1);
*val |= ret << 16;
}
if (ret < 0) { *val = ret & 0xffff;
dev_err_ratelimited(&bus->dev, return 0;
"failed to read qca8k 32bit register\n");
*val = 0; err:
return ret; dev_err_ratelimited(&bus->dev,
} "failed to read qca8k 32bit lo register\n");
*val = 0;
return ret;
}
static int
qca8k_mii_read_hi(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 << 16;
return 0; return 0;
err:
dev_err_ratelimited(&bus->dev,
"failed to read qca8k 32bit hi register\n");
*val = 0;
return ret;
} }
static void static int
qca8k_mii_write32(struct qca8k_priv *priv, int phy_id, u32 regnum, u32 val) qca8k_mii_read32(struct mii_bus *bus, int phy_id, u32 regnum, u32 *val)
{ {
u16 lo, hi; u32 hi, lo;
int ret; int ret;
lo = val & 0xffff; *val = 0;
hi = (u16)(val >> 16);
ret = qca8k_set_lo(priv, phy_id, regnum, lo); ret = qca8k_mii_read_lo(bus, phy_id, regnum, &lo);
if (ret >= 0) if (ret < 0)
ret = qca8k_set_hi(priv, phy_id, regnum + 1, hi); 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 static int
...@@ -146,7 +173,16 @@ static void qca8k_rw_reg_ack_handler(struct dsa_switch *ds, struct sk_buff *skb) ...@@ -146,7 +173,16 @@ static void qca8k_rw_reg_ack_handler(struct dsa_switch *ds, struct sk_buff *skb)
command = get_unaligned_le32(&mgmt_ethhdr->command); command = get_unaligned_le32(&mgmt_ethhdr->command);
cmd = FIELD_GET(QCA_HDR_MGMT_CMD, command); cmd = FIELD_GET(QCA_HDR_MGMT_CMD, command);
len = FIELD_GET(QCA_HDR_MGMT_LENGTH, 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 */ /* Make sure the seq match the requested packet */
if (get_unaligned_le32(&mgmt_ethhdr->seq) == mgmt_eth_data->seq) if (get_unaligned_le32(&mgmt_ethhdr->seq) == mgmt_eth_data->seq)
...@@ -193,17 +229,33 @@ static struct sk_buff *qca8k_alloc_mdio_header(enum mdio_cmd cmd, u32 reg, u32 * ...@@ -193,17 +229,33 @@ static struct sk_buff *qca8k_alloc_mdio_header(enum mdio_cmd cmd, u32 reg, u32 *
if (!skb) if (!skb)
return NULL; return NULL;
/* Max value for len reg is 15 (0xf) but the switch actually return 16 byte /* Hdr mgmt length value is in step of word size.
* Actually for some reason the steps are: * As an example to process 4 byte of data the correct length to set is 2.
* 0: nothing * To process 8 byte 4, 12 byte 6, 16 byte 8...
* 1-4: first 4 byte *
* 5-6: first 12 byte * Odd values will always return the next size on the ack packet.
* 7-15: all 16 byte * (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 = DIV_ROUND_UP(len, sizeof(u16));
real_len = 15;
else /* We check if the result len is odd and we round up another time to
real_len = len; * 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_reset_mac_header(skb);
skb_set_network_header(skb, skb->len); skb_set_network_header(skb, skb->len);
...@@ -417,7 +469,7 @@ qca8k_regmap_write(void *ctx, uint32_t reg, uint32_t val) ...@@ -417,7 +469,7 @@ qca8k_regmap_write(void *ctx, uint32_t reg, uint32_t val)
if (ret < 0) if (ret < 0)
goto exit; goto exit;
qca8k_mii_write32(priv, 0x10 | r2, r1, val); qca8k_mii_write32(bus, 0x10 | r2, r1, val);
exit: exit:
mutex_unlock(&bus->mdio_lock); mutex_unlock(&bus->mdio_lock);
...@@ -450,7 +502,7 @@ qca8k_regmap_update_bits(void *ctx, uint32_t reg, uint32_t mask, uint32_t write_ ...@@ -450,7 +502,7 @@ qca8k_regmap_update_bits(void *ctx, uint32_t reg, uint32_t mask, uint32_t write_
val &= ~mask; val &= ~mask;
val |= write_val; val |= write_val;
qca8k_mii_write32(priv, 0x10 | r2, r1, val); qca8k_mii_write32(bus, 0x10 | r2, r1, val);
exit: exit:
mutex_unlock(&bus->mdio_lock); mutex_unlock(&bus->mdio_lock);
...@@ -688,9 +740,9 @@ qca8k_mdio_busy_wait(struct mii_bus *bus, u32 reg, u32 mask) ...@@ -688,9 +740,9 @@ qca8k_mdio_busy_wait(struct mii_bus *bus, u32 reg, u32 mask)
qca8k_split_addr(reg, &r1, &r2, &page); 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, 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 /* Check if qca8k_read has failed for a different reason
* before returnting -ETIMEDOUT * before returnting -ETIMEDOUT
...@@ -725,14 +777,14 @@ qca8k_mdio_write(struct qca8k_priv *priv, int phy, int regnum, u16 data) ...@@ -725,14 +777,14 @@ qca8k_mdio_write(struct qca8k_priv *priv, int phy, int regnum, u16 data)
if (ret) if (ret)
goto exit; 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, ret = qca8k_mdio_busy_wait(bus, QCA8K_MDIO_MASTER_CTRL,
QCA8K_MDIO_MASTER_BUSY); QCA8K_MDIO_MASTER_BUSY);
exit: exit:
/* even if the busy_wait timeouts try to clear the MASTER_EN */ /* even if the busy_wait timeouts try to clear the MASTER_EN */
qca8k_mii_write32(priv, 0x10 | r2, r1, 0); qca8k_mii_write_hi(bus, 0x10 | r2, r1 + 1, 0);
mutex_unlock(&bus->mdio_lock); mutex_unlock(&bus->mdio_lock);
...@@ -762,18 +814,18 @@ qca8k_mdio_read(struct qca8k_priv *priv, int phy, int regnum) ...@@ -762,18 +814,18 @@ qca8k_mdio_read(struct qca8k_priv *priv, int phy, int regnum)
if (ret) if (ret)
goto exit; goto exit;
qca8k_mii_write32(priv, 0x10 | r2, r1, val); qca8k_mii_write_hi(bus, 0x10 | r2, r1 + 1, val);
ret = qca8k_mdio_busy_wait(bus, QCA8K_MDIO_MASTER_CTRL, ret = qca8k_mdio_busy_wait(bus, QCA8K_MDIO_MASTER_CTRL,
QCA8K_MDIO_MASTER_BUSY); QCA8K_MDIO_MASTER_BUSY);
if (ret) if (ret)
goto exit; goto exit;
ret = qca8k_mii_read32(bus, 0x10 | r2, r1, &val); ret = qca8k_mii_read_lo(bus, 0x10 | r2, r1, &val);
exit: exit:
/* even if the busy_wait timeouts try to clear the MASTER_EN */ /* even if the busy_wait timeouts try to clear the MASTER_EN */
qca8k_mii_write32(priv, 0x10 | r2, r1, 0); qca8k_mii_write_hi(bus, 0x10 | r2, r1 + 1, 0);
mutex_unlock(&bus->mdio_lock); mutex_unlock(&bus->mdio_lock);
...@@ -1943,8 +1995,6 @@ qca8k_sw_probe(struct mdio_device *mdiodev) ...@@ -1943,8 +1995,6 @@ qca8k_sw_probe(struct mdio_device *mdiodev)
} }
priv->mdio_cache.page = 0xffff; priv->mdio_cache.page = 0xffff;
priv->mdio_cache.lo = 0xffff;
priv->mdio_cache.hi = 0xffff;
/* Check the detected switch id */ /* Check the detected switch id */
ret = qca8k_read_switch_id(priv); ret = qca8k_read_switch_id(priv);
......
...@@ -375,11 +375,6 @@ struct qca8k_mdio_cache { ...@@ -375,11 +375,6 @@ struct qca8k_mdio_cache {
* mdio writes * mdio writes
*/ */
u16 page; 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_pcs { struct qca8k_pcs {
......
...@@ -45,8 +45,8 @@ struct sk_buff; ...@@ -45,8 +45,8 @@ struct sk_buff;
QCA_HDR_MGMT_COMMAND_LEN + \ QCA_HDR_MGMT_COMMAND_LEN + \
QCA_HDR_MGMT_DATA1_LEN) QCA_HDR_MGMT_DATA1_LEN)
#define QCA_HDR_MGMT_DATA2_LEN 12 /* Other 12 byte for the mdio data */ #define QCA_HDR_MGMT_DATA2_LEN 28 /* Other 28 byte for the mdio data */
#define QCA_HDR_MGMT_PADDING_LEN 34 /* Padding to reach the min Ethernet packet */ #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 + \ #define QCA_HDR_MGMT_PKT_LEN (QCA_HDR_MGMT_HEADER_LEN + \
QCA_HDR_LEN + \ QCA_HDR_LEN + \
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment