[PATCH 5/5] pwm: stm32: Fix enable count for clk in .probe()

[Uwe Kleine-König]

On Tue, Nov 14, 2023 at 02:35:19PM +0100, Fabrice Gasnier wrote:
> On 10/19/23 22:07, Uwe Kleine-König wrote:
> > From: Philipp Zabel <p.zabel at pengutronix.de>
> > 
> > Make the driver take over hardware state without disabling in .probe()
> > and enable the clock for each enabled channel.
> > 
> > Signed-off-by: Philipp Zabel <p.zabel at pengutronix.de>
> > [ukleinek: split off from a patch that also implemented .get_state()]
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig at pengutronix.de>
> > ---
> >  drivers/pwm/pwm-stm32.c | 18 ++++++++++++++----
> >  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> 
> Hi Uwe,
> 
> I'd only have a suggestion on the commit title:
> pwm: stm32: Fix enable count for clk in .probe()
>             ^
> On first sight, the "Fix" may suggest that this fixes something older in
> the tree. This would suggest a Fixes tag which isn't the case in this
> series, as this is a split of the .get_state() patch.
> Is it possible to rephrase ?

IMHO it is a fix, the hw state wasn't properly taken over before.
If you want a Fixes line, that's:

Fixes: 7edf7369205b ("pwm: Add driver for STM32 plaftorm")

> something like: set clk enable count when probing channels ?
> 
> Apart from that, you can add my:
> Reviewed-by: Fabrice Gasnier <fabrice.gasnier at foss.st.com>
> 
> ---
> I've additional questions, unrelated to this patch. I had a look at
> pwm-stm32-lp.c, the clock enabling is done directly in the .get_state()
> routine. I think this should be moved to the .probe() routine as done
> here. There's likely a risk, that clk enable count gets increased
> artificially, at least since commit cfc4c189bc70 "pwm: Read initial
> hardware state at request time".
> Shall I send a patch for pwm-stm32-lp.c, similar as this patch ? Or has
> it been spotted already ?

I'm not aware of someone working on stm32-lp, so feel free to prepare a
patch!

Best regards and thanks for your review,
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20231114/04370bb8/attachment.sig>