[PATCH v2 2/9] cmd: boot: implement PMIC based poweroff

[Simon Glass]

Hi Svyatoslav,

On Mon, 31 Jul 2023 at 12:07, Svyatoslav Ryhel <clamor95 at gmail.com> wrote:
>
>
>
> 31 липня 2023 р. 20:08:06 GMT+03:00, Simon Glass <sjg at chromium.org> написав(-ла):
> >Hi Svyatoslav,
> >
> >On Sun, 30 Jul 2023 at 01:23, Svyatoslav Ryhel <clamor95 at gmail.com> wrote:
> >>
> >>
> >>
> >> 24 липня 2023 р. 05:28:24 GMT+03:00, Simon Glass <sjg at chromium.org> написав(-ла):
> >> >Hi Svyatoslav,
> >> >
> >> >On Sun, 23 Jul 2023 at 03:00, Svyatoslav Ryhel <clamor95 at gmail.com> wrote:
> >> >>
> >> >> нд, 23 лип. 2023 р. о 06:48 Simon Glass <sjg at chromium.org> пише:
> >> >> >
> >> >> > Hi Svyatoslav,
> >> >> >
> >> >> > On Thu, 20 Jul 2023 at 02:48, Svyatoslav Ryhel <clamor95 at gmail.com> wrote:
> >> >> > >
> >> >> > > Use new PMIC ops to perform device poweroff.
> >> >> > >
> >> >> > > Signed-off-by: Svyatoslav Ryhel <clamor95 at gmail.com>
> >> >> > > ---
> >> >> > >  cmd/Kconfig |  6 ++++++
> >> >> > >  cmd/boot.c  | 40 ++++++++++++++++++++++++++++++++++++++++
> >> >> > >  2 files changed, 46 insertions(+)
> >> >> > >
> >> >> > > diff --git a/cmd/Kconfig b/cmd/Kconfig
> >> >> > > index ecfd575237..47654297f8 100644
> >> >> > > --- a/cmd/Kconfig
> >> >> > > +++ b/cmd/Kconfig
> >> >> > > @@ -1439,6 +1439,12 @@ config CMD_POWEROFF
> >> >> > >         help
> >> >> > >           Poweroff/Shutdown the system
> >> >> > >
> >> >> > > +config CMD_PMIC_POWEROFF
> >> >> > > +       bool "PMIC poweroff"
> >> >> > > +       select CMD_POWEROFF
> >> >> > > +       help
> >> >> > > +         Shutdown the system using PMIC poweroff sequence.
> >> >> > > +
> >> >> > >  config CMD_READ
> >> >> > >         bool "read - Read binary data from a partition"
> >> >> > >         help
> >> >> > > diff --git a/cmd/boot.c b/cmd/boot.c
> >> >> > > index 14839c1ced..4270034194 100644
> >> >> > > --- a/cmd/boot.c
> >> >> > > +++ b/cmd/boot.c
> >> >> > > @@ -9,7 +9,13 @@
> >> >> > >   */
> >> >> > >  #include <common.h>
> >> >> > >  #include <command.h>
> >> >> > > +#include <dm.h>
> >> >> > > +#include <log.h>
> >> >> > >  #include <net.h>
> >> >> > > +#include <dm/device-internal.h>
> >> >> > > +#include <dm/uclass-internal.h>
> >> >> > > +#include <power/pmic.h>
> >> >> > > +#include <linux/delay.h>
> >> >> > >
> >> >> > >  #ifdef CONFIG_CMD_GO
> >> >> > >
> >> >> > > @@ -64,6 +70,40 @@ U_BOOT_CMD(
> >> >> > >  );
> >> >> > >
> >> >> > >  #ifdef CONFIG_CMD_POWEROFF
> >> >> > > +#ifdef CONFIG_CMD_PMIC_POWEROFF
> >> >> > > +int do_poweroff(struct cmd_tbl *cmdtp, int flag,
> >> >> > > +               int argc, char *const argv[])
> >> >> > > +{
> >> >> > > +       struct uc_pmic_priv *pmic_priv;
> >> >> > > +       struct udevice *dev;
> >> >> > > +       int ret;
> >> >> > > +
> >> >> > > +       for (uclass_find_first_device(UCLASS_PMIC, &dev);
> >> >> > > +            dev;
> >> >> > > +            uclass_find_next_device(&dev)) {
> >> >> > > +               if (dev && !device_active(dev)) {
> >> >> > > +                       ret = device_probe(dev);
> >> >> > > +                       if (ret)
> >> >> > > +                               return ret;
> >> >> > > +               }
> >> >> > > +
> >> >> > > +               /* value we need to check is set after probe */
> >> >> > > +               pmic_priv = dev_get_uclass_priv(dev);
> >> >> > > +               if (pmic_priv->sys_pow_ctrl) {
> >> >> > > +                       ret = pmic_poweroff(dev);
> >> >> > > +
> >> >> > > +                       /* wait some time and then print error */
> >> >> > > +                       mdelay(5000);
> >> >> > > +                       log_err("Failed to power off!!!\n");
> >> >> > > +                       return ret;
> >> >> > > +               }
> >> >> > > +       }
> >> >> > > +
> >> >> > > +       /* no device should reach here */
> >> >> > > +       return 1;
> >> >> > > +}
> >> >> > > +#endif
> >> >> > > +
> >> >> > >  U_BOOT_CMD(
> >> >> > >         poweroff, 1, 0, do_poweroff,
> >> >> > >         "Perform POWEROFF of the device",
> >> >> > > --
> >> >> > > 2.39.2
> >> >> > >
> >> >> >
> >> >> > How does this relate to sysreset_walk(SYSRESET_POWER_OFF) and
> >> >> > do_poweroff() in cmd/boot.c?
> >> >> >
> >> >>
> >> >> Yes, it seems that I have misunderstood you, but non the less, you say
> >> >> that I have to implement a new separate pmic subdriver just to support
> >> >> poweroff?
> >> >
> >> >Well it sounds like a lot, but it is not that hard. We try to model
> >> >devices by their functionality, which maps to a uclass, so when we
> >> >have a multi-function device we tend to model that with children, each
> >> >with an appropriate uclass.
> >>
> >> Alright, fair. I have prepared and tested sysreset for all PMICs I have sent and for Tegra in general. Unfortunately, they cannot be sent before existing patches are accepted and merged.
> >
> >You can ping the maintainer, or just note (in your cover letter) that
> >they depend on another series, e.g. with a patchwork link. I often
> >have 3-4 series backed up.
>
> This is definitely a nice advice, thank you. Though, sysreset subdevices will change pmic mfd, so this basically is changing code which even does not exist yet.
>
> P. S. If you remember our recent conversation about gpio cell of pmic, I have tested it with max77663 (single cell pmic) and modded gpio-uclass code. It worked surprisingly well with the regulator handling code I have proposed previously, and can provide gpios for keys and regulators.
>
> Those regulator changes affect mostly rockchip and srm32mp1. Maybe if you have rk3399 or stm32mp1 could you test them?

Yes I have rk3399, but I have lost track of your work. Can you please
send me the patchwork link for your series?

Regards,
Simon