linux-kernel - Re: [PATCH] power: reset: Add MAX77620 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: <20170119122357.GF30182@ulmo.ba.sec>
Date:   Thu, 19 Jan 2017 13:23:57 +0100
From:   Thierry Reding <thierry.reding@...il.com>
To:     Sebastian Reichel <sre@...nel.org>
Cc:     Laxman Dewangan <ldewangan@...dia.com>,
        Martin Michlmayr <tbm@...ius.com>, linux-pm@...r.kernel.org,
        linux-kernel@...r.kernel.org, Chaitanya Bandi <bandik@...dia.com>
Subject: Re: [PATCH] power: reset: Add MAX77620 support

On Fri, Jan 13, 2017 at 04:44:25AM +0100, Sebastian Reichel wrote:
> On Thu, Jan 12, 2017 at 05:15:07PM +0100, Thierry Reding wrote:
[...]
> > +	if (!of_property_read_bool(np, "system-power-controller"))
> > +		return 0;
> > +
> > +	power = devm_kzalloc(&pdev->dev, sizeof(*power), GFP_KERNEL);
> > +	if (!power)
> > +		return -ENOMEM;
> > +
> > +	power->regmap = max77620->rmap;
> > +	power->dev = &pdev->dev;
> > +
> > +	err = regmap_read(power->regmap, MAX77620_REG_NVERC, &value);
> > +	if (err < 0) {
> > +		dev_err(power->dev, "failed to read event recorder: %d\n", err);
> > +		return err;
> > +	}
> > +
> > +	dev_dbg(&pdev->dev, "event recorder: %#x\n", value);
> > +
> > +	system_power_controller = power;
> > +	pm_power_off = max77620_pm_power_off;
> 
> > +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
> > +	arm_pm_restart = max77620_pm_restart;
> > +#endif
> 
> Please use register_restart_handler() instead. It has support for
> priorities, is not arm specific and properly supports unregistering
> (some handler with lower priority will take over). You can check
> gpio-restart as an example for the API.
> 
> If you have some other arm_pm_restart handler (those are tried first)
> I suggest to convert that to the restart handler framework. Actually
> it may be a good idea to convert all of them and drop arm_pm_restart,
> since there are only 4 left:
> 
> $ git grep "arm_pm_restart ="
> drivers/firmware/psci.c:        arm_pm_restart = psci_sys_reset;
> arch/arm/mach-prima2/rstc.c:    arm_pm_restart = sirfsoc_restart;
> arch/arm/xen/enlighten.c:       arm_pm_restart = xen_restart;
> arch/arm/kernel/setup.c:                arm_pm_restart = mdesc->restart;
> 
> The first 3 should be easy to update and the last one could
> be solved by registering a wrapper function as restart handler,
> which calls mdesc->restart().

I remember this not being the first time that this confuses me. And from
looking around a little it seems like I'm not the only one.

Do you know if there's ever been any attempts to formalize all of this
by adding some sort of framework for this? That way some of the
confusion may be removed and architecture-specific bits could be
eliminated more easily.

Thierry

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