Hi Sakari - thanks for the reviews in previous emails
On 21/12/2020 09:34, Sakari Ailus wrote:
> Hi Daniel and Heikki,
>
> On Thu, Dec 17, 2020 at 11:43:31PM +0000, Daniel Scally wrote:
>
> +static struct fwnode_handle *
> +software_node_graph_get_port_parent(struct fwnode_handle *fwnode)
> +{
> + struct swnode *swnode = to_swnode(fwnode);
> + struct fwnode_handle *parent;
> +
> + if (!strcmp(swnode->parent->node->name, "ports"))
> + parent = &swnode->parent->parent->fwnode;
> + else
> + parent = &swnode->parent->fwnode;
> If you happen to call this function on a non-port node for whatever reason,
> you may end up accessing a pointer that's NULL, can't you?
Yes, actually.
> Instead I'd do
> something like:
>
> swnode = swnode->parent;
> if (swnode && !strcmp(swnode->node->name, "ports"))
> swnode = swnode->parent;
>
> return swnode ? software_node_get(&swnode->fwnode) : NULL;
>
> You can also drop parent as a by-product of this.
Yes ok, that looks good to me - thanks.
>> +
>> + return software_node_get(parent);
>> +}
>> +
>> +static int
>> +software_node_graph_parse_endpoint(const struct fwnode_handle *fwnode,
>> + struct fwnode_endpoint *endpoint)
>> +{
>> + struct swnode *swnode = to_swnode(fwnode);
>> + int ret;
>> +
>> + ret = kstrtou32(swnode->parent->node->name + 5, 10, &endpoint->port);
>> + if (ret)
>> + return ret;
>> +
>> + endpoint->id = swnode->id;
>> + endpoint->local_fwnode = fwnode;
>> +
>> + return 0;
>> +}
>> +
>> static const struct fwnode_operations software_node_ops = {
>> .get = software_node_get,
>> .put = software_node_put,
>> @@ -551,7 +655,11 @@ static const struct fwnode_operations software_node_ops = {
>> .get_parent = software_node_get_parent,
>> .get_next_child_node = software_node_get_next_child,
>> .get_named_child_node = software_node_get_named_child_node,
>> - .get_reference_args = software_node_get_reference_args
>> + .get_reference_args = software_node_get_reference_args,
>> + .graph_get_next_endpoint = software_node_graph_get_next_endpoint,
>> + .graph_get_remote_endpoint = software_node_graph_get_remote_endpoint,
>> + .graph_get_port_parent = software_node_graph_get_port_parent,
>> + .graph_parse_endpoint = software_node_graph_parse_endpoint,
>> };
>>
>> /* -------------------------------------------------------------------------- */
