From: Paul Cercueil <email@example.com> To: Suman Anna <firstname.lastname@example.org> Cc: Mathieu Poirier <email@example.com>, Ohad Ben-Cohen <firstname.lastname@example.org>, Bjorn Andersson <email@example.com>, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org Subject: Re: [PATCH] remoteproc: Add module parameter 'auto_boot' Date: Sat, 21 Nov 2020 18:47:48 +0000 [thread overview] Message-ID: <OJT5KQ.QDDSGNHAM2LN1@crapouillou.net> (raw) In-Reply-To: <email@example.com> Hi Suman, Le ven. 20 nov. 2020 à 17:06, Suman Anna <firstname.lastname@example.org> a écrit : > Hi Paul, > > On 11/20/20 4:37 PM, Mathieu Poirier wrote: >> 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? >> >>> 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. > > I agree. Even it is a module, all it is setting up is default > behavior, and > doesn't buy you much. If you have one or more remoteproc drivers > supporting > different instances, and each one wants different behavior, you would > have to > customize it in the drivers anyway. ST drivers are customizing this > using a DT flag. Devicetree is supposed to describe the hardware, not how you're supposed to use the hardware... > Given that the individual platform drivers have to be modules, is > there any > issue in customizing this in your platform driver? No, I can patch the platform driver instead, but to me it clearly is a core issue. Cheers, -Paul > regards > Suman > >> >> Thanks, >> Mathieu >> >>> Signed-off-by: Paul Cercueil <email@example.com> >>> --- >>> 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; >>> - rproc->auto_boot = true; >>> + rproc->auto_boot = auto_boot; >>> rproc->elf_class = ELFCLASSNONE; >>> rproc->elf_machine = EM_NONE; >>> >>> -- >>> 2.29.2 >>> >
next prev parent reply other threads:[~2020-11-21 18:48 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-15 11:50 Paul Cercueil 2020-11-20 22:37 ` Mathieu Poirier 2020-11-20 23:06 ` Suman Anna 2020-11-21 18:47 ` Paul Cercueil [this message] 2020-11-22 17:42 ` Suman Anna 2020-11-21 18:38 ` Paul Cercueil 2020-11-22 5:28 ` Bjorn Andersson 2020-11-23 22:44 ` Mathieu Poirier
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=OJT5KQ.QDDSGNHAM2LN1@crapouillou.net \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCH] remoteproc: Add module parameter '\''auto_boot'\''' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).