| Message ID | 20241205051746.2465490-1-zmw12306@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | pwm: stm32-lp: Add check for clk_enable() | expand |
On Thu, Dec 05, 2024 at 12:17:46AM -0500, Mingwei Zheng wrote: > Add check for the return value of clk_enable() to catch the potential > error. Is this something that you actually hit, or just a janitoral fix you noticed while browsing the code (or reading some checker output)? > Fixes: e70a540b4e02 ("pwm: Add STM32 LPTimer PWM driver") > Signed-off-by: Mingwei Zheng <zmw12306@gmail.com> > Signed-off-by: Jiasheng Jiang <jiashengjiangcool@gmail.com> > --- > drivers/pwm/pwm-stm32-lp.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/pwm/pwm-stm32-lp.c b/drivers/pwm/pwm-stm32-lp.c > index 989731256f50..4abef304417d 100644 > --- a/drivers/pwm/pwm-stm32-lp.c > +++ b/drivers/pwm/pwm-stm32-lp.c > @@ -163,12 +163,16 @@ static int stm32_pwm_lp_get_state(struct pwm_chip *chip, > unsigned long rate = clk_get_rate(priv->clk); > u32 val, presc, prd; > u64 tmp; > + int ret; Please move this variable to the block where it's used. No need for such a big scope. Otherwise looks fine. Best regards Uwe
> On Dec 5, 2024, at 3:34 AM, Uwe Kleine-König <ukleinek@kernel.org> wrote: > > On Thu, Dec 05, 2024 at 12:17:46AM -0500, Mingwei Zheng wrote: >> Add check for the return value of clk_enable() to catch the potential >> error. > > Is this something that you actually hit, or just a janitoral fix you > noticed while browsing the code (or reading some checker output)? We detected this through static analysis, instead of actually hit. > >> Fixes: e70a540b4e02 ("pwm: Add STM32 LPTimer PWM driver") >> Signed-off-by: Mingwei Zheng <zmw12306@gmail.com> >> Signed-off-by: Jiasheng Jiang <jiashengjiangcool@gmail.com> >> --- >> drivers/pwm/pwm-stm32-lp.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/pwm/pwm-stm32-lp.c b/drivers/pwm/pwm-stm32-lp.c >> index 989731256f50..4abef304417d 100644 >> --- a/drivers/pwm/pwm-stm32-lp.c >> +++ b/drivers/pwm/pwm-stm32-lp.c >> @@ -163,12 +163,16 @@ static int stm32_pwm_lp_get_state(struct pwm_chip *chip, >> unsigned long rate = clk_get_rate(priv->clk); >> u32 val, presc, prd; >> u64 tmp; >> + int ret; > > Please move this variable to the block where it's used. No need for such > a big scope. > > Otherwise looks fine. > > Best regards > Uwe Thank you! We will submit a v2 patch for you to review! Best, Mingwei
diff --git a/drivers/pwm/pwm-stm32-lp.c b/drivers/pwm/pwm-stm32-lp.c index 989731256f50..4abef304417d 100644 --- a/drivers/pwm/pwm-stm32-lp.c +++ b/drivers/pwm/pwm-stm32-lp.c @@ -163,12 +163,16 @@ static int stm32_pwm_lp_get_state(struct pwm_chip *chip, unsigned long rate = clk_get_rate(priv->clk); u32 val, presc, prd; u64 tmp; + int ret; regmap_read(priv->regmap, STM32_LPTIM_CR, &val); state->enabled = !!FIELD_GET(STM32_LPTIM_ENABLE, val); /* Keep PWM counter clock refcount in sync with PWM initial state */ - if (state->enabled) - clk_enable(priv->clk); + if (state->enabled) { + ret = clk_enable(priv->clk); + if (ret) + return ret; + } regmap_read(priv->regmap, STM32_LPTIM_CFGR, &val); presc = FIELD_GET(STM32_LPTIM_PRESC, val);
Add check for the return value of clk_enable() to catch the potential error. Fixes: e70a540b4e02 ("pwm: Add STM32 LPTimer PWM driver") Signed-off-by: Mingwei Zheng <zmw12306@gmail.com> Signed-off-by: Jiasheng Jiang <jiashengjiangcool@gmail.com> --- drivers/pwm/pwm-stm32-lp.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)