On 4/22/25 2:10 PM, Mathieu Poirier wrote:
On Tue, 22 Apr 2025 at 12:30, Tanmay Shah <tanmay.s...@amd.com> wrote:
On 4/22/25 12:49 PM, Mathieu Poirier wrote:
On Tue, 22 Apr 2025 at 10:10, Tanmay Shah <tanmay.s...@amd.com> wrote:
On 4/22/25 10:59 AM, Mathieu Poirier wrote:
Good morning,
On Mon, Apr 14, 2025 at 11:46:01AM -0700, Tanmay Shah wrote:
Powering off RPU using force_pwrdwn call results in system failure
if there are multiple users of that RPU node. Better mechanism is to use
request_node and release_node EEMI calls. With use of these EEMI calls,
platform management controller will take-care of powering off RPU
when there is no user.
Signed-off-by: Tanmay Shah <tanmay.s...@amd.com>
---
drivers/remoteproc/xlnx_r5_remoteproc.c | 29 ++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/remoteproc/xlnx_r5_remoteproc.c
b/drivers/remoteproc/xlnx_r5_remoteproc.c
index 5aeedeaf3c41..3597359c0fc8 100644
--- a/drivers/remoteproc/xlnx_r5_remoteproc.c
+++ b/drivers/remoteproc/xlnx_r5_remoteproc.c
@@ -380,6 +380,18 @@ static int zynqmp_r5_rproc_start(struct rproc *rproc)
dev_dbg(r5_core->dev, "RPU boot addr 0x%llx from %s.", rproc->bootaddr,
bootmem == PM_RPU_BOOTMEM_HIVEC ? "OCM" : "TCM");
+ /* Request node before starting RPU core if new version of API is
supported */
+ if (zynqmp_pm_feature(PM_REQUEST_NODE) > 1) {
+ ret = zynqmp_pm_request_node(r5_core->pm_domain_id,
+ ZYNQMP_PM_CAPABILITY_ACCESS, 0,
+ ZYNQMP_PM_REQUEST_ACK_BLOCKING);
+ if (ret < 0) {
+ dev_err(r5_core->dev, "failed to request 0x%x",
+ r5_core->pm_domain_id);
+ return ret;
+ }
+ }
+
ret = zynqmp_pm_request_wake(r5_core->pm_domain_id, 1,
bootmem, ZYNQMP_PM_REQUEST_ACK_NO);
if (ret)
@@ -401,10 +413,25 @@ static int zynqmp_r5_rproc_stop(struct rproc *rproc)
struct zynqmp_r5_core *r5_core = rproc->priv;
int ret;
+ /* Use release node API to stop core if new version of API is supported */
+ if (zynqmp_pm_feature(PM_RELEASE_NODE) > 1) {
+ ret = zynqmp_pm_release_node(r5_core->pm_domain_id);
+ if (ret)
+ dev_err(r5_core->dev, "failed to stop remoteproc RPU
%d\n", ret);
+ return ret;
+ }
+
+ if (zynqmp_pm_feature(PM_FORCE_POWERDOWN) < 1) {
+ dev_dbg(r5_core->dev, "EEMI interface %d not supported\n",
+ PM_FORCE_POWERDOWN);
+ return -EOPNOTSUPP;
+ }
Here I have to guess, because it is not documented, that it is the check to see
if zynqmp_pm_force_pwrdwn() is available. I'm not sure why it is needed because
zynqmp_pm_force_pwrdwn() returns and error code.
Hello,
Thanks for reviews. Yes you are correct. Actually instead, the check
should be for version 1 of PM_FORCE_POWER_DOWN. If version 1 is
supported, only then execute the call otherwise print the error.
Hence, the check should be something like:
if (zynqmp_pm_feature(PM_FORCE_POWERDOWN) != 1) {
error out.
}
The above still doesn't answer my question, i.e _why_ is a check
needed when zynqmp_pm_force_pwrdwn() returns an error code? To me, if
something happens in zynqmp_pm_force_pwrdwn() then an error code is
reported and the current implementation is able to deal with it.
PM_FORCE_POWERDOWN will print redundant error messages from firmware if
called for feature that is not supported. By doing above version check,
we are avoiding those unnecessary error/warning messages. Other than
that, you are correct we don't need to do version check as
PM_FORCE_POWERDOWN will send respective error code and we will fail
here. But version check helps to differentiate between actual error log
from firmware when call is expected to work.
That is the kind of information that would be useful as comments in
the code. Otherwise there is simply no way to tell...
Yes that makes sense. I will update comment accordingly.
I will fix and add comment as well.
Thanks,
Mathieu
+
+ /* maintain force pwr down for backward compatibility */
ret = zynqmp_pm_force_pwrdwn(r5_core->pm_domain_id,
ZYNQMP_PM_REQUEST_ACK_BLOCKING);
if (ret)
- dev_err(r5_core->dev, "failed to stop remoteproc RPU %d\n", ret);
+ dev_err(r5_core->dev, "core force power down failed\n");
return ret;
}
base-commit: 8532691d0a85ab2a826808207e904f7d62a9d804
--
2.34.1
