From 9513167e6c3343f4ec8e04eb89e9b130eb90e58a Mon Sep 17 00:00:00 2001 From: Antoine Tenart Date: Tue, 26 May 2020 18:22:53 +0200 Subject: [PATCH 1/4] net: phy: mscc-miim: use more reasonable delays The MSCC MIIM MDIO driver uses delays to read poll a status register. I made multiple tests on a Ocelot PCS120 platform which led me to reduce those delays. The delay in between which the polling function is allowed to sleep is reduced from 100us to 50us which in almost all cases is a good value to succeed at the first retry. The overall delay is also lowered as the prior value was really way to high, 10000us is large enough. Signed-off-by: Antoine Tenart Reviewed-by: Alexandre Belloni Reviewed-by: Florian Fainelli Signed-off-by: David S. Miller --- drivers/net/phy/mdio-mscc-miim.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/phy/mdio-mscc-miim.c b/drivers/net/phy/mdio-mscc-miim.c index badbc99bedd3..0b7544f593fb 100644 --- a/drivers/net/phy/mdio-mscc-miim.c +++ b/drivers/net/phy/mdio-mscc-miim.c @@ -44,7 +44,7 @@ static int mscc_miim_wait_ready(struct mii_bus *bus) u32 val; readl_poll_timeout(miim->regs + MSCC_MIIM_REG_STATUS, val, - !(val & MSCC_MIIM_STATUS_STAT_BUSY), 100, 250000); + !(val & MSCC_MIIM_STATUS_STAT_BUSY), 50, 10000); if (val & MSCC_MIIM_STATUS_STAT_BUSY) return -ETIMEDOUT; From f5112c8ae22f8d5796b10d9f5db0014b3546dd00 Mon Sep 17 00:00:00 2001 From: Antoine Tenart Date: Tue, 26 May 2020 18:22:54 +0200 Subject: [PATCH 2/4] net: phy: mscc-miim: remove redundant timeout check readl_poll_timeout already returns -ETIMEDOUT if the condition isn't satisfied, there's no need to check again the condition after calling it. Remove the redundant timeout check. Signed-off-by: Antoine Tenart Reviewed-by: Alexandre Belloni Reviewed-by: Florian Fainelli Signed-off-by: David S. Miller --- drivers/net/phy/mdio-mscc-miim.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/net/phy/mdio-mscc-miim.c b/drivers/net/phy/mdio-mscc-miim.c index 0b7544f593fb..42119f661452 100644 --- a/drivers/net/phy/mdio-mscc-miim.c +++ b/drivers/net/phy/mdio-mscc-miim.c @@ -43,12 +43,8 @@ static int mscc_miim_wait_ready(struct mii_bus *bus) struct mscc_miim_dev *miim = bus->priv; u32 val; - readl_poll_timeout(miim->regs + MSCC_MIIM_REG_STATUS, val, - !(val & MSCC_MIIM_STATUS_STAT_BUSY), 50, 10000); - if (val & MSCC_MIIM_STATUS_STAT_BUSY) - return -ETIMEDOUT; - - return 0; + return readl_poll_timeout(miim->regs + MSCC_MIIM_REG_STATUS, val, + !(val & MSCC_MIIM_STATUS_STAT_BUSY), 50, 10000); } static int mscc_miim_read(struct mii_bus *bus, int mii_id, int regnum) From d9c6de35e051c17474ec8a1fe2fdb8cd2b6f1a87 Mon Sep 17 00:00:00 2001 From: Antoine Tenart Date: Tue, 26 May 2020 18:22:55 +0200 Subject: [PATCH 3/4] net: phy: mscc-miim: improve waiting logic The MSCC MIIM MDIO driver uses a waiting logic to wait for the MDIO bus to be ready to accept next commands. It does so by polling the BUSY status bit which indicates the MDIO bus has completed all pending operations. This can take time, and the controller supports writing the next command as soon as there are no pending commands (which happens while the MDIO bus is busy completing its current command). This patch implements this improved logic by adding an helper to poll the PENDING status bit, and by adjusting where we should wait for the bus to not be busy or to not be pending. Signed-off-by: Antoine Tenart Reviewed-by: Alexandre Belloni Reviewed-by: Florian Fainelli Signed-off-by: David S. Miller --- drivers/net/phy/mdio-mscc-miim.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/net/phy/mdio-mscc-miim.c b/drivers/net/phy/mdio-mscc-miim.c index 42119f661452..aed9afa1e8f1 100644 --- a/drivers/net/phy/mdio-mscc-miim.c +++ b/drivers/net/phy/mdio-mscc-miim.c @@ -16,6 +16,7 @@ #include #define MSCC_MIIM_REG_STATUS 0x0 +#define MSCC_MIIM_STATUS_STAT_PENDING BIT(2) #define MSCC_MIIM_STATUS_STAT_BUSY BIT(3) #define MSCC_MIIM_REG_CMD 0x8 #define MSCC_MIIM_CMD_OPR_WRITE BIT(1) @@ -47,13 +48,23 @@ static int mscc_miim_wait_ready(struct mii_bus *bus) !(val & MSCC_MIIM_STATUS_STAT_BUSY), 50, 10000); } +static int mscc_miim_wait_pending(struct mii_bus *bus) +{ + struct mscc_miim_dev *miim = bus->priv; + u32 val; + + return readl_poll_timeout(miim->regs + MSCC_MIIM_REG_STATUS, val, + !(val & MSCC_MIIM_STATUS_STAT_PENDING), + 50, 10000); +} + static int mscc_miim_read(struct mii_bus *bus, int mii_id, int regnum) { struct mscc_miim_dev *miim = bus->priv; u32 val; int ret; - ret = mscc_miim_wait_ready(bus); + ret = mscc_miim_wait_pending(bus); if (ret) goto out; @@ -82,7 +93,7 @@ static int mscc_miim_write(struct mii_bus *bus, int mii_id, struct mscc_miim_dev *miim = bus->priv; int ret; - ret = mscc_miim_wait_ready(bus); + ret = mscc_miim_wait_pending(bus); if (ret < 0) goto out; From a021ada2b7a3a79394ed2f476ec7615a184bb488 Mon Sep 17 00:00:00 2001 From: Antoine Tenart Date: Tue, 26 May 2020 18:22:56 +0200 Subject: [PATCH 4/4] net: phy: mscc-miim: read poll when high resolution timers are disabled The driver uses a read polling mechanism to check the status of the MDIO bus, to know if it is ready to accept next commands. This polling mechanism uses usleep_delay() under the hood between reads which is fine as long as high resolution timers are enabled. Otherwise the delays will end up to be much longer than expected. This patch fixes this by using udelay() under the hood when CONFIG_HIGH_RES_TIMERS isn't enabled. This increases CPU usage. Signed-off-by: Antoine Tenart Reviewed-by: Florian Fainelli Signed-off-by: David S. Miller --- drivers/net/phy/Kconfig | 3 ++- drivers/net/phy/mdio-mscc-miim.c | 22 +++++++++++++++++----- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index 2a32f26ead0b..047c27087b10 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -184,7 +184,8 @@ config MDIO_MSCC_MIIM depends on HAS_IOMEM help This driver supports the MIIM (MDIO) interface found in the network - switches of the Microsemi SoCs + switches of the Microsemi SoCs; it is recommended to switch on + CONFIG_HIGH_RES_TIMERS config MDIO_MVUSB tristate "Marvell USB to MDIO Adapter" diff --git a/drivers/net/phy/mdio-mscc-miim.c b/drivers/net/phy/mdio-mscc-miim.c index aed9afa1e8f1..11f583fd4611 100644 --- a/drivers/net/phy/mdio-mscc-miim.c +++ b/drivers/net/phy/mdio-mscc-miim.c @@ -39,13 +39,25 @@ struct mscc_miim_dev { void __iomem *phy_regs; }; +/* When high resolution timers aren't built-in: we can't use usleep_range() as + * we would sleep way too long. Use udelay() instead. + */ +#define mscc_readl_poll_timeout(addr, val, cond, delay_us, timeout_us) \ +({ \ + if (!IS_ENABLED(CONFIG_HIGH_RES_TIMERS)) \ + readl_poll_timeout_atomic(addr, val, cond, delay_us, \ + timeout_us); \ + readl_poll_timeout(addr, val, cond, delay_us, timeout_us); \ +}) + static int mscc_miim_wait_ready(struct mii_bus *bus) { struct mscc_miim_dev *miim = bus->priv; u32 val; - return readl_poll_timeout(miim->regs + MSCC_MIIM_REG_STATUS, val, - !(val & MSCC_MIIM_STATUS_STAT_BUSY), 50, 10000); + return mscc_readl_poll_timeout(miim->regs + MSCC_MIIM_REG_STATUS, val, + !(val & MSCC_MIIM_STATUS_STAT_BUSY), 50, + 10000); } static int mscc_miim_wait_pending(struct mii_bus *bus) @@ -53,9 +65,9 @@ static int mscc_miim_wait_pending(struct mii_bus *bus) struct mscc_miim_dev *miim = bus->priv; u32 val; - return readl_poll_timeout(miim->regs + MSCC_MIIM_REG_STATUS, val, - !(val & MSCC_MIIM_STATUS_STAT_PENDING), - 50, 10000); + return mscc_readl_poll_timeout(miim->regs + MSCC_MIIM_REG_STATUS, val, + !(val & MSCC_MIIM_STATUS_STAT_PENDING), + 50, 10000); } static int mscc_miim_read(struct mii_bus *bus, int mii_id, int regnum)