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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <pilx2etxezmr6rhbwculwjqcxotzkxus5bqdpjrt5na5c7fqhj@mdfm2yh2aptp>
Date: Mon, 1 Dec 2025 15:15:35 +0100
From: Uwe Kleine-König <ukleinek@...nel.org>
To: Biju Das <biju.das.jz@...renesas.com>
Cc: "biju.das.au" <biju.das.au@...il.com>, 
	Philipp Zabel <p.zabel@...gutronix.de>, "linux-pwm@...r.kernel.org" <linux-pwm@...r.kernel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Geert Uytterhoeven <geert+renesas@...der.be>, 
	Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...renesas.com>, 
	"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>, "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>
Subject: Re: [PATCH v3 6/8] pwm: rzg2l-gpt: Add suspend/resume support

Hello Biju,

On Mon, Dec 01, 2025 at 11:09:50AM +0000, Biju Das wrote:
> > -----Original Message-----
> > From: Uwe Kleine-König <ukleinek@...nel.org>
> > Sent: 30 November 2025 08:39
> > Subject: Re: [PATCH v3 6/8] pwm: rzg2l-gpt: Add suspend/resume support
> > 
> > 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.)
> 
> In str case, there is no power on the system during suspend and exit is, SoC reset followed by
> restoring registers from DDR. So, it does not matter for the suspend path.
> 
> In the resume case, If the calls to error path fail, then device won't work.

I'm not sure you understand my concern. IFAIUI a device that fails to
resume stays suspended from the POV of the kernel. When in this state
the resume is tried again at a later point in time you get
inconsistencies if the first reset was already deasserted from the
previous resume run (because reset_control_assert() failed in the resume
callback's error path).

Best regards
Uwe

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)