* [PATCH] remoteproc: Add module parameter 'auto_boot' @ 2020-11-15 11:50 Paul Cercueil 2020-11-20 22:37 ` Mathieu Poirier 0 siblings, 1 reply; 8+ messages in thread From: Paul Cercueil @ 2020-11-15 11:50 UTC (permalink / raw) To: Ohad Ben-Cohen, Bjorn Andersson Cc: od, linux-remoteproc, linux-kernel, Paul Cercueil 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. 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. 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] remoteproc: Add module parameter 'auto_boot' 2020-11-15 11:50 [PATCH] remoteproc: Add module parameter 'auto_boot' Paul Cercueil @ 2020-11-20 22:37 ` Mathieu Poirier 2020-11-20 23:06 ` Suman Anna 2020-11-21 18:38 ` Paul Cercueil 0 siblings, 2 replies; 8+ messages in thread From: Mathieu Poirier @ 2020-11-20 22:37 UTC (permalink / raw) To: Paul Cercueil Cc: Ohad Ben-Cohen, Bjorn Andersson, od, linux-remoteproc, linux-kernel 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. Thanks, Mathieu > 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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] remoteproc: Add module parameter 'auto_boot' 2020-11-20 22:37 ` Mathieu Poirier @ 2020-11-20 23:06 ` Suman Anna 2020-11-21 18:47 ` Paul Cercueil 2020-11-21 18:38 ` Paul Cercueil 1 sibling, 1 reply; 8+ messages in thread From: Suman Anna @ 2020-11-20 23:06 UTC (permalink / raw) To: Mathieu Poirier, Paul Cercueil Cc: Ohad Ben-Cohen, Bjorn Andersson, od, linux-remoteproc, linux-kernel 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. Given that the individual platform drivers have to be modules, is there any issue in customizing this in your platform driver? regards Suman > > Thanks, > Mathieu > >> 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 >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] remoteproc: Add module parameter 'auto_boot' 2020-11-20 23:06 ` Suman Anna @ 2020-11-21 18:47 ` Paul Cercueil 2020-11-22 17:42 ` Suman Anna 0 siblings, 1 reply; 8+ messages in thread From: Paul Cercueil @ 2020-11-21 18:47 UTC (permalink / raw) To: Suman Anna Cc: Mathieu Poirier, Ohad Ben-Cohen, Bjorn Andersson, od, linux-remoteproc, linux-kernel Hi Suman, Le ven. 20 nov. 2020 à 17:06, Suman Anna <s-anna@ti.com> 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 <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 >>> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] remoteproc: Add module parameter 'auto_boot' 2020-11-21 18:47 ` Paul Cercueil @ 2020-11-22 17:42 ` Suman Anna 0 siblings, 0 replies; 8+ messages in thread From: Suman Anna @ 2020-11-22 17:42 UTC (permalink / raw) To: Paul Cercueil Cc: Mathieu Poirier, Ohad Ben-Cohen, Bjorn Andersson, od, linux-remoteproc, linux-kernel Hi Paul, On 11/21/20 12:47 PM, Paul Cercueil wrote: > Hi Suman, > > Le ven. 20 nov. 2020 à 17:06, Suman Anna <s-anna@ti.com> 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... I agree, but it sneaked in somehow. Anyway, I am not proposing that you repeat the same approach. > >> 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. So, remoteproc core is just setting up a default, and it is upto the individual drivers to override it. I actually have two different rproc drivers on the same SoC wanting different behavior for example. I do like the runtime config in general compared to a build-time Kconfig option, but let's say we do add this option, then you would also need every rproc driver to actually set this field specifically to ensure their expected behavior doesn't change if it is installed as false. regards Suman > > Cheers, > -Paul > >> regards >> Suman >> >>> >>> Thanks, >>> Mathieu >>> >>>> 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 >>>> >> > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] remoteproc: Add module parameter 'auto_boot' 2020-11-20 22:37 ` Mathieu Poirier 2020-11-20 23:06 ` Suman Anna @ 2020-11-21 18:38 ` Paul Cercueil 2020-11-22 5:28 ` Bjorn Andersson 2020-11-23 22:44 ` Mathieu Poirier 1 sibling, 2 replies; 8+ messages in thread From: Paul Cercueil @ 2020-11-21 18:38 UTC (permalink / raw) To: Mathieu Poirier Cc: Ohad Ben-Cohen, Bjorn Andersson, od, linux-remoteproc, linux-kernel 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 >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] remoteproc: Add module parameter 'auto_boot' 2020-11-21 18:38 ` Paul Cercueil @ 2020-11-22 5:28 ` Bjorn Andersson 2020-11-23 22:44 ` Mathieu Poirier 1 sibling, 0 replies; 8+ messages in thread From: Bjorn Andersson @ 2020-11-22 5:28 UTC (permalink / raw) To: Paul Cercueil Cc: Mathieu Poirier, Ohad Ben-Cohen, od, linux-remoteproc, linux-kernel 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 > > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] remoteproc: Add module parameter 'auto_boot' 2020-11-21 18:38 ` Paul Cercueil 2020-11-22 5:28 ` Bjorn Andersson @ 2020-11-23 22:44 ` Mathieu Poirier 1 sibling, 0 replies; 8+ messages in thread From: Mathieu Poirier @ 2020-11-23 22:44 UTC (permalink / raw) To: Paul Cercueil Cc: Ohad Ben-Cohen, Bjorn Andersson, od, linux-remoteproc, linux-kernel On Sat, Nov 21, 2020 at 06:38:49PM +0000, 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. That is why I suggested to add the module parameter to the platform driver(s) that needs it. That way user space may decide to do whatever it wants based on the scenario to enact. I see your point of view about putting the setting in the core but that is introducing drawbacks when dealing with more than one remote processor on the same system. Thanks, Mathieu > > 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 > > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-11-23 22:44 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-15 11:50 [PATCH] remoteproc: Add module parameter 'auto_boot' Paul Cercueil 2020-11-20 22:37 ` Mathieu Poirier 2020-11-20 23:06 ` Suman Anna 2020-11-21 18:47 ` Paul Cercueil 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
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).