by Russell King (Oracle)

[permalink] [raw]

Subject: Re: [PATCH 1/7] dsa: marvell: Provide per device information about max frame size

On Fri, Mar 10, 2023 at 02:02:35PM +0200, Vladimir Oltean wrote:
> By ops:
>
> port_set_jumbo_size:
> static const struct mv88e6xxx_ops mv88e6131_ops = {
> static const struct mv88e6xxx_ops mv88e6141_ops = {
> static const struct mv88e6xxx_ops mv88e6171_ops = {
> static const struct mv88e6xxx_ops mv88e6172_ops = {
> static const struct mv88e6xxx_ops mv88e6175_ops = {
> static const struct mv88e6xxx_ops mv88e6176_ops = {
> static const struct mv88e6xxx_ops mv88e6190_ops = {
> static const struct mv88e6xxx_ops mv88e6190x_ops = {
> static const struct mv88e6xxx_ops mv88e6240_ops = {
> static const struct mv88e6xxx_ops mv88e6320_ops = {
> static const struct mv88e6xxx_ops mv88e6321_ops = {
> static const struct mv88e6xxx_ops mv88e6341_ops = {
> static const struct mv88e6xxx_ops mv88e6350_ops = {
> static const struct mv88e6xxx_ops mv88e6351_ops = {
> static const struct mv88e6xxx_ops mv88e6352_ops = {
> static const struct mv88e6xxx_ops mv88e6390_ops = {
> static const struct mv88e6xxx_ops mv88e6390x_ops = {
> static const struct mv88e6xxx_ops mv88e6393x_ops = {
>
> set_max_frame_size:
> static const struct mv88e6xxx_ops mv88e6085_ops = {
> static const struct mv88e6xxx_ops mv88e6095_ops = {
> static const struct mv88e6xxx_ops mv88e6097_ops = {
> static const struct mv88e6xxx_ops mv88e6123_ops = {
> static const struct mv88e6xxx_ops mv88e6161_ops = {
> static const struct mv88e6xxx_ops mv88e6185_ops = {
>
> none of the above:
> static const struct mv88e6xxx_ops mv88e6165_ops = {
> static const struct mv88e6xxx_ops mv88e6191_ops = {
> static const struct mv88e6xxx_ops mv88e6250_ops = {
> static const struct mv88e6xxx_ops mv88e6290_ops = {
>
>
> By info:
>
> port_set_jumbo_size (10240):
> [MV88E6131] = {
> [MV88E6141] = {
> [MV88E6171] = {
> [MV88E6172] = {
> [MV88E6175] = {
> [MV88E6176] = {
> [MV88E6190] = {
> [MV88E6190X] = {
> [MV88E6240] = {
> [MV88E6320] = {
> [MV88E6321] = {
> [MV88E6341] = {
> [MV88E6350] = {
> [MV88E6351] = {
> [MV88E6352] = {
> [MV88E6390] = {
> [MV88E6390X] = {
> [MV88E6191X] = {
> [MV88E6193X] = {
> [MV88E6393X] = {
>
> set_max_frame_size (1632):
> [MV88E6085] = {
> [MV88E6095] = {
> [MV88E6097] = {
> [MV88E6123] = {
> [MV88E6161] = {
> [MV88E6185] = {
>
> none of the above (1522):
> [MV88E6165] = {
> [MV88E6191] = {
> [MV88E6220] = {
> [MV88E6250] = {
> [MV88E6290] = {
>
>
> Whereas your analysis seems to have determined this:
>
> port_set_jumbo_size (10240):
> [MV88E6131] = {
> [MV88E6141] = {
> [MV88E6171] = {
> [MV88E6172] = {
> [MV88E6175] = {
> [MV88E6176] = {
> [MV88E6190] = {
> [MV88E6240] = {
> [MV88E6320] = {
> [MV88E6321] = {
> [MV88E6341] = {
> [MV88E6350] = {
> [MV88E6351] = {
> [MV88E6352] = {
> [MV88E6390] = {
> [MV88E6390X] = {
> [MV88E6393X] = {
>
> set_max_frame_size (1632):
> [MV88E6095] = {
> [MV88E6097] = {
> [MV88E6123] = {
> [MV88E6161] = {
> [MV88E6165] = {
> [MV88E6185] = {
>
> none of the above (1522):
> [MV88E6085] = {
> [MV88E6190X] = {
> [MV88E6191] = {
> [MV88E6191X] = {
> [MV88E6193X] = {
> [MV88E6290] = {
>
> what's up with these?! (no max_frame_size)
> [MV88E6220] = {
> [MV88E6250] = {
>
>
> So our analysis differs for:
>
> MV88E6190X (I say 10240, you say 1522)
> MV88E6191X (I say 10240, you say 1522)
> MV88E6193X (I say 10240, you say 1522)
> MV88E6085 (I say 1632, you say 1522)
> MV88E6165 (I say 1522, you say 1632)
> MV88E6220 (I say 1522, not clear what you say)
> MV88E6250 (I say 1522, not clear what you say)
>
> Double-checking with the code, I believe my analysis to be the correct one...

This is similar analysis to what I did for a previous patch set, and
came to the conclusion that we need code in the driver to validate
that the addition of these values is in fact correct. See my previous
reviews and my recommendations on how to structure these patch sets,
so we as reviewers don't _have_ to go to this level of verification.

> I have also noticed that you have not acted upon my previous review comment:
> https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
>
> | 1522 - 30 = 1492.
> |
> | I don't believe that there are switches which don't support the standard
> | MTU of 1500 ?!
> |
> | > .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?
>
> I guess I will have to fix this now, since you haven't done it.

I'm sorry, but why is this Lukasz's problem to fix? If it's broken today
when using mv88e6xxx with this PHY, and Lukasz doesn't have this PHY,
why does Lukasz have to solve this?

> > +
> > + /* Max Frame Size.
> > + * This value corresponds to the memory allocated in switch internal
> > + * memory to store single frame.
> > + */
>
> What is the source of this definition?
>
> I'm asking because I know of other switches where the internal memory
> allocation scheme has nothing to do with the frame size. Instead, there
> are SRAM cells of fixed and small size (say 60 octets) chained together.

The switch documentation only really talks about maximum frame sizes
that the switch can handle, with a few bits that configure what the
maximum frame size is. We also know how large the SRAM is, but how
the SRAM is allocated to packets is for Marvell engineers to know
and not us mere mortals.

So, the base definition for this is the information provided in the
switch documentation.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

2023-03-10 13:04:58

by Vladimir Oltean

[permalink] [raw]

Subject: Re: [PATCH 1/7] dsa: marvell: Provide per device information about max frame size

On Fri, Mar 10, 2023 at 12:25:59PM +0000, Russell King (Oracle) wrote:
> This is similar analysis to what I did for a previous patch set, and
> came to the conclusion that we need code in the driver to validate
> that the addition of these values is in fact correct. See my previous
> reviews and my recommendations on how to structure these patch sets,
> so we as reviewers don't _have_ to go to this level of verification.

Ok, I haven't read other patches or comments except for 1/7 yet.

> > I guess I will have to fix this now, since you haven't done it.
>
> I'm sorry, but why is this Lukasz's problem to fix? If it's broken today
> when using mv88e6xxx with this PHY, and Lukasz doesn't have this PHY,
> why does Lukasz have to solve this?

Well, in principle no one has to solve it. It would be good to not move
around broken code if we know it's broken, is what I'm saying. This is
because eventually, someone who *is* affected *will* want to fix it, and
that fix will conflict with the refactoring. Lukasz would have had the
interest in fixing it because he's touching that code. Again, I will do
this when I find the time.

> > > + /* Max Frame Size.
> > > + * This value corresponds to the memory allocated in switch internal
> > > + * memory to store single frame.
> > > + */
> >
> > What is the source of this definition?
> >
> > I'm asking because I know of other switches where the internal memory
> > allocation scheme has nothing to do with the frame size. Instead, there
> > are SRAM cells of fixed and small size (say 60 octets) chained together.
>
> The switch documentation only really talks about maximum frame sizes
> that the switch can handle, with a few bits that configure what the
> maximum frame size is. We also know how large the SRAM is, but how
> the SRAM is allocated to packets is for Marvell engineers to know
> and not us mere mortals.
>
> So, the base definition for this is the information provided in the
> switch documentation.

Agree with the "mere mortals" comment. I was trying to suggest that the
given definition tries to make it appear that we know more about the
switch internal memory allocation than we really do. "This value
corresponds to the memory allocated in switch internal memory to store
single frame" -> how do we know that it corresponds?

2023-03-10 13:06:43

by Vladimir Oltean

[permalink] [raw]

Subject: Re: [PATCH 1/7] dsa: marvell: Provide per device information about max frame size

On Fri, Mar 10, 2023 at 03:04:46PM +0200, Vladimir Oltean wrote:
> > I'm sorry, but why is this Lukasz's problem to fix? If it's broken today
> > when using mv88e6xxx with this PHY, and Lukasz doesn't have this PHY,
> > why does Lukasz have to solve this?
>
> Well, in principle no one has to solve it. It would be good to not move
> around broken code if we know it's broken, is what I'm saying. This is
> because eventually, someone who *is* affected *will* want to fix it, and
> that fix will conflict with the refactoring. Lukasz would have had the
> interest in fixing it because he's touching that code. Again, I will do
> this when I find the time.

also: what PHY? :-/

2023-03-10 13:17:38

by Lukasz Majewski

[permalink] [raw]

Subject: Re: [PATCH 1/7] dsa: marvell: Provide per device information about max frame size

Hi Vladimir,

> On Thu, Mar 09, 2023 at 01:54:15PM +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.
>
> What is the criterion based on which 1632 is the "in-driver standard
> value"?

It comes from the documentation I suppose. Moreover, this might be the
the "first" used value when set_max_mtu callback was introduced.

>
> > On the other hand - mv88e6250 supports 2048 bytes.
>
> What you mean to suggest here is that, using the current
> classification from mv88e6xxx_get_max_mtu(), mv88e6250 falls into the
> "none of the above" bucket, for which the driver returns 1522 -
> VLAN_ETH_HLEN - EDSA_HLEN - ETH_FCS_LEN // 1492. But it truly
> supports a maximum frame length of 2048, per your research.
>

And this cannot be easily fixed.

I could just provide callback to .set_max_frame_size in mv88e6250_ops
and the mv88e6250 would use 1632 bytes which would prevent errors.

However, when I dig deeper - it turned out that this value is larger.
And hence I wanted to do it in "right way".

> The problem is that I needed to spend 30 minutes to understand this,
> and the true motivation for this patch.
>

The motivation is correctly stated in the commit message. There is a
bug - mv88e6250 and friends use 2048 max frame size value.

> > To be more interesting - devices supporting jumbo frames - use yet
> > another value (10240 bytes)
>
> What's interesting about this?
>
> >
> > 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.
>
> You need to provide a justification for why the existing code
> structure is not good enough.

It is clearly stated in patch 3/7 where the existing scheme is replaced
with this one.

>
> >
> > 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 switch driver
> > (e.g. .set_max_frame_size, .port_set_jumbo_size).
> >
> > Signed-off-by: Lukasz Majewski <[email protected]>
> >
> > ---
> > 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
> >
> > Changes for v5:
> > - Move some code fragments (like get_mtu callback changes) to
> > separate patches
>
> you have change log up to v5, but your subject prefix is [PATCH 1/7]
> which implies v1?

My mistake - I've forgotten to add proper subject-prefix parameter.

>
> > ---
> > drivers/net/dsa/mv88e6xxx/chip.c | 31
> > +++++++++++++++++++++++++++++++ drivers/net/dsa/mv88e6xxx/chip.h |
> > 6 ++++++ 2 files changed, 37 insertions(+)
> >
> > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c
> > b/drivers/net/dsa/mv88e6xxx/chip.c index 0a5d6c7bb128..c097a0b19ba6
> > 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c
> > +++ b/drivers/net/dsa/mv88e6xxx/chip.c
>
> It would be good if the commit message contained the procedure based
> on which you had made these changes - and preferably they were
> mechanical. Having a small C program written would be absolutely
> ideal. This is so that reviewers wouldn't have to do it in parallel...
>
> My analysis has determined the following 3 categories:
>
> 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; // 10210 else if (chip->info->ops->set_max_frame_size)
> return 1632 - VLAN_ETH_HLEN - EDSA_HLEN -
> ETH_FCS_LEN; // 1602 return 1522 - VLAN_ETH_HLEN - EDSA_HLEN -
> ETH_FCS_LEN; // 1492 }
>
> By ops:
>
> port_set_jumbo_size:
> static const struct mv88e6xxx_ops mv88e6131_ops = {
> static const struct mv88e6xxx_ops mv88e6141_ops = {
> static const struct mv88e6xxx_ops mv88e6171_ops = {
> static const struct mv88e6xxx_ops mv88e6172_ops = {
> static const struct mv88e6xxx_ops mv88e6175_ops = {
> static const struct mv88e6xxx_ops mv88e6176_ops = {
> static const struct mv88e6xxx_ops mv88e6190_ops = {
> static const struct mv88e6xxx_ops mv88e6190x_ops = {
> static const struct mv88e6xxx_ops mv88e6240_ops = {
> static const struct mv88e6xxx_ops mv88e6320_ops = {
> static const struct mv88e6xxx_ops mv88e6321_ops = {
> static const struct mv88e6xxx_ops mv88e6341_ops = {
> static const struct mv88e6xxx_ops mv88e6350_ops = {
> static const struct mv88e6xxx_ops mv88e6351_ops = {
> static const struct mv88e6xxx_ops mv88e6352_ops = {
> static const struct mv88e6xxx_ops mv88e6390_ops = {
> static const struct mv88e6xxx_ops mv88e6390x_ops = {
> static const struct mv88e6xxx_ops mv88e6393x_ops = {
>
> set_max_frame_size:
> static const struct mv88e6xxx_ops mv88e6085_ops = {
> static const struct mv88e6xxx_ops mv88e6095_ops = {
> static const struct mv88e6xxx_ops mv88e6097_ops = {
> static const struct mv88e6xxx_ops mv88e6123_ops = {
> static const struct mv88e6xxx_ops mv88e6161_ops = {
> static const struct mv88e6xxx_ops mv88e6185_ops = {
>
> none of the above:
> static const struct mv88e6xxx_ops mv88e6165_ops = {
> static const struct mv88e6xxx_ops mv88e6191_ops = {
> static const struct mv88e6xxx_ops mv88e6250_ops = {
> static const struct mv88e6xxx_ops mv88e6290_ops = {
>
>
> By info:
>
> port_set_jumbo_size (10240):
> [MV88E6131] = {
> [MV88E6141] = {
> [MV88E6171] = {
> [MV88E6172] = {
> [MV88E6175] = {
> [MV88E6176] = {
> [MV88E6190] = {
> [MV88E6190X] = {
> [MV88E6240] = {
> [MV88E6320] = {
> [MV88E6321] = {
> [MV88E6341] = {
> [MV88E6350] = {
> [MV88E6351] = {
> [MV88E6352] = {
> [MV88E6390] = {
> [MV88E6390X] = {
> [MV88E6191X] = {
> [MV88E6193X] = {
> [MV88E6393X] = {
>
> set_max_frame_size (1632):
> [MV88E6085] = {
> [MV88E6095] = {
> [MV88E6097] = {
> [MV88E6123] = {
> [MV88E6161] = {
> [MV88E6185] = {
>
> none of the above (1522):
> [MV88E6165] = {
> [MV88E6191] = {
> [MV88E6220] = {
> [MV88E6250] = {
> [MV88E6290] = {
>
>
> Whereas your analysis seems to have determined this:
>
> port_set_jumbo_size (10240):
> [MV88E6131] = {
> [MV88E6141] = {
> [MV88E6171] = {
> [MV88E6172] = {
> [MV88E6175] = {
> [MV88E6176] = {
> [MV88E6190] = {
> [MV88E6240] = {
> [MV88E6320] = {
> [MV88E6321] = {
> [MV88E6341] = {
> [MV88E6350] = {
> [MV88E6351] = {
> [MV88E6352] = {
> [MV88E6390] = {
> [MV88E6390X] = {
> [MV88E6393X] = {
>
> set_max_frame_size (1632):
> [MV88E6095] = {
> [MV88E6097] = {
> [MV88E6123] = {
> [MV88E6161] = {
> [MV88E6165] = {
> [MV88E6185] = {
>
> none of the above (1522):
> [MV88E6085] = {
> [MV88E6190X] = {
> [MV88E6191] = {
> [MV88E6191X] = {
> [MV88E6193X] = {
> [MV88E6290] = {
>
> what's up with these?! (no max_frame_size)
> [MV88E6220] = {
> [MV88E6250] = {
>
>
> So our analysis differs for:
>
> MV88E6190X (I say 10240, you say 1522)
> MV88E6191X (I say 10240, you say 1522)
> MV88E6193X (I say 10240, you say 1522)
> MV88E6085 (I say 1632, you say 1522)
> MV88E6165 (I say 1522, you say 1632)
> MV88E6220 (I say 1522, not clear what you say)
> MV88E6250 (I say 1522, not clear what you say)
>
> Double-checking with the code, I believe my analysis to be the
> correct one...
>

This is explained and provided in the following commits as advised by
Russel.

>
> I have also noticed that you have not acted upon my previous review
> comment:
> https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
>
> | 1522 - 30 = 1492.
> |
> | I don't believe that there are switches which don't support the
> standard | MTU of 1500 ?!
> |
> | > .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?
>
> I guess I will have to fix this now, since you haven't done it.
>

I do agree with Russel's reply here.

Moreover, as 6250 and 6220 also have max frame size equal to 2048
bytes, this would be fixed in v6 after getting the "validation"
function run.

This is the "patch 4" in the comment sent by Russel (to fix stuff which
is already broken, but it has been visible after running the validation
code):

https://lists.openwall.net/netdev/2023/03/09/233

> > diff --git a/drivers/net/dsa/mv88e6xxx/chip.h
> > b/drivers/net/dsa/mv88e6xxx/chip.h index da6e1339f809..e2b88f1f8376
> > 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.h
> > +++ b/drivers/net/dsa/mv88e6xxx/chip.h
> > @@ -132,6 +132,12 @@ struct mv88e6xxx_info {
> > unsigned int num_gpio;
> > unsigned int max_vid;
> > unsigned int max_sid;
> > +
> > + /* Max Frame Size.
> > + * This value corresponds to the memory allocated in
> > switch internal
> > + * memory to store single frame.
> > + */
>
> What is the source of this definition?
>
> I'm asking because I know of other switches where the internal memory
> allocation scheme has nothing to do with the frame size. Instead,
> there are SRAM cells of fixed and small size (say 60 octets) chained
> together.
>
> > + unsigned int max_frame_size;
> > unsigned int port_base_addr;
> > unsigned int phy_base_addr;
> > unsigned int global1_addr;
> > --
> > 2.20.1
> >

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: [email protected]

Attachments:

(No filename) (488.00 B)
OpenPGP digital signature

2023-03-10 13:19:42

Hello Lukasz,

On Thu, Mar 09, 2023 at 01:54:14PM +0100, Lukasz Majewski wrote:
> This patch set provides following changes:
>
> - Provide support for mv88e6020 and mv88e6071 switch circuits (the
> "Link Street" family of products including added earlier to this
> driver mv88e6250 and mv88e6220).
>
> - Add the max_frame size variable to specify the buffer size for the
> maximal frame size
>
> - The above change required adjusting all supported devices in the
> mv88e6xxx driver, as the current value assignment is depending
> on the set of provided callbacks for each switch circuit - i.e.
> until now the value was not explicitly specified.
>
> - As the driver for Marvell's mv88e6xxx switches was rather complicated
> the intermediate function (removed by the end of this patch set)
> has been introduced. It was supposed to both validate the provided
> values deduced from the code and leave a trace of the exact
> methodology used.

The problem with MTU 1492 has been resolved as commit 7e9517375a14
("net: dsa: mv88e6xxx: fix max_mtu of 1492 on 6165, 6191, 6220, 6250,
6290"), is present in net-next, and as such, you are free to resubmit
this patchset on top of the current net-next.gi/main whenever you feel
like it, no need to wait for a merge window.