| Message ID | 20201205152814.7867-1-TheSven73@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [net,v1,1/2] net: dsa: microchip: fix devicetree parsing of cpu node | expand |
Andrew, Jakub, On Sat, Dec 5, 2020 at 10:28 AM Sven Van Asbroeck <thesven73@gmail.com> wrote: > > From: Sven Van Asbroeck <thesven73@gmail.com> > > On the ksz8795, if the devicetree contains a cpu node, > devicetree parsing fails and the whole driver errors out. > > Fix the devicetree parsing code by making it use the > correct number of ports. > > Fixes: 912aae27c6af ("net: dsa: microchip: really look for phy-mode in port nodes") > Tested-by: Sven Van Asbroeck <thesven73@gmail.com> # ksz8795 > Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com> > --- Any chance that this patch could still get merged? I believe this will work fine on both ksz8795 and ksz9477, even though num_ports is defined differently, because: ksz8795: /* set the real number of ports */ dev->ds->num_ports = dev->port_cnt + 1; https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/dsa/microchip/ksz8795.c?h=v5.10-rc7#n1266 ksz9477: /* set the real number of ports */ dev->ds->num_ports = dev->port_cnt; https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/dsa/microchip/ksz9477.c?h=v5.10-rc7#n1585 Would it be possible to merge this into net, so users get working cpu nodes? I don't think this will prevent you from harmonizing port_cnt in net-next.
On Tue, 8 Dec 2020 10:33:28 -0500 Sven Van Asbroeck wrote: > Andrew, Jakub, > > On Sat, Dec 5, 2020 at 10:28 AM Sven Van Asbroeck <thesven73@gmail.com> wrote: > > > > From: Sven Van Asbroeck <thesven73@gmail.com> > > > > On the ksz8795, if the devicetree contains a cpu node, > > devicetree parsing fails and the whole driver errors out. > > > > Fix the devicetree parsing code by making it use the > > correct number of ports. > > > > Fixes: 912aae27c6af ("net: dsa: microchip: really look for phy-mode in port nodes") > > Tested-by: Sven Van Asbroeck <thesven73@gmail.com> # ksz8795 > > Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com> > > --- > > Any chance that this patch could still get merged? > I believe this will work fine on both ksz8795 and ksz9477, even though num_ports > is defined differently, because: > > ksz8795: > /* set the real number of ports */ > dev->ds->num_ports = dev->port_cnt + 1; > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/dsa/microchip/ksz8795.c?h=v5.10-rc7#n1266 > > ksz9477: > /* set the real number of ports */ > dev->ds->num_ports = dev->port_cnt; > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/dsa/microchip/ksz9477.c?h=v5.10-rc7#n1585 > > Would it be possible to merge this into net, so users get working cpu nodes? > I don't think this will prevent you from harmonizing port_cnt in net-next. What I was talking about in the email yesterday was 0x8794 support in ksz8795.c. Is the cpu port configuration going to work there? Isn't the CPU port always port 5 (index 4)? It sure as hell looked like it until commit c9f4633b93ea ("net: dsa: microchip: remove usage of mib_port_count") came along. I wonder if ksz8794 works on net-next :/
On Tue, Dec 8, 2020 at 11:52 AM Jakub Kicinski <kuba@kernel.org> wrote: > > What I was talking about in the email yesterday was 0x8794 support > in ksz8795.c. Is the cpu port configuration going to work there? > Isn't the CPU port always port 5 (index 4)? Understood. You expressed concern that .cpu_ports = 0x10 even on the 3-port switch. I noticed that .cpu_ports is never used in ksz8794.c, so I hoped the fix would be fine. But digging a bit deeper, I see in ksz8795.c: dev->mib_port_cnt = TOTAL_PORT_NUM; /* = 5 */ ... dev->cpu_port = dev->mib_port_cnt - 1; /* = 4 */ And that is unlikely to work on the 3-port switch. So yeah, you are right, my patch won't fix the general issues here :(
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c index e5f047129b15..17b804c44c53 100644 --- a/drivers/net/dsa/microchip/ksz_common.c +++ b/drivers/net/dsa/microchip/ksz_common.c @@ -431,7 +431,7 @@ int ksz_switch_register(struct ksz_device *dev, if (of_property_read_u32(port, "reg", &port_num)) continue; - if (port_num >= dev->port_cnt) + if (port_num >= dev->ds->num_ports) return -EINVAL; of_get_phy_mode(port, &dev->ports[port_num].interface);