lkml.org 
[lkml]   [2023]   [Jan]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4 1/3] dsa: marvell: Provide per device information about max frame size
Hi Vladimir,

> On Fri, Jan 06, 2023 at 11:16:49AM +0100, Lukasz Majewski wrote:
> > Different Marvell DSA switches support different size of max frame
> > bytes to be sent. This value corresponds to the memory allocated
> > in switch to store single frame.
> >
> > For example mv88e6185 supports max 1632 bytes, which is now
> > in-driver standard value. On the other hand - mv88e6250 supports
> > 2048 bytes. To be more interresting - devices supporting jumbo
> > frames - use yet another value (10240 bytes)
> >
> > As this value is internal and may be different for each switch IC,
> > new entry in struct mv88e6xxx_info has been added to store it.
> >
> > This commit doesn't change the code functionality - it just provides
> > the max frame size value explicitly - up till now it has been
> > assigned depending on the callback provided by the IC driver
> > (e.g. .set_max_frame_size, .port_set_jumbo_size).
> >
> > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> >
> > ---
> > Changes for v2:
> > - Define max_frame_size with default value of 1632 bytes,
> > - Set proper value for the mv88e6250 switch SoC (linkstreet) family
> >
> > Changes for v3:
> > - Add default value for 1632B of the max frame size (to avoid
> > problems with not defined values)
> >
> > Changes for v4:
> > - Rework the mv88e6xxx_get_max_mtu() by using per device defined
> > max_frame_size value
> >
> > - Add WARN_ON_ONCE() when max_frame_size is not defined
> >
> > - Add description for the new 'max_frame_size' member of
> > mv88e6xxx_info ---
> > drivers/net/dsa/mv88e6xxx/chip.c | 41
> > ++++++++++++++++++++++++++++---- drivers/net/dsa/mv88e6xxx/chip.h |
> > 6 +++++ 2 files changed, 42 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c
> > b/drivers/net/dsa/mv88e6xxx/chip.c index 242b8b325504..fc6d98c4a029
> > 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c
> > +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> > @@ -3545,11 +3545,10 @@ static int mv88e6xxx_get_max_mtu(struct
> > dsa_switch *ds, int port) {
> > struct mv88e6xxx_chip *chip = ds->priv;
> >
> > - if (chip->info->ops->port_set_jumbo_size)
> > - return 10240 - VLAN_ETH_HLEN - EDSA_HLEN -
> > ETH_FCS_LEN;
> > - else if (chip->info->ops->set_max_frame_size)
> > - return 1632 - VLAN_ETH_HLEN - EDSA_HLEN -
> > ETH_FCS_LEN;
> > - return 1522 - VLAN_ETH_HLEN - EDSA_HLEN - ETH_FCS_LEN;
> > + WARN_ON_ONCE(!chip->info->max_frame_size);
> > +
> > + return chip->info->max_frame_size - VLAN_ETH_HLEN -
> > EDSA_HLEN
> > + - ETH_FCS_LEN;
>
> VLAN_ETH_HLEN (18) + EDSA_HLEN (8) + ETH_FCS_LEN (4) = 30
>
> > }
> >
> > static int mv88e6xxx_change_mtu(struct dsa_switch *ds, int port,
> > int new_mtu) @@ -4955,6 +4954,7 @@ static const struct
> > mv88e6xxx_ops mv88e6250_ops = { .avb_ops = &mv88e6352_avb_ops,
> > .ptp_ops = &mv88e6250_ptp_ops,
> > .phylink_get_caps = mv88e6250_phylink_get_caps,
> > + .set_max_frame_size = mv88e6185_g1_set_max_frame_size,
> > };
> >
> > static const struct mv88e6xxx_ops mv88e6290_ops = {
> > @@ -5543,6 +5543,7 @@ static const struct mv88e6xxx_info
> > mv88e6xxx_table[] = { .num_internal_phys = 5,
> > .max_vid = 4095,
> > .max_sid = 63,
> > + .max_frame_size = 1522,
>
> 1522 - 30 = 1492.
>
> I don't believe that there are switches which don't support the
> standard MTU of 1500 ?!

I think that this commit [1], made the adjustment to fix yet another
issue.

It looks like the missing 8 bytes are added in the
mv88e6xxx_change_mtu() function.

>
> > .port_base_addr = 0x10,
> > .phy_base_addr = 0x0,
> > .global1_addr = 0x1b,
>
> Note that I see this behavior isn't new. But I've simulated it, and it
> will produce the following messages on probe:
>
> [ 7.425752] mscc_felix 0000:00:00.5 swp0 (uninitialized): PHY
> [0000:00:00.3:10] driver [Microsemi GE VSC8514 SyncE] (irq=POLL) [
> 7.437516] mscc_felix 0000:00:00.5: nonfatal error -34 setting MTU to
> 1500 on port 0 [ 7.588585] mscc_felix 0000:00:00.5 swp1
> (uninitialized): PHY [0000:00:00.3:11] driver [Microsemi GE VSC8514
> SyncE] (irq=POLL) [ 7.600433] mscc_felix 0000:00:00.5: nonfatal
> error -34 setting MTU to 1500 on port 1 [ 7.752613] mscc_felix
> 0000:00:00.5 swp2 (uninitialized): PHY [0000:00:00.3:12] driver
> [Microsemi GE VSC8514 SyncE] (irq=POLL) [ 7.764457] mscc_felix
> 0000:00:00.5: nonfatal error -34 setting MTU to 1500 on port 2 [
> 7.900771] mscc_felix 0000:00:00.5 swp3 (uninitialized): PHY
> [0000:00:00.3:13] driver [Microsemi GE VSC8514 SyncE] (irq=POLL) [
> 7.912501] mscc_felix 0000:00:00.5: nonfatal error -34 setting MTU to
> 1500 on port 3
>
> I wonder, shouldn't we first fix that, and apply this patch set
> afterwards?

IMHO, it is up to Andrew to decide how to proceed, as the
aforementioned patch [1] is an attempt to fix yet another issue [2].



Links:

[1] -
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b9c587fed61cf88bd45822c3159644445f6d5aa6

[2] -
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1baf0fac10fbe3084975d7cb0a4378eb18871482

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2023-03-26 23:39    [W:0.105 / U:0.312 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site