From: Esben Haabendal <[email protected]>
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 <[email protected]>
Cc: Rasmus Villemoes <[email protected]>
---
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
> 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
Hi Esben,
-----Original Message-----
From: [email protected] <[email protected]> On Behalf Of Esben Haabendal
Sent: Thursday, April 05, 2018 7:05 PM
To: [email protected]
Cc: Esben Haabendal <[email protected]>; Rasmus Villemoes <[email protected]>; Andrew Lunn <[email protected]>; Florian Fainelli <[email protected]>; open list <[email protected]>
Subject: [PATCH] net: phy: marvell: Enable interrupt function on LED2 pin
From: Esben Haabendal <[email protected]>
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 <[email protected]>
Cc: Rasmus Villemoes <[email protected]>
---
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!
From: Esben Haabendal <[email protected]>
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 <[email protected]>
Cc: Rasmus Villemoes <[email protected]>
---
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
Andrew Lunn <[email protected]> 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
On Thu, Apr 05, 2018 at 10:40:29PM +0200, Esben Haabendal wrote:
> From: Esben Haabendal <[email protected]>
>
> 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 <[email protected]>
> Cc: Rasmus Villemoes <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
From: Andrew Lunn <[email protected]>
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 <[email protected]>
>>
>> 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 <[email protected]>
>> Cc: Rasmus Villemoes <[email protected]>
>
> Reviewed-by: Andrew Lunn <[email protected]>
Applied, thanks everyone.