[U-Boot] [PATCH] rk8xx: implement poweroff

[Simon Glass]

Hi Urja,

On Fri, 26 Apr 2019 at 04:47, Urja Rannikko <urjaman at gmail.com> wrote:
>
> Hi,
>
> On Wed, Apr 24, 2019 at 3:54 AM Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi,
> >
> > On Wed, 3 Apr 2019 at 06:21, Urja Rannikko <urjaman at gmail.com> wrote:
> > >
> > > Based on snooping around the linux kernel rk8xx driver, and
> > > tested to work on the ASUS C201.
> > >
> > > Signed-off-by: Urja Rannikko <urjaman at gmail.com>
> > > ---
> > > This is really handy to be able to poweroff (without pressing power button
> > > for a long time) the C201 from u-boot, so i'm sending this as is.
> > > The thing that is bothering me is the pmic_get --- i checked that
> > > every rk8xx is named "pmic" in the device tree so it should work, but
> > > it just feels really weird that this seems to be the best way to access
> > > the driver...
> > >
> > >  drivers/power/pmic/rk8xx.c | 34 ++++++++++++++++++++++++++++++++++
> > >  include/power/rk8xx_pmic.h |  4 ++++
> > >  2 files changed, 38 insertions(+)
> >
> > This should really use a sysreset driver.
> Thanks for the pointer since it wasnt very obvious because there isnt
> really any code that does this right now (nor does "sysreset" make one
> think of poweroff...).

Yes, and that was only recently added to the sysreset uclass.

See for example http://patchwork.ozlabs.org/patch/1091216/

>
> > You can make the PMIC have a child sysreset driver to implement this function.
> I looked for an example of this (even partial), and the only one i
> found was in drivers/power/pmic/stpmic1.c - so I assume like that but
> instead implement SYSRESET_POWER_OFF (could also implement
> SYSRESET_POWER, but that is kinda outside the scope of what i need
> right now...).

If in doubt sandbox is already a good thing to look for. E.g.
drivers/power/pmic/sandbox.c

>
> And then add a generic sysreset-based poweroff command implementation.

Sounds good - CONFIG_CMD_POWEROFF perhaps. Or maybe we should have a
'sysreset' command with sub-commands for warm, cold, power reset and
power off? We could make the existing 'reset' command use sysreset if
CONFIG_SYSRESET is enabled.

>
> (I'm just saying my ideas out loud before implementing them so that
> you have a chance to say if they're all wrong, regardless i'll be back
> with patches when i have a moment to work on this...)
>

Yes good idea. Whatever you do, sandbox is a great test environment
for new commands.

Regards,
Simon