[PATCH v2 04/16] ACPI: property: Return present device nodes only on fwnode interface

Sakari Ailus posted 16 patches 1 week ago
[PATCH v2 04/16] ACPI: property: Return present device nodes only on fwnode interface
Posted by Sakari Ailus 1 week ago
fwnode_graph_get_next_subnode() may return fwnode backed by ACPI device
nodes and there has been no check these devices are present in the system,
unlike there has been on fwnode OF backend. In order to provide consistent
behaviour towards callers, add a check for device presence by introducing
a new function acpi_get_next_present_subnode(), used as the
get_next_child_node() fwnode operation that also checks device node
presence.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 5438592dc136..01f3880ffcce 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -1319,6 +1319,26 @@ acpi_get_next_subnode(const struct fwnode_handle *fwnode,
 	return NULL;
 }
 
+/**
+ * acpi_get_next_present_subnode - Return the next present child node handle for a fwnode
+ * @fwnode: Firmware node to find the next child node for.
+ * @child: Handle to one of the device's child nodes or a null handle.
+ * Like acpi_get_next_subnode(), but the device nodes returned by
+ * acpi_get_next_present_subnode() are guaranteed to be present.
+ * Returns: The next sub-node fwnode handle.
+ */
+static struct fwnode_handle *
+acpi_get_next_present_subnode(const struct fwnode_handle *fwnode,
+			      struct fwnode_handle *child)
+{
+	do {
+		child = acpi_get_next_subnode(fwnode, child);
+	} while (is_acpi_device_node(child) &&
+		 !acpi_device_is_present(to_acpi_device_node(child)));
+
+	return child;
+}
+
 /**
  * acpi_node_get_parent - Return parent fwnode of this fwnode
  * @fwnode: Firmware node whose parent to get
@@ -1664,7 +1684,7 @@ static int acpi_fwnode_irq_get(const struct fwnode_handle *fwnode,
 		.property_read_string_array =				\
 			acpi_fwnode_property_read_string_array,		\
 		.get_parent = acpi_node_get_parent,			\
-		.get_next_child_node = acpi_get_next_subnode,		\
+		.get_next_child_node = acpi_get_next_present_subnode,	\
 		.get_named_child_node = acpi_fwnode_get_named_child_node, \
 		.get_name = acpi_fwnode_get_name,			\
 		.get_name_prefix = acpi_fwnode_get_name_prefix,		\
-- 
2.47.3
Re: [PATCH v2 04/16] ACPI: property: Return present device nodes only on fwnode interface
Posted by Jonathan Cameron 2 days, 16 hours ago
On Wed, 24 Sep 2025 10:45:50 +0300
Sakari Ailus <sakari.ailus@linux.intel.com> wrote:

> fwnode_graph_get_next_subnode() may return fwnode backed by ACPI device
> nodes and there has been no check these devices are present in the system,
> unlike there has been on fwnode OF backend. In order to provide consistent
> behaviour towards callers, add a check for device presence by introducing
> a new function acpi_get_next_present_subnode(), used as the
> get_next_child_node() fwnode operation that also checks device node
> presence.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

I think this is fine because of the bit in the ACPI spec that says all
bits are set if _STA is missing.   It seems much less likely we'll see
problems with hardware disappearing because _STA is there but says
the device isn't present.

Always a regression risk though :(

With the formatting changes Laurent asked for
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Re: [PATCH v2 04/16] ACPI: property: Return present device nodes only on fwnode interface
Posted by Laurent Pinchart 1 week ago
On Wed, Sep 24, 2025 at 10:45:50AM +0300, Sakari Ailus wrote:
> fwnode_graph_get_next_subnode() may return fwnode backed by ACPI device
> nodes and there has been no check these devices are present in the system,
> unlike there has been on fwnode OF backend. In order to provide consistent
> behaviour towards callers, add a check for device presence by introducing
> a new function acpi_get_next_present_subnode(), used as the
> get_next_child_node() fwnode operation that also checks device node
> presence.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/acpi/property.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> index 5438592dc136..01f3880ffcce 100644
> --- a/drivers/acpi/property.c
> +++ b/drivers/acpi/property.c
> @@ -1319,6 +1319,26 @@ acpi_get_next_subnode(const struct fwnode_handle *fwnode,
>  	return NULL;
>  }
>  
> +/**

/*

as the function is static ?

> + * acpi_get_next_present_subnode - Return the next present child node handle for a fwnode
> + * @fwnode: Firmware node to find the next child node for.
> + * @child: Handle to one of the device's child nodes or a null handle.

A blank line here would be nice.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> + * Like acpi_get_next_subnode(), but the device nodes returned by
> + * acpi_get_next_present_subnode() are guaranteed to be present.
> + * Returns: The next sub-node fwnode handle.
> + */
> +static struct fwnode_handle *
> +acpi_get_next_present_subnode(const struct fwnode_handle *fwnode,
> +			      struct fwnode_handle *child)
> +{
> +	do {
> +		child = acpi_get_next_subnode(fwnode, child);
> +	} while (is_acpi_device_node(child) &&
> +		 !acpi_device_is_present(to_acpi_device_node(child)));
> +
> +	return child;
> +}
> +
>  /**
>   * acpi_node_get_parent - Return parent fwnode of this fwnode
>   * @fwnode: Firmware node whose parent to get
> @@ -1664,7 +1684,7 @@ static int acpi_fwnode_irq_get(const struct fwnode_handle *fwnode,
>  		.property_read_string_array =				\
>  			acpi_fwnode_property_read_string_array,		\
>  		.get_parent = acpi_node_get_parent,			\
> -		.get_next_child_node = acpi_get_next_subnode,		\
> +		.get_next_child_node = acpi_get_next_present_subnode,	\
>  		.get_named_child_node = acpi_fwnode_get_named_child_node, \
>  		.get_name = acpi_fwnode_get_name,			\
>  		.get_name_prefix = acpi_fwnode_get_name_prefix,		\

-- 
Regards,

Laurent Pinchart