Re: [PATCH v4 2/2] pwm: Add support for RZ/G2L GPT

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 1214 bytes --]

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?!

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]