Hi Svyatoslav, On Mon, 31 Jul 2023 at 12:07, Svyatoslav Ryhel <clamo...@gmail.com> wrote: > > > > 31 липня 2023 р. 20:08:06 GMT+03:00, Simon Glass <s...@chromium.org> > написав(-ла): > >Hi Svyatoslav, > > > >On Sun, 30 Jul 2023 at 01:23, Svyatoslav Ryhel <clamo...@gmail.com> wrote: > >> > >> > >> > >> 24 липня 2023 р. 05:28:24 GMT+03:00, Simon Glass <s...@chromium.org> > >> написав(-ла): > >> >Hi Svyatoslav, > >> > > >> >On Sun, 23 Jul 2023 at 03:00, Svyatoslav Ryhel <clamo...@gmail.com> wrote: > >> >> > >> >> нд, 23 лип. 2023 р. о 06:48 Simon Glass <s...@chromium.org> пише: > >> >> > > >> >> > Hi Svyatoslav, > >> >> > > >> >> > On Thu, 20 Jul 2023 at 02:48, Svyatoslav Ryhel <clamo...@gmail.com> > >> >> > wrote: > >> >> > > > >> >> > > Use new PMIC ops to perform device poweroff. > >> >> > > > >> >> > > Signed-off-by: Svyatoslav Ryhel <clamo...@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