mbox series

[devicetree,0/4] DT bindings for Felix DSA switch on LS1028A

Message ID 20200217144414.409-1-olteanv@gmail.com
Headers show
Series DT bindings for Felix DSA switch on LS1028A | expand

Message

Vladimir Oltean Feb. 17, 2020, 2:44 p.m. UTC
From: Vladimir Oltean <vladimir.oltean@nxp.com>

This series officializes the device tree bindings for the embedded
Ethernet switch on NXP LS1028A (and for the reference design board).
The driver has been in the tree since v5.4-rc6.

Claudiu Manoil (2):
  arm64: dts: fsl: ls1028a: add node for Felix switch
  arm64: dts: fsl: ls1028a: enable switch PHYs on RDB

Vladimir Oltean (2):
  arm64: dts: fsl: ls1028a: delete extraneous #interrupt-cells for ENETC
    RCIE
  dt-bindings: net: dsa: ocelot: document the vsc9959 core

 .../devicetree/bindings/net/dsa/ocelot.txt    | 97 +++++++++++++++++++
 .../boot/dts/freescale/fsl-ls1028a-rdb.dts    | 51 ++++++++++
 .../arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 85 +++++++++++++++-
 3 files changed, 231 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/dsa/ocelot.txt

Comments

Andrew Lunn Feb. 17, 2020, 3:29 p.m. UTC | #1
Hi Vladimir

> +					/* Internal port with DSA tagging */
> +					mscc_felix_port4: port@4 {
> +						reg = <4>;
> +						phy-mode = "gmii";

Is it really using gmii? Often in SoC connections use something else,
and phy-mode = "internal" is more appropriate.

> +						ethernet = <&enetc_port2>;
> +
> +						fixed-link {
> +							speed = <2500>;
> +							full-duplex;
> +						};

gmii and 2500 also don't really go together.

     Andrew
Andrew Lunn Feb. 17, 2020, 3:30 p.m. UTC | #2
On Mon, Feb 17, 2020 at 04:44:14PM +0200, Vladimir Oltean wrote:
> From: Claudiu Manoil <claudiu.manoil@nxp.com>
> 
> Link the switch PHY nodes to the central MDIO controller PCIe endpoint
> node on LS1028A (implemented as PF3) so that PHYs are accessible via
> MDIO.
> 
> Enable SGMII AN on the Felix PCS by telling PHYLINK that the VSC8514
> quad PHY is capable of in-band-status.
> 
> The PHYs are used in poll mode due to an issue with the interrupt line
> on current revisions of the LS1028A-RDB board.
> 
> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
> Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
Vladimir Oltean Feb. 17, 2020, 3:33 p.m. UTC | #3
Hi Andrew,

On Mon, 17 Feb 2020 at 17:29, Andrew Lunn <andrew@lunn.ch> wrote:
>
> Hi Vladimir
>
> > +                                     /* Internal port with DSA tagging */
> > +                                     mscc_felix_port4: port@4 {
> > +                                             reg = <4>;
> > +                                             phy-mode = "gmii";
>
> Is it really using gmii? Often in SoC connections use something else,
> and phy-mode = "internal" is more appropriate.
>

What would be that "something else"? Given that the host port and the
switch are completely different hardware IP blocks, I would assume
that a parallel GMII is what's connecting them, no optimizations done.
Certainly no serializer. But I don't know for sure.
Does it matter, in the end?

> > +                                             ethernet = <&enetc_port2>;
> > +
> > +                                             fixed-link {
> > +                                                     speed = <2500>;
> > +                                                     full-duplex;
> > +                                             };
>
> gmii and 2500 also don't really go together.

Not even if you raise the clock frequency?

>
>      Andrew

Thanks,
-Vladimir
Vladimir Oltean Feb. 17, 2020, 5:24 p.m. UTC | #4
On Mon, 17 Feb 2020 at 17:33, Vladimir Oltean <olteanv@gmail.com> wrote:
>
> Hi Andrew,
>
> On Mon, 17 Feb 2020 at 17:29, Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > Hi Vladimir
> >
> > > +                                     /* Internal port with DSA tagging */
> > > +                                     mscc_felix_port4: port@4 {
> > > +                                             reg = <4>;
> > > +                                             phy-mode = "gmii";
> >
> > Is it really using gmii? Often in SoC connections use something else,
> > and phy-mode = "internal" is more appropriate.
> >
>
> What would be that "something else"? Given that the host port and the
> switch are completely different hardware IP blocks, I would assume
> that a parallel GMII is what's connecting them, no optimizations done.
> Certainly no serializer. But I don't know for sure.
> Does it matter, in the end?
>

To clarify, the reason I'm asking whether it matters is because I'd
have to modify PHY_INTERFACE_MODE_GMII in
drivers/net/dsa/ocelot/felix_vsc9959.c too, for the internal ports.
Then I'm not sure anymore what tree this device tree patch should go
in through.

> > > +                                             ethernet = <&enetc_port2>;
> > > +
> > > +                                             fixed-link {
> > > +                                                     speed = <2500>;
> > > +                                                     full-duplex;
> > > +                                             };
> >
> > gmii and 2500 also don't really go together.
>
> Not even if you raise the clock frequency?
>
> >
> >      Andrew
>
> Thanks,
> -Vladimir
Vladimir Oltean Feb. 18, 2020, 11:15 p.m. UTC | #5
Hi Andrew,

On Mon, 17 Feb 2020 at 19:24, Vladimir Oltean <olteanv@gmail.com> wrote:
>
> On Mon, 17 Feb 2020 at 17:33, Vladimir Oltean <olteanv@gmail.com> wrote:
> >
> > Hi Andrew,
> >
> > On Mon, 17 Feb 2020 at 17:29, Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > Hi Vladimir
> > >
> > > > +                                     /* Internal port with DSA tagging */
> > > > +                                     mscc_felix_port4: port@4 {
> > > > +                                             reg = <4>;
> > > > +                                             phy-mode = "gmii";
> > >
> > > Is it really using gmii? Often in SoC connections use something else,
> > > and phy-mode = "internal" is more appropriate.
> > >
> >
> > What would be that "something else"? Given that the host port and the
> > switch are completely different hardware IP blocks, I would assume
> > that a parallel GMII is what's connecting them, no optimizations done.
> > Certainly no serializer. But I don't know for sure.
> > Does it matter, in the end?
> >
>
> To clarify, the reason I'm asking whether it matters is because I'd
> have to modify PHY_INTERFACE_MODE_GMII in
> drivers/net/dsa/ocelot/felix_vsc9959.c too, for the internal ports.
> Then I'm not sure anymore what tree this device tree patch should go
> in through.
>
> > > > +                                             ethernet = <&enetc_port2>;
> > > > +
> > > > +                                             fixed-link {
> > > > +                                                     speed = <2500>;
> > > > +                                                     full-duplex;
> > > > +                                             };
> > >
> > > gmii and 2500 also don't really go together.
> >
> > Not even if you raise the clock frequency?
> >
> > >
> > >      Andrew
> >
> > Thanks,
> > -Vladimir

Correct me if I'm wrong, but I think that PHY_INTERFACE_MODE_INTERNAL
is added by Florian in 2017 as a generalization of the BCM7445 DSA
switch bindings with internal PHY ports, and later became "popular"
with other DSA drivers (ar9331, lantiq gswip). Of those, ar9331 is
actually using phy-mode = "gmii" for the CPU port, and phy-mode =
"internal" for the embedded copper PHYs.
I hate to be making this sort of non-binary decision. Is it a GMII
interface _or_ an internal interface? Prior to 2017, this would have
probably been a non-question. The patch series which adds it does not
clarify "you should use this mode in situation A, and this mode in
situation B" either.

Regards,
-Vladimir