LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] net: phy: marvell: Enable interrupt function on LED2 pin
@ 2018-04-05 13:35 Esben Haabendal
  2018-04-05 14:00 ` Bhadram Varka
  2018-04-05 20:40 ` [PATCH v2] " Esben Haabendal
  0 siblings, 2 replies; 7+ messages in thread
From: Esben Haabendal @ 2018-04-05 13:35 UTC (permalink / raw)
  To: netdev
  Cc: Esben Haabendal, Rasmus Villemoes, Andrew Lunn, Florian Fainelli,
	open list

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

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

* RE: [PATCH] net: phy: marvell: Enable interrupt function on LED2 pin
  2018-04-05 13:35 [PATCH] net: phy: marvell: Enable interrupt function on LED2 pin Esben Haabendal
@ 2018-04-05 14:00 ` Bhadram Varka
  2018-04-05 14:43   ` Andrew Lunn
  2018-04-05 20:40 ` [PATCH v2] " Esben Haabendal
  1 sibling, 1 reply; 7+ messages in thread
From: Bhadram Varka @ 2018-04-05 14:00 UTC (permalink / raw)
  To: Esben Haabendal
  Cc: Esben Haabendal, Rasmus Villemoes, Andrew Lunn, Florian Fainelli,
	open list, netdev

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!

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

* Re: [PATCH] net: phy: marvell: Enable interrupt function on LED2 pin
  2018-04-05 14:00 ` Bhadram Varka
@ 2018-04-05 14:43   ` Andrew Lunn
  2018-04-06 17:10     ` Esben Haabendal
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Lunn @ 2018-04-05 14:43 UTC (permalink / raw)
  To: Bhadram Varka
  Cc: Esben Haabendal, Esben Haabendal, Rasmus Villemoes,
	Florian Fainelli, open list, netdev

> 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

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

* [PATCH v2] net: phy: marvell: Enable interrupt function on LED2 pin
  2018-04-05 13:35 [PATCH] net: phy: marvell: Enable interrupt function on LED2 pin Esben Haabendal
  2018-04-05 14:00 ` Bhadram Varka
@ 2018-04-05 20:40 ` Esben Haabendal
  2018-04-06 17:29   ` Andrew Lunn
  1 sibling, 1 reply; 7+ messages in thread
From: Esben Haabendal @ 2018-04-05 20:40 UTC (permalink / raw)
  To: netdev
  Cc: Esben Haabendal, Rasmus Villemoes, Andrew Lunn, Florian Fainelli,
	open list

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

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

* Re: [PATCH] net: phy: marvell: Enable interrupt function on LED2 pin
  2018-04-05 14:43   ` Andrew Lunn
@ 2018-04-06 17:10     ` Esben Haabendal
  0 siblings, 0 replies; 7+ messages in thread
From: Esben Haabendal @ 2018-04-06 17:10 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Bhadram Varka, Rasmus Villemoes, Florian Fainelli, open list, netdev

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

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

* Re: [PATCH v2] net: phy: marvell: Enable interrupt function on LED2 pin
  2018-04-05 20:40 ` [PATCH v2] " Esben Haabendal
@ 2018-04-06 17:29   ` Andrew Lunn
  2018-04-06 17:37     ` David Miller
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Lunn @ 2018-04-06 17:29 UTC (permalink / raw)
  To: Esben Haabendal
  Cc: netdev, Esben Haabendal, Rasmus Villemoes, Florian Fainelli, open list

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

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

* Re: [PATCH v2] net: phy: marvell: Enable interrupt function on LED2 pin
  2018-04-06 17:29   ` Andrew Lunn
@ 2018-04-06 17:37     ` David Miller
  0 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2018-04-06 17:37 UTC (permalink / raw)
  To: andrew
  Cc: esben.haabendal, netdev, eha, rasmus.villemoes, f.fainelli, linux-kernel

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.

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

end of thread, other threads:[~2018-04-06 17:37 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-05 13:35 [PATCH] net: phy: marvell: Enable interrupt function on LED2 pin Esben Haabendal
2018-04-05 14:00 ` Bhadram Varka
2018-04-05 14:43   ` Andrew Lunn
2018-04-06 17:10     ` Esben Haabendal
2018-04-05 20:40 ` [PATCH v2] " Esben Haabendal
2018-04-06 17:29   ` Andrew Lunn
2018-04-06 17:37     ` David Miller

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).