Re: [PATCH] power: reset: Add MAX77620 support

[Thierry Reding <thierry.reding@gmail.com>]

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

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

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