Messages in this thread | | | Date | Sat, 21 Nov 2020 18:38:49 +0000 | From | Paul Cercueil <> | Subject | Re: [PATCH] remoteproc: Add module parameter 'auto_boot' |
| |
Hi Mathieu,
Le ven. 20 nov. 2020 à 15:37, Mathieu Poirier <mathieu.poirier@linaro.org> a écrit : > Hi Paul, > > On Sun, Nov 15, 2020 at 11:50:56AM +0000, Paul Cercueil wrote: >> Until now the remoteproc core would always default to trying to >> boot the >> remote processor at startup. The various remoteproc drivers could >> however override that setting. >> >> Whether or not we want the remote processor to boot, really depends >> on >> the nature of the processor itself - a processor built into a WiFi >> chip >> will need to be booted for the WiFi hardware to be usable, for >> instance, >> but a general-purpose co-processor does not have any predeterminated >> function, and as such we cannot assume that the OS will want the >> processor to be booted - yet alone that we have a single do-it-all >> firmware to load. >> > > If I understand correctly you have various remote processors that use > the same firmware > but are serving different purposes - is this correct?
That's the opposite actually. I have one remote processor which is general-purpose, and as such userspace may or may not want it started at boot time - depending on what it wants to do with it. The kernel shouldn't decide itself whether or not the remote processor should be started, because that's policy.
> >> Add a 'auto_boot' module parameter that instructs the remoteproc >> whether >> or not it should auto-boot the remote processor, which will default >> to >> "true" to respect the previous behaviour. >> > > Given that the core can't be a module I wonder if this isn't > something that > would be better off in the specific platform driver or the device > tree... Other > people might have an opinion as well.
Hardcoded in the platform driver or flagged in the device tree, doesn't change the fundamental problem - it should be up to the userspace to decide whether or not the remote processor should boot.
Cheers, -Paul
> >> Signed-off-by: Paul Cercueil <paul@crapouillou.net> >> --- >> drivers/remoteproc/remoteproc_core.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/remoteproc/remoteproc_core.c >> b/drivers/remoteproc/remoteproc_core.c >> index dab2c0f5caf0..687b1bfd49db 100644 >> --- a/drivers/remoteproc/remoteproc_core.c >> +++ b/drivers/remoteproc/remoteproc_core.c >> @@ -44,6 +44,11 @@ >> >> #define HIGH_BITS_MASK 0xFFFFFFFF00000000ULL >> >> +static bool auto_boot = true; >> +module_param(auto_boot, bool, 0400); >> +MODULE_PARM_DESC(auto_boot, >> + "Auto-boot the remote processor [default=true]"); >> + >> static DEFINE_MUTEX(rproc_list_mutex); >> static LIST_HEAD(rproc_list); >> static struct notifier_block rproc_panic_nb; >> @@ -2176,7 +2181,7 @@ struct rproc *rproc_alloc(struct device *dev, >> const char *name, >> return NULL; >> >> rproc->priv = &rproc[1]; >> - rproc->auto_boot = true; >> + rproc->auto_boot = auto_boot; >> rproc->elf_class = ELFCLASSNONE; >> rproc->elf_machine = EM_NONE; >> >> -- >> 2.29.2 >>
| |