| Message ID | 20230309125421.3900962-7-lukma@denx.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | dsa: marvell: Add support for mv88e6071 and 6020 switches | expand |
On Thu, Mar 09, 2023 at 01:54:20PM +0100, Lukasz Majewski wrote: > Running of the mv88e6xxx_validate_frame_size() function provided following > results: > > [ 1.585565] BUG: Marvell 88E6020 has differing max_frame_size: 1632 != 2048 > [ 1.592540] BUG: Marvell 88E6071 has differing max_frame_size: 1632 != 2048 > ^------ Correct -> mv88e6250 family max frame size = 2048B > > [ 1.599507] BUG: Marvell 88E6085 has differing max_frame_size: 1632 != 1522 > [ 1.606476] BUG: Marvell 88E6165 has differing max_frame_size: 1522 != 1632 > [ 1.613445] BUG: Marvell 88E6190X has differing max_frame_size: 10240 != 1522 > [ 1.620590] BUG: Marvell 88E6191X has differing max_frame_size: 10240 != 1522 > [ 1.627730] BUG: Marvell 88E6193X has differing max_frame_size: 10240 != 1522 > ^------ Needs to be fixed!!! > > [ 1.634871] BUG: Marvell 88E6220 has differing max_frame_size: 1632 != 2048 > [ 1.641842] BUG: Marvell 88E6250 has differing max_frame_size: 1632 != 2048 > ^------ Correct -> mv88e6250 family max frame size = 2048B If I understand this correctly, in patch 4, you add a call to the 6250 family to call mv88e6185_g1_set_max_frame_size(), which sets a bit called MV88E6185_G1_CTL1_MAX_FRAME_1632 if the frame size is larger than 1518. However, you're saying that 6250 has a frame size of 2048. That's fine, but it makes MV88E6185_G1_CTL1_MAX_FRAME_1632 rather misleading as a definition. While the bit may increase the frame size, I think if we're going to do this, then this definition ought to be renamed. That said, I would like Andrew and Vladimir's thoughts on this too. Finally, I would expect, if this series was done the way I suggested, that patch 1 should set the max frame size according to how the existing code works, which means patch 2, being the validation patch, should be completely silent if patch 1 is correct - and that's the entire point of validating. It's to make sure that patch 1 is correct. If it isn't correct, then patch 1 is wrong and should be updated. Essentially, this patch should only exist if the values we are using today are actually incorrect. To put this another way, the conversion from our existing way of determining the max mtu to using the .max_frame_size method should be an entire no-op from the driver operation point of view. Then any errors in those values should be fixed and explained in a separate commit. Then the new support added. At least that's how I see it. Andrew and Vladimir may disagree.
Hi Russell, > On Thu, Mar 09, 2023 at 01:54:20PM +0100, Lukasz Majewski wrote: > > Running of the mv88e6xxx_validate_frame_size() function provided > > following results: > > > > [ 1.585565] BUG: Marvell 88E6020 has differing max_frame_size: > > 1632 != 2048 [ 1.592540] BUG: Marvell 88E6071 has differing > > max_frame_size: 1632 != 2048 ^------ Correct -> mv88e6250 family > > max frame size = 2048B > > > > [ 1.599507] BUG: Marvell 88E6085 has differing max_frame_size: > > 1632 != 1522 [ 1.606476] BUG: Marvell 88E6165 has differing > > max_frame_size: 1522 != 1632 [ 1.613445] BUG: Marvell 88E6190X > > has differing max_frame_size: 10240 != 1522 [ 1.620590] BUG: > > Marvell 88E6191X has differing max_frame_size: 10240 != 1522 [ > > 1.627730] BUG: Marvell 88E6193X has differing max_frame_size: 10240 > > != 1522 ^------ Needs to be fixed!!! > > > > [ 1.634871] BUG: Marvell 88E6220 has differing max_frame_size: > > 1632 != 2048 [ 1.641842] BUG: Marvell 88E6250 has differing > > max_frame_size: 1632 != 2048 ^------ Correct -> mv88e6250 family > > max frame size = 2048B > > If I understand this correctly, in patch 4, you add a call to the 6250 > family to call mv88e6185_g1_set_max_frame_size(), which sets a bit > called MV88E6185_G1_CTL1_MAX_FRAME_1632 if the frame size is larger > than 1518. Yes, correct. > > However, you're saying that 6250 has a frame size of 2048. That's > fine, but it makes MV88E6185_G1_CTL1_MAX_FRAME_1632 rather misleading > as a definition. While the bit may increase the frame size, I think > if we're going to do this, then this definition ought to be renamed. > I thought about rename, but then I've double checked; register offset and exact bit definition is the same as for 6185, so to avoid unnecessary code duplication - I've reused the existing function. Maybe comment would be just enough? > That said, I would like Andrew and Vladimir's thoughts on this too. > Ok. > Finally, I would expect, if this series was done the way I suggested, > that patch 1 should set the max frame size according to how the > existing code works, which means patch 2, being the validation patch, > should be completely silent if patch 1 is correct - and that's the > entire point of validating. It's to make sure that patch 1 is > correct. Ok. > > If it isn't correct, then patch 1 is wrong and should be updated. > Please correct my understanding - I do see two approaches here: A. In patch 1 I do set the max_frame_size values (deduced). Then I add validation function (patch 2). This function shows "BUG:...." only when we do have a mismatch. In patch 3 I do correct the max_frame_size values (according to validation function) and remove the validation function. This is how it is done in v5 and is going to be done in v6. B. Having showed the v5 in public, the validation function is known. Then I do prepare v6 with only patch 1 having correct values (from the outset) and provide in the commit message the code for validation function. Then patch 2 and 3 (validation function and the corrected values of max_frame_size) can be omitted in v6. For me it would be better to choose approach B. > Essentially, this patch should only exist if the values we are using > today are actually incorrect. > > To put this another way, the conversion from our existing way of > determining the max mtu to using the .max_frame_size method should be > an entire no-op from the driver operation point of view. Then any > errors in those values should be fixed and explained in a separate > commit. Then the new support added. > > At least that's how I see it. Andrew and Vladimir may disagree. > 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
On Thu, Mar 09, 2023 at 03:43:50PM +0100, Lukasz Majewski wrote: > Hi Russell, > > Please correct my understanding - I do see two approaches here: > > A. In patch 1 I do set the max_frame_size values (deduced). Then I add > validation function (patch 2). This function shows "BUG:...." only when > we do have a mismatch. In patch 3 I do correct the max_frame_size > values (according to validation function) and remove the validation > function. This is how it is done in v5 and is going to be done in v6. I don't see much point in adding the validation, then correcting the values that were added in patch 1 that were identified by patch 2 in patch 3 - because that means patch 1's deduction was incorrect in some way. If there is any correction to be done, then it should be: patch 1 - add the max_frame_size values patch 2 - add validation (which should not produce any errors) patch 3 - convert to use max_frame_size, and remove validation, stating that the validation had no errors patch 4 (if necessary) - corrections to max_frame_size values if they are actually incorrect (in other words, they were buggy before patch 1.) patch 5 onwards - the rest of the series. > B. Having showed the v5 in public, the validation function is known. > Then I do prepare v6 with only patch 1 having correct values (from the > outset) and provide in the commit message the code for validation > function. Then patch 2 and 3 (validation function and the corrected > values of max_frame_size) can be omitted in v6. > > For me it would be better to choose approach B. I would suggest that is acceptable for the final round of patches, but I'm wary about saying "yes" to it because... what if something changes in that table between the time you've validated it, and when it eventually gets accepted. Keeping the validation code means that during the review of the series, and subsequent updates onto net-next (which should of course include re-running the validation code) we can be more certain that nothing has changed that would impact it. What I worry about is if something changes, the patch adding the values mis-patches (e.g. due to other changes - much of the context for each hunk is quite similar) then we will have quite a problem to sort it out.
> > If I understand this correctly, in patch 4, you add a call to the 6250 > > family to call mv88e6185_g1_set_max_frame_size(), which sets a bit > > called MV88E6185_G1_CTL1_MAX_FRAME_1632 if the frame size is larger > > than 1518. > > Yes, correct. > > > > > However, you're saying that 6250 has a frame size of 2048. That's > > fine, but it makes MV88E6185_G1_CTL1_MAX_FRAME_1632 rather misleading > > as a definition. While the bit may increase the frame size, I think > > if we're going to do this, then this definition ought to be renamed. > > > > I thought about rename, but then I've double checked; register offset > and exact bit definition is the same as for 6185, so to avoid > unnecessary code duplication - I've reused the existing function. > > Maybe comment would be just enough? The driver takes care with its namespace in order to add per switch family defines. So you can add MV88E6250_G1_CTL1_MAX_FRAME_2048. It does not matter if it is the same bit. You can also add a mv88e6250_g1_set_max_frame_size() and it also does not matter if it is in effect the same as mv88e6185_g1_set_max_frame_size(). We should always make the driver understandably first, compact and without redundancy second. We are then less likely to get into situations like this again where it is not clear what MTU a device actually supports because the code is cryptic. Andrew
Hi Russell, > On Thu, Mar 09, 2023 at 03:43:50PM +0100, Lukasz Majewski wrote: > > Hi Russell, > > > > Please correct my understanding - I do see two approaches here: > > > > A. In patch 1 I do set the max_frame_size values (deduced). Then I > > add validation function (patch 2). This function shows "BUG:...." > > only when we do have a mismatch. In patch 3 I do correct the > > max_frame_size values (according to validation function) and remove > > the validation function. This is how it is done in v5 and is going > > to be done in v6. > > I don't see much point in adding the validation, then correcting the > values that were added in patch 1 that were identified by patch 2 in > patch 3 - because that means patch 1's deduction was incorrect in > some way. Yes. I do agree. > > If there is any correction to be done, then it should be: > > patch 1 - add the max_frame_size values > patch 2 - add validation (which should not produce any errors) > patch 3 - convert to use max_frame_size, and remove validation, > stating that the validation had no errors > patch 4 (if necessary) - corrections to max_frame_size values if they > are actually incorrect (in other words, they were buggy before patch > 1.) > patch 5 onwards - the rest of the series. > Ok. I will restructure patches to follow above scheme. > > B. Having showed the v5 in public, the validation function is known. > > Then I do prepare v6 with only patch 1 having correct values (from > > the outset) and provide in the commit message the code for > > validation function. Then patch 2 and 3 (validation function and > > the corrected values of max_frame_size) can be omitted in v6. > > > > For me it would be better to choose approach B. > > I would suggest that is acceptable for the final round of patches, but > I'm wary about saying "yes" to it because... what if something changes > in that table between the time you've validated it, and when it > eventually gets accepted. The "peace" of changes for this code is rather slow, so the risk is minimal. Moreover, next ICs added would _require_ to have the max_frame_size field set (the WARN_ON() clause). > Keeping the validation code means that > during the review of the series, and subsequent updates onto net-next > (which should of course include re-running the validation code) we > can be more certain that nothing has changed that would impact it. > > What I worry about is if something changes, the patch adding the > values mis-patches (e.g. due to other changes - much of the context > for each hunk is quite similar) then we will have quite a problem to > sort it out. > Ok. I hope that we will avoid this threat. 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
Hi Andrew, > > > If I understand this correctly, in patch 4, you add a call to the > > > 6250 family to call mv88e6185_g1_set_max_frame_size(), which sets > > > a bit called MV88E6185_G1_CTL1_MAX_FRAME_1632 if the frame size > > > is larger than 1518. > > > > Yes, correct. > > > > > > > > However, you're saying that 6250 has a frame size of 2048. That's > > > fine, but it makes MV88E6185_G1_CTL1_MAX_FRAME_1632 rather > > > misleading as a definition. While the bit may increase the frame > > > size, I think if we're going to do this, then this definition > > > ought to be renamed. > > > > I thought about rename, but then I've double checked; register > > offset and exact bit definition is the same as for 6185, so to avoid > > unnecessary code duplication - I've reused the existing function. > > > > Maybe comment would be just enough? > > The driver takes care with its namespace in order to add per switch > family defines. So you can add MV88E6250_G1_CTL1_MAX_FRAME_2048. It > does not matter if it is the same bit. You can also add a > mv88e6250_g1_set_max_frame_size() and it also does not matter if it is > in effect the same as mv88e6185_g1_set_max_frame_size(). > > We should always make the driver understandably first, compact and > without redundancy second. We are then less likely to get into > situations like this again where it is not clear what MTU a device > actually supports because the code is cryptic. Ok, I will add new function. Thanks for hints. > > Andrew 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
On Fri, Mar 10, 2023 at 12:53:46PM +0100, Lukasz Majewski wrote: > Hi Andrew, > > > > > If I understand this correctly, in patch 4, you add a call to the > > > > 6250 family to call mv88e6185_g1_set_max_frame_size(), which sets > > > > a bit called MV88E6185_G1_CTL1_MAX_FRAME_1632 if the frame size > > > > is larger than 1518. > > > > > > Yes, correct. > > > > > > > > > > > However, you're saying that 6250 has a frame size of 2048. That's > > > > fine, but it makes MV88E6185_G1_CTL1_MAX_FRAME_1632 rather > > > > misleading as a definition. While the bit may increase the frame > > > > size, I think if we're going to do this, then this definition > > > > ought to be renamed. > > > > > > I thought about rename, but then I've double checked; register > > > offset and exact bit definition is the same as for 6185, so to avoid > > > unnecessary code duplication - I've reused the existing function. > > > > > > Maybe comment would be just enough? > > > > The driver takes care with its namespace in order to add per switch > > family defines. So you can add MV88E6250_G1_CTL1_MAX_FRAME_2048. It > > does not matter if it is the same bit. You can also add a > > mv88e6250_g1_set_max_frame_size() and it also does not matter if it is > > in effect the same as mv88e6185_g1_set_max_frame_size(). > > > > We should always make the driver understandably first, compact and > > without redundancy second. We are then less likely to get into > > situations like this again where it is not clear what MTU a device > > actually supports because the code is cryptic. > > Ok, I will add new function. > > Thanks for hints. It may be worth doing: static int mv88e6xxx_g1_modify(struct mv88e6xxx_chip *chip, int reg, u16 mask, u16 val) { int addr = chip->info->global1_addr; int err; u16 v; err = mv88e6xxx_read(chip, addr, reg, &v); if (err < 0) return err; v = (v & ~mask) | val; return mv88e6xxx_write(chip, addr, reg, v); } Then, mv88e6185_g1_set_max_frame_size() becomes: int mv88e6185_g1_set_max_frame_size(struct mv88e6xxx_chip *chip, int mtu) { u16 val = 0; if (mtu + ETH_HLEN + ETH_FCS_LEN > 1518) val = MV88E6185_G1_CTL1_MAX_FRAME_1632; return mv88e6xxx_g1_modify(chip, MV88E6XXX_G1_CTL1, MV88E6185_G1_CTL1_MAX_FRAME_1632, val); } The 6250 variant becomes similar. We can also think about converting all those other read-modify-writes to use mv88e6xxx_g1_modify(). The strange thing is... we already have mv88e6xxx_g1_ctl2_mask() which is an implementation of mv88e6xxx_g1_modify() specifically for MV88E6XXX_G1_CTL2 register, although it uses (val & mask) rather than just val. That wouldn't be necessary if the bitfield macros (e.g. FIELD_PREP() were used rather than explicit __bf_shf().
Hi Russell, > On Fri, Mar 10, 2023 at 12:53:46PM +0100, Lukasz Majewski wrote: > > Hi Andrew, > > > > > > > If I understand this correctly, in patch 4, you add a call to > > > > > the 6250 family to call mv88e6185_g1_set_max_frame_size(), > > > > > which sets a bit called MV88E6185_G1_CTL1_MAX_FRAME_1632 if > > > > > the frame size is larger than 1518. > > > > > > > > Yes, correct. > > > > > > > > > > > > > > However, you're saying that 6250 has a frame size of 2048. > > > > > That's fine, but it makes MV88E6185_G1_CTL1_MAX_FRAME_1632 > > > > > rather misleading as a definition. While the bit may increase > > > > > the frame size, I think if we're going to do this, then this > > > > > definition ought to be renamed. > > > > > > > > I thought about rename, but then I've double checked; register > > > > offset and exact bit definition is the same as for 6185, so to > > > > avoid unnecessary code duplication - I've reused the existing > > > > function. > > > > > > > > Maybe comment would be just enough? > > > > > > The driver takes care with its namespace in order to add per > > > switch family defines. So you can add > > > MV88E6250_G1_CTL1_MAX_FRAME_2048. It does not matter if it is the > > > same bit. You can also add a mv88e6250_g1_set_max_frame_size() > > > and it also does not matter if it is in effect the same as > > > mv88e6185_g1_set_max_frame_size(). > > > > > > We should always make the driver understandably first, compact and > > > without redundancy second. We are then less likely to get into > > > situations like this again where it is not clear what MTU a device > > > actually supports because the code is cryptic. > > > > Ok, I will add new function. > > > > Thanks for hints. > > It may be worth doing: > > static int mv88e6xxx_g1_modify(struct mv88e6xxx_chip *chip, int reg, > u16 mask, u16 val) > { > int addr = chip->info->global1_addr; > int err; > u16 v; > > err = mv88e6xxx_read(chip, addr, reg, &v); > if (err < 0) > return err; > > v = (v & ~mask) | val; > > return mv88e6xxx_write(chip, addr, reg, v); > } > > Then, mv88e6185_g1_set_max_frame_size() becomes: > > int mv88e6185_g1_set_max_frame_size(struct mv88e6xxx_chip *chip, int > mtu) { > u16 val = 0; > > if (mtu + ETH_HLEN + ETH_FCS_LEN > 1518) > val = MV88E6185_G1_CTL1_MAX_FRAME_1632; > > return mv88e6xxx_g1_modify(chip, MV88E6XXX_G1_CTL1, > MV88E6185_G1_CTL1_MAX_FRAME_1632, > val); } > Yes, correct. > The 6250 variant becomes similar. > > We can also think about converting all those other read-modify-writes > to use mv88e6xxx_g1_modify(). > > The strange thing is... we already have mv88e6xxx_g1_ctl2_mask() which > is an implementation of mv88e6xxx_g1_modify() specifically for > MV88E6XXX_G1_CTL2 register, although it uses (val & mask) rather than > just val. That wouldn't be necessary if the bitfield macros (e.g. > FIELD_PREP() were used rather than explicit __bf_shf(). > I do have the impression that major refactoring of the mv6xxx driver would be welcome... 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
On Fri, Mar 10, 2023 at 12:06:15PM +0000, Russell King (Oracle) wrote: > It may be worth doing: > > static int mv88e6xxx_g1_modify(struct mv88e6xxx_chip *chip, int reg, > u16 mask, u16 val) > { > int addr = chip->info->global1_addr; > int err; > u16 v; > > err = mv88e6xxx_read(chip, addr, reg, &v); > if (err < 0) > return err; > > v = (v & ~mask) | val; > > return mv88e6xxx_write(chip, addr, reg, v); > } > > Then, mv88e6185_g1_set_max_frame_size() becomes: > > int mv88e6185_g1_set_max_frame_size(struct mv88e6xxx_chip *chip, int mtu) > { > u16 val = 0; > > if (mtu + ETH_HLEN + ETH_FCS_LEN > 1518) > val = MV88E6185_G1_CTL1_MAX_FRAME_1632; > > return mv88e6xxx_g1_modify(chip, MV88E6XXX_G1_CTL1, > MV88E6185_G1_CTL1_MAX_FRAME_1632, val); > } > > The 6250 variant becomes similar. +1, sounds good to have separate mv88e6185_g1_set_max_frame_size() and mv88e6250_g1_set_max_frame_size() with a common implementation. It is a lot less confusing than the two driving a bit named in the same way but meaning different things.
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index af14eb8a1bfd..dbb69787f4ef 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -5669,7 +5669,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = { .num_internal_phys = 5, .max_vid = 4095, .max_sid = 63, - .max_frame_size = 1522, + .max_frame_size = 1632, .port_base_addr = 0x10, .phy_base_addr = 0x0, .global1_addr = 0x1b, @@ -5837,7 +5837,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = { .num_internal_phys = 0, .max_vid = 4095, .max_sid = 63, - .max_frame_size = 1632, + .max_frame_size = 1522, .port_base_addr = 0x10, .phy_base_addr = 0x0, .global1_addr = 0x1b, @@ -6012,7 +6012,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = { .num_gpio = 16, .max_vid = 8191, .max_sid = 63, - .max_frame_size = 1522, + .max_frame_size = 10240, .port_base_addr = 0x0, .phy_base_addr = 0x0, .global1_addr = 0x1b, @@ -6060,7 +6060,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = { .num_internal_phys = 9, .max_vid = 8191, .max_sid = 63, - .max_frame_size = 1522, + .max_frame_size = 10240, .port_base_addr = 0x0, .phy_base_addr = 0x0, .global1_addr = 0x1b, @@ -6084,7 +6084,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = { .num_internal_phys = 9, .max_vid = 8191, .max_sid = 63, - .max_frame_size = 1522, + .max_frame_size = 10240, .port_base_addr = 0x0, .phy_base_addr = 0x0, .global1_addr = 0x1b, @@ -7169,27 +7169,6 @@ static int __maybe_unused mv88e6xxx_resume(struct device *dev) static SIMPLE_DEV_PM_OPS(mv88e6xxx_pm_ops, mv88e6xxx_suspend, mv88e6xxx_resume); -static void mv88e6xxx_validate_frame_size(void) -{ - int max; - int i; - - for (i = 0; i < ARRAY_SIZE(mv88e6xxx_table); i++) { - /* same logic as in mv88e6xxx_get_max_mtu() */ - if (mv88e6xxx_table[i].ops->port_set_jumbo_size) - max = 10240; - else if (mv88e6xxx_table[i].ops->set_max_frame_size) - max = 1632; - else - max = 1522; - - if (mv88e6xxx_table[i].max_frame_size != max) - pr_err("BUG: %s has differing max_frame_size: %d != %d\n", - mv88e6xxx_table[i].name, max, - mv88e6xxx_table[i].max_frame_size); - } -} - static int mv88e6xxx_probe(struct mdio_device *mdiodev) { struct dsa_mv88e6xxx_pdata *pdata = mdiodev->dev.platform_data; @@ -7323,7 +7302,6 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev) if (err) goto out_mdio; - mv88e6xxx_validate_frame_size(); return 0; out_mdio:
Running of the mv88e6xxx_validate_frame_size() function provided following results: [ 1.585565] BUG: Marvell 88E6020 has differing max_frame_size: 1632 != 2048 [ 1.592540] BUG: Marvell 88E6071 has differing max_frame_size: 1632 != 2048 ^------ Correct -> mv88e6250 family max frame size = 2048B [ 1.599507] BUG: Marvell 88E6085 has differing max_frame_size: 1632 != 1522 [ 1.606476] BUG: Marvell 88E6165 has differing max_frame_size: 1522 != 1632 [ 1.613445] BUG: Marvell 88E6190X has differing max_frame_size: 10240 != 1522 [ 1.620590] BUG: Marvell 88E6191X has differing max_frame_size: 10240 != 1522 [ 1.627730] BUG: Marvell 88E6193X has differing max_frame_size: 10240 != 1522 ^------ Needs to be fixed!!! [ 1.634871] BUG: Marvell 88E6220 has differing max_frame_size: 1632 != 2048 [ 1.641842] BUG: Marvell 88E6250 has differing max_frame_size: 1632 != 2048 ^------ Correct -> mv88e6250 family max frame size = 2048B This commit removes the validation function and provides correct values for the max frame size field. Signed-off-by: Lukasz Majewski <lukma@denx.de> --- Changes for v5: - New patch --- drivers/net/dsa/mv88e6xxx/chip.c | 32 +++++--------------------------- 1 file changed, 5 insertions(+), 27 deletions(-)