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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 21 Nov 2019 19:51:02 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     Horatiu Vultur <horatiu.vultur@...rochip.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

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)

Thanks,
-Vladimir