[RFC,net-next,5/5] net: dsa: mt7530: Add mediatek,ephy-handle to isolate external phy

On some platforms the external phy can only interface with the port 5 of
the switch because the xMII TX and RX lines are swapped. But it still can
be useful to use the internal phy of the switch to act as a WAN port which
connectes to the 2nd GMAC. This gives the SOC a double the bandwidth
between LAN and WAN. Because LAN and WAN don't share the same interface
anymore.

By adding an optional property mediatek,ephy-handle, the external phy
is put in isolation mode when internal phy is linked with 2nd GMAC via
phy-handle property.

Signed-off-by: René van Dorst <opensource@vdorst.com>
---
 drivers/net/dsa/mt7530.c | 28 ++++++++++++++++++++++++++++
 drivers/net/dsa/mt7530.h |  2 ++
 2 files changed, 30 insertions(+)

> +static int mt7530_isolate_ephy(struct dsa_switch *ds,
> +			       struct device_node *ephy_node)
> +{
> +	struct phy_device *phydev = of_phy_find_device(ephy_node);
> +	int ret;
> +
> +	if (!phydev)
> +		return 0;
> +
> +	ret = phy_modify(phydev, MII_BMCR, 0, (BMCR_ISOLATE | BMCR_PDOWN));

genphy_suspend() does what you want.

> +	if (ret)
> +		dev_err(ds->dev, "Failed to put phy %s in isolation mode!\n",
> +			ephy_node->full_name);
> +	else
> +		dev_info(ds->dev, "Phy %s in isolation mode!\n",
> +			 ephy_node->full_name);

No need to clog up the system with yet more kernel messages.

   Andrew

On 6/24/19 4:52 PM, Andrew Lunn wrote:
>> +static int mt7530_isolate_ephy(struct dsa_switch *ds,
>> +			       struct device_node *ephy_node)
>> +{
>> +	struct phy_device *phydev = of_phy_find_device(ephy_node);
>> +	int ret;
>> +
>> +	if (!phydev)
>> +		return 0;
>> +
>> +	ret = phy_modify(phydev, MII_BMCR, 0, (BMCR_ISOLATE | BMCR_PDOWN));
> genphy_suspend() does what you want.
>
>> +	if (ret)
>> +		dev_err(ds->dev, "Failed to put phy %s in isolation mode!\n",
>> +			ephy_node->full_name);
>> +	else
>> +		dev_info(ds->dev, "Phy %s in isolation mode!\n",
>> +			 ephy_node->full_name);
> No need to clog up the system with yet more kernel messages.
>
>    Andrew
>
Yes, keep in mind that many mt7530-based devices have a 56k serial
console that gets ring buffer spew.  This created a real problem on the
mt7620 wifi drivers when they spewed every time a packet needed to be
dropped.  So at the very least, for any message that can be spammed,
rate limit it please.

Daniel

Quoting Andrew Lunn <andrew@lunn.ch>:

Hi Andrew,

>> +static int mt7530_isolate_ephy(struct dsa_switch *ds,
>> +			       struct device_node *ephy_node)
>> +{
>> +	struct phy_device *phydev = of_phy_find_device(ephy_node);
>> +	int ret;
>> +
>> +	if (!phydev)
>> +		return 0;
>> +
>> +	ret = phy_modify(phydev, MII_BMCR, 0, (BMCR_ISOLATE | BMCR_PDOWN));
>
> genphy_suspend() does what you want.

In case my device has AT8033 PHY which act as a RGMII-to-SGMII  
converter for the
SFP cage.

Qoute of the AR8031/33 datasheet:

The AR8033 device supports the low power mode with software power-down.
To enter the standard IEEE power-down mode, set the bit[11] POWER_DOWN of
Control register - copper page or Control register — fiber page to 1.
In this mode, AR8033 ignores all MAC interface signals except the MDC/MDIO and
does not respond to any activity on the media side. AR8033 cannot wake  
up on its
own and is only waken up by setting the POWER_DOWN bit to 0.


Does "standard IEEE power-down mode" describ this behavior that in power-down
mode the RGMII are also put in tri-state?

Reading the datasheet does not give me any clues.
Putting RGMII signals in tri-state is important in this case.

>
>> +	if (ret)
>> +		dev_err(ds->dev, "Failed to put phy %s in isolation mode!\n",
>> +			ephy_node->full_name);
>> +	else
>> +		dev_info(ds->dev, "Phy %s in isolation mode!\n",
>> +			 ephy_node->full_name);
>
> No need to clog up the system with yet more kernel messages.

OK, I remove it.

>
>    Andrew

Greats,

René