mfd: arizona: Remove duplicate set of ret variable

Register addresses are normally displayed in hex throughout the Arizona driver. Update the arizona_poll_reg function to follow this convention.

Signed-off-by Charles Keepax <[email protected]>

---
 drivers/mfd/arizona-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/mfd/arizona-core.c +2 -2

index e00f577..4cb34c3 100644
@@ -245,7 +245,7 @@ static int arizona_poll_reg(struct arizona *arizona,
 	for (i = 0; i < timeout; i++) {
 		ret = regmap_read(arizona->regmap, reg, &val);
 		if (ret != 0) {
-			dev_err(arizona->dev, "Failed to read reg %u: %d\n",
+			dev_err(arizona->dev, "Failed to read reg 0x%x: %d\n",
 				reg, ret);
 			continue;
 		}

@@ -256,7 +256,7 @@ static int arizona_poll_reg(struct arizona *arizona,
 		usleep_range(1000, 5000);
 	}
 
-	dev_err(arizona->dev, "Polling reg %u timed out: %x\n", reg, val);
+	dev_err(arizona->dev, "Polling reg 0x%x timed out: %x\n", reg, val);
 	return -ETIMEDOUT;
 }
 

Currently, we specify the timeout in terms of the number of polls but it is more clear from a user of the functions perspective to specify the timeout directly in milliseconds, as such update the function to these new semantics.

Signed-off-by Charles Keepax <[email protected]>

---
 drivers/mfd/arizona-core.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

drivers/mfd/arizona-core.c +12 -6

index 4cb34c3..09d48ed 100644
@@ -235,14 +235,19 @@ static irqreturn_t arizona_overclocked(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+#define ARIZONA_REG_POLL_DELAY_MS 5
+#define ARIZONA_REG_POLL_DELAY_US (ARIZONA_REG_POLL_DELAY_MS * 1000)
+
 static int arizona_poll_reg(struct arizona *arizona,
-			    int timeout, unsigned int reg,
+			    int timeout_ms, unsigned int reg,
 			    unsigned int mask, unsigned int target)
 {
+	unsigned int npolls = (timeout_ms + ARIZONA_REG_POLL_DELAY_MS - 1) /
+			      ARIZONA_REG_POLL_DELAY_MS;
 	unsigned int val = 0;
 	int ret, i;
 
-	for (i = 0; i < timeout; i++) {
+	for (i = 0; i < npolls; i++) {
 		ret = regmap_read(arizona->regmap, reg, &val);
 		if (ret != 0) {
 			dev_err(arizona->dev, "Failed to read reg 0x%x: %d\n",

@@ -253,7 +258,8 @@ static int arizona_poll_reg(struct arizona *arizona,
 		if ((val & mask) == target)
 			return 0;
 
-		usleep_range(1000, 5000);
+		usleep_range(ARIZONA_REG_POLL_DELAY_US,
+			     ARIZONA_REG_POLL_DELAY_US * 2);
 	}
 
 	dev_err(arizona->dev, "Polling reg 0x%x timed out: %x\n", reg, val);

@@ -269,7 +275,7 @@ static int arizona_wait_for_boot(struct arizona *arizona)
 	 * we won't race with the interrupt handler as it'll be blocked on
 	 * runtime resume.
 	 */
-	ret = arizona_poll_reg(arizona, 5, ARIZONA_INTERRUPT_RAW_STATUS_5,
+	ret = arizona_poll_reg(arizona, 25, ARIZONA_INTERRUPT_RAW_STATUS_5,
 			       ARIZONA_BOOT_DONE_STS, ARIZONA_BOOT_DONE_STS);
 
 	if (!ret)

@@ -339,7 +345,7 @@ static int arizona_enable_freerun_sysclk(struct arizona *arizona,
 			ret);
 		return ret;
 	}
-	ret = arizona_poll_reg(arizona, 25, ARIZONA_INTERRUPT_RAW_STATUS_5,
+	ret = arizona_poll_reg(arizona, 125, ARIZONA_INTERRUPT_RAW_STATUS_5,
 			       ARIZONA_FLL1_CLOCK_OK_STS,
 			       ARIZONA_FLL1_CLOCK_OK_STS);
 	if (ret)

@@ -403,7 +409,7 @@ static int wm5102_apply_hardware_patch(struct arizona *arizona)
 		goto err;
 	}
 
-	ret = arizona_poll_reg(arizona, 5, ARIZONA_WRITE_SEQUENCER_CTRL_1,
+	ret = arizona_poll_reg(arizona, 25, ARIZONA_WRITE_SEQUENCER_CTRL_1,
 			       ARIZONA_WSEQ_BUSY, 0);
 	if (ret)
 		regmap_write(arizona->regmap, ARIZONA_WRITE_SEQUENCER_CTRL_0,

arizona_poll_reg essentially hard-codes regmap_read_poll_timeout, this patch updates the implementation to use regmap_read_poll_timeout. We still keep arizona_poll_reg around as regmap_read_poll_timeout is a macro so rather than expand this for each caller keep it wrapped in arizona_poll_reg.

Whilst we are doing this make the timeouts a little more generous as the previous system had a bit more slack as it was done as a delay per iteration of the loop whereas regmap_read_poll_timeout compares ktime's.

Signed-off-by Charles Keepax <[email protected]>

---
 drivers/mfd/arizona-core.c | 38 ++++++++++++++------------------------
 1 file changed, 14 insertions(+), 24 deletions(-)

drivers/mfd/arizona-core.c +14 -24

index 09d48ed..75488e6 100644
@@ -235,35 +235,25 @@ static irqreturn_t arizona_overclocked(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-#define ARIZONA_REG_POLL_DELAY_MS 5
-#define ARIZONA_REG_POLL_DELAY_US (ARIZONA_REG_POLL_DELAY_MS * 1000)
+#define ARIZONA_REG_POLL_DELAY_US 7500
 
 static int arizona_poll_reg(struct arizona *arizona,
 			    int timeout_ms, unsigned int reg,
 			    unsigned int mask, unsigned int target)
 {
-	unsigned int npolls = (timeout_ms + ARIZONA_REG_POLL_DELAY_MS - 1) /
-			      ARIZONA_REG_POLL_DELAY_MS;
 	unsigned int val = 0;
-	int ret, i;
-
-	for (i = 0; i < npolls; i++) {
-		ret = regmap_read(arizona->regmap, reg, &val);
-		if (ret != 0) {
-			dev_err(arizona->dev, "Failed to read reg 0x%x: %d\n",
-				reg, ret);
-			continue;
-		}
-
-		if ((val & mask) == target)
-			return 0;
+	int ret;
 
-		usleep_range(ARIZONA_REG_POLL_DELAY_US,
-			     ARIZONA_REG_POLL_DELAY_US * 2);
-	}
+	ret = regmap_read_poll_timeout(arizona->regmap,
+				       ARIZONA_INTERRUPT_RAW_STATUS_5, val,
+				       ((val & mask) == target),
+				       ARIZONA_REG_POLL_DELAY_US,
+				       timeout_ms * 1000);
+	if (ret)
+		dev_err(arizona->dev, "Polling reg 0x%x timed out: %x\n",
+			reg, val);
 
-	dev_err(arizona->dev, "Polling reg 0x%x timed out: %x\n", reg, val);
-	return -ETIMEDOUT;
+	return ret;
 }
 
 static int arizona_wait_for_boot(struct arizona *arizona)

@@ -275,7 +265,7 @@ static int arizona_wait_for_boot(struct arizona *arizona)
 	 * we won't race with the interrupt handler as it'll be blocked on
 	 * runtime resume.
 	 */
-	ret = arizona_poll_reg(arizona, 25, ARIZONA_INTERRUPT_RAW_STATUS_5,
+	ret = arizona_poll_reg(arizona, 30, ARIZONA_INTERRUPT_RAW_STATUS_5,
 			       ARIZONA_BOOT_DONE_STS, ARIZONA_BOOT_DONE_STS);
 
 	if (!ret)

@@ -345,7 +335,7 @@ static int arizona_enable_freerun_sysclk(struct arizona *arizona,
 			ret);
 		return ret;
 	}
-	ret = arizona_poll_reg(arizona, 125, ARIZONA_INTERRUPT_RAW_STATUS_5,
+	ret = arizona_poll_reg(arizona, 180, ARIZONA_INTERRUPT_RAW_STATUS_5,
 			       ARIZONA_FLL1_CLOCK_OK_STS,
 			       ARIZONA_FLL1_CLOCK_OK_STS);
 	if (ret)

@@ -409,7 +399,7 @@ static int wm5102_apply_hardware_patch(struct arizona *arizona)
 		goto err;
 	}
 
-	ret = arizona_poll_reg(arizona, 25, ARIZONA_WRITE_SEQUENCER_CTRL_1,
+	ret = arizona_poll_reg(arizona, 30, ARIZONA_WRITE_SEQUENCER_CTRL_1,
 			       ARIZONA_WSEQ_BUSY, 0);
 	if (ret)
 		regmap_write(arizona->regmap, ARIZONA_WRITE_SEQUENCER_CTRL_0,

Quoted 10 lines · Charles Keepax

arizona_poll_reg essentially hard-codes regmap_read_poll_timeout, this patch updates the implementation to use regmap_read_poll_timeout. We still keep arizona_poll_reg around as regmap_read_poll_timeout is a macro so rather than expand this for each caller keep it wrapped in arizona_poll_reg.

Whilst we are doing this make the timeouts a little more generous as the previous system had a bit more slack as it was done as a delay per iteration of the loop whereas regmap_read_poll_timeout compares ktime's.

Quoted 4 lines

Signed-off-by Charles Keepax <[email protected]>

---
 drivers/mfd/arizona-core.c | 38 ++++++++++++++------------------------
 1 file changed, 14 insertions(+), 24 deletions(-)

Apart from patch count, is there any technical reason why this patch shouldn't just be rolled into patch 3?

Quoted 17 lines · Lee Jones

Quoted 14 lines · Charles Keepax

arizona_poll_reg essentially hard-codes regmap_read_poll_timeout, this patch updates the implementation to use regmap_read_poll_timeout. We still keep arizona_poll_reg around as regmap_read_poll_timeout is a macro so rather than expand this for each caller keep it wrapped in arizona_poll_reg.

Whilst we are doing this make the timeouts a little more generous as the previous system had a bit more slack as it was done as a delay per iteration of the loop whereas regmap_read_poll_timeout compares ktime's.

Signed-off-by Charles Keepax <[email protected]>

---
 drivers/mfd/arizona-core.c | 38 ++++++++++++++------------------------
 1 file changed, 14 insertions(+), 24 deletions(-)

Quoted 3 lines

Apart from patch count, is there any technical reason why this patch shouldn't just be rolled into patch 3?

I prefer it as two patches as its clearer what happened from the history. One patch changes the interface for the function, the other updates the implementation. Can squash if you feel strongly about it though?

Quoted 22 lines · Charles Keepax

Quoted 20 lines · Lee Jones

Quoted 6 lines · Charles Keepax

arizona_poll_reg essentially hard-codes regmap_read_poll_timeout, this patch updates the implementation to use regmap_read_poll_timeout. We still keep arizona_poll_reg around as regmap_read_poll_timeout is a macro so rather than expand this for each caller keep it wrapped in arizona_poll_reg.

Quoted 8 lines

Whilst we are doing this make the timeouts a little more generous as the previous system had a bit more slack as it was done as a delay per iteration of the loop whereas regmap_read_poll_timeout compares ktime's.

Signed-off-by Charles Keepax <[email protected]>

---
 drivers/mfd/arizona-core.c | 38 ++++++++++++++------------------------
 1 file changed, 14 insertions(+), 24 deletions(-)

Apart from patch count, is there any technical reason why this patch shouldn't just be rolled into patch 3?

Quoted 4 lines

I prefer it as two patches as its clearer what happened from the history. One patch changes the interface for the function, the other updates the implementation. Can squash if you feel strongly about it though?

I don't feel that strongly about it, but to me it looks like patch 4 reworks everything patch 3 did.

arizona_poll_reg already returns ETIMEDOUT if we don't see the expected register changes before the time out, so remove pointless local setting of ETIMEDOUT.

Signed-off-by Charles Keepax <[email protected]>

---
 drivers/mfd/arizona-core.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

drivers/mfd/arizona-core.c +2 -6

index b6d4bc6..e00f577 100644
@@ -342,10 +342,8 @@ static int arizona_enable_freerun_sysclk(struct arizona *arizona,
 	ret = arizona_poll_reg(arizona, 25, ARIZONA_INTERRUPT_RAW_STATUS_5,
 			       ARIZONA_FLL1_CLOCK_OK_STS,
 			       ARIZONA_FLL1_CLOCK_OK_STS);
-	if (ret) {
-		ret = -ETIMEDOUT;
+	if (ret)
 		goto err_fll;
-	}
 
 	ret = regmap_write(arizona->regmap, ARIZONA_SYSTEM_CLOCK_1, 0x0144);
 	if (ret) {

@@ -407,11 +405,9 @@ static int wm5102_apply_hardware_patch(struct arizona *arizona)
 
 	ret = arizona_poll_reg(arizona, 5, ARIZONA_WRITE_SEQUENCER_CTRL_1,
 			       ARIZONA_WSEQ_BUSY, 0);
-	if (ret) {
+	if (ret)
 		regmap_write(arizona->regmap, ARIZONA_WRITE_SEQUENCER_CTRL_0,
 			     ARIZONA_WSEQ_ABORT);
-		ret = -ETIMEDOUT;
-	}
 
 err:
 	err = arizona_disable_freerun_sysclk(arizona, &state);