netdev - Re: [PATCH net-next 0/2] Convert Ocelot and Felix switches to PHYLINK

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] [day] [month] [year] [list]
Message-ID: <20191122194548.mbkpi4edxdmmm7b5@soft-dev3.microsemi.net>
Date:   Fri, 22 Nov 2019 20:45:49 +0100
From:   Horatiu Vultur <horatiu.vultur@...rochip.com>
To:     Vladimir Oltean <olteanv@...il.com>
CC:     Andrew Lunn <andrew@...n.ch>,
        "David S. Miller" <davem@...emloft.net>,
        Russell King - ARM Linux admin <linux@...linux.org.uk>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Joergen Andreasen <joergen.andreasen@...rochip.com>,
        "Allan W. Nielsen" <allan.nielsen@...rochip.com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Alexandru Marginean <alexandru.marginean@....com>,
        Xiaoliang Yang <xiaoliang.yang_1@....com>,
        "Y.b. Lu" <yangbo.lu@....com>, netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 0/2] Convert Ocelot and Felix switches to PHYLINK

The 11/21/2019 19:51, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Thu, 21 Nov 2019 at 01:21, Horatiu Vultur
> <horatiu.vultur@...rochip.com> wrote:
> >
> > > >  };
> > > >
> > > >  &port0 {
> > > > +       phy-mode = "sgmii";
> > > >         phy-handle = <&phy0>;
> > > >  };
> > > >
> > > >  &port1 {
> > > > +       phy-mode = "sgmii";
> > > >         phy-handle = <&phy1>;
> > > >  };
> > > >
> > > >  &port2 {
> > > > +       phy-mode = "sgmii";
> > > >         phy-handle = <&phy2>;
> > > >  };
> > > >
> > > >  &port3 {
> > > > +       phy-mode = "sgmii";
> > > >         phy-handle = <&phy3>;
> > > >  };
> > > > diff --git a/drivers/net/ethernet/mscc/ocelot_board.c b/drivers/net/ethernet/mscc/ocelot_board.c
> > > > index aecaf4ef6ef4..9dad031900b5 100644
> > > > --- a/drivers/net/ethernet/mscc/ocelot_board.c
> > > > +++ b/drivers/net/ethernet/mscc/ocelot_board.c
> > > > @@ -513,6 +513,10 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
> > > >                 if (IS_ERR(regs))
> > > >                         continue;
> > > >
> > > > +               of_get_phy_mode(portnp, &phy_mode);
> > > > +               if (phy_mode == PHY_INTERFACE_MODE_NA)
> > > > +                       continue;
> > > > +
> > >
> > > So this effectively reverts your own patch 4214fa1efffd ("net: mscc:
> > > ocelot: omit error check from of_get_phy_mode")?
> >
> > Not really, at that point it was OK to have interface
> > PHY_INTERFACE_MODE_NA. There were few more checks before creating the
> > network device. Now with your changes you were creating
> > a network device for each port of the soc even if some ports
> > were not used on a board. Also with your changes you first create the
> > port and after that you create the phylink but between these two calls it
> > was the switch which continue for the interface PHY_INTERFACE_MODE_NA,
> > which is not correct. So these are the 2 reason why I have added the
> > property phy-mode to the ports and add back the check to see which ports
> > are used on each board.
> >
> > >
> > > >                 err = ocelot_probe_port(ocelot, port, regs);
> > > >                 if (err) {
> > > >                         of_node_put(portnp);
> > > > @@ -523,11 +527,7 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
> > > >                 priv = container_of(ocelot_port, struct ocelot_port_private,
> > > >                                     port);
> > > >
> > > > -               of_get_phy_mode(portnp, &phy_mode);
> > > > -
> > > >                 switch (phy_mode) {
> > > > -               case PHY_INTERFACE_MODE_NA:
> > > > -                       continue;
> > > >                 case PHY_INTERFACE_MODE_SGMII:
> > > >                         break;
> > > >                 case PHY_INTERFACE_MODE_QSGMII:
> > > > @@ -549,20 +549,7 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
> > > >                 }
> > > >
> > > >                 serdes = devm_of_phy_get(ocelot->dev, portnp, NULL);
> > > > -               if (IS_ERR(serdes)) {
> > > > -                       err = PTR_ERR(serdes);
> > > > -                       if (err == -EPROBE_DEFER)
> > > > -                               dev_dbg(ocelot->dev, "deferring probe\n");
> > >
> > > Why did you remove the probe deferral for the serdes phy?
> > Because not all the ports have the "phys" property.
> > >
> > > > -                       else
> > > > -                               dev_err(ocelot->dev,
> > > > -                                       "missing SerDes phys for port%d\n",
> > > > -                                       port);
> > > > -
> > > > -                       of_node_put(portnp);
> > > > -                       goto out_put_ports;
> > > > -               }
> > > > -
> > > > -               if (serdes) {
> > > > +               if (!IS_ERR(serdes)) {
> > > >                         err = phy_set_mode_ext(serdes, PHY_MODE_ETHERNET,
> > > >                                                phy_mode);
> > > >                         if (err) {
> > > > --
> > > > 2.17.1
> > > >
> > > >
> > > > >
> > > > >    Andrew
> > > >
> > > > --
> > > > /Horatiu
> > >
> > > Thanks,
> > > -Vladimir
> >
> > --
> > /Horatiu
> 
> Horatiu,
> 
> Do the 10/100 speeds work over the SGMII ports on your board? (not
> with this patch, but in general)

I have done a mistake the interface is not SGMII but GMII for the 4
internal phys. Sorry for the confusion.
I can test on Monday to see if it is working with speed 10/100 on SGMII
ports if are still interested.

> 
> Thanks,
> -Vladimir

-- 
/Horatiu