| Message ID | 20240407072330.229076-1-pvkumar5749404@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [next] drivers: soc: qcom: Auto cleanup using __free(device_node) | expand |
Are there any comments for me regarding the patch ? Please let me know On Sun, Apr 7, 2024 at 12:53 PM Prabhav Kumar Vaish <pvkumar5749404@gmail.com> wrote: > > Use automated cleanup to replace of_node_put() in qcom_smem_resolve_mem(). > > Signed-off-by: Prabhav Kumar Vaish <pvkumar5749404@gmail.com> > --- > drivers/soc/qcom/smem.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c > index 7191fa0c087f..ad1cf8dcc6ec 100644 > --- a/drivers/soc/qcom/smem.c > +++ b/drivers/soc/qcom/smem.c > @@ -1032,18 +1032,16 @@ static int qcom_smem_resolve_mem(struct qcom_smem *smem, const char *name, > struct smem_region *region) > { > struct device *dev = smem->dev; > - struct device_node *np; > struct resource r; > int ret; > + struct device_node *np __free(device_node) = of_parse_phandle(dev->of_node, name, 0); > > - np = of_parse_phandle(dev->of_node, name, 0); > if (!np) { > dev_err(dev, "No %s specified\n", name); > return -EINVAL; > } > > ret = of_address_to_resource(np, 0, &r); > - of_node_put(np); > if (ret) > return ret; > > -- > 2.34.1 >
On Sun, Apr 07, 2024 at 12:53:30PM +0530, Prabhav Kumar Vaish wrote: > Use automated cleanup to replace of_node_put() in qcom_smem_resolve_mem(). > I don't find this easier to read or maintain. Also, your subject prefix does not match other changes to this driver. Regards, Bjorn > Signed-off-by: Prabhav Kumar Vaish <pvkumar5749404@gmail.com> > --- > drivers/soc/qcom/smem.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c > index 7191fa0c087f..ad1cf8dcc6ec 100644 > --- a/drivers/soc/qcom/smem.c > +++ b/drivers/soc/qcom/smem.c > @@ -1032,18 +1032,16 @@ static int qcom_smem_resolve_mem(struct qcom_smem *smem, const char *name, > struct smem_region *region) > { > struct device *dev = smem->dev; > - struct device_node *np; > struct resource r; > int ret; > + struct device_node *np __free(device_node) = of_parse_phandle(dev->of_node, name, 0); > > - np = of_parse_phandle(dev->of_node, name, 0); > if (!np) { > dev_err(dev, "No %s specified\n", name); > return -EINVAL; > } > > ret = of_address_to_resource(np, 0, &r); > - of_node_put(np); > if (ret) > return ret; > > -- > 2.34.1 >
On Wed, May 22, 2024 at 12:50 AM Bjorn Andersson <quic_bjorande@quicinc.com> wrote: > > On Sun, Apr 07, 2024 at 12:53:30PM +0530, Prabhav Kumar Vaish wrote: > > Use automated cleanup to replace of_node_put() in qcom_smem_resolve_mem(). > > > > I don't find this easier to read or maintain. Should I change it , explaining the change ?? > > Also, your subject prefix does not match other changes to this driver. The patch is to add a __free function attribute to np pointer initialization ensuring the pointers are freed as soon as they go out of scope, thus removing the work to manually free them using of_node_put. > > Regards, > Bjorn > > > Signed-off-by: Prabhav Kumar Vaish <pvkumar5749404@gmail.com> > > --- > > drivers/soc/qcom/smem.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c > > index 7191fa0c087f..ad1cf8dcc6ec 100644 > > --- a/drivers/soc/qcom/smem.c > > +++ b/drivers/soc/qcom/smem.c > > @@ -1032,18 +1032,16 @@ static int qcom_smem_resolve_mem(struct qcom_smem *smem, const char *name, > > struct smem_region *region) > > { > > struct device *dev = smem->dev; > > - struct device_node *np; > > struct resource r; > > int ret; > > + struct device_node *np __free(device_node) = of_parse_phandle(dev->of_node, name, 0); > > > > - np = of_parse_phandle(dev->of_node, name, 0); > > if (!np) { > > dev_err(dev, "No %s specified\n", name); > > return -EINVAL; > > } > > > > ret = of_address_to_resource(np, 0, &r); > > - of_node_put(np); > > if (ret) > > return ret; > > > > -- > > 2.34.1 > >
On Wed, May 22, 2024 at 1:13 AM Bjorn Andersson <quic_bjorande@quicinc.com> wrote: > > On Wed, May 22, 2024 at 01:01:22AM +0530, prabhav kumar wrote: > > On Wed, May 22, 2024 at 12:50 AM Bjorn Andersson > > <quic_bjorande@quicinc.com> wrote: > > > > > > On Sun, Apr 07, 2024 at 12:53:30PM +0530, Prabhav Kumar Vaish wrote: > > > > Use automated cleanup to replace of_node_put() in qcom_smem_resolve_mem(). > > > > > > > > > > I don't find this easier to read or maintain. > > Should I change it , explaining the change ?? > > > > > > Also, your subject prefix does not match other changes to this driver. > > The patch is to add a __free function attribute to np pointer > > initialization ensuring > > the pointers are freed as soon as they go out of scope, thus removing > > the work to > > manually free them using of_node_put. > > Yes, that is what the __free() attribute does, but I don't find it > easier to read and unless I'm missing something you're not fixing an > actual issue here? > There is not any issue here, the patch sent is the cleanup of unnecessary of_node_put() function call. Thanks, Prabhav > Regards, > Bjorn > > > > > > > Regards, > > > Bjorn > > > > > > > Signed-off-by: Prabhav Kumar Vaish <pvkumar5749404@gmail.com> > > > > --- > > > > drivers/soc/qcom/smem.c | 4 +--- > > > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > > > > > diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c > > > > index 7191fa0c087f..ad1cf8dcc6ec 100644 > > > > --- a/drivers/soc/qcom/smem.c > > > > +++ b/drivers/soc/qcom/smem.c > > > > @@ -1032,18 +1032,16 @@ static int qcom_smem_resolve_mem(struct qcom_smem *smem, const char *name, > > > > struct smem_region *region) > > > > { > > > > struct device *dev = smem->dev; > > > > - struct device_node *np; > > > > struct resource r; > > > > int ret; > > > > + struct device_node *np __free(device_node) = of_parse_phandle(dev->of_node, name, 0); > > > > > > > > - np = of_parse_phandle(dev->of_node, name, 0); > > > > if (!np) { > > > > dev_err(dev, "No %s specified\n", name); > > > > return -EINVAL; > > > > } > > > > > > > > ret = of_address_to_resource(np, 0, &r); > > > > - of_node_put(np); > > > > if (ret) > > > > return ret; > > > > > > > > -- > > > > 2.34.1 > > > >
diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c index 7191fa0c087f..ad1cf8dcc6ec 100644 --- a/drivers/soc/qcom/smem.c +++ b/drivers/soc/qcom/smem.c @@ -1032,18 +1032,16 @@ static int qcom_smem_resolve_mem(struct qcom_smem *smem, const char *name, struct smem_region *region) { struct device *dev = smem->dev; - struct device_node *np; struct resource r; int ret; + struct device_node *np __free(device_node) = of_parse_phandle(dev->of_node, name, 0); - np = of_parse_phandle(dev->of_node, name, 0); if (!np) { dev_err(dev, "No %s specified\n", name); return -EINVAL; } ret = of_address_to_resource(np, 0, &r); - of_node_put(np); if (ret) return ret;
Use automated cleanup to replace of_node_put() in qcom_smem_resolve_mem(). Signed-off-by: Prabhav Kumar Vaish <pvkumar5749404@gmail.com> --- drivers/soc/qcom/smem.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)