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

Elliot Berman quic_eberman at quicinc.com
Fri Aug 9 09:58:31 PDT 2024


On Fri, Aug 09, 2024 at 03:30:38PM +0200, Lorenzo Pieralisi wrote:
> On Wed, Aug 07, 2024 at 11:10:50AM -0700, Elliot Berman wrote:
> 
> [...]
> 
> > > > +static void psci_vendor_sys_reset2(unsigned long action, void *data)
> > > 
> > > 'action' is unused and therefore it is not really needed.
> > > 
> > > > +{
> > > > +	const char *cmd = data;
> > > > +	unsigned long ret;
> > > > +	size_t i;
> > > > +
> > > > +	for (i = 0; i < num_psci_reset_params; i++) {
> > > > +		if (!strcmp(psci_reset_params[i].mode, cmd)) {
> > > > +			ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
> > > > +					     psci_reset_params[i].reset_type,
> > > > +					     psci_reset_params[i].cookie, 0);
> > > > +			pr_err("failed to perform reset \"%s\": %ld\n",
> > > > +				cmd, (long)ret);
> > > > +		}
> > > > +	}
> > > > +}
> > > > +
> > > >  static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
> > > >  			  void *data)
> > > >  {
> > > > +	if (data && num_psci_reset_params)
> > > 
> > > So, reboot_mode here is basically ignored; if there is a vendor defined
> > > reset, we fire it off.
> > > 
> > > I think Mark mentioned his concerns earlier related to REBOOT_* mode and
> > > reset type (granted, the context was different):
> > > 
> > > https://lore.kernel.org/all/20200320120105.GA36658@C02TD0UTHF1T.local/
> > > 
> > > I would like to understand if this is the right thing to do before
> > > accepting this patchset.
> > > 
> > 
> > I don't have any concerns to move this part below checking reboot_mode.
> > Or, I could add reboot_mode == REBOOT_COLD check.
> 
> The question is how can we map vendor specific reboot magic to Linux
> reboot modes sensibly in generic PSCI code - that's by definition
> vendor specific.
> 

I don't think it's a reasonable thing to do. "reboot bootloader" or
"reboot edl" don't make sense to the Linux reboot modes.

I believe the Linux reboot modes enum is oriented to perspective of
Linux itself and the vendor resets are oriented towards behavior of the
SoC.

Thanks,
Elliot




More information about the linux-arm-kernel mailing list