| Message ID | 20221201152223.3133-1-andre.przywara@arm.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | pwm: sun4i: Propagate errors in .get_state() to the caller | expand |
On Thu, Dec 01, 2022 at 03:22:23PM +0000, Andre Przywara wrote: > .get_state() can return an error indication now. Make use of it to > propagate an impossible prescaler encoding, should that have sneaked in > somehow. > Also check the return value of clk_get_rate(). That's unlikely to fail, > but we use that in two divide operations down in the code, so let's > avoid a divide-by-zero condition on the way. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > Hi, > > this goes on top of Uwe's series to introduce and observe .get_state > failures: https://lore.kernel.org/linux-pwm/20221130152148.2769768-12-u.kleine-koenig@pengutronix.de/T/#m9af39aa03bbd9bb7b31b3600f110c65ee0e8e70b > Actually it only relies on patch 01/11 from that. I recommend to put this info in machine-readable form into your patch. If you applied my patch #1 on v6.1-rc1 and this on top, you'd do git format-patch -1 --base v6.1-rc1 This results in two additional lines that the build bots can evaluate, to find the right setup to test your patch. Apart from that: Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> and thanks to pick up on this topic, Uwe
On Thu, 1 Dec 2022 16:48:22 +0100 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > On Thu, Dec 01, 2022 at 03:22:23PM +0000, Andre Przywara wrote: > > .get_state() can return an error indication now. Make use of it to > > propagate an impossible prescaler encoding, should that have sneaked in > > somehow. > > Also check the return value of clk_get_rate(). That's unlikely to fail, > > but we use that in two divide operations down in the code, so let's > > avoid a divide-by-zero condition on the way. > > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > --- > > Hi, > > > > this goes on top of Uwe's series to introduce and observe .get_state > > failures: https://lore.kernel.org/linux-pwm/20221130152148.2769768-12-u.kleine-koenig@pengutronix.de/T/#m9af39aa03bbd9bb7b31b3600f110c65ee0e8e70b > > Actually it only relies on patch 01/11 from that. > > I recommend to put this info in machine-readable form into your patch. > If you applied my patch #1 on v6.1-rc1 and this on top, you'd do > > git format-patch -1 --base v6.1-rc1 > > This results in two additional lines that the build bots can evaluate, > to find the right setup to test your patch. Ah, nice one, didn't know about that. And just went I wanted to complain that this relies on public branches and committed patches, I learned that it doesn't. So I got: base-commit: eb7081409f94a9a8608593d0fb63a1aa3d6f95d8 prerequisite-patch-id: 3022328f2919301d79d852ef148c3f9dc4d723d6 Do you want me to resend the patch with this information? > Apart from that: > > Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Thanks for that! Cheers, Andre > > and thanks to pick up on this topic, > Uwe >
On 12/1/22 09:22, Andre Przywara wrote: > .get_state() can return an error indication now. Make use of it to > propagate an impossible prescaler encoding, should that have sneaked in > somehow. > Also check the return value of clk_get_rate(). That's unlikely to fail, > but we use that in two divide operations down in the code, so let's > avoid a divide-by-zero condition on the way. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > Hi, > > this goes on top of Uwe's series to introduce and observe .get_state > failures: https://lore.kernel.org/linux-pwm/20221130152148.2769768-12-u.kleine-koenig@pengutronix.de/T/#m9af39aa03bbd9bb7b31b3600f110c65ee0e8e70b > Actually it only relies on patch 01/11 from that. > > Cheers, > Andre > > drivers/pwm/pwm-sun4i.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) Reviewed-by: Samuel Holland <samuel@sholland.org>
Dne četrtek, 01. december 2022 ob 16:22:23 CET je Andre Przywara napisal(a): > .get_state() can return an error indication now. Make use of it to > propagate an impossible prescaler encoding, should that have sneaked in > somehow. > Also check the return value of clk_get_rate(). That's unlikely to fail, > but we use that in two divide operations down in the code, so let's > avoid a divide-by-zero condition on the way. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com> Best regards, Jernej
diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c index 37d75e252d4e..b973da73e9ab 100644 --- a/drivers/pwm/pwm-sun4i.c +++ b/drivers/pwm/pwm-sun4i.c @@ -118,6 +118,8 @@ static int sun4i_pwm_get_state(struct pwm_chip *chip, unsigned int prescaler; clk_rate = clk_get_rate(sun4i_pwm->clk); + if (!clk_rate) + return -EINVAL; val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG); @@ -142,7 +144,7 @@ static int sun4i_pwm_get_state(struct pwm_chip *chip, prescaler = prescaler_table[PWM_REG_PRESCAL(val, pwm->hwpwm)]; if (prescaler == 0) - return 0; + return -EINVAL; if (val & BIT_CH(PWM_ACT_STATE, pwm->hwpwm)) state->polarity = PWM_POLARITY_NORMAL;
.get_state() can return an error indication now. Make use of it to propagate an impossible prescaler encoding, should that have sneaked in somehow. Also check the return value of clk_get_rate(). That's unlikely to fail, but we use that in two divide operations down in the code, so let's avoid a divide-by-zero condition on the way. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- Hi, this goes on top of Uwe's series to introduce and observe .get_state failures: https://lore.kernel.org/linux-pwm/20221130152148.2769768-12-u.kleine-koenig@pengutronix.de/T/#m9af39aa03bbd9bb7b31b3600f110c65ee0e8e70b Actually it only relies on patch 01/11 from that. Cheers, Andre drivers/pwm/pwm-sun4i.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)