* [PATCH net-next 0/4] net: dsa: b53: Clean up CPU/IMP ports
@ 2021-09-16 12:03 Rafał Miłecki
2021-09-16 12:03 ` [PATCH net-next 1/4] net: dsa: b53: Include all ports in "enabled_ports" Rafał Miłecki
` (5 more replies)
0 siblings, 6 replies; 17+ messages in thread
From: Rafał Miłecki @ 2021-09-16 12:03 UTC (permalink / raw)
To: Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S . Miller,
Jakub Kicinski
Cc: Florian Fainelli, netdev, bcm-kernel-feedback-list,
Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
This has been tested on:
1. Luxul XBR-4500 with used CPU port 5
[ 8.361438] b53-srab-switch 18007000.ethernet-switch: found switch: BCM53012, rev 0
2. Netgear R8000 with used CPU port 8
[ 4.453858] b53-srab-switch 18007000.ethernet-switch: found switch: BCM53012, rev 5
Rafał Miłecki (4):
net: dsa: b53: Include all ports in "enabled_ports"
net: dsa: b53: Drop BCM5301x workaround for a wrong CPU/IMP port
net: dsa: b53: Improve flow control setup on BCM5301x
net: dsa: b53: Drop unused "cpu_port" field
drivers/net/dsa/b53/b53_common.c | 59 +++++++-------------------------
drivers/net/dsa/b53/b53_priv.h | 1 -
2 files changed, 12 insertions(+), 48 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH net-next 1/4] net: dsa: b53: Include all ports in "enabled_ports" 2021-09-16 12:03 [PATCH net-next 0/4] net: dsa: b53: Clean up CPU/IMP ports Rafał Miłecki @ 2021-09-16 12:03 ` Rafał Miłecki 2021-09-16 21:44 ` Florian Fainelli 2021-09-16 12:03 ` [PATCH net-next 2/4] net: dsa: b53: Drop BCM5301x workaround for a wrong CPU/IMP port Rafał Miłecki ` (4 subsequent siblings) 5 siblings, 1 reply; 17+ messages in thread From: Rafał Miłecki @ 2021-09-16 12:03 UTC (permalink / raw) To: Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S . Miller, Jakub Kicinski Cc: Florian Fainelli, netdev, bcm-kernel-feedback-list, Rafał Miłecki From: Rafał Miłecki <rafal@milecki.pl> Make "enabled_ports" bitfield contain all available switch ports including a CPU port. This way there is no need for fixup during initialization. For BCM53010, BCM53018 and BCM53019 include also other available ports. Signed-off-by: Rafał Miłecki <rafal@milecki.pl> --- drivers/net/dsa/b53/b53_common.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c index 604f54112665..47a00c5364c7 100644 --- a/drivers/net/dsa/b53/b53_common.c +++ b/drivers/net/dsa/b53/b53_common.c @@ -2302,7 +2302,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .chip_id = BCM5325_DEVICE_ID, .dev_name = "BCM5325", .vlans = 16, - .enabled_ports = 0x1f, + .enabled_ports = 0x3f, .arl_bins = 2, .arl_buckets = 1024, .imp_port = 5, @@ -2313,7 +2313,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .chip_id = BCM5365_DEVICE_ID, .dev_name = "BCM5365", .vlans = 256, - .enabled_ports = 0x1f, + .enabled_ports = 0x3f, .arl_bins = 2, .arl_buckets = 1024, .imp_port = 5, @@ -2324,7 +2324,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .chip_id = BCM5389_DEVICE_ID, .dev_name = "BCM5389", .vlans = 4096, - .enabled_ports = 0x1f, + .enabled_ports = 0x11f, .arl_bins = 4, .arl_buckets = 1024, .imp_port = 8, @@ -2338,7 +2338,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .chip_id = BCM5395_DEVICE_ID, .dev_name = "BCM5395", .vlans = 4096, - .enabled_ports = 0x1f, + .enabled_ports = 0x11f, .arl_bins = 4, .arl_buckets = 1024, .imp_port = 8, @@ -2352,7 +2352,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .chip_id = BCM5397_DEVICE_ID, .dev_name = "BCM5397", .vlans = 4096, - .enabled_ports = 0x1f, + .enabled_ports = 0x11f, .arl_bins = 4, .arl_buckets = 1024, .imp_port = 8, @@ -2366,7 +2366,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .chip_id = BCM5398_DEVICE_ID, .dev_name = "BCM5398", .vlans = 4096, - .enabled_ports = 0x7f, + .enabled_ports = 0x17f, .arl_bins = 4, .arl_buckets = 1024, .imp_port = 8, @@ -2380,7 +2380,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .chip_id = BCM53115_DEVICE_ID, .dev_name = "BCM53115", .vlans = 4096, - .enabled_ports = 0x1f, + .enabled_ports = 0x11f, .arl_bins = 4, .arl_buckets = 1024, .vta_regs = B53_VTA_REGS, @@ -2394,7 +2394,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .chip_id = BCM53125_DEVICE_ID, .dev_name = "BCM53125", .vlans = 4096, - .enabled_ports = 0xff, + .enabled_ports = 0x1ff, .arl_bins = 4, .arl_buckets = 1024, .imp_port = 8, @@ -2436,7 +2436,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .chip_id = BCM53010_DEVICE_ID, .dev_name = "BCM53010", .vlans = 4096, - .enabled_ports = 0x1f, + .enabled_ports = 0x1bf, .arl_bins = 4, .arl_buckets = 1024, .imp_port = 8, @@ -2478,7 +2478,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .chip_id = BCM53018_DEVICE_ID, .dev_name = "BCM53018", .vlans = 4096, - .enabled_ports = 0x1f, + .enabled_ports = 0x1bf, .arl_bins = 4, .arl_buckets = 1024, .imp_port = 8, @@ -2492,7 +2492,7 @@ static const struct b53_chip_data b53_switch_chips[] = { .chip_id = BCM53019_DEVICE_ID, .dev_name = "BCM53019", .vlans = 4096, - .enabled_ports = 0x1f, + .enabled_ports = 0x1bf, .arl_bins = 4, .arl_buckets = 1024, .imp_port = 8, @@ -2634,7 +2634,6 @@ static int b53_switch_init(struct b53_device *dev) dev->cpu_port = 5; } - dev->enabled_ports |= BIT(dev->cpu_port); dev->num_ports = fls(dev->enabled_ports); dev->ds->num_ports = min_t(unsigned int, dev->num_ports, DSA_MAX_PORTS); -- 2.26.2 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 1/4] net: dsa: b53: Include all ports in "enabled_ports" 2021-09-16 12:03 ` [PATCH net-next 1/4] net: dsa: b53: Include all ports in "enabled_ports" Rafał Miłecki @ 2021-09-16 21:44 ` Florian Fainelli 0 siblings, 0 replies; 17+ messages in thread From: Florian Fainelli @ 2021-09-16 21:44 UTC (permalink / raw) To: Rafał Miłecki, Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S . Miller, Jakub Kicinski Cc: netdev, bcm-kernel-feedback-list, Rafał Miłecki On 9/16/21 5:03 AM, Rafał Miłecki wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > Make "enabled_ports" bitfield contain all available switch ports > including a CPU port. This way there is no need for fixup during > initialization. > > For BCM53010, BCM53018 and BCM53019 include also other available ports. > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> Tested-by: Florian Fainelli <f.fainelli@gmail.com> -- Florian ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next 2/4] net: dsa: b53: Drop BCM5301x workaround for a wrong CPU/IMP port 2021-09-16 12:03 [PATCH net-next 0/4] net: dsa: b53: Clean up CPU/IMP ports Rafał Miłecki 2021-09-16 12:03 ` [PATCH net-next 1/4] net: dsa: b53: Include all ports in "enabled_ports" Rafał Miłecki @ 2021-09-16 12:03 ` Rafał Miłecki 2021-09-16 21:44 ` Florian Fainelli 2021-09-16 12:03 ` [PATCH net-next 3/4] net: dsa: b53: Improve flow control setup on BCM5301x Rafał Miłecki ` (3 subsequent siblings) 5 siblings, 1 reply; 17+ messages in thread From: Rafał Miłecki @ 2021-09-16 12:03 UTC (permalink / raw) To: Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S . Miller, Jakub Kicinski Cc: Florian Fainelli, netdev, bcm-kernel-feedback-list, Rafał Miłecki From: Rafał Miłecki <rafal@milecki.pl> On BCM5301x port 8 requires a fixed link when used. Years ago when b53 was an OpenWrt downstream driver (with configuration based on sometimes bugged NVRAM) there was a need for a fixup. In case of forcing fixed link for (incorrectly specified) port 5 the code had to actually setup port 8 link. For upstream b53 driver with setup based on DT there is no need for that workaround. In DT we have and require correct ports setup. Signed-off-by: Rafał Miłecki <rafal@milecki.pl> --- drivers/net/dsa/b53/b53_common.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c index 47a00c5364c7..ca84e32baca0 100644 --- a/drivers/net/dsa/b53/b53_common.c +++ b/drivers/net/dsa/b53/b53_common.c @@ -1291,12 +1291,6 @@ static void b53_adjust_link(struct dsa_switch *ds, int port, return; } } - } else if (is5301x(dev)) { - if (port != dev->cpu_port) { - b53_force_port_config(dev, dev->cpu_port, 2000, - DUPLEX_FULL, true, true); - b53_force_link(dev, dev->cpu_port, 1); - } } /* Re-negotiate EEE if it was enabled already */ -- 2.26.2 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 2/4] net: dsa: b53: Drop BCM5301x workaround for a wrong CPU/IMP port 2021-09-16 12:03 ` [PATCH net-next 2/4] net: dsa: b53: Drop BCM5301x workaround for a wrong CPU/IMP port Rafał Miłecki @ 2021-09-16 21:44 ` Florian Fainelli 0 siblings, 0 replies; 17+ messages in thread From: Florian Fainelli @ 2021-09-16 21:44 UTC (permalink / raw) To: Rafał Miłecki, Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S . Miller, Jakub Kicinski Cc: netdev, bcm-kernel-feedback-list, Rafał Miłecki On 9/16/21 5:03 AM, Rafał Miłecki wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > On BCM5301x port 8 requires a fixed link when used. > > Years ago when b53 was an OpenWrt downstream driver (with configuration > based on sometimes bugged NVRAM) there was a need for a fixup. In case > of forcing fixed link for (incorrectly specified) port 5 the code had to > actually setup port 8 link. > > For upstream b53 driver with setup based on DT there is no need for that > workaround. In DT we have and require correct ports setup. > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> Tested-by: Florian Fainelli <f.fainelli@gmail.com> -- Florian ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next 3/4] net: dsa: b53: Improve flow control setup on BCM5301x 2021-09-16 12:03 [PATCH net-next 0/4] net: dsa: b53: Clean up CPU/IMP ports Rafał Miłecki 2021-09-16 12:03 ` [PATCH net-next 1/4] net: dsa: b53: Include all ports in "enabled_ports" Rafał Miłecki 2021-09-16 12:03 ` [PATCH net-next 2/4] net: dsa: b53: Drop BCM5301x workaround for a wrong CPU/IMP port Rafał Miłecki @ 2021-09-16 12:03 ` Rafał Miłecki 2021-09-16 21:44 ` Florian Fainelli 2021-09-16 12:03 ` [PATCH net-next 4/4] net: dsa: b53: Drop unused "cpu_port" field Rafał Miłecki ` (2 subsequent siblings) 5 siblings, 1 reply; 17+ messages in thread From: Rafał Miłecki @ 2021-09-16 12:03 UTC (permalink / raw) To: Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S . Miller, Jakub Kicinski Cc: Florian Fainelli, netdev, bcm-kernel-feedback-list, Rafał Miłecki From: Rafał Miłecki <rafal@milecki.pl> According to the Broadcom's reference driver flow control needs to be enabled for any CPU switch port (5, 7 or 8 - depending on which one is used). Current code makes it work only for the port 5. Use dsa_is_cpu_port() which solved that problem. Signed-off-by: Rafał Miłecki <rafal@milecki.pl> --- drivers/net/dsa/b53/b53_common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c index ca84e32baca0..13f337a102ba 100644 --- a/drivers/net/dsa/b53/b53_common.c +++ b/drivers/net/dsa/b53/b53_common.c @@ -1222,7 +1222,7 @@ static void b53_adjust_link(struct dsa_switch *ds, int port, return; /* Enable flow control on BCM5301x's CPU port */ - if (is5301x(dev) && port == dev->cpu_port) + if (is5301x(dev) && dsa_is_cpu_port(ds, port)) tx_pause = rx_pause = true; if (phydev->pause) { -- 2.26.2 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 3/4] net: dsa: b53: Improve flow control setup on BCM5301x 2021-09-16 12:03 ` [PATCH net-next 3/4] net: dsa: b53: Improve flow control setup on BCM5301x Rafał Miłecki @ 2021-09-16 21:44 ` Florian Fainelli 0 siblings, 0 replies; 17+ messages in thread From: Florian Fainelli @ 2021-09-16 21:44 UTC (permalink / raw) To: Rafał Miłecki, Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S . Miller, Jakub Kicinski Cc: netdev, bcm-kernel-feedback-list, Rafał Miłecki On 9/16/21 5:03 AM, Rafał Miłecki wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > According to the Broadcom's reference driver flow control needs to be > enabled for any CPU switch port (5, 7 or 8 - depending on which one is > used). Current code makes it work only for the port 5. Use > dsa_is_cpu_port() which solved that problem. > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> Tested-by: Florian Fainelli <f.fainelli@gmail.com> -- Florian ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next 4/4] net: dsa: b53: Drop unused "cpu_port" field 2021-09-16 12:03 [PATCH net-next 0/4] net: dsa: b53: Clean up CPU/IMP ports Rafał Miłecki ` (2 preceding siblings ...) 2021-09-16 12:03 ` [PATCH net-next 3/4] net: dsa: b53: Improve flow control setup on BCM5301x Rafał Miłecki @ 2021-09-16 12:03 ` Rafał Miłecki 2021-09-16 21:44 ` Florian Fainelli 2021-09-16 16:23 ` [PATCH net-next 0/4] net: dsa: b53: Clean up CPU/IMP ports Florian Fainelli 2021-09-17 2:58 ` Jakub Kicinski 5 siblings, 1 reply; 17+ messages in thread From: Rafał Miłecki @ 2021-09-16 12:03 UTC (permalink / raw) To: Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S . Miller, Jakub Kicinski Cc: Florian Fainelli, netdev, bcm-kernel-feedback-list, Rafał Miłecki From: Rafał Miłecki <rafal@milecki.pl> It's set but never used anymore. Signed-off-by: Rafał Miłecki <rafal@milecki.pl> --- drivers/net/dsa/b53/b53_common.c | 28 ---------------------------- drivers/net/dsa/b53/b53_priv.h | 1 - 2 files changed, 29 deletions(-) diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c index 13f337a102ba..06279ba64cc8 100644 --- a/drivers/net/dsa/b53/b53_common.c +++ b/drivers/net/dsa/b53/b53_common.c @@ -2300,7 +2300,6 @@ static const struct b53_chip_data b53_switch_chips[] = { .arl_bins = 2, .arl_buckets = 1024, .imp_port = 5, - .cpu_port = B53_CPU_PORT_25, .duplex_reg = B53_DUPLEX_STAT_FE, }, { @@ -2311,7 +2310,6 @@ static const struct b53_chip_data b53_switch_chips[] = { .arl_bins = 2, .arl_buckets = 1024, .imp_port = 5, - .cpu_port = B53_CPU_PORT_25, .duplex_reg = B53_DUPLEX_STAT_FE, }, { @@ -2322,7 +2320,6 @@ static const struct b53_chip_data b53_switch_chips[] = { .arl_bins = 4, .arl_buckets = 1024, .imp_port = 8, - .cpu_port = B53_CPU_PORT, .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, .jumbo_pm_reg = B53_JUMBO_PORT_MASK, @@ -2336,7 +2333,6 @@ static const struct b53_chip_data b53_switch_chips[] = { .arl_bins = 4, .arl_buckets = 1024, .imp_port = 8, - .cpu_port = B53_CPU_PORT, .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, .jumbo_pm_reg = B53_JUMBO_PORT_MASK, @@ -2350,7 +2346,6 @@ static const struct b53_chip_data b53_switch_chips[] = { .arl_bins = 4, .arl_buckets = 1024, .imp_port = 8, - .cpu_port = B53_CPU_PORT, .vta_regs = B53_VTA_REGS_9798, .duplex_reg = B53_DUPLEX_STAT_GE, .jumbo_pm_reg = B53_JUMBO_PORT_MASK, @@ -2364,7 +2359,6 @@ static const struct b53_chip_data b53_switch_chips[] = { .arl_bins = 4, .arl_buckets = 1024, .imp_port = 8, - .cpu_port = B53_CPU_PORT, .vta_regs = B53_VTA_REGS_9798, .duplex_reg = B53_DUPLEX_STAT_GE, .jumbo_pm_reg = B53_JUMBO_PORT_MASK, @@ -2379,7 +2373,6 @@ static const struct b53_chip_data b53_switch_chips[] = { .arl_buckets = 1024, .vta_regs = B53_VTA_REGS, .imp_port = 8, - .cpu_port = B53_CPU_PORT, .duplex_reg = B53_DUPLEX_STAT_GE, .jumbo_pm_reg = B53_JUMBO_PORT_MASK, .jumbo_size_reg = B53_JUMBO_MAX_SIZE, @@ -2392,7 +2385,6 @@ static const struct b53_chip_data b53_switch_chips[] = { .arl_bins = 4, .arl_buckets = 1024, .imp_port = 8, - .cpu_port = B53_CPU_PORT, .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, .jumbo_pm_reg = B53_JUMBO_PORT_MASK, @@ -2406,7 +2398,6 @@ static const struct b53_chip_data b53_switch_chips[] = { .arl_bins = 4, .arl_buckets = 1024, .imp_port = 8, - .cpu_port = B53_CPU_PORT, .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, .jumbo_pm_reg = B53_JUMBO_PORT_MASK, @@ -2420,7 +2411,6 @@ static const struct b53_chip_data b53_switch_chips[] = { .arl_bins = 4, .arl_buckets = 1024, .imp_port = 8, - .cpu_port = B53_CPU_PORT, .vta_regs = B53_VTA_REGS_63XX, .duplex_reg = B53_DUPLEX_STAT_63XX, .jumbo_pm_reg = B53_JUMBO_PORT_MASK_63XX, @@ -2434,7 +2424,6 @@ static const struct b53_chip_data b53_switch_chips[] = { .arl_bins = 4, .arl_buckets = 1024, .imp_port = 8, - .cpu_port = B53_CPU_PORT_25, /* TODO: auto detect */ .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, .jumbo_pm_reg = B53_JUMBO_PORT_MASK, @@ -2448,7 +2437,6 @@ static const struct b53_chip_data b53_switch_chips[] = { .arl_bins = 4, .arl_buckets = 1024, .imp_port = 8, - .cpu_port = B53_CPU_PORT_25, /* TODO: auto detect */ .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, .jumbo_pm_reg = B53_JUMBO_PORT_MASK, @@ -2462,7 +2450,6 @@ static const struct b53_chip_data b53_switch_chips[] = { .arl_bins = 4, .arl_buckets = 1024, .imp_port = 8, - .cpu_port = B53_CPU_PORT_25, /* TODO: auto detect */ .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, .jumbo_pm_reg = B53_JUMBO_PORT_MASK, @@ -2476,7 +2463,6 @@ static const struct b53_chip_data b53_switch_chips[] = { .arl_bins = 4, .arl_buckets = 1024, .imp_port = 8, - .cpu_port = B53_CPU_PORT_25, /* TODO: auto detect */ .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, .jumbo_pm_reg = B53_JUMBO_PORT_MASK, @@ -2490,7 +2476,6 @@ static const struct b53_chip_data b53_switch_chips[] = { .arl_bins = 4, .arl_buckets = 1024, .imp_port = 8, - .cpu_port = B53_CPU_PORT_25, /* TODO: auto detect */ .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, .jumbo_pm_reg = B53_JUMBO_PORT_MASK, @@ -2504,7 +2489,6 @@ static const struct b53_chip_data b53_switch_chips[] = { .arl_bins = 4, .arl_buckets = 1024, .imp_port = 8, - .cpu_port = B53_CPU_PORT, .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, .jumbo_pm_reg = B53_JUMBO_PORT_MASK, @@ -2518,7 +2502,6 @@ static const struct b53_chip_data b53_switch_chips[] = { .arl_bins = 4, .arl_buckets = 1024, .imp_port = 8, - .cpu_port = B53_CPU_PORT, .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, .jumbo_pm_reg = B53_JUMBO_PORT_MASK, @@ -2533,7 +2516,6 @@ static const struct b53_chip_data b53_switch_chips[] = { .arl_bins = 4, .arl_buckets = 256, .imp_port = 8, - .cpu_port = 8, /* TODO: ports 4, 5, 8 */ .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, .jumbo_pm_reg = B53_JUMBO_PORT_MASK, @@ -2547,7 +2529,6 @@ static const struct b53_chip_data b53_switch_chips[] = { .arl_bins = 4, .arl_buckets = 1024, .imp_port = 8, - .cpu_port = B53_CPU_PORT, .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, .jumbo_pm_reg = B53_JUMBO_PORT_MASK, @@ -2561,7 +2542,6 @@ static const struct b53_chip_data b53_switch_chips[] = { .arl_bins = 4, .arl_buckets = 256, .imp_port = 8, - .cpu_port = B53_CPU_PORT, .vta_regs = B53_VTA_REGS, .duplex_reg = B53_DUPLEX_STAT_GE, .jumbo_pm_reg = B53_JUMBO_PORT_MASK, @@ -2587,7 +2567,6 @@ static int b53_switch_init(struct b53_device *dev) dev->vta_regs[2] = chip->vta_regs[2]; dev->jumbo_pm_reg = chip->jumbo_pm_reg; dev->imp_port = chip->imp_port; - dev->cpu_port = chip->cpu_port; dev->num_vlans = chip->vlans; dev->num_arl_bins = chip->arl_bins; dev->num_arl_buckets = chip->arl_buckets; @@ -2619,13 +2598,6 @@ static int b53_switch_init(struct b53_device *dev) break; #endif } - } else if (dev->chip_id == BCM53115_DEVICE_ID) { - u64 strap_value; - - b53_read48(dev, B53_STAT_PAGE, B53_STRAP_VALUE, &strap_value); - /* use second IMP port if GMII is enabled */ - if (strap_value & SV_GMII_CTRL_115) - dev->cpu_port = 5; } dev->num_ports = fls(dev->enabled_ports); diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h index 5d068acf7cf8..5be8fe000037 100644 --- a/drivers/net/dsa/b53/b53_priv.h +++ b/drivers/net/dsa/b53/b53_priv.h @@ -124,7 +124,6 @@ struct b53_device { /* used ports mask */ u16 enabled_ports; unsigned int imp_port; - unsigned int cpu_port; /* connect specific data */ u8 current_page; -- 2.26.2 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 4/4] net: dsa: b53: Drop unused "cpu_port" field 2021-09-16 12:03 ` [PATCH net-next 4/4] net: dsa: b53: Drop unused "cpu_port" field Rafał Miłecki @ 2021-09-16 21:44 ` Florian Fainelli 0 siblings, 0 replies; 17+ messages in thread From: Florian Fainelli @ 2021-09-16 21:44 UTC (permalink / raw) To: Rafał Miłecki, Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S . Miller, Jakub Kicinski Cc: netdev, bcm-kernel-feedback-list, Rafał Miłecki On 9/16/21 5:03 AM, Rafał Miłecki wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > It's set but never used anymore. > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> Tested-by: Florian Fainelli <f.fainelli@gmail.com> -- Florian ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 0/4] net: dsa: b53: Clean up CPU/IMP ports 2021-09-16 12:03 [PATCH net-next 0/4] net: dsa: b53: Clean up CPU/IMP ports Rafał Miłecki ` (3 preceding siblings ...) 2021-09-16 12:03 ` [PATCH net-next 4/4] net: dsa: b53: Drop unused "cpu_port" field Rafał Miłecki @ 2021-09-16 16:23 ` Florian Fainelli 2021-09-16 21:46 ` Florian Fainelli 2021-09-17 2:58 ` Jakub Kicinski 5 siblings, 1 reply; 17+ messages in thread From: Florian Fainelli @ 2021-09-16 16:23 UTC (permalink / raw) To: Rafał Miłecki, Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S . Miller, Jakub Kicinski Cc: netdev, bcm-kernel-feedback-list, Rafał Miłecki On 9/16/21 5:03 AM, Rafał Miłecki wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > This has been tested on: > > 1. Luxul XBR-4500 with used CPU port 5 > [ 8.361438] b53-srab-switch 18007000.ethernet-switch: found switch: BCM53012, rev 0 > > 2. Netgear R8000 with used CPU port 8 > [ 4.453858] b53-srab-switch 18007000.ethernet-switch: found switch: BCM53012, rev 5 These look good at first glance, let me give them a try on 7445 and 7278 at least before responding with Reviewed-by/Tested-by tags, thanks! -- Florian ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 0/4] net: dsa: b53: Clean up CPU/IMP ports 2021-09-16 16:23 ` [PATCH net-next 0/4] net: dsa: b53: Clean up CPU/IMP ports Florian Fainelli @ 2021-09-16 21:46 ` Florian Fainelli 2021-09-16 22:19 ` Rafał Miłecki 0 siblings, 1 reply; 17+ messages in thread From: Florian Fainelli @ 2021-09-16 21:46 UTC (permalink / raw) To: Rafał Miłecki, Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S . Miller, Jakub Kicinski Cc: netdev, bcm-kernel-feedback-list, Rafał Miłecki On 9/16/21 9:23 AM, Florian Fainelli wrote: > On 9/16/21 5:03 AM, Rafał Miłecki wrote: >> From: Rafał Miłecki <rafal@milecki.pl> >> >> This has been tested on: >> >> 1. Luxul XBR-4500 with used CPU port 5 >> [ 8.361438] b53-srab-switch 18007000.ethernet-switch: found switch: BCM53012, rev 0 >> >> 2. Netgear R8000 with used CPU port 8 >> [ 4.453858] b53-srab-switch 18007000.ethernet-switch: found switch: BCM53012, rev 5 > > These look good at first glance, let me give them a try on 7445 and 7278 > at least before responding with Reviewed-by/Tested-by tags, thanks! > Found some issues on 7445 and 7278 while moving to the latest net-next which I will be addressing but this worked nicely. What do you think about removing dev->enabled_ports and b53_for_each_port entirely and using a DSA helper that iterates over the switch's port list? Now that we have dev->num_ports accurately reflect the number of ports it should be equivalent. -- Florian ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 0/4] net: dsa: b53: Clean up CPU/IMP ports 2021-09-16 21:46 ` Florian Fainelli @ 2021-09-16 22:19 ` Rafał Miłecki 2021-09-17 10:00 ` Vladimir Oltean 0 siblings, 1 reply; 17+ messages in thread From: Rafał Miłecki @ 2021-09-16 22:19 UTC (permalink / raw) To: Florian Fainelli, Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S . Miller, Jakub Kicinski Cc: netdev, bcm-kernel-feedback-list, Rafał Miłecki On 16.09.2021 23:46, Florian Fainelli wrote: > On 9/16/21 9:23 AM, Florian Fainelli wrote: >> On 9/16/21 5:03 AM, Rafał Miłecki wrote: >>> From: Rafał Miłecki <rafal@milecki.pl> >>> >>> This has been tested on: >>> >>> 1. Luxul XBR-4500 with used CPU port 5 >>> [ 8.361438] b53-srab-switch 18007000.ethernet-switch: found switch: BCM53012, rev 0 >>> >>> 2. Netgear R8000 with used CPU port 8 >>> [ 4.453858] b53-srab-switch 18007000.ethernet-switch: found switch: BCM53012, rev 5 >> >> These look good at first glance, let me give them a try on 7445 and 7278 >> at least before responding with Reviewed-by/Tested-by tags, thanks! >> > Found some issues on 7445 and 7278 while moving to the latest net-next > which I will be addressing but this worked nicely. > > What do you think about removing dev->enabled_ports and > b53_for_each_port entirely and using a DSA helper that iterates over the > switch's port list? Now that we have dev->num_ports accurately reflect > the number of ports it should be equivalent. The limitation I see in DSA is skipping unavailable ports. E.g. BCM5301x switches that don't have port 6. The closest match for such case I found is DSA_PORT_TYPE_UNUSED but I'm not sure if it's enough to handle those cases. That DSA_PORT_TYPE_UNUSED would probably require investigating DSA & b53 behaviour *and* discussing it with DSA maintainer to make sure we don't abuse that. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 0/4] net: dsa: b53: Clean up CPU/IMP ports 2021-09-16 22:19 ` Rafał Miłecki @ 2021-09-17 10:00 ` Vladimir Oltean 2021-09-17 12:21 ` Andrew Lunn 2021-09-17 16:39 ` Florian Fainelli 0 siblings, 2 replies; 17+ messages in thread From: Vladimir Oltean @ 2021-09-17 10:00 UTC (permalink / raw) To: Rafał Miłecki Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, David S . Miller, Jakub Kicinski, netdev, bcm-kernel-feedback-list, Rafał Miłecki On Fri, Sep 17, 2021 at 12:19:02AM +0200, Rafał Miłecki wrote: > On 16.09.2021 23:46, Florian Fainelli wrote: > > On 9/16/21 9:23 AM, Florian Fainelli wrote: > > > On 9/16/21 5:03 AM, Rafał Miłecki wrote: > > > > From: Rafał Miłecki <rafal@milecki.pl> > > > > > > > > This has been tested on: > > > > > > > > 1. Luxul XBR-4500 with used CPU port 5 > > > > [ 8.361438] b53-srab-switch 18007000.ethernet-switch: found switch: BCM53012, rev 0 > > > > > > > > 2. Netgear R8000 with used CPU port 8 > > > > [ 4.453858] b53-srab-switch 18007000.ethernet-switch: found switch: BCM53012, rev 5 > > > > > > These look good at first glance, let me give them a try on 7445 and 7278 > > > at least before responding with Reviewed-by/Tested-by tags, thanks! > > > > > Found some issues on 7445 and 7278 while moving to the latest net-next > > which I will be addressing but this worked nicely. > > > > What do you think about removing dev->enabled_ports and > > b53_for_each_port entirely and using a DSA helper that iterates over the > > switch's port list? Now that we have dev->num_ports accurately reflect > > the number of ports it should be equivalent. > > The limitation I see in DSA is skipping unavailable ports. E.g. BCM5301x > switches that don't have port 6. The closest match for such case I found > is DSA_PORT_TYPE_UNUSED but I'm not sure if it's enough to handle those > cases. > > That DSA_PORT_TYPE_UNUSED would probably require investigating DSA & b53 > behaviour *and* discussing it with DSA maintainer to make sure we don't > abuse that. How absent are these ports in hardware? For DSA_PORT_TYPE_UNUSED we do register a devlink port, but if those ports are really not present in hardware, I'm thinking maybe the easiest way would be to supply a ds->disabled_port_mask before dsa_register_switch(), and DSA will simply skip those ports when allocating the dp, the devlink_port etc. So you will literally have nothing for them. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 0/4] net: dsa: b53: Clean up CPU/IMP ports 2021-09-17 10:00 ` Vladimir Oltean @ 2021-09-17 12:21 ` Andrew Lunn 2021-09-17 12:31 ` Vladimir Oltean 2021-09-17 16:39 ` Florian Fainelli 1 sibling, 1 reply; 17+ messages in thread From: Andrew Lunn @ 2021-09-17 12:21 UTC (permalink / raw) To: Vladimir Oltean Cc: Rafał Miłecki, Florian Fainelli, Vivien Didelot, David S . Miller, Jakub Kicinski, netdev, bcm-kernel-feedback-list, Rafał Miłecki > > That DSA_PORT_TYPE_UNUSED would probably require investigating DSA & b53 > > behaviour *and* discussing it with DSA maintainer to make sure we don't > > abuse that. > > How absent are these ports in hardware? For DSA_PORT_TYPE_UNUSED we do > register a devlink port, but if those ports are really not present in > hardware, I'm thinking maybe the easiest way would be to supply a > ds->disabled_port_mask before dsa_register_switch(), and DSA will simply > skip those ports when allocating the dp, the devlink_port etc. So you > will literally have nothing for them. The basic idea seems O.K, we just need to be careful. We have code like: static inline bool dsa_is_dsa_port(struct dsa_switch *ds, int p) { return dsa_to_port(ds, p)->type == DSA_PORT_TYPE_DSA; } static inline bool dsa_is_user_port(struct dsa_switch *ds, int p) { return dsa_to_port(ds, p)->type == DSA_PORT_TYPE_USER; } dsa_to_port(ds, p) will return NULL, and then bad things will happen. Maybe it would be safer to add DSA_PORT_TYPE_PHANTOM and do allocate the dp? Andrew ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 0/4] net: dsa: b53: Clean up CPU/IMP ports 2021-09-17 12:21 ` Andrew Lunn @ 2021-09-17 12:31 ` Vladimir Oltean 0 siblings, 0 replies; 17+ messages in thread From: Vladimir Oltean @ 2021-09-17 12:31 UTC (permalink / raw) To: Andrew Lunn Cc: Rafał Miłecki, Florian Fainelli, Vivien Didelot, David S . Miller, Jakub Kicinski, netdev, bcm-kernel-feedback-list, Rafał Miłecki On Fri, Sep 17, 2021 at 02:21:16PM +0200, Andrew Lunn wrote: > > > That DSA_PORT_TYPE_UNUSED would probably require investigating DSA & b53 > > > behaviour *and* discussing it with DSA maintainer to make sure we don't > > > abuse that. > > > > How absent are these ports in hardware? For DSA_PORT_TYPE_UNUSED we do > > register a devlink port, but if those ports are really not present in > > hardware, I'm thinking maybe the easiest way would be to supply a > > ds->disabled_port_mask before dsa_register_switch(), and DSA will simply > > skip those ports when allocating the dp, the devlink_port etc. So you > > will literally have nothing for them. > > The basic idea seems O.K, we just need to be careful. > > We have code like: > > static inline bool dsa_is_dsa_port(struct dsa_switch *ds, int p) > { > return dsa_to_port(ds, p)->type == DSA_PORT_TYPE_DSA; > } > > static inline bool dsa_is_user_port(struct dsa_switch *ds, int p) > { > return dsa_to_port(ds, p)->type == DSA_PORT_TYPE_USER; > } > > dsa_to_port(ds, p) will return NULL, and then bad things will happen. > > Maybe it would be safer to add DSA_PORT_TYPE_PHANTOM and do allocate > the dp? I think DSA is not yet large enough for us to take the defeatist position of inserting bits of code that simply protect from other bits of code. Especially after these patches: https://patchwork.kernel.org/project/netdevbpf/cover/20210810161448.1879192-1-vladimir.oltean@nxp.com/ the DSA core should basically never iterate over ports using for (port = 0; port < ds->num_ports; port++) but instead using dsa_switch_for_each_port(dp, ds) which basically ensures that we never dereference dsa_to_port(ds, p) for a p with no associated dp (the "dp" from "dsa_switch_for_each_port" is taken from a list). Conversely, driver code is "mostly" event-driven, so DSA should only call dsa_switch_ops methods for ports where the dp structure does exist. What we would need to audit are just the drivers which set ds->disabled_port_mask (b53, maybe ksz switches?). I guess that would be on the developer who makes the change in those respective drivers to ensure that the "dp" which is searched for does in fact exist. My 2 cents anyway, I might be missing some trickier cases. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 0/4] net: dsa: b53: Clean up CPU/IMP ports 2021-09-17 10:00 ` Vladimir Oltean 2021-09-17 12:21 ` Andrew Lunn @ 2021-09-17 16:39 ` Florian Fainelli 1 sibling, 0 replies; 17+ messages in thread From: Florian Fainelli @ 2021-09-17 16:39 UTC (permalink / raw) To: Vladimir Oltean, Rafał Miłecki Cc: Andrew Lunn, Vivien Didelot, David S . Miller, Jakub Kicinski, netdev, bcm-kernel-feedback-list, Rafał Miłecki On 9/17/21 3:00 AM, Vladimir Oltean wrote: > On Fri, Sep 17, 2021 at 12:19:02AM +0200, Rafał Miłecki wrote: >> On 16.09.2021 23:46, Florian Fainelli wrote: >>> On 9/16/21 9:23 AM, Florian Fainelli wrote: >>>> On 9/16/21 5:03 AM, Rafał Miłecki wrote: >>>>> From: Rafał Miłecki <rafal@milecki.pl> >>>>> >>>>> This has been tested on: >>>>> >>>>> 1. Luxul XBR-4500 with used CPU port 5 >>>>> [ 8.361438] b53-srab-switch 18007000.ethernet-switch: found switch: BCM53012, rev 0 >>>>> >>>>> 2. Netgear R8000 with used CPU port 8 >>>>> [ 4.453858] b53-srab-switch 18007000.ethernet-switch: found switch: BCM53012, rev 5 >>>> >>>> These look good at first glance, let me give them a try on 7445 and 7278 >>>> at least before responding with Reviewed-by/Tested-by tags, thanks! >>>> >>> Found some issues on 7445 and 7278 while moving to the latest net-next >>> which I will be addressing but this worked nicely. >>> >>> What do you think about removing dev->enabled_ports and >>> b53_for_each_port entirely and using a DSA helper that iterates over the >>> switch's port list? Now that we have dev->num_ports accurately reflect >>> the number of ports it should be equivalent. >> >> The limitation I see in DSA is skipping unavailable ports. E.g. BCM5301x >> switches that don't have port 6. The closest match for such case I found >> is DSA_PORT_TYPE_UNUSED but I'm not sure if it's enough to handle those >> cases. >> >> That DSA_PORT_TYPE_UNUSED would probably require investigating DSA & b53 >> behaviour *and* discussing it with DSA maintainer to make sure we don't >> abuse that. > > How absent are these ports in hardware? For DSA_PORT_TYPE_UNUSED we do > register a devlink port, but if those ports are really not present in > hardware, I'm thinking maybe the easiest way would be to supply a > ds->disabled_port_mask before dsa_register_switch(), and DSA will simply > skip those ports when allocating the dp, the devlink_port etc. So you > will literally have nothing for them. > Port 6 on all of the newer switches where the "ideal" IMP port is 8 is completely absent and does not exist at all as a hardware resource. The registers are not necessarily consistent however and you typically see two patterns: - specifying bit 6 or port 6 as a numerical port does not nothing and no special casing is required, this is the majority of the registers and the maximum supported bitmask is 0x1ff and you can also set bit 8 to address port 8 of the CPU (I would say this is intuitive) - specifying bit 6/number 6 may alias to port 7, this is the case with the CFP code for instance that specifically checks for port >= 7 and subtracts one when needed (this is not intuitive) Whether we truly consider a port being absent from a port being unused is not probably making much difference from a semantic perspective as long as you do not try to switch a port from unused to used (whether it is DSA, CPU or USER). This is not an use case we support today, but if we did in the future (say in the context of multi CPU port devices), we would have to call back into the driver most likely and the driver could veto changing that port from unused to used. What do you think? NB: the enabled_port mask for the 7278 and 7445 switches is currently incorrectly advertising the presence of port 6 (0x1ff), that needs fixing. -- Florian ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 0/4] net: dsa: b53: Clean up CPU/IMP ports 2021-09-16 12:03 [PATCH net-next 0/4] net: dsa: b53: Clean up CPU/IMP ports Rafał Miłecki ` (4 preceding siblings ...) 2021-09-16 16:23 ` [PATCH net-next 0/4] net: dsa: b53: Clean up CPU/IMP ports Florian Fainelli @ 2021-09-17 2:58 ` Jakub Kicinski 5 siblings, 0 replies; 17+ messages in thread From: Jakub Kicinski @ 2021-09-17 2:58 UTC (permalink / raw) To: Rafał Miłecki, Florian Fainelli Cc: Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S . Miller, netdev, bcm-kernel-feedback-list, Rafał Miłecki On Thu, 16 Sep 2021 14:03:50 +0200 Rafał Miłecki wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > This has been tested on: > > 1. Luxul XBR-4500 with used CPU port 5 > [ 8.361438] b53-srab-switch 18007000.ethernet-switch: found switch: BCM53012, rev 0 > > 2. Netgear R8000 with used CPU port 8 > [ 4.453858] b53-srab-switch 18007000.ethernet-switch: found switch: BCM53012, rev 5 Applied, thanks! ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2021-09-17 16:39 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-16 12:03 [PATCH net-next 0/4] net: dsa: b53: Clean up CPU/IMP ports Rafał Miłecki 2021-09-16 12:03 ` [PATCH net-next 1/4] net: dsa: b53: Include all ports in "enabled_ports" Rafał Miłecki 2021-09-16 21:44 ` Florian Fainelli 2021-09-16 12:03 ` [PATCH net-next 2/4] net: dsa: b53: Drop BCM5301x workaround for a wrong CPU/IMP port Rafał Miłecki 2021-09-16 21:44 ` Florian Fainelli 2021-09-16 12:03 ` [PATCH net-next 3/4] net: dsa: b53: Improve flow control setup on BCM5301x Rafał Miłecki 2021-09-16 21:44 ` Florian Fainelli 2021-09-16 12:03 ` [PATCH net-next 4/4] net: dsa: b53: Drop unused "cpu_port" field Rafał Miłecki 2021-09-16 21:44 ` Florian Fainelli 2021-09-16 16:23 ` [PATCH net-next 0/4] net: dsa: b53: Clean up CPU/IMP ports Florian Fainelli 2021-09-16 21:46 ` Florian Fainelli 2021-09-16 22:19 ` Rafał Miłecki 2021-09-17 10:00 ` Vladimir Oltean 2021-09-17 12:21 ` Andrew Lunn 2021-09-17 12:31 ` Vladimir Oltean 2021-09-17 16:39 ` Florian Fainelli 2021-09-17 2:58 ` Jakub Kicinski
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.