Hi Svyatoslav,
On Tue, 8 Aug 2023 at 22:46, Svyatoslav Ryhel <clamo...@gmail.com> wrote:
>
>
>
> 9 серпня 2023 р. 05:03:41 GMT+03:00, Simon Glass <s...@chromium.org>
> написав(-ла):
> >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?
>
> Jonas Karlman proposed a nice adjustments making my patch less invasive. I
> enclose the link to regulator changes.
>
> What I can tell from my side, it works correctly and I have adjusted dm tests
> so they pass and test implemented changes at the same time.
>
> Currently I face i2c errors with pmic always-on/boot-on regulators (regulator
> probes ok, pmic probes and works ok, i2c gives error on xfer).
>
> Error occurs only if pmic regulator is probed after bind, if it is probed on
> pmic post_probe I have no error. Maybe you can reproduse this on your hw.
Well probe after bind is bad. We should let all devices be bound, then
worry about probing them when needed.
If you need this to happen from the board, why? It really should be an
lazy init, only when needed. If boards need this in general, then
let's think about an event to kick it off.
I don't see any failures with that series on Bob,
>
> Best regards,
> Svyatoslav R.
>
> Link:
> https://patchwork.ozlabs.org/project/uboot/list/?series=367513
>
> >Regards,
> >Simon
Regards,
Simon
