netdev - Re: [RFC PATCH v3 net-next 10/10] docs: devicetree: add documentation for the VSC7512 SPI device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 14 Aug 2021 18:08:45 -0700
From:   Colin Foster <colin.foster@...advantage.com>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     andrew@...n.ch, vivien.didelot@...il.com, f.fainelli@...il.com,
        davem@...emloft.net, kuba@...nel.org, robh+dt@...nel.org,
        claudiu.manoil@....com, alexandre.belloni@...tlin.com,
        UNGLinuxDriver@...rochip.com, hkallweit1@...il.com,
        linux@...linux.org.uk, netdev@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH v3 net-next 10/10] docs: devicetree: add
 documentation for the VSC7512 SPI device

On Sun, Aug 15, 2021 at 03:00:41AM +0300, Vladimir Oltean wrote:
> On Sat, Aug 14, 2021 at 04:41:58PM -0700, Colin Foster wrote:
> > So DSA requires a fixed-link property.
> 
> How did you come to that conclusion? As mentioned twice already, DSA
> registers a phylink for the CPU port, and phylink works with either a
> phy-handle or a fixed-link.
> 
> Support for this has been added more than 2 years ago:
> https://patchwork.ozlabs.org/project/netdev/patch/1558992127-26008-11-git-send-email-ioana.ciornei@nxp.com/
> 
> You have a PHY... so use a phy-handle.

My misunderstanding. I think I saw all the documentation / examples and
understood that to be "it must be this way". I shouldn't have drawn that
conclusion.

> 
> > And that makes sense... who in
> > their right mind would connect switches on a board using an RJ45
> > connection :) Then the only reason any of this is working is because I
> > have eth0 set up as an RJ45 connection, and because of that I need the
> > hack to enable the phy on the switch port 0...
> > 
> > Maybe that's a question:
> > Is my devicetree incorrect for claiming the connection is SGMII when it
> > should be RJ45?
> 
> Your device tree description is absolutely incorrect by all accounts.
> 
> First of all, "is SGMII" does not really preclude "is RJ45", because you
> can have an external PHY connected to your MAC via SGMII, and that
> external PHY would provide RJ45 access. That would be absolutely fine too.
> 
> That would be described as:
> 
> 	port@0 {
> 		phy-mode = "sgmii";
> 		phy-handle = <&external_phy>;
> 	};
> 
> It would be absolutely fine as well to describe the RJ45 port via an
> internal PHY if that's how things are hooked up in your eval board
> (really don't know what PHY you have, sorry):
> 
> 	port@0 {
> 		phy-mode = "internal";
> 		phy-handle = <&internal_phy>;
> 	};
> 
> But in the absence of a phy-handle and the presence of fixed-link, like
> the way you are describing it, you are telling Linux that you have an
> SGMII PHY-less system, where the SGMII lane goes directly towards the
> outside world.
> 

Understood, and thank you for the feedback. I am definitely not
currently running in a PHY-less system on this interface. I also had
some confusion about phy-mode = "internal" vs phy-mode = "sgmii". It
seems like I have ports 1-3 incorrectly confiugred as well - they are
internal to the VSC7512 chip.

> I think it is actually written somewhere in the documentation that
> describing a connection to a PHY using a fixed-link is wrong and
> strongly discouraged.

I have some reading to do. I made assumptions early on and now that
things seem to be getting close, it is becoming clear that those
misunderstandings were leading me down the wrong path.

> 
> > Or is my setup incorrect for using RJ45 and there's no
> > way to configure it that way, so the fact that it functions is an
> > anomaly?
> 
> No, the setup is not incorrect, it is just fine and both DSA and phylink
> support it as long as it is described properly, with the adequate
> phy-handle on the CPU port.

This is very good to know. Thank you. I'm sorry that you're having to
troubleshoot my devicetree, but it is incerdibly helpful to just know
"the devicetree is wrong" instead of "my implementation of the driver
has this shortcoming." I have another round of dev / testing ahead of
me.

Again, a sincere thank you for the feedback. I understand I'm making a
lot of mistakes. I'm hopeful that these mistakes can come off as
"inexperienced" and, frankly, that I'm not annoying everyone.