* [PATCH] net: dsa: mt7530: support setting MTU
@ 2020-10-28 18:12 DENG Qingfang
2020-10-28 18:31 ` Vladimir Oltean
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: DENG Qingfang @ 2020-10-28 18:12 UTC (permalink / raw)
To: netdev
Cc: Sean Wang, Landen Chao, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, David S . Miller,
René van Dorst
MT7530/7531 has a global RX packet length register, which can be used
to set MTU.
Signed-off-by: DENG Qingfang <dqfext@gmail.com>
---
drivers/net/dsa/mt7530.c | 36 ++++++++++++++++++++++++++++++++++++
drivers/net/dsa/mt7530.h | 12 ++++++++++++
2 files changed, 48 insertions(+)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index de7692b763d8..7764c66a47c9 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -1021,6 +1021,40 @@ mt7530_port_disable(struct dsa_switch *ds, int port)
mutex_unlock(&priv->reg_mutex);
}
+static int
+mt7530_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
+{
+ struct mt7530_priv *priv = ds->priv;
+ int length;
+
+ /* When a new MTU is set, DSA always set the CPU port's MTU to the largest MTU
+ * of the slave ports. Because the switch only has a global RX length register,
+ * only allowing CPU port here is enough.
+ */
+ if (!dsa_is_cpu_port(ds, port))
+ return 0;
+
+ /* RX length also includes Ethernet header, MTK tag, and FCS length */
+ length = new_mtu + ETH_HLEN + MTK_HDR_LEN + ETH_FCS_LEN;
+ if (length <= 1522)
+ mt7530_rmw(priv, MT7530_GMACCR, MAX_RX_PKT_LEN_MASK, MAX_RX_PKT_LEN_1522);
+ else if (length <= 1536)
+ mt7530_rmw(priv, MT7530_GMACCR, MAX_RX_PKT_LEN_MASK, MAX_RX_PKT_LEN_1536);
+ else if (length <= 1552)
+ mt7530_rmw(priv, MT7530_GMACCR, MAX_RX_PKT_LEN_MASK, MAX_RX_PKT_LEN_1552);
+ else
+ mt7530_rmw(priv, MT7530_GMACCR, MAX_RX_JUMBO_MASK | MAX_RX_PKT_LEN_MASK,
+ MAX_RX_JUMBO(DIV_ROUND_UP(length, 1024)) | MAX_RX_PKT_LEN_JUMBO);
+
+ return 0;
+}
+
+static int
+mt7530_port_max_mtu(struct dsa_switch *ds, int port)
+{
+ return MT7530_MAX_MTU;
+}
+
static void
mt7530_stp_state_set(struct dsa_switch *ds, int port, u8 state)
{
@@ -2519,6 +2553,8 @@ static const struct dsa_switch_ops mt7530_switch_ops = {
.get_sset_count = mt7530_get_sset_count,
.port_enable = mt7530_port_enable,
.port_disable = mt7530_port_disable,
+ .port_change_mtu = mt7530_port_change_mtu,
+ .port_max_mtu = mt7530_port_max_mtu,
.port_stp_state_set = mt7530_stp_state_set,
.port_bridge_join = mt7530_port_bridge_join,
.port_bridge_leave = mt7530_port_bridge_leave,
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 9278a8e3d04e..77a6222d2635 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -11,6 +11,9 @@
#define MT7530_NUM_FDB_RECORDS 2048
#define MT7530_ALL_MEMBERS 0xff
+#define MTK_HDR_LEN 4
+#define MT7530_MAX_MTU (15 * 1024 - ETH_HLEN - ETH_FCS_LEN - MTK_HDR_LEN)
+
enum mt753x_id {
ID_MT7530 = 0,
ID_MT7621 = 1,
@@ -289,6 +292,15 @@ enum mt7530_vlan_port_attr {
#define MT7531_DBG_CNT(x) (0x3018 + (x) * 0x100)
#define MT7531_DIS_CLR BIT(31)
+#define MT7530_GMACCR 0x30e0
+#define MAX_RX_JUMBO(x) ((x) << 2)
+#define MAX_RX_JUMBO_MASK GENMASK(5, 2)
+#define MAX_RX_PKT_LEN_MASK GENMASK(1, 0)
+#define MAX_RX_PKT_LEN_1522 0x0
+#define MAX_RX_PKT_LEN_1536 0x1
+#define MAX_RX_PKT_LEN_1552 0x2
+#define MAX_RX_PKT_LEN_JUMBO 0x3
+
/* Register for MIB */
#define MT7530_PORT_MIB_COUNTER(x) (0x4000 + (x) * 0x100)
#define MT7530_MIB_CCR 0x4fe0
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] net: dsa: mt7530: support setting MTU
2020-10-28 18:12 [PATCH] net: dsa: mt7530: support setting MTU DENG Qingfang
@ 2020-10-28 18:31 ` Vladimir Oltean
2020-10-29 3:32 ` DENG Qingfang
2020-10-28 18:46 ` Florian Fainelli
2020-10-29 22:42 ` Jakub Kicinski
2 siblings, 1 reply; 8+ messages in thread
From: Vladimir Oltean @ 2020-10-28 18:31 UTC (permalink / raw)
To: DENG Qingfang, Jakub Kicinski
Cc: netdev, Sean Wang, Landen Chao, Andrew Lunn, Vivien Didelot,
Florian Fainelli, David S . Miller, René van Dorst,
Linus Walleij
On Thu, Oct 29, 2020 at 02:12:21AM +0800, DENG Qingfang wrote:
> MT7530/7531 has a global RX packet length register, which can be used
> to set MTU.
>
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>
> ---
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
Also, please format your patches with --subject-prefix="PATCH net-next"
in the future. Jakub installed some patchwork scripts that "guess" the
tree based on the commit message, but maybe sometimes they might fail:
https://patchwork.ozlabs.org/project/netdev/patch/e5fdcddeda21884a21162e441d1e8a04994f2825.1603837679.git.pavana.sharma@digi.com/
> drivers/net/dsa/mt7530.c | 36 ++++++++++++++++++++++++++++++++++++
> drivers/net/dsa/mt7530.h | 12 ++++++++++++
> 2 files changed, 48 insertions(+)
>
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index de7692b763d8..7764c66a47c9 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -1021,6 +1021,40 @@ mt7530_port_disable(struct dsa_switch *ds, int port)
> mutex_unlock(&priv->reg_mutex);
> }
>
> +static int
> +mt7530_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
> +{
> + struct mt7530_priv *priv = ds->priv;
> + int length;
> +
> + /* When a new MTU is set, DSA always set the CPU port's MTU to the largest MTU
> + * of the slave ports. Because the switch only has a global RX length register,
> + * only allowing CPU port here is enough.
> + */
Good point, please tell that to Linus (cc) - I'm talking about
e0b2e0d8e669 ("net: dsa: rtl8366rb: Roof MTU for switch"),
> + if (!dsa_is_cpu_port(ds, port))
> + return 0;
> +
> + /* RX length also includes Ethernet header, MTK tag, and FCS length */
> + length = new_mtu + ETH_HLEN + MTK_HDR_LEN + ETH_FCS_LEN;
> + if (length <= 1522)
> + mt7530_rmw(priv, MT7530_GMACCR, MAX_RX_PKT_LEN_MASK, MAX_RX_PKT_LEN_1522);
> + else if (length <= 1536)
> + mt7530_rmw(priv, MT7530_GMACCR, MAX_RX_PKT_LEN_MASK, MAX_RX_PKT_LEN_1536);
> + else if (length <= 1552)
> + mt7530_rmw(priv, MT7530_GMACCR, MAX_RX_PKT_LEN_MASK, MAX_RX_PKT_LEN_1552);
> + else
> + mt7530_rmw(priv, MT7530_GMACCR, MAX_RX_JUMBO_MASK | MAX_RX_PKT_LEN_MASK,
> + MAX_RX_JUMBO(DIV_ROUND_UP(length, 1024)) | MAX_RX_PKT_LEN_JUMBO);
> +
> + return 0;
> +}
> +
> +static int
> +mt7530_port_max_mtu(struct dsa_switch *ds, int port)
> +{
> + return MT7530_MAX_MTU;
> +}
> +
> static void
> mt7530_stp_state_set(struct dsa_switch *ds, int port, u8 state)
> {
> @@ -2519,6 +2553,8 @@ static const struct dsa_switch_ops mt7530_switch_ops = {
> .get_sset_count = mt7530_get_sset_count,
> .port_enable = mt7530_port_enable,
> .port_disable = mt7530_port_disable,
> + .port_change_mtu = mt7530_port_change_mtu,
> + .port_max_mtu = mt7530_port_max_mtu,
> .port_stp_state_set = mt7530_stp_state_set,
> .port_bridge_join = mt7530_port_bridge_join,
> .port_bridge_leave = mt7530_port_bridge_leave,
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net: dsa: mt7530: support setting MTU
2020-10-28 18:12 [PATCH] net: dsa: mt7530: support setting MTU DENG Qingfang
2020-10-28 18:31 ` Vladimir Oltean
@ 2020-10-28 18:46 ` Florian Fainelli
2020-10-29 22:42 ` Jakub Kicinski
2 siblings, 0 replies; 8+ messages in thread
From: Florian Fainelli @ 2020-10-28 18:46 UTC (permalink / raw)
To: DENG Qingfang, netdev
Cc: Sean Wang, Landen Chao, Andrew Lunn, Vivien Didelot,
Vladimir Oltean, David S . Miller, René van Dorst
On 10/28/2020 11:12 AM, DENG Qingfang wrote:
> MT7530/7531 has a global RX packet length register, which can be used
> to set MTU.
>
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net: dsa: mt7530: support setting MTU
2020-10-28 18:31 ` Vladimir Oltean
@ 2020-10-29 3:32 ` DENG Qingfang
2020-10-29 8:11 ` Jonathan McDowell
0 siblings, 1 reply; 8+ messages in thread
From: DENG Qingfang @ 2020-10-29 3:32 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Jakub Kicinski, netdev, Sean Wang, Landen Chao, Andrew Lunn,
Vivien Didelot, Florian Fainelli, David S . Miller,
René van Dorst, Linus Walleij, Murali Krishna Policharla,
Jonathan McDowell, Chris Packham
On Thu, Oct 29, 2020 at 2:31 AM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> On Thu, Oct 29, 2020 at 02:12:21AM +0800, DENG Qingfang wrote:
> > MT7530/7531 has a global RX packet length register, which can be used
> > to set MTU.
> >
> > Signed-off-by: DENG Qingfang <dqfext@gmail.com>
> > ---
>
> Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
>
> Also, please format your patches with --subject-prefix="PATCH net-next"
> in the future. Jakub installed some patchwork scripts that "guess" the
> tree based on the commit message, but maybe sometimes they might fail:
>
> https://patchwork.ozlabs.org/project/netdev/patch/e5fdcddeda21884a21162e441d1e8a04994f2825.1603837679.git.pavana.sharma@digi.com/
>
> > drivers/net/dsa/mt7530.c | 36 ++++++++++++++++++++++++++++++++++++
> > drivers/net/dsa/mt7530.h | 12 ++++++++++++
> > 2 files changed, 48 insertions(+)
> >
> > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> > index de7692b763d8..7764c66a47c9 100644
> > --- a/drivers/net/dsa/mt7530.c
> > +++ b/drivers/net/dsa/mt7530.c
> > @@ -1021,6 +1021,40 @@ mt7530_port_disable(struct dsa_switch *ds, int port)
> > mutex_unlock(&priv->reg_mutex);
> > }
> >
> > +static int
> > +mt7530_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
> > +{
> > + struct mt7530_priv *priv = ds->priv;
> > + int length;
> > +
> > + /* When a new MTU is set, DSA always set the CPU port's MTU to the largest MTU
> > + * of the slave ports. Because the switch only has a global RX length register,
> > + * only allowing CPU port here is enough.
> > + */
>
> Good point, please tell that to Linus (cc) - I'm talking about
> e0b2e0d8e669 ("net: dsa: rtl8366rb: Roof MTU for switch"),
And 6ae5834b983a ("net: dsa: b53: add MTU configuration support"),
1baf0fac10fb ("net: dsa: mv88e6xxx: Use chip-wide max frame size for MTU"),
f58d2598cf70 ("net: dsa: qca8k: implement the port MTU callbacks")
CC'd them as well.
Also, the commit e0b2e0d8e669 states that the new_mtu parameter is L2
frame length instead of L2 payload. But according to my tests, it is
L2 payload (i.e. the same as the MTU shown in `ip link` or `ifconfig`.
Is that right?
>
> > + if (!dsa_is_cpu_port(ds, port))
> > + return 0;
> > +
> > + /* RX length also includes Ethernet header, MTK tag, and FCS length */
> > + length = new_mtu + ETH_HLEN + MTK_HDR_LEN + ETH_FCS_LEN;
> > + if (length <= 1522)
> > + mt7530_rmw(priv, MT7530_GMACCR, MAX_RX_PKT_LEN_MASK, MAX_RX_PKT_LEN_1522);
> > + else if (length <= 1536)
> > + mt7530_rmw(priv, MT7530_GMACCR, MAX_RX_PKT_LEN_MASK, MAX_RX_PKT_LEN_1536);
> > + else if (length <= 1552)
> > + mt7530_rmw(priv, MT7530_GMACCR, MAX_RX_PKT_LEN_MASK, MAX_RX_PKT_LEN_1552);
> > + else
> > + mt7530_rmw(priv, MT7530_GMACCR, MAX_RX_JUMBO_MASK | MAX_RX_PKT_LEN_MASK,
> > + MAX_RX_JUMBO(DIV_ROUND_UP(length, 1024)) | MAX_RX_PKT_LEN_JUMBO);
> > +
> > + return 0;
> > +}
> > +
> > +static int
> > +mt7530_port_max_mtu(struct dsa_switch *ds, int port)
> > +{
> > + return MT7530_MAX_MTU;
> > +}
> > +
> > static void
> > mt7530_stp_state_set(struct dsa_switch *ds, int port, u8 state)
> > {
> > @@ -2519,6 +2553,8 @@ static const struct dsa_switch_ops mt7530_switch_ops = {
> > .get_sset_count = mt7530_get_sset_count,
> > .port_enable = mt7530_port_enable,
> > .port_disable = mt7530_port_disable,
> > + .port_change_mtu = mt7530_port_change_mtu,
> > + .port_max_mtu = mt7530_port_max_mtu,
> > .port_stp_state_set = mt7530_stp_state_set,
> > .port_bridge_join = mt7530_port_bridge_join,
> > .port_bridge_leave = mt7530_port_bridge_leave,
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net: dsa: mt7530: support setting MTU
2020-10-29 3:32 ` DENG Qingfang
@ 2020-10-29 8:11 ` Jonathan McDowell
2020-10-29 8:19 ` DENG Qingfang
0 siblings, 1 reply; 8+ messages in thread
From: Jonathan McDowell @ 2020-10-29 8:11 UTC (permalink / raw)
To: DENG Qingfang
Cc: Vladimir Oltean, Jakub Kicinski, netdev, Sean Wang, Landen Chao,
Andrew Lunn, Vivien Didelot, Florian Fainelli, David S . Miller,
René van Dorst, Linus Walleij, Murali Krishna Policharla,
Chris Packham
On Thu, Oct 29, 2020 at 11:32:36AM +0800, DENG Qingfang wrote:
> On Thu, Oct 29, 2020 at 2:31 AM Vladimir Oltean <olteanv@gmail.com> wrote:
> >
> > On Thu, Oct 29, 2020 at 02:12:21AM +0800, DENG Qingfang wrote:
...
> > > +static int
> > > +mt7530_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
> > > +{
> > > + struct mt7530_priv *priv = ds->priv;
> > > + int length;
> > > +
> > > + /* When a new MTU is set, DSA always set the CPU port's MTU to the largest MTU
> > > + * of the slave ports. Because the switch only has a global RX length register,
> > > + * only allowing CPU port here is enough.
> > > + */
> >
> > Good point, please tell that to Linus (cc) - I'm talking about
> > e0b2e0d8e669 ("net: dsa: rtl8366rb: Roof MTU for switch"),
>
> And 6ae5834b983a ("net: dsa: b53: add MTU configuration support"),
> 1baf0fac10fb ("net: dsa: mv88e6xxx: Use chip-wide max frame size for MTU"),
> f58d2598cf70 ("net: dsa: qca8k: implement the port MTU callbacks")
>
> CC'd them as well.
qca8k tracks and use the maximum provided mtu; perhaps that could be
optimised by only allow the CPU port to be set but it feels a bit more
future proof (e.g. if/when we support multiple CPU ports).
> Also, the commit e0b2e0d8e669 states that the new_mtu parameter is L2
> frame length instead of L2 payload. But according to my tests, it is
> L2 payload (i.e. the same as the MTU shown in `ip link` or `ifconfig`.
> Is that right?
Certainly that's what I saw; qca8k sets the MTU to the provided MTU +
ETH_HLEN + ETH_FCS_LEN.
J.
--
Pretty please, with sugar on top, clean the f**king car.
This .sig brought to you by the letter J and the number 13
Product of the Republic of HuggieTag
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net: dsa: mt7530: support setting MTU
2020-10-29 8:11 ` Jonathan McDowell
@ 2020-10-29 8:19 ` DENG Qingfang
0 siblings, 0 replies; 8+ messages in thread
From: DENG Qingfang @ 2020-10-29 8:19 UTC (permalink / raw)
To: Jonathan McDowell
Cc: Vladimir Oltean, Jakub Kicinski, netdev, Sean Wang, Landen Chao,
Andrew Lunn, Vivien Didelot, Florian Fainelli, David S . Miller,
René van Dorst, Linus Walleij, Murali Krishna Policharla,
Chris Packham
On Thu, Oct 29, 2020 at 4:11 PM Jonathan McDowell <noodles@earth.li> wrote:
>
> On Thu, Oct 29, 2020 at 11:32:36AM +0800, DENG Qingfang wrote:
> > On Thu, Oct 29, 2020 at 2:31 AM Vladimir Oltean <olteanv@gmail.com> wrote:
> > >
> > > On Thu, Oct 29, 2020 at 02:12:21AM +0800, DENG Qingfang wrote:
> ...
> > > > +static int
> > > > +mt7530_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
> > > > +{
> > > > + struct mt7530_priv *priv = ds->priv;
> > > > + int length;
> > > > +
> > > > + /* When a new MTU is set, DSA always set the CPU port's MTU to the largest MTU
> > > > + * of the slave ports. Because the switch only has a global RX length register,
> > > > + * only allowing CPU port here is enough.
> > > > + */
> > >
> > > Good point, please tell that to Linus (cc) - I'm talking about
> > > e0b2e0d8e669 ("net: dsa: rtl8366rb: Roof MTU for switch"),
> >
> > And 6ae5834b983a ("net: dsa: b53: add MTU configuration support"),
> > 1baf0fac10fb ("net: dsa: mv88e6xxx: Use chip-wide max frame size for MTU"),
> > f58d2598cf70 ("net: dsa: qca8k: implement the port MTU callbacks")
> >
> > CC'd them as well.
>
> qca8k tracks and use the maximum provided mtu; perhaps that could be
> optimised by only allow the CPU port to be set but it feels a bit more
> future proof (e.g. if/when we support multiple CPU ports).
btw, there is a bug in your commit f58d2598cf70, in
qca8k_port_change_mtu the loop variable is not used inside the for
loop.
>
> > Also, the commit e0b2e0d8e669 states that the new_mtu parameter is L2
> > frame length instead of L2 payload. But according to my tests, it is
> > L2 payload (i.e. the same as the MTU shown in `ip link` or `ifconfig`.
> > Is that right?
>
> Certainly that's what I saw; qca8k sets the MTU to the provided MTU +
> ETH_HLEN + ETH_FCS_LEN.
>
> J.
>
> --
> Pretty please, with sugar on top, clean the f**king car.
> This .sig brought to you by the letter J and the number 13
> Product of the Republic of HuggieTag
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net: dsa: mt7530: support setting MTU
2020-10-28 18:12 [PATCH] net: dsa: mt7530: support setting MTU DENG Qingfang
2020-10-28 18:31 ` Vladimir Oltean
2020-10-28 18:46 ` Florian Fainelli
@ 2020-10-29 22:42 ` Jakub Kicinski
2020-10-30 2:58 ` DENG Qingfang
2 siblings, 1 reply; 8+ messages in thread
From: Jakub Kicinski @ 2020-10-29 22:42 UTC (permalink / raw)
To: DENG Qingfang
Cc: netdev, Sean Wang, Landen Chao, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, David S . Miller,
René van Dorst
On Thu, 29 Oct 2020 02:12:21 +0800 DENG Qingfang wrote:
> MT7530/7531 has a global RX packet length register, which can be used
> to set MTU.
>
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>
Please wrap your code at 80 chars.
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index de7692b763d8..7764c66a47c9 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -1021,6 +1021,40 @@ mt7530_port_disable(struct dsa_switch *ds, int port)
> mutex_unlock(&priv->reg_mutex);
> }
>
> +static int
> +mt7530_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
> +{
> + struct mt7530_priv *priv = ds->priv;
> + int length;
> +
> + /* When a new MTU is set, DSA always set the CPU port's MTU to the largest MTU
> + * of the slave ports. Because the switch only has a global RX length register,
> + * only allowing CPU port here is enough.
> + */
> + if (!dsa_is_cpu_port(ds, port))
> + return 0;
> +
> + /* RX length also includes Ethernet header, MTK tag, and FCS length */
> + length = new_mtu + ETH_HLEN + MTK_HDR_LEN + ETH_FCS_LEN;
> + if (length <= 1522)
> + mt7530_rmw(priv, MT7530_GMACCR, MAX_RX_PKT_LEN_MASK, MAX_RX_PKT_LEN_1522);
> + else if (length <= 1536)
> + mt7530_rmw(priv, MT7530_GMACCR, MAX_RX_PKT_LEN_MASK, MAX_RX_PKT_LEN_1536);
> + else if (length <= 1552)
> + mt7530_rmw(priv, MT7530_GMACCR, MAX_RX_PKT_LEN_MASK, MAX_RX_PKT_LEN_1552);
> + else
> + mt7530_rmw(priv, MT7530_GMACCR, MAX_RX_JUMBO_MASK | MAX_RX_PKT_LEN_MASK,
> + MAX_RX_JUMBO(DIV_ROUND_UP(length, 1024)) | MAX_RX_PKT_LEN_JUMBO);
this line should start under priv, so it aligns to the opening
parenthesis.
Besides, don't you need to reset the JUMBO bit when going from jumbo to
non-jumbo? The mask should always include jumbo.
I assume you're duplicating the mt7530_rmw() for the benefit of the
constant validation, but it seems to be counterproductive here.
> + return 0;
> +}
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net: dsa: mt7530: support setting MTU
2020-10-29 22:42 ` Jakub Kicinski
@ 2020-10-30 2:58 ` DENG Qingfang
0 siblings, 0 replies; 8+ messages in thread
From: DENG Qingfang @ 2020-10-30 2:58 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, Sean Wang, Landen Chao, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Vladimir Oltean, David S . Miller,
René van Dorst
On Fri, Oct 30, 2020 at 6:42 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 29 Oct 2020 02:12:21 +0800 DENG Qingfang wrote:
> > MT7530/7531 has a global RX packet length register, which can be used
> > to set MTU.
> >
> > Signed-off-by: DENG Qingfang <dqfext@gmail.com>
>
> Please wrap your code at 80 chars.
>
> > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> > index de7692b763d8..7764c66a47c9 100644
> > --- a/drivers/net/dsa/mt7530.c
> > +++ b/drivers/net/dsa/mt7530.c
> > @@ -1021,6 +1021,40 @@ mt7530_port_disable(struct dsa_switch *ds, int port)
> > mutex_unlock(&priv->reg_mutex);
> > }
> >
> > +static int
> > +mt7530_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
> > +{
> > + struct mt7530_priv *priv = ds->priv;
> > + int length;
> > +
> > + /* When a new MTU is set, DSA always set the CPU port's MTU to the largest MTU
> > + * of the slave ports. Because the switch only has a global RX length register,
> > + * only allowing CPU port here is enough.
> > + */
> > + if (!dsa_is_cpu_port(ds, port))
> > + return 0;
> > +
> > + /* RX length also includes Ethernet header, MTK tag, and FCS length */
> > + length = new_mtu + ETH_HLEN + MTK_HDR_LEN + ETH_FCS_LEN;
> > + if (length <= 1522)
> > + mt7530_rmw(priv, MT7530_GMACCR, MAX_RX_PKT_LEN_MASK, MAX_RX_PKT_LEN_1522);
> > + else if (length <= 1536)
> > + mt7530_rmw(priv, MT7530_GMACCR, MAX_RX_PKT_LEN_MASK, MAX_RX_PKT_LEN_1536);
> > + else if (length <= 1552)
> > + mt7530_rmw(priv, MT7530_GMACCR, MAX_RX_PKT_LEN_MASK, MAX_RX_PKT_LEN_1552);
> > + else
> > + mt7530_rmw(priv, MT7530_GMACCR, MAX_RX_JUMBO_MASK | MAX_RX_PKT_LEN_MASK,
> > + MAX_RX_JUMBO(DIV_ROUND_UP(length, 1024)) | MAX_RX_PKT_LEN_JUMBO);
>
> this line should start under priv, so it aligns to the opening
> parenthesis.
>
> Besides, don't you need to reset the JUMBO bit when going from jumbo to
> non-jumbo? The mask should always include jumbo.
MAX_RX_JUMBO works only when MAX_RX_PKT_LEN is set to 0x3, so just
changing MAX_RX_PKT_LEN to non-jumbo is enough.
FYI, the default value of MAX_RX_JUMBO is 0x9.
>
> I assume you're duplicating the mt7530_rmw() for the benefit of the
> constant validation, but it seems to be counterproductive here.
As I mentioned above, MAX_RX_JUMBO does not need to be changed when
going from jumbo to non-jumbo.
Perhaps I should use mt7530_mii_read() and mt7530_mii_write() instead?
>
> > + return 0;
> > +}
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-10-30 2:58 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-28 18:12 [PATCH] net: dsa: mt7530: support setting MTU DENG Qingfang
2020-10-28 18:31 ` Vladimir Oltean
2020-10-29 3:32 ` DENG Qingfang
2020-10-29 8:11 ` Jonathan McDowell
2020-10-29 8:19 ` DENG Qingfang
2020-10-28 18:46 ` Florian Fainelli
2020-10-29 22:42 ` Jakub Kicinski
2020-10-30 2:58 ` DENG Qingfang
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).