[prev in list] [next in list] [prev in thread] [next in thread]
List: u-boot
Subject: Re: [PATCH v2 2/9] cmd: boot: implement PMIC based poweroff
From: Svyatoslav Ryhel <clamor95 () gmail ! com>
Date: 2023-07-31 18:07:39
Message-ID: EA85A350-082B-4420-9623-11A452874692 () gmail ! com
[Download RAW message or body]
31 липня 2023 р. 20:08:06 GMT+03:00, Simon Glass <sjg@chromium.org> \
написав(-ла):
> Hi Svyatoslav,
>
> On Sun, 30 Jul 2023 at 01:23, Svyatoslav Ryhel <clamor95@gmail.com> wrote:
> >
> >
> >
> > 24 липня 2023 р. 05:28:24 GMT+03:00, Simon Glass <sjg@chromium.org> \
> > написав(-ла):
> > > Hi Svyatoslav,
> > >
> > > On Sun, 23 Jul 2023 at 03:00, Svyatoslav Ryhel <clamor95@gmail.com> wrote:
> > > >
> > > > нд, 23 лип. 2023 р. о 06:48 Simon Glass <sjg@chromium.org> пише:
> > > > >
> > > > > Hi Svyatoslav,
> > > > >
> > > > > On Thu, 20 Jul 2023 at 02:48, Svyatoslav Ryhel <clamor95@gmail.com> wrote:
> > > > > >
> > > > > > Use new PMIC ops to perform device poweroff.
> > > > > >
> > > > > > Signed-off-by: Svyatoslav Ryhel <clamor95@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?
Best regards,
Svyatoslav R.
> Regards,
> Simon
>
> >
> > > Regards,
> > > Simon
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic