Re: [RFC 2/8] net-next: phy: dont auto handle carrier state when multiple phys are attached

[Florian Fainelli]

On 22/11/15 00:40, John Crispin wrote:
> A network core might have more than one phy attached to its cpu port via a
> switch. The current code will set the carrier state to on/off when ever a
> cable is plugged into any of these ports.

Not really, no. The current PHY library implementation does not allow
more than one net_device instance to be bound to multiple PHY devices.
Since this is an integrated switch, you should really expose per-port
network devices, that is the paradigm we settled down on using for
Ethernet switches now (irrespective of using DSA or switchdev, or both).

> 
> The patch adds a new bool that allows the driver to tell the phy_device to not
> set the carrier state. Instead the driver can manually handle the carrier
> state.

I am missing the bigger picture of how this is used, also, if link down
is a problem, would not link up be for the same reasons?

> 
> Signed-off-by: John Crispin 
> Signed-off-by: Felix Fietkau 
> Signed-off-by: Michael Lee 
> Cc: Florian Fainelli 
> ---
>  drivers/net/phy/phy.c |9 ++---
>  include/linux/phy.h   |1 +
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 48ce6ef..bd2df40 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -843,7 +843,8 @@ void phy_state_machine(struct work_struct *work)
>   /* If the link is down, give up on negotiation for now */
>   if (!phydev->link) {
>   phydev->state = PHY_NOLINK;
> - netif_carrier_off(phydev->attached_dev);
> + if (!phydev->no_auto_carrier_off)
> + netif_carrier_off(phydev->attached_dev);
>   phydev->adjust_link(phydev->attached_dev);
>   break;
>   }
> @@ -926,7 +927,8 @@ void phy_state_machine(struct work_struct *work)
>   netif_carrier_on(phydev->attached_dev);
>   } else {
>   phydev->state = PHY_NOLINK;
> - netif_carrier_off(phydev->attached_dev);
> + if (!phydev->no_auto_carrier_off)
> + netif_carrier_off(phydev->attached_dev);
>   }
>  
>   phydev->adjust_link(phydev->attached_dev);
> @@ -938,7 +940,8 @@ void phy_state_machine(struct work_struct *work)
>   case PHY_HALTED:
>   if (phydev->link) {
>   phydev->link = 0;
> - netif_carrier_off(phydev->attached_dev);
> + if (!phydev->no_auto_carrier_off)
> + netif_carrier_off(phydev->attached_dev);
>   phydev->adjust_link(phydev->attached_dev);
>   do_suspend = true;
>   }
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 05fde31..276ab8a 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -377,6 +377,7 @@ struct phy_device {
>   bool is_pseudo_fixed_link;
>   bool has_fixups;
>   bool suspended;
> + bool no_auto_carrier_off;
>  
>   enum phy_state state;
>  
> 


-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC 2/8] net-next: phy: dont auto handle carrier state when multiple phys are attached

[John Crispin]

A network core might have more than one phy attached to its cpu port via a
switch. The current code will set the carrier state to on/off when ever a
cable is plugged into any of these ports.

The patch adds a new bool that allows the driver to tell the phy_device to not
set the carrier state. Instead the driver can manually handle the carrier
state.

Signed-off-by: John Crispin 
Signed-off-by: Felix Fietkau 
Signed-off-by: Michael Lee 
Cc: Florian Fainelli 
---
 drivers/net/phy/phy.c |9 ++---
 include/linux/phy.h   |1 +
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 48ce6ef..bd2df40 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -843,7 +843,8 @@ void phy_state_machine(struct work_struct *work)
/* If the link is down, give up on negotiation for now */
if (!phydev->link) {
phydev->state = PHY_NOLINK;
-   netif_carrier_off(phydev->attached_dev);
+   if (!phydev->no_auto_carrier_off)
+   netif_carrier_off(phydev->attached_dev);
phydev->adjust_link(phydev->attached_dev);
break;
}
@@ -926,7 +927,8 @@ void phy_state_machine(struct work_struct *work)
netif_carrier_on(phydev->attached_dev);
} else {
phydev->state = PHY_NOLINK;
-   netif_carrier_off(phydev->attached_dev);
+   if (!phydev->no_auto_carrier_off)
+   netif_carrier_off(phydev->attached_dev);
}
 
phydev->adjust_link(phydev->attached_dev);
@@ -938,7 +940,8 @@ void phy_state_machine(struct work_struct *work)
case PHY_HALTED:
if (phydev->link) {
phydev->link = 0;
-   netif_carrier_off(phydev->attached_dev);
+   if (!phydev->no_auto_carrier_off)
+   netif_carrier_off(phydev->attached_dev);
phydev->adjust_link(phydev->attached_dev);
do_suspend = true;
}
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 05fde31..276ab8a 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -377,6 +377,7 @@ struct phy_device {
bool is_pseudo_fixed_link;
bool has_fixups;
bool suspended;
+   bool no_auto_carrier_off;
 
enum phy_state state;
 
-- 
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html