| Message ID | 20221108082330.2086671-7-lukma@denx.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: Add support for mv88e6020 and mv88e6071 | expand |
On Tue, Nov 08, 2022 at 09:23:27AM +0100, Lukasz Majewski wrote: > Different Marvell DSA switches support different size of max frame > bytes to be sent. > > For example mv88e6185 supports max 1632B, which is now in-driver > standard value. On the other hand - mv88e6071 supports 2048 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. > > When the 'max_frame_size' is not defined (and hence zeroed by > the kvzalloc()) the default of 1632 bytes is used. I would prefer every entry states the value. That both simplifies the code, and probably reduces the likelihood of somebody forgetting to set it when adding a new chip. Andrew
Hi Andrew, > On Tue, Nov 08, 2022 at 09:23:27AM +0100, Lukasz Majewski wrote: > > Different Marvell DSA switches support different size of max frame > > bytes to be sent. > > > > For example mv88e6185 supports max 1632B, which is now in-driver > > standard value. On the other hand - mv88e6071 supports 2048 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. > > > > When the 'max_frame_size' is not defined (and hence zeroed by > > the kvzalloc()) the default of 1632 bytes is used. > > I would prefer every entry states the value. You mean to add it explicitly (i.e. for each supported switch version) to the struct mv88e6xxx_ops ? > That both simplifies the > code, and probably reduces the likelihood of somebody forgetting to > set it when adding a new chip. Ok. > > Andrew Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk 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, Nov 10, 2022 at 04:36:37PM +0100, Lukasz Majewski wrote: > Hi Andrew, > > > On Tue, Nov 08, 2022 at 09:23:27AM +0100, Lukasz Majewski wrote: > > > Different Marvell DSA switches support different size of max frame > > > bytes to be sent. > > > > > > For example mv88e6185 supports max 1632B, which is now in-driver > > > standard value. On the other hand - mv88e6071 supports 2048 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. > > > > > > When the 'max_frame_size' is not defined (and hence zeroed by > > > the kvzalloc()) the default of 1632 bytes is used. > > > > I would prefer every entry states the value. > > You mean to add it explicitly (i.e. for each supported switch version) > to the struct mv88e6xxx_ops ? To the info structure. You added it for just your devices and left it to 0 for the rest. Rather than special case 0, always set the value and remove the special case. Maybe even -EINVAL if you find a 0 there. Andrew
Hi Andrew, > On Thu, Nov 10, 2022 at 04:36:37PM +0100, Lukasz Majewski wrote: > > Hi Andrew, > > > > > On Tue, Nov 08, 2022 at 09:23:27AM +0100, Lukasz Majewski wrote: > > > > Different Marvell DSA switches support different size of max > > > > frame bytes to be sent. > > > > > > > > For example mv88e6185 supports max 1632B, which is now in-driver > > > > standard value. On the other hand - mv88e6071 supports 2048 > > > > 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. > > > > > > > > When the 'max_frame_size' is not defined (and hence zeroed by > > > > the kvzalloc()) the default of 1632 bytes is used. > > > > > > I would prefer every entry states the value. > > > > You mean to add it explicitly (i.e. for each supported switch > > version) to the struct mv88e6xxx_ops ? > > To the info structure. You added it for just your devices and left it > to 0 for the rest. Rather than special case 0, always set the value > and remove the special case. Maybe even -EINVAL if you find a 0 there. > Ok. > Andrew Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index 09877a464665..d90835b4c606 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -3542,11 +3542,19 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port) static int mv88e6xxx_get_max_mtu(struct dsa_switch *ds, int port) { struct mv88e6xxx_chip *chip = ds->priv; + int max_frame_size; 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; + else if (chip->info->ops->set_max_frame_size) { + if (chip->info->max_frame_size) + max_frame_size = chip->info->max_frame_size; + else + max_frame_size = 1632; + + return max_frame_size - VLAN_ETH_HLEN - EDSA_HLEN - ETH_FCS_LEN; + } + return 1522 - VLAN_ETH_HLEN - EDSA_HLEN - ETH_FCS_LEN; } diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h index 2fcab41e03b7..6ec4010fd634 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.h +++ b/drivers/net/dsa/mv88e6xxx/chip.h @@ -132,6 +132,7 @@ struct mv88e6xxx_info { unsigned int num_ports; unsigned int num_internal_phys; unsigned int num_gpio; + unsigned int max_frame_size; unsigned int max_vid; unsigned int max_sid; unsigned int port_base_addr;
Different Marvell DSA switches support different size of max frame bytes to be sent. For example mv88e6185 supports max 1632B, which is now in-driver standard value. On the other hand - mv88e6071 supports 2048 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. When the 'max_frame_size' is not defined (and hence zeroed by the kvzalloc()) the default of 1632 bytes is used. Signed-off-by: Lukasz Majewski <lukma@denx.de> --- drivers/net/dsa/mv88e6xxx/chip.c | 12 ++++++++++-- drivers/net/dsa/mv88e6xxx/chip.h | 1 + 2 files changed, 11 insertions(+), 2 deletions(-)