[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+h21hpDL=cLsZXyyk3V7=gQnaf-ZdyyuHjcaZ-DY+zRUcnJOw@mail.gmail.com>
Date: Wed, 20 Nov 2019 15:13:38 +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 Wed, 20 Nov 2019 at 14:08, Horatiu Vultur
<horatiu.vultur@...rochip.com> wrote:
>
> The 11/19/2019 22:42, Andrew Lunn wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > > Before this commit it was ok to use PHY_INTERFACE_MODE_NA but now that
> > > is not true anymore. In this case we have 4 ports that have phy and
> > > then 6 sfp ports. So I was looking to describe this in DT but without
> > > any success. If you have any advice that would be great.
> >
> > Is it the copper ports causing the trouble, or the SFP? Ideally, you
> > should describe the SFPs as SFPs. But i don't think the driver has the
> > needed support for that yet. So you might need to use fixed-link for
> > the moment.
>
> It was both of them. So I have done few small changes to these patches.
> - first I added the phy-mode in DT on the interfaces that have a
> phy(internal or external)
> - add a check for PHY_INTERFACE_MODE_NA before the port is probed so it
> would not create net device if the phy mode is PHY_INTERFACE_MODE_NA
> because in that case the phylink was not created.
>
> With these changes now only the ports that have phy are probed. This is
> the same behaviour as before these patches. I have tried to configure
> the sfp ports as fixed-links but unfortunetly it didn't work, I think
> because of some missconfiguration on MAC or SerDes, which I need to
> figure out. But I think this can be fix in a different patch.
>
> I have done few tests and they seem to work fine.
> Here are my changes.
>
> diff --git a/arch/mips/boot/dts/mscc/ocelot_pcb120.dts b/arch/mips/boot/dts/mscc/ocelot_pcb120.dts
> index 33991fd209f5..0800a86b7f16 100644
> --- a/arch/mips/boot/dts/mscc/ocelot_pcb120.dts
> +++ b/arch/mips/boot/dts/mscc/ocelot_pcb120.dts
> @@ -60,18 +60,22 @@
>
> &port0 {
> phy-handle = <&phy0>;
> + phy-mode = "sgmii";
> };
>
> &port1 {
> phy-handle = <&phy1>;
> + phy-mode = "sgmii";
> };
>
> &port2 {
> phy-handle = <&phy2>;
> + phy-mode = "sgmii";
> };
>
> &port3 {
> phy-handle = <&phy3>;
> + phy-mode = "sgmii";
> };
>
> &port4 {
> diff --git a/arch/mips/boot/dts/mscc/ocelot_pcb123.dts b/arch/mips/boot/dts/mscc/ocelot_pcb123.dts
> index ef852f382da8..6b0b1fb358ad 100644
> --- a/arch/mips/boot/dts/mscc/ocelot_pcb123.dts
> +++ b/arch/mips/boot/dts/mscc/ocelot_pcb123.dts
> @@ -47,17 +47,21 @@
> };
>
> &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")?
> 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?
> - 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
Powered by blists - more mailing lists