| Message ID | 20231201173545.1215940-3-tobias@waldekranz.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: mvmdio: Performance related improvements | expand |
On Fri, 1 Dec 2023 18:35:44 +0100 Tobias Waldekranz wrote: > @@ -94,23 +88,24 @@ static int orion_mdio_wait_ready(const struct orion_mdio_ops *ops, > struct mii_bus *bus) > { > struct orion_mdio_dev *dev = bus->priv; > - unsigned long timeout = usecs_to_jiffies(MVMDIO_SMI_TIMEOUT); > - unsigned long end = jiffies + timeout; > - int timedout = 0; > + unsigned long end, timeout; > + int done, timedout; > > - while (1) { > - if (ops->is_done(dev)) > + if (dev->err_interrupt <= 0) { > + if (!read_poll_timeout_atomic(ops->is_done, done, done, 2, > + MVMDIO_SMI_TIMEOUT, false, dev)) > return 0; > - else if (timedout) > - break; > - > - if (dev->err_interrupt <= 0) { > - usleep_range(ops->poll_interval_min, > - ops->poll_interval_max); > + } else { > + timeout = usecs_to_jiffies(MVMDIO_SMI_TIMEOUT); > + end = jiffies + timeout; > + timedout = 0; > + > + while (1) { > + if (ops->is_done(dev)) > + return 0; > + else if (timedout) > + break; > > - if (time_is_before_jiffies(end)) > - ++timedout; > - } else { > /* wait_event_timeout does not guarantee a delay of at > * least one whole jiffie, so timeout must be no less > * than two. drivers/net/ethernet/marvell/mvmdio.c:91:16: warning: variable 'end' set but not used [-Wunused-but-set-variable] 91 | unsigned long end, timeout; | ^
On lör, dec 02, 2023 at 12:45, Jakub Kicinski <kuba@kernel.org> wrote: > On Fri, 1 Dec 2023 18:35:44 +0100 Tobias Waldekranz wrote: >> @@ -94,23 +88,24 @@ static int orion_mdio_wait_ready(const struct orion_mdio_ops *ops, >> struct mii_bus *bus) >> { >> struct orion_mdio_dev *dev = bus->priv; >> - unsigned long timeout = usecs_to_jiffies(MVMDIO_SMI_TIMEOUT); >> - unsigned long end = jiffies + timeout; >> - int timedout = 0; >> + unsigned long end, timeout; >> + int done, timedout; >> >> - while (1) { >> - if (ops->is_done(dev)) >> + if (dev->err_interrupt <= 0) { >> + if (!read_poll_timeout_atomic(ops->is_done, done, done, 2, >> + MVMDIO_SMI_TIMEOUT, false, dev)) >> return 0; >> - else if (timedout) >> - break; >> - >> - if (dev->err_interrupt <= 0) { >> - usleep_range(ops->poll_interval_min, >> - ops->poll_interval_max); >> + } else { >> + timeout = usecs_to_jiffies(MVMDIO_SMI_TIMEOUT); >> + end = jiffies + timeout; >> + timedout = 0; >> + >> + while (1) { >> + if (ops->is_done(dev)) >> + return 0; >> + else if (timedout) >> + break; >> >> - if (time_is_before_jiffies(end)) >> - ++timedout; >> - } else { >> /* wait_event_timeout does not guarantee a delay of at >> * least one whole jiffie, so timeout must be no less >> * than two. > > drivers/net/ethernet/marvell/mvmdio.c:91:16: warning: variable 'end' set but not used [-Wunused-but-set-variable] > 91 | unsigned long end, timeout; > | ^ Rookie mistake, sorry about that. Looking at it again, I think I was too scared of touching the original interrupt path, as I have no means of testing it (no hardware). I will try to simplify this in v2, and hope that someone else can test it.
> Looking at it again, I think I was too scared of touching the original > interrupt path, as I have no means of testing it (no hardware). I will > try to simplify this in v2, and hope that someone else can test it. I have a couple of kirkwood machines which have the interrupt. So i can test this. Andrew
diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c index 89f26402f8fb..1de2175269bf 100644 --- a/drivers/net/ethernet/marvell/mvmdio.c +++ b/drivers/net/ethernet/marvell/mvmdio.c @@ -23,6 +23,7 @@ #include <linux/delay.h> #include <linux/interrupt.h> #include <linux/io.h> +#include <linux/iopoll.h> #include <linux/kernel.h> #include <linux/mod_devicetable.h> #include <linux/module.h> @@ -58,11 +59,6 @@ * - Armada 370 (Globalscale Mirabox): 41us to 43us (Polled) */ #define MVMDIO_SMI_TIMEOUT 1000 /* 1000us = 1ms */ -#define MVMDIO_SMI_POLL_INTERVAL_MIN 45 -#define MVMDIO_SMI_POLL_INTERVAL_MAX 55 - -#define MVMDIO_XSMI_POLL_INTERVAL_MIN 150 -#define MVMDIO_XSMI_POLL_INTERVAL_MAX 160 struct orion_mdio_dev { void __iomem *regs; @@ -84,8 +80,6 @@ enum orion_mdio_bus_type { struct orion_mdio_ops { int (*is_done)(struct orion_mdio_dev *); - unsigned int poll_interval_min; - unsigned int poll_interval_max; }; /* Wait for the SMI unit to be ready for another operation @@ -94,23 +88,24 @@ static int orion_mdio_wait_ready(const struct orion_mdio_ops *ops, struct mii_bus *bus) { struct orion_mdio_dev *dev = bus->priv; - unsigned long timeout = usecs_to_jiffies(MVMDIO_SMI_TIMEOUT); - unsigned long end = jiffies + timeout; - int timedout = 0; + unsigned long end, timeout; + int done, timedout; - while (1) { - if (ops->is_done(dev)) + if (dev->err_interrupt <= 0) { + if (!read_poll_timeout_atomic(ops->is_done, done, done, 2, + MVMDIO_SMI_TIMEOUT, false, dev)) return 0; - else if (timedout) - break; - - if (dev->err_interrupt <= 0) { - usleep_range(ops->poll_interval_min, - ops->poll_interval_max); + } else { + timeout = usecs_to_jiffies(MVMDIO_SMI_TIMEOUT); + end = jiffies + timeout; + timedout = 0; + + while (1) { + if (ops->is_done(dev)) + return 0; + else if (timedout) + break; - if (time_is_before_jiffies(end)) - ++timedout; - } else { /* wait_event_timeout does not guarantee a delay of at * least one whole jiffie, so timeout must be no less * than two. @@ -135,8 +130,6 @@ static int orion_mdio_smi_is_done(struct orion_mdio_dev *dev) static const struct orion_mdio_ops orion_mdio_smi_ops = { .is_done = orion_mdio_smi_is_done, - .poll_interval_min = MVMDIO_SMI_POLL_INTERVAL_MIN, - .poll_interval_max = MVMDIO_SMI_POLL_INTERVAL_MAX, }; static int orion_mdio_smi_read(struct mii_bus *bus, int mii_id, @@ -194,8 +187,6 @@ static int orion_mdio_xsmi_is_done(struct orion_mdio_dev *dev) static const struct orion_mdio_ops orion_mdio_xsmi_ops = { .is_done = orion_mdio_xsmi_is_done, - .poll_interval_min = MVMDIO_XSMI_POLL_INTERVAL_MIN, - .poll_interval_max = MVMDIO_XSMI_POLL_INTERVAL_MAX, }; static int orion_mdio_xsmi_read_c45(struct mii_bus *bus, int mii_id,
Before this change, when operating in polled mode, i.e. no IRQ is available, every individual C45 access would be hit with a 150us sleep after the bus access. For example, on a board with a CN9130 SoC connected to an MV88X3310 PHY, a single C45 read would take around 165us: root@infix:~$ mdio f212a600.mdio-mii mmd 4:1 bench 0xc003 Performed 1000 reads in 165ms By replacing the long sleep with a tighter poll loop, we observe a 10x increase in bus throughput: root@infix:~$ mdio f212a600.mdio-mii mmd 4:1 bench 0xc003 Performed 1000 reads in 15ms Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> --- drivers/net/ethernet/marvell/mvmdio.c | 41 +++++++++++---------------- 1 file changed, 16 insertions(+), 25 deletions(-)