* [PATCH 0/3] Defer TI's Remoteproc's Probe until Mailbox is Probed
@ 2024-05-30 9:07 Beleswar Padhi
2024-05-30 9:07 ` [PATCH 1/3] remoteproc: k3-r5: Use devm_rproc_alloc() helper Beleswar Padhi
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Beleswar Padhi @ 2024-05-30 9:07 UTC (permalink / raw)
To: andersson, mathieu.poirier
Cc: hnagalla, u-kumar1, afd, linux-remoteproc, linux-kernel
Hello All,
This series adds deferred probe functionality in the TI's Remoteproc
drivers. The remoteproc drivers are dependent on the omap-mailbox driver
for mbox functionalities. Sometimes, the remoteproc driver could be
probed before the mailbox driver leading to rproc boot failures. Thus,
defer the probe routine of remoteproc drivers until mailbox driver is
probed by checking the mbox_request_channel handle in probe.
Also, use the acquired mbox handle in probe during rproc start/attach
routine instead of re-requesting. Do not free mbox handle during
stop/detach routine or error paths. This makes our k3_rproc_attach() &
k3_rproc_detach() functions NOP.
Also, use the devm_rproc_alloc() helper to automatically free created
rprocs incase of a probe defer.
Beleswar Padhi (3):
remoteproc: k3-r5: Use devm_rproc_alloc() helper
remoteproc: k3-r5: Acquire mailbox handle during probe
remoteproc: k3-dsp: Acquire mailbox handle during probe routine
drivers/remoteproc/ti_k3_dsp_remoteproc.c | 67 +++++++--------------
drivers/remoteproc/ti_k3_r5_remoteproc.c | 72 ++++++++---------------
2 files changed, 44 insertions(+), 95 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] remoteproc: k3-r5: Use devm_rproc_alloc() helper
2024-05-30 9:07 [PATCH 0/3] Defer TI's Remoteproc's Probe until Mailbox is Probed Beleswar Padhi
@ 2024-05-30 9:07 ` Beleswar Padhi
2024-05-30 9:07 ` [PATCH 2/3] remoteproc: k3-r5: Acquire mailbox handle during probe Beleswar Padhi
2024-05-30 9:07 ` [PATCH 3/3] remoteproc: k3-dsp: Acquire mailbox handle during probe routine Beleswar Padhi
2 siblings, 0 replies; 6+ messages in thread
From: Beleswar Padhi @ 2024-05-30 9:07 UTC (permalink / raw)
To: andersson, mathieu.poirier
Cc: hnagalla, u-kumar1, afd, linux-remoteproc, linux-kernel
Use the device lifecycle managed allocation function. This helps prevent
mistakes like freeing out of order in cleanup functions and forgetting
to free on error paths.
Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 50e486bcfa10..26362a509ae3 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -1258,8 +1258,8 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev)
goto out;
}
- rproc = rproc_alloc(cdev, dev_name(cdev), &k3_r5_rproc_ops,
- fw_name, sizeof(*kproc));
+ rproc = devm_rproc_alloc(cdev, dev_name(cdev), &k3_r5_rproc_ops,
+ fw_name, sizeof(*kproc));
if (!rproc) {
ret = -ENOMEM;
goto out;
@@ -1351,7 +1351,6 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev)
err_add:
k3_r5_reserved_mem_exit(kproc);
err_config:
- rproc_free(rproc);
core->rproc = NULL;
out:
/* undo core0 upon any failures on core1 in split-mode */
@@ -1398,7 +1397,6 @@ static void k3_r5_cluster_rproc_exit(void *data)
k3_r5_reserved_mem_exit(kproc);
- rproc_free(rproc);
core->rproc = NULL;
}
}
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/3] remoteproc: k3-r5: Acquire mailbox handle during probe
2024-05-30 9:07 [PATCH 0/3] Defer TI's Remoteproc's Probe until Mailbox is Probed Beleswar Padhi
2024-05-30 9:07 ` [PATCH 1/3] remoteproc: k3-r5: Use devm_rproc_alloc() helper Beleswar Padhi
@ 2024-05-30 9:07 ` Beleswar Padhi
2024-05-30 14:16 ` Andrew Davis
2024-05-30 9:07 ` [PATCH 3/3] remoteproc: k3-dsp: Acquire mailbox handle during probe routine Beleswar Padhi
2 siblings, 1 reply; 6+ messages in thread
From: Beleswar Padhi @ 2024-05-30 9:07 UTC (permalink / raw)
To: andersson, mathieu.poirier
Cc: hnagalla, u-kumar1, afd, linux-remoteproc, linux-kernel
Acquire the mailbox handle during device probe and do not release handle
in stop/detach routine or error paths. This removes the redundant
requests for mbox handle later during rproc start/attach. This also
allows to defer remoteproc driver's probe if mailbox is not probed yet.
Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 66 ++++++++----------------
1 file changed, 21 insertions(+), 45 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 26362a509ae3..157e8fd57665 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -391,6 +391,7 @@ static int k3_r5_rproc_request_mbox(struct rproc *rproc)
struct mbox_client *client = &kproc->client;
struct device *dev = kproc->dev;
int ret;
+ long err;
client->dev = dev;
client->tx_done = NULL;
@@ -400,10 +401,9 @@ static int k3_r5_rproc_request_mbox(struct rproc *rproc)
kproc->mbox = mbox_request_channel(client, 0);
if (IS_ERR(kproc->mbox)) {
- ret = -EBUSY;
- dev_err(dev, "mbox_request_channel failed: %ld\n",
- PTR_ERR(kproc->mbox));
- return ret;
+ err = PTR_ERR(kproc->mbox);
+ dev_err(dev, "mbox_request_channel failed: %ld\n", err);
+ return (err == -EPROBE_DEFER) ? -EPROBE_DEFER : -EBUSY;
}
/*
@@ -552,10 +552,6 @@ static int k3_r5_rproc_start(struct rproc *rproc)
u32 boot_addr;
int ret;
- ret = k3_r5_rproc_request_mbox(rproc);
- if (ret)
- return ret;
-
boot_addr = rproc->bootaddr;
/* TODO: add boot_addr sanity checking */
dev_dbg(dev, "booting R5F core using boot addr = 0x%x\n", boot_addr);
@@ -564,7 +560,7 @@ static int k3_r5_rproc_start(struct rproc *rproc)
core = kproc->core;
ret = ti_sci_proc_set_config(core->tsp, boot_addr, 0, 0);
if (ret)
- goto put_mbox;
+ goto out;
/* unhalt/run all applicable cores */
if (cluster->mode == CLUSTER_MODE_LOCKSTEP) {
@@ -581,12 +577,12 @@ static int k3_r5_rproc_start(struct rproc *rproc)
dev_err(dev, "%s: can not start core 1 before core 0\n",
__func__);
ret = -EPERM;
- goto put_mbox;
+ goto out;
}
ret = k3_r5_core_run(core);
if (ret)
- goto put_mbox;
+ goto out;
}
return 0;
@@ -596,8 +592,7 @@ static int k3_r5_rproc_start(struct rproc *rproc)
if (k3_r5_core_halt(core))
dev_warn(core->dev, "core halt back failed\n");
}
-put_mbox:
- mbox_free_channel(kproc->mbox);
+out:
return ret;
}
@@ -658,8 +653,6 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
goto out;
}
- mbox_free_channel(kproc->mbox);
-
return 0;
unroll_core_halt:
@@ -674,42 +667,21 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
/*
* Attach to a running R5F remote processor (IPC-only mode)
*
- * The R5F attach callback only needs to request the mailbox, the remote
- * processor is already booted, so there is no need to issue any TI-SCI
- * commands to boot the R5F cores in IPC-only mode. This callback is invoked
- * only in IPC-only mode.
+ * The R5F attach callback is a NOP. The remote processor is already booted, and
+ * all required resources have been acquired during probe routine, so there is
+ * no need to issue any TI-SCI commands to boot the R5F cores in IPC-only mode.
+ * This callback is invoked only in IPC-only mode and exists for sanity sake.
*/
-static int k3_r5_rproc_attach(struct rproc *rproc)
-{
- struct k3_r5_rproc *kproc = rproc->priv;
- struct device *dev = kproc->dev;
- int ret;
-
- ret = k3_r5_rproc_request_mbox(rproc);
- if (ret)
- return ret;
-
- dev_info(dev, "R5F core initialized in IPC-only mode\n");
- return 0;
-}
+static int k3_r5_rproc_attach(struct rproc *rproc) { return 0; }
/*
* Detach from a running R5F remote processor (IPC-only mode)
*
- * The R5F detach callback performs the opposite operation to attach callback
- * and only needs to release the mailbox, the R5F cores are not stopped and
- * will be left in booted state in IPC-only mode. This callback is invoked
- * only in IPC-only mode.
+ * The R5F detach callback is a NOP. The R5F cores are not stopped and will be
+ * left in booted state in IPC-only mode. This callback is invoked only in
+ * IPC-only mode and exists for sanity sake.
*/
-static int k3_r5_rproc_detach(struct rproc *rproc)
-{
- struct k3_r5_rproc *kproc = rproc->priv;
- struct device *dev = kproc->dev;
-
- mbox_free_channel(kproc->mbox);
- dev_info(dev, "R5F core deinitialized in IPC-only mode\n");
- return 0;
-}
+static int k3_r5_rproc_detach(struct rproc *rproc) { return 0; }
/*
* This function implements the .get_loaded_rsc_table() callback and is used
@@ -1277,6 +1249,10 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev)
kproc->rproc = rproc;
core->rproc = rproc;
+ ret = k3_r5_rproc_request_mbox(rproc);
+ if (ret)
+ return ret;
+
ret = k3_r5_rproc_configure_mode(kproc);
if (ret < 0)
goto err_config;
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/3] remoteproc: k3-dsp: Acquire mailbox handle during probe routine
2024-05-30 9:07 [PATCH 0/3] Defer TI's Remoteproc's Probe until Mailbox is Probed Beleswar Padhi
2024-05-30 9:07 ` [PATCH 1/3] remoteproc: k3-r5: Use devm_rproc_alloc() helper Beleswar Padhi
2024-05-30 9:07 ` [PATCH 2/3] remoteproc: k3-r5: Acquire mailbox handle during probe Beleswar Padhi
@ 2024-05-30 9:07 ` Beleswar Padhi
2 siblings, 0 replies; 6+ messages in thread
From: Beleswar Padhi @ 2024-05-30 9:07 UTC (permalink / raw)
To: andersson, mathieu.poirier
Cc: hnagalla, u-kumar1, afd, linux-remoteproc, linux-kernel
Acquire the mailbox handle during device probe and do not release handle
in stop/detach routine or error paths. This removes the redundant
requests for mbox handle later during rproc start/attach. This also
allows to defer remoteproc driver's probe if mailbox is not probed yet.
Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
---
drivers/remoteproc/ti_k3_dsp_remoteproc.c | 67 +++++++----------------
1 file changed, 21 insertions(+), 46 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c
index 3555b535b168..88cda609a5eb 100644
--- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c
@@ -222,6 +222,7 @@ static int k3_dsp_rproc_request_mbox(struct rproc *rproc)
struct mbox_client *client = &kproc->client;
struct device *dev = kproc->dev;
int ret;
+ long err;
client->dev = dev;
client->tx_done = NULL;
@@ -231,10 +232,9 @@ static int k3_dsp_rproc_request_mbox(struct rproc *rproc)
kproc->mbox = mbox_request_channel(client, 0);
if (IS_ERR(kproc->mbox)) {
- ret = -EBUSY;
- dev_err(dev, "mbox_request_channel failed: %ld\n",
- PTR_ERR(kproc->mbox));
- return ret;
+ err = PTR_ERR(kproc->mbox);
+ dev_err(dev, "mbox_request_channel failed: %ld\n", err);
+ return (err == -EPROBE_DEFER) ? -EPROBE_DEFER : -EBUSY;
}
/*
@@ -315,31 +315,25 @@ static int k3_dsp_rproc_start(struct rproc *rproc)
u32 boot_addr;
int ret;
- ret = k3_dsp_rproc_request_mbox(rproc);
- if (ret)
- return ret;
-
boot_addr = rproc->bootaddr;
if (boot_addr & (kproc->data->boot_align_addr - 1)) {
dev_err(dev, "invalid boot address 0x%x, must be aligned on a 0x%x boundary\n",
boot_addr, kproc->data->boot_align_addr);
ret = -EINVAL;
- goto put_mbox;
+ goto out;
}
dev_err(dev, "booting DSP core using boot addr = 0x%x\n", boot_addr);
ret = ti_sci_proc_set_config(kproc->tsp, boot_addr, 0, 0);
if (ret)
- goto put_mbox;
+ goto out;
ret = k3_dsp_rproc_release(kproc);
if (ret)
- goto put_mbox;
+ goto out;
return 0;
-
-put_mbox:
- mbox_free_channel(kproc->mbox);
+out:
return ret;
}
@@ -353,8 +347,6 @@ static int k3_dsp_rproc_stop(struct rproc *rproc)
{
struct k3_dsp_rproc *kproc = rproc->priv;
- mbox_free_channel(kproc->mbox);
-
k3_dsp_rproc_reset(kproc);
return 0;
@@ -363,42 +355,21 @@ static int k3_dsp_rproc_stop(struct rproc *rproc)
/*
* Attach to a running DSP remote processor (IPC-only mode)
*
- * This rproc attach callback only needs to request the mailbox, the remote
- * processor is already booted, so there is no need to issue any TI-SCI
- * commands to boot the DSP core. This callback is invoked only in IPC-only
- * mode.
+ * This rproc attach callback is a NOP. The remote processor is already booted,
+ * and all required resources have been acquired during probe routine, so there
+ * is no need to issue any TI-SCI commands to boot the DSP core. This callback
+ * is invoked only in IPC-only mode and exists for sanity sake.
*/
-static int k3_dsp_rproc_attach(struct rproc *rproc)
-{
- struct k3_dsp_rproc *kproc = rproc->priv;
- struct device *dev = kproc->dev;
- int ret;
-
- ret = k3_dsp_rproc_request_mbox(rproc);
- if (ret)
- return ret;
-
- dev_info(dev, "DSP initialized in IPC-only mode\n");
- return 0;
-}
+static int k3_dsp_rproc_attach(struct rproc *rproc) { return 0; }
/*
* Detach from a running DSP remote processor (IPC-only mode)
*
- * This rproc detach callback performs the opposite operation to attach callback
- * and only needs to release the mailbox, the DSP core is not stopped and will
- * be left to continue to run its booted firmware. This callback is invoked only
- * in IPC-only mode.
+ * This rproc detach callback is a NOP. The DSP core is not stopped and will be
+ * left to continue to run its booted firmware. This callback is invoked only in
+ * IPC-only mode and exists for sanity sake.
*/
-static int k3_dsp_rproc_detach(struct rproc *rproc)
-{
- struct k3_dsp_rproc *kproc = rproc->priv;
- struct device *dev = kproc->dev;
-
- mbox_free_channel(kproc->mbox);
- dev_info(dev, "DSP deinitialized in IPC-only mode\n");
- return 0;
-}
+static int k3_dsp_rproc_detach(struct rproc *rproc) { return 0; }
/*
* This function implements the .get_loaded_rsc_table() callback and is used
@@ -697,6 +668,10 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev)
kproc->dev = dev;
kproc->data = data;
+ ret = k3_dsp_rproc_request_mbox(rproc);
+ if (ret)
+ return ret;
+
kproc->ti_sci = devm_ti_sci_get_by_phandle(dev, "ti,sci");
if (IS_ERR(kproc->ti_sci))
return dev_err_probe(dev, PTR_ERR(kproc->ti_sci),
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] remoteproc: k3-r5: Acquire mailbox handle during probe
2024-05-30 9:07 ` [PATCH 2/3] remoteproc: k3-r5: Acquire mailbox handle during probe Beleswar Padhi
@ 2024-05-30 14:16 ` Andrew Davis
2024-06-04 5:24 ` Beleswar Prasad Padhi
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Davis @ 2024-05-30 14:16 UTC (permalink / raw)
To: Beleswar Padhi, andersson, mathieu.poirier
Cc: hnagalla, u-kumar1, linux-remoteproc, linux-kernel
On 5/30/24 4:07 AM, Beleswar Padhi wrote:
> Acquire the mailbox handle during device probe and do not release handle
> in stop/detach routine or error paths. This removes the redundant
> requests for mbox handle later during rproc start/attach. This also
> allows to defer remoteproc driver's probe if mailbox is not probed yet.
>
> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
> ---
> drivers/remoteproc/ti_k3_r5_remoteproc.c | 66 ++++++++----------------
> 1 file changed, 21 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c
> index 26362a509ae3..157e8fd57665 100644
> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
> @@ -391,6 +391,7 @@ static int k3_r5_rproc_request_mbox(struct rproc *rproc)
> struct mbox_client *client = &kproc->client;
> struct device *dev = kproc->dev;
> int ret;
> + long err;
>
> client->dev = dev;
> client->tx_done = NULL;
> @@ -400,10 +401,9 @@ static int k3_r5_rproc_request_mbox(struct rproc *rproc)
>
> kproc->mbox = mbox_request_channel(client, 0);
> if (IS_ERR(kproc->mbox)) {
> - ret = -EBUSY;
> - dev_err(dev, "mbox_request_channel failed: %ld\n",
> - PTR_ERR(kproc->mbox));
> - return ret;
> + err = PTR_ERR(kproc->mbox);
> + dev_err(dev, "mbox_request_channel failed: %ld\n", err);
> + return (err == -EPROBE_DEFER) ? -EPROBE_DEFER : -EBUSY;
Why turn all other errors into EBUSY? If you just return the error as-is you
can simply make these 3 lines just:
return dev_err_probe(dev, PTR_ERR(kproc->mbox), "mbox_request_channel failed\n");
> }
>
> /*
> @@ -552,10 +552,6 @@ static int k3_r5_rproc_start(struct rproc *rproc)
> u32 boot_addr;
> int ret;
>
> - ret = k3_r5_rproc_request_mbox(rproc);
> - if (ret)
> - return ret;
> -
> boot_addr = rproc->bootaddr;
> /* TODO: add boot_addr sanity checking */
> dev_dbg(dev, "booting R5F core using boot addr = 0x%x\n", boot_addr);
> @@ -564,7 +560,7 @@ static int k3_r5_rproc_start(struct rproc *rproc)
> core = kproc->core;
> ret = ti_sci_proc_set_config(core->tsp, boot_addr, 0, 0);
> if (ret)
> - goto put_mbox;
> + goto out;
The label "out" doesn't do anything, just directly `return ret;` here and
in the other cases below.
>
> /* unhalt/run all applicable cores */
> if (cluster->mode == CLUSTER_MODE_LOCKSTEP) {
> @@ -581,12 +577,12 @@ static int k3_r5_rproc_start(struct rproc *rproc)
> dev_err(dev, "%s: can not start core 1 before core 0\n",
> __func__);
> ret = -EPERM;
> - goto put_mbox;
> + goto out;
> }
>
> ret = k3_r5_core_run(core);
> if (ret)
> - goto put_mbox;
> + goto out;
> }
>
> return 0;
> @@ -596,8 +592,7 @@ static int k3_r5_rproc_start(struct rproc *rproc)
> if (k3_r5_core_halt(core))
> dev_warn(core->dev, "core halt back failed\n");
> }
> -put_mbox:
> - mbox_free_channel(kproc->mbox);
> +out:
> return ret;
> }
>
> @@ -658,8 +653,6 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
> goto out;
> }
>
> - mbox_free_channel(kproc->mbox);
> -
> return 0;
>
> unroll_core_halt:
> @@ -674,42 +667,21 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
> /*
> * Attach to a running R5F remote processor (IPC-only mode)
> *
> - * The R5F attach callback only needs to request the mailbox, the remote
> - * processor is already booted, so there is no need to issue any TI-SCI
> - * commands to boot the R5F cores in IPC-only mode. This callback is invoked
> - * only in IPC-only mode.
> + * The R5F attach callback is a NOP. The remote processor is already booted, and
> + * all required resources have been acquired during probe routine, so there is
> + * no need to issue any TI-SCI commands to boot the R5F cores in IPC-only mode.
> + * This callback is invoked only in IPC-only mode and exists for sanity sake.
> */
> -static int k3_r5_rproc_attach(struct rproc *rproc)
> -{
> - struct k3_r5_rproc *kproc = rproc->priv;
> - struct device *dev = kproc->dev;
> - int ret;
> -
> - ret = k3_r5_rproc_request_mbox(rproc);
> - if (ret)
> - return ret;
> -
> - dev_info(dev, "R5F core initialized in IPC-only mode\n");
> - return 0;
> -}
> +static int k3_r5_rproc_attach(struct rproc *rproc) { return 0; }
>
> /*
> * Detach from a running R5F remote processor (IPC-only mode)
> *
> - * The R5F detach callback performs the opposite operation to attach callback
> - * and only needs to release the mailbox, the R5F cores are not stopped and
> - * will be left in booted state in IPC-only mode. This callback is invoked
> - * only in IPC-only mode.
> + * The R5F detach callback is a NOP. The R5F cores are not stopped and will be
> + * left in booted state in IPC-only mode. This callback is invoked only in
> + * IPC-only mode and exists for sanity sake.
> */
> -static int k3_r5_rproc_detach(struct rproc *rproc)
> -{
> - struct k3_r5_rproc *kproc = rproc->priv;
> - struct device *dev = kproc->dev;
> -
> - mbox_free_channel(kproc->mbox);
> - dev_info(dev, "R5F core deinitialized in IPC-only mode\n");
> - return 0;
> -}
> +static int k3_r5_rproc_detach(struct rproc *rproc) { return 0; }
Do we still need to disable the mbox channel somehow here to prevent
receiving more messages from the detached core?
>
> /*
> * This function implements the .get_loaded_rsc_table() callback and is used
> @@ -1277,6 +1249,10 @@ static int k3_r5_cluster_rproc_init(struct platform_device *pdev)
> kproc->rproc = rproc;
> core->rproc = rproc;
>
> + ret = k3_r5_rproc_request_mbox(rproc);
Now that we get the channel here in init you'll want to add a matching
mbox_free_channel() call to k3_r5_cluster_rproc_exit().
Andrew
> + if (ret)
> + return ret;
> +
> ret = k3_r5_rproc_configure_mode(kproc);
> if (ret < 0)
> goto err_config;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] remoteproc: k3-r5: Acquire mailbox handle during probe
2024-05-30 14:16 ` Andrew Davis
@ 2024-06-04 5:24 ` Beleswar Prasad Padhi
0 siblings, 0 replies; 6+ messages in thread
From: Beleswar Prasad Padhi @ 2024-06-04 5:24 UTC (permalink / raw)
To: Andrew Davis, andersson, mathieu.poirier
Cc: hnagalla, u-kumar1, linux-remoteproc, linux-kernel
Hi Andrew,
On 30/05/24 19:46, Andrew Davis wrote:
> On 5/30/24 4:07 AM, Beleswar Padhi wrote:
>> Acquire the mailbox handle during device probe and do not release handle
>> in stop/detach routine or error paths. This removes the redundant
>> requests for mbox handle later during rproc start/attach. This also
>> allows to defer remoteproc driver's probe if mailbox is not probed yet.
>>
>> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
>> ---
>> drivers/remoteproc/ti_k3_r5_remoteproc.c | 66 ++++++++----------------
>> 1 file changed, 21 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
>> b/drivers/remoteproc/ti_k3_r5_remoteproc.c
>> index 26362a509ae3..157e8fd57665 100644
>> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
>> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
>> @@ -391,6 +391,7 @@ static int k3_r5_rproc_request_mbox(struct rproc
>> *rproc)
>> struct mbox_client *client = &kproc->client;
>> struct device *dev = kproc->dev;
>> int ret;
>> + long err;
>> client->dev = dev;
>> client->tx_done = NULL;
>> @@ -400,10 +401,9 @@ static int k3_r5_rproc_request_mbox(struct rproc
>> *rproc)
>> kproc->mbox = mbox_request_channel(client, 0);
>> if (IS_ERR(kproc->mbox)) {
>> - ret = -EBUSY;
>> - dev_err(dev, "mbox_request_channel failed: %ld\n",
>> - PTR_ERR(kproc->mbox));
>> - return ret;
>> + err = PTR_ERR(kproc->mbox);
>> + dev_err(dev, "mbox_request_channel failed: %ld\n", err);
>> + return (err == -EPROBE_DEFER) ? -EPROBE_DEFER : -EBUSY;
>
> Why turn all other errors into EBUSY? If you just return the error
> as-is you
> can simply make these 3 lines just:
>
> return dev_err_probe(dev, PTR_ERR(kproc->mbox), "mbox_request_channel
> failed\n");
>
>> }
>> /*
>> @@ -552,10 +552,6 @@ static int k3_r5_rproc_start(struct rproc *rproc)
>> u32 boot_addr;
>> int ret;
>> - ret = k3_r5_rproc_request_mbox(rproc);
>> - if (ret)
>> - return ret;
>> -
>> boot_addr = rproc->bootaddr;
>> /* TODO: add boot_addr sanity checking */
>> dev_dbg(dev, "booting R5F core using boot addr = 0x%x\n",
>> boot_addr);
>> @@ -564,7 +560,7 @@ static int k3_r5_rproc_start(struct rproc *rproc)
>> core = kproc->core;
>> ret = ti_sci_proc_set_config(core->tsp, boot_addr, 0, 0);
>> if (ret)
>> - goto put_mbox;
>> + goto out;
>
> The label "out" doesn't do anything, just directly `return ret;` here and
> in the other cases below.
>
>> /* unhalt/run all applicable cores */
>> if (cluster->mode == CLUSTER_MODE_LOCKSTEP) {
>> @@ -581,12 +577,12 @@ static int k3_r5_rproc_start(struct rproc *rproc)
>> dev_err(dev, "%s: can not start core 1 before core 0\n",
>> __func__);
>> ret = -EPERM;
>> - goto put_mbox;
>> + goto out;
>> }
>> ret = k3_r5_core_run(core);
>> if (ret)
>> - goto put_mbox;
>> + goto out;
>> }
>> return 0;
>> @@ -596,8 +592,7 @@ static int k3_r5_rproc_start(struct rproc *rproc)
>> if (k3_r5_core_halt(core))
>> dev_warn(core->dev, "core halt back failed\n");
>> }
>> -put_mbox:
>> - mbox_free_channel(kproc->mbox);
>> +out:
>> return ret;
>> }
>> @@ -658,8 +653,6 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
>> goto out;
>> }
>> - mbox_free_channel(kproc->mbox);
>> -
>> return 0;
>> unroll_core_halt:
>> @@ -674,42 +667,21 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
>> /*
>> * Attach to a running R5F remote processor (IPC-only mode)
>> *
>> - * The R5F attach callback only needs to request the mailbox, the
>> remote
>> - * processor is already booted, so there is no need to issue any TI-SCI
>> - * commands to boot the R5F cores in IPC-only mode. This callback is
>> invoked
>> - * only in IPC-only mode.
>> + * The R5F attach callback is a NOP. The remote processor is already
>> booted, and
>> + * all required resources have been acquired during probe routine,
>> so there is
>> + * no need to issue any TI-SCI commands to boot the R5F cores in
>> IPC-only mode.
>> + * This callback is invoked only in IPC-only mode and exists for
>> sanity sake.
>> */
>> -static int k3_r5_rproc_attach(struct rproc *rproc)
>> -{
>> - struct k3_r5_rproc *kproc = rproc->priv;
>> - struct device *dev = kproc->dev;
>> - int ret;
>> -
>> - ret = k3_r5_rproc_request_mbox(rproc);
>> - if (ret)
>> - return ret;
>> -
>> - dev_info(dev, "R5F core initialized in IPC-only mode\n");
>> - return 0;
>> -}
>> +static int k3_r5_rproc_attach(struct rproc *rproc) { return 0; }
>> /*
>> * Detach from a running R5F remote processor (IPC-only mode)
>> *
>> - * The R5F detach callback performs the opposite operation to attach
>> callback
>> - * and only needs to release the mailbox, the R5F cores are not
>> stopped and
>> - * will be left in booted state in IPC-only mode. This callback is
>> invoked
>> - * only in IPC-only mode.
>> + * The R5F detach callback is a NOP. The R5F cores are not stopped
>> and will be
>> + * left in booted state in IPC-only mode. This callback is invoked
>> only in
>> + * IPC-only mode and exists for sanity sake.
>> */
>> -static int k3_r5_rproc_detach(struct rproc *rproc)
>> -{
>> - struct k3_r5_rproc *kproc = rproc->priv;
>> - struct device *dev = kproc->dev;
>> -
>> - mbox_free_channel(kproc->mbox);
>> - dev_info(dev, "R5F core deinitialized in IPC-only mode\n");
>> - return 0;
>> -}
>> +static int k3_r5_rproc_detach(struct rproc *rproc) { return 0; }
>
> Do we still need to disable the mbox channel somehow here to prevent
> receiving more messages from the detached core?
>
>> /*
>> * This function implements the .get_loaded_rsc_table() callback
>> and is used
>> @@ -1277,6 +1249,10 @@ static int k3_r5_cluster_rproc_init(struct
>> platform_device *pdev)
>> kproc->rproc = rproc;
>> core->rproc = rproc;
>> + ret = k3_r5_rproc_request_mbox(rproc);
>
> Now that we get the channel here in init you'll want to add a matching
> mbox_free_channel() call to k3_r5_cluster_rproc_exit().
>
> Andrew
Thanks for the review! I have sent out v2 addressing these comments:
https://lore.kernel.org/all/20240604051722.3608750-1-b-padhi@ti.com/
>
>> + if (ret)
>> + return ret;
>> +
>> ret = k3_r5_rproc_configure_mode(kproc);
>> if (ret < 0)
>> goto err_config;
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-06-04 5:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-30 9:07 [PATCH 0/3] Defer TI's Remoteproc's Probe until Mailbox is Probed Beleswar Padhi
2024-05-30 9:07 ` [PATCH 1/3] remoteproc: k3-r5: Use devm_rproc_alloc() helper Beleswar Padhi
2024-05-30 9:07 ` [PATCH 2/3] remoteproc: k3-r5: Acquire mailbox handle during probe Beleswar Padhi
2024-05-30 14:16 ` Andrew Davis
2024-06-04 5:24 ` Beleswar Prasad Padhi
2024-05-30 9:07 ` [PATCH 3/3] remoteproc: k3-dsp: Acquire mailbox handle during probe routine Beleswar Padhi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®