Re: [PATCH] powerpc/kmcent2: update the ethernet devices' phy properties

[Valentin Longchamp <valentin@longchamp.me>]

Le mar. 30 juil. 2019 à 11:44, Madalin-cristian Bucur
<madalin.bucur@nxp.com> a écrit :
>
> > -----Original Message-----
> >
> > > Le dim. 14 juil. 2019 à 22:05, Valentin Longchamp
> > > <valentin@longchamp.me> a écrit :
> > > >
> > > > Change all phy-connection-type properties to phy-mode that are better
> > > > supported by the fman driver.
> > > >
> > > > Use the more readable fixed-link node for the 2 sgmii links.
> > > >
> > > > Change the RGMII link to rgmii-id as the clock delays are added by the
> > > > phy.
> > > >
> > > > Signed-off-by: Valentin Longchamp <valentin@longchamp.me>
> >
> > I don't see any other uses of phy-mode in arch/powerpc/boot/dts/fsl, and I see
> > lots of phy-connection-type with fman.  Madalin, does this patch look OK?
> >
> > -Scott
>
> Hi,
>
> we are using "phy-connection-type" not "phy-mode" for the NXP (former Freescale)
> DPAA platforms. While the two seem to be interchangeable ("phy-mode" seems to be
> more recent, looking at the device tree bindings), the driver code in Linux seems
> to use one or the other, not both so one should stick with the variant the driver
> is using. To make things more complex, there may be dependencies in bootloaders,
> I see code in u-boot using only "phy-connection-type" or only "phy-mode".
>
> I'd leave "phy-connection-type" as is.

So I have finally had time to have a look and now I understand what
happens. You are right, there are bootloader dependencies: u-boot
calls fdt_fixup_phy_connection() that somehow in our case adds (or
changes if already in the device tree) the phy-connection-type
property to a wrong value ! By having a phy-mode in the device tree,
that is not changed by u-boot and by chance picked up by the kernel
fman driver (of_get_phy_mode() ) over phy-connection-mode, the below
patch fixes it for us.

I agree with you, it's not correct to have both phy-connection-type
and phy-mode. Ideally, u-boot on the board should be reworked so that
it does not perform the above wrong fixup. However, in an "unfixed"
.dtb (I have disabled fdt_fixup_phy_connection), the device tree in
the end only has either phy-connection-type or phy-mode, according to
what was chosen in the .dts file. And the fman driver works well with
both (thanks to the call to of_get_phy_mode() ). I would therefore
argue that even if all other DPAA platforms use phy-connection-type,
phy-mode is valid as well. (Furthermore we already have hundreds of
such boards in the field and we don't really support "remote" u-boot
update, so the u-boot fix is going to be difficult for us to pull).

Valentin

>
> Madalin
>
> > > > ---
> > > >  arch/powerpc/boot/dts/fsl/kmcent2.dts | 16 +++++++++++-----
> > > >  1 file changed, 11 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/arch/powerpc/boot/dts/fsl/kmcent2.dts
> > > > b/arch/powerpc/boot/dts/fsl/kmcent2.dts
> > > > index 48b7f9797124..c3e0741cafb1 100644
> > > > --- a/arch/powerpc/boot/dts/fsl/kmcent2.dts
> > > > +++ b/arch/powerpc/boot/dts/fsl/kmcent2.dts
> > > > @@ -210,13 +210,19 @@
> > > >
> > > >                 fman@400000 {
> > > >                         ethernet@e0000 {
> > > > -                               fixed-link = <0 1 1000 0 0>;
> > > > -                               phy-connection-type = "sgmii";
> > > > +                               phy-mode = "sgmii";
> > > > +                               fixed-link {
> > > > +                                       speed = <1000>;
> > > > +                                       full-duplex;
> > > > +                               };
> > > >                         };
> > > >
> > > >                         ethernet@e2000 {
> > > > -                               fixed-link = <1 1 1000 0 0>;
> > > > -                               phy-connection-type = "sgmii";
> > > > +                               phy-mode = "sgmii";
> > > > +                               fixed-link {
> > > > +                                       speed = <1000>;
> > > > +                                       full-duplex;
> > > > +                               };
> > > >                         };
> > > >
> > > >                         ethernet@e4000 {
> > > > @@ -229,7 +235,7 @@
> > > >
> > > >                         ethernet@e8000 {
> > > >                                 phy-handle = <&front_phy>;
> > > > -                               phy-connection-type = "rgmii";
> > > > +                               phy-mode = "rgmii-id";
> > > >                         };
> > > >
> > > >                         mdio0: mdio@fc000 {
> > > > --
> > > > 2.17.1
> > > >
> > >
> > >
>