On Fri, Jan 31, 2025 at 06:33:58PM +0200, Laurentiu Palcu wrote:
> -static int max96712_parse_dt(struct max96712_priv *priv)
> +static int max96712_parse_rx_ports(struct max96712_priv *priv, struct device_node *node,
> + struct of_endpoint *ep)
> +{
> + struct device *dev = &priv->client->dev;
> + struct max96712_rx_port *source;
> + struct fwnode_handle *remote_port_parent;
> +
> + if (priv->rx_ports[ep->port].fwnode) {
> + dev_info(dev, "Multiple port endpoints are not supported: %d", ep->port);
> + return 0;
> + }
> +
> + source = &priv->rx_ports[ep->port];
> + source->fwnode = fwnode_graph_get_remote_endpoint(of_fwnode_handle(node));
> + if (!source->fwnode) {
> + dev_info(dev, "Endpoint %pOF has no remote endpoint connection\n", ep->local_node);
> + return 0;
> + }
> +
> + remote_port_parent = fwnode_graph_get_remote_port_parent(of_fwnode_handle(node));
> +
> + if (!fwnode_device_is_available(remote_port_parent)) {
> + dev_dbg(dev, "Skipping port %d as remote port parent is disabled.\n",
> + ep->port);
> + source->fwnode = NULL;
I don't understand the refcounting in this function. Should we call
fwnode_handle_put(source->fwnode); before setting this to NULL?
> + goto fwnode_put;
> + }
> +
> + priv->rx_port_mask |= BIT(ep->port);
> + priv->n_rx_ports++;
> +
> +fwnode_put:
> + fwnode_handle_put(remote_port_parent);
> + fwnode_handle_put(source->fwnode);
> + return 0;
I don't understand why we save source->fwnode but drop the refcount on
the success path. My instinct says that it should be a source_fwnode
local stack variable instead. (But again, I've only glanced at this
code so I could be wrong).
> +}
> +
regards,
dan carpenter
