[PATCH net-next] net: dsa: mt7530: enable jumbo frame

[PATCH net-next] net: dsa: mt7530: enable jumbo frame

[DENG Qingfang]

Enable jumbo frame for MT7530
It allows frames up to 15 KB (including FCS)

Signed-off-by: DENG Qingfang <dqfext@gmail.com>
---
 drivers/net/dsa/mt7530.c | 16 ++++++++++++++++
 drivers/net/dsa/mt7530.h |  7 +++++++
 2 files changed, 23 insertions(+)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 2d0d91db0ddb..ed080ded34a7 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1284,6 +1284,19 @@ mtk_get_tag_protocol(struct dsa_switch *ds, int port,
 	}
 }
 
+static void
+mt7530_set_jumbo(struct mt7530_priv *priv, u8 kilobytes)
+{
+	if (kilobytes > 15)
+		kilobytes = 15;
+
+	if (kilobytes > 1)
+		mt7530_rmw(priv, MT7530_GMACCR, MAX_RX_JUMBO_MASK | MAX_RX_PKT_LEN_MASK,
+			   MAX_RX_JUMBO_KB(kilobytes) | MAX_RX_PKT_LEN_JMB);
+	else
+		mt7530_rmw(priv, MT7530_GMACCR, MAX_RX_PKT_LEN_MASK, MAX_RX_PKT_LEN_1536);
+}
+
 static int
 mt7530_setup(struct dsa_switch *ds)
 {
@@ -1428,6 +1441,9 @@ mt7530_setup(struct dsa_switch *ds)
 	if (ret < 0)
 		return ret;
 
+	/* Enable jumbo frame up to 15 KB */
+	mt7530_set_jumbo(priv, 15);
+
 	return 0;
 }
 
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index ef9b52f3152b..62f5ea64670d 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -223,6 +223,13 @@ enum mt7530_vlan_port_attr {
 #define  PMSR_DPX			BIT(1)
 #define  PMSR_LINK			BIT(0)
 
+#define MT7530_GMACCR			0x30e0
+#define  MAX_RX_JUMBO_KB(x)		((x) << 2)
+#define  MAX_RX_JUMBO_MASK		MAX_RX_JUMBO_KB(0xf)
+#define  MAX_RX_PKT_LEN_MASK		0x3
+#define  MAX_RX_PKT_LEN_1536		0x1
+#define  MAX_RX_PKT_LEN_JMB		MAX_RX_PKT_LEN_MASK
+
 /* Register for MIB */
 #define MT7530_PORT_MIB_COUNTER(x)	(0x4000 + (x) * 0x100)
 #define MT7530_MIB_CCR			0x4fe0
-- 
2.26.0

Re: [PATCH net-next] net: dsa: mt7530: enable jumbo frame

[David Miller]

From: DENG Qingfang <dqfext@gmail.com>
Date: Thu,  9 Apr 2020 23:54:09 +0800

> +static void
> +mt7530_set_jumbo(struct mt7530_priv *priv, u8 kilobytes)
> +{
> +	if (kilobytes > 15)
> +		kilobytes = 15;
 ...
> +	/* Enable jumbo frame up to 15 KB */
> +	mt7530_set_jumbo(priv, 15);

You've made the test quite pointless, honestly.

Re: [PATCH net-next] net: dsa: mt7530: enable jumbo frame

[DENG Qingfang]

So, since nothing else uses the mt7530_set_jumbo function, should I
remove the function and just add a single rmw to mt7530_setup?

On Fri, Apr 10, 2020 at 1:20 AM David Miller <davem@davemloft.net> wrote:
>
> From: DENG Qingfang <dqfext@gmail.com>
> Date: Thu,  9 Apr 2020 23:54:09 +0800
>
> > +static void
> > +mt7530_set_jumbo(struct mt7530_priv *priv, u8 kilobytes)
> > +{
> > +     if (kilobytes > 15)
> > +             kilobytes = 15;
>  ...
> > +     /* Enable jumbo frame up to 15 KB */
> > +     mt7530_set_jumbo(priv, 15);
>
> You've made the test quite pointless, honestly.

Re: [PATCH net-next] net: dsa: mt7530: enable jumbo frame

[Florian Fainelli]

On 4/9/2020 7:19 PM, DENG Qingfang wrote:
> So, since nothing else uses the mt7530_set_jumbo function, should I
> remove the function and just add a single rmw to mt7530_setup?

(please do not top-post on netdev)

There is a proper way to support the MTU configuration for DSA switch
drivers which is:

        /*
         * MTU change functionality. Switches can also adjust their MRU
through
         * this method. By MTU, one understands the SDU (L2 payload) length.
         * If the switch needs to account for the DSA tag on the CPU
port, this
         * method needs to to do so privately.
         */
        int     (*port_change_mtu)(struct dsa_switch *ds, int port,
                                   int new_mtu);
        int     (*port_max_mtu)(struct dsa_switch *ds, int port);

> 
> On Fri, Apr 10, 2020 at 1:20 AM David Miller <davem@davemloft.net> wrote:
>>
>> From: DENG Qingfang <dqfext@gmail.com>
>> Date: Thu,  9 Apr 2020 23:54:09 +0800
>>
>>> +static void
>>> +mt7530_set_jumbo(struct mt7530_priv *priv, u8 kilobytes)
>>> +{
>>> +     if (kilobytes > 15)
>>> +             kilobytes = 15;
>>  ...
>>> +     /* Enable jumbo frame up to 15 KB */
>>> +     mt7530_set_jumbo(priv, 15);
>>
>> You've made the test quite pointless, honestly.

-- 
Florian

Re: [PATCH net-next] net: dsa: mt7530: enable jumbo frame

[DENG Qingfang]

On Fri, Apr 10, 2020 at 10:27 AM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>
>
> On 4/9/2020 7:19 PM, DENG Qingfang wrote:
> > So, since nothing else uses the mt7530_set_jumbo function, should I
> > remove the function and just add a single rmw to mt7530_setup?
>
> (please do not top-post on netdev)
>
> There is a proper way to support the MTU configuration for DSA switch
> drivers which is:
>
>         /*
>          * MTU change functionality. Switches can also adjust their MRU
> through
>          * this method. By MTU, one understands the SDU (L2 payload) length.
>          * If the switch needs to account for the DSA tag on the CPU
> port, this
>          * method needs to to do so privately.
>          */
>         int     (*port_change_mtu)(struct dsa_switch *ds, int port,
>                                    int new_mtu);
>         int     (*port_max_mtu)(struct dsa_switch *ds, int port);

MT7530 does not support configuring jumbo frame per-port
The register affects globally

>
> >
> > On Fri, Apr 10, 2020 at 1:20 AM David Miller <davem@davemloft.net> wrote:
> >>
> >> From: DENG Qingfang <dqfext@gmail.com>
> >> Date: Thu,  9 Apr 2020 23:54:09 +0800
> >>
> >>> +static void
> >>> +mt7530_set_jumbo(struct mt7530_priv *priv, u8 kilobytes)
> >>> +{
> >>> +     if (kilobytes > 15)
> >>> +             kilobytes = 15;
> >>  ...
> >>> +     /* Enable jumbo frame up to 15 KB */
> >>> +     mt7530_set_jumbo(priv, 15);
> >>
> >> You've made the test quite pointless, honestly.
>
> --
> Florian

Re: [PATCH net-next] net: dsa: mt7530: enable jumbo frame

[Vladimir Oltean]

Hi Qingfang,

On Fri, 10 Apr 2020 at 05:51, DENG Qingfang <dqfext@gmail.com> wrote:
>
> On Fri, Apr 10, 2020 at 10:27 AM Florian Fainelli <f.fainelli@gmail.com> wrote:
> >
> >
> >
> > On 4/9/2020 7:19 PM, DENG Qingfang wrote:
> > > So, since nothing else uses the mt7530_set_jumbo function, should I
> > > remove the function and just add a single rmw to mt7530_setup?
> >
> > (please do not top-post on netdev)
> >
> > There is a proper way to support the MTU configuration for DSA switch
> > drivers which is:
> >
> >         /*
> >          * MTU change functionality. Switches can also adjust their MRU
> > through
> >          * this method. By MTU, one understands the SDU (L2 payload) length.
> >          * If the switch needs to account for the DSA tag on the CPU
> > port, this
> >          * method needs to to do so privately.
> >          */
> >         int     (*port_change_mtu)(struct dsa_switch *ds, int port,
> >                                    int new_mtu);
> >         int     (*port_max_mtu)(struct dsa_switch *ds, int port);
>
> MT7530 does not support configuring jumbo frame per-port
> The register affects globally
>
> >
> > --
> > Florian

This is a bit more tricky, but I think you can still deal with it
using the port_change_mtu functionality. Basically it is only a
problem when the other ports are standalone - otherwise the
dsa_bridge_mtu_normalization function should kick in.
So if you implement port_change_mtu, you should do something along the lines of:

for (i = 0; i < MT7530_NUM_PORTS; i++) {
    struct net_device *slave;

    if (!dsa_is_user_port(ds, i))
        continue;

    slave = ds->ports[i].slave;

    slave->mtu = new_mtu;
}

to update the MTU known by the stack for all net devices.

Hope this helps,
-Vladimir

Re: [PATCH net-next] net: dsa: mt7530: enable jumbo frame

[DENG Qingfang]

On Fri, Apr 10, 2020 at 6:46 PM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> Hi Qingfang,
>
> On Fri, 10 Apr 2020 at 05:51, DENG Qingfang <dqfext@gmail.com> wrote:
> >
> > On Fri, Apr 10, 2020 at 10:27 AM Florian Fainelli <f.fainelli@gmail.com> wrote:
> > >
> > >
> > >
> > > On 4/9/2020 7:19 PM, DENG Qingfang wrote:
> > > > So, since nothing else uses the mt7530_set_jumbo function, should I
> > > > remove the function and just add a single rmw to mt7530_setup?
> > >
> > > (please do not top-post on netdev)
> > >
> > > There is a proper way to support the MTU configuration for DSA switch
> > > drivers which is:
> > >
> > >         /*
> > >          * MTU change functionality. Switches can also adjust their MRU
> > > through
> > >          * this method. By MTU, one understands the SDU (L2 payload) length.
> > >          * If the switch needs to account for the DSA tag on the CPU
> > > port, this
> > >          * method needs to to do so privately.
> > >          */
> > >         int     (*port_change_mtu)(struct dsa_switch *ds, int port,
> > >                                    int new_mtu);
> > >         int     (*port_max_mtu)(struct dsa_switch *ds, int port);
> >
> > MT7530 does not support configuring jumbo frame per-port
> > The register affects globally
> >
> > >
> > > --
> > > Florian
>
> This is a bit more tricky, but I think you can still deal with it
> using the port_change_mtu functionality. Basically it is only a
> problem when the other ports are standalone - otherwise the
> dsa_bridge_mtu_normalization function should kick in.
> So if you implement port_change_mtu, you should do something along the lines of:
>
> for (i = 0; i < MT7530_NUM_PORTS; i++) {
>     struct net_device *slave;
>
>     if (!dsa_is_user_port(ds, i))
>         continue;
>
>     slave = ds->ports[i].slave;
>
>     slave->mtu = new_mtu;
> }
>
> to update the MTU known by the stack for all net devices.
Should we warn users that all ports will be affected?
>
> Hope this helps,
> -Vladimir

Re: [PATCH net-next] net: dsa: mt7530: enable jumbo frame

[Vladimir Oltean]

On Tue, 14 Apr 2020 at 06:03, DENG Qingfang <dqfext@gmail.com> wrote:
>
> On Fri, Apr 10, 2020 at 6:46 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> >
> > Hi Qingfang,
> >
> > On Fri, 10 Apr 2020 at 05:51, DENG Qingfang <dqfext@gmail.com> wrote:
> > >
> > > On Fri, Apr 10, 2020 at 10:27 AM Florian Fainelli <f.fainelli@gmail.com> wrote:
> > > >
> > > >
> > > >
> > > > On 4/9/2020 7:19 PM, DENG Qingfang wrote:
> > > > > So, since nothing else uses the mt7530_set_jumbo function, should I
> > > > > remove the function and just add a single rmw to mt7530_setup?
> > > >
> > > > (please do not top-post on netdev)
> > > >
> > > > There is a proper way to support the MTU configuration for DSA switch
> > > > drivers which is:
> > > >
> > > >         /*
> > > >          * MTU change functionality. Switches can also adjust their MRU
> > > > through
> > > >          * this method. By MTU, one understands the SDU (L2 payload) length.
> > > >          * If the switch needs to account for the DSA tag on the CPU
> > > > port, this
> > > >          * method needs to to do so privately.
> > > >          */
> > > >         int     (*port_change_mtu)(struct dsa_switch *ds, int port,
> > > >                                    int new_mtu);
> > > >         int     (*port_max_mtu)(struct dsa_switch *ds, int port);
> > >
> > > MT7530 does not support configuring jumbo frame per-port
> > > The register affects globally
> > >
> > > >
> > > > --
> > > > Florian
> >
> > This is a bit more tricky, but I think you can still deal with it
> > using the port_change_mtu functionality. Basically it is only a
> > problem when the other ports are standalone - otherwise the
> > dsa_bridge_mtu_normalization function should kick in.
> > So if you implement port_change_mtu, you should do something along the lines of:
> >
> > for (i = 0; i < MT7530_NUM_PORTS; i++) {
> >     struct net_device *slave;
> >
> >     if (!dsa_is_user_port(ds, i))
> >         continue;
> >
> >     slave = ds->ports[i].slave;
> >
> >     slave->mtu = new_mtu;
> > }
> >
> > to update the MTU known by the stack for all net devices.
> Should we warn users that all ports will be affected?
> >
> > Hope this helps,
> > -Vladimir

Unless I'm missing something, all ports are affected anyway, so
changing the MTU _is_ informing users that all switch ports are
affected.

Thanks,
-Vladimir