Messages in this thread | | | Date | Wed, 8 Dec 2021 15:21:11 +0100 | From | Ansuel Smith <> | Subject | Re: [net-next RFC PATCH v2 7/8] net: dsa: qca8k: Add support for mdio read/write in Ethernet packet |
| |
On Wed, Dec 08, 2021 at 02:18:50PM +0200, Vladimir Oltean wrote: > On Wed, Dec 08, 2021 at 04:40:39AM +0100, Ansuel Smith wrote: > > Add qca8k side support for mdio read/write in Ethernet packet. > > qca8k supports some specially crafted Ethernet packet that can be used > > for mdio read/write instead of the legacy method uart/internal mdio. > > This add support for the qca8k side to craft the packet and enqueue it. > > Each port and the qca8k_priv have a special struct to put data in it. > > The completion API is used to wait for the packet to be received back > > with the requested data. > > > > The various steps are: > > 1. Craft the special packet with the qca hdr set to mdio read/write > > mode. > > 2. Set the lock in the dedicated mdio struct. > > 3. Reinit the completion. > > 4. Enqueue the packet. > > 5. Wait the packet to be received. > > 6. Use the data set by the tagger to complete the mdio operation. > > > > If the completion timeouts or the ack value is not true, the legacy > > mdio way is used. > > > > It has to be considered that in the initial setup mdio is still used and > > mdio is still used until DSA is ready to accept and tag packet. > > > > tag_proto_connect() is used to fill the required handler for the tagger > > to correctly parse and elaborate the special Ethernet mdio packet. > > > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> > > --- > > drivers/net/dsa/qca8k.c | 214 +++++++++++++++++++++++++++++++++++- > > drivers/net/dsa/qca8k.h | 4 + > > include/linux/dsa/tag_qca.h | 10 ++ > > 3 files changed, 226 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c > > index 96a7fbf8700c..5b7508a6e4ba 100644 > > --- a/drivers/net/dsa/qca8k.c > > +++ b/drivers/net/dsa/qca8k.c > > @@ -20,6 +20,7 @@ > > #include <linux/phylink.h> > > #include <linux/gpio/consumer.h> > > #include <linux/etherdevice.h> > > +#include <linux/dsa/tag_qca.h> > > > > #include "qca8k.h" > > > > @@ -170,14 +171,162 @@ qca8k_rmw(struct qca8k_priv *priv, u32 reg, u32 mask, u32 write_val) > > return regmap_update_bits(priv->regmap, reg, mask, write_val); > > } > > > > +static struct sk_buff *qca8k_alloc_mdio_header(struct qca8k_port_tag *header, enum mdio_cmd cmd, > > + u32 reg, u32 *val) > > +{ > > + struct mdio_ethhdr *mdio_ethhdr; > > + struct sk_buff *skb; > > + u16 hdr; > > + > > + skb = dev_alloc_skb(QCA_HDR_MDIO_PKG_LEN); > > + > > + prefetchw(skb->data); > > + > > + skb_reset_mac_header(skb); > > + skb_set_network_header(skb, skb->len); > > + > > + mdio_ethhdr = skb_push(skb, QCA_HDR_MDIO_HEADER_LEN + QCA_HDR_LEN); > > + > > + hdr = FIELD_PREP(QCA_HDR_XMIT_VERSION, QCA_HDR_VERSION); > > + hdr |= QCA_HDR_XMIT_FROM_CPU; > > + hdr |= FIELD_PREP(QCA_HDR_XMIT_DP_BIT, BIT(0)); > > + hdr |= FIELD_PREP(QCA_HDR_XMIT_CONTROL, QCA_HDR_XMIT_TYPE_RW_REG); > > + > > + mdio_ethhdr->seq = FIELD_PREP(QCA_HDR_MDIO_SEQ_NUM, 200); > > + > > + mdio_ethhdr->command = FIELD_PREP(QCA_HDR_MDIO_ADDR, reg); > > + mdio_ethhdr->command |= FIELD_PREP(QCA_HDR_MDIO_LENGTH, 4); > > + mdio_ethhdr->command |= FIELD_PREP(QCA_HDR_MDIO_CMD, cmd); > > + mdio_ethhdr->command |= FIELD_PREP(QCA_HDR_MDIO_CHECK_CODE, MDIO_CHECK_CODE_VAL); > > + > > + if (cmd == MDIO_WRITE) > > + mdio_ethhdr->mdio_data = *val; > > + > > + mdio_ethhdr->hdr = htons(hdr); > > + > > + skb_put_zero(skb, QCA_HDR_MDIO_DATA2_LEN); > > + skb_put_zero(skb, QCA_HDR_MDIO_PADDING_LEN); > > + > > + return skb; > > +} > > + > > +static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val) > > +{ > > + struct qca8k_port_tag *header = priv->header_mdio; > > + struct sk_buff *skb; > > + bool ack; > > + int ret; > > + > > + skb = qca8k_alloc_mdio_header(header, MDIO_READ, reg, 0); > > + skb->dev = dsa_to_port(priv->ds, 0)->master; > > + > > + mutex_lock(&header->mdio_mutex); > > + > > + reinit_completion(&header->rw_done); > > + header->seq = 200; > > + header->ack = false; > > + > > + dev_queue_xmit(skb); > > + > > + ret = wait_for_completion_timeout(&header->rw_done, QCA8K_MDIO_RW_ETHERNET); > > + > > + *val = header->data[0]; > > + ack = header->ack; > > + > > + mutex_unlock(&header->mdio_mutex); > > + > > + if (ret <= 0) > > + return -ETIMEDOUT; > > + > > + if (!ack) > > + return -EINVAL; > > + > > + return 0; > > +} > > + > > +static void qca8k_rw_reg_ack_handler(struct dsa_port *dp, struct sk_buff *skb) > > +{ > > + struct qca8k_port_tag *header = dp->priv; > > + struct mdio_ethhdr *mdio_ethhdr; > > + > > + mdio_ethhdr = (struct mdio_ethhdr *)skb_mac_header(skb); > > + > > + header->data[0] = mdio_ethhdr->mdio_data; > > + > > + /* Get the rest of the 12 byte of data */ > > + memcpy(header->data + 1, skb->data, QCA_HDR_MDIO_DATA2_LEN); > > + > > + /* Make sure the seq match the requested packet */ > > + if (mdio_ethhdr->seq == header->seq) > > + header->ack = true; > > + > > + complete(&header->rw_done); > > +} > > + > > +static int qca8k_write_eth(struct qca8k_priv *priv, u32 reg, u32 val) > > +{ > > + struct qca8k_port_tag *header = priv->header_mdio; > > + struct sk_buff *skb; > > + bool ack; > > + int ret; > > + > > + skb = qca8k_alloc_mdio_header(header, MDIO_WRITE, reg, &val); > > + skb->dev = dsa_to_port(priv->ds, 0)->master; > > + > > + mutex_lock(&header->mdio_mutex); > > + > > + dev_queue_xmit(skb); > > + > > + reinit_completion(&header->rw_done); > > + header->ack = false; > > + header->seq = 200; > > + > > + ret = wait_for_completion_timeout(&header->rw_done, QCA8K_MDIO_RW_ETHERNET); > > + > > + ack = header->ack; > > + > > + mutex_unlock(&header->mdio_mutex); > > + > > + if (ret <= 0) > > + return -ETIMEDOUT; > > + > > + if (!ack) > > + return -EINVAL; > > + > > + return 0; > > +} > > + > > +static int > > +qca8k_regmap_update_bits_eth(struct qca8k_priv *priv, u32 reg, u32 mask, u32 write_val) > > +{ > > + u32 val = 0; > > + int ret; > > + > > + ret = qca8k_read_eth(priv, reg, &val); > > + if (ret) > > + return ret; > > + > > + val &= ~mask; > > + val |= write_val; > > + > > + return qca8k_write_eth(priv, reg, val); > > +} > > + > > static int > > qca8k_regmap_read(void *ctx, uint32_t reg, uint32_t *val) > > { > > struct qca8k_priv *priv = (struct qca8k_priv *)ctx; > > + struct qca8k_port_tag *header_mdio; > > struct mii_bus *bus = priv->bus; > > u16 r1, r2, page; > > int ret; > > > > + header_mdio = priv->header_mdio; > > + > > + if (header_mdio) > > + if (!qca8k_read_eth(priv, reg, val)) > > + return 0; > > + > > qca8k_split_addr(reg, &r1, &r2, &page); > > > > mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED); > > @@ -197,10 +346,17 @@ static int > > qca8k_regmap_write(void *ctx, uint32_t reg, uint32_t val) > > { > > struct qca8k_priv *priv = (struct qca8k_priv *)ctx; > > + struct qca8k_port_tag *header_mdio; > > struct mii_bus *bus = priv->bus; > > u16 r1, r2, page; > > int ret; > > > > + header_mdio = priv->header_mdio; > > + > > + if (header_mdio) > > + if (!qca8k_write_eth(priv, reg, val)) > > + return 0; > > + > > qca8k_split_addr(reg, &r1, &r2, &page); > > > > mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED); > > @@ -220,11 +376,18 @@ static int > > qca8k_regmap_update_bits(void *ctx, uint32_t reg, uint32_t mask, uint32_t write_val) > > { > > struct qca8k_priv *priv = (struct qca8k_priv *)ctx; > > + struct qca8k_port_tag *header_mdio; > > struct mii_bus *bus = priv->bus; > > u16 r1, r2, page; > > u32 val; > > int ret; > > > > + header_mdio = priv->header_mdio; > > + > > + if (header_mdio) > > + if (!qca8k_regmap_update_bits_eth(priv, reg, mask, write_val)) > > + return 0; > > + > > qca8k_split_addr(reg, &r1, &r2, &page); > > > > mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED); > > @@ -1223,8 +1386,13 @@ qca8k_setup(struct dsa_switch *ds) > > * Configure specific switch configuration for ports > > */ > > for (i = 0; i < QCA8K_NUM_PORTS; i++) { > > + struct dsa_port *dp = dsa_to_port(ds, i); > > + > > + /* Set the header_mdio to be accessible by the qca tagger */ > > + dp->priv = priv->header_mdio; > > Why are you setting up the dp->priv pointer from the switch driver? > I though the whole point of qca_tag_connect is to set up that memory by > itself. >
Yes sorry. I forgot to remove this from the my testing buildroot to the clean linux repo. In the testing this wasn't present just to make it clear.
> > + > > /* CPU port gets connected to all user ports of the switch */ > > - if (dsa_is_cpu_port(ds, i)) { > > + if (dsa_port_is_cpu(dp)) { > > ret = qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(i), > > QCA8K_PORT_LOOKUP_MEMBER, dsa_user_ports(ds)); > > if (ret) > > @@ -1232,7 +1400,7 @@ qca8k_setup(struct dsa_switch *ds) > > } > > > > /* Individual user ports get connected to CPU port only */ > > - if (dsa_is_user_port(ds, i)) { > > + if (dsa_port_is_user(dp)) { > > ret = qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(i), > > QCA8K_PORT_LOOKUP_MEMBER, > > BIT(cpu_port)); > > @@ -2382,6 +2550,38 @@ qca8k_port_lag_leave(struct dsa_switch *ds, int port, > > return qca8k_lag_refresh_portmap(ds, port, lag, true); > > } > > > > +static int qca8k_tag_proto_connect(struct dsa_switch *ds, > > + const struct dsa_device_ops *tag_ops) > > +{ > > + struct qca8k_priv *qca8k_priv = ds->priv; > > + struct qca8k_port_tag *priv; > > + struct dsa_port *dp; > > + int i; > > + > > + switch (tag_ops->proto) { > > + case DSA_TAG_PROTO_QCA: > > + for (i = 0; i < QCA8K_NUM_PORTS; i++) { > > + dp = dsa_to_port(ds, i); > > + priv = dp->priv; > > + > > + if (!dsa_port_is_cpu(dp)) > > + continue; > > + > > + mutex_init(&priv->mdio_mutex); > > + init_completion(&priv->rw_done); > > + /* Cache the header mdio */ > > + qca8k_priv->header_mdio = priv; > > + priv->rw_reg_ack_handler = qca8k_rw_reg_ack_handler; > > + } > > + > > + break; > > + default: > > + return -EOPNOTSUPP; > > + } > > + > > + return 0; > > +} > > + > > static const struct dsa_switch_ops qca8k_switch_ops = { > > .get_tag_protocol = qca8k_get_tag_protocol, > > .setup = qca8k_setup, > > @@ -2417,6 +2617,7 @@ static const struct dsa_switch_ops qca8k_switch_ops = { > > .get_phy_flags = qca8k_get_phy_flags, > > .port_lag_join = qca8k_port_lag_join, > > .port_lag_leave = qca8k_port_lag_leave, > > + .tag_proto_connect = qca8k_tag_proto_connect, > > }; > > > > static int qca8k_read_switch_id(struct qca8k_priv *priv) > > @@ -2452,6 +2653,7 @@ static int qca8k_read_switch_id(struct qca8k_priv *priv) > > static int > > qca8k_sw_probe(struct mdio_device *mdiodev) > > { > > + struct qca8k_port_tag *header_mdio; > > struct qca8k_priv *priv; > > int ret; > > > > @@ -2462,6 +2664,13 @@ qca8k_sw_probe(struct mdio_device *mdiodev) > > if (!priv) > > return -ENOMEM; > > > > + header_mdio = devm_kzalloc(&mdiodev->dev, sizeof(*header_mdio), GFP_KERNEL); > > + if (!header_mdio) > > + return -ENOMEM; > > + > > + mutex_init(&header_mdio->mdio_mutex); > > + init_completion(&header_mdio->rw_done); > > + > > priv->bus = mdiodev->bus; > > priv->dev = &mdiodev->dev; > > > > @@ -2501,6 +2710,7 @@ qca8k_sw_probe(struct mdio_device *mdiodev) > > priv->ds->priv = priv; > > priv->ops = qca8k_switch_ops; > > priv->ds->ops = &priv->ops; > > + priv->header_mdio = header_mdio; > > mutex_init(&priv->reg_mutex); > > dev_set_drvdata(&mdiodev->dev, priv); > > > > diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h > > index ab4a417b25a9..25aa1509e0c0 100644 > > --- a/drivers/net/dsa/qca8k.h > > +++ b/drivers/net/dsa/qca8k.h > > @@ -11,6 +11,9 @@ > > #include <linux/delay.h> > > #include <linux/regmap.h> > > #include <linux/gpio.h> > > +#include <linux/dsa/tag_qca.h> > > + > > +#define QCA8K_MDIO_RW_ETHERNET 100 > > > > #define QCA8K_NUM_PORTS 7 > > #define QCA8K_NUM_CPU_PORTS 2 > > @@ -353,6 +356,7 @@ struct qca8k_priv { > > struct dsa_switch_ops ops; > > struct gpio_desc *reset_gpio; > > unsigned int port_mtu[QCA8K_NUM_PORTS]; > > + struct qca8k_port_tag *header_mdio; > > }; > > > > struct qca8k_mib_desc { > > diff --git a/include/linux/dsa/tag_qca.h b/include/linux/dsa/tag_qca.h > > index f403fdab6f29..2b823d971ae9 100644 > > --- a/include/linux/dsa/tag_qca.h > > +++ b/include/linux/dsa/tag_qca.h > > @@ -59,11 +59,21 @@ struct mdio_ethhdr { > > u16 hdr; /* qca hdr */ > > } __packed; > > > > +enum mdio_cmd { > > + MDIO_WRITE = 0x0, > > + MDIO_READ > > +}; > > + > > struct qca8k_port_tag { > > void (*rw_reg_ack_handler)(struct dsa_port *dp, > > struct sk_buff *skb); > > void (*mib_autocast_handler)(struct dsa_port *dp, > > struct sk_buff *skb); > > + struct completion rw_done; > > + struct mutex mdio_mutex; /* Enforce one mdio read/write at time */ > > + bool ack; > > + u32 seq; > > + u32 data[4]; > > None of these structures need to stay in the data structure shared with > the tagger. They can be in qca8k_priv. The tagger should only see the > function pointers. It doesn't care what is done with the packets. > > > };
Ok so the handler should access these data by reaching the qca8k_priv from the dsa port. Correct?
> > > > #endif /* __TAG_QCA_H */ > > -- > > 2.32.0 > >
-- Ansuel
| |