[PATCH RESEND 1/4] mfd: arizona: Remove duplicate set of ret variable

[PATCH RESEND 1/4] mfd: arizona: Remove duplicate set of ret variable

[Charles Keepax]

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 <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/mfd/arizona-core.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
index b6d4bc6..e00f577 100644
--- a/drivers/mfd/arizona-core.c
+++ b/drivers/mfd/arizona-core.c
@@ -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);
-- 
2.1.4

[PATCH RESEND 2/4] mfd: arizona: Display register addresses in hex

[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 <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/mfd/arizona-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
index e00f577..4cb34c3 100644
--- a/drivers/mfd/arizona-core.c
+++ b/drivers/mfd/arizona-core.c
@@ -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;
 }
 
-- 
2.1.4

[PATCH RESEND 3/4] mfd: arizona: Update arizona_poll_reg to take a timeout in milliseconds

[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 <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/mfd/arizona-core.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
index 4cb34c3..09d48ed 100644
--- a/drivers/mfd/arizona-core.c
+++ b/drivers/mfd/arizona-core.c
@@ -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,
-- 
2.1.4

[PATCH RESEND 4/4] mfd: arizona: Use regmap_read_poll_timeout instead of hard coding it

[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 <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/mfd/arizona-core.c | 38 ++++++++++++++------------------------
 1 file changed, 14 insertions(+), 24 deletions(-)

diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
index 09d48ed..75488e6 100644
--- a/drivers/mfd/arizona-core.c
+++ b/drivers/mfd/arizona-core.c
@@ -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,
-- 
2.1.4

Re: [PATCH RESEND 1/4] mfd: arizona: Remove duplicate set of ret variable

[Lee Jones]

On Thu, 09 Mar 2017, Charles Keepax wrote:

> 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 <ckeepax@opensource.wolfsonmicro.com>
> ---
>  drivers/mfd/arizona-core.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
  
> diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
> index b6d4bc6..e00f577 100644
> --- a/drivers/mfd/arizona-core.c
> +++ b/drivers/mfd/arizona-core.c
> @@ -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);

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH RESEND 2/4] mfd: arizona: Display register addresses in hex

[Lee Jones]

On Thu, 09 Mar 2017, Charles Keepax wrote:

> 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 <ckeepax@opensource.wolfsonmicro.com>
> ---
>  drivers/mfd/arizona-core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
  
> diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
> index e00f577..4cb34c3 100644
> --- a/drivers/mfd/arizona-core.c
> +++ b/drivers/mfd/arizona-core.c
> @@ -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;
>  }
>  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH RESEND 3/4] mfd: arizona: Update arizona_poll_reg to take a timeout in milliseconds

[Lee Jones]

On Thu, 09 Mar 2017, Charles Keepax wrote:

> 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 <ckeepax@opensource.wolfsonmicro.com>
> ---
>  drivers/mfd/arizona-core.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
  
> diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
> index 4cb34c3..09d48ed 100644
> --- a/drivers/mfd/arizona-core.c
> +++ b/drivers/mfd/arizona-core.c
> @@ -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,

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH RESEND 4/4] mfd: arizona: Use regmap_read_poll_timeout instead of hard coding it

[Lee Jones]

On Thu, 09 Mar 2017, Charles Keepax wrote:

> 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 <ckeepax@opensource.wolfsonmicro.com>
> ---
>  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?

> diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
> index 09d48ed..75488e6 100644
> --- a/drivers/mfd/arizona-core.c
> +++ b/drivers/mfd/arizona-core.c
> @@ -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,

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH RESEND 4/4] mfd: arizona: Use regmap_read_poll_timeout instead of hard coding it

[Charles Keepax]

On Tue, Mar 14, 2017 at 05:07:04PM +0000, Lee Jones wrote:
> On Thu, 09 Mar 2017, Charles Keepax wrote:
> 
> > 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 <ckeepax@opensource.wolfsonmicro.com>
> > ---
> >  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?
> 

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?

Thanks,
Charles

Re: [PATCH RESEND 4/4] mfd: arizona: Use regmap_read_poll_timeout instead of hard coding it

[Lee Jones]

On Tue, 14 Mar 2017, Charles Keepax wrote:

> On Tue, Mar 14, 2017 at 05:07:04PM +0000, Lee Jones wrote:
> > On Thu, 09 Mar 2017, Charles Keepax wrote:
> > 
> > > 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 <ckeepax@opensource.wolfsonmicro.com>
> > > ---
> > >  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?
> > 
> 
> 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.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH RESEND 4/4] mfd: arizona: Use regmap_read_poll_timeout instead of hard coding it

[Charles Keepax]

On Wed, Mar 15, 2017 at 12:17:03PM +0000, Lee Jones wrote:
> On Tue, 14 Mar 2017, Charles Keepax wrote:
> 
> > On Tue, Mar 14, 2017 at 05:07:04PM +0000, Lee Jones wrote:
> > > On Thu, 09 Mar 2017, Charles Keepax wrote:
> > > 
> > > > 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 <ckeepax@opensource.wolfsonmicro.com>
> > > > ---
> > > >  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?
> > > 
> > 
> > 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.
> 

I will spin a new version and squash them.

Thanks,
Charles

Re: [PATCH RESEND 1/4] mfd: arizona: Remove duplicate set of ret variable

[Lee Jones]

On Tue, 31 Jan 2017, Charles Keepax wrote:

> 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 <ckeepax@opensource.wolfsonmicro.com>
> ---
>  drivers/mfd/arizona-core.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
  
> diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
> index b6d4bc6..e00f577 100644
> --- a/drivers/mfd/arizona-core.c
> +++ b/drivers/mfd/arizona-core.c
> @@ -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);

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH RESEND 1/4] mfd: arizona: Remove duplicate set of ret variable

[Charles Keepax]

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 <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/mfd/arizona-core.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
index b6d4bc6..e00f577 100644
--- a/drivers/mfd/arizona-core.c
+++ b/drivers/mfd/arizona-core.c
@@ -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);
-- 
2.1.4