Re: [PATCH v3 01/13] device property: Add fwnode_graph_get_port_by_id()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] `fwnode_graph_get_port_by_id()` fails to find a port if its endpoints are unconnected or connected to disabled remote devices due to missing `FWNODE_GRAPH_DEVICE_DISABLED` flag.
- [Medium] `fwnode_graph_get_port_by_id()` cannot find valid port nodes that do not contain any endpoints, diverging from `of_graph_get_port_by_id()` behavior.
--

commit c798c3e5cfa59227921f3fc93df8113ad82e4544
Author: Chen-Yu Tsai <wenst@chromium.org>

device property: Add fwnode_graph_get_port_by_id()

This commit introduces fwnode_graph_get_port_by_id() to allow drivers to
obtain a reference to a port firmware node by its ID. The helper is
implemented by finding the matching endpoint first and then returning its
parent node. It is intended to support upcoming USB power sequencing
integration.

> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index e08eadd66f4f9..c5e7e00f1b0cd 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
[ ... ]
> @@ -1346,6 +1346,28 @@ int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
[ ... ]
> +struct fwnode_handle *fwnode_graph_get_port_by_id(struct fwnode_handle *fwnode, u32 id)
> +{
> +	struct fwnode_handle *ep;
> +

[Severity: Medium]
Could this regression cause the function to miss port nodes that are
completely empty of endpoints?

Because this relies on fwnode_graph_get_endpoint_by_id() (which iterates over
endpoints), if a port node is defined in firmware solely to hold properties
(like vbus-supply) but has no endpoint child nodes, this function will return
NULL. This seems to diverge from the behavior of of_graph_get_port_by_id(),
which correctly finds empty port nodes.

[Severity: High]
Does this call need the FWNODE_GRAPH_DEVICE_DISABLED flag to avoid a
regression where ports with disabled or unconnected remote devices are skipped?

By omitting FWNODE_GRAPH_DEVICE_DISABLED, fwnode_graph_get_endpoint_by_id()
evaluates with enabled_only = true. If a port's remote device is disabled
in the device tree, or if the port is unconnected, it will skip those
endpoints and return NULL. This could cause USB port devices to lose their
firmware node association if the remote connector is disabled, preventing
subsystems from reading the port's properties.

> +	ep = fwnode_graph_get_endpoint_by_id(fwnode, id, 0, FWNODE_GRAPH_ENDPOINT_NEXT);
> +	if (!ep)
> +		return NULL;
> +
> +	return fwnode_get_next_parent(ep);
> +}
> +EXPORT_SYMBOL_GPL(fwnode_graph_get_port_by_id);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703110317.1283411-1-wenst@chromium.org?part=1