mfd: arizona: Remove duplicate set of ret variable
Subsystems (2)
PATCH v4 1/3 mfd: arizona: Remove duplicate set of ret variable
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.
---
No changes since v3.
drivers/mfd/arizona-core.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
drivers/mfd/arizona-core.c +2 -6
@@ -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);
Signature · 1 line
2.1.4
PATCH v4 2/3 mfd: arizona: Display register addresses in hex
Register addresses are normally displayed in hex throughout the Arizona driver. Update the arizona_poll_reg function to follow this convention.
---
No changes since v4.
drivers/mfd/arizona-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/mfd/arizona-core.c +2 -2
@@ -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 ; }
Signature · 1 line
2.1.4
Lee Jones
AppliedRe: [PATCH v4 2/3] mfd: arizona: Display register addresses in hex
Quoted 10 lines · Charles Keepax
Register addresses are normally displayed in hex throughout the Arizona driver. Update the
arizona_poll_regfunction to follow this convention.Signed-off-by Charles Keepax <[email protected]>---
No changes since v4.
drivers/mfd/arizona-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Applied, thanks.
Quoted 22 lines
drivers/mfd/arizona-core.c +2
-2@@ -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 ; }
Signature · 4 lines
Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
PATCH v4 3/3 mfd: arizona: Refactor arizona_poll_reg
In reply to Charles KeepaxCurrently, 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.
Additionally, arizona_poll_reg essentially hard-codes regmap_read_poll_timeout, update 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.
---
Changes since v3:
- Squashed together patches 3/4 from the v3 chain.
Thanks,
Charles
drivers/mfd/arizona-core.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-)
drivers/mfd/arizona-core.c +16 -20
@@ -235,29 +235,25 @@ static irqreturn_t arizona_overclocked(int irq, void *data)return IRQ_HANDLED ; } +#define ARIZONA_REG_POLL_DELAY_US 7500 +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 val =0 ; -int ret, i; - -for (i =0 ; i < timeout; 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 (1000 ,5000 ); - } + 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)
@@ -269,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,5 ,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)
@@ -339,7 +335,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,180 ,ARIZONA_INTERRUPT_RAW_STATUS_5 ,ARIZONA_FLL1_CLOCK_OK_STS ,ARIZONA_FLL1_CLOCK_OK_STS );if (ret)
@@ -403,7 +399,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,30 ,ARIZONA_WRITE_SEQUENCER_CTRL_1 ,ARIZONA_WSEQ_BUSY ,0 );if (ret) regmap_write(arizona->regmap, ARIZONA_WRITE_SEQUENCER_CTRL_0,
Signature · 1 line
2.1.4
Lee Jones
AppliedRe: [PATCH v4 3/3] mfd: arizona: Refactor arizona_poll_reg
Quoted 18 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.
Additionally,
arizona_poll_regessentially hard-codesregmap_read_poll_timeout, update the implementation to useregmap_read_poll_timeout. We still keeparizona_poll_regaround asregmap_read_poll_timeoutis a macro so rather than expand this for each caller keep it wrapped inarizona_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_timeoutcompares ktime's.Signed-off-by Charles Keepax <[email protected]>---
Quoted 8 lines
Changes since v3:
- Squashed together patches 3/4 from the v3 chain.Thanks,
Charlesdrivers/mfd/arizona-core.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-)
Much better, thanks.
Applied, thanks.
Quoted 74 lines
drivers/mfd/arizona-core.c +16
-20@@ -235,29 +235,25 @@ static irqreturn_t arizona_overclocked(int irq, void *data)return IRQ_HANDLED ; } +#define ARIZONA_REG_POLL_DELAY_US 7500 +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 val =0 ; -int ret, i; - -for (i =0 ; i < timeout; 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 (1000 ,5000 ); - } + 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)@@ -269,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,5 ,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)@@ -339,7 +335,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,180 ,ARIZONA_INTERRUPT_RAW_STATUS_5 ,ARIZONA_FLL1_CLOCK_OK_STS ,ARIZONA_FLL1_CLOCK_OK_STS );if (ret)@@ -403,7 +399,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,30 ,ARIZONA_WRITE_SEQUENCER_CTRL_1 ,ARIZONA_WSEQ_BUSY ,0 );if (ret) regmap_write(arizona->regmap, ARIZONA_WRITE_SEQUENCER_CTRL_0,
Signature · 4 lines
Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH v4 3/3] mfd: arizona: Refactor arizona_poll_reg
In reply to Charles KeepaxHi Charles,
Quoted 51 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.
Additionally,
arizona_poll_regessentially hard-codesregmap_read_poll_timeout, update the implementation to useregmap_read_poll_timeout. We still keeparizona_poll_regaround asregmap_read_poll_timeoutis a macro so rather than expand this for each caller keep it wrapped inarizona_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_timeoutcompares ktime's.Signed-off-by Charles Keepax <[email protected]>---
Changes since v3:
- Squashed together patches 3/4 from the v3 chain.Thanks,
Charlesdrivers/mfd/arizona-core.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-)drivers/mfd/arizona-core.c +3
-9@@ -235,29 +235,25 @@ static irqreturn_t arizona_overclocked(int irq, void *data)return IRQ_HANDLED ; } +#define ARIZONA_REG_POLL_DELAY_US 7500 +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 val =0 ; -int ret, i; - -for (i =0 ; i < timeout; 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 ;
drivers/mfd/arizona-core.c static irqreturn_t arizona_overclocked(int irq, void *data) · line 248
Quoted 10 lines
drivers/mfd/arizona-core.c +3
-6- } - -if ((val & mask) == target) -return 0 ; +int ret; -usleep_range (1000 ,5000 ); - } + ret = regmap_read_poll_timeout(arizona->regmap, + ARIZONA_INTERRUPT_RAW_STATUS_5, val,
s/ARIZONA_INTERRUPT_RAW_STATUS_5/reg/;
Regards
Andrzej
Quoted 41 lines
drivers/mfd/arizona-core.c +10
-5+ ((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)@@ -269,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,5 ,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)@@ -339,7 +335,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,180 ,ARIZONA_INTERRUPT_RAW_STATUS_5 ,ARIZONA_FLL1_CLOCK_OK_STS ,ARIZONA_FLL1_CLOCK_OK_STS );if (ret)@@ -403,7 +399,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,30 ,ARIZONA_WRITE_SEQUENCER_CTRL_1 ,ARIZONA_WSEQ_BUSY ,0 );if (ret) regmap_write(arizona->regmap, ARIZONA_WRITE_SEQUENCER_CTRL_0,
Re: [PATCH v4 3/3] mfd: arizona: Refactor arizona_poll_reg
Quoted 11 lines · Andrzej Hajda
Hi Charles,
drivers/mfd/arizona-core.c static irqreturn_t arizona_overclocked(int irq, void *data) · line 248
Quoted 4 lines · Charles Keepax
drivers/mfd/arizona-core.c +2
-2-usleep_range (1000 ,5000 ); - } + ret = regmap_read_poll_timeout(arizona->regmap, + ARIZONA_INTERRUPT_RAW_STATUS_5, val,s/ARIZONA_INTERRUPT_RAW_STATUS_5/reg/;
Quoted 3 lines
Regards
Andrzej
Thank you very much for spotting that I will send a fix on Monday.
Thanks,
Charles
Lee Jones
AppliedQuoted 11 lines · Charles Keepax
arizona_poll_regalready 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]>---
No changes since v3.
drivers/mfd/arizona-core.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
Applied, thanks.
Quoted 29 lines
drivers/mfd/arizona-core.c +2
-6@@ -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);
Signature · 4 lines
Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog