Re: [PATCH v3 6/8] pwm: rzg2l-gpt: Add suspend/resume support

[Uwe Kleine-König]

On Tue, Sep 23, 2025 at 03:45:10PM +0100, Biju wrote:
> +static int rzg2l_gpt_suspend(struct device *dev)
> +{
> +	struct pwm_chip *chip = dev_get_drvdata(dev);
> +	struct rzg2l_gpt_chip *rzg2l_gpt = to_rzg2l_gpt_chip(chip);
> +	unsigned int i;
> +
> +	for (i = 0; i < RZG2L_MAX_HW_CHANNELS; i++) {
> +		if (!rzg2l_gpt->channel_enable_count[i])
> +			continue;
> +
> +		rzg2l_gpt->hw_cache[i].gtpr = rzg2l_gpt_read(rzg2l_gpt, RZG2L_GTPR(i));
> +		rzg2l_gpt->hw_cache[i].gtccr[0] = rzg2l_gpt_read(rzg2l_gpt, RZG2L_GTCCR(i, 0));
> +		rzg2l_gpt->hw_cache[i].gtccr[1] = rzg2l_gpt_read(rzg2l_gpt, RZG2L_GTCCR(i, 1));
> +		rzg2l_gpt->hw_cache[i].gtcr = rzg2l_gpt_read(rzg2l_gpt, RZG2L_GTCR(i));
> +		rzg2l_gpt->hw_cache[i].gtior = rzg2l_gpt_read(rzg2l_gpt, RZG2L_GTIOR(i));
> +	}
> +
> +	clk_disable_unprepare(rzg2l_gpt->clk);
> +	clk_disable_unprepare(rzg2l_gpt->bus_clk);
> +	reset_control_assert(rzg2l_gpt->rst_s);
> +	reset_control_assert(rzg2l_gpt->rst);
> +
> +	return 0;
> +}
> +
> +static int rzg2l_gpt_resume(struct device *dev)
> +{
> +	struct pwm_chip *chip = dev_get_drvdata(dev);
> +	struct rzg2l_gpt_chip *rzg2l_gpt = to_rzg2l_gpt_chip(chip);
> +	unsigned int i;
> +	int ret;
> +
> +	ret = reset_control_deassert(rzg2l_gpt->rst);
> +	if (ret)
> +		return ret;
> +
> +	ret = reset_control_deassert(rzg2l_gpt->rst_s);
> +	if (ret)
> +		goto fail_reset;
> +
> +	ret = clk_prepare_enable(rzg2l_gpt->bus_clk);
> +	if (ret)
> +		goto fail_reset_all;
> +
> +	ret = clk_prepare_enable(rzg2l_gpt->clk);
> +	if (ret)
> +		goto fail_bus_clk;
> +
> +	for (i = 0; i < RZG2L_MAX_HW_CHANNELS; i++) {
> +		if (!rzg2l_gpt->channel_enable_count[i])
> +			continue;
> +
> +		rzg2l_gpt_write(rzg2l_gpt, RZG2L_GTPR(i), rzg2l_gpt->hw_cache[i].gtpr);
> +		rzg2l_gpt_write(rzg2l_gpt, RZG2L_GTCCR(i, 0), rzg2l_gpt->hw_cache[i].gtccr[0]);
> +		rzg2l_gpt_write(rzg2l_gpt, RZG2L_GTCCR(i, 1), rzg2l_gpt->hw_cache[i].gtccr[1]);
> +		rzg2l_gpt_write(rzg2l_gpt, RZG2L_GTCR(i), rzg2l_gpt->hw_cache[i].gtcr);
> +		rzg2l_gpt_write(rzg2l_gpt, RZG2L_GTIOR(i), rzg2l_gpt->hw_cache[i].gtior);
> +	}
> +
> +	return 0;
> +fail_bus_clk:
> +	clk_disable_unprepare(rzg2l_gpt->bus_clk);
> +fail_reset_all:
> +	reset_control_assert(rzg2l_gpt->rst_s);
> +fail_reset:
> +	reset_control_assert(rzg2l_gpt->rst);
> +	return ret;

I wonder what happens if these calls in the error path fail. I think the
correct way would be to track the actual state to handle the state on
the next invokation for .resume() properly. But note that suspend/resume
is a somewhat blind spot for me, so I'm unsure here. (And I'm aware that
most resume callbacks don't cope cleanly here.)

I added linux-pm to Cc:, maybe someone can speak up about the
expectations here?

Best regards
Uwe

Attachment: signature.asc
Description: PGP signature