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

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH 1/4] mfd: arizona: Remove duplicate set of ret variable
@ 2017-01-06 14:11 Charles Keepax
  2017-01-06 14:11 ` [PATCH 2/4] mfd: arizona: Display register addresses in hex Charles Keepax
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Charles Keepax @ 2017-01-06 14:11 UTC (permalink / raw)
  To: lee.jones; +Cc: patches, linux-kernel

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/4] mfd: arizona: Display register addresses in hex
  2017-01-06 14:11 [PATCH 1/4] mfd: arizona: Remove duplicate set of ret variable Charles Keepax
@ 2017-01-06 14:11 ` Charles Keepax
  2017-01-06 14:11 ` [PATCH 3/4] mfd: arizona: Update arizona_poll_reg to take a timeout in milliseconds Charles Keepax
  2017-01-06 14:11 ` [PATCH v2 4/4] mfd: arizona: Use regmap_read_poll_timeout instead of hard coding it Charles Keepax
  2 siblings, 0 replies; 6+ messages in thread
From: Charles Keepax @ 2017-01-06 14:11 UTC (permalink / raw)
  To: lee.jones; +Cc: patches, linux-kernel

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/4] mfd: arizona: Update arizona_poll_reg to take a timeout in milliseconds
  2017-01-06 14:11 [PATCH 1/4] mfd: arizona: Remove duplicate set of ret variable Charles Keepax
  2017-01-06 14:11 ` [PATCH 2/4] mfd: arizona: Display register addresses in hex Charles Keepax
@ 2017-01-06 14:11 ` Charles Keepax
  2017-01-06 14:19   ` Charles Keepax
  2017-01-06 14:11 ` [PATCH v2 4/4] mfd: arizona: Use regmap_read_poll_timeout instead of hard coding it Charles Keepax
  2 siblings, 1 reply; 6+ messages in thread
From: Charles Keepax @ 2017-01-06 14:11 UTC (permalink / raw)
  To: lee.jones; +Cc: patches, linux-kernel

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

diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
index 4cb34c3..1307294 100644
--- a/drivers/mfd/arizona-core.c
+++ b/drivers/mfd/arizona-core.c
@@ -235,14 +235,18 @@ static irqreturn_t arizona_overclocked(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+#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;
 	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;
 
-		usleep_range(1000, 5000);
+		usleep_range((ARIZONA_REG_POLL_DELAY_MS * 1000) / 2,
+			     ARIZONA_REG_POLL_DELAY_MS);
 	}
 
 	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,
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/4] mfd: arizona: Update arizona_poll_reg to take a timeout in milliseconds
  2017-01-06 14:11 ` [PATCH 3/4] mfd: arizona: Update arizona_poll_reg to take a timeout in milliseconds Charles Keepax
@ 2017-01-06 14:19   ` Charles Keepax
  0 siblings, 0 replies; 6+ messages in thread
From: Charles Keepax @ 2017-01-06 14:19 UTC (permalink / raw)
  To: lee.jones; +Cc: patches, linux-kernel

On Fri, Jan 06, 2017 at 02:11:35PM +0000, 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 | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
> index 4cb34c3..1307294 100644
> --- a/drivers/mfd/arizona-core.c
> +++ b/drivers/mfd/arizona-core.c
> @@ -235,14 +235,18 @@ static irqreturn_t arizona_overclocked(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
>  
> +#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;
>  	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;
>  
> -		usleep_range(1000, 5000);
> +		usleep_range((ARIZONA_REG_POLL_DELAY_MS * 1000) / 2,
> +			     ARIZONA_REG_POLL_DELAY_MS);

Oops sorry let me send another version missing a * 1000 here.

Thanks,
Charles

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 4/4] mfd: arizona: Use regmap_read_poll_timeout instead of hard coding it
  2017-01-06 14:11 [PATCH 1/4] mfd: arizona: Remove duplicate set of ret variable Charles Keepax
  2017-01-06 14:11 ` [PATCH 2/4] mfd: arizona: Display register addresses in hex Charles Keepax
  2017-01-06 14:11 ` [PATCH 3/4] mfd: arizona: Update arizona_poll_reg to take a timeout in milliseconds Charles Keepax
@ 2017-01-06 14:11 ` Charles Keepax
  2 siblings, 0 replies; 6+ messages in thread
From: Charles Keepax @ 2017-01-06 14:11 UTC (permalink / raw)
  To: lee.jones; +Cc: patches, linux-kernel

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

Changes since v1:
 - API updated to take a total time out rather than the number of
   polls of the register that should be done.

Thanks,
Charles

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

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 1/4] mfd: arizona: Remove duplicate set of ret variable
@ 2017-01-06 14:22 Charles Keepax
  2017-01-06 14:22 ` [PATCH 2/4] mfd: arizona: Display register addresses in hex Charles Keepax
  0 siblings, 1 reply; 6+ messages in thread
From: Charles Keepax @ 2017-01-06 14:22 UTC (permalink / raw)
  To: lee.jones; +Cc: patches, linux-kernel

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/4] mfd: arizona: Display register addresses in hex
  2017-01-06 14:22 [PATCH 1/4] mfd: arizona: Remove duplicate set of ret variable Charles Keepax
@ 2017-01-06 14:22 ` Charles Keepax
  0 siblings, 0 replies; 6+ messages in thread
From: Charles Keepax @ 2017-01-06 14:22 UTC (permalink / raw)
  To: lee.jones; +Cc: patches, linux-kernel

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-01-06 14:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-06 14:11 [PATCH 1/4] mfd: arizona: Remove duplicate set of ret variable Charles Keepax
2017-01-06 14:11 ` [PATCH 2/4] mfd: arizona: Display register addresses in hex Charles Keepax
2017-01-06 14:11 ` [PATCH 3/4] mfd: arizona: Update arizona_poll_reg to take a timeout in milliseconds Charles Keepax
2017-01-06 14:19   ` Charles Keepax
2017-01-06 14:11 ` [PATCH v2 4/4] mfd: arizona: Use regmap_read_poll_timeout instead of hard coding it Charles Keepax
  -- strict thread matches above, loose matches on Subject: below --
2017-01-06 14:22 [PATCH 1/4] mfd: arizona: Remove duplicate set of ret variable Charles Keepax
2017-01-06 14:22 ` [PATCH 2/4] mfd: arizona: Display register addresses in hex Charles Keepax

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.