linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net: phy: at803x: Register 'link_change_notify' only for AT8030
@ 2016-04-27 11:34 Sebastian Frias
  2016-04-27 13:50 ` Sebastian Frias
  0 siblings, 1 reply; 2+ messages in thread
From: Sebastian Frias @ 2016-04-27 11:34 UTC (permalink / raw)
  To: Daniel Mack, David S. Miller, netdev; +Cc: lkml, mason, Sergei Shtylyov

There is no need to register the callback introduced by
commit 13a56b449325 ("net: phy: at803x: Add support for hardware reset")
for non faulty PHYs.

The check on the PHY ID is not necessary anymore and thus has been
removed from the callback implementation as well.

Fixes: 13a56b449325 ("net: phy: at803x: Add support for hardware reset")

Signed-off-by: Sebastian Frias <sf84@laposte.net>
---
 drivers/net/phy/at803x.c | 43 ++++++++++++++++++++-----------------------
 1 file changed, 20 insertions(+), 23 deletions(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index b3ffaee..7fdc676 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -353,33 +353,32 @@ static void at803x_link_change_notify(struct phy_device *phydev)
 	struct at803x_priv *priv = phydev->priv;
 
 	/*
-	 * Conduct a hardware reset for AT8030 every time a link loss is
+	 * Conduct a hardware reset for AT8030 (this callback is only
+	 * registered for AT8030 at the moment) every time a link loss is
 	 * signalled. This is necessary to circumvent a hardware bug that
 	 * occurs when the cable is unplugged while TX packets are pending
 	 * in the FIFO. In such cases, the FIFO enters an error mode it
 	 * cannot recover from by software.
 	 */
-	if (phydev->drv->phy_id == ATH8030_PHY_ID) {
-		if (phydev->state == PHY_NOLINK) {
-			if (priv->gpiod_reset && !priv->phy_reset) {
-				struct at803x_context context;
-
-				at803x_context_save(phydev, &context);
-
-				gpiod_set_value(priv->gpiod_reset, 1);
-				msleep(1);
-				gpiod_set_value(priv->gpiod_reset, 0);
-				msleep(1);
-
-				at803x_context_restore(phydev, &context);
-
-				phydev_dbg(phydev, "%s(): phy was reset\n",
-					   __func__);
-				priv->phy_reset = true;
-			}
-		} else {
-			priv->phy_reset = false;
+	if (phydev->state == PHY_NOLINK) {
+		if (priv->gpiod_reset && !priv->phy_reset) {
+			struct at803x_context context;
+
+			at803x_context_save(phydev, &context);
+
+			gpiod_set_value(priv->gpiod_reset, 1);
+			msleep(1);
+			gpiod_set_value(priv->gpiod_reset, 0);
+			msleep(1);
+
+			at803x_context_restore(phydev, &context);
+
+			phydev_dbg(phydev, "%s(): phy was reset\n",
+				   __func__);
+			priv->phy_reset = true;
 		}
+	} else {
+		priv->phy_reset = false;
 	}
 }
 
@@ -391,7 +390,6 @@ static struct phy_driver at803x_driver[] = {
 	.phy_id_mask		= 0xffffffef,
 	.probe			= at803x_probe,
 	.config_init		= at803x_config_init,
-	.link_change_notify	= at803x_link_change_notify,
 	.set_wol		= at803x_set_wol,
 	.get_wol		= at803x_get_wol,
 	.suspend		= at803x_suspend,
@@ -427,7 +425,6 @@ static struct phy_driver at803x_driver[] = {
 	.phy_id_mask		= 0xffffffef,
 	.probe			= at803x_probe,
 	.config_init		= at803x_config_init,
-	.link_change_notify	= at803x_link_change_notify,
 	.set_wol		= at803x_set_wol,
 	.get_wol		= at803x_get_wol,
 	.suspend		= at803x_suspend,
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] net: phy: at803x: Register 'link_change_notify' only for AT8030
  2016-04-27 11:34 [PATCH] net: phy: at803x: Register 'link_change_notify' only for AT8030 Sebastian Frias
@ 2016-04-27 13:50 ` Sebastian Frias
  0 siblings, 0 replies; 2+ messages in thread
From: Sebastian Frias @ 2016-04-27 13:50 UTC (permalink / raw)
  To: Daniel Mack, David S. Miller, netdev
  Cc: lkml, mason, Sergei Shtylyov, timur, Florian Fainelli

Hi,

Sergei pointed out that the same patch was submitted yesterday by Timur (what are the chances?! :-) ) https://patchwork.ozlabs.org/patch/615019/

Regards,

Sebastian

On 04/27/2016 01:34 PM, Sebastian Frias wrote:
> There is no need to register the callback introduced by
> commit 13a56b449325 ("net: phy: at803x: Add support for hardware reset")
> for non faulty PHYs.
> 
> The check on the PHY ID is not necessary anymore and thus has been
> removed from the callback implementation as well.
> 
> Fixes: 13a56b449325 ("net: phy: at803x: Add support for hardware reset")
> 
> Signed-off-by: Sebastian Frias <sf84@laposte.net>
> ---
>  drivers/net/phy/at803x.c | 43 ++++++++++++++++++++-----------------------
>  1 file changed, 20 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index b3ffaee..7fdc676 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -353,33 +353,32 @@ static void at803x_link_change_notify(struct phy_device *phydev)
>  	struct at803x_priv *priv = phydev->priv;
>  
>  	/*
> -	 * Conduct a hardware reset for AT8030 every time a link loss is
> +	 * Conduct a hardware reset for AT8030 (this callback is only
> +	 * registered for AT8030 at the moment) every time a link loss is
>  	 * signalled. This is necessary to circumvent a hardware bug that
>  	 * occurs when the cable is unplugged while TX packets are pending
>  	 * in the FIFO. In such cases, the FIFO enters an error mode it
>  	 * cannot recover from by software.
>  	 */
> -	if (phydev->drv->phy_id == ATH8030_PHY_ID) {
> -		if (phydev->state == PHY_NOLINK) {
> -			if (priv->gpiod_reset && !priv->phy_reset) {
> -				struct at803x_context context;
> -
> -				at803x_context_save(phydev, &context);
> -
> -				gpiod_set_value(priv->gpiod_reset, 1);
> -				msleep(1);
> -				gpiod_set_value(priv->gpiod_reset, 0);
> -				msleep(1);
> -
> -				at803x_context_restore(phydev, &context);
> -
> -				phydev_dbg(phydev, "%s(): phy was reset\n",
> -					   __func__);
> -				priv->phy_reset = true;
> -			}
> -		} else {
> -			priv->phy_reset = false;
> +	if (phydev->state == PHY_NOLINK) {
> +		if (priv->gpiod_reset && !priv->phy_reset) {
> +			struct at803x_context context;
> +
> +			at803x_context_save(phydev, &context);
> +
> +			gpiod_set_value(priv->gpiod_reset, 1);
> +			msleep(1);
> +			gpiod_set_value(priv->gpiod_reset, 0);
> +			msleep(1);
> +
> +			at803x_context_restore(phydev, &context);
> +
> +			phydev_dbg(phydev, "%s(): phy was reset\n",
> +				   __func__);
> +			priv->phy_reset = true;
>  		}
> +	} else {
> +		priv->phy_reset = false;
>  	}
>  }
>  
> @@ -391,7 +390,6 @@ static struct phy_driver at803x_driver[] = {
>  	.phy_id_mask		= 0xffffffef,
>  	.probe			= at803x_probe,
>  	.config_init		= at803x_config_init,
> -	.link_change_notify	= at803x_link_change_notify,
>  	.set_wol		= at803x_set_wol,
>  	.get_wol		= at803x_get_wol,
>  	.suspend		= at803x_suspend,
> @@ -427,7 +425,6 @@ static struct phy_driver at803x_driver[] = {
>  	.phy_id_mask		= 0xffffffef,
>  	.probe			= at803x_probe,
>  	.config_init		= at803x_config_init,
> -	.link_change_notify	= at803x_link_change_notify,
>  	.set_wol		= at803x_set_wol,
>  	.get_wol		= at803x_get_wol,
>  	.suspend		= at803x_suspend,
> 

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2016-04-27 13:51 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-27 11:34 [PATCH] net: phy: at803x: Register 'link_change_notify' only for AT8030 Sebastian Frias
2016-04-27 13:50 ` Sebastian Frias

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).