[PATCH net-next v1 1/2] net: dsa: mv88e6xxx: Default CMODE to 1000BaseX only on 6390X

[PATCH net-next v1 1/2] net: dsa: mv88e6xxx: Default CMODE to 1000BaseX only on 6390X

[Marek Behún]

Commit 787799a9d555 sets the SERDES interfaces of 6390 and 6390X to
1000BaseX, but this is only needed on 6390X, since there are SERDES
interfaces which can be used on lower ports on 6390.

This commit fixes this by returning to previous behaviour on 6390.
(Previous behaviour means that CMODE is not set at all if requested mode
is NA).

This is needed on Turris MOX, where the 88e6190 is connected to CPU in
2500BaseX mode.

Fixes: 787799a9d555 ("net: dsa: mv88e6xxx: Default ports 9/10 6390X CMODE to 1000BaseX")
Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 drivers/net/dsa/mv88e6xxx/port.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
index ebd26b6a93e6..ee7029f4ee22 100644
--- a/drivers/net/dsa/mv88e6xxx/port.c
+++ b/drivers/net/dsa/mv88e6xxx/port.c
@@ -444,6 +444,8 @@ int mv88e6390_port_set_cmode(struct mv88e6xxx_chip *chip, int port,
 			     phy_interface_t mode)
 {
 	switch (mode) {
+	case PHY_INTERFACE_MODE_NA:
+		return 0;
 	case PHY_INTERFACE_MODE_XGMII:
 	case PHY_INTERFACE_MODE_XAUI:
 	case PHY_INTERFACE_MODE_RXAUI:
-- 
2.19.2

[PATCH net-next v1 2/2] net: dsa: mv88e6xxx: Fix phylink_validate for Topaz family

[Marek Behún]

The Topaz family should have different phylink_validate method from the
Peridot, since on Topaz the port supporting 2500BaseX mode is port 5,
not 9 and 10.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 8dca2c949e73..2b285b0b9e06 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -632,6 +632,20 @@ static void mv88e6185_phylink_validate(struct mv88e6xxx_chip *chip, int port,
 	mv88e6065_phylink_validate(chip, port, mask, state);
 }
 
+static void mv88e6341_phylink_validate(struct mv88e6xxx_chip *chip, int port,
+				       unsigned long *mask,
+				       struct phylink_link_state *state)
+{
+	if (port >= 5)
+		phylink_set(mask, 2500baseX_Full);
+
+	/* No ethtool bits for 200Mbps */
+	phylink_set(mask, 1000baseT_Full);
+	phylink_set(mask, 1000baseX_Full);
+
+	mv88e6065_phylink_validate(chip, port, mask, state);
+}
+
 static void mv88e6352_phylink_validate(struct mv88e6xxx_chip *chip, int port,
 				       unsigned long *mask,
 				       struct phylink_link_state *state)
@@ -3052,7 +3066,7 @@ static const struct mv88e6xxx_ops mv88e6141_ops = {
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
 	.serdes_power = mv88e6341_serdes_power,
 	.gpio_ops = &mv88e6352_gpio_ops,
-	.phylink_validate = mv88e6390_phylink_validate,
+	.phylink_validate = mv88e6341_phylink_validate,
 };
 
 static const struct mv88e6xxx_ops mv88e6161_ops = {
@@ -3684,7 +3698,7 @@ static const struct mv88e6xxx_ops mv88e6341_ops = {
 	.gpio_ops = &mv88e6352_gpio_ops,
 	.avb_ops = &mv88e6390_avb_ops,
 	.ptp_ops = &mv88e6352_ptp_ops,
-	.phylink_validate = mv88e6390_phylink_validate,
+	.phylink_validate = mv88e6341_phylink_validate,
 };
 
 static const struct mv88e6xxx_ops mv88e6350_ops = {
-- 
2.19.2

Re: [PATCH net-next v1 2/2] net: dsa: mv88e6xxx: Fix phylink_validate for Topaz family

[Andrew Lunn]

On Thu, Jan 24, 2019 at 04:43:09PM +0100, Marek Behún wrote:
> The Topaz family should have different phylink_validate method from the
> Peridot, since on Topaz the port supporting 2500BaseX mode is port 5,
> not 9 and 10.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>

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

    Andrew

Re: [PATCH net-next v1 1/2] net: dsa: mv88e6xxx: Default CMODE to 1000BaseX only on 6390X

[Andrew Lunn]

On Thu, Jan 24, 2019 at 04:43:08PM +0100, Marek Behún wrote:
> Commit 787799a9d555 sets the SERDES interfaces of 6390 and 6390X to
> 1000BaseX, but this is only needed on 6390X, since there are SERDES
> interfaces which can be used on lower ports on 6390.
> 
> This commit fixes this by returning to previous behaviour on 6390.
> (Previous behaviour means that CMODE is not set at all if requested mode
> is NA).
> 
> This is needed on Turris MOX, where the 88e6190 is connected to CPU in
> 2500BaseX mode.
> 
> Fixes: 787799a9d555 ("net: dsa: mv88e6xxx: Default ports 9/10 6390X CMODE to 1000BaseX")
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> ---
>  drivers/net/dsa/mv88e6xxx/port.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
> index ebd26b6a93e6..ee7029f4ee22 100644
> --- a/drivers/net/dsa/mv88e6xxx/port.c
> +++ b/drivers/net/dsa/mv88e6xxx/port.c
> @@ -444,6 +444,8 @@ int mv88e6390_port_set_cmode(struct mv88e6xxx_chip *chip, int port,
>  			     phy_interface_t mode)
>  {
>  	switch (mode) {
> +	case PHY_INTERFACE_MODE_NA:
> +		return 0;

Hi Marek

Although the previous behaviour might of allowed the cmode to be NA,
i'm not sure that is a good idea. How do you know the port actually is
using 2500BaseX, and not SGMII for example?

You really should set the interface mode in the device tree file, so
you know it does have the value you want.

    Andrew

Re: [PATCH net-next v1 1/2] net: dsa: mv88e6xxx: Default CMODE to 1000BaseX only on 6390X

[Marek Behun]

> Hi Marek
> 
> Although the previous behaviour might of allowed the cmode to be NA,
> i'm not sure that is a good idea. How do you know the port actually is
> using 2500BaseX, and not SGMII for example?
> 
> You really should set the interface mode in the device tree file, so
> you know it does have the value you want.
> 
>     Andrew

Hi Andrew,

yes, I wanted to set the cpu port mode in DTS at first, but if I
understood the code correctly (maybe I made a mistake), the CPU port
mode is not determined from DTS (since there is no phy). Am I wrong?

It would be better if it was forced from DTS from the CPU port node.

On Turris Mox the mode is set by pin configuration after reset.

Marek

Re: [PATCH net-next v1 1/2] net: dsa: mv88e6xxx: Default CMODE to 1000BaseX only on 6390X

[Andrew Lunn]

> yes, I wanted to set the cpu port mode in DTS at first, but if I
> understood the code correctly (maybe I made a mistake), the CPU port
> mode is not determined from DTS (since there is no phy). Am I wrong?

It should work. I have boards which use it for DSA ports. I'm not sure
i have a board with it for a CPU port though.

   Andrew

Re: [PATCH net-next v1 1/2] net: dsa: mv88e6xxx: Default CMODE to 1000BaseX only on 6390X

[Marek Behun]

What properties does the cpu port node need to contain to force it?
phy-mode = "2500base-x"; is not enough.
Marek

On Thu, 24 Jan 2019 17:48:05 +0100
Andrew Lunn <andrew@lunn.ch> wrote:

> > yes, I wanted to set the cpu port mode in DTS at first, but if I
> > understood the code correctly (maybe I made a mistake), the CPU port
> > mode is not determined from DTS (since there is no phy). Am I
> > wrong?  
> 
> It should work. I have boards which use it for DSA ports. I'm not sure
> i have a board with it for a CPU port though.
> 
>    Andrew

Re: [PATCH net-next v1 1/2] net: dsa: mv88e6xxx: Default CMODE to 1000BaseX only on 6390X

[Andrew Lunn]

On Thu, Jan 24, 2019 at 07:04:51PM +0100, Marek Behun wrote:
> What properties does the cpu port node need to contain to force it?
> phy-mode = "2500base-x"; is not enough.

Hi Marek

For DSA ports we have:

                                                phy-mode = "rgmii-txid";
                                                fixed-link {
                                                        speed = <1000>;
                                                        full-duplex;
                                                };

See dsa_port_fixed_link_register_of()

    Andrew

Re: [PATCH net-next v1 1/2] net: dsa: mv88e6xxx: Default CMODE to 1000BaseX only on 6390X

[Marek Behun]

On Thu, 24 Jan 2019 19:11:59 +0100
Andrew Lunn <andrew@lunn.ch> wrote:

> On Thu, Jan 24, 2019 at 07:04:51PM +0100, Marek Behun wrote:
> > What properties does the cpu port node need to contain to force it?
> > phy-mode = "2500base-x"; is not enough.  
> 
> Hi Marek
> 
> For DSA ports we have:
> 
>                                                 phy-mode =
> "rgmii-txid"; fixed-link {
>                                                         speed =
> <1000>; full-duplex;
>                                                 };
> 
> See dsa_port_fixed_link_register_of()
> 
>     Andrew

Hi Andrew,
the configuration
  phy-mode = "2500base-x";
  fixed-link {
    speed = <2500>;
    full-duplex;
  };
does not work, because swphy does not support speed=2500 (only 10, 100
and 1000).
  managed = "in-band-status";
does not work either.

If I use speed = <1000>, then the swphy is created correctly, cmode is
set correctly to 2500base-x, but speed register on the port is set to
1000, and the connection does not work.

The easiest way would probably be to implement swphy to support speed
2500. But I don't know what values should the simulated PHY registers
contain...

The function dsa_port_fixed_link_register_of creates this phy device,
adjusts the link and then calls  put_device(&phydev->mdio.dev);
Does this mean that the phy device is immediately destroyed?

Marek

Re: [PATCH net-next v1 1/2] net: dsa: mv88e6xxx: Default CMODE to 1000BaseX only on 6390X

[Florian Fainelli]

On 1/24/19 11:24 AM, Marek Behun wrote:
> On Thu, 24 Jan 2019 19:11:59 +0100
> Andrew Lunn <andrew@lunn.ch> wrote:
> 
>> On Thu, Jan 24, 2019 at 07:04:51PM +0100, Marek Behun wrote:
>>> What properties does the cpu port node need to contain to force it?
>>> phy-mode = "2500base-x"; is not enough.  
>>
>> Hi Marek
>>
>> For DSA ports we have:
>>
>>                                                 phy-mode =
>> "rgmii-txid"; fixed-link {
>>                                                         speed =
>> <1000>; full-duplex;
>>                                                 };
>>
>> See dsa_port_fixed_link_register_of()
>>
>>     Andrew
> 
> Hi Andrew,
> the configuration
>   phy-mode = "2500base-x";
>   fixed-link {
>     speed = <2500>;
>     full-duplex;
>   };
> does not work, because swphy does not support speed=2500 (only 10, 100
> and 1000).
>   managed = "in-band-status";
> does not work either.
> 
> If I use speed = <1000>, then the swphy is created correctly, cmode is
> set correctly to 2500base-x, but speed register on the port is set to
> 1000, and the connection does not work.
> 
> The easiest way would probably be to implement swphy to support speed
> 2500. But I don't know what values should the simulated PHY registers
> contain...
> 
> The function dsa_port_fixed_link_register_of creates this phy device,
> adjusts the link and then calls  put_device(&phydev->mdio.dev);
> Does this mean that the phy device is immediately destroyed?

Yes, we should actually migrate that code over to PHYLINK, because in
PHYLINK the fixed links don't require creating a phy_device instance.
This is something that has a potential of breaking a lot of people, so I
have not really started doing it just yet :)
-- 
Florian

Re: [PATCH net-next v1 1/2] net: dsa: mv88e6xxx: Default CMODE to 1000BaseX only on 6390X

[Andrew Lunn]

> > The function dsa_port_fixed_link_register_of creates this phy device,
> > adjusts the link and then calls  put_device(&phydev->mdio.dev);
> > Does this mean that the phy device is immediately destroyed?
> 
> Yes, we should actually migrate that code over to PHYLINK, because in
> PHYLINK the fixed links don't require creating a phy_device instance.
> This is something that has a potential of breaking a lot of people, so I
> have not really started doing it just yet :)

Hi Florian

It also has the advantage of being easier to implement. In order to
support 2.5G, i think we need C45 support, etc in the simulator.
Avoiding that would be nice.

	 Andrew

Re: [PATCH net-next v1 1/2] net: dsa: mv88e6xxx: Default CMODE to 1000BaseX only on 6390X

[Marek Behun]

On Thu, 24 Jan 2019 11:27:54 -0800
Florian Fainelli <f.fainelli@gmail.com> wrote:

> On 1/24/19 11:24 AM, Marek Behun wrote:
> > On Thu, 24 Jan 2019 19:11:59 +0100
> > Andrew Lunn <andrew@lunn.ch> wrote:
> >   
> >> On Thu, Jan 24, 2019 at 07:04:51PM +0100, Marek Behun wrote:  
> >>> What properties does the cpu port node need to contain to force
> >>> it? phy-mode = "2500base-x"; is not enough.    
> >>
> >> Hi Marek
> >>
> >> For DSA ports we have:
> >>
> >>                                                 phy-mode =
> >> "rgmii-txid"; fixed-link {
> >>                                                         speed =
> >> <1000>; full-duplex;
> >>                                                 };
> >>
> >> See dsa_port_fixed_link_register_of()
> >>
> >>     Andrew  
> > 
> > Hi Andrew,
> > the configuration
> >   phy-mode = "2500base-x";
> >   fixed-link {
> >     speed = <2500>;
> >     full-duplex;
> >   };
> > does not work, because swphy does not support speed=2500 (only 10,
> > 100 and 1000).
> >   managed = "in-band-status";
> > does not work either.
> > 
> > If I use speed = <1000>, then the swphy is created correctly, cmode
> > is set correctly to 2500base-x, but speed register on the port is
> > set to 1000, and the connection does not work.
> > 
> > The easiest way would probably be to implement swphy to support
> > speed 2500. But I don't know what values should the simulated PHY
> > registers contain...
> > 
> > The function dsa_port_fixed_link_register_of creates this phy
> > device, adjusts the link and then calls
> > put_device(&phydev->mdio.dev); Does this mean that the phy device
> > is immediately destroyed?  
> 
> Yes, we should actually migrate that code over to PHYLINK, because in
> PHYLINK the fixed links don't require creating a phy_device instance.
> This is something that has a potential of breaking a lot of people,
> so I have not really started doing it just yet :)

phylink_create requires a net_device as first argument. what should be
the device for cpu/dsa ports?

Marek

Re: [PATCH net-next v1 1/2] net: dsa: mv88e6xxx: Default CMODE to 1000BaseX only on 6390X

[Florian Fainelli]

On 1/24/19 11:37 AM, Marek Behun wrote:
> On Thu, 24 Jan 2019 11:27:54 -0800
> Florian Fainelli <f.fainelli@gmail.com> wrote:
> 
>> On 1/24/19 11:24 AM, Marek Behun wrote:
>>> On Thu, 24 Jan 2019 19:11:59 +0100
>>> Andrew Lunn <andrew@lunn.ch> wrote:
>>>   
>>>> On Thu, Jan 24, 2019 at 07:04:51PM +0100, Marek Behun wrote:  
>>>>> What properties does the cpu port node need to contain to force
>>>>> it? phy-mode = "2500base-x"; is not enough.    
>>>>
>>>> Hi Marek
>>>>
>>>> For DSA ports we have:
>>>>
>>>>                                                 phy-mode =
>>>> "rgmii-txid"; fixed-link {
>>>>                                                         speed =
>>>> <1000>; full-duplex;
>>>>                                                 };
>>>>
>>>> See dsa_port_fixed_link_register_of()
>>>>
>>>>     Andrew  
>>>
>>> Hi Andrew,
>>> the configuration
>>>   phy-mode = "2500base-x";
>>>   fixed-link {
>>>     speed = <2500>;
>>>     full-duplex;
>>>   };
>>> does not work, because swphy does not support speed=2500 (only 10,
>>> 100 and 1000).
>>>   managed = "in-band-status";
>>> does not work either.
>>>
>>> If I use speed = <1000>, then the swphy is created correctly, cmode
>>> is set correctly to 2500base-x, but speed register on the port is
>>> set to 1000, and the connection does not work.
>>>
>>> The easiest way would probably be to implement swphy to support
>>> speed 2500. But I don't know what values should the simulated PHY
>>> registers contain...
>>>
>>> The function dsa_port_fixed_link_register_of creates this phy
>>> device, adjusts the link and then calls
>>> put_device(&phydev->mdio.dev); Does this mean that the phy device
>>> is immediately destroyed?  
>>
>> Yes, we should actually migrate that code over to PHYLINK, because in
>> PHYLINK the fixed links don't require creating a phy_device instance.
>> This is something that has a potential of breaking a lot of people,
>> so I have not really started doing it just yet :)
> 
> phylink_create requires a net_device as first argument. what should be
> the device for cpu/dsa ports?

We would have to somehow change that assumption or introduce some low
level function for PHYLINK that are just meant to deal with the phylink
object, without the netdev, that is among other things why it has not
happened yet.
-- 
Florian

Re: [PATCH net-next v1 1/2] net: dsa: mv88e6xxx: Default CMODE to 1000BaseX only on 6390X

[Marek Behun]

On Thu, 24 Jan 2019 11:27:54 -0800
Florian Fainelli <f.fainelli@gmail.com> wrote:

> On 1/24/19 11:24 AM, Marek Behun wrote:
> > On Thu, 24 Jan 2019 19:11:59 +0100
> > Andrew Lunn <andrew@lunn.ch> wrote:
> >   
> >> On Thu, Jan 24, 2019 at 07:04:51PM +0100, Marek Behun wrote:  
> >>> What properties does the cpu port node need to contain to force
> >>> it? phy-mode = "2500base-x"; is not enough.    
> >>
> >> Hi Marek
> >>
> >> For DSA ports we have:
> >>
> >>                                                 phy-mode =
> >> "rgmii-txid"; fixed-link {
> >>                                                         speed =
> >> <1000>; full-duplex;
> >>                                                 };
> >>
> >> See dsa_port_fixed_link_register_of()
> >>
> >>     Andrew  
> > 
> > Hi Andrew,
> > the configuration
> >   phy-mode = "2500base-x";
> >   fixed-link {
> >     speed = <2500>;
> >     full-duplex;
> >   };
> > does not work, because swphy does not support speed=2500 (only 10,
> > 100 and 1000).
> >   managed = "in-band-status";
> > does not work either.
> > 
> > If I use speed = <1000>, then the swphy is created correctly, cmode
> > is set correctly to 2500base-x, but speed register on the port is
> > set to 1000, and the connection does not work.
> > 
> > The easiest way would probably be to implement swphy to support
> > speed 2500. But I don't know what values should the simulated PHY
> > registers contain...
> > 
> > The function dsa_port_fixed_link_register_of creates this phy
> > device, adjusts the link and then calls
> > put_device(&phydev->mdio.dev); Does this mean that the phy device
> > is immediately destroyed?  
> 
> Yes, we should actually migrate that code over to PHYLINK, because in
> PHYLINK the fixed links don't require creating a phy_device instance.
> This is something that has a potential of breaking a lot of people,
> so I have not really started doing it just yet :)

Can't then this patch be applied so that Turris Mox will work again? At
least till the cpu/dsa port connection is converted to phylink.
Marek

Re: [PATCH net-next v1 1/2] net: dsa: mv88e6xxx: Default CMODE to 1000BaseX only on 6390X

[Florian Fainelli]

On 1/24/19 11:39 AM, Marek Behun wrote:
> On Thu, 24 Jan 2019 11:27:54 -0800
> Florian Fainelli <f.fainelli@gmail.com> wrote:
> 
>> On 1/24/19 11:24 AM, Marek Behun wrote:
>>> On Thu, 24 Jan 2019 19:11:59 +0100
>>> Andrew Lunn <andrew@lunn.ch> wrote:
>>>   
>>>> On Thu, Jan 24, 2019 at 07:04:51PM +0100, Marek Behun wrote:  
>>>>> What properties does the cpu port node need to contain to force
>>>>> it? phy-mode = "2500base-x"; is not enough.    
>>>>
>>>> Hi Marek
>>>>
>>>> For DSA ports we have:
>>>>
>>>>                                                 phy-mode =
>>>> "rgmii-txid"; fixed-link {
>>>>                                                         speed =
>>>> <1000>; full-duplex;
>>>>                                                 };
>>>>
>>>> See dsa_port_fixed_link_register_of()
>>>>
>>>>     Andrew  
>>>
>>> Hi Andrew,
>>> the configuration
>>>   phy-mode = "2500base-x";
>>>   fixed-link {
>>>     speed = <2500>;
>>>     full-duplex;
>>>   };
>>> does not work, because swphy does not support speed=2500 (only 10,
>>> 100 and 1000).
>>>   managed = "in-band-status";
>>> does not work either.
>>>
>>> If I use speed = <1000>, then the swphy is created correctly, cmode
>>> is set correctly to 2500base-x, but speed register on the port is
>>> set to 1000, and the connection does not work.
>>>
>>> The easiest way would probably be to implement swphy to support
>>> speed 2500. But I don't know what values should the simulated PHY
>>> registers contain...
>>>
>>> The function dsa_port_fixed_link_register_of creates this phy
>>> device, adjusts the link and then calls
>>> put_device(&phydev->mdio.dev); Does this mean that the phy device
>>> is immediately destroyed?  
>>
>> Yes, we should actually migrate that code over to PHYLINK, because in
>> PHYLINK the fixed links don't require creating a phy_device instance.
>> This is something that has a potential of breaking a lot of people,
>> so I have not really started doing it just yet :)
> 
> Can't then this patch be applied so that Turris Mox will work again? At
> least till the cpu/dsa port connection is converted to phylink.

How about the following hack until we can support netdev less PHYLINK
instances while having the Device Tree being corrected to have the
correct phy-mode = "2500base-x" property since we will need it for
PHYLINK on CPU/DSA ports later on:

diff --git a/net/dsa/port.c b/net/dsa/port.c
index 2d7e01b23572..9ea052c30b68 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -359,6 +359,14 @@ static int dsa_port_fixed_link_register_of(struct
dsa_port *dp)
        if (mode < 0)
                mode = PHY_INTERFACE_MODE_NA;
        phydev->interface = mode;
+       switch (mode) {
+       case PHY_INTERFACE_MODE_2500BASEX:
+               phydev->speed = SPEED_2500;
+               break;
+       case PHY_INTERFACE_MODE_10GKR:
+               phydev->speed = SPEED_10000;
+               break;
+       }

        genphy_config_init(phydev);
        genphy_read_status(phydev);
-- 
Florian

Re: [PATCH net-next v1 1/2] net: dsa: mv88e6xxx: Default CMODE to 1000BaseX only on 6390X

[Marek Behun]

On Fri, 25 Jan 2019 10:48:38 -0800
Florian Fainelli <f.fainelli@gmail.com> wrote:

> On 1/24/19 11:39 AM, Marek Behun wrote:
> > On Thu, 24 Jan 2019 11:27:54 -0800
> > Florian Fainelli <f.fainelli@gmail.com> wrote:
> >   
> >> On 1/24/19 11:24 AM, Marek Behun wrote:  
> >>> On Thu, 24 Jan 2019 19:11:59 +0100
> >>> Andrew Lunn <andrew@lunn.ch> wrote:
> >>>     
> >>>> On Thu, Jan 24, 2019 at 07:04:51PM +0100, Marek Behun wrote:    
> >>>>> What properties does the cpu port node need to contain to force
> >>>>> it? phy-mode = "2500base-x"; is not enough.      
> >>>>
> >>>> Hi Marek
> >>>>
> >>>> For DSA ports we have:
> >>>>
> >>>>                                                 phy-mode =
> >>>> "rgmii-txid"; fixed-link {
> >>>>                                                         speed =
> >>>> <1000>; full-duplex;
> >>>>                                                 };
> >>>>
> >>>> See dsa_port_fixed_link_register_of()
> >>>>
> >>>>     Andrew    
> >>>
> >>> Hi Andrew,
> >>> the configuration
> >>>   phy-mode = "2500base-x";
> >>>   fixed-link {
> >>>     speed = <2500>;
> >>>     full-duplex;
> >>>   };
> >>> does not work, because swphy does not support speed=2500 (only 10,
> >>> 100 and 1000).
> >>>   managed = "in-band-status";
> >>> does not work either.
> >>>
> >>> If I use speed = <1000>, then the swphy is created correctly,
> >>> cmode is set correctly to 2500base-x, but speed register on the
> >>> port is set to 1000, and the connection does not work.
> >>>
> >>> The easiest way would probably be to implement swphy to support
> >>> speed 2500. But I don't know what values should the simulated PHY
> >>> registers contain...
> >>>
> >>> The function dsa_port_fixed_link_register_of creates this phy
> >>> device, adjusts the link and then calls
> >>> put_device(&phydev->mdio.dev); Does this mean that the phy device
> >>> is immediately destroyed?    
> >>
> >> Yes, we should actually migrate that code over to PHYLINK, because
> >> in PHYLINK the fixed links don't require creating a phy_device
> >> instance. This is something that has a potential of breaking a lot
> >> of people, so I have not really started doing it just yet :)  
> > 
> > Can't then this patch be applied so that Turris Mox will work
> > again? At least till the cpu/dsa port connection is converted to
> > phylink.  
> 
> How about the following hack until we can support netdev less PHYLINK
> instances while having the Device Tree being corrected to have the
> correct phy-mode = "2500base-x" property since we will need it for
> PHYLINK on CPU/DSA ports later on:
> 
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index 2d7e01b23572..9ea052c30b68 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -359,6 +359,14 @@ static int dsa_port_fixed_link_register_of(struct
> dsa_port *dp)
>         if (mode < 0)
>                 mode = PHY_INTERFACE_MODE_NA;
>         phydev->interface = mode;
> +       switch (mode) {
> +       case PHY_INTERFACE_MODE_2500BASEX:
> +               phydev->speed = SPEED_2500;
> +               break;
> +       case PHY_INTERFACE_MODE_10GKR:
> +               phydev->speed = SPEED_10000;
> +               break;
> +       }
> 
>         genphy_config_init(phydev);
>         genphy_read_status(phydev);

But in the dts there would still have to be speed=<1000>; otherwise
swphy will fail...

Re: [PATCH net-next v1 1/2] net: dsa: mv88e6xxx: Default CMODE to 1000BaseX only on 6390X

[Florian Fainelli]

On 1/24/19 7:43 AM, Marek Behún wrote:
> Commit 787799a9d555 sets the SERDES interfaces of 6390 and 6390X to
> 1000BaseX, but this is only needed on 6390X, since there are SERDES
> interfaces which can be used on lower ports on 6390.
> 
> This commit fixes this by returning to previous behaviour on 6390.
> (Previous behaviour means that CMODE is not set at all if requested mode
> is NA).
> 
> This is needed on Turris MOX, where the 88e6190 is connected to CPU in
> 2500BaseX mode.
> 
> Fixes: 787799a9d555 ("net: dsa: mv88e6xxx: Default ports 9/10 6390X CMODE to 1000BaseX")
> Signed-off-by: Marek Behún <marek.behun@nic.cz>

I suppose for now, this is the best way to approach that problem given
the shortcomings of the fixed link support in net/dsa/port.c:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

Thanks!
-- 
Florian