Hi Rob,
Thanks for the review.
On 05/08/17 21:23, Rob Herring wrote:
> On Thu, May 4, 2017 at 2:14 AM, Sakari Ailus
> <sakari.ailus@xxxxxxxxxxxxxxx> wrote:
...
>> @@ -1184,24 +1167,11 @@ EXPORT_SYMBOL_GPL(fwnode_graph_get_next_endpoint);
>> struct fwnode_handle *
>> fwnode_graph_get_remote_port_parent(struct fwnode_handle *fwnode)
>> {
>> - struct fwnode_handle *parent = NULL;
>> -
>> - if (is_of_node(fwnode)) {
>> - struct device_node *node;
>> -
>> - node = of_graph_get_remote_port_parent(to_of_node(fwnode));
>> - if (node)
>> - parent = &node->fwnode;
>> - } else if (is_acpi_node(fwnode)) {
>> - int ret;
>> -
>> - ret = acpi_graph_get_remote_endpoint(fwnode, &parent, NULL,
>> - NULL);
>> - if (ret)
>> - return NULL;
>> - }
>> + fwnode = fwnode_graph_get_remote_port(fwnode);
>> + if (!fwnode)
>> + return NULL;
>>
>> - return parent;
>> + return fwnode_call_ptr_op(fwnode, graph_get_port_parent);
>
> Just make fwnode_call_ptr_op(NULL, ...) return NULL.
Hmm. That already appears to be the case... I'll take that account
above, too.
>
>> }
>> EXPORT_SYMBOL_GPL(fwnode_graph_get_remote_port_parent);
>>
>> @@ -1213,23 +1183,11 @@ EXPORT_SYMBOL_GPL(fwnode_graph_get_remote_port_parent);
>> */
>> struct fwnode_handle *fwnode_graph_get_remote_port(struct fwnode_handle *fwnode)
>> {
>> - struct fwnode_handle *port = NULL;
>> -
>> - if (is_of_node(fwnode)) {
>> - struct device_node *node;
>> -
>> - node = of_graph_get_remote_port(to_of_node(fwnode));
>> - if (node)
>> - port = &node->fwnode;
>> - } else if (is_acpi_node(fwnode)) {
>> - int ret;
>> -
>> - ret = acpi_graph_get_remote_endpoint(fwnode, NULL, &port, NULL);
>> - if (ret)
>> - return NULL;
>> - }
>> + fwnode = fwnode_graph_get_remote_endpoint(fwnode);
>> + if (!fwnode)
>> + return NULL;
>>
>> - return port;
>> + return fwnode_get_parent(fwnode);
>
> Just make fwnode_get_parent(NULL) return NULL.
The same.
>
>> }
>> EXPORT_SYMBOL_GPL(fwnode_graph_get_remote_port);
>>
>> @@ -1242,25 +1200,7 @@ EXPORT_SYMBOL_GPL(fwnode_graph_get_remote_port);
>> struct fwnode_handle *
>> fwnode_graph_get_remote_endpoint(struct fwnode_handle *fwnode)
>> {
>> - struct fwnode_handle *endpoint = NULL;
>> -
>> - if (is_of_node(fwnode)) {
>> - struct device_node *node;
>> -
>> - node = of_parse_phandle(to_of_node(fwnode), "remote-endpoint",
>> - 0);
>> - if (node)
>> - endpoint = &node->fwnode;
>> - } else if (is_acpi_node(fwnode)) {
>> - int ret;
>> -
>> - ret = acpi_graph_get_remote_endpoint(fwnode, NULL, NULL,
>> - &endpoint);
>> - if (ret)
>> - return NULL;
>> - }
>> -
>> - return endpoint;
>> + return fwnode_call_ptr_op(fwnode, graph_get_remote_endpoint);
>> }
>> EXPORT_SYMBOL_GPL(fwnode_graph_get_remote_endpoint);
>>
>> @@ -1276,22 +1216,8 @@ EXPORT_SYMBOL_GPL(fwnode_graph_get_remote_endpoint);
>> int fwnode_graph_parse_endpoint(struct fwnode_handle *fwnode,
>> struct fwnode_endpoint *endpoint)
>> {
>> - struct fwnode_handle *port_fwnode = fwnode_get_parent(fwnode);
>> -
>> memset(endpoint, 0, sizeof(*endpoint));
>>
>> - endpoint->local_fwnode = fwnode;
>> -
>> - if (is_acpi_node(port_fwnode)) {
>> - fwnode_property_read_u32(port_fwnode, "port", &endpoint->port);
>> - fwnode_property_read_u32(fwnode, "endpoint", &endpoint->id);
>> - } else {
>> - fwnode_property_read_u32(port_fwnode, "reg", &endpoint->port);
>> - fwnode_property_read_u32(fwnode, "reg", &endpoint->id);
>> - }
>> -
>> - fwnode_handle_put(port_fwnode);
>> -
>> - return 0;
>> + return fwnode_call_int_op(fwnode, graph_parse_endpoint, endpoint);
>> }
>> EXPORT_SYMBOL(fwnode_graph_parse_endpoint);
>> diff --git a/drivers/of/property.c b/drivers/of/property.c
>> index 984e37e..bb6ac73 100644
>> --- a/drivers/of/property.c
>> +++ b/drivers/of/property.c
>> @@ -869,6 +869,59 @@ static struct fwnode_handle *of_fwnode_get_named_child_node(
>> return NULL;
>> }
>>
>> +static struct fwnode_handle *of_fwnode_graph_get_next_endpoint(
>> + struct fwnode_handle *fwnode, struct fwnode_handle *prev)
>> +{
>> + struct device_node *node;
>> +
>> + node = of_graph_get_next_endpoint(to_of_node(fwnode),
>> + to_of_node(prev));
>> + if (node)
>> + return of_fwnode_handle(node);
>> +
>> + return NULL;
>
> Can't of_fwnode_handle() return NULL when node is NULL? Then these
> functions become one liners:
>
> return of_fwnode_handle(of_graph_get_next_endpoint(to_of_node(fwnode),
> to_of_node(prev)));
Good point. of_fwnode_handle() is a macro (in order to support const /
non-const cases) and it'll be a little bit more complex but I guess
that's not really an issue.
--
Kind regards,
Sakari Ailus
sakari.ailus@xxxxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
