2019-12-10 16:41:28

by Paul Cercueil

[permalink] [raw]
Subject: [PATCH v4 3/5] remoteproc: Add prepare/unprepare callbacks

The .prepare() callback is called before the firmware is loaded to
memory. This is useful for instance in the case where some setup is
required for the memory to be accessible.

Signed-off-by: Paul Cercueil <[email protected]>
---

Notes:
v2-v4: No change

drivers/remoteproc/remoteproc_core.c | 16 +++++++++++++++-
include/linux/remoteproc.h | 4 ++++
2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 0a9fc7fdd1c3..3ea5f675a148 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1299,11 +1299,19 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
struct device *dev = &rproc->dev;
int ret;

+ if (rproc->ops->prepare) {
+ ret = rproc->ops->prepare(rproc);
+ if (ret) {
+ dev_err(dev, "Failed to prepare rproc: %d\n", ret);
+ return ret;
+ }
+ }
+
/* load the ELF segments to memory */
ret = rproc_load_segments(rproc, fw);
if (ret) {
dev_err(dev, "Failed to load program segments: %d\n", ret);
- return ret;
+ goto unprepare_rproc;
}

/*
@@ -1354,6 +1362,9 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
rproc_unprepare_subdevices(rproc);
reset_table_ptr:
rproc->table_ptr = rproc->cached_table;
+unprepare_rproc:
+ if (rproc->ops->unprepare)
+ rproc->ops->unprepare(rproc);

return ret;
}
@@ -1483,6 +1494,9 @@ static int rproc_stop(struct rproc *rproc, bool crashed)

rproc->state = RPROC_OFFLINE;

+ if (rproc->ops->unprepare)
+ rproc->ops->unprepare(rproc);
+
dev_info(dev, "stopped remote processor %s\n", rproc->name);

return 0;
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 5f201f0c86c3..a6272d1ba384 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -355,6 +355,8 @@ enum rsc_handling_status {

/**
* struct rproc_ops - platform-specific device handlers
+ * @prepare: prepare the device for power up (before the firmware is loaded)
+ * @unprepare: unprepare the device after it is stopped
* @start: power on the device and boot it
* @stop: power off the device
* @kick: kick a virtqueue (virtqueue id given as a parameter)
@@ -371,6 +373,8 @@ enum rsc_handling_status {
* @get_boot_addr: get boot address to entry point specified in firmware
*/
struct rproc_ops {
+ int (*prepare)(struct rproc *rproc);
+ void (*unprepare)(struct rproc *rproc);
int (*start)(struct rproc *rproc);
int (*stop)(struct rproc *rproc);
void (*kick)(struct rproc *rproc, int vqid);
--
2.24.0


2019-12-12 10:05:51

by Fabien DESSENNE

[permalink] [raw]
Subject: Re: [PATCH v4 3/5] remoteproc: Add prepare/unprepare callbacks

Hi Paul


On 10/12/2019 5:40 PM, Paul Cercueil wrote:
> The .prepare() callback is called before the firmware is loaded to
> memory. This is useful for instance in the case where some setup is
> required for the memory to be accessible.


I am trying to figure out what king of 'setup' may be required. From the
ingenic driver I understand that you need to enable clocks to allow some
memory access.

Instead of adding this new ops, why not enabling clocks in probe()?

BR

Fabien


>
> Signed-off-by: Paul Cercueil <[email protected]>
> ---
>
> Notes:
> v2-v4: No change
>
> drivers/remoteproc/remoteproc_core.c | 16 +++++++++++++++-
> include/linux/remoteproc.h | 4 ++++
> 2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 0a9fc7fdd1c3..3ea5f675a148 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -1299,11 +1299,19 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
> struct device *dev = &rproc->dev;
> int ret;
>
> + if (rproc->ops->prepare) {
> + ret = rproc->ops->prepare(rproc);
> + if (ret) {
> + dev_err(dev, "Failed to prepare rproc: %d\n", ret);
> + return ret;
> + }
> + }
> +
> /* load the ELF segments to memory */
> ret = rproc_load_segments(rproc, fw);
> if (ret) {
> dev_err(dev, "Failed to load program segments: %d\n", ret);
> - return ret;
> + goto unprepare_rproc;
> }
>
> /*
> @@ -1354,6 +1362,9 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
> rproc_unprepare_subdevices(rproc);
> reset_table_ptr:
> rproc->table_ptr = rproc->cached_table;
> +unprepare_rproc:
> + if (rproc->ops->unprepare)
> + rproc->ops->unprepare(rproc);
>
> return ret;
> }
> @@ -1483,6 +1494,9 @@ static int rproc_stop(struct rproc *rproc, bool crashed)
>
> rproc->state = RPROC_OFFLINE;
>
> + if (rproc->ops->unprepare)
> + rproc->ops->unprepare(rproc);
> +
> dev_info(dev, "stopped remote processor %s\n", rproc->name);
>
> return 0;
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index 5f201f0c86c3..a6272d1ba384 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -355,6 +355,8 @@ enum rsc_handling_status {
>
> /**
> * struct rproc_ops - platform-specific device handlers
> + * @prepare: prepare the device for power up (before the firmware is loaded)
> + * @unprepare: unprepare the device after it is stopped
> * @start: power on the device and boot it
> * @stop: power off the device
> * @kick: kick a virtqueue (virtqueue id given as a parameter)
> @@ -371,6 +373,8 @@ enum rsc_handling_status {
> * @get_boot_addr: get boot address to entry point specified in firmware
> */
> struct rproc_ops {
> + int (*prepare)(struct rproc *rproc);
> + void (*unprepare)(struct rproc *rproc);
> int (*start)(struct rproc *rproc);
> int (*stop)(struct rproc *rproc);
> void (*kick)(struct rproc *rproc, int vqid);

2019-12-14 22:32:01

by Paul Cercueil

[permalink] [raw]
Subject: Re: [PATCH v4 3/5] remoteproc: Add prepare/unprepare callbacks

Hi Fabien,


Le jeu., d?c. 12, 2019 at 10:03, Fabien DESSENNE
<[email protected]> a ?crit :
> Hi Paul
>
>
> On 10/12/2019 5:40 PM, Paul Cercueil wrote:
>> The .prepare() callback is called before the firmware is loaded to
>> memory. This is useful for instance in the case where some setup is
>> required for the memory to be accessible.
>
>
> I am trying to figure out what king of 'setup' may be required. From
> the
> ingenic driver I understand that you need to enable clocks to allow
> some
> memory access.
>
> Instead of adding this new ops, why not enabling clocks in probe()?

Enabling the clocks in the probe means that the clocks will be
unconditionally enabled until the driver is removed, even if the remote
processor end up being unused. That would be a waste of power.

Cheers,
-Paul


>
> BR
>
> Fabien
>
>
>>
>> Signed-off-by: Paul Cercueil <[email protected]>
>> ---
>>
>> Notes:
>> v2-v4: No change
>>
>> drivers/remoteproc/remoteproc_core.c | 16 +++++++++++++++-
>> include/linux/remoteproc.h | 4 ++++
>> 2 files changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/remoteproc/remoteproc_core.c
>> b/drivers/remoteproc/remoteproc_core.c
>> index 0a9fc7fdd1c3..3ea5f675a148 100644
>> --- a/drivers/remoteproc/remoteproc_core.c
>> +++ b/drivers/remoteproc/remoteproc_core.c
>> @@ -1299,11 +1299,19 @@ static int rproc_start(struct rproc *rproc,
>> const struct firmware *fw)
>> struct device *dev = &rproc->dev;
>> int ret;
>>
>> + if (rproc->ops->prepare) {
>> + ret = rproc->ops->prepare(rproc);
>> + if (ret) {
>> + dev_err(dev, "Failed to prepare rproc: %d\n", ret);
>> + return ret;
>> + }
>> + }
>> +
>> /* load the ELF segments to memory */
>> ret = rproc_load_segments(rproc, fw);
>> if (ret) {
>> dev_err(dev, "Failed to load program segments: %d\n", ret);
>> - return ret;
>> + goto unprepare_rproc;
>> }
>>
>> /*
>> @@ -1354,6 +1362,9 @@ static int rproc_start(struct rproc *rproc,
>> const struct firmware *fw)
>> rproc_unprepare_subdevices(rproc);
>> reset_table_ptr:
>> rproc->table_ptr = rproc->cached_table;
>> +unprepare_rproc:
>> + if (rproc->ops->unprepare)
>> + rproc->ops->unprepare(rproc);
>>
>> return ret;
>> }
>> @@ -1483,6 +1494,9 @@ static int rproc_stop(struct rproc *rproc,
>> bool crashed)
>>
>> rproc->state = RPROC_OFFLINE;
>>
>> + if (rproc->ops->unprepare)
>> + rproc->ops->unprepare(rproc);
>> +
>> dev_info(dev, "stopped remote processor %s\n", rproc->name);
>>
>> return 0;
>> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
>> index 5f201f0c86c3..a6272d1ba384 100644
>> --- a/include/linux/remoteproc.h
>> +++ b/include/linux/remoteproc.h
>> @@ -355,6 +355,8 @@ enum rsc_handling_status {
>>
>> /**
>> * struct rproc_ops - platform-specific device handlers
>> + * @prepare: prepare the device for power up (before the firmware
>> is loaded)
>> + * @unprepare: unprepare the device after it is stopped
>> * @start: power on the device and boot it
>> * @stop: power off the device
>> * @kick: kick a virtqueue (virtqueue id given as a parameter)
>> @@ -371,6 +373,8 @@ enum rsc_handling_status {
>> * @get_boot_addr: get boot address to entry point specified in
>> firmware
>> */
>> struct rproc_ops {
>> + int (*prepare)(struct rproc *rproc);
>> + void (*unprepare)(struct rproc *rproc);
>> int (*start)(struct rproc *rproc);
>> int (*stop)(struct rproc *rproc);
>> void (*kick)(struct rproc *rproc, int vqid);


2019-12-21 20:58:53

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v4 3/5] remoteproc: Add prepare/unprepare callbacks

On Tue 10 Dec 08:40 PST 2019, Paul Cercueil wrote:

> The .prepare() callback is called before the firmware is loaded to
> memory. This is useful for instance in the case where some setup is
> required for the memory to be accessible.
>

Would it make sense to somehow tie this prepare/unprepare to the actual
struct rproc_mem_entry that needs the resource enabled?

Regards,
Bjorn

> Signed-off-by: Paul Cercueil <[email protected]>
> ---
>
> Notes:
> v2-v4: No change
>
> drivers/remoteproc/remoteproc_core.c | 16 +++++++++++++++-
> include/linux/remoteproc.h | 4 ++++
> 2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 0a9fc7fdd1c3..3ea5f675a148 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -1299,11 +1299,19 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
> struct device *dev = &rproc->dev;
> int ret;
>
> + if (rproc->ops->prepare) {
> + ret = rproc->ops->prepare(rproc);
> + if (ret) {
> + dev_err(dev, "Failed to prepare rproc: %d\n", ret);
> + return ret;
> + }
> + }
> +
> /* load the ELF segments to memory */
> ret = rproc_load_segments(rproc, fw);
> if (ret) {
> dev_err(dev, "Failed to load program segments: %d\n", ret);
> - return ret;
> + goto unprepare_rproc;
> }
>
> /*
> @@ -1354,6 +1362,9 @@ static int rproc_start(struct rproc *rproc, const struct firmware *fw)
> rproc_unprepare_subdevices(rproc);
> reset_table_ptr:
> rproc->table_ptr = rproc->cached_table;
> +unprepare_rproc:
> + if (rproc->ops->unprepare)
> + rproc->ops->unprepare(rproc);
>
> return ret;
> }
> @@ -1483,6 +1494,9 @@ static int rproc_stop(struct rproc *rproc, bool crashed)
>
> rproc->state = RPROC_OFFLINE;
>
> + if (rproc->ops->unprepare)
> + rproc->ops->unprepare(rproc);
> +
> dev_info(dev, "stopped remote processor %s\n", rproc->name);
>
> return 0;
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index 5f201f0c86c3..a6272d1ba384 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -355,6 +355,8 @@ enum rsc_handling_status {
>
> /**
> * struct rproc_ops - platform-specific device handlers
> + * @prepare: prepare the device for power up (before the firmware is loaded)
> + * @unprepare: unprepare the device after it is stopped
> * @start: power on the device and boot it
> * @stop: power off the device
> * @kick: kick a virtqueue (virtqueue id given as a parameter)
> @@ -371,6 +373,8 @@ enum rsc_handling_status {
> * @get_boot_addr: get boot address to entry point specified in firmware
> */
> struct rproc_ops {
> + int (*prepare)(struct rproc *rproc);
> + void (*unprepare)(struct rproc *rproc);
> int (*start)(struct rproc *rproc);
> int (*stop)(struct rproc *rproc);
> void (*kick)(struct rproc *rproc, int vqid);
> --
> 2.24.0
>

2020-01-15 21:31:55

by Paul Cercueil

[permalink] [raw]
Subject: Re: [PATCH v4 3/5] remoteproc: Add prepare/unprepare callbacks

Hi Bjorn,


Le sam., d?c. 21, 2019 at 12:20, Bjorn Andersson
<[email protected]> a ?crit :
> On Tue 10 Dec 08:40 PST 2019, Paul Cercueil wrote:
>
>> The .prepare() callback is called before the firmware is loaded to
>> memory. This is useful for instance in the case where some setup is
>> required for the memory to be accessible.
>>
>
> Would it make sense to somehow tie this prepare/unprepare to the
> actual
> struct rproc_mem_entry that needs the resource enabled?

Do you need such granularity?

In my case, the three memories need the same clock to be enabled.

-Paul


>
>> Signed-off-by: Paul Cercueil <[email protected]>
>> ---
>>
>> Notes:
>> v2-v4: No change
>>
>> drivers/remoteproc/remoteproc_core.c | 16 +++++++++++++++-
>> include/linux/remoteproc.h | 4 ++++
>> 2 files changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/remoteproc/remoteproc_core.c
>> b/drivers/remoteproc/remoteproc_core.c
>> index 0a9fc7fdd1c3..3ea5f675a148 100644
>> --- a/drivers/remoteproc/remoteproc_core.c
>> +++ b/drivers/remoteproc/remoteproc_core.c
>> @@ -1299,11 +1299,19 @@ static int rproc_start(struct rproc *rproc,
>> const struct firmware *fw)
>> struct device *dev = &rproc->dev;
>> int ret;
>>
>> + if (rproc->ops->prepare) {
>> + ret = rproc->ops->prepare(rproc);
>> + if (ret) {
>> + dev_err(dev, "Failed to prepare rproc: %d\n", ret);
>> + return ret;
>> + }
>> + }
>> +
>> /* load the ELF segments to memory */
>> ret = rproc_load_segments(rproc, fw);
>> if (ret) {
>> dev_err(dev, "Failed to load program segments: %d\n", ret);
>> - return ret;
>> + goto unprepare_rproc;
>> }
>>
>> /*
>> @@ -1354,6 +1362,9 @@ static int rproc_start(struct rproc *rproc,
>> const struct firmware *fw)
>> rproc_unprepare_subdevices(rproc);
>> reset_table_ptr:
>> rproc->table_ptr = rproc->cached_table;
>> +unprepare_rproc:
>> + if (rproc->ops->unprepare)
>> + rproc->ops->unprepare(rproc);
>>
>> return ret;
>> }
>> @@ -1483,6 +1494,9 @@ static int rproc_stop(struct rproc *rproc,
>> bool crashed)
>>
>> rproc->state = RPROC_OFFLINE;
>>
>> + if (rproc->ops->unprepare)
>> + rproc->ops->unprepare(rproc);
>> +
>> dev_info(dev, "stopped remote processor %s\n", rproc->name);
>>
>> return 0;
>> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
>> index 5f201f0c86c3..a6272d1ba384 100644
>> --- a/include/linux/remoteproc.h
>> +++ b/include/linux/remoteproc.h
>> @@ -355,6 +355,8 @@ enum rsc_handling_status {
>>
>> /**
>> * struct rproc_ops - platform-specific device handlers
>> + * @prepare: prepare the device for power up (before the firmware
>> is loaded)
>> + * @unprepare: unprepare the device after it is stopped
>> * @start: power on the device and boot it
>> * @stop: power off the device
>> * @kick: kick a virtqueue (virtqueue id given as a parameter)
>> @@ -371,6 +373,8 @@ enum rsc_handling_status {
>> * @get_boot_addr: get boot address to entry point specified in
>> firmware
>> */
>> struct rproc_ops {
>> + int (*prepare)(struct rproc *rproc);
>> + void (*unprepare)(struct rproc *rproc);
>> int (*start)(struct rproc *rproc);
>> int (*stop)(struct rproc *rproc);
>> void (*kick)(struct rproc *rproc, int vqid);
>> --
>> 2.24.0
>>


2020-01-20 20:21:19

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v4 3/5] remoteproc: Add prepare/unprepare callbacks

On Wed 15 Jan 13:15 PST 2020, Paul Cercueil wrote:

> Hi Bjorn,
>
>
> Le sam., d?c. 21, 2019 at 12:20, Bjorn Andersson
> <[email protected]> a ?crit :
> > On Tue 10 Dec 08:40 PST 2019, Paul Cercueil wrote:
> >
> > > The .prepare() callback is called before the firmware is loaded to
> > > memory. This is useful for instance in the case where some setup is
> > > required for the memory to be accessible.
> > >
> >
> > Would it make sense to somehow tie this prepare/unprepare to the actual
> > struct rproc_mem_entry that needs the resource enabled?
>
> Do you need such granularity?
>

I don't have such needs, but given some of the memory structure that
Suman and Loic has been talking about I would expect that such need
exists.

> In my case, the three memories need the same clock to be enabled.
>

But we can update your driver to associate your one clock with the
memory objects if/when we end up implementing this later.

Regards,
Bjorn

> -Paul
>
>
> >
> > > Signed-off-by: Paul Cercueil <[email protected]>
> > > ---
> > >
> > > Notes:
> > > v2-v4: No change
> > >
> > > drivers/remoteproc/remoteproc_core.c | 16 +++++++++++++++-
> > > include/linux/remoteproc.h | 4 ++++
> > > 2 files changed, 19 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/remoteproc/remoteproc_core.c
> > > b/drivers/remoteproc/remoteproc_core.c
> > > index 0a9fc7fdd1c3..3ea5f675a148 100644
> > > --- a/drivers/remoteproc/remoteproc_core.c
> > > +++ b/drivers/remoteproc/remoteproc_core.c
> > > @@ -1299,11 +1299,19 @@ static int rproc_start(struct rproc *rproc,
> > > const struct firmware *fw)
> > > struct device *dev = &rproc->dev;
> > > int ret;
> > >
> > > + if (rproc->ops->prepare) {
> > > + ret = rproc->ops->prepare(rproc);
> > > + if (ret) {
> > > + dev_err(dev, "Failed to prepare rproc: %d\n", ret);
> > > + return ret;
> > > + }
> > > + }
> > > +
> > > /* load the ELF segments to memory */
> > > ret = rproc_load_segments(rproc, fw);
> > > if (ret) {
> > > dev_err(dev, "Failed to load program segments: %d\n", ret);
> > > - return ret;
> > > + goto unprepare_rproc;
> > > }
> > >
> > > /*
> > > @@ -1354,6 +1362,9 @@ static int rproc_start(struct rproc *rproc,
> > > const struct firmware *fw)
> > > rproc_unprepare_subdevices(rproc);
> > > reset_table_ptr:
> > > rproc->table_ptr = rproc->cached_table;
> > > +unprepare_rproc:
> > > + if (rproc->ops->unprepare)
> > > + rproc->ops->unprepare(rproc);
> > >
> > > return ret;
> > > }
> > > @@ -1483,6 +1494,9 @@ static int rproc_stop(struct rproc *rproc,
> > > bool crashed)
> > >
> > > rproc->state = RPROC_OFFLINE;
> > >
> > > + if (rproc->ops->unprepare)
> > > + rproc->ops->unprepare(rproc);
> > > +
> > > dev_info(dev, "stopped remote processor %s\n", rproc->name);
> > >
> > > return 0;
> > > diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> > > index 5f201f0c86c3..a6272d1ba384 100644
> > > --- a/include/linux/remoteproc.h
> > > +++ b/include/linux/remoteproc.h
> > > @@ -355,6 +355,8 @@ enum rsc_handling_status {
> > >
> > > /**
> > > * struct rproc_ops - platform-specific device handlers
> > > + * @prepare: prepare the device for power up (before the firmware
> > > is loaded)
> > > + * @unprepare: unprepare the device after it is stopped
> > > * @start: power on the device and boot it
> > > * @stop: power off the device
> > > * @kick: kick a virtqueue (virtqueue id given as a parameter)
> > > @@ -371,6 +373,8 @@ enum rsc_handling_status {
> > > * @get_boot_addr: get boot address to entry point specified in
> > > firmware
> > > */
> > > struct rproc_ops {
> > > + int (*prepare)(struct rproc *rproc);
> > > + void (*unprepare)(struct rproc *rproc);
> > > int (*start)(struct rproc *rproc);
> > > int (*stop)(struct rproc *rproc);
> > > void (*kick)(struct rproc *rproc, int vqid);
> > > --
> > > 2.24.0
> > >
>
>

2020-01-21 10:27:02

by Arnaud POULIQUEN

[permalink] [raw]
Subject: Re: [PATCH v4 3/5] remoteproc: Add prepare/unprepare callbacks

Hi Bjorn,Paul,

On 1/20/20 9:19 PM, Bjorn Andersson wrote:
> On Wed 15 Jan 13:15 PST 2020, Paul Cercueil wrote:
>
>> Hi Bjorn,
>>
>>
>> Le sam., déc. 21, 2019 at 12:20, Bjorn Andersson
>> <[email protected]> a écrit :
>>> On Tue 10 Dec 08:40 PST 2019, Paul Cercueil wrote:
>>>
>>>> The .prepare() callback is called before the firmware is loaded to
>>>> memory. This is useful for instance in the case where some setup is
>>>> required for the memory to be accessible.
>>>>
>>>
>>> Would it make sense to somehow tie this prepare/unprepare to the actual
>>> struct rproc_mem_entry that needs the resource enabled?
>>
>> Do you need such granularity?
>>
>
> I don't have such needs, but given some of the memory structure that
> Suman and Loic has been talking about I would expect that such need
> exists
>
>> In my case, the three memories need the same clock to be enabled.
>>
>
> But we can update your driver to associate your one clock with the
> memory objects if/when we end up implementing this later.

IMHO, as mention Fabien the prepare/unprepare should probably not in the stop/start
but in rproc_fw_boot and rproc_resource_cleanup. This should cover more usecases (such as power domain) and
avoid a unprepare/prepare on crash recovery.

Nevertheless I'm wondering if it's more like PM runtime that should be implemented here...?

Regards,
Arnaud

>
> Regards,
> Bjorn
>
>> -Paul
>>
>>
>>>
>>>> Signed-off-by: Paul Cercueil <[email protected]>
>>>> ---
>>>>
>>>> Notes:
>>>> v2-v4: No change
>>>>
>>>> drivers/remoteproc/remoteproc_core.c | 16 +++++++++++++++-
>>>> include/linux/remoteproc.h | 4 ++++
>>>> 2 files changed, 19 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/remoteproc/remoteproc_core.c
>>>> b/drivers/remoteproc/remoteproc_core.c
>>>> index 0a9fc7fdd1c3..3ea5f675a148 100644
>>>> --- a/drivers/remoteproc/remoteproc_core.c
>>>> +++ b/drivers/remoteproc/remoteproc_core.c
>>>> @@ -1299,11 +1299,19 @@ static int rproc_start(struct rproc *rproc,
>>>> const struct firmware *fw)
>>>> struct device *dev = &rproc->dev;
>>>> int ret;
>>>>
>>>> + if (rproc->ops->prepare) {
>>>> + ret = rproc->ops->prepare(rproc);
>>>> + if (ret) {
>>>> + dev_err(dev, "Failed to prepare rproc: %d\n", ret);
>>>> + return ret;
>>>> + }
>>>> + }
>>>> +

>>>> /* load the ELF segments to memory */
>>>> ret = rproc_load_segments(rproc, fw);
>>>> if (ret) {
>>>> dev_err(dev, "Failed to load program segments: %d\n", ret);
>>>> - return ret;
>>>> + goto unprepare_rproc;
>>>> }
>>>>
>>>> /*
>>>> @@ -1354,6 +1362,9 @@ static int rproc_start(struct rproc *rproc,
>>>> const struct firmware *fw)
>>>> rproc_unprepare_subdevices(rproc);
>>>> reset_table_ptr:
>>>> rproc->table_ptr = rproc->cached_table;
>>>> +unprepare_rproc:
>>>> + if (rproc->ops->unprepare)
>>>> + rproc->ops->unprepare(rproc);
>>>>
>>>> return ret;
>>>> }
>>>> @@ -1483,6 +1494,9 @@ static int rproc_stop(struct rproc *rproc,
>>>> bool crashed)
>>>>
>>>> rproc->state = RPROC_OFFLINE;
>>>>
>>>> + if (rproc->ops->unprepare)
>>>> + rproc->ops->unprepare(rproc);
>>>> +
>>>> dev_info(dev, "stopped remote processor %s\n", rproc->name);
>>>>
>>>> return 0;
>>>> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
>>>> index 5f201f0c86c3..a6272d1ba384 100644
>>>> --- a/include/linux/remoteproc.h
>>>> +++ b/include/linux/remoteproc.h
>>>> @@ -355,6 +355,8 @@ enum rsc_handling_status {
>>>>
>>>> /**
>>>> * struct rproc_ops - platform-specific device handlers
>>>> + * @prepare: prepare the device for power up (before the firmware
>>>> is loaded)
>>>> + * @unprepare: unprepare the device after it is stopped
>>>> * @start: power on the device and boot it
>>>> * @stop: power off the device
>>>> * @kick: kick a virtqueue (virtqueue id given as a parameter)
>>>> @@ -371,6 +373,8 @@ enum rsc_handling_status {
>>>> * @get_boot_addr: get boot address to entry point specified in
>>>> firmware
>>>> */
>>>> struct rproc_ops {
>>>> + int (*prepare)(struct rproc *rproc);
>>>> + void (*unprepare)(struct rproc *rproc);
>>>> int (*start)(struct rproc *rproc);
>>>> int (*stop)(struct rproc *rproc);
>>>> void (*kick)(struct rproc *rproc, int vqid);
>>>> --
>>>> 2.24.0
>>>>
>>
>>