| Message ID | 20210128064112.372883-3-prasanna.vengateshan@microchip.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: microchip: DSA driver support for LAN937x switch | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 1 maintainers not CCed: woojung.huh@microchip.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 20 this patch: 20 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 110 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 20 this patch: 20 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Thu, Jan 28, 2021 at 12:11:06PM +0530, Prasanna Vengateshan wrote: > The Microchip LAN937X switches have a tagging protocol which is > very similar to KSZ tagging. So that the implementation is added to > tag_ksz.c and reused common APIs > > Signed-off-by: Prasanna Vengateshan <prasanna.vengateshan@microchip.com> > --- > include/net/dsa.h | 2 ++ > net/dsa/Kconfig | 4 +-- > net/dsa/tag_ksz.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 78 insertions(+), 2 deletions(-) > > diff --git a/include/net/dsa.h b/include/net/dsa.h > index 2f5435d3d1db..b9bc7a9a8c15 100644 > --- a/include/net/dsa.h > +++ b/include/net/dsa.h > @@ -47,6 +47,7 @@ struct phylink_link_state; > #define DSA_TAG_PROTO_RTL4_A_VALUE 17 > #define DSA_TAG_PROTO_HELLCREEK_VALUE 18 > #define DSA_TAG_PROTO_XRS700X_VALUE 19 > +#define DSA_TAG_PROTO_LAN937X_VALUE 20 > > enum dsa_tag_protocol { > DSA_TAG_PROTO_NONE = DSA_TAG_PROTO_NONE_VALUE, > @@ -69,6 +70,7 @@ enum dsa_tag_protocol { > DSA_TAG_PROTO_RTL4_A = DSA_TAG_PROTO_RTL4_A_VALUE, > DSA_TAG_PROTO_HELLCREEK = DSA_TAG_PROTO_HELLCREEK_VALUE, > DSA_TAG_PROTO_XRS700X = DSA_TAG_PROTO_XRS700X_VALUE, > + DSA_TAG_PROTO_LAN937X = DSA_TAG_PROTO_LAN937X_VALUE, > }; > > struct packet_type; > diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig > index 2d226a5c085f..217fa0f8d13e 100644 > --- a/net/dsa/Kconfig > +++ b/net/dsa/Kconfig > @@ -92,10 +92,10 @@ config NET_DSA_TAG_MTK > Mediatek switches. > > config NET_DSA_TAG_KSZ > - tristate "Tag driver for Microchip 8795/9477/9893 families of switches" > + tristate "Tag driver for Microchip 8795/9477/9893/937x families of switches" You might want to keep these in numerical order. Andrew
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. > +#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? > + 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. > +{ > + /* 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 >
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 > >
diff --git a/include/net/dsa.h b/include/net/dsa.h index 2f5435d3d1db..b9bc7a9a8c15 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -47,6 +47,7 @@ struct phylink_link_state; #define DSA_TAG_PROTO_RTL4_A_VALUE 17 #define DSA_TAG_PROTO_HELLCREEK_VALUE 18 #define DSA_TAG_PROTO_XRS700X_VALUE 19 +#define DSA_TAG_PROTO_LAN937X_VALUE 20 enum dsa_tag_protocol { DSA_TAG_PROTO_NONE = DSA_TAG_PROTO_NONE_VALUE, @@ -69,6 +70,7 @@ enum dsa_tag_protocol { DSA_TAG_PROTO_RTL4_A = DSA_TAG_PROTO_RTL4_A_VALUE, DSA_TAG_PROTO_HELLCREEK = DSA_TAG_PROTO_HELLCREEK_VALUE, DSA_TAG_PROTO_XRS700X = DSA_TAG_PROTO_XRS700X_VALUE, + DSA_TAG_PROTO_LAN937X = DSA_TAG_PROTO_LAN937X_VALUE, }; struct packet_type; diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig index 2d226a5c085f..217fa0f8d13e 100644 --- a/net/dsa/Kconfig +++ b/net/dsa/Kconfig @@ -92,10 +92,10 @@ config NET_DSA_TAG_MTK Mediatek switches. config NET_DSA_TAG_KSZ - tristate "Tag driver for Microchip 8795/9477/9893 families of switches" + tristate "Tag driver for Microchip 8795/9477/9893/937x families of switches" help Say Y if you want to enable support for tagging frames for the - Microchip 8795/9477/9893 families of switches. + Microchip 8795/9477/9893/937x families of switches. config NET_DSA_TAG_RTL4_A tristate "Tag driver for Realtek 4 byte protocol A tags" 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) + */ +#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 */ + 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) +{ + /* 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);
The Microchip LAN937X switches have a tagging protocol which is very similar to KSZ tagging. So that the implementation is added to tag_ksz.c and reused common APIs Signed-off-by: Prasanna Vengateshan <prasanna.vengateshan@microchip.com> --- include/net/dsa.h | 2 ++ net/dsa/Kconfig | 4 +-- net/dsa/tag_ksz.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 2 deletions(-)