Messages in this thread | | | Date | Tue, 25 Jan 2022 19:48:27 -0800 | Subject | Re: [RFC PATCH v7 15/16] net: da: qca8k: add support for larger read/write size with mgmt Ethernet | From | Florian Fainelli <> |
| |
On 1/22/2022 5:33 PM, Ansuel Smith wrote: > mgmt Ethernet packet can read/write up to 16byte at times. The len reg > is limited to 15 (0xf). The switch actually sends and accepts data in 4 > different steps of len values. > Len steps: > - 0: nothing > - 1-4: first 4 byte > - 5-6: first 12 byte > - 7-15: all 16 byte
This is really odd, it almost felt like the length was a byte enable bitmask, but it is not?
> > In the allock skb function we check if the len is 16 and we fix it to a > len of 15.
s/allock/alloc/
> It the read/write function interest to extract the real asked > data. The tagger handler will always copy the fully 16byte with a READ > command. This is useful for some big regs like the fdb reg that are > more than 4byte of data. This permits to introduce a bulk function that > will send and request the entire entry in one go. > Write function is changed and it does now require to pass the pointer to > val to also handle array val. > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> > --- > drivers/net/dsa/qca8k.c | 56 ++++++++++++++++++++++++++++++----------- > 1 file changed, 41 insertions(+), 15 deletions(-) > > diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c > index 2a43fb9aeef2..0183ce2d5b74 100644 > --- a/drivers/net/dsa/qca8k.c > +++ b/drivers/net/dsa/qca8k.c > @@ -219,7 +219,9 @@ static void qca8k_rw_reg_ack_handler(struct dsa_switch *ds, struct sk_buff *skb) > if (cmd == MDIO_READ) { > mgmt_hdr_data->data[0] = mgmt_ethhdr->mdio_data; > > - /* Get the rest of the 12 byte of data */ > + /* Get the rest of the 12 byte of data. > + * The read/write function will extract the requested data. > + */ > if (len > QCA_HDR_MGMT_DATA1_LEN) > memcpy(mgmt_hdr_data->data + 1, skb->data, > QCA_HDR_MGMT_DATA2_LEN); > @@ -229,16 +231,30 @@ static void qca8k_rw_reg_ack_handler(struct dsa_switch *ds, struct sk_buff *skb) > } > > static struct sk_buff *qca8k_alloc_mdio_header(enum mdio_cmd cmd, u32 reg, u32 *val, > - int seq_num, int priority) > + int seq_num, int priority, int len)
unsigned int len
> { > struct mgmt_ethhdr *mgmt_ethhdr; > struct sk_buff *skb; > + int real_len;
Likewise.
> + u32 *data2; > u16 hdr; > > skb = dev_alloc_skb(QCA_HDR_MGMT_PKG_LEN); > 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 > + */ > + if (len == 16) > + real_len = 15; > + else > + real_len = len; > + > skb_reset_mac_header(skb); > skb_set_network_header(skb, skb->len); > > @@ -253,7 +269,7 @@ static struct sk_buff *qca8k_alloc_mdio_header(enum mdio_cmd cmd, u32 reg, u32 * > mgmt_ethhdr->seq = FIELD_PREP(QCA_HDR_MGMT_SEQ_NUM, seq_num); > > mgmt_ethhdr->command = FIELD_PREP(QCA_HDR_MGMT_ADDR, reg); > - mgmt_ethhdr->command |= FIELD_PREP(QCA_HDR_MGMT_LENGTH, 4); > + mgmt_ethhdr->command |= FIELD_PREP(QCA_HDR_MGMT_LENGTH, real_len); > mgmt_ethhdr->command |= FIELD_PREP(QCA_HDR_MGMT_CMD, cmd); > mgmt_ethhdr->command |= FIELD_PREP(QCA_HDR_MGMT_CHECK_CODE, > QCA_HDR_MGMT_CHECK_CODE_VAL); > @@ -263,19 +279,22 @@ static struct sk_buff *qca8k_alloc_mdio_header(enum mdio_cmd cmd, u32 reg, u32 * > > mgmt_ethhdr->hdr = htons(hdr); > > - skb_put_zero(skb, QCA_HDR_MGMT_DATA2_LEN + QCA_HDR_MGMT_PADDING_LEN); > + data2 = skb_put_zero(skb, QCA_HDR_MGMT_DATA2_LEN + QCA_HDR_MGMT_PADDING_LEN); > + if (cmd == MDIO_WRITE && len > QCA_HDR_MGMT_DATA1_LEN) > + memcpy(data2, val + 1, len - QCA_HDR_MGMT_DATA1_LEN); > > return skb; > } > > -static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val) > +static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len) > { > struct qca8k_mgmt_hdr_data *mgmt_hdr_data = &priv->mgmt_hdr_data; > struct sk_buff *skb; > bool ack; > int ret; > > - skb = qca8k_alloc_mdio_header(MDIO_READ, reg, NULL, 200, QCA8K_ETHERNET_MDIO_PRIORITY); > + skb = qca8k_alloc_mdio_header(MDIO_READ, reg, NULL, 200, > + QCA8K_ETHERNET_MDIO_PRIORITY, len); > if (!skb) > return -ENOMEM; > > @@ -297,6 +316,9 @@ static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val) > msecs_to_jiffies(QCA8K_ETHERNET_TIMEOUT)); > > *val = mgmt_hdr_data->data[0]; > + if (len > QCA_HDR_MGMT_DATA1_LEN) > + memcpy(val + 1, mgmt_hdr_data->data + 1, len - QCA_HDR_MGMT_DATA1_LEN); > + > ack = mgmt_hdr_data->ack; > > mutex_unlock(&mgmt_hdr_data->mutex); > @@ -310,14 +332,15 @@ static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val) > return 0; > } > > -static int qca8k_write_eth(struct qca8k_priv *priv, u32 reg, u32 val) > +static int qca8k_write_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len) > { > struct qca8k_mgmt_hdr_data *mgmt_hdr_data = &priv->mgmt_hdr_data; > struct sk_buff *skb; > bool ack; > int ret; > > - skb = qca8k_alloc_mdio_header(MDIO_WRITE, reg, &val, 200, QCA8K_ETHERNET_MDIO_PRIORITY); > + skb = qca8k_alloc_mdio_header(MDIO_WRITE, reg, val, 200, > + QCA8K_ETHERNET_MDIO_PRIORITY, len); > if (!skb) > return -ENOMEM; > > @@ -357,14 +380,14 @@ qca8k_regmap_update_bits_eth(struct qca8k_priv *priv, u32 reg, u32 mask, u32 wri > u32 val = 0; > int ret; > > - ret = qca8k_read_eth(priv, reg, &val); > + ret = qca8k_read_eth(priv, reg, &val, 4);
sizeof(val) instead of 4.
> if (ret) > return ret; > > val &= ~mask; > val |= write_val; > > - return qca8k_write_eth(priv, reg, val); > + return qca8k_write_eth(priv, reg, &val, 4);
Likewise
> } > > static int > @@ -376,7 +399,7 @@ qca8k_regmap_read(void *ctx, uint32_t reg, uint32_t *val) > u16 r1, r2, page; > int ret; > > - if (priv->mgmt_master && !qca8k_read_eth(priv, reg, val)) > + if (priv->mgmt_master && !qca8k_read_eth(priv, reg, val, 4))
Likewise and everywhere below as well. -- Florian
| |