linux-kernel - RE: [PATCH v2] gpio: pca953x: do not enable regmap cache when there is no regulator

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] [day] [month] [year] [list]

Message-ID:
 <DU0PR04MB9496A16FB98C8492C9F012CD902A2@DU0PR04MB9496.eurprd04.prod.outlook.com>
Date: Fri, 29 Nov 2024 02:26:13 +0000
From: Bough Chen <haibo.chen@....com>
To: Mark Brown <broonie@...nel.org>
CC: "linus.walleij@...aro.org" <linus.walleij@...aro.org>, "brgl@...ev.pl"
	<brgl@...ev.pl>, "lgirdwood@...il.com" <lgirdwood@...il.com>,
	"marek.vasut@...il.com" <marek.vasut@...il.com>, "linux-gpio@...r.kernel.org"
	<linux-gpio@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "imx@...ts.linux.dev" <imx@...ts.linux.dev>
Subject: RE: [PATCH v2] gpio: pca953x: do not enable regmap cache when there
 is no regulator

> -----Original Message-----
> From: Mark Brown <broonie@...nel.org>
> Sent: 2024年11月28日 20:28
> To: Bough Chen <haibo.chen@....com>
> Cc: linus.walleij@...aro.org; brgl@...ev.pl; lgirdwood@...il.com;
> marek.vasut@...il.com; linux-gpio@...r.kernel.org;
> linux-kernel@...r.kernel.org; imx@...ts.linux.dev
> Subject: Re: [PATCH v2] gpio: pca953x: do not enable regmap cache when there
> is no regulator
> 
> On Thu, Nov 28, 2024 at 10:00:42AM +0800, haibo.chen@....com wrote:
> 
> > Regmap cache mechanism is enabled in default. Thus, IO expander
> > wouldn't handle GPIO set really before resuming back.
> 
> > But there are cases need to toggle gpio in NO_IRQ stage.
> > e.g. To align with PCIe specification, PERST# signal connected on the
> > IO expander must be toggled during PCIe RC's NO_IRQ_RESUME.
> 
> > Do not enable the regmap cache when IO expander doesn't have the
> > regulator during system PM. That means the power of IO expander would
> > be kept on, and the GPIOs of the IO expander can be toggled really during
> system PM.
> 
> Aside from the fact that the device will always have a regulator (power isn't
> optional) this is obviously not related to any sequencing needs that you have on
> resume.  It might happen to work on your system, but it will potentially break
> other systems which do actually need the registers restoring but don't have a
> regulator explicitly defined and will fail to do anything on a system with ordering

Yes, you are right. I did not take this case into consideration.

> requirements that do have one defined.  The fix for this is to make this driver
> resume early if it's needed by other things that run in the resume sequence.

I once thought to move current system PM to NOIRQ PM, but seems not all i2c bus controller support i2c operation during NOIRQ PM.
Let me think whether there is a better solution, or do you have any suggestion?
 
> 
> > -	reg = devm_regulator_get(dev, "vcc");
> > -	if (IS_ERR(reg))
> > -		return dev_err_probe(dev, PTR_ERR(reg), "reg get err\n");
> > +	reg = devm_regulator_get_optional(dev, "vcc");
> > +	if (IS_ERR(reg)) {
> 
> This is obviously buggy, the main supply for the device is not going to be optional.

According to Documentation/devicetree/bindings/gpio/gpio-pca95xx.yaml, the vcc regulator is optional.

Regards
Haibo Chen