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;
 }
 

Quoted 3 lines · Charles Keepax

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

Quoted 4 lines

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

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

For my own reference:

Acked-for-MFD-by Lee Jones <[email protected]>

Quoted 10 lines · Lee Jones

Quoted 7 lines · Charles Keepax

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(-)

Quoted 3 lines

For my own reference:

Acked-for-MFD-by Lee Jones <[email protected]>

Apologies but I am not sure do you want me to add these tags to new versions of the patches? I would with a regular Acked-by, but didn't with your last version of these.

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 | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

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

index 4cb34c3..ae4cdc4 100644
@@ -235,14 +235,18 @@ static irqreturn_t arizona_overclocked(int irq, void *data)
 	return IRQ_HANDLED;
 }

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

 	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",

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

 	}
 
 	dev_err(arizona->dev, "Polling reg 0x%x timed out: %x\n", reg, val);

@@ -269,7 +274,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 +344,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 +408,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,

Quoted 17 lines · Charles Keepax

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 | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

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

index 4cb34c3..ae4cdc4 100644
@@ -235,14 +235,18 @@ static irqreturn_t arizona_overclocked(int irq, void *data)
 	return IRQ_HANDLED;
 }

drivers/mfd/arizona-core.c static irqreturn_t arizona_overclocked(int irq, void *data) · line 245

Quoted 10 lines

drivers/mfd/arizona-core.c +5 -1

 
+#define ARIZONA_REG_POLL_DELAY_MS 5
+
 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;

Why the over-complication?

Shouldn't this just be "timeout_ms / ARIZONA_REG_POLL_DELAY_MS"?

Quoted 13 lines

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

 	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 +257,8 @@ static int arizona_poll_reg(struct arizona *arizona,
 		if ((val & mask) == target)
 			return 0;
 

drivers/mfd/arizona-core.c static int arizona_poll_reg(struct arizona *arizona, · line 261

Quoted 3 lines

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

-		usleep_range(1000, 5000);
+		usleep_range((ARIZONA_REG_POLL_DELAY_MS * 1000) / 2,
+			     ARIZONA_REG_POLL_DELAY_MS * 1000);

I'm sure there is a macro for conversion from ms to us.

By using such a wide range, you are now not honouring the timeout set by the caller by as much as 50%.

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 | 37 ++++++++++++++-----------------------
 1 file changed, 14 insertions(+), 23 deletions(-)

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

index ae4cdc4..75488e6 100644
@@ -235,34 +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 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_MS * 1000) / 2,
-			     ARIZONA_REG_POLL_DELAY_MS * 1000);
-	}
+	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)

@@ -274,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)

@@ -344,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)

@@ -408,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 | 37 ++++++++++++++-----------------------
 1 file changed, 14 insertions(+), 23 deletions(-)

Roll this up into 3/4.

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);