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 <paul@xxxxxxxxxxxxxxx> > --- > > 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);