Hi Uwe, Thanks for the feedback. > Subject: Re: [PATCH v4 2/2] pwm: Add support for RZ/G2L GPT > > Hello, > > On Mon, Aug 01, 2022 at 07:24:06PM +0000, Biju Das wrote: > > > On Thu, Jul 28, 2022 at 05:25:26PM +0100, Biju Das wrote: > > > > + > > > > + if (pc->pwm_enabled_by_bootloader) > > > > + clk_disable(pc->clk); > > > > > > When this function is called as part of remove, not disabling the > > > clk is wrong, isn't it? > > > > I will remove pwm_enabled_by_bootloader variable and use the below > > changes, so it is taken care for the bootloader case. > > > > + rzg2l_gpt->clk = devm_clk_get_enabled(&pdev->dev, NULL); > > + if (IS_ERR(rzg2l_gpt->clk)) > > + return dev_err_probe(&pdev->dev, PTR_ERR(rzg2l_gpt->clk), > > + "cannot get clock\n"); > > + > > + rzg2l_gpt->rate = clk_get_rate(rzg2l_gpt->clk); > > + > > + /* > > + * We need to keep the clock on, in case the bootloader enabled > PWM and > > + * is running during probe(). > > + */ > > + if (!(rzg2l_gpt_read(rzg2l_gpt, RZG2L_GTCR) & RZG2L_GTCR_CST)) > > + devm_clk_put(&pdev->dev, rzg2l_gpt->clk); > > devm_clk_put looks wrong here. You only want to disable, not put?! It should be ok as to free the Clock, after getting the clock rate as the actual clock management is done by PM routines. Bootloader with PWM on case:- ------------------------------ Clk is turned on by "devm_clk_get_enabled" and after that PM manages the clock. Bootloader with PWM off case:- ----------------------------- PM manages the clk(Here no clk framework involvement, after freeing it). Please correct me if anything wrong here. Cheers, Biju
