linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net v1] net: dsa: ksz8795: use correct number of physical ports
@ 2020-12-03 21:46 Sven Van Asbroeck
  2020-12-03 21:55 ` Florian Fainelli
  2020-12-04 23:24 ` Jakub Kicinski
  0 siblings, 2 replies; 5+ messages in thread
From: Sven Van Asbroeck @ 2020-12-03 21:46 UTC (permalink / raw)
  To: Woojung Huh, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Vladimir Oltean, David S Miller, Jakub Kicinski
  Cc: Sven Van Asbroeck, Marek Vasut, Tristram Ha,
	Microchip Linux Driver Support, netdev, linux-kernel

From: Sven Van Asbroeck <thesven73@gmail.com>

The ksz8795 has five physical ports, but the driver assumes
it has only four. This prevents the driver from working correctly.

Fix by indicating the correct number of physical ports.

Fixes: e66f840c08a23 ("net: dsa: ksz: Add Microchip KSZ8795 DSA driver")
Tested-by: Sven Van Asbroeck <thesven73@gmail.com> # ksz8795
Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com>
---

Tree: git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git # 39e367766fe1

To: Woojung Huh <woojung.huh@microchip.com>
To: Andrew Lunn <andrew@lunn.ch>
To: Vivien Didelot <vivien.didelot@gmail.com>
To: Florian Fainelli <f.fainelli@gmail.com>
To: Vladimir Oltean <olteanv@gmail.com>
To: "David S. Miller" <davem@davemloft.net>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Marek Vasut <marex@denx.de>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org (open list)

 drivers/net/dsa/microchip/ksz8795.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 1e101ab56cea..367cebe37ae6 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -1194,7 +1194,7 @@ static const struct ksz_chip_data ksz8795_switch_chips[] = {
 		.num_alus = 0,
 		.num_statics = 8,
 		.cpu_ports = 0x10,	/* can be configured as cpu port */
-		.port_cnt = 4,		/* total physical port count */
+		.port_cnt = 5,		/* total physical port count */
 	},
 	{
 		.chip_id = 0x8794,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net v1] net: dsa: ksz8795: use correct number of physical ports
  2020-12-03 21:46 [PATCH net v1] net: dsa: ksz8795: use correct number of physical ports Sven Van Asbroeck
@ 2020-12-03 21:55 ` Florian Fainelli
  2020-12-04 23:24 ` Jakub Kicinski
  1 sibling, 0 replies; 5+ messages in thread
From: Florian Fainelli @ 2020-12-03 21:55 UTC (permalink / raw)
  To: Sven Van Asbroeck, Woojung Huh, Andrew Lunn, Vivien Didelot,
	Vladimir Oltean, David S Miller, Jakub Kicinski
  Cc: Marek Vasut, Tristram Ha, Microchip Linux Driver Support, netdev,
	linux-kernel



On 12/3/2020 1:46 PM, Sven Van Asbroeck wrote:
> From: Sven Van Asbroeck <thesven73@gmail.com>
> 
> The ksz8795 has five physical ports, but the driver assumes
> it has only four. This prevents the driver from working correctly.
> 
> Fix by indicating the correct number of physical ports.
> 
> Fixes: e66f840c08a23 ("net: dsa: ksz: Add Microchip KSZ8795 DSA driver")
> Tested-by: Sven Van Asbroeck <thesven73@gmail.com> # ksz8795
> Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net v1] net: dsa: ksz8795: use correct number of physical ports
  2020-12-03 21:46 [PATCH net v1] net: dsa: ksz8795: use correct number of physical ports Sven Van Asbroeck
  2020-12-03 21:55 ` Florian Fainelli
@ 2020-12-04 23:24 ` Jakub Kicinski
  2020-12-04 23:43   ` Andrew Lunn
  1 sibling, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2020-12-04 23:24 UTC (permalink / raw)
  To: Sven Van Asbroeck
  Cc: Woojung Huh, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Vladimir Oltean, David S Miller, Marek Vasut, Tristram Ha,
	Microchip Linux Driver Support, netdev, linux-kernel

On Thu,  3 Dec 2020 16:46:45 -0500 Sven Van Asbroeck wrote:
> From: Sven Van Asbroeck <thesven73@gmail.com>
> 
> The ksz8795 has five physical ports, but the driver assumes
> it has only four. This prevents the driver from working correctly.
> 
> Fix by indicating the correct number of physical ports.
> 
> Fixes: e66f840c08a23 ("net: dsa: ksz: Add Microchip KSZ8795 DSA driver")
> Tested-by: Sven Van Asbroeck <thesven73@gmail.com> # ksz8795
> Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com>

All the port counts here are -1 compared to datasheets, so I'm assuming
the are not supposed to include the host facing port or something?

Can you describe the exact problem you're trying to solve?

DSA devices are not supposed to have a netdev for the host facing port
on the switch (sorry for stating the obvious).

> diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
> index 1e101ab56cea..367cebe37ae6 100644
> --- a/drivers/net/dsa/microchip/ksz8795.c
> +++ b/drivers/net/dsa/microchip/ksz8795.c
> @@ -1194,7 +1194,7 @@ static const struct ksz_chip_data ksz8795_switch_chips[] = {
>  		.num_alus = 0,
>  		.num_statics = 8,
>  		.cpu_ports = 0x10,	/* can be configured as cpu port */
> -		.port_cnt = 4,		/* total physical port count */
> +		.port_cnt = 5,		/* total physical port count */
>  	},
>  	{
>  		.chip_id = 0x8794,


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net v1] net: dsa: ksz8795: use correct number of physical ports
  2020-12-04 23:24 ` Jakub Kicinski
@ 2020-12-04 23:43   ` Andrew Lunn
  2020-12-05 14:35     ` Sven Van Asbroeck
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Lunn @ 2020-12-04 23:43 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Sven Van Asbroeck, Woojung Huh, Vivien Didelot, Florian Fainelli,
	Vladimir Oltean, David S Miller, Marek Vasut, Tristram Ha,
	Microchip Linux Driver Support, netdev, linux-kernel

On Fri, Dec 04, 2020 at 03:24:56PM -0800, Jakub Kicinski wrote:
> On Thu,  3 Dec 2020 16:46:45 -0500 Sven Van Asbroeck wrote:
> > From: Sven Van Asbroeck <thesven73@gmail.com>
> > 
> > The ksz8795 has five physical ports, but the driver assumes
> > it has only four. This prevents the driver from working correctly.
> > 
> > Fix by indicating the correct number of physical ports.
> > 
> > Fixes: e66f840c08a23 ("net: dsa: ksz: Add Microchip KSZ8795 DSA driver")
> > Tested-by: Sven Van Asbroeck <thesven73@gmail.com> # ksz8795
> > Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com>
> 
> All the port counts here are -1 compared to datasheets, so I'm assuming
> the are not supposed to include the host facing port or something?
> 
> Can you describe the exact problem you're trying to solve?
> 
> DSA devices are not supposed to have a netdev for the host facing port
> on the switch (sorry for stating the obvious).

Hi Jakub

It is the DSA core layer which takes care of that creating/not
creating netdevs. The switch should declare all the ports it has.

There has been issues with the ksz collection of drivers having
different meanings for port_cnt. There recently was some changes in
this area, maybe it broke something?

     Andrew

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net v1] net: dsa: ksz8795: use correct number of physical ports
  2020-12-04 23:43   ` Andrew Lunn
@ 2020-12-05 14:35     ` Sven Van Asbroeck
  0 siblings, 0 replies; 5+ messages in thread
From: Sven Van Asbroeck @ 2020-12-05 14:35 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Jakub Kicinski, Woojung Huh, Vivien Didelot, Florian Fainelli,
	Vladimir Oltean, David S Miller, Marek Vasut, Tristram Ha,
	Microchip Linux Driver Support, netdev,
	Linux Kernel Mailing List

Jakub, Andrew,

On Fri, Dec 4, 2020 at 6:24 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> All the port counts here are -1 compared to datasheets, so I'm assuming
> the are not supposed to include the host facing port or something?
>
> Can you describe the exact problem you're trying to solve?
>

The ksz8795 driver refuses to accept my devicetree (see below). It doesn't like
the presence of the cpu node. But I really need a cpu node, because phy-mode is
crucially important to my application: without phy-mode = "rgmii-id", the
ksz8795's ingress bit does not get set, and communication fails.

I was already suspicious of my fix: why would such an important chip property
be wrong? But changing it was very seductive: after all, the ksz8795's headline
is "Integrated 5-Port 10/100-Managed Ethernet Switch", and the driver code says
this:

        .port_cnt = 4,          /* total physical port count */

I can see now that this should be fixed more generally. The of parsing code
is using the wrong port count variable. I'll submit that shortly.

That said, when I look at this driver, I get very confused between port_cnt,
num_ports, and whether they include or exclude the cpu port. Until this gets
cleaned up, maybe the comment above can be improved, so developers don't get
too confused at least? I'll submit a patch for that too.

ethernet-switch@0 {
        compatible = "microchip,ksz8795";
        spi-max-frequency = <1000000>;
        reg = <0>;

        ports {
                #address-cells = <1>;
                #size-cells = <0>;
                port@0 {
                        reg = <0>;
                        label = "lan1";
                };
                port@1 {
                        reg = <1>;
                        label = "lan2";
                };
                port@2 {
                        reg = <2>;
                        label = "lan3";
                };
                port@3 {
                        reg = <3>;
                        label = "lan4";
                };
                port@4 {
                        /* driver errors out because
                         * reg >= dev->port_cnt (4)
                         */
                        reg = <4>;
                        label = "cpu";
                        ethernet = <&something>;
                        phy-mode = "rgmii-id";
                        fixed-link {
                                speed = <1000>;
                                full-duplex;
                        };
                };
        };
};

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-12-05 15:43 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-03 21:46 [PATCH net v1] net: dsa: ksz8795: use correct number of physical ports Sven Van Asbroeck
2020-12-03 21:55 ` Florian Fainelli
2020-12-04 23:24 ` Jakub Kicinski
2020-12-04 23:43   ` Andrew Lunn
2020-12-05 14:35     ` Sven Van Asbroeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).