Re: [PATCH v2 5/6] regulator: pfuze100-regulator: provide pm_power_off_prepare handler

From: Andrew Morton
Date: Wed May 31 2017 - 16:59:48 EST


On Wed, 31 May 2017 08:14:56 +0200 Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx> wrote:

> On some boards the SoC can use one pin "PMIC_STBY_REQ" to notify th PMIC
> about state changes. In this case internal state of PMIC must be
> preconfigured for upcomming state change.
> It works fine with the current regulator framework, except with the
> power-off case.
>
> This patch is providing an optional pm_power_off_prepare handler
> which will configure the PMIC_StandBy state to disable all power lines.
>
> In my power consumption test on RIoTBoard, I got the following results:
> poweroff without this patch: 320 mA
> poweroff with this patch: 2 mA
> suspend to ram: 40 mA
>
> ...
>
> +static int pfuze_poweroff_pre_init(struct pfuze_chip *pfuze_chip)
> +{
> + if (pfuze_chip->chip_id != PFUZE100) {
> + dev_warn(pfuze_chip->dev, "Requested pm_power_off_prepare handler for not supoorted chip\n");
> + return -ENODEV;
> + }
> +
> + if (pm_power_off_prepare) {
> + dev_warn(pfuze_chip->dev, "pm_power_off_prepare is already registred.\n");
> + return -EBUSY;
> + }
> +
> + syspm_pfuze_chip = pfuze_chip;
> + pm_power_off_prepare = pfuze_poweroff_pre;
> +
> + return 0;
> +}
> +

Ah, there it is.

This looks a bit dodgy. What happens after someone does rmmod on this
driver?

(typo in comment: "supoorted")

(I wish we could get "poweroff" and "power_off" consistent)