| Message ID | 20210119205518.470974-1-marex@denx.de (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: dsa: microchip: ksz8795: Fix KSZ8794 port map again | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 5 maintainers not CCed: olteanv@gmail.com vivien.didelot@gmail.com davem@davemloft.net woojung.huh@microchip.com UNGLinuxDriver@microchip.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: networking block comments don't use an empty /* line, use /* Comment... |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Tue, Jan 19, 2021 at 09:55:18PM +0100, Marek Vasut wrote: > The KSZ8795 switch has 4 external ports {0,1,2,3} and 1 CPU port {4}, so > does the KSZ8765. The KSZ8794 seems to be repackaged KSZ8795 with different > ID and port 3 not routed out, however the port 3 registers are present in > the silicon, so the KSZ8794 switch has 3 external ports {0,1,2} and 1 CPU > port {4}. Currently the driver always uses the last port as CPU port, on > KSZ8795/KSZ8765 that is port 4 and that is OK, but on KSZ8794 that is port > 3 and that is not OK, as it must also be port 4. > > This patch adjusts the driver such that it always registers a switch with > 5 ports total (4 external ports, 1 CPU port), always sets the CPU port to > switch port 4, and then configures the external port mask according to > the switch model -- 3 ports for KSZ8794 and 4 for KSZ8795/KSZ8765. Hi Marek What appears to be missing is any checks for somebody trying to use the 'somewhat non-existent' port. Assuming all the drivers set dev->port_mask properly, i think you can add a check to ksz_enable_port() and return -EINVAL if the requested port is not a member of port_mask. Andrew
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c index c973db101b72..a4570ba29c83 100644 --- a/drivers/net/dsa/microchip/ksz8795.c +++ b/drivers/net/dsa/microchip/ksz8795.c @@ -1187,6 +1187,20 @@ static const struct ksz_chip_data ksz8795_switch_chips[] = { .port_cnt = 5, /* total cpu and user ports */ }, { + /* + * WARNING + * ======= + * KSZ8794 is similar to KSZ8795, except the port map + * contains a gap between external and CPU ports, the + * port map is NOT continuous. The per-port register + * map is shifted accordingly too, i.e. registers at + * offset 0x40 are NOT used on KSZ8794 and they ARE + * used on KSZ8795 for external port 3. + * external cpu + * KSZ8794 0,1,2 4 + * KSZ8795 0,1,2,3 4 + * KSZ8765 0,1,2,3 4 + */ .chip_id = 0x8794, .dev_name = "KSZ8794", .num_vlans = 4096, @@ -1220,9 +1234,13 @@ static int ksz8795_switch_init(struct ksz_device *dev) dev->num_vlans = chip->num_vlans; dev->num_alus = chip->num_alus; dev->num_statics = chip->num_statics; - dev->port_cnt = chip->port_cnt; + dev->port_cnt = fls(chip->cpu_ports); + dev->cpu_port = fls(chip->cpu_ports) - 1; + dev->phy_port_cnt = dev->port_cnt - 1; dev->cpu_ports = chip->cpu_ports; - + dev->host_mask = chip->cpu_ports; + dev->port_mask = (BIT(dev->phy_port_cnt) - 1) | + chip->cpu_ports; break; } } @@ -1231,17 +1249,9 @@ static int ksz8795_switch_init(struct ksz_device *dev) if (!dev->cpu_ports) return -ENODEV; - dev->port_mask = BIT(dev->port_cnt) - 1; - dev->port_mask |= dev->host_mask; - dev->reg_mib_cnt = KSZ8795_COUNTER_NUM; dev->mib_cnt = ARRAY_SIZE(mib_names); - dev->phy_port_cnt = dev->port_cnt - 1; - - dev->cpu_port = dev->port_cnt - 1; - dev->host_mask = BIT(dev->cpu_port); - dev->ports = devm_kzalloc(dev->dev, dev->port_cnt * sizeof(struct ksz_port), GFP_KERNEL);
The KSZ8795 switch has 4 external ports {0,1,2,3} and 1 CPU port {4}, so does the KSZ8765. The KSZ8794 seems to be repackaged KSZ8795 with different ID and port 3 not routed out, however the port 3 registers are present in the silicon, so the KSZ8794 switch has 3 external ports {0,1,2} and 1 CPU port {4}. Currently the driver always uses the last port as CPU port, on KSZ8795/KSZ8765 that is port 4 and that is OK, but on KSZ8794 that is port 3 and that is not OK, as it must also be port 4. This patch adjusts the driver such that it always registers a switch with 5 ports total (4 external ports, 1 CPU port), always sets the CPU port to switch port 4, and then configures the external port mask according to the switch model -- 3 ports for KSZ8794 and 4 for KSZ8795/KSZ8765. Fixes: 68a1b676db52 ("net: dsa: microchip: ksz8795: remove superfluous port_cnt assignment") Fixes: 4ce2a984abd8 ("net: dsa: microchip: ksz8795: use phy_port_cnt where possible") Fixes: 241ed719bc98 ("net: dsa: microchip: ksz8795: use port_cnt instead of TOTOAL_PORT_NUM") Signed-off-by: Marek Vasut <marex@denx.de> Cc: Andrew Lunn <andrew@lunn.ch> Cc: Florian Fainelli <f.fainelli@gmail.com> Cc: Michael Grzeschik <m.grzeschik@pengutronix.de> Cc: Jakub Kicinski <kuba@kernel.org> --- NOTE: In case of KSZ8794, the DT node port@4 is the CPU port and port@3 should not be defined. --- drivers/net/dsa/microchip/ksz8795.c | 30 +++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-)