| Message ID | 20240801-device_child_node_access-v1-0-ddfa21bef6f2@gmail.com |
|---|---|
| Headers | show |
| Series | use device_for_each_child_node_scoped to access device child nodes | expand |
On 01/08/2024 11:20, Suzuki K Poulose wrote: > On 01/08/2024 07:13, Javier Carrasco wrote: >> Drop the manual access to the fwnode of the device to iterate over its >> child nodes. `device_for_each_child_node` macro provides direct access >> to the child nodes, and given that they are only required within the >> loop, the scoped variant of the macro can be used. >> >> Use the `device_for_each_child_node_scoped` macro to iterate over the >> direct child nodes of the device. >> >> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> >> --- >> drivers/hwtracing/coresight/coresight-cti-platform.c | 10 +++------- >> 1 file changed, 3 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-cti-platform.c b/ >> drivers/hwtracing/coresight/coresight-cti-platform.c >> index ccef04f27f12..d0ae10bf6128 100644 >> --- a/drivers/hwtracing/coresight/coresight-cti-platform.c >> +++ b/drivers/hwtracing/coresight/coresight-cti-platform.c >> @@ -416,20 +416,16 @@ static int >> cti_plat_create_impdef_connections(struct device *dev, >> struct cti_drvdata *drvdata) >> { >> int rc = 0; >> - struct fwnode_handle *fwnode = dev_fwnode(dev); >> - struct fwnode_handle *child = NULL; >> - if (IS_ERR_OR_NULL(fwnode)) >> + if (IS_ERR_OR_NULL(dev_fwnode(dev))) >> return -EINVAL; >> - fwnode_for_each_child_node(fwnode, child) { >> + device_for_each_child_node_scoped(dev, child) { >> if (cti_plat_node_name_eq(child, CTI_DT_CONNS)) >> - rc = cti_plat_create_connection(dev, drvdata, >> - child); >> + rc = cti_plat_create_connection(dev, drvdata, child); >> if (rc != 0) >> break; > > Don't we need to fwnode_handle_put(child) here, since we removed the > outer one ? > > Suzuki > Hi Suzuki, No, we don't need fwnode_handle_put(child) anymore because the scoped variant of the macro is used. Best regards, Javier Carrasco
On 01/08/2024 11:37, Javier Carrasco wrote: > On 01/08/2024 11:20, Suzuki K Poulose wrote: >> On 01/08/2024 07:13, Javier Carrasco wrote: >>> Drop the manual access to the fwnode of the device to iterate over its >>> child nodes. `device_for_each_child_node` macro provides direct access >>> to the child nodes, and given that they are only required within the >>> loop, the scoped variant of the macro can be used. >>> >>> Use the `device_for_each_child_node_scoped` macro to iterate over the >>> direct child nodes of the device. >>> >>> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> >>> --- >>> drivers/hwtracing/coresight/coresight-cti-platform.c | 10 +++------- >>> 1 file changed, 3 insertions(+), 7 deletions(-) >>> >>> diff --git a/drivers/hwtracing/coresight/coresight-cti-platform.c b/ >>> drivers/hwtracing/coresight/coresight-cti-platform.c >>> index ccef04f27f12..d0ae10bf6128 100644 >>> --- a/drivers/hwtracing/coresight/coresight-cti-platform.c >>> +++ b/drivers/hwtracing/coresight/coresight-cti-platform.c >>> @@ -416,20 +416,16 @@ static int >>> cti_plat_create_impdef_connections(struct device *dev, >>> struct cti_drvdata *drvdata) >>> { >>> int rc = 0; >>> - struct fwnode_handle *fwnode = dev_fwnode(dev); >>> - struct fwnode_handle *child = NULL; >>> - if (IS_ERR_OR_NULL(fwnode)) >>> + if (IS_ERR_OR_NULL(dev_fwnode(dev))) >>> return -EINVAL; >>> - fwnode_for_each_child_node(fwnode, child) { >>> + device_for_each_child_node_scoped(dev, child) { >>> if (cti_plat_node_name_eq(child, CTI_DT_CONNS)) >>> - rc = cti_plat_create_connection(dev, drvdata, >>> - child); >>> + rc = cti_plat_create_connection(dev, drvdata, child); >>> if (rc != 0) >>> break; >> >> Don't we need to fwnode_handle_put(child) here, since we removed the >> outer one ? >> >> Suzuki >> > > Hi Suzuki, > > No, we don't need fwnode_handle_put(child) anymore because the scoped > variant of the macro is used. Ah, apologies, was looking at the non-scoped version. I will queue this. Suzuki > > Best regards, > Javier Carrasco
This series removes accesses to the device `fwnode` to iterate over its own child nodes. Using the `device_for_each_child_node` macro provides direct access to the device child nodes, and given that in all cases they are only required within the loop, the scoped variant of the macro can be used. It has been stated in previous discussions [1] that `device_for_each_*` should be used to access device child nodes, removing the need to access its internal fwnode, and restricting `fwnode_for_each_*` to traversing subnodes when required. Note that `device_for_each_*` implies availability, which means that after this conversion, unavailable nodes will not be accessible within the loop. The affected drivers does not seem to have any reason to iterate over unavailable nodes, though. But if someone has a case where the affected drivers might require accessing unavailable nodes, please let me know. Link: https://lore.kernel.org/linux-hwmon/cffb5885-3cbc-480c-ab6d-4a442d1afb8a@gmail.com/ [1] Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> --- Javier Carrasco (4): coresight: cti: use device_* to iterate over device child nodes iio: adc: ad7768-1: use device_* to iterate over device child nodes iio: adc: xilinx-ams: use device_* to iterate over device child nodes leds: as3645a: use device_* to iterate over device child nodes drivers/hwtracing/coresight/coresight-cti-platform.c | 10 +++------- drivers/iio/adc/ad7768-1.c | 5 +---- drivers/iio/adc/xilinx-ams.c | 7 ++----- drivers/leds/flash/leds-as3645a.c | 8 +++----- 4 files changed, 9 insertions(+), 21 deletions(-) --- base-commit: cd19ac2f903276b820f5d0d89de0c896c27036ed change-id: 20240725-device_child_node_access-442533910a6f Best regards,