Re: [PATCH v5 3/4] firmware: psci: Read and use vendor reset types

[Elliot Berman <quic_eberman@quicinc.com>]

On Wed, Jun 19, 2024 at 02:51:43PM +0100, Sudeep Holla wrote:
> On Mon, Jun 17, 2024 at 10:18:09AM -0700, Elliot Berman wrote:
> > SoC vendors have different types of resets and are controlled through
> > various registers. For instance, Qualcomm chipsets can reboot to a
> > "download mode" that allows a RAM dump to be collected. Another example
> > is they also support writing a cookie that can be read by bootloader
> > during next boot. PSCI offers a mechanism, SYSTEM_RESET2, for these
> > vendor reset types to be implemented without requiring drivers for every
> > register/cookie.
> > 
> > Add support in PSCI to statically map reboot mode commands from
> > userspace to a vendor reset and cookie value using the device tree.
> > 
> > A separate initcall is needed to parse the devicetree, instead of using
> > psci_dt_init because mm isn't sufficiently set up to allocate memory.
> > 
> > Reboot mode framework is close but doesn't quite fit with the
> > design and requirements for PSCI SYSTEM_RESET2. Some of these issues can
> > be solved but doesn't seem reasonable in sum:
> >  1. reboot mode registers against the reboot_notifier_list, which is too
> >     early to call SYSTEM_RESET2. PSCI would need to remember the reset
> >     type from the reboot-mode framework callback and use it
> >     psci_sys_reset.
> >  2. reboot mode assumes only one cookie/parameter is described in the
> >     device tree. SYSTEM_RESET2 uses 2: one for the type and one for
> >     cookie.
> >  3. psci cpuidle driver already registers a driver against the
> >     arm,psci-1.0 compatible. Refactoring would be needed to have both a
> >     cpuidle and reboot-mode driver.
> >
> 
> I need to think through it but when you first introduced the generic
> Documentation/devicetree/bindings/power/reset/reboot-mode.yaml bindings
> I also looked at drivers/power/reset/reboot-mode.c
> 
> I assumed this extension to that binding would reuse the same and
> PSCI would just do reboot_mode_register(). I didn't expect to see these
> changes. I might have missing something but since the bindings is still
> quite generic with additional cells that act as additional cookie for
> reboot call, I still think that should be possible.
> 
> What am I missing here then ?
> 

Right, if that was only thing to "solve" to make it easy to use
reboot-mode framework, I agree we should update reboot mode framework to
work with the additional cells. There are a few other issues I mention
above which, when combined, make me feel that PSCI is different enough
from how reboot mode framework works that we shouldn't try to make PSCI
work with the framework. Issues #1 and #2 are pretty easy to solve
(whether they should be solved is different); I'm not sure a good
approach to issue #3.

Thanks,
Elliot