* [PATCH net-next 04/15] net: phy: icplus: remove the use .ack_interrupt()
From: Ioana Ciornei @ 2020-11-23 15:38 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, Florian Fainelli,
Jakub Kicinski, netdev, linux-kernel
Cc: Ioana Ciornei, Martin Blumenstingl
In-Reply-To: <20201123153817.1616814-1-ciorneiioana@gmail.com>
From: Ioana Ciornei <ioana.ciornei@nxp.com>
In preparation of removing the .ack_interrupt() callback, we must replace
its occurrences (aka phy_clear_interrupt), from the 2 places where it is
called from (phy_enable_interrupts and phy_disable_interrupts), with
equivalent functionality.
This means that clearing interrupts now becomes something that the PHY
driver is responsible of doing, before enabling interrupts and after
clearing them. Make this driver follow the new contract.
Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
drivers/net/phy/icplus.c | 38 +++++++++++++++++++++++++-------------
1 file changed, 25 insertions(+), 13 deletions(-)
diff --git a/drivers/net/phy/icplus.c b/drivers/net/phy/icplus.c
index a74ff45fa99c..b632947cbcdf 100644
--- a/drivers/net/phy/icplus.c
+++ b/drivers/net/phy/icplus.c
@@ -272,17 +272,39 @@ static int ip101a_g_config_init(struct phy_device *phydev)
return phy_write(phydev, IP10XX_SPEC_CTRL_STATUS, c);
}
+static int ip101a_g_ack_interrupt(struct phy_device *phydev)
+{
+ int err = phy_read(phydev, IP101A_G_IRQ_CONF_STATUS);
+
+ if (err < 0)
+ return err;
+
+ return 0;
+}
+
static int ip101a_g_config_intr(struct phy_device *phydev)
{
u16 val;
+ int err;
+
+ if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+ err = ip101a_g_ack_interrupt(phydev);
+ if (err)
+ return err;
- if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
/* INTR pin used: Speed/link/duplex will cause an interrupt */
val = IP101A_G_IRQ_PIN_USED;
- else
+ err = phy_write(phydev, IP101A_G_IRQ_CONF_STATUS, val);
+ } else {
val = IP101A_G_IRQ_ALL_MASK;
+ err = phy_write(phydev, IP101A_G_IRQ_CONF_STATUS, val);
+ if (err)
+ return err;
+
+ err = ip101a_g_ack_interrupt(phydev);
+ }
- return phy_write(phydev, IP101A_G_IRQ_CONF_STATUS, val);
+ return err;
}
static irqreturn_t ip101a_g_handle_interrupt(struct phy_device *phydev)
@@ -305,15 +327,6 @@ static irqreturn_t ip101a_g_handle_interrupt(struct phy_device *phydev)
return IRQ_HANDLED;
}
-static int ip101a_g_ack_interrupt(struct phy_device *phydev)
-{
- int err = phy_read(phydev, IP101A_G_IRQ_CONF_STATUS);
- if (err < 0)
- return err;
-
- return 0;
-}
-
static struct phy_driver icplus_driver[] = {
{
.phy_id = 0x02430d80,
@@ -340,7 +353,6 @@ static struct phy_driver icplus_driver[] = {
/* PHY_BASIC_FEATURES */
.probe = ip101a_g_probe,
.config_intr = ip101a_g_config_intr,
- .ack_interrupt = ip101a_g_ack_interrupt,
.handle_interrupt = ip101a_g_handle_interrupt,
.config_init = &ip101a_g_config_init,
.suspend = genphy_suspend,
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 06/15] net: phy: meson-gxl: remove the use of .ack_callback()
From: Ioana Ciornei @ 2020-11-23 15:38 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, Florian Fainelli,
Jakub Kicinski, netdev, linux-kernel
Cc: Ioana Ciornei, Jerome Brunet, Neil Armstrong
In-Reply-To: <20201123153817.1616814-1-ciorneiioana@gmail.com>
From: Ioana Ciornei <ioana.ciornei@nxp.com>
In preparation of removing the .ack_interrupt() callback, we must replace
its occurrences (aka phy_clear_interrupt), from the 2 places where it is
called from (phy_enable_interrupts and phy_disable_interrupts), with
equivalent functionality.
This means that clearing interrupts now becomes something that the PHY
driver is responsible of doing, before enabling interrupts and after
clearing them. Make this driver follow the new contract.
Cc: Jerome Brunet <jbrunet@baylibre.com>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
drivers/net/phy/meson-gxl.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index b16b1cc89165..7e7904fee1d9 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -204,22 +204,27 @@ static int meson_gxl_config_intr(struct phy_device *phydev)
int ret;
if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+ /* Ack any pending IRQ */
+ ret = meson_gxl_ack_interrupt(phydev);
+ if (ret)
+ return ret;
+
val = INTSRC_ANEG_PR
| INTSRC_PARALLEL_FAULT
| INTSRC_ANEG_LP_ACK
| INTSRC_LINK_DOWN
| INTSRC_REMOTE_FAULT
| INTSRC_ANEG_COMPLETE;
+ ret = phy_write(phydev, INTSRC_MASK, val);
} else {
val = 0;
- }
+ ret = phy_write(phydev, INTSRC_MASK, val);
- /* Ack any pending IRQ */
- ret = meson_gxl_ack_interrupt(phydev);
- if (ret)
- return ret;
+ /* Ack any pending IRQ */
+ ret = meson_gxl_ack_interrupt(phydev);
+ }
- return phy_write(phydev, INTSRC_MASK, val);
+ return ret;
}
static irqreturn_t meson_gxl_handle_interrupt(struct phy_device *phydev)
@@ -249,7 +254,6 @@ static struct phy_driver meson_gxl_phy[] = {
.soft_reset = genphy_soft_reset,
.config_init = meson_gxl_config_init,
.read_status = meson_gxl_read_status,
- .ack_interrupt = meson_gxl_ack_interrupt,
.config_intr = meson_gxl_config_intr,
.handle_interrupt = meson_gxl_handle_interrupt,
.suspend = genphy_suspend,
@@ -260,7 +264,6 @@ static struct phy_driver meson_gxl_phy[] = {
/* PHY_BASIC_FEATURES */
.flags = PHY_IS_INTERNAL,
.soft_reset = genphy_soft_reset,
- .ack_interrupt = meson_gxl_ack_interrupt,
.config_intr = meson_gxl_config_intr,
.handle_interrupt = meson_gxl_handle_interrupt,
.suspend = genphy_suspend,
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 08/15] net: phy: micrel: remove the use of .ack_interrupt()
From: Ioana Ciornei @ 2020-11-23 15:38 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, Florian Fainelli,
Jakub Kicinski, netdev, linux-kernel
Cc: Ioana Ciornei, Divya Koppera, Oleksij Rempel, Philippe Schenker,
Marek Vasut, Antoine Tenart
In-Reply-To: <20201123153817.1616814-1-ciorneiioana@gmail.com>
From: Ioana Ciornei <ioana.ciornei@nxp.com>
In preparation of removing the .ack_interrupt() callback, we must replace
its occurrences (aka phy_clear_interrupt), from the 2 places where it is
called from (phy_enable_interrupts and phy_disable_interrupts), with
equivalent functionality.
This means that clearing interrupts now becomes something that the PHY
driver is responsible of doing, before enabling interrupts and after
clearing them. Make this driver follow the new contract.
Cc: Divya Koppera <Divya.Koppera@microchip.com>
Cc: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: Philippe Schenker <philippe.schenker@toradex.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Antoine Tenart <atenart@kernel.org>
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
drivers/net/phy/micrel.c | 31 +++++++++++++++----------------
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 9aa96ebd31c8..97f08f20630b 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -162,7 +162,7 @@ static int kszphy_ack_interrupt(struct phy_device *phydev)
static int kszphy_config_intr(struct phy_device *phydev)
{
const struct kszphy_type *type = phydev->drv->driver_data;
- int temp;
+ int temp, err;
u16 mask;
if (type && type->interrupt_level_mask)
@@ -178,12 +178,23 @@ static int kszphy_config_intr(struct phy_device *phydev)
phy_write(phydev, MII_KSZPHY_CTRL, temp);
/* enable / disable interrupts */
- if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
+ if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+ err = kszphy_ack_interrupt(phydev);
+ if (err)
+ return err;
+
temp = KSZPHY_INTCS_ALL;
- else
+ err = phy_write(phydev, MII_KSZPHY_INTCS, temp);
+ } else {
temp = 0;
+ err = phy_write(phydev, MII_KSZPHY_INTCS, temp);
+ if (err)
+ return err;
+
+ err = kszphy_ack_interrupt(phydev);
+ }
- return phy_write(phydev, MII_KSZPHY_INTCS, temp);
+ return err;
}
static irqreturn_t kszphy_handle_interrupt(struct phy_device *phydev)
@@ -1182,7 +1193,6 @@ static struct phy_driver ksphy_driver[] = {
/* PHY_BASIC_FEATURES */
.driver_data = &ks8737_type,
.config_init = kszphy_config_init,
- .ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
.handle_interrupt = kszphy_handle_interrupt,
.suspend = genphy_suspend,
@@ -1195,7 +1205,6 @@ static struct phy_driver ksphy_driver[] = {
.driver_data = &ksz8021_type,
.probe = kszphy_probe,
.config_init = kszphy_config_init,
- .ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
.handle_interrupt = kszphy_handle_interrupt,
.get_sset_count = kszphy_get_sset_count,
@@ -1211,7 +1220,6 @@ static struct phy_driver ksphy_driver[] = {
.driver_data = &ksz8021_type,
.probe = kszphy_probe,
.config_init = kszphy_config_init,
- .ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
.handle_interrupt = kszphy_handle_interrupt,
.get_sset_count = kszphy_get_sset_count,
@@ -1228,7 +1236,6 @@ static struct phy_driver ksphy_driver[] = {
.probe = kszphy_probe,
.config_init = ksz8041_config_init,
.config_aneg = ksz8041_config_aneg,
- .ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
.handle_interrupt = kszphy_handle_interrupt,
.get_sset_count = kszphy_get_sset_count,
@@ -1244,7 +1251,6 @@ static struct phy_driver ksphy_driver[] = {
.driver_data = &ksz8041_type,
.probe = kszphy_probe,
.config_init = kszphy_config_init,
- .ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
.handle_interrupt = kszphy_handle_interrupt,
.get_sset_count = kszphy_get_sset_count,
@@ -1258,7 +1264,6 @@ static struct phy_driver ksphy_driver[] = {
.driver_data = &ksz8051_type,
.probe = kszphy_probe,
.config_init = kszphy_config_init,
- .ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
.handle_interrupt = kszphy_handle_interrupt,
.get_sset_count = kszphy_get_sset_count,
@@ -1275,7 +1280,6 @@ static struct phy_driver ksphy_driver[] = {
.driver_data = &ksz8041_type,
.probe = kszphy_probe,
.config_init = kszphy_config_init,
- .ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
.handle_interrupt = kszphy_handle_interrupt,
.get_sset_count = kszphy_get_sset_count,
@@ -1291,7 +1295,6 @@ static struct phy_driver ksphy_driver[] = {
.driver_data = &ksz8081_type,
.probe = kszphy_probe,
.config_init = ksz8081_config_init,
- .ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
.handle_interrupt = kszphy_handle_interrupt,
.get_sset_count = kszphy_get_sset_count,
@@ -1305,7 +1308,6 @@ static struct phy_driver ksphy_driver[] = {
.phy_id_mask = MICREL_PHY_ID_MASK,
/* PHY_BASIC_FEATURES */
.config_init = ksz8061_config_init,
- .ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
.handle_interrupt = kszphy_handle_interrupt,
.suspend = genphy_suspend,
@@ -1319,7 +1321,6 @@ static struct phy_driver ksphy_driver[] = {
.probe = kszphy_probe,
.get_features = ksz9031_get_features,
.config_init = ksz9021_config_init,
- .ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
.handle_interrupt = kszphy_handle_interrupt,
.get_sset_count = kszphy_get_sset_count,
@@ -1339,7 +1340,6 @@ static struct phy_driver ksphy_driver[] = {
.config_init = ksz9031_config_init,
.soft_reset = genphy_soft_reset,
.read_status = ksz9031_read_status,
- .ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
.handle_interrupt = kszphy_handle_interrupt,
.get_sset_count = kszphy_get_sset_count,
@@ -1369,7 +1369,6 @@ static struct phy_driver ksphy_driver[] = {
.probe = kszphy_probe,
.config_init = ksz9131_config_init,
.read_status = genphy_read_status,
- .ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
.handle_interrupt = kszphy_handle_interrupt,
.get_sset_count = kszphy_get_sset_count,
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 03/15] net: phy: icplus: implement generic .handle_interrupt() callback
From: Ioana Ciornei @ 2020-11-23 15:38 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, Florian Fainelli,
Jakub Kicinski, netdev, linux-kernel
Cc: Ioana Ciornei, Martin Blumenstingl
In-Reply-To: <20201123153817.1616814-1-ciorneiioana@gmail.com>
From: Ioana Ciornei <ioana.ciornei@nxp.com>
In an attempt to actually support shared IRQs in phylib, we now move the
responsibility of triggering the phylib state machine or just returning
IRQ_NONE, based on the IRQ status register, to the PHY driver. Having
3 different IRQ handling callbacks (.handle_interrupt(),
.did_interrupt() and .ack_interrupt() ) is confusing so let the PHY
driver implement directly an IRQ handler like any other device driver.
Make this driver follow the new convention.
Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
drivers/net/phy/icplus.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/net/phy/icplus.c b/drivers/net/phy/icplus.c
index d6e8516cd146..a74ff45fa99c 100644
--- a/drivers/net/phy/icplus.c
+++ b/drivers/net/phy/icplus.c
@@ -285,16 +285,24 @@ static int ip101a_g_config_intr(struct phy_device *phydev)
return phy_write(phydev, IP101A_G_IRQ_CONF_STATUS, val);
}
-static int ip101a_g_did_interrupt(struct phy_device *phydev)
+static irqreturn_t ip101a_g_handle_interrupt(struct phy_device *phydev)
{
- int val = phy_read(phydev, IP101A_G_IRQ_CONF_STATUS);
+ int irq_status;
- if (val < 0)
- return 0;
+ irq_status = phy_read(phydev, IP101A_G_IRQ_CONF_STATUS);
+ if (irq_status < 0) {
+ phy_error(phydev);
+ return IRQ_NONE;
+ }
+
+ if (!(irq_status & (IP101A_G_IRQ_SPEED_CHANGE |
+ IP101A_G_IRQ_DUPLEX_CHANGE |
+ IP101A_G_IRQ_LINK_CHANGE)))
+ return IRQ_NONE;
+
+ phy_trigger_machine(phydev);
- return val & (IP101A_G_IRQ_SPEED_CHANGE |
- IP101A_G_IRQ_DUPLEX_CHANGE |
- IP101A_G_IRQ_LINK_CHANGE);
+ return IRQ_HANDLED;
}
static int ip101a_g_ack_interrupt(struct phy_device *phydev)
@@ -332,8 +340,8 @@ static struct phy_driver icplus_driver[] = {
/* PHY_BASIC_FEATURES */
.probe = ip101a_g_probe,
.config_intr = ip101a_g_config_intr,
- .did_interrupt = ip101a_g_did_interrupt,
.ack_interrupt = ip101a_g_ack_interrupt,
+ .handle_interrupt = ip101a_g_handle_interrupt,
.config_init = &ip101a_g_config_init,
.suspend = genphy_suspend,
.resume = genphy_resume,
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 11/15] net: phy: ti: implement generic .handle_interrupt() callback
From: Ioana Ciornei @ 2020-11-23 15:38 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, Florian Fainelli,
Jakub Kicinski, netdev, linux-kernel
Cc: Ioana Ciornei, Dan Murphy
In-Reply-To: <20201123153817.1616814-1-ciorneiioana@gmail.com>
From: Ioana Ciornei <ioana.ciornei@nxp.com>
In an attempt to actually support shared IRQs in phylib, we now move the
responsibility of triggering the phylib state machine or just returning
IRQ_NONE, based on the IRQ status register, to the PHY driver. Having
3 different IRQ handling callbacks (.handle_interrupt(),
.did_interrupt() and .ack_interrupt() ) is confusing so let the PHY
driver implement directly an IRQ handler like any other device driver.
Make this driver follow the new convention.
Cc: Dan Murphy <dmurphy@ti.com>
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
drivers/net/phy/dp83640.c | 27 +++++++++++++++++++++++
drivers/net/phy/dp83822.c | 37 +++++++++++++++++++++++++++++++
drivers/net/phy/dp83848.c | 33 ++++++++++++++++++++++++++++
drivers/net/phy/dp83867.c | 25 +++++++++++++++++++++
drivers/net/phy/dp83869.c | 25 +++++++++++++++++++++
drivers/net/phy/dp83tc811.c | 44 +++++++++++++++++++++++++++++++++++++
6 files changed, 191 insertions(+)
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index f2caccaf4408..89577f1d3576 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -50,6 +50,14 @@
#define MII_DP83640_MISR_LINK_INT_EN 0x20
#define MII_DP83640_MISR_ED_INT_EN 0x40
#define MII_DP83640_MISR_LQ_INT_EN 0x80
+#define MII_DP83640_MISR_ANC_INT 0x400
+#define MII_DP83640_MISR_DUP_INT 0x800
+#define MII_DP83640_MISR_SPD_INT 0x1000
+#define MII_DP83640_MISR_LINK_INT 0x2000
+#define MII_DP83640_MISR_INT_MASK (MII_DP83640_MISR_ANC_INT |\
+ MII_DP83640_MISR_DUP_INT |\
+ MII_DP83640_MISR_SPD_INT |\
+ MII_DP83640_MISR_LINK_INT)
/* phyter seems to miss the mark by 16 ns */
#define ADJTIME_FIX 16
@@ -1193,6 +1201,24 @@ static int dp83640_config_intr(struct phy_device *phydev)
}
}
+static irqreturn_t dp83640_handle_interrupt(struct phy_device *phydev)
+{
+ int irq_status;
+
+ irq_status = phy_read(phydev, MII_DP83640_MISR);
+ if (irq_status < 0) {
+ phy_error(phydev);
+ return IRQ_NONE;
+ }
+
+ if (!(irq_status & MII_DP83640_MISR_INT_MASK))
+ return IRQ_NONE;
+
+ phy_trigger_machine(phydev);
+
+ return IRQ_HANDLED;
+}
+
static int dp83640_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
{
struct dp83640_private *dp83640 =
@@ -1517,6 +1543,7 @@ static struct phy_driver dp83640_driver = {
.config_init = dp83640_config_init,
.ack_interrupt = dp83640_ack_interrupt,
.config_intr = dp83640_config_intr,
+ .handle_interrupt = dp83640_handle_interrupt,
};
static int __init dp83640_init(void)
diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index c162c9551bd1..bb512ac3f533 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -303,6 +303,41 @@ static int dp83822_config_intr(struct phy_device *phydev)
return phy_write(phydev, MII_DP83822_PHYSCR, physcr_status);
}
+static irqreturn_t dp83822_handle_interrupt(struct phy_device *phydev)
+{
+ int irq_status;
+
+ /* The MISR1 and MISR2 registers are holding the interrupt status in
+ * the upper half (15:8), while the lower half (7:0) is used for
+ * controlling the interrupt enable state of those individual interrupt
+ * sources. To determine the possible interrupt sources, just read the
+ * MISR* register and use it directly to know which interrupts have
+ * been enabled previously or not.
+ */
+ irq_status = phy_read(phydev, MII_DP83822_MISR1);
+ if (irq_status < 0) {
+ phy_error(phydev);
+ return IRQ_NONE;
+ }
+ if (irq_status & ((irq_status & GENMASK(7, 0)) << 8))
+ goto trigger_machine;
+
+ irq_status = phy_read(phydev, MII_DP83822_MISR2);
+ if (irq_status < 0) {
+ phy_error(phydev);
+ return IRQ_NONE;
+ }
+ if (irq_status & ((irq_status & GENMASK(7, 0)) << 8))
+ goto trigger_machine;
+
+ return IRQ_NONE;
+
+trigger_machine:
+ phy_trigger_machine(phydev);
+
+ return IRQ_HANDLED;
+}
+
static int dp8382x_disable_wol(struct phy_device *phydev)
{
int value = DP83822_WOL_EN | DP83822_WOL_MAGIC_EN |
@@ -576,6 +611,7 @@ static int dp83822_resume(struct phy_device *phydev)
.set_wol = dp83822_set_wol, \
.ack_interrupt = dp83822_ack_interrupt, \
.config_intr = dp83822_config_intr, \
+ .handle_interrupt = dp83822_handle_interrupt, \
.suspend = dp83822_suspend, \
.resume = dp83822_resume, \
}
@@ -591,6 +627,7 @@ static int dp83822_resume(struct phy_device *phydev)
.set_wol = dp83822_set_wol, \
.ack_interrupt = dp83822_ack_interrupt, \
.config_intr = dp83822_config_intr, \
+ .handle_interrupt = dp83822_handle_interrupt, \
.suspend = dp83822_suspend, \
.resume = dp83822_resume, \
}
diff --git a/drivers/net/phy/dp83848.c b/drivers/net/phy/dp83848.c
index 54c7c1b44e4d..b707a9b27847 100644
--- a/drivers/net/phy/dp83848.c
+++ b/drivers/net/phy/dp83848.c
@@ -37,6 +37,20 @@
DP83848_MISR_SPD_INT_EN | \
DP83848_MISR_LINK_INT_EN)
+#define DP83848_MISR_RHF_INT BIT(8)
+#define DP83848_MISR_FHF_INT BIT(9)
+#define DP83848_MISR_ANC_INT BIT(10)
+#define DP83848_MISR_DUP_INT BIT(11)
+#define DP83848_MISR_SPD_INT BIT(12)
+#define DP83848_MISR_LINK_INT BIT(13)
+#define DP83848_MISR_ED_INT BIT(14)
+
+#define DP83848_INT_MASK \
+ (DP83848_MISR_ANC_INT | \
+ DP83848_MISR_DUP_INT | \
+ DP83848_MISR_SPD_INT | \
+ DP83848_MISR_LINK_INT)
+
static int dp83848_ack_interrupt(struct phy_device *phydev)
{
int err = phy_read(phydev, DP83848_MISR);
@@ -66,6 +80,24 @@ static int dp83848_config_intr(struct phy_device *phydev)
return phy_write(phydev, DP83848_MICR, control);
}
+static irqreturn_t dp83848_handle_interrupt(struct phy_device *phydev)
+{
+ int irq_status;
+
+ irq_status = phy_read(phydev, DP83848_MISR);
+ if (irq_status < 0) {
+ phy_error(phydev);
+ return IRQ_NONE;
+ }
+
+ if (!(irq_status & DP83848_INT_MASK))
+ return IRQ_NONE;
+
+ phy_trigger_machine(phydev);
+
+ return IRQ_HANDLED;
+}
+
static int dp83848_config_init(struct phy_device *phydev)
{
int val;
@@ -104,6 +136,7 @@ MODULE_DEVICE_TABLE(mdio, dp83848_tbl);
/* IRQ related */ \
.ack_interrupt = dp83848_ack_interrupt, \
.config_intr = dp83848_config_intr, \
+ .handle_interrupt = dp83848_handle_interrupt, \
}
static struct phy_driver dp83848_driver[] = {
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index 69d3eacc2b96..aba4e4c1f75c 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -310,6 +310,30 @@ static int dp83867_config_intr(struct phy_device *phydev)
return phy_write(phydev, MII_DP83867_MICR, micr_status);
}
+static irqreturn_t dp83867_handle_interrupt(struct phy_device *phydev)
+{
+ int irq_status, irq_enabled;
+
+ irq_status = phy_read(phydev, MII_DP83867_ISR);
+ if (irq_status < 0) {
+ phy_error(phydev);
+ return IRQ_NONE;
+ }
+
+ irq_enabled = phy_read(phydev, MII_DP83867_MICR);
+ if (irq_enabled < 0) {
+ phy_error(phydev);
+ return IRQ_NONE;
+ }
+
+ if (!(irq_status & irq_enabled))
+ return IRQ_NONE;
+
+ phy_trigger_machine(phydev);
+
+ return IRQ_HANDLED;
+}
+
static int dp83867_read_status(struct phy_device *phydev)
{
int status = phy_read(phydev, MII_DP83867_PHYSTS);
@@ -827,6 +851,7 @@ static struct phy_driver dp83867_driver[] = {
/* IRQ related */
.ack_interrupt = dp83867_ack_interrupt,
.config_intr = dp83867_config_intr,
+ .handle_interrupt = dp83867_handle_interrupt,
.suspend = genphy_suspend,
.resume = genphy_resume,
diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c
index cf6dec7b7d8e..487d1b8beec5 100644
--- a/drivers/net/phy/dp83869.c
+++ b/drivers/net/phy/dp83869.c
@@ -207,6 +207,30 @@ static int dp83869_config_intr(struct phy_device *phydev)
return phy_write(phydev, MII_DP83869_MICR, micr_status);
}
+static irqreturn_t dp83869_handle_interrupt(struct phy_device *phydev)
+{
+ int irq_status, irq_enabled;
+
+ irq_status = phy_read(phydev, MII_DP83869_ISR);
+ if (irq_status < 0) {
+ phy_error(phydev);
+ return IRQ_NONE;
+ }
+
+ irq_enabled = phy_read(phydev, MII_DP83869_MICR);
+ if (irq_enabled < 0) {
+ phy_error(phydev);
+ return IRQ_NONE;
+ }
+
+ if (!(irq_status & irq_enabled))
+ return IRQ_NONE;
+
+ phy_trigger_machine(phydev);
+
+ return IRQ_HANDLED;
+}
+
static int dp83869_set_wol(struct phy_device *phydev,
struct ethtool_wolinfo *wol)
{
@@ -852,6 +876,7 @@ static struct phy_driver dp83869_driver[] = {
/* IRQ related */
.ack_interrupt = dp83869_ack_interrupt,
.config_intr = dp83869_config_intr,
+ .handle_interrupt = dp83869_handle_interrupt,
.read_status = dp83869_read_status,
.get_tunable = dp83869_get_tunable,
diff --git a/drivers/net/phy/dp83tc811.c b/drivers/net/phy/dp83tc811.c
index d73725312c7c..a93c64ac76a3 100644
--- a/drivers/net/phy/dp83tc811.c
+++ b/drivers/net/phy/dp83tc811.c
@@ -254,6 +254,49 @@ static int dp83811_config_intr(struct phy_device *phydev)
return err;
}
+static irqreturn_t dp83811_handle_interrupt(struct phy_device *phydev)
+{
+ int irq_status;
+
+ /* The INT_STAT registers 1, 2 and 3 are holding the interrupt status
+ * in the upper half (15:8), while the lower half (7:0) is used for
+ * controlling the interrupt enable state of those individual interrupt
+ * sources. To determine the possible interrupt sources, just read the
+ * INT_STAT* register and use it directly to know which interrupts have
+ * been enabled previously or not.
+ */
+ irq_status = phy_read(phydev, MII_DP83811_INT_STAT1);
+ if (irq_status < 0) {
+ phy_error(phydev);
+ return IRQ_NONE;
+ }
+ if (irq_status & ((irq_status & GENMASK(7, 0)) << 8))
+ goto trigger_machine;
+
+ irq_status = phy_read(phydev, MII_DP83811_INT_STAT2);
+ if (irq_status < 0) {
+ phy_error(phydev);
+ return IRQ_NONE;
+ }
+ if (irq_status & ((irq_status & GENMASK(7, 0)) << 8))
+ goto trigger_machine;
+
+ irq_status = phy_read(phydev, MII_DP83811_INT_STAT3);
+ if (irq_status < 0) {
+ phy_error(phydev);
+ return IRQ_NONE;
+ }
+ if (irq_status & ((irq_status & GENMASK(7, 0)) << 8))
+ goto trigger_machine;
+
+ return IRQ_NONE;
+
+trigger_machine:
+ phy_trigger_machine(phydev);
+
+ return IRQ_HANDLED;
+}
+
static int dp83811_config_aneg(struct phy_device *phydev)
{
int value, err;
@@ -345,6 +388,7 @@ static struct phy_driver dp83811_driver[] = {
.set_wol = dp83811_set_wol,
.ack_interrupt = dp83811_ack_interrupt,
.config_intr = dp83811_config_intr,
+ .handle_interrupt = dp83811_handle_interrupt,
.suspend = dp83811_suspend,
.resume = dp83811_resume,
},
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 07/15] net: phy: micrel: implement generic .handle_interrupt() callback
From: Ioana Ciornei @ 2020-11-23 15:38 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, Florian Fainelli,
Jakub Kicinski, netdev, linux-kernel
Cc: Ioana Ciornei, Divya Koppera, Oleksij Rempel, Philippe Schenker,
Marek Vasut, Antoine Tenart
In-Reply-To: <20201123153817.1616814-1-ciorneiioana@gmail.com>
From: Ioana Ciornei <ioana.ciornei@nxp.com>
In an attempt to actually support shared IRQs in phylib, we now move the
responsibility of triggering the phylib state machine or just returning
IRQ_NONE, based on the IRQ status register, to the PHY driver. Having
3 different IRQ handling callbacks (.handle_interrupt(),
.did_interrupt() and .ack_interrupt() ) is confusing so let the PHY
driver implement directly an IRQ handler like any other device driver.
Make this driver follow the new convention.
Cc: Divya Koppera <Divya.Koppera@microchip.com>
Cc: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: Philippe Schenker <philippe.schenker@toradex.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Antoine Tenart <atenart@kernel.org>
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
drivers/net/phy/micrel.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index a7f74b3b97af..9aa96ebd31c8 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -48,6 +48,10 @@
#define KSZPHY_INTCS_LINK_UP BIT(8)
#define KSZPHY_INTCS_ALL (KSZPHY_INTCS_LINK_UP |\
KSZPHY_INTCS_LINK_DOWN)
+#define KSZPHY_INTCS_LINK_DOWN_STATUS BIT(2)
+#define KSZPHY_INTCS_LINK_UP_STATUS BIT(0)
+#define KSZPHY_INTCS_STATUS (KSZPHY_INTCS_LINK_DOWN_STATUS |\
+ KSZPHY_INTCS_LINK_UP_STATUS)
/* PHY Control 1 */
#define MII_KSZPHY_CTRL_1 0x1e
@@ -182,6 +186,24 @@ static int kszphy_config_intr(struct phy_device *phydev)
return phy_write(phydev, MII_KSZPHY_INTCS, temp);
}
+static irqreturn_t kszphy_handle_interrupt(struct phy_device *phydev)
+{
+ int irq_status;
+
+ irq_status = phy_read(phydev, MII_KSZPHY_INTCS);
+ if (irq_status < 0) {
+ phy_error(phydev);
+ return IRQ_NONE;
+ }
+
+ if ((irq_status & KSZPHY_INTCS_STATUS))
+ return IRQ_NONE;
+
+ phy_trigger_machine(phydev);
+
+ return IRQ_HANDLED;
+}
+
static int kszphy_rmii_clk_sel(struct phy_device *phydev, bool val)
{
int ctrl;
@@ -1162,6 +1184,7 @@ static struct phy_driver ksphy_driver[] = {
.config_init = kszphy_config_init,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
+ .handle_interrupt = kszphy_handle_interrupt,
.suspend = genphy_suspend,
.resume = genphy_resume,
}, {
@@ -1174,6 +1197,7 @@ static struct phy_driver ksphy_driver[] = {
.config_init = kszphy_config_init,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
+ .handle_interrupt = kszphy_handle_interrupt,
.get_sset_count = kszphy_get_sset_count,
.get_strings = kszphy_get_strings,
.get_stats = kszphy_get_stats,
@@ -1189,6 +1213,7 @@ static struct phy_driver ksphy_driver[] = {
.config_init = kszphy_config_init,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
+ .handle_interrupt = kszphy_handle_interrupt,
.get_sset_count = kszphy_get_sset_count,
.get_strings = kszphy_get_strings,
.get_stats = kszphy_get_stats,
@@ -1205,6 +1230,7 @@ static struct phy_driver ksphy_driver[] = {
.config_aneg = ksz8041_config_aneg,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
+ .handle_interrupt = kszphy_handle_interrupt,
.get_sset_count = kszphy_get_sset_count,
.get_strings = kszphy_get_strings,
.get_stats = kszphy_get_stats,
@@ -1220,6 +1246,7 @@ static struct phy_driver ksphy_driver[] = {
.config_init = kszphy_config_init,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
+ .handle_interrupt = kszphy_handle_interrupt,
.get_sset_count = kszphy_get_sset_count,
.get_strings = kszphy_get_strings,
.get_stats = kszphy_get_stats,
@@ -1233,6 +1260,7 @@ static struct phy_driver ksphy_driver[] = {
.config_init = kszphy_config_init,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
+ .handle_interrupt = kszphy_handle_interrupt,
.get_sset_count = kszphy_get_sset_count,
.get_strings = kszphy_get_strings,
.get_stats = kszphy_get_stats,
@@ -1249,6 +1277,7 @@ static struct phy_driver ksphy_driver[] = {
.config_init = kszphy_config_init,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
+ .handle_interrupt = kszphy_handle_interrupt,
.get_sset_count = kszphy_get_sset_count,
.get_strings = kszphy_get_strings,
.get_stats = kszphy_get_stats,
@@ -1264,6 +1293,7 @@ static struct phy_driver ksphy_driver[] = {
.config_init = ksz8081_config_init,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
+ .handle_interrupt = kszphy_handle_interrupt,
.get_sset_count = kszphy_get_sset_count,
.get_strings = kszphy_get_strings,
.get_stats = kszphy_get_stats,
@@ -1277,6 +1307,7 @@ static struct phy_driver ksphy_driver[] = {
.config_init = ksz8061_config_init,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
+ .handle_interrupt = kszphy_handle_interrupt,
.suspend = genphy_suspend,
.resume = genphy_resume,
}, {
@@ -1290,6 +1321,7 @@ static struct phy_driver ksphy_driver[] = {
.config_init = ksz9021_config_init,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
+ .handle_interrupt = kszphy_handle_interrupt,
.get_sset_count = kszphy_get_sset_count,
.get_strings = kszphy_get_strings,
.get_stats = kszphy_get_stats,
@@ -1309,6 +1341,7 @@ static struct phy_driver ksphy_driver[] = {
.read_status = ksz9031_read_status,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
+ .handle_interrupt = kszphy_handle_interrupt,
.get_sset_count = kszphy_get_sset_count,
.get_strings = kszphy_get_strings,
.get_stats = kszphy_get_stats,
@@ -1338,6 +1371,7 @@ static struct phy_driver ksphy_driver[] = {
.read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
+ .handle_interrupt = kszphy_handle_interrupt,
.get_sset_count = kszphy_get_sset_count,
.get_strings = kszphy_get_strings,
.get_stats = kszphy_get_stats,
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 15/15] net: phy: remove the .did_interrupt() and .ack_interrupt() callback
From: Ioana Ciornei @ 2020-11-23 15:38 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, Florian Fainelli,
Jakub Kicinski, netdev, linux-kernel
Cc: Ioana Ciornei
In-Reply-To: <20201123153817.1616814-1-ciorneiioana@gmail.com>
From: Ioana Ciornei <ioana.ciornei@nxp.com>
Now that all the PHY drivers have been migrated to directly implement
the generic .handle_interrupt() callback for a seamless support of
shared IRQs and all the .config_inter() implementations clear any
pending interrupts, we can safely remove the two callbacks.
With this patch, phylib has a proper support for shared IRQs (and not
just for multi-PHY devices. A PHY driver must implement both the
.handle_interrupt() and .config_intr() callbacks for the IRQs to be
actually used.
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
drivers/net/phy/phy.c | 48 ++----------------------------------
drivers/net/phy/phy_device.c | 2 +-
include/linux/phy.h | 19 +++-----------
3 files changed, 7 insertions(+), 62 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index dce86bad8231..45f75533c47c 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -113,23 +113,6 @@ void phy_print_status(struct phy_device *phydev)
}
EXPORT_SYMBOL(phy_print_status);
-/**
- * phy_clear_interrupt - Ack the phy device's interrupt
- * @phydev: the phy_device struct
- *
- * If the @phydev driver has an ack_interrupt function, call it to
- * ack and clear the phy device's interrupt.
- *
- * Returns 0 on success or < 0 on error.
- */
-static int phy_clear_interrupt(struct phy_device *phydev)
-{
- if (phydev->drv->ack_interrupt)
- return phydev->drv->ack_interrupt(phydev);
-
- return 0;
-}
-
/**
* phy_config_interrupt - configure the PHY device for the requested interrupts
* @phydev: the phy_device struct
@@ -943,15 +926,8 @@ EXPORT_SYMBOL(phy_error);
*/
int phy_disable_interrupts(struct phy_device *phydev)
{
- int err;
-
/* Disable PHY interrupts */
- err = phy_config_interrupt(phydev, PHY_INTERRUPT_DISABLED);
- if (err)
- return err;
-
- /* Clear the interrupt */
- return phy_clear_interrupt(phydev);
+ return phy_config_interrupt(phydev, PHY_INTERRUPT_DISABLED);
}
/**
@@ -966,22 +942,7 @@ static irqreturn_t phy_interrupt(int irq, void *phy_dat)
struct phy_device *phydev = phy_dat;
struct phy_driver *drv = phydev->drv;
- if (drv->handle_interrupt)
- return drv->handle_interrupt(phydev);
-
- if (drv->did_interrupt && !drv->did_interrupt(phydev))
- return IRQ_NONE;
-
- /* reschedule state queue work to run as soon as possible */
- phy_trigger_machine(phydev);
-
- /* did_interrupt() may have cleared the interrupt already */
- if (!drv->did_interrupt && phy_clear_interrupt(phydev)) {
- phy_error(phydev);
- return IRQ_NONE;
- }
-
- return IRQ_HANDLED;
+ return drv->handle_interrupt(phydev);
}
/**
@@ -990,11 +951,6 @@ static irqreturn_t phy_interrupt(int irq, void *phy_dat)
*/
static int phy_enable_interrupts(struct phy_device *phydev)
{
- int err = phy_clear_interrupt(phydev);
-
- if (err < 0)
- return err;
-
return phy_config_interrupt(phydev, PHY_INTERRUPT_ENABLED);
}
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 81f672911305..80c2e646c093 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2826,7 +2826,7 @@ EXPORT_SYMBOL(phy_get_internal_delay);
static bool phy_drv_supports_irq(struct phy_driver *phydrv)
{
- return phydrv->config_intr && (phydrv->ack_interrupt || phydrv->handle_interrupt);
+ return phydrv->config_intr && phydrv->handle_interrupt;
}
/**
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 8849a00a093f..381a95732b6a 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -743,18 +743,11 @@ struct phy_driver {
/** @read_status: Determines the negotiated speed and duplex */
int (*read_status)(struct phy_device *phydev);
- /** @ack_interrupt: Clears any pending interrupts */
- int (*ack_interrupt)(struct phy_device *phydev);
-
- /** @config_intr: Enables or disables interrupts */
- int (*config_intr)(struct phy_device *phydev);
-
- /**
- * @did_interrupt: Checks if the PHY generated an interrupt.
- * For multi-PHY devices with shared PHY interrupt pin
- * Set interrupt bits have to be cleared.
+ /** @config_intr: Enables or disables interrupts.
+ * It should also clear any pending interrupts prior to enabling the
+ * IRQs and after disabling them.
*/
- int (*did_interrupt)(struct phy_device *phydev);
+ int (*config_intr)(struct phy_device *phydev);
/** @handle_interrupt: Override default interrupt handling */
irqreturn_t (*handle_interrupt)(struct phy_device *phydev);
@@ -1487,10 +1480,6 @@ static inline int genphy_config_aneg(struct phy_device *phydev)
return __genphy_config_aneg(phydev, false);
}
-static inline int genphy_no_ack_interrupt(struct phy_device *phydev)
-{
- return 0;
-}
static inline int genphy_no_config_intr(struct phy_device *phydev)
{
return 0;
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 12/15] net: phy: ti: remove the use of .ack_interrupt()
From: Ioana Ciornei @ 2020-11-23 15:38 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, Florian Fainelli,
Jakub Kicinski, netdev, linux-kernel
Cc: Ioana Ciornei, Dan Murphy
In-Reply-To: <20201123153817.1616814-1-ciorneiioana@gmail.com>
From: Ioana Ciornei <ioana.ciornei@nxp.com>
In preparation of removing the .ack_interrupt() callback, we must replace
its occurrences (aka phy_clear_interrupt), from the 2 places where it is
called from (phy_enable_interrupts and phy_disable_interrupts), with
equivalent functionality.
This means that clearing interrupts now becomes something that the PHY
driver is responsible of doing, before enabling interrupts and after
clearing them. Make this driver follow the new contract.
Cc: Dan Murphy <dmurphy@ti.com>
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
drivers/net/phy/dp83640.c | 11 +++++++++--
drivers/net/phy/dp83822.c | 17 -----------------
drivers/net/phy/dp83848.c | 14 ++++++++++++--
drivers/net/phy/dp83867.c | 19 ++++++++++++++-----
drivers/net/phy/dp83869.c | 17 +++++++++++++----
drivers/net/phy/dp83tc811.c | 9 ++++++++-
6 files changed, 56 insertions(+), 31 deletions(-)
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 89577f1d3576..f1001ae1df51 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -1159,6 +1159,10 @@ static int dp83640_config_intr(struct phy_device *phydev)
int err;
if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+ err = dp83640_ack_interrupt(phydev);
+ if (err)
+ return err;
+
misr = phy_read(phydev, MII_DP83640_MISR);
if (misr < 0)
return misr;
@@ -1197,7 +1201,11 @@ static int dp83640_config_intr(struct phy_device *phydev)
MII_DP83640_MISR_DUP_INT_EN |
MII_DP83640_MISR_SPD_INT_EN |
MII_DP83640_MISR_LINK_INT_EN);
- return phy_write(phydev, MII_DP83640_MISR, misr);
+ err = phy_write(phydev, MII_DP83640_MISR, misr);
+ if (err)
+ return err;
+
+ return dp83640_ack_interrupt(phydev);
}
}
@@ -1541,7 +1549,6 @@ static struct phy_driver dp83640_driver = {
.remove = dp83640_remove,
.soft_reset = dp83640_soft_reset,
.config_init = dp83640_config_init,
- .ack_interrupt = dp83640_ack_interrupt,
.config_intr = dp83640_config_intr,
.handle_interrupt = dp83640_handle_interrupt,
};
diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index bb512ac3f533..fff371ca1086 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -119,21 +119,6 @@ struct dp83822_private {
u16 fx_sd_enable;
};
-static int dp83822_ack_interrupt(struct phy_device *phydev)
-{
- int err;
-
- err = phy_read(phydev, MII_DP83822_MISR1);
- if (err < 0)
- return err;
-
- err = phy_read(phydev, MII_DP83822_MISR2);
- if (err < 0)
- return err;
-
- return 0;
-}
-
static int dp83822_set_wol(struct phy_device *phydev,
struct ethtool_wolinfo *wol)
{
@@ -609,7 +594,6 @@ static int dp83822_resume(struct phy_device *phydev)
.read_status = dp83822_read_status, \
.get_wol = dp83822_get_wol, \
.set_wol = dp83822_set_wol, \
- .ack_interrupt = dp83822_ack_interrupt, \
.config_intr = dp83822_config_intr, \
.handle_interrupt = dp83822_handle_interrupt, \
.suspend = dp83822_suspend, \
@@ -625,7 +609,6 @@ static int dp83822_resume(struct phy_device *phydev)
.config_init = dp8382x_config_init, \
.get_wol = dp83822_get_wol, \
.set_wol = dp83822_set_wol, \
- .ack_interrupt = dp83822_ack_interrupt, \
.config_intr = dp83822_config_intr, \
.handle_interrupt = dp83822_handle_interrupt, \
.suspend = dp83822_suspend, \
diff --git a/drivers/net/phy/dp83848.c b/drivers/net/phy/dp83848.c
index b707a9b27847..937061acfc61 100644
--- a/drivers/net/phy/dp83848.c
+++ b/drivers/net/phy/dp83848.c
@@ -67,17 +67,28 @@ static int dp83848_config_intr(struct phy_device *phydev)
return control;
if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+ ret = dp83848_ack_interrupt(phydev);
+ if (ret)
+ return ret;
+
control |= DP83848_MICR_INT_OE;
control |= DP83848_MICR_INTEN;
ret = phy_write(phydev, DP83848_MISR, DP83848_INT_EN_MASK);
if (ret < 0)
return ret;
+
+ ret = phy_write(phydev, DP83848_MICR, control);
} else {
control &= ~DP83848_MICR_INTEN;
+ ret = phy_write(phydev, DP83848_MICR, control);
+ if (ret)
+ return ret;
+
+ ret = dp83848_ack_interrupt(phydev);
}
- return phy_write(phydev, DP83848_MICR, control);
+ return ret;
}
static irqreturn_t dp83848_handle_interrupt(struct phy_device *phydev)
@@ -134,7 +145,6 @@ MODULE_DEVICE_TABLE(mdio, dp83848_tbl);
.resume = genphy_resume, \
\
/* IRQ related */ \
- .ack_interrupt = dp83848_ack_interrupt, \
.config_intr = dp83848_config_intr, \
.handle_interrupt = dp83848_handle_interrupt, \
}
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index aba4e4c1f75c..9bd9a5c0b1db 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -288,9 +288,13 @@ static void dp83867_get_wol(struct phy_device *phydev,
static int dp83867_config_intr(struct phy_device *phydev)
{
- int micr_status;
+ int micr_status, err;
if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+ err = dp83867_ack_interrupt(phydev);
+ if (err)
+ return err;
+
micr_status = phy_read(phydev, MII_DP83867_MICR);
if (micr_status < 0)
return micr_status;
@@ -303,11 +307,17 @@ static int dp83867_config_intr(struct phy_device *phydev)
MII_DP83867_MICR_DUP_MODE_CHNG_INT_EN |
MII_DP83867_MICR_SLEEP_MODE_CHNG_INT_EN);
- return phy_write(phydev, MII_DP83867_MICR, micr_status);
+ err = phy_write(phydev, MII_DP83867_MICR, micr_status);
+ } else {
+ micr_status = 0x0;
+ err = phy_write(phydev, MII_DP83867_MICR, micr_status);
+ if (err)
+ return err;
+
+ err = dp83867_ack_interrupt(phydev);
}
- micr_status = 0x0;
- return phy_write(phydev, MII_DP83867_MICR, micr_status);
+ return err;
}
static irqreturn_t dp83867_handle_interrupt(struct phy_device *phydev)
@@ -849,7 +859,6 @@ static struct phy_driver dp83867_driver[] = {
.set_wol = dp83867_set_wol,
/* IRQ related */
- .ack_interrupt = dp83867_ack_interrupt,
.config_intr = dp83867_config_intr,
.handle_interrupt = dp83867_handle_interrupt,
diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c
index 487d1b8beec5..b30bc142d82e 100644
--- a/drivers/net/phy/dp83869.c
+++ b/drivers/net/phy/dp83869.c
@@ -186,9 +186,13 @@ static int dp83869_ack_interrupt(struct phy_device *phydev)
static int dp83869_config_intr(struct phy_device *phydev)
{
- int micr_status = 0;
+ int micr_status = 0, err;
if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+ err = dp83869_ack_interrupt(phydev);
+ if (err)
+ return err;
+
micr_status = phy_read(phydev, MII_DP83869_MICR);
if (micr_status < 0)
return micr_status;
@@ -201,10 +205,16 @@ static int dp83869_config_intr(struct phy_device *phydev)
MII_DP83869_MICR_DUP_MODE_CHNG_INT_EN |
MII_DP83869_MICR_SLEEP_MODE_CHNG_INT_EN);
- return phy_write(phydev, MII_DP83869_MICR, micr_status);
+ err = phy_write(phydev, MII_DP83869_MICR, micr_status);
+ } else {
+ err = phy_write(phydev, MII_DP83869_MICR, micr_status);
+ if (err)
+ return err;
+
+ err = dp83869_ack_interrupt(phydev);
}
- return phy_write(phydev, MII_DP83869_MICR, micr_status);
+ return err;
}
static irqreturn_t dp83869_handle_interrupt(struct phy_device *phydev)
@@ -874,7 +884,6 @@ static struct phy_driver dp83869_driver[] = {
.soft_reset = dp83869_phy_reset,
/* IRQ related */
- .ack_interrupt = dp83869_ack_interrupt,
.config_intr = dp83869_config_intr,
.handle_interrupt = dp83869_handle_interrupt,
.read_status = dp83869_read_status,
diff --git a/drivers/net/phy/dp83tc811.c b/drivers/net/phy/dp83tc811.c
index a93c64ac76a3..688fadffb249 100644
--- a/drivers/net/phy/dp83tc811.c
+++ b/drivers/net/phy/dp83tc811.c
@@ -197,6 +197,10 @@ static int dp83811_config_intr(struct phy_device *phydev)
int misr_status, err;
if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+ err = dp83811_ack_interrupt(phydev);
+ if (err)
+ return err;
+
misr_status = phy_read(phydev, MII_DP83811_INT_STAT1);
if (misr_status < 0)
return misr_status;
@@ -249,6 +253,10 @@ static int dp83811_config_intr(struct phy_device *phydev)
return err;
err = phy_write(phydev, MII_DP83811_INT_STAT3, 0);
+ if (err < 0)
+ return err;
+
+ err = dp83811_ack_interrupt(phydev);
}
return err;
@@ -386,7 +394,6 @@ static struct phy_driver dp83811_driver[] = {
.soft_reset = dp83811_phy_reset,
.get_wol = dp83811_get_wol,
.set_wol = dp83811_set_wol,
- .ack_interrupt = dp83811_ack_interrupt,
.config_intr = dp83811_config_intr,
.handle_interrupt = dp83811_handle_interrupt,
.suspend = dp83811_suspend,
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 14/15] net: phy: qsemi: remove the use of .ack_interrupt()
From: Ioana Ciornei @ 2020-11-23 15:38 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, Florian Fainelli,
Jakub Kicinski, netdev, linux-kernel
Cc: Ioana Ciornei
In-Reply-To: <20201123153817.1616814-1-ciorneiioana@gmail.com>
From: Ioana Ciornei <ioana.ciornei@nxp.com>
In preparation of removing the .ack_interrupt() callback, we must replace
its occurrences (aka phy_clear_interrupt), from the 2 places where it is
called from (phy_enable_interrupts and phy_disable_interrupts), with
equivalent functionality.
This means that clearing interrupts now becomes something that the PHY
driver is responsible of doing, before enabling interrupts and after
clearing them. Make this driver follow the new contract.
Also, add a comment describing the multiple step interrupt
acknoledgement process.
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
drivers/net/phy/qsemi.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/qsemi.c b/drivers/net/phy/qsemi.c
index 97f29ed2f0ca..d5c1aaa8236a 100644
--- a/drivers/net/phy/qsemi.c
+++ b/drivers/net/phy/qsemi.c
@@ -75,6 +75,10 @@ static int qs6612_ack_interrupt(struct phy_device *phydev)
{
int err;
+ /* The Interrupt Source register is not self-clearing, bits 4 and 5 are
+ * cleared when MII_BMSR is read and bits 1 and 3 are cleared when
+ * MII_EXPANSION is read
+ */
err = phy_read(phydev, MII_QS6612_ISR);
if (err < 0)
@@ -96,11 +100,22 @@ static int qs6612_ack_interrupt(struct phy_device *phydev)
static int qs6612_config_intr(struct phy_device *phydev)
{
int err;
- if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
+ if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+ /* clear any interrupts before enabling them */
+ err = qs6612_ack_interrupt(phydev);
+ if (err)
+ return err;
+
err = phy_write(phydev, MII_QS6612_IMR,
MII_QS6612_IMR_INIT);
- else
+ } else {
err = phy_write(phydev, MII_QS6612_IMR, 0);
+ if (err)
+ return err;
+
+ /* clear any leftover interrupts */
+ err = qs6612_ack_interrupt(phydev);
+ }
return err;
@@ -133,7 +148,6 @@ static struct phy_driver qs6612_driver[] = { {
.phy_id_mask = 0xfffffff0,
/* PHY_BASIC_FEATURES */
.config_init = qs6612_config_init,
- .ack_interrupt = qs6612_ack_interrupt,
.config_intr = qs6612_config_intr,
.handle_interrupt = qs6612_handle_interrupt,
} };
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 13/15] net: phy: qsemi: implement generic .handle_interrupt() callback
From: Ioana Ciornei @ 2020-11-23 15:38 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, Florian Fainelli,
Jakub Kicinski, netdev, linux-kernel
Cc: Ioana Ciornei
In-Reply-To: <20201123153817.1616814-1-ciorneiioana@gmail.com>
From: Ioana Ciornei <ioana.ciornei@nxp.com>
In an attempt to actually support shared IRQs in phylib, we now move the
responsibility of triggering the phylib state machine or just returning
IRQ_NONE, based on the IRQ status register, to the PHY driver. Having
3 different IRQ handling callbacks (.handle_interrupt(),
.did_interrupt() and .ack_interrupt() ) is confusing so let the PHY
driver implement directly an IRQ handler like any other device driver.
Make this driver follow the new convention.
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
drivers/net/phy/qsemi.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/net/phy/qsemi.c b/drivers/net/phy/qsemi.c
index 1b15a991ee06..97f29ed2f0ca 100644
--- a/drivers/net/phy/qsemi.c
+++ b/drivers/net/phy/qsemi.c
@@ -106,6 +106,27 @@ static int qs6612_config_intr(struct phy_device *phydev)
}
+static irqreturn_t qs6612_handle_interrupt(struct phy_device *phydev)
+{
+ int irq_status;
+
+ irq_status = phy_read(phydev, MII_QS6612_ISR);
+ if (irq_status < 0) {
+ phy_error(phydev);
+ return IRQ_NONE;
+ }
+
+ if (!(irq_status & MII_QS6612_IMR_INIT))
+ return IRQ_NONE;
+
+ /* the interrupt source register is not self-clearing */
+ qs6612_ack_interrupt(phydev);
+
+ phy_trigger_machine(phydev);
+
+ return IRQ_HANDLED;
+}
+
static struct phy_driver qs6612_driver[] = { {
.phy_id = 0x00181440,
.name = "QS6612",
@@ -114,6 +135,7 @@ static struct phy_driver qs6612_driver[] = { {
.config_init = qs6612_config_init,
.ack_interrupt = qs6612_ack_interrupt,
.config_intr = qs6612_config_intr,
+ .handle_interrupt = qs6612_handle_interrupt,
} };
module_phy_driver(qs6612_driver);
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 09/15] net: phy: national: implement generic .handle_interrupt() callback
From: Ioana Ciornei @ 2020-11-23 15:38 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, Florian Fainelli,
Jakub Kicinski, netdev, linux-kernel
Cc: Ioana Ciornei
In-Reply-To: <20201123153817.1616814-1-ciorneiioana@gmail.com>
From: Ioana Ciornei <ioana.ciornei@nxp.com>
In an attempt to actually support shared IRQs in phylib, we now move the
responsibility of triggering the phylib state machine or just returning
IRQ_NONE, based on the IRQ status register, to the PHY driver. Having
3 different IRQ handling callbacks (.handle_interrupt(),
.did_interrupt() and .ack_interrupt() ) is confusing so let the PHY
driver implement directly an IRQ handler like any other device driver.
Make this driver follow the new convention.
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
drivers/net/phy/national.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/net/phy/national.c b/drivers/net/phy/national.c
index a5bf0874c7d8..8c71fd66b0b1 100644
--- a/drivers/net/phy/national.c
+++ b/drivers/net/phy/national.c
@@ -89,6 +89,27 @@ static int ns_ack_interrupt(struct phy_device *phydev)
return ret;
}
+static irqreturn_t ns_handle_interrupt(struct phy_device *phydev)
+{
+ int irq_status;
+
+ irq_status = phy_read(phydev, DP83865_INT_STATUS);
+ if (irq_status < 0) {
+ phy_error(phydev);
+ return IRQ_NONE;
+ }
+
+ if (!(irq_status & DP83865_INT_MASK_DEFAULT))
+ return IRQ_NONE;
+
+ /* clear the interrupt */
+ phy_write(phydev, DP83865_INT_CLEAR, irq_status & ~0x7);
+
+ phy_trigger_machine(phydev);
+
+ return IRQ_HANDLED;
+}
+
static void ns_giga_speed_fallback(struct phy_device *phydev, int mode)
{
int bmcr = phy_read(phydev, MII_BMCR);
@@ -135,6 +156,7 @@ static struct phy_driver dp83865_driver[] = { {
.config_init = ns_config_init,
.ack_interrupt = ns_ack_interrupt,
.config_intr = ns_config_intr,
+ .handle_interrupt = ns_handle_interrupt,
} };
module_phy_driver(dp83865_driver);
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 10/15] net: phy: national: remove the use of the .ack_interrupt()
From: Ioana Ciornei @ 2020-11-23 15:38 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, Florian Fainelli,
Jakub Kicinski, netdev, linux-kernel
Cc: Ioana Ciornei
In-Reply-To: <20201123153817.1616814-1-ciorneiioana@gmail.com>
From: Ioana Ciornei <ioana.ciornei@nxp.com>
In preparation of removing the .ack_interrupt() callback, we must replace
its occurrences (aka phy_clear_interrupt), from the 2 places where it is
called from (phy_enable_interrupts and phy_disable_interrupts), with
equivalent functionality.
This means that clearing interrupts now becomes something that the PHY
driver is responsible of doing, before enabling interrupts and after
clearing them. Make this driver follow the new contract.
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
drivers/net/phy/national.c | 36 ++++++++++++++++++++++--------------
1 file changed, 22 insertions(+), 14 deletions(-)
diff --git a/drivers/net/phy/national.c b/drivers/net/phy/national.c
index 8c71fd66b0b1..5a8c8eb18582 100644
--- a/drivers/net/phy/national.c
+++ b/drivers/net/phy/national.c
@@ -63,19 +63,6 @@ static void ns_exp_write(struct phy_device *phydev, u16 reg, u8 data)
phy_write(phydev, NS_EXP_MEM_DATA, data);
}
-static int ns_config_intr(struct phy_device *phydev)
-{
- int err;
-
- if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
- err = phy_write(phydev, DP83865_INT_MASK,
- DP83865_INT_MASK_DEFAULT);
- else
- err = phy_write(phydev, DP83865_INT_MASK, 0);
-
- return err;
-}
-
static int ns_ack_interrupt(struct phy_device *phydev)
{
int ret = phy_read(phydev, DP83865_INT_STATUS);
@@ -110,6 +97,28 @@ static irqreturn_t ns_handle_interrupt(struct phy_device *phydev)
return IRQ_HANDLED;
}
+static int ns_config_intr(struct phy_device *phydev)
+{
+ int err;
+
+ if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+ err = ns_ack_interrupt(phydev);
+ if (err)
+ return err;
+
+ err = phy_write(phydev, DP83865_INT_MASK,
+ DP83865_INT_MASK_DEFAULT);
+ } else {
+ err = phy_write(phydev, DP83865_INT_MASK, 0);
+ if (err)
+ return err;
+
+ err = ns_ack_interrupt(phydev);
+ }
+
+ return err;
+}
+
static void ns_giga_speed_fallback(struct phy_device *phydev, int mode)
{
int bmcr = phy_read(phydev, MII_BMCR);
@@ -154,7 +163,6 @@ static struct phy_driver dp83865_driver[] = { {
.name = "NatSemi DP83865",
/* PHY_GBIT_FEATURES */
.config_init = ns_config_init,
- .ack_interrupt = ns_ack_interrupt,
.config_intr = ns_config_intr,
.handle_interrupt = ns_handle_interrupt,
} };
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 05/15] net: phy: meson-gxl: implement generic .handle_interrupt() callback
From: Ioana Ciornei @ 2020-11-23 15:38 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, Florian Fainelli,
Jakub Kicinski, netdev, linux-kernel
Cc: Ioana Ciornei, Jerome Brunet, Neil Armstrong
In-Reply-To: <20201123153817.1616814-1-ciorneiioana@gmail.com>
From: Ioana Ciornei <ioana.ciornei@nxp.com>
In an attempt to actually support shared IRQs in phylib, we now move the
responsibility of triggering the phylib state machine or just returning
IRQ_NONE, based on the IRQ status register, to the PHY driver. Having
3 different IRQ handling callbacks (.handle_interrupt(),
.did_interrupt() and .ack_interrupt() ) is confusing so let the PHY
driver implement directly an IRQ handler like any other device driver.
Make this driver follow the new convention.
Cc: Jerome Brunet <jbrunet@baylibre.com>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
drivers/net/phy/meson-gxl.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index e8f2ca625837..b16b1cc89165 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -222,6 +222,24 @@ static int meson_gxl_config_intr(struct phy_device *phydev)
return phy_write(phydev, INTSRC_MASK, val);
}
+static irqreturn_t meson_gxl_handle_interrupt(struct phy_device *phydev)
+{
+ int irq_status;
+
+ irq_status = phy_read(phydev, INTSRC_FLAG);
+ if (irq_status < 0) {
+ phy_error(phydev);
+ return IRQ_NONE;
+ }
+
+ if (irq_status == 0)
+ return IRQ_NONE;
+
+ phy_trigger_machine(phydev);
+
+ return IRQ_HANDLED;
+}
+
static struct phy_driver meson_gxl_phy[] = {
{
PHY_ID_MATCH_EXACT(0x01814400),
@@ -233,6 +251,7 @@ static struct phy_driver meson_gxl_phy[] = {
.read_status = meson_gxl_read_status,
.ack_interrupt = meson_gxl_ack_interrupt,
.config_intr = meson_gxl_config_intr,
+ .handle_interrupt = meson_gxl_handle_interrupt,
.suspend = genphy_suspend,
.resume = genphy_resume,
}, {
@@ -243,6 +262,7 @@ static struct phy_driver meson_gxl_phy[] = {
.soft_reset = genphy_soft_reset,
.ack_interrupt = meson_gxl_ack_interrupt,
.config_intr = meson_gxl_config_intr,
+ .handle_interrupt = meson_gxl_handle_interrupt,
.suspend = genphy_suspend,
.resume = genphy_resume,
},
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 02/15] net: phy: intel-xway: remove the use of .ack_interrupt()
From: Ioana Ciornei @ 2020-11-23 15:38 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, Florian Fainelli,
Jakub Kicinski, netdev, linux-kernel
Cc: Ioana Ciornei, Mathias Kresin, Hauke Mehrtens
In-Reply-To: <20201123153817.1616814-1-ciorneiioana@gmail.com>
From: Ioana Ciornei <ioana.ciornei@nxp.com>
In preparation of removing the .ack_interrupt() callback, we must replace
its occurrences (aka phy_clear_interrupt), from the 2 places where it is
called from (phy_enable_interrupts and phy_disable_interrupts), with
equivalent functionality.
This means that clearing interrupts now becomes something that the PHY
driver is responsible of doing, before enabling interrupts and after
clearing them. Make this driver follow the new contract.
Cc: Mathias Kresin <dev@kresin.me>
Cc: Hauke Mehrtens <hauke@hauke-m.de>
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
drivers/net/phy/intel-xway.c | 27 +++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/net/phy/intel-xway.c b/drivers/net/phy/intel-xway.c
index 1a27ae1bec5c..6eac50d4b42f 100644
--- a/drivers/net/phy/intel-xway.c
+++ b/drivers/net/phy/intel-xway.c
@@ -212,11 +212,24 @@ static int xway_gphy_ack_interrupt(struct phy_device *phydev)
static int xway_gphy_config_intr(struct phy_device *phydev)
{
u16 mask = 0;
+ int err;
+
+ if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+ err = xway_gphy_ack_interrupt(phydev);
+ if (err)
+ return err;
- if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
mask = XWAY_MDIO_INIT_MASK;
+ err = phy_write(phydev, XWAY_MDIO_IMASK, mask);
+ } else {
+ err = phy_write(phydev, XWAY_MDIO_IMASK, mask);
+ if (err)
+ return err;
+
+ err = xway_gphy_ack_interrupt(phydev);
+ }
- return phy_write(phydev, XWAY_MDIO_IMASK, mask);
+ return err;
}
static irqreturn_t xway_gphy_handle_interrupt(struct phy_device *phydev)
@@ -245,7 +258,6 @@ static struct phy_driver xway_gphy[] = {
/* PHY_GBIT_FEATURES */
.config_init = xway_gphy_config_init,
.config_aneg = xway_gphy14_config_aneg,
- .ack_interrupt = xway_gphy_ack_interrupt,
.handle_interrupt = xway_gphy_handle_interrupt,
.config_intr = xway_gphy_config_intr,
.suspend = genphy_suspend,
@@ -257,7 +269,6 @@ static struct phy_driver xway_gphy[] = {
/* PHY_BASIC_FEATURES */
.config_init = xway_gphy_config_init,
.config_aneg = xway_gphy14_config_aneg,
- .ack_interrupt = xway_gphy_ack_interrupt,
.handle_interrupt = xway_gphy_handle_interrupt,
.config_intr = xway_gphy_config_intr,
.suspend = genphy_suspend,
@@ -269,7 +280,6 @@ static struct phy_driver xway_gphy[] = {
/* PHY_GBIT_FEATURES */
.config_init = xway_gphy_config_init,
.config_aneg = xway_gphy14_config_aneg,
- .ack_interrupt = xway_gphy_ack_interrupt,
.handle_interrupt = xway_gphy_handle_interrupt,
.config_intr = xway_gphy_config_intr,
.suspend = genphy_suspend,
@@ -281,7 +291,6 @@ static struct phy_driver xway_gphy[] = {
/* PHY_BASIC_FEATURES */
.config_init = xway_gphy_config_init,
.config_aneg = xway_gphy14_config_aneg,
- .ack_interrupt = xway_gphy_ack_interrupt,
.handle_interrupt = xway_gphy_handle_interrupt,
.config_intr = xway_gphy_config_intr,
.suspend = genphy_suspend,
@@ -292,7 +301,6 @@ static struct phy_driver xway_gphy[] = {
.name = "Intel XWAY PHY11G (PEF 7071/PEF 7072) v1.5 / v1.6",
/* PHY_GBIT_FEATURES */
.config_init = xway_gphy_config_init,
- .ack_interrupt = xway_gphy_ack_interrupt,
.handle_interrupt = xway_gphy_handle_interrupt,
.config_intr = xway_gphy_config_intr,
.suspend = genphy_suspend,
@@ -303,7 +311,6 @@ static struct phy_driver xway_gphy[] = {
.name = "Intel XWAY PHY22F (PEF 7061) v1.5 / v1.6",
/* PHY_BASIC_FEATURES */
.config_init = xway_gphy_config_init,
- .ack_interrupt = xway_gphy_ack_interrupt,
.handle_interrupt = xway_gphy_handle_interrupt,
.config_intr = xway_gphy_config_intr,
.suspend = genphy_suspend,
@@ -314,7 +321,6 @@ static struct phy_driver xway_gphy[] = {
.name = "Intel XWAY PHY11G (xRX v1.1 integrated)",
/* PHY_GBIT_FEATURES */
.config_init = xway_gphy_config_init,
- .ack_interrupt = xway_gphy_ack_interrupt,
.handle_interrupt = xway_gphy_handle_interrupt,
.config_intr = xway_gphy_config_intr,
.suspend = genphy_suspend,
@@ -325,7 +331,6 @@ static struct phy_driver xway_gphy[] = {
.name = "Intel XWAY PHY22F (xRX v1.1 integrated)",
/* PHY_BASIC_FEATURES */
.config_init = xway_gphy_config_init,
- .ack_interrupt = xway_gphy_ack_interrupt,
.handle_interrupt = xway_gphy_handle_interrupt,
.config_intr = xway_gphy_config_intr,
.suspend = genphy_suspend,
@@ -336,7 +341,6 @@ static struct phy_driver xway_gphy[] = {
.name = "Intel XWAY PHY11G (xRX v1.2 integrated)",
/* PHY_GBIT_FEATURES */
.config_init = xway_gphy_config_init,
- .ack_interrupt = xway_gphy_ack_interrupt,
.handle_interrupt = xway_gphy_handle_interrupt,
.config_intr = xway_gphy_config_intr,
.suspend = genphy_suspend,
@@ -347,7 +351,6 @@ static struct phy_driver xway_gphy[] = {
.name = "Intel XWAY PHY22F (xRX v1.2 integrated)",
/* PHY_BASIC_FEATURES */
.config_init = xway_gphy_config_init,
- .ack_interrupt = xway_gphy_ack_interrupt,
.handle_interrupt = xway_gphy_handle_interrupt,
.config_intr = xway_gphy_config_intr,
.suspend = genphy_suspend,
--
2.28.0
^ permalink raw reply related
* [PATCH net-next 01/15] net: phy: intel-xway: implement generic .handle_interrupt() callback
From: Ioana Ciornei @ 2020-11-23 15:38 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, Florian Fainelli,
Jakub Kicinski, netdev, linux-kernel
Cc: Ioana Ciornei, Mathias Kresin, Hauke Mehrtens
In-Reply-To: <20201123153817.1616814-1-ciorneiioana@gmail.com>
From: Ioana Ciornei <ioana.ciornei@nxp.com>
In an attempt to actually support shared IRQs in phylib, we now move the
responsibility of triggering the phylib state machine or just returning
IRQ_NONE, based on the IRQ status register, to the PHY driver. Having
3 different IRQ handling callbacks (.handle_interrupt(),
.did_interrupt() and .ack_interrupt() ) is confusing so let the PHY
driver implement directly an IRQ handler like any other device driver.
Make this driver follow the new convention.
Cc: Mathias Kresin <dev@kresin.me>
Cc: Hauke Mehrtens <hauke@hauke-m.de>
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
drivers/net/phy/intel-xway.c | 46 ++++++++++++++++++++++--------------
1 file changed, 28 insertions(+), 18 deletions(-)
diff --git a/drivers/net/phy/intel-xway.c b/drivers/net/phy/intel-xway.c
index b7875b36097f..1a27ae1bec5c 100644
--- a/drivers/net/phy/intel-xway.c
+++ b/drivers/net/phy/intel-xway.c
@@ -209,14 +209,6 @@ static int xway_gphy_ack_interrupt(struct phy_device *phydev)
return (reg < 0) ? reg : 0;
}
-static int xway_gphy_did_interrupt(struct phy_device *phydev)
-{
- int reg;
-
- reg = phy_read(phydev, XWAY_MDIO_ISTAT);
- return reg & XWAY_MDIO_INIT_MASK;
-}
-
static int xway_gphy_config_intr(struct phy_device *phydev)
{
u16 mask = 0;
@@ -227,6 +219,24 @@ static int xway_gphy_config_intr(struct phy_device *phydev)
return phy_write(phydev, XWAY_MDIO_IMASK, mask);
}
+static irqreturn_t xway_gphy_handle_interrupt(struct phy_device *phydev)
+{
+ int irq_status;
+
+ irq_status = phy_read(phydev, XWAY_MDIO_ISTAT);
+ if (irq_status < 0) {
+ phy_error(phydev);
+ return IRQ_NONE;
+ }
+
+ if (!(irq_status & XWAY_MDIO_INIT_MASK))
+ return IRQ_NONE;
+
+ phy_trigger_machine(phydev);
+
+ return IRQ_HANDLED;
+}
+
static struct phy_driver xway_gphy[] = {
{
.phy_id = PHY_ID_PHY11G_1_3,
@@ -236,7 +246,7 @@ static struct phy_driver xway_gphy[] = {
.config_init = xway_gphy_config_init,
.config_aneg = xway_gphy14_config_aneg,
.ack_interrupt = xway_gphy_ack_interrupt,
- .did_interrupt = xway_gphy_did_interrupt,
+ .handle_interrupt = xway_gphy_handle_interrupt,
.config_intr = xway_gphy_config_intr,
.suspend = genphy_suspend,
.resume = genphy_resume,
@@ -248,7 +258,7 @@ static struct phy_driver xway_gphy[] = {
.config_init = xway_gphy_config_init,
.config_aneg = xway_gphy14_config_aneg,
.ack_interrupt = xway_gphy_ack_interrupt,
- .did_interrupt = xway_gphy_did_interrupt,
+ .handle_interrupt = xway_gphy_handle_interrupt,
.config_intr = xway_gphy_config_intr,
.suspend = genphy_suspend,
.resume = genphy_resume,
@@ -260,7 +270,7 @@ static struct phy_driver xway_gphy[] = {
.config_init = xway_gphy_config_init,
.config_aneg = xway_gphy14_config_aneg,
.ack_interrupt = xway_gphy_ack_interrupt,
- .did_interrupt = xway_gphy_did_interrupt,
+ .handle_interrupt = xway_gphy_handle_interrupt,
.config_intr = xway_gphy_config_intr,
.suspend = genphy_suspend,
.resume = genphy_resume,
@@ -272,7 +282,7 @@ static struct phy_driver xway_gphy[] = {
.config_init = xway_gphy_config_init,
.config_aneg = xway_gphy14_config_aneg,
.ack_interrupt = xway_gphy_ack_interrupt,
- .did_interrupt = xway_gphy_did_interrupt,
+ .handle_interrupt = xway_gphy_handle_interrupt,
.config_intr = xway_gphy_config_intr,
.suspend = genphy_suspend,
.resume = genphy_resume,
@@ -283,7 +293,7 @@ static struct phy_driver xway_gphy[] = {
/* PHY_GBIT_FEATURES */
.config_init = xway_gphy_config_init,
.ack_interrupt = xway_gphy_ack_interrupt,
- .did_interrupt = xway_gphy_did_interrupt,
+ .handle_interrupt = xway_gphy_handle_interrupt,
.config_intr = xway_gphy_config_intr,
.suspend = genphy_suspend,
.resume = genphy_resume,
@@ -294,7 +304,7 @@ static struct phy_driver xway_gphy[] = {
/* PHY_BASIC_FEATURES */
.config_init = xway_gphy_config_init,
.ack_interrupt = xway_gphy_ack_interrupt,
- .did_interrupt = xway_gphy_did_interrupt,
+ .handle_interrupt = xway_gphy_handle_interrupt,
.config_intr = xway_gphy_config_intr,
.suspend = genphy_suspend,
.resume = genphy_resume,
@@ -305,7 +315,7 @@ static struct phy_driver xway_gphy[] = {
/* PHY_GBIT_FEATURES */
.config_init = xway_gphy_config_init,
.ack_interrupt = xway_gphy_ack_interrupt,
- .did_interrupt = xway_gphy_did_interrupt,
+ .handle_interrupt = xway_gphy_handle_interrupt,
.config_intr = xway_gphy_config_intr,
.suspend = genphy_suspend,
.resume = genphy_resume,
@@ -316,7 +326,7 @@ static struct phy_driver xway_gphy[] = {
/* PHY_BASIC_FEATURES */
.config_init = xway_gphy_config_init,
.ack_interrupt = xway_gphy_ack_interrupt,
- .did_interrupt = xway_gphy_did_interrupt,
+ .handle_interrupt = xway_gphy_handle_interrupt,
.config_intr = xway_gphy_config_intr,
.suspend = genphy_suspend,
.resume = genphy_resume,
@@ -327,7 +337,7 @@ static struct phy_driver xway_gphy[] = {
/* PHY_GBIT_FEATURES */
.config_init = xway_gphy_config_init,
.ack_interrupt = xway_gphy_ack_interrupt,
- .did_interrupt = xway_gphy_did_interrupt,
+ .handle_interrupt = xway_gphy_handle_interrupt,
.config_intr = xway_gphy_config_intr,
.suspend = genphy_suspend,
.resume = genphy_resume,
@@ -338,7 +348,7 @@ static struct phy_driver xway_gphy[] = {
/* PHY_BASIC_FEATURES */
.config_init = xway_gphy_config_init,
.ack_interrupt = xway_gphy_ack_interrupt,
- .did_interrupt = xway_gphy_did_interrupt,
+ .handle_interrupt = xway_gphy_handle_interrupt,
.config_intr = xway_gphy_config_intr,
.suspend = genphy_suspend,
.resume = genphy_resume,
--
2.28.0
^ permalink raw reply related
* Re: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
From: Russell King - ARM Linux admin @ 2020-11-23 15:33 UTC (permalink / raw)
To: Stefan Chulski
Cc: netdev@vger.kernel.org, thomas.petazzoni@bootlin.com,
davem@davemloft.net, Nadav Haklai, Yan Markman,
linux-kernel@vger.kernel.org, kuba@kernel.org, mw@semihalf.com,
antoine.tenart@bootlin.com, andrew@lunn.ch
In-Reply-To: <CO6PR18MB3873522226E3F9A608371289B0FC0@CO6PR18MB3873.namprd18.prod.outlook.com>
On Mon, Nov 23, 2020 at 03:26:11PM +0000, Stefan Chulski wrote:
> > -----Original Message-----
> > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > Sent: Monday, November 23, 2020 5:11 PM
> > To: Stefan Chulski <stefanc@marvell.com>
> > Cc: netdev@vger.kernel.org; thomas.petazzoni@bootlin.com;
> > davem@davemloft.net; Nadav Haklai <nadavh@marvell.com>; Yan Markman
> > <ymarkman@marvell.com>; linux-kernel@vger.kernel.org; kuba@kernel.org;
> > mw@semihalf.com; antoine.tenart@bootlin.com; andrew@lunn.ch
> > Subject: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > Hi,
> >
> > On Mon, Nov 23, 2020 at 04:52:40PM +0200, stefanc@marvell.com wrote:
> > > From: Stefan Chulski <stefanc@marvell.com>
> > >
> > > Tx/Rx FIFO is a HW resource limited by total size, but shared by all
> > > ports of same CP110 and impacting port-performance.
> > > Do not divide the FIFO for ports which are not enabled in DTS, so
> > > active ports could have more FIFO.
> > >
> > > The active port mapping should be done in probe before FIFO-init.
> >
> > It would be nice to know what the effect is from this - is it a small or large
> > boost in performance?
>
> I didn't saw any significant improvement with LINUX bridge or forwarding, but
> this reduced PPv2 overruns drops, reduced CRC sent errors with DPDK user space application.
> So this improved zero loss throughput. Probably with XDP we would see a similar effect.
>
> > What is the effect when the ports on a CP110 are configured for 10G, 1G, and
> > 2.5G in that order, as is the case on the Macchiatobin board?
>
> Macchiatobin has two CP's. CP1 has 3 ports, so the distribution of FIFO would be the same as before.
> On CP0 which has a single port, all FIFO would be allocated for 10G port.
Your code allocates for CP1:
32K to port 0 (the 10G port on Macchiatobin)
8K to port 1 (the 1G dedicated ethernet port on Macchiatobin)
4K to port 2 (the 1G/2.5G SFP port on Macchiatobin)
I'm questioning that allocation for port 1 and 2.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH net] bonding: fix feature flag setting at init time
From: kernel test robot @ 2020-11-23 15:32 UTC (permalink / raw)
To: Jarod Wilson, linux-kernel
Cc: kbuild-all, Jarod Wilson, Ivan Vecera, Jay Vosburgh,
Veaceslav Falico, Andy Gospodarek, Jakub Kicinski, Thomas Davis,
netdev
In-Reply-To: <20201123031716.6179-1-jarod@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 3134 bytes --]
Hi Jarod,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
[also build test ERROR on next-20201123]
[cannot apply to net/master linux/master linus/master sparc-next/master v5.10-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Jarod-Wilson/bonding-fix-feature-flag-setting-at-init-time/20201123-111956
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git f9e425e99b0756c1479042afe761073779df2a30
config: x86_64-rhel (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/6d883c4c2b01573ba9dddcb9fe109f961a8b7f10
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Jarod-Wilson/bonding-fix-feature-flag-setting-at-init-time/20201123-111956
git checkout 6d883c4c2b01573ba9dddcb9fe109f961a8b7f10
# save the attached .config to linux build tree
make W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/net/bonding/bond_options.c: In function 'bond_option_mode_set':
>> drivers/net/bonding/bond_options.c:752:38: error: 'BOND_XFRM_FEATURES' undeclared (first use in this function)
752 | netdev_features_t mask = features & BOND_XFRM_FEATURES;
| ^~~~~~~~~~~~~~~~~~
drivers/net/bonding/bond_options.c:752:38: note: each undeclared identifier is reported only once for each function it appears in
drivers/net/bonding/bond_options.c:752:20: warning: unused variable 'mask' [-Wunused-variable]
752 | netdev_features_t mask = features & BOND_XFRM_FEATURES;
| ^~~~
vim +/BOND_XFRM_FEATURES +752 drivers/net/bonding/bond_options.c
747
748 static int bond_option_mode_set(struct bonding *bond,
749 const struct bond_opt_value *newval)
750 {
751 netdev_features_t features = bond->dev->features;
> 752 netdev_features_t mask = features & BOND_XFRM_FEATURES;
753
754 if (!bond_mode_uses_arp(newval->value)) {
755 if (bond->params.arp_interval) {
756 netdev_dbg(bond->dev, "%s mode is incompatible with arp monitoring, start mii monitoring\n",
757 newval->string);
758 /* disable arp monitoring */
759 bond->params.arp_interval = 0;
760 }
761
762 if (!bond->params.miimon) {
763 /* set miimon to default value */
764 bond->params.miimon = BOND_DEFAULT_MIIMON;
765 netdev_dbg(bond->dev, "Setting MII monitoring interval to %d\n",
766 bond->params.miimon);
767 }
768 }
769
770 if (newval->value == BOND_MODE_ALB)
771 bond->params.tlb_dynamic_lb = 1;
772
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 45410 bytes --]
^ permalink raw reply
* Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
From: Russell King - ARM Linux admin @ 2020-11-23 15:30 UTC (permalink / raw)
To: stefanc
Cc: netdev, thomas.petazzoni, davem, nadavh, ymarkman, linux-kernel,
kuba, mw, andrew
In-Reply-To: <20201123151049.GV1551@shell.armlinux.org.uk>
On Mon, Nov 23, 2020 at 03:10:49PM +0000, Russell King - ARM Linux admin wrote:
> Hi,
>
> On Mon, Nov 23, 2020 at 04:52:40PM +0200, stefanc@marvell.com wrote:
> > From: Stefan Chulski <stefanc@marvell.com>
> >
> > Tx/Rx FIFO is a HW resource limited by total size, but shared
> > by all ports of same CP110 and impacting port-performance.
> > Do not divide the FIFO for ports which are not enabled in DTS,
> > so active ports could have more FIFO.
> >
> > The active port mapping should be done in probe before FIFO-init.
>
> It would be nice to know what the effect is from this - is it a
> small or large boost in performance?
>
> What is the effect when the ports on a CP110 are configured for
> 10G, 1G, and 2.5G in that order, as is the case on the Macchiatobin
> board?
(dropped Antoine, his email is bouncing.)
I've rechecked, and on Macchiatobin, it certainly is:
Port 0 = 10G SFP/88x3310
Port 1 = 1G dedicated ethernet
Port 2 = 1G/2.5G SFP slot
and we do run the SFP slot at 2.5G speeds.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* RE: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
From: Stefan Chulski @ 2020-11-23 15:26 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: netdev@vger.kernel.org, thomas.petazzoni@bootlin.com,
davem@davemloft.net, Nadav Haklai, Yan Markman,
linux-kernel@vger.kernel.org, kuba@kernel.org, mw@semihalf.com,
antoine.tenart@bootlin.com, andrew@lunn.ch
In-Reply-To: <20201123151049.GV1551@shell.armlinux.org.uk>
> -----Original Message-----
> From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> Sent: Monday, November 23, 2020 5:11 PM
> To: Stefan Chulski <stefanc@marvell.com>
> Cc: netdev@vger.kernel.org; thomas.petazzoni@bootlin.com;
> davem@davemloft.net; Nadav Haklai <nadavh@marvell.com>; Yan Markman
> <ymarkman@marvell.com>; linux-kernel@vger.kernel.org; kuba@kernel.org;
> mw@semihalf.com; antoine.tenart@bootlin.com; andrew@lunn.ch
> Subject: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
>
> External Email
>
> ----------------------------------------------------------------------
> Hi,
>
> On Mon, Nov 23, 2020 at 04:52:40PM +0200, stefanc@marvell.com wrote:
> > From: Stefan Chulski <stefanc@marvell.com>
> >
> > Tx/Rx FIFO is a HW resource limited by total size, but shared by all
> > ports of same CP110 and impacting port-performance.
> > Do not divide the FIFO for ports which are not enabled in DTS, so
> > active ports could have more FIFO.
> >
> > The active port mapping should be done in probe before FIFO-init.
>
> It would be nice to know what the effect is from this - is it a small or large
> boost in performance?
I didn't saw any significant improvement with LINUX bridge or forwarding, but
this reduced PPv2 overruns drops, reduced CRC sent errors with DPDK user space application.
So this improved zero loss throughput. Probably with XDP we would see a similar effect.
> What is the effect when the ports on a CP110 are configured for 10G, 1G, and
> 2.5G in that order, as is the case on the Macchiatobin board?
Macchiatobin has two CP's. CP1 has 3 ports, so the distribution of FIFO would be the same as before.
On CP0 which has a single port, all FIFO would be allocated for 10G port.
Regards.
^ permalink raw reply
* Re: [PATCH] dpaa2-eth: Fix compile error due to missing devlink support
From: Ioana Ciornei @ 2020-11-23 15:15 UTC (permalink / raw)
To: Ezequiel Garcia
Cc: netdev@vger.kernel.org, Jakub Kicinski, David S . Miller,
Ioana Ciocoi Radulescu, kernel@collabora.com
In-Reply-To: <c0e98111cafbdbb5d4d29e5e87ae779144370cf6.camel@collabora.com>
On Mon, Nov 23, 2020 at 12:06:14PM -0300, Ezequiel Garcia wrote:
> Hi Ioana,
>
> On Mon, 2020-11-23 at 09:39 +0000, Ioana Ciornei wrote:
> > Hi Ezequiel,
> >
> > Thanks a lot for the fix, I overlooked this when adding devlink support.
> >
>
> No worries :)
>
> > On Sat, Nov 21, 2020 at 09:23:36PM -0300, Ezequiel Garcia wrote:
> > > The dpaa2 driver depends on devlink, so it should select
> > > NET_DEVLINK in order to fix compile errors, such as:
> > >
> > > drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.o: in function `dpaa2_eth_rx_err':
> > > dpaa2-eth.c:(.text+0x3cec): undefined reference to `devlink_trap_report'
> > > drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.o: in function `dpaa2_eth_dl_info_get':
> > > dpaa2-eth-devlink.c:(.text+0x160): undefined reference to `devlink_info_driver_name_put'
> > >
> >
> > What tree is this intended for?
> >
>
> Oops, I forgot about netdev rules. I guess I haven't sent
> a net patch in a long time.
>
> This patch is a fix, so I guess it's for the 'net' tree.
>
> > Maybe add a fixes tag and send this towards the net tree?
> >
>
> Would you mind too much taking care of this, putting the
> Fixes you think matches best?
>
> That would be really appreciated!
>
Sure, I'll respin this.
Ioana
^ permalink raw reply
* Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
From: Russell King - ARM Linux admin @ 2020-11-23 15:10 UTC (permalink / raw)
To: stefanc
Cc: netdev, thomas.petazzoni, davem, nadavh, ymarkman, linux-kernel,
kuba, mw, antoine.tenart, andrew
In-Reply-To: <1606143160-25589-1-git-send-email-stefanc@marvell.com>
Hi,
On Mon, Nov 23, 2020 at 04:52:40PM +0200, stefanc@marvell.com wrote:
> From: Stefan Chulski <stefanc@marvell.com>
>
> Tx/Rx FIFO is a HW resource limited by total size, but shared
> by all ports of same CP110 and impacting port-performance.
> Do not divide the FIFO for ports which are not enabled in DTS,
> so active ports could have more FIFO.
>
> The active port mapping should be done in probe before FIFO-init.
It would be nice to know what the effect is from this - is it a
small or large boost in performance?
What is the effect when the ports on a CP110 are configured for
10G, 1G, and 2.5G in that order, as is the case on the Macchiatobin
board?
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH] dpaa2-eth: Fix compile error due to missing devlink support
From: Ezequiel Garcia @ 2020-11-23 15:06 UTC (permalink / raw)
To: Ioana Ciornei
Cc: netdev@vger.kernel.org, Jakub Kicinski, David S . Miller,
Ioana Ciocoi Radulescu, kernel@collabora.com
In-Reply-To: <20201123093928.pfvlpcdssjaxa37d@skbuf>
Hi Ioana,
On Mon, 2020-11-23 at 09:39 +0000, Ioana Ciornei wrote:
> Hi Ezequiel,
>
> Thanks a lot for the fix, I overlooked this when adding devlink support.
>
No worries :)
> On Sat, Nov 21, 2020 at 09:23:36PM -0300, Ezequiel Garcia wrote:
> > The dpaa2 driver depends on devlink, so it should select
> > NET_DEVLINK in order to fix compile errors, such as:
> >
> > drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.o: in function `dpaa2_eth_rx_err':
> > dpaa2-eth.c:(.text+0x3cec): undefined reference to `devlink_trap_report'
> > drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.o: in function `dpaa2_eth_dl_info_get':
> > dpaa2-eth-devlink.c:(.text+0x160): undefined reference to `devlink_info_driver_name_put'
> >
>
> What tree is this intended for?
>
Oops, I forgot about netdev rules. I guess I haven't sent
a net patch in a long time.
This patch is a fix, so I guess it's for the 'net' tree.
> Maybe add a fixes tag and send this towards the net tree?
>
Would you mind too much taking care of this, putting the
Fixes you think matches best?
That would be really appreciated!
Thanks,
Ezequiel
> Ioana
>
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > ---
> > drivers/net/ethernet/freescale/dpaa2/Kconfig | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/ethernet/freescale/dpaa2/Kconfig b/drivers/net/ethernet/freescale/dpaa2/Kconfig
> > index cfd369cf4c8c..aee59ead7250 100644
> > --- a/drivers/net/ethernet/freescale/dpaa2/Kconfig
> > +++ b/drivers/net/ethernet/freescale/dpaa2/Kconfig
> > @@ -2,6 +2,7 @@
> > config FSL_DPAA2_ETH
> > tristate "Freescale DPAA2 Ethernet"
> > depends on FSL_MC_BUS && FSL_MC_DPIO
> > + select NET_DEVLINK
> > select PHYLINK
> > select PCS_LYNX
> > help
> > --
> > 2.27.0
^ permalink raw reply
* [PATCH v3 1/1] xdp: remove the function xsk_map_inc
From: Zhu Yanjun @ 2020-11-23 15:05 UTC (permalink / raw)
To: magnus.karlsson, bjorn.topel, davem, netdev; +Cc: Zhu Yanjun
From: Zhu Yanjun <zyjzyj2000@gmail.com>
The function xsk_map_inc is a simple wrapper of bpf_map_inc and
always returns zero. As such, replacing this function with bpf_map_inc
and removing the test code.
Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
---
net/xdp/xsk.c | 2 +-
net/xdp/xsk.h | 1 -
net/xdp/xskmap.c | 13 +------------
3 files changed, 2 insertions(+), 14 deletions(-)
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index cfbec3989a76..a3c1f07d77d8 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -548,7 +548,7 @@ static struct xsk_map *xsk_get_map_list_entry(struct xdp_sock *xs,
node = list_first_entry_or_null(&xs->map_list, struct xsk_map_node,
node);
if (node) {
- WARN_ON(xsk_map_inc(node->map));
+ bpf_map_inc(&node->map->map);
map = node->map;
*map_entry = node->map_entry;
}
diff --git a/net/xdp/xsk.h b/net/xdp/xsk.h
index b9e896cee5bb..0aad25c0e223 100644
--- a/net/xdp/xsk.h
+++ b/net/xdp/xsk.h
@@ -41,7 +41,6 @@ static inline struct xdp_sock *xdp_sk(struct sock *sk)
void xsk_map_try_sock_delete(struct xsk_map *map, struct xdp_sock *xs,
struct xdp_sock **map_entry);
-int xsk_map_inc(struct xsk_map *map);
void xsk_map_put(struct xsk_map *map);
void xsk_clear_pool_at_qid(struct net_device *dev, u16 queue_id);
int xsk_reg_pool_at_qid(struct net_device *dev, struct xsk_buff_pool *pool,
diff --git a/net/xdp/xskmap.c b/net/xdp/xskmap.c
index 49da2b8ace8b..6b7e9a72b101 100644
--- a/net/xdp/xskmap.c
+++ b/net/xdp/xskmap.c
@@ -11,12 +11,6 @@
#include "xsk.h"
-int xsk_map_inc(struct xsk_map *map)
-{
- bpf_map_inc(&map->map);
- return 0;
-}
-
void xsk_map_put(struct xsk_map *map)
{
bpf_map_put(&map->map);
@@ -26,17 +20,12 @@ static struct xsk_map_node *xsk_map_node_alloc(struct xsk_map *map,
struct xdp_sock **map_entry)
{
struct xsk_map_node *node;
- int err;
node = kzalloc(sizeof(*node), GFP_ATOMIC | __GFP_NOWARN);
if (!node)
return ERR_PTR(-ENOMEM);
- err = xsk_map_inc(map);
- if (err) {
- kfree(node);
- return ERR_PTR(err);
- }
+ bpf_map_inc(&map->map);
node->map = map;
node->map_entry = map_entry;
--
2.25.1
^ permalink raw reply related
* Re: [PATCH net] netdevice.h: Fix unintentional disable of ALL_FOR_ALL features on upper device
From: Eric Dumazet @ 2020-11-23 14:55 UTC (permalink / raw)
To: Tariq Toukan
Cc: David S. Miller, Jakub Kicinski, Herbert Xu, netdev,
Moshe Shemesh, Tariq Toukan, Maxim Mikityanskiy, Saeed Mahameed
In-Reply-To: <20201123141256.14208-1-tariqt@nvidia.com>
On Mon, Nov 23, 2020 at 3:13 PM Tariq Toukan <tariqt@nvidia.com> wrote:
>
> Calling netdev_increment_features() on upper/master device from
> netdev_add_tso_features() implies unintentional clearance of ALL_FOR_ALL
> features supported by all slaves. Fix it by passing ALL_FOR_ALL in
> addition to ALL_TSO.
>
> Fixes: b0ce3508b25e ("bonding: allow TSO being set on bonding master")
I think you should give more details to your bug report, because
netdev_add_tso_features() is used from different
places.
Thanks.
^ permalink raw reply
* [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
From: stefanc @ 2020-11-23 14:52 UTC (permalink / raw)
To: netdev
Cc: thomas.petazzoni, davem, nadavh, ymarkman, linux-kernel, stefanc,
kuba, linux, mw, antoine.tenart, andrew, rmk+kernel
From: Stefan Chulski <stefanc@marvell.com>
Tx/Rx FIFO is a HW resource limited by total size, but shared
by all ports of same CP110 and impacting port-performance.
Do not divide the FIFO for ports which are not enabled in DTS,
so active ports could have more FIFO.
The active port mapping should be done in probe before FIFO-init.
Signed-off-by: Stefan Chulski <stefanc@marvell.com>
---
drivers/net/ethernet/marvell/mvpp2/mvpp2.h | 23 +++--
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 129 +++++++++++++++++-------
2 files changed, 108 insertions(+), 44 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
index 8347758..6bd7e40 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
@@ -695,6 +695,9 @@
/* Maximum number of supported ports */
#define MVPP2_MAX_PORTS 4
+/* Loopback port index */
+#define MVPP2_LOOPBACK_PORT_INDEX 3
+
/* Maximum number of TXQs used by single port */
#define MVPP2_MAX_TXQ 8
@@ -729,22 +732,21 @@
#define MVPP2_TX_DESC_ALIGN (MVPP2_DESC_ALIGNED_SIZE - 1)
/* RX FIFO constants */
+#define MVPP2_RX_FIFO_PORT_DATA_SIZE_44KB 0xb000
#define MVPP2_RX_FIFO_PORT_DATA_SIZE_32KB 0x8000
#define MVPP2_RX_FIFO_PORT_DATA_SIZE_8KB 0x2000
#define MVPP2_RX_FIFO_PORT_DATA_SIZE_4KB 0x1000
-#define MVPP2_RX_FIFO_PORT_ATTR_SIZE_32KB 0x200
-#define MVPP2_RX_FIFO_PORT_ATTR_SIZE_8KB 0x80
+#define MVPP2_RX_FIFO_PORT_ATTR_SIZE(data_size) ((data_size) >> 6)
#define MVPP2_RX_FIFO_PORT_ATTR_SIZE_4KB 0x40
#define MVPP2_RX_FIFO_PORT_MIN_PKT 0x80
/* TX FIFO constants */
-#define MVPP22_TX_FIFO_DATA_SIZE_10KB 0xa
-#define MVPP22_TX_FIFO_DATA_SIZE_3KB 0x3
-#define MVPP2_TX_FIFO_THRESHOLD_MIN 256
-#define MVPP2_TX_FIFO_THRESHOLD_10KB \
- (MVPP22_TX_FIFO_DATA_SIZE_10KB * 1024 - MVPP2_TX_FIFO_THRESHOLD_MIN)
-#define MVPP2_TX_FIFO_THRESHOLD_3KB \
- (MVPP22_TX_FIFO_DATA_SIZE_3KB * 1024 - MVPP2_TX_FIFO_THRESHOLD_MIN)
+#define MVPP22_TX_FIFO_DATA_SIZE_16KB 16
+#define MVPP22_TX_FIFO_DATA_SIZE_10KB 10
+#define MVPP22_TX_FIFO_DATA_SIZE_3KB 3
+#define MVPP2_TX_FIFO_THRESHOLD_MIN 256 /* Bytes */
+#define MVPP2_TX_FIFO_THRESHOLD(kb) \
+ ((kb) * 1024 - MVPP2_TX_FIFO_THRESHOLD_MIN)
/* RX buffer constants */
#define MVPP2_SKB_SHINFO_SIZE \
@@ -946,6 +948,9 @@ struct mvpp2 {
/* List of pointers to port structures */
int port_count;
struct mvpp2_port *port_list[MVPP2_MAX_PORTS];
+ /* Map of enabled ports */
+ unsigned long port_map;
+
struct mvpp2_tai *tai;
/* Number of Tx threads used */
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index f6616c8..08c237a 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -6601,32 +6601,56 @@ static void mvpp2_rx_fifo_init(struct mvpp2 *priv)
mvpp2_write(priv, MVPP2_RX_FIFO_INIT_REG, 0x1);
}
-static void mvpp22_rx_fifo_init(struct mvpp2 *priv)
+static void mvpp22_rx_fifo_set_hw(struct mvpp2 *priv, int port, int data_size)
{
- int port;
+ int attr_size = MVPP2_RX_FIFO_PORT_ATTR_SIZE(data_size);
- /* The FIFO size parameters are set depending on the maximum speed a
- * given port can handle:
- * - Port 0: 10Gbps
- * - Port 1: 2.5Gbps
- * - Ports 2 and 3: 1Gbps
- */
+ mvpp2_write(priv, MVPP2_RX_DATA_FIFO_SIZE_REG(port), data_size);
+ mvpp2_write(priv, MVPP2_RX_ATTR_FIFO_SIZE_REG(port), attr_size);
+}
- mvpp2_write(priv, MVPP2_RX_DATA_FIFO_SIZE_REG(0),
- MVPP2_RX_FIFO_PORT_DATA_SIZE_32KB);
- mvpp2_write(priv, MVPP2_RX_ATTR_FIFO_SIZE_REG(0),
- MVPP2_RX_FIFO_PORT_ATTR_SIZE_32KB);
+/* Initialize TX FIFO's: the total FIFO size is 48kB on PPv2.2.
+ * 4kB fixed space must be assigned for the loopback port.
+ * Redistribute remaining avialable 44kB space among all active ports.
+ * Guarantee minimum 32kB for 10G port and 8kB for port 1, capable of 2.5G
+ * SGMII link.
+ */
+static void mvpp22_rx_fifo_init(struct mvpp2 *priv)
+{
+ int remaining_ports_count;
+ unsigned long port_map;
+ int size_remainder;
+ int port, size;
+
+ /* The loopback requires fixed 4kB of the FIFO space assignment. */
+ mvpp22_rx_fifo_set_hw(priv, MVPP2_LOOPBACK_PORT_INDEX,
+ MVPP2_RX_FIFO_PORT_DATA_SIZE_4KB);
+ port_map = priv->port_map & ~BIT(MVPP2_LOOPBACK_PORT_INDEX);
+
+ /* Set RX FIFO size to 0 for inactive ports. */
+ for_each_clear_bit(port, &port_map, MVPP2_LOOPBACK_PORT_INDEX)
+ mvpp22_rx_fifo_set_hw(priv, port, 0);
+
+ /* Assign remaining RX FIFO space among all active ports. */
+ size_remainder = MVPP2_RX_FIFO_PORT_DATA_SIZE_44KB;
+ remaining_ports_count = hweight_long(port_map);
+
+ for_each_set_bit(port, &port_map, MVPP2_LOOPBACK_PORT_INDEX) {
+ if (remaining_ports_count == 1)
+ size = size_remainder;
+ else if (port == 0)
+ size = max(size_remainder / remaining_ports_count,
+ MVPP2_RX_FIFO_PORT_DATA_SIZE_32KB);
+ else if (port == 1)
+ size = max(size_remainder / remaining_ports_count,
+ MVPP2_RX_FIFO_PORT_DATA_SIZE_8KB);
+ else
+ size = size_remainder / remaining_ports_count;
- mvpp2_write(priv, MVPP2_RX_DATA_FIFO_SIZE_REG(1),
- MVPP2_RX_FIFO_PORT_DATA_SIZE_8KB);
- mvpp2_write(priv, MVPP2_RX_ATTR_FIFO_SIZE_REG(1),
- MVPP2_RX_FIFO_PORT_ATTR_SIZE_8KB);
+ size_remainder -= size;
+ remaining_ports_count--;
- for (port = 2; port < MVPP2_MAX_PORTS; port++) {
- mvpp2_write(priv, MVPP2_RX_DATA_FIFO_SIZE_REG(port),
- MVPP2_RX_FIFO_PORT_DATA_SIZE_4KB);
- mvpp2_write(priv, MVPP2_RX_ATTR_FIFO_SIZE_REG(port),
- MVPP2_RX_FIFO_PORT_ATTR_SIZE_4KB);
+ mvpp22_rx_fifo_set_hw(priv, port, size);
}
mvpp2_write(priv, MVPP2_RX_MIN_PKT_SIZE_REG,
@@ -6634,24 +6658,53 @@ static void mvpp22_rx_fifo_init(struct mvpp2 *priv)
mvpp2_write(priv, MVPP2_RX_FIFO_INIT_REG, 0x1);
}
-/* Initialize Tx FIFO's: the total FIFO size is 19kB on PPv2.2 and 10G
- * interfaces must have a Tx FIFO size of 10kB. As only port 0 can do 10G,
- * configure its Tx FIFO size to 10kB and the others ports Tx FIFO size to 3kB.
+static void mvpp22_tx_fifo_set_hw(struct mvpp2 *priv, int port, int size)
+{
+ int threshold = MVPP2_TX_FIFO_THRESHOLD(size);
+
+ mvpp2_write(priv, MVPP22_TX_FIFO_SIZE_REG(port), size);
+ mvpp2_write(priv, MVPP22_TX_FIFO_THRESH_REG(port), threshold);
+}
+
+/* Initialize TX FIFO's: the total FIFO size is 19kB on PPv2.2.
+ * 3kB fixed space must be assigned for the loopback port.
+ * Redistribute remaining avialable 16kB space among all active ports.
+ * The 10G interface should use 10kB (which is maximum possible size
+ * per single port).
*/
static void mvpp22_tx_fifo_init(struct mvpp2 *priv)
{
- int port, size, thrs;
-
- for (port = 0; port < MVPP2_MAX_PORTS; port++) {
- if (port == 0) {
+ int remaining_ports_count;
+ unsigned long port_map;
+ int size_remainder;
+ int port, size;
+
+ /* The loopback requires fixed 3kB of the FIFO space assignment. */
+ mvpp22_tx_fifo_set_hw(priv, MVPP2_LOOPBACK_PORT_INDEX,
+ MVPP22_TX_FIFO_DATA_SIZE_3KB);
+ port_map = priv->port_map & ~BIT(MVPP2_LOOPBACK_PORT_INDEX);
+
+ /* Set TX FIFO size to 0 for inactive ports. */
+ for_each_clear_bit(port, &port_map, MVPP2_LOOPBACK_PORT_INDEX)
+ mvpp22_tx_fifo_set_hw(priv, port, 0);
+
+ /* Assign remaining TX FIFO space among all active ports. */
+ size_remainder = MVPP22_TX_FIFO_DATA_SIZE_16KB;
+ remaining_ports_count = hweight_long(port_map);
+
+ for_each_set_bit(port, &port_map, MVPP2_LOOPBACK_PORT_INDEX) {
+ if (remaining_ports_count == 1)
+ size = min(size_remainder,
+ MVPP22_TX_FIFO_DATA_SIZE_10KB);
+ else if (port == 0)
size = MVPP22_TX_FIFO_DATA_SIZE_10KB;
- thrs = MVPP2_TX_FIFO_THRESHOLD_10KB;
- } else {
- size = MVPP22_TX_FIFO_DATA_SIZE_3KB;
- thrs = MVPP2_TX_FIFO_THRESHOLD_3KB;
- }
- mvpp2_write(priv, MVPP22_TX_FIFO_SIZE_REG(port), size);
- mvpp2_write(priv, MVPP22_TX_FIFO_THRESH_REG(port), thrs);
+ else
+ size = size_remainder / remaining_ports_count;
+
+ size_remainder -= size;
+ remaining_ports_count--;
+
+ mvpp22_tx_fifo_set_hw(priv, port, size);
}
}
@@ -6952,6 +7005,12 @@ static int mvpp2_probe(struct platform_device *pdev)
goto err_axi_clk;
}
+ /* Map DTS-active ports. Should be done before FIFO mvpp2_init */
+ fwnode_for_each_available_child_node(fwnode, port_fwnode) {
+ if (!fwnode_property_read_u32(port_fwnode, "port-id", &i))
+ priv->port_map |= BIT(i);
+ }
+
/* Initialize network controller */
err = mvpp2_init(pdev, priv);
if (err < 0) {
--
1.9.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox