| Message ID | 20240807062256.1721682-2-b-padhi@ti.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Defer TI's Remoteproc's Probe until Mailbox is Probed | expand |
On 8/7/24 1:22 AM, Beleswar Padhi wrote: > 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 39a47540c590..dbcd8840ae8d 100644 > --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c > +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c > @@ -1259,8 +1259,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; > @@ -1352,7 +1352,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; I'd remove this line too, we don't check for NULL later, we check core->rproc->status, which will cause a nullptr dereference error instead of correctly detecting that the core is "offline". Same below. Andrew > out: > /* undo core0 upon any failures on core1 in split-mode */ > @@ -1399,7 +1398,6 @@ static void k3_r5_cluster_rproc_exit(void *data) > > k3_r5_reserved_mem_exit(kproc); > > - rproc_free(rproc); > core->rproc = NULL; > } > }
Hi Andrew, On 07/08/24 19:07, Andrew Davis wrote: > On 8/7/24 1:22 AM, Beleswar Padhi wrote: >> 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 39a47540c590..dbcd8840ae8d 100644 >> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c >> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c >> @@ -1259,8 +1259,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; >> @@ -1352,7 +1352,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; > > I'd remove this line too, we don't check for NULL later, we check > core->rproc->status, which will cause a nullptr dereference error > instead of correctly detecting that the core is "offline". Thanks for catching this! Will post revision with this change. > > Same below. > > Andrew > >> out: >> /* undo core0 upon any failures on core1 in split-mode */ >> @@ -1399,7 +1398,6 @@ static void k3_r5_cluster_rproc_exit(void *data) >> k3_r5_reserved_mem_exit(kproc); >> - rproc_free(rproc); >> core->rproc = NULL; >> } >> }
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c index 39a47540c590..dbcd8840ae8d 100644 --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c @@ -1259,8 +1259,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; @@ -1352,7 +1352,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 */ @@ -1399,7 +1398,6 @@ static void k3_r5_cluster_rproc_exit(void *data) k3_r5_reserved_mem_exit(kproc); - rproc_free(rproc); core->rproc = NULL; } }
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(-)