Re: [PATCH 3/5] remoteproc: k3-r5: Add devm action to release tsp

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Andrew,

On 17/12/24 21:30, Andrew Davis wrote:
On 12/4/24 5:11 AM, Beleswar Padhi wrote:
Use a device lifecycle managed action to release tsp ti_sci_proc handle.
This helps prevent mistakes like releasing out of order in cleanup
functions and forgetting to release on error paths.

Signed-off-by: Beleswar Padhi <b-padhi@xxxxxx>
---
  drivers/remoteproc/ti_k3_r5_remoteproc.c | 17 +++++++++++------
  1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 0753a5c35c7e..2966cb210403 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -1513,6 +1513,13 @@ static int k3_r5_core_of_get_sram_memories(struct platform_device *pdev,
      return 0;
  }
  +static void k3_r5_release_tsp(void *data)
+{
+    struct ti_sci_proc *tsp = data;
+
+    ti_sci_proc_release(tsp);
+}
+
  static int k3_r5_core_of_init(struct platform_device *pdev)
  {
      struct device *dev = &pdev->dev;
@@ -1606,6 +1613,10 @@ static int k3_r5_core_of_init(struct platform_device *pdev)
          goto err;
      }
  +    ret = devm_add_action_or_reset(dev, k3_r5_release_tsp, core->tsp);
+    if (ret)
+        goto err;
+
      platform_set_drvdata(pdev, core);
      devres_close_group(dev, k3_r5_core_of_init);
  @@ -1622,13 +1633,7 @@ static int k3_r5_core_of_init(struct platform_device *pdev)
   */
  static void k3_r5_core_of_exit(struct platform_device *pdev)
  {
-    struct k3_r5_core *core = platform_get_drvdata(pdev);
      struct device *dev = &pdev->dev;
-    int ret;
-
-    ret = ti_sci_proc_release(core->tsp);
-    if (ret)
-        dev_err(dev, "failed to release proc, ret = %d\n", ret);

One thing to remember is devm unrolling happens after remove(). So
here you are changing the order things happen. ti_sci_proc_release()
now will get called after the below functions. This most likely
isn't wrong, but to make review easier it helps to start from the
last called function in remove() and work backwards so nothing
is reordered.


That's a great insight! Will send out v2 following this order.

Thanks,
Beleswar


Andrew

      platform_set_drvdata(pdev, NULL);
      devres_release_group(dev, k3_r5_core_of_init);




[Index of Archives]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Photo Sharing]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux