[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20221110173714.6ea59de5@wsk>
Date: Thu, 10 Nov 2022 17:37:14 +0100
From: Lukasz Majewski <lukma@...x.de>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Andrew Lunn <andrew@...n.ch>,
Vivien Didelot <vivien.didelot@...il.com>,
Florian Fainelli <f.fainelli@...il.com>,
"David S . Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
Matthias Schiffer <matthias.schiffer@...tq-group.com>
Subject: Re: [PATCH 3/9] net: dsa: mv88e6xxx: implement get_phy_address
Hi Vladimir,
> On Tue, Nov 08, 2022 at 09:23:24AM +0100, Lukasz Majewski wrote:
> > From: Matthias Schiffer <matthias.schiffer@...tq-group.com>
> >
> > Avoid the need to specify a PHY for each physical port in the
> > device tree when phy_base_addr is not 0 (6250 and 6341 families).
> >
> > This change should be backwards-compatible with existing device
> > trees, as it only adds sensible defaults where explicit definitions
> > were required before.
> >
> > Signed-off-by: Matthias Schiffer
> > <matthias.schiffer@...tq-group.com>
>
> Needs your Signed-off-by tag as well.
Ok.
>
> > ---
>
> Would it be possible to do like armada-3720-turris-mox.dts does, and
> put the phy-handle in the device tree, avoiding the need for so many
> PHY address translation quirks?
As far as I can tell - the mv88e6xxx driver
(./drivers/net/dsa/mv88e6xxx) now uses hardcoded values for each member
of mv88e6xxx_info struct.
Those values are "port_base_addr" and "phy_base_addr". Those values
could be read from DTS description as pasted below.
>
> If you're going to have U-Boot support for this switch as well, the
> phy-handle mechanism is the only thing that U-Boot supports, so device
> trees written in this way will work for both (and can be passed by
> U-Boot to Linux):
>
> switch1@11 {
> compatible = "marvell,mv88e6190";
> reg = <0x11>;
> dsa,member = <0 1>;
> interrupt-parent = <&moxtet>;
> interrupts = <MOXTET_IRQ_PERIDOT(1)>;
> status = "disabled";
>
> mdio {
> #address-cells = <1>;
> #size-cells = <0>;
>
> switch1phy1: switch1phy1@1 {
> reg = <0x1>;
> };
>
> switch1phy2: switch1phy2@2 {
> reg = <0x2>;
> };
>
> switch1phy3: switch1phy3@3 {
> reg = <0x3>;
> };
>
> switch1phy4: switch1phy4@4 {
> reg = <0x4>;
> };
>
> switch1phy5: switch1phy5@5 {
> reg = <0x5>;
> };
>
> switch1phy6: switch1phy6@6 {
> reg = <0x6>;
> };
>
> switch1phy7: switch1phy7@7 {
> reg = <0x7>;
> };
>
> switch1phy8: switch1phy8@8 {
> reg = <0x8>;
> };
> };
>
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
>
> port@1 {
> reg = <0x1>;
> label = "lan9";
> phy-handle = <&switch1phy1>;
> };
>
> port@2 {
> reg = <0x2>;
> label = "lan10";
> phy-handle = <&switch1phy2>;
> };
>
> port@3 {
> reg = <0x3>;
> label = "lan11";
> phy-handle = <&switch1phy3>;
> };
>
> port@4 {
> reg = <0x4>;
> label = "lan12";
> phy-handle = <&switch1phy4>;
> };
>
> port@5 {
> reg = <0x5>;
> label = "lan13";
> phy-handle = <&switch1phy5>;
> };
>
> port@6 {
> reg = <0x6>;
> label = "lan14";
> phy-handle = <&switch1phy6>;
> };
>
> port@7 {
> reg = <0x7>;
> label = "lan15";
> phy-handle = <&switch1phy7>;
> };
>
> port@8 {
> reg = <0x8>;
> label = "lan16";
> phy-handle = <&switch1phy8>;
> };
>
> switch1port9: port@9 {
> reg = <0x9>;
> label = "dsa";
> phy-mode = "2500base-x";
> managed = "in-band-status";
> link = <&switch0port10>;
> };
>
> switch1port10: port@a {
> reg = <0xa>;
> label = "dsa";
> phy-mode = "2500base-x";
> managed = "in-band-status";
> link = <&switch2port9>;
> status = "disabled";
> };
>
> port-sfp@a {
> reg = <0xa>;
> label = "sfp";
> sfp = <&sfp>;
> phy-mode = "sgmii";
> managed = "in-band-status";
> status = "disabled";
> };
> };
> };
The u-boot mailine has basic support for mv88e6071 and mv88e6020 (and
also some 'extension' patches which are floating around [1]).
For the current code - I'm using:
mdio {
#address-cells = <1>;
#size-cells = <0>;
switch@0 {
compatible = "marvell,mv88e6250";
reg = <0x00>;
interrupt-parent = <&gpio2>;
interrupts = <20 IRQ_TYPE_LEVEL_LOW>;
interrupt-controller;
#interrupt-cells = <2>;
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@5 {
reg = <5>;
label = "cpu";
phy-mode = "rgmii-id";
ethernet = <&fec1>;
fixed-link {
speed = <100>;
full-duplex;
};
};
};
The only "hack" which I see from time to time is the replacement of
'switch@0' with 'switch@8' to take into account the R0_LED/ADDRES4
bootstrap pin value (to shift up ports addresses).
Links:
[1] - https://lists.denx.de/pipermail/u-boot/2021-March/444827.html
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@...x.de
Content of type "application/pgp-signature" skipped
Powered by blists - more mailing lists