diff mbox series

[RFC,net-next,v2,02/17] net: mdio: ipq8064: switch to write/readl function

Message ID 20210502230710.30676-2-ansuelsmth@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [RFC,net-next,v2,01/17] net: mdio: ipq8064: clean whitespaces in define | expand

Checks

Context Check Description
netdev/apply fail Patch does not apply to net-next
netdev/tree_selection success Clearly marked for net-next

Commit Message

Christian Marangi May 2, 2021, 11:06 p.m. UTC 
Use readl/writel function instead of regmap function to make sure no
value is cached and align to other similar mdio driver.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/net/mdio/mdio-ipq8064.c | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

Comments

Andrew Lunn May 3, 2021, 12:49 p.m. UTC | #1 
On Mon, May 03, 2021 at 01:06:54AM +0200, Ansuel Smith wrote:
> Use readl/writel function instead of regmap function to make sure no
> value is cached and align to other similar mdio driver.

regmap is O.K. to use, so long as you tell it not to cache. Look at
how to use volatile in regmap.

You might be able to follow what lan9303_mdio.c is doing.

    Andrew
Christian Marangi May 3, 2021, 2:24 p.m. UTC | #2 
On Mon, May 03, 2021 at 02:49:34PM +0200, Andrew Lunn wrote:
> On Mon, May 03, 2021 at 01:06:54AM +0200, Ansuel Smith wrote:
> > Use readl/writel function instead of regmap function to make sure no
> > value is cached and align to other similar mdio driver.
> 
> regmap is O.K. to use, so long as you tell it not to cache. Look at
> how to use volatile in regmap.
> 
> You might be able to follow what lan9303_mdio.c is doing.
> 
>     Andrew

Was thinking more about the overhead of using regmap instead of plain
writel. Or is it minimal?
Andrew Lunn May 3, 2021, 2:44 p.m. UTC | #3 
On Mon, May 03, 2021 at 04:24:15PM +0200, Ansuel Smith wrote:
> On Mon, May 03, 2021 at 02:49:34PM +0200, Andrew Lunn wrote:
> > On Mon, May 03, 2021 at 01:06:54AM +0200, Ansuel Smith wrote:
> > > Use readl/writel function instead of regmap function to make sure no
> > > value is cached and align to other similar mdio driver.
> > 
> > regmap is O.K. to use, so long as you tell it not to cache. Look at
> > how to use volatile in regmap.
> > 
> > You might be able to follow what lan9303_mdio.c is doing.
> > 
> >     Andrew
> 
> Was thinking more about the overhead of using regmap instead of plain
> writel. Or is it minimal?

It is likely other parts of the system are using regmap. So it will
not be adding much overhead.

    Andrew
diff mbox series

Patch

diff --git a/drivers/net/mdio/mdio-ipq8064.c b/drivers/net/mdio/mdio-ipq8064.c
index fb1614242e13..8ae5379eda9d 100644
--- a/drivers/net/mdio/mdio-ipq8064.c
+++ b/drivers/net/mdio/mdio-ipq8064.c
@@ -37,7 +37,7 @@ 
 #define MII_MDIO_RETRY_MSEC			(10)
 
 struct ipq8064_mdio {
-	struct regmap *base; /* NSS_GMAC0_BASE */
+	void __iomem *base; /* NSS_GMAC0_BASE */
 };
 
 static int
@@ -45,9 +45,9 @@  ipq8064_mdio_wait_busy(struct ipq8064_mdio *priv)
 {
 	u32 busy;
 
-	return regmap_read_poll_timeout(priv->base, MII_ADDR_REG_ADDR, busy,
-					!(busy & MII_BUSY), MII_MDIO_DELAY_USEC,
-					MII_MDIO_RETRY_MSEC * USEC_PER_MSEC);
+	return readl_poll_timeout(priv->base + MII_ADDR_REG_ADDR, busy,
+				  !(busy & MII_BUSY), MII_MDIO_DELAY_USEC,
+				  MII_MDIO_RETRY_MSEC * USEC_PER_MSEC);
 }
 
 static int
@@ -55,7 +55,6 @@  ipq8064_mdio_read(struct mii_bus *bus, int phy_addr, int reg_offset)
 {
 	u32 miiaddr = MII_BUSY | MII_CLKRANGE_250_300M;
 	struct ipq8064_mdio *priv = bus->priv;
-	u32 ret_val;
 	int err;
 
 	/* Reject clause 45 */
@@ -65,15 +64,14 @@  ipq8064_mdio_read(struct mii_bus *bus, int phy_addr, int reg_offset)
 	miiaddr |= ((phy_addr << MII_ADDR_SHIFT) & MII_ADDR_MASK) |
 		   ((reg_offset << MII_REG_SHIFT) & MII_REG_MASK);
 
-	regmap_write(priv->base, MII_ADDR_REG_ADDR, miiaddr);
+	writel(miiaddr, priv->base + MII_ADDR_REG_ADDR);
 	usleep_range(8, 10);
 
 	err = ipq8064_mdio_wait_busy(priv);
 	if (err)
 		return err;
 
-	regmap_read(priv->base, MII_DATA_REG_ADDR, &ret_val);
-	return (int)ret_val;
+	return (int)readl(priv->base + MII_DATA_REG_ADDR);
 }
 
 static int
@@ -86,12 +84,12 @@  ipq8064_mdio_write(struct mii_bus *bus, int phy_addr, int reg_offset, u16 data)
 	if (reg_offset & MII_ADDR_C45)
 		return -EOPNOTSUPP;
 
-	regmap_write(priv->base, MII_DATA_REG_ADDR, data);
+	writel(data, priv->base + MII_DATA_REG_ADDR);
 
 	miiaddr |= ((phy_addr << MII_ADDR_SHIFT) & MII_ADDR_MASK) |
 		   ((reg_offset << MII_REG_SHIFT) & MII_REG_MASK);
 
-	regmap_write(priv->base, MII_ADDR_REG_ADDR, miiaddr);
+	writel(miiaddr, priv->base + MII_ADDR_REG_ADDR);
 	usleep_range(8, 10);
 
 	return ipq8064_mdio_wait_busy(priv);
@@ -116,15 +114,9 @@  ipq8064_mdio_probe(struct platform_device *pdev)
 	bus->parent = &pdev->dev;
 
 	priv = bus->priv;
-	priv->base = device_node_to_regmap(np);
-	if (IS_ERR(priv->base)) {
-		if (priv->base == ERR_PTR(-EPROBE_DEFER))
-			return -EPROBE_DEFER;
-
-		dev_err(&pdev->dev, "error getting device regmap, error=%pe\n",
-			priv->base);
+	priv->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(priv->base))
 		return PTR_ERR(priv->base);
-	}
 
 	ret = of_mdiobus_register(bus, np);
 	if (ret)