[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <7e81c093-c578-46ad-b00b-a45da074939a@redhat.com>
Date: Thu, 15 Jan 2026 15:57:09 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Daniel Golle <daniel@...rotopia.org>, Andrew Lunn <andrew@...n.ch>,
Vladimir Oltean <olteanv@...il.com>, "David S. Miller"
<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Heiner Kallweit <hkallweit1@...il.com>,
Russell King <linux@...linux.org.uk>, Simon Horman <horms@...nel.org>,
netdev@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Cc: Frank Wunderlich <frankwu@....de>, Chad Monroe <chad@...roe.io>,
Cezary Wilmanski <cezary.wilmanski@...ran.com>,
Avinash Jayaraman <ajayaraman@...linear.com>, Bing tao Xu
<bxu@...linear.com>, Liang Xu <lxu@...linear.com>,
Juraj Povazanec <jpovazanec@...linear.com>,
"Fanni (Fang-Yi) Chan" <fchan@...linear.com>,
"Benny (Ying-Tsan) Weng" <yweng@...linear.com>,
"Livia M. Rosu" <lrosu@...linear.com>, John Crispin <john@...ozen.org>
Subject: Re: [PATCH net-next v5 2/4] net: dsa: add tag format for MxL862xx
switches
On 1/12/26 2:51 PM, Daniel Golle wrote:
> Add proprietary special tag format for the MaxLinear MXL862xx family of
> switches. While using the same Ethertype as MaxLinear's GSW1xx switches,
> the actual tag format differs significantly, hence we need a dedicated
> tag driver for that.
>
> Signed-off-by: Daniel Golle <daniel@...rotopia.org>
> ---
> v5:
> * remove unused macro definitions
>
> RFC v4:
> * describe fields and variables with comments
> * sub-interface is only 5 bits
> * harmonize Kconfig symbol name
> * maintain alphabetic order in Kconfig
> * fix typo s/beginnig/beginning/
> * fix typo s/swtiches/switches/
> * arrange local variables in reverse xmas tree order
>
> RFC v3: no changes
> RFC v2: make sure all tag fields are initialized
>
> MAINTAINERS | 1 +
> include/net/dsa.h | 2 +
> net/dsa/Kconfig | 7 +++
> net/dsa/Makefile | 1 +
> net/dsa/tag_mxl862xx.c | 116 +++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 127 insertions(+)
> create mode 100644 net/dsa/tag_mxl862xx.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 162859534a77c..686591074b34d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15619,6 +15619,7 @@ M: Daniel Golle <daniel@...rotopia.org>
> L: netdev@...r.kernel.org
> S: Maintained
> F: Documentation/devicetree/bindings/net/dsa/maxlinear,mxl862xx.yaml
> +F: net/dsa/tag_mxl862xx.c
>
> MCAN DEVICE DRIVER
> M: Markus Schneider-Pargmann <msp@...libre.com>
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 6b2b5ed64ea4c..1e33242b6d94d 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -57,6 +57,7 @@ struct tc_action;
> #define DSA_TAG_PROTO_BRCM_LEGACY_FCS_VALUE 29
> #define DSA_TAG_PROTO_YT921X_VALUE 30
> #define DSA_TAG_PROTO_MXL_GSW1XX_VALUE 31
> +#define DSA_TAG_PROTO_MXL862_VALUE 32
>
> enum dsa_tag_protocol {
> DSA_TAG_PROTO_NONE = DSA_TAG_PROTO_NONE_VALUE,
> @@ -91,6 +92,7 @@ enum dsa_tag_protocol {
> DSA_TAG_PROTO_VSC73XX_8021Q = DSA_TAG_PROTO_VSC73XX_8021Q_VALUE,
> DSA_TAG_PROTO_YT921X = DSA_TAG_PROTO_YT921X_VALUE,
> DSA_TAG_PROTO_MXL_GSW1XX = DSA_TAG_PROTO_MXL_GSW1XX_VALUE,
> + DSA_TAG_PROTO_MXL862 = DSA_TAG_PROTO_MXL862_VALUE,
> };
>
> struct dsa_switch;
> diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
> index f86b30742122f..efc95759a10e1 100644
> --- a/net/dsa/Kconfig
> +++ b/net/dsa/Kconfig
> @@ -104,6 +104,13 @@ config NET_DSA_TAG_MTK
> Say Y or M if you want to enable support for tagging frames for
> Mediatek switches.
>
> +config NET_DSA_TAG_MXL_862XX
> + tristate "Tag driver for MxL862xx switches"
> + help
> + Say Y or M if you want to enable support for tagging frames for the
> + Maxlinear MxL86252 and MxL86282 switches using their native 8-byte
> + tagging protocol.
> +
> config NET_DSA_TAG_MXL_GSW1XX
> tristate "Tag driver for MaxLinear GSW1xx switches"
> help
> diff --git a/net/dsa/Makefile b/net/dsa/Makefile
> index 42d173f5a7013..bf7247759a64a 100644
> --- a/net/dsa/Makefile
> +++ b/net/dsa/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_NET_DSA_TAG_HELLCREEK) += tag_hellcreek.o
> obj-$(CONFIG_NET_DSA_TAG_KSZ) += tag_ksz.o
> obj-$(CONFIG_NET_DSA_TAG_LAN9303) += tag_lan9303.o
> obj-$(CONFIG_NET_DSA_TAG_MTK) += tag_mtk.o
> +obj-$(CONFIG_NET_DSA_TAG_MXL_862XX) += tag_mxl862xx.o
> obj-$(CONFIG_NET_DSA_TAG_MXL_GSW1XX) += tag_mxl-gsw1xx.o
> obj-$(CONFIG_NET_DSA_TAG_NONE) += tag_none.o
> obj-$(CONFIG_NET_DSA_TAG_OCELOT) += tag_ocelot.o
> diff --git a/net/dsa/tag_mxl862xx.c b/net/dsa/tag_mxl862xx.c
> new file mode 100644
> index 0000000000000..c4d0ea2eb48f9
> --- /dev/null
> +++ b/net/dsa/tag_mxl862xx.c
> @@ -0,0 +1,116 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * DSA Special Tag for MaxLinear 862xx switch chips
> + *
> + * Copyright (C) 2025 Daniel Golle <daniel@...rotopia.org>
> + * Copyright (C) 2024 MaxLinear Inc.
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/etherdevice.h>
> +#include <linux/skbuff.h>
> +#include <net/dsa.h>
> +#include "tag.h"
> +
> +#define MXL862_NAME "mxl862xx"
> +
> +#define MXL862_HEADER_LEN 8
> +
> +/* Word 0 -> EtherType */
> +
> +/* Word 2 */
> +#define MXL862_SUBIF_ID GENMASK(4, 0)
> +
> +/* Word 3 */
> +#define MXL862_IGP_EGP GENMASK(3, 0)
> +
> +static struct sk_buff *mxl862_tag_xmit(struct sk_buff *skb,
> + struct net_device *dev)
> +{
> + unsigned int cpu_port, usr_port, sub_interface;
> + struct dsa_port *dp = dsa_user_to_port(dev);
> + struct dsa_port *cpu_dp = dp->cpu_dp;
> + __be16 *mxl862_tag;
> +
> + /* switch firmware expects ports to be counted starting from 1 */
> + cpu_port = cpu_dp->index + 1;
> + usr_port = dp->index + 1;
> +
> + /* target port sub-interface ID relative to the CPU port */
> + sub_interface = usr_port + 16 - cpu_port;
> +
> + if (!skb)
> + return skb;
I think skb can't be NULL here?!? Otherwise it would be better to move
the check at the beginning of the function and return NULL directly.
> +
> + /* provide additional space 'MXL862_HEADER_LEN' bytes */
> + skb_push(skb, MXL862_HEADER_LEN);
> +
> + /* shift MAC address to the beginning of the enlarged buffer,
> + * releasing the space required for DSA tag (between MAC address and
> + * Ethertype)
> + */
> + dsa_alloc_etype_header(skb, MXL862_HEADER_LEN);
> +
> + /* special tag ingress */
> + mxl862_tag = dsa_etype_header_pos_tx(skb);
> + mxl862_tag[0] = htons(ETH_P_MXLGSW);
> + mxl862_tag[1] = 0;
> + mxl862_tag[2] = htons(FIELD_PREP(MXL862_SUBIF_ID, sub_interface));
> + mxl862_tag[3] = htons(FIELD_PREP(MXL862_IGP_EGP, cpu_port));
> +
> + return skb;
> +}
> +
> +static struct sk_buff *mxl862_tag_rcv(struct sk_buff *skb,
> + struct net_device *dev)
> +{
> + __be16 *mxl862_tag;
> + int port;
> +
> + if (unlikely(!pskb_may_pull(skb, MXL862_HEADER_LEN))) {
> + dev_warn_ratelimited(&dev->dev, "Cannot pull SKB, packet dropped\n");
> + return NULL;
> + }
> +
> + mxl862_tag = dsa_etype_header_pos_rx(skb);
> +
> + if (unlikely(mxl862_tag[0] != htons(ETH_P_MXLGSW))) {
> + dev_warn_ratelimited(&dev->dev, "Invalid special tag marker, packet dropped\n");
> + dev_warn_ratelimited(&dev->dev, "Rx Packet Tag: %8ph\n",
> + mxl862_tag);
Why breaking the message in 2 strings? a single dev_warn_ratelimited()
would be more readable and will generate smaller code. Similar thing below.
/P
Powered by blists - more mailing lists