From: Prasanna Vengateshan Varadharajan <prasanna.vengateshan@microchip.com>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: <andrew@lunn.ch>, <netdev@vger.kernel.org>, <robh+dt@kernel.org>,
<kuba@kernel.org>, <vivien.didelot@gmail.com>,
<f.fainelli@gmail.com>, <davem@davemloft.net>,
<UNGLinuxDriver@microchip.com>, <Woojung.Huh@microchip.com>,
<linux-kernel@vger.kernel.org>, <devicetree@vger.kernel.org>
Subject: Re: [PATCH net-next 2/8] net: dsa: tag_ksz: add tag handling for Microchip LAN937x
Date: Wed, 10 Feb 2021 17:25:38 +0530 [thread overview]
Message-ID: <fc0549661e10c28d6344be74eb8b29336c1ebdf5.camel@microchip.com> (raw)
In-Reply-To: <20210130022709.ai5kq7w52gpqrb4n@skbuf>
On Sat, 2021-01-30 at 04:27 +0200, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> On Thu, Jan 28, 2021 at 12:11:06PM +0530, Prasanna Vengateshan wrote:
> > diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
> > index 4820dbcedfa2..6fac39c2b7d5 100644
> > --- a/net/dsa/tag_ksz.c
> > +++ b/net/dsa/tag_ksz.c
> > @@ -190,10 +190,84 @@ static const struct dsa_device_ops
> > ksz9893_netdev_ops = {
> > DSA_TAG_DRIVER(ksz9893_netdev_ops);
> > MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9893);
> >
> > +/* For Ingress (Host -> LAN937x), 2 bytes are added before FCS.
> > + * -------------------------------------------------------------
> > --------------
> > + *
> > DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|tag1(1byte)|FCS
> > (4bytes)
> > + * -------------------------------------------------------------
> > --------------
> > + * tag0 : represents tag override, lookup and valid
> > + * tag1 : each bit represents port (eg, 0x01=port1, 0x02=port2,
> > 0x80=port8)
> > + *
> > + * For Egress (LAN937x -> Host), 1 byte is added before FCS.
> > + * -------------------------------------------------------------
> > --------------
> > + * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|FCS(4bytes)
> > + * -------------------------------------------------------------
> > --------------
> > + * tag0 : zero-based value represents port
> > + * (eg, 0x00=port1, 0x02=port3, 0x07=port8)
> > + */
>
> You messed up the comment, right now it's as good as not having it.
> The one-hot port encoding is for xmit. The zero-based encoding is for
> rcv, not the other way around.
I understand the problem is with the term Egress & Ingress w.r.to
LAN937x. I will make sure that the comment is added w.r.to xmit() &
rcv().
>
> > +#define LAN937X_INGRESS_TAG_LEN 2
> > +
> > +#define LAN937X_TAIL_TAG_OVERRIDE BIT(11)
> > +#define LAN937X_TAIL_TAG_LOOKUP BIT(12)
> > +#define LAN937X_TAIL_TAG_VALID BIT(13)
> > +#define LAN937X_TAIL_TAG_PORT_MASK 7
> > +
> > +static struct sk_buff *lan937x_xmit(struct sk_buff *skb,
> > + struct net_device *dev)
> > +{
> > + struct dsa_port *dp = dsa_slave_to_port(dev);
> > + __be16 *tag;
> > + u8 *addr;
> > + u16 val;
> > +
> > + /* Tag encoding */
>
> Do we really need this comment and the one with "Tag decoding" from
> rcv?
Okay, will correct it.
>
> > + tag = skb_put(skb, LAN937X_INGRESS_TAG_LEN);
> > + addr = skb_mac_header(skb);
> > +
> > + val = BIT(dp->index);
> > +
> > + if (is_link_local_ether_addr(addr))
> > + val |= LAN937X_TAIL_TAG_OVERRIDE;
> > +
> > + /* Tail tag valid bit - This bit should always be set by the
> > CPU*/
> > + val |= LAN937X_TAIL_TAG_VALID;
> > +
> > + *tag = cpu_to_be16(val);
> > +
> > + return skb;
> > +}
> > +
> > +static struct sk_buff *lan937x_rcv(struct sk_buff *skb, struct
> > net_device *dev,
> > + struct packet_type *pt)
>
> You can reuse ksz9477_rcv.
Sure, will reuse ksz9477_rcv.
>
> > +{
> > + /* Tag decoding */
> > + u8 *tag = skb_tail_pointer(skb) - KSZ_EGRESS_TAG_LEN;
> > + unsigned int port = tag[0] & LAN937X_TAIL_TAG_PORT_MASK;
> > + unsigned int len = KSZ_EGRESS_TAG_LEN;
> > +
> > + /* Extra 4-bytes PTP timestamp */
> > + if (tag[0] & KSZ9477_PTP_TAG_INDICATION)
> > + len += KSZ9477_PTP_TAG_LEN;
> > +
> > + return ksz_common_rcv(skb, dev, port, len);
> > +}
> > +
> > +static const struct dsa_device_ops lan937x_netdev_ops = {
> > + .name = "lan937x",
> > + .proto = DSA_TAG_PROTO_LAN937X,
> > + .xmit = lan937x_xmit,
> > + .rcv = lan937x_rcv,
> > + .overhead = LAN937X_INGRESS_TAG_LEN,
> > + .tail_tag = true,
> > +};
> > +
> > +DSA_TAG_DRIVER(lan937x_netdev_ops);
> > +MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_LAN937X);
> > +
> > static struct dsa_tag_driver *dsa_tag_driver_array[] = {
> > &DSA_TAG_DRIVER_NAME(ksz8795_netdev_ops),
> > &DSA_TAG_DRIVER_NAME(ksz9477_netdev_ops),
> > &DSA_TAG_DRIVER_NAME(ksz9893_netdev_ops),
> > + &DSA_TAG_DRIVER_NAME(lan937x_netdev_ops),
> > };
> >
> > module_dsa_tag_drivers(dsa_tag_driver_array);
> > --
> > 2.25.1
> >
next prev parent reply other threads:[~2021-02-10 11:59 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-28 6:41 [PATCH net-next 0/8] net: dsa: microchip: DSA driver support for LAN937x switch Prasanna Vengateshan
2021-01-28 6:41 ` [PATCH net-next 1/8] dt-bindings: net: dsa: dt bindings for microchip lan937x Prasanna Vengateshan
2021-01-30 2:02 ` Vladimir Oltean
2021-02-10 11:46 ` Prasanna Vengateshan Varadharajan
2021-02-10 18:57 ` Andrew Lunn
2021-02-09 19:35 ` Rob Herring
2021-01-28 6:41 ` [PATCH net-next 2/8] net: dsa: tag_ksz: add tag handling for Microchip LAN937x Prasanna Vengateshan
2021-01-28 18:03 ` Andrew Lunn
2021-01-30 2:27 ` Vladimir Oltean
2021-02-10 11:55 ` Prasanna Vengateshan Varadharajan [this message]
2021-01-28 6:41 ` [PATCH net-next 3/8] net: dsa: microchip: add DSA support for microchip lan937x Prasanna Vengateshan
2021-01-29 1:07 ` Andrew Lunn
2021-02-05 12:48 ` Prasanna Vengateshan Varadharajan
2021-02-05 13:27 ` Andrew Lunn
2021-03-15 6:25 ` Prasanna Vengateshan Varadharajan
2021-01-28 6:41 ` [PATCH net-next 4/8] net: dsa: microchip: add support for phylink management Prasanna Vengateshan
2021-01-29 1:12 ` Andrew Lunn
2021-01-28 6:41 ` [PATCH net-next 5/8] net: dsa: microchip: add support for ethtool port counters Prasanna Vengateshan
2021-01-28 6:41 ` [PATCH net-next 6/8] net: dsa: microchip: add support for port mirror operations Prasanna Vengateshan
2021-01-28 6:41 ` [PATCH net-next 7/8] net: dsa: microchip: add support for fdb and mdb management Prasanna Vengateshan
2021-01-28 6:41 ` [PATCH net-next 8/8] net: dsa: microchip: add support for vlan operations Prasanna Vengateshan
2021-01-28 17:55 ` [PATCH net-next 0/8] net: dsa: microchip: DSA driver support for LAN937x switch Florian Fainelli
2021-01-30 2:09 ` Vladimir Oltean
2021-02-02 1:25 ` Woojung.Huh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fc0549661e10c28d6344be74eb8b29336c1ebdf5.camel@microchip.com \
--to=prasanna.vengateshan@microchip.com \
--cc=UNGLinuxDriver@microchip.com \
--cc=Woojung.Huh@microchip.com \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=f.fainelli@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=olteanv@gmail.com \
--cc=robh+dt@kernel.org \
--cc=vivien.didelot@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).