lkml.org 
[lkml]   [2020]   [Nov]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] remoteproc: Add module parameter 'auto_boot'
On Sat 21 Nov 12:38 CST 2020, Paul Cercueil wrote:

> 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.
>

Unfortunately it depends on what you're using your remoteprocs for. And
in a system with multiple remoteproc instances I don't think a single
global parameter is sufficient - not even a per-driver setting is.

I do agree with you that there are types of systems where the decision
to auto boot things would happen after the kernel and/or DT has been
written.

Regards,
Bjorn

> 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
> > >
>
>

\
 
 \ /
  Last update: 2020-11-22 06:31    [W:0.058 / U:6.416 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site