Re: [PATCH v2 01/16] device property: Add fwnode_graph_get_port_by_id()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] fwnode_graph_get_port_by_id() incorrectly fails to find valid ports if they lack active endpoints, have unconnected endpoints, or are connected to disabled devices.
--

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

device property: Add fwnode_graph_get_port_by_id()

This commit introduces the fwnode_graph_get_port_by_id() helper to retrieve
a port firmware node matching a given id. It is intended for cases where
drivers need a reference to the port firmware node, such as USB power
sequencing integration.

> diff --git a/drivers/base/property.c b/drivers/base/property.c
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c

[ ... ]

> @@ -1346,6 +1346,28 @@ int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
>  }
>  EXPORT_SYMBOL(fwnode_graph_parse_endpoint);
>  
> +/**
> + * fwnode_graph_get_port_by_id - get the port matching a given id
> + * @fwnode: parent fwnode_handle containing the graph
> + * @id: id of the port
> + *
> + * Return: A 'port' firmware node pointer with refcount incremented.
> + *
> + * The caller is responsible for calling fwnode_handle_put() on the returned
> + * fwnode pointer.
> + */
> +struct fwnode_handle *fwnode_graph_get_port_by_id(struct fwnode_handle *fwnode, u32 id)
> +{
> +	struct fwnode_handle *ep;
> +
> +	ep = fwnode_graph_get_endpoint_by_id(fwnode, id, 0, FWNODE_GRAPH_ENDPOINT_NEXT);

[Severity: High]
Will this fail to find valid ports if they lack active endpoints or have
disabled remote targets?

By calling fwnode_graph_get_endpoint_by_id() without passing the
FWNODE_GRAPH_DEVICE_DISABLED flag, it appears this enforces an implicit
availability check. This skips any endpoints that are unconnected or whose
remote target is disabled.

Additionally, since port discovery is routed exclusively through endpoint
iteration, will this completely miss valid port nodes that contain no
endpoint child nodes?

> +	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/20260610084053.2059858-1-wenst@chromium.org?part=1