[PATCH] net: phy: marvell: Enable interrupt function on LED2 pin

[PATCH] net: phy: marvell: Enable interrupt function on LED2 pin

[Esben Haabendal]

From: Esben Haabendal <eha@deif.com>

The LED2[2]/INTn pin on Marvell 88E1318S as well as 88E1510/12/14/18 needs
to be configured to be usable as interrupt not only when WOL is enabled,
but whenever we rely on interrupts from the PHY.

Signed-off-by: Esben Haabendal <eha@deif.com>
Cc: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/net/phy/marvell.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 0e0978d8a0eb..f03a510f1247 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -457,6 +457,21 @@ static int marvell_of_reg_init(struct phy_device *phydev)
 }
 #endif /* CONFIG_OF_MDIO */
 
+static int m88e1318_config_intr(struct phy_device *phydev)
+{
+	int err;
+
+	err = marvell_config_intr(phydev);
+	if (err)
+		return err;
+
+	/* Setup LED[2] as interrupt pin (active low) */
+	return phy_modify(phydev, MII_88E1318S_PHY_LED_TCR,
+			  MII_88E1318S_PHY_LED_TCR_FORCE_INT,
+			  MII_88E1318S_PHY_LED_TCR_INTn_ENABLE |
+			  MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW);
+}
+
 static int m88e1121_config_aneg_rgmii_delays(struct phy_device *phydev)
 {
 	int mscr;
@@ -2090,7 +2105,7 @@ static struct phy_driver marvell_drivers[] = {
 		.config_aneg = &m88e1318_config_aneg,
 		.read_status = &marvell_read_status,
 		.ack_interrupt = &marvell_ack_interrupt,
-		.config_intr = &marvell_config_intr,
+		.config_intr = &m88e1318_config_intr,
 		.did_interrupt = &m88e1121_did_interrupt,
 		.get_wol = &m88e1318_get_wol,
 		.set_wol = &m88e1318_set_wol,
@@ -2189,7 +2204,7 @@ static struct phy_driver marvell_drivers[] = {
 		.config_aneg = &m88e1510_config_aneg,
 		.read_status = &marvell_read_status,
 		.ack_interrupt = &marvell_ack_interrupt,
-		.config_intr = &marvell_config_intr,
+		.config_intr = &m88e1318_config_intr,
 		.did_interrupt = &m88e1121_did_interrupt,
 		.get_wol = &m88e1318_get_wol,
 		.set_wol = &m88e1318_set_wol,
-- 
2.16.3

RE: [PATCH] net: phy: marvell: Enable interrupt function on LED2 pin

[Bhadram Varka]

Hi Esben,

-----Original Message-----
From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On Behalf Of Esben Haabendal
Sent: Thursday, April 05, 2018 7:05 PM
To: netdev@vger.kernel.org
Cc: Esben Haabendal <eha@deif.com>; Rasmus Villemoes <rasmus.villemoes@prevas.dk>; Andrew Lunn <andrew@lunn.ch>; Florian Fainelli <f.fainelli@gmail.com>; open list <linux-kernel@vger.kernel.org>
Subject: [PATCH] net: phy: marvell: Enable interrupt function on LED2 pin

From: Esben Haabendal <eha@deif.com>

The LED2[2]/INTn pin on Marvell 88E1318S as well as 88E1510/12/14/18 needs to be configured to be usable as interrupt not only when WOL is enabled, but whenever we rely on interrupts from the PHY.

Signed-off-by: Esben Haabendal <eha@deif.com>
Cc: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/net/phy/marvell.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index 0e0978d8a0eb..f03a510f1247 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -457,6 +457,21 @@ static int marvell_of_reg_init(struct phy_device *phydev)  }  #endif /* CONFIG_OF_MDIO */
 
+static int m88e1318_config_intr(struct phy_device *phydev) {
+	int err;
+
+	err = marvell_config_intr(phydev);
+	if (err)
+		return err;
+
+	/* Setup LED[2] as interrupt pin (active low) */
+	return phy_modify(phydev, MII_88E1318S_PHY_LED_TCR,
+			  MII_88E1318S_PHY_LED_TCR_FORCE_INT,
+			  MII_88E1318S_PHY_LED_TCR_INTn_ENABLE |
+			  MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW);

Can we move this part of the code to m88e1121_config_init() ?

Every time whether we disable or enable the interrupts this part of code will execute.

Thanks!

Re: [PATCH] net: phy: marvell: Enable interrupt function on LED2 pin

[Andrew Lunn]

> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index 0e0978d8a0eb..f03a510f1247 100644
> --- a/drivers/net/phy/marvell.c
> +++ b/drivers/net/phy/marvell.c
> @@ -457,6 +457,21 @@ static int marvell_of_reg_init(struct phy_device *phydev)  }  #endif /* CONFIG_OF_MDIO */
>  
> +static int m88e1318_config_intr(struct phy_device *phydev) {
> +	int err;
> +
> +	err = marvell_config_intr(phydev);
> +	if (err)
> +		return err;
> +
> +	/* Setup LED[2] as interrupt pin (active low) */
> +	return phy_modify(phydev, MII_88E1318S_PHY_LED_TCR,
> +			  MII_88E1318S_PHY_LED_TCR_FORCE_INT,
> +			  MII_88E1318S_PHY_LED_TCR_INTn_ENABLE |
> +			  MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW);
> 
> Can we move this part of the code to m88e1121_config_init() ?
> 
> Every time whether we disable or enable the interrupts this part of code will execute.

Yes, doing this once would be better. But please allow the LED pin to
be used as an LED when not using interrupts. phy_interrupt_is_valid()
should be involved somehow.

       Andrew

[PATCH v2] net: phy: marvell: Enable interrupt function on LED2 pin

[Esben Haabendal]

From: Esben Haabendal <eha@deif.com>

The LED2[2]/INTn pin on Marvell 88E1318S as well as 88E1510/12/14/18 needs
to be configured to be usable as interrupt not only when WOL is enabled,
but whenever we rely on interrupts from the PHY.

Signed-off-by: Esben Haabendal <eha@deif.com>
Cc: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/net/phy/marvell.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 0e0978d8a0eb..a6ad0255c512 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -828,6 +828,22 @@ static int m88e1121_config_init(struct phy_device *phydev)
 	return marvell_config_init(phydev);
 }
 
+static int m88e1318_config_init(struct phy_device *phydev)
+{
+	if (phy_interrupt_is_valid(phydev)) {
+		int err = phy_modify_paged(
+			phydev, MII_MARVELL_LED_PAGE,
+			MII_88E1318S_PHY_LED_TCR,
+			MII_88E1318S_PHY_LED_TCR_FORCE_INT,
+			MII_88E1318S_PHY_LED_TCR_INTn_ENABLE |
+			MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW);
+		if (err < 0)
+			return err;
+	}
+
+	return m88e1121_config_init(phydev);
+}
+
 static int m88e1510_config_init(struct phy_device *phydev)
 {
 	int err;
@@ -870,7 +886,7 @@ static int m88e1510_config_init(struct phy_device *phydev)
 		phydev->advertising &= ~pause;
 	}
 
-	return m88e1121_config_init(phydev);
+	return m88e1318_config_init(phydev);
 }
 
 static int m88e1118_config_aneg(struct phy_device *phydev)
@@ -2086,7 +2102,7 @@ static struct phy_driver marvell_drivers[] = {
 		.features = PHY_GBIT_FEATURES,
 		.flags = PHY_HAS_INTERRUPT,
 		.probe = marvell_probe,
-		.config_init = &m88e1121_config_init,
+		.config_init = &m88e1318_config_init,
 		.config_aneg = &m88e1318_config_aneg,
 		.read_status = &marvell_read_status,
 		.ack_interrupt = &marvell_ack_interrupt,
-- 
2.16.3

Re: [PATCH] net: phy: marvell: Enable interrupt function on LED2 pin

[Esben Haabendal]

Andrew Lunn <andrew@lunn.ch> writes:

>> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index
>> 0e0978d8a0eb..f03a510f1247 100644
>> --- a/drivers/net/phy/marvell.c
>> +++ b/drivers/net/phy/marvell.c
>> @@ -457,6 +457,21 @@ static int marvell_of_reg_init(struct phy_device
>> *phydev) } #endif /* CONFIG_OF_MDIO */
>>  
>> +static int m88e1318_config_intr(struct phy_device *phydev) {
>> +	int err;
>> +
>> +	err = marvell_config_intr(phydev);
>> +	if (err)
>> +		return err;
>> +
>> +	/* Setup LED[2] as interrupt pin (active low) */
>> +	return phy_modify(phydev, MII_88E1318S_PHY_LED_TCR,
>> +			  MII_88E1318S_PHY_LED_TCR_FORCE_INT,
>> +			  MII_88E1318S_PHY_LED_TCR_INTn_ENABLE |
>> +			  MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW);
>> 
>> Can we move this part of the code to m88e1121_config_init() ?
>> 
>> Every time whether we disable or enable the interrupts this part of code
>> will execute.
>
> Yes, doing this once would be better. But please allow the LED pin to
> be used as an LED when not using interrupts. phy_interrupt_is_valid()
> should be involved somehow.

This should be addressed in v2 of the patch which I have already posted.

/Esben

Re: [PATCH v2] net: phy: marvell: Enable interrupt function on LED2 pin

[Andrew Lunn]

On Thu, Apr 05, 2018 at 10:40:29PM +0200, Esben Haabendal wrote:
> From: Esben Haabendal <eha@deif.com>
> 
> The LED2[2]/INTn pin on Marvell 88E1318S as well as 88E1510/12/14/18 needs
> to be configured to be usable as interrupt not only when WOL is enabled,
> but whenever we rely on interrupts from the PHY.
> 
> Signed-off-by: Esben Haabendal <eha@deif.com>
> Cc: Rasmus Villemoes <rasmus.villemoes@prevas.dk>

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

    Andrew

Re: [PATCH v2] net: phy: marvell: Enable interrupt function on LED2 pin

[David Miller]

From: Andrew Lunn <andrew@lunn.ch>
Date: Fri, 6 Apr 2018 19:29:28 +0200

> On Thu, Apr 05, 2018 at 10:40:29PM +0200, Esben Haabendal wrote:
>> From: Esben Haabendal <eha@deif.com>
>> 
>> The LED2[2]/INTn pin on Marvell 88E1318S as well as 88E1510/12/14/18 needs
>> to be configured to be usable as interrupt not only when WOL is enabled,
>> but whenever we rely on interrupts from the PHY.
>> 
>> Signed-off-by: Esben Haabendal <eha@deif.com>
>> Cc: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

Applied, thanks everyone.