[net-next] net: ethernet: ti: davinci_mdio: switch to readl/writel_relaxed()

Commit Message

Grygorii Strashko March 28, 2019, 5:09 p.m. UTC

Switch to readl/writel_relaxed() APIs, because this is recommended
API and the MDIO block is reused on Keystone 2 SoCs
where LE/BE modes are supported.

Signed-off-by: Grygorii Strashko <[e-mail]>
---
 drivers/net/ethernet/ti/davinci_mdio.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

Comments

Arnd Bergmann March 29, 2019, 1 a.m. UTC | #1

On Fri, Mar 29, 2019 at 1:10 AM Grygorii Strashko
<[e-mail]> wrote:
>
> Switch to readl/writel_relaxed() APIs, because this is recommended
> API and the MDIO block is reused on Keystone 2 SoCs
> where LE/BE modes are supported.
>
> Signed-off-by: Grygorii Strashko <[e-mail]>

In general, my recommendation would be to use readl/writel instead of
readl_relaxed/writel_relaxed everywhere, because they have much
more predictible behavior. If you want to use the _relaxed version
in the fast-path for data traffic, that is usually fine, but then I would
add a comment next to it to explain why it is safe there without extra
barriers to serialize with spinlocks and DMA.

Changing away from __raw_*() to enable big-endian mode is of
course a good idea as well.

      Arnd

Grygorii Strashko March 29, 2019, 9:11 a.m. UTC | #2

Hi Arnd,

On 29.03.19 03:00, Arnd Bergmann wrote:
> On Fri, Mar 29, 2019 at 1:10 AM Grygorii Strashko
> <[e-mail]> wrote:
>>
>> Switch to readl/writel_relaxed() APIs, because this is recommended
>> API and the MDIO block is reused on Keystone 2 SoCs
>> where LE/BE modes are supported.
>>
>> Signed-off-by: Grygorii Strashko <[e-mail]>
> 
> In general, my recommendation would be to use readl/writel instead of
> readl_relaxed/writel_relaxed everywhere, because they have much
> more predictible behavior. If you want to use the _relaxed version
> in the fast-path for data traffic, that is usually fine, but then I would
> add a comment next to it to explain why it is safe there without extra
> barriers to serialize with spinlocks and DMA.
> 

it's not fast path, so readl/writel should be ok. I'll resend.

Patch

diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index a98aedae1b41..64776b47799b 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -140,7 +140,7 @@  static void davinci_mdio_init_clk(struct davinci_mdio_data *data)
 static void davinci_mdio_enable(struct davinci_mdio_data *data)
 {
 	/* set enable and clock divider */
-	__raw_writel(data->clk_div | CONTROL_ENABLE, &data->regs->control);
+	writel_relaxed(data->clk_div | CONTROL_ENABLE, &data->regs->control);
 }
 
 static int davinci_mdio_reset(struct mii_bus *bus)
@@ -159,7 +159,7 @@  static int davinci_mdio_reset(struct mii_bus *bus)
 	msleep(PHY_MAX_ADDR * data->access_time);
 
 	/* dump hardware version info */
-	ver = __raw_readl(&data->regs->version);
+	ver = readl_relaxed(&data->regs->version);
 	dev_info(data->dev,
 		 "davinci mdio revision %d.%d, bus freq %ld\n",
 		 (ver >> 8) & 0xff, ver & 0xff,
@@ -169,7 +169,7 @@  static int davinci_mdio_reset(struct mii_bus *bus)
 		goto done;
 
 	/* get phy mask from the alive register */
-	phy_mask = __raw_readl(&data->regs->alive);
+	phy_mask = readl_relaxed(&data->regs->alive);
 	if (phy_mask) {
 		/* restrict mdio bus to live phys only */
 		dev_info(data->dev, "detected phy mask %x\n", ~phy_mask);
@@ -196,11 +196,11 @@  static inline int wait_for_user_access(struct davinci_mdio_data *data)
 	u32 reg;
 
 	while (time_after(timeout, jiffies)) {
-		reg = __raw_readl(&regs->user[0].access);
+		reg = readl_relaxed(&regs->user[0].access);
 		if ((reg & USERACCESS_GO) == 0)
 			return 0;
 
-		reg = __raw_readl(&regs->control);
+		reg = readl_relaxed(&regs->control);
 		if ((reg & CONTROL_IDLE) == 0) {
 			usleep_range(100, 200);
 			continue;
@@ -216,7 +216,7 @@  static inline int wait_for_user_access(struct davinci_mdio_data *data)
 		return -EAGAIN;
 	}
 
-	reg = __raw_readl(&regs->user[0].access);
+	reg = readl_relaxed(&regs->user[0].access);
 	if ((reg & USERACCESS_GO) == 0)
 		return 0;
 
@@ -263,7 +263,7 @@  static int davinci_mdio_read(struct mii_bus *bus, int phy_id, int phy_reg)
 		if (ret < 0)
 			break;
 
-		__raw_writel(reg, &data->regs->user[0].access);
+		writel_relaxed(reg, &data->regs->user[0].access);
 
 		ret = wait_for_user_access(data);
 		if (ret == -EAGAIN)
@@ -271,7 +271,7 @@  static int davinci_mdio_read(struct mii_bus *bus, int phy_id, int phy_reg)
 		if (ret < 0)
 			break;
 
-		reg = __raw_readl(&data->regs->user[0].access);
+		reg = readl_relaxed(&data->regs->user[0].access);
 		ret = (reg & USERACCESS_ACK) ? (reg & USERACCESS_DATA) : -EIO;
 		break;
 	}
@@ -307,7 +307,7 @@  static int davinci_mdio_write(struct mii_bus *bus, int phy_id,
 		if (ret < 0)
 			break;
 
-		__raw_writel(reg, &data->regs->user[0].access);
+		writel_relaxed(reg, &data->regs->user[0].access);
 
 		ret = wait_for_user_access(data);
 		if (ret == -EAGAIN)
@@ -472,9 +472,9 @@  static int davinci_mdio_runtime_suspend(struct device *dev)
 	u32 ctrl;
 
 	/* shutdown the scan state machine */
-	ctrl = __raw_readl(&data->regs->control);
+	ctrl = readl_relaxed(&data->regs->control);
 	ctrl &= ~CONTROL_ENABLE;
-	__raw_writel(ctrl, &data->regs->control);
+	writel_relaxed(ctrl, &data->regs->control);
 	wait_for_idle(data);
 
 	return 0;