Hi Andy On 04/01/2021 10:22, Andy Shevchenko wrote: > On Sun, Jan 03, 2021 at 11:12:28PM +0000, Daniel Scally wrote: >> From: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx> >> >> This implements the remaining .graph_*() callbacks in the fwnode >> operations structure for the software nodes. That makes the >> fwnode_graph_*() functions available in the drivers also when software >> nodes are used. >> >> The implementation tries to mimic the "OF graph" as much as possible, but >> there is no support for the "reg" device property. The ports will need to >> have the index in their name which starts with "port@" (for example >> "port@0", "port@1", ...) and endpoints will use the index of the software >> node that is given to them during creation. The port nodes can also be >> grouped under a specially named "ports" subnode, just like in DT, if >> necessary. >> >> The remote-endpoints are reference properties under the endpoint nodes >> that are named "remote-endpoint". > Couple of nitpicks below (can be considered as follow up, depends on yours and > maintainer wishes). > Thanks > +static int > +software_node_graph_parse_endpoint(const struct fwnode_handle *fwnode, > + struct fwnode_endpoint *endpoint) > +{ > + struct swnode *swnode = to_swnode(fwnode); > + const char *parent_name = swnode->parent->node->name; > + int ret; > + > + if (!(strlen(parent_name) > strlen("port@")) || > A nit: > > if (strlen("port@") >= strlen(parent_name) || > > better to read yeah agreed > >> + strncmp(parent_name, "port@", strlen("port@"))) >> + return -EINVAL; >> + >> + /* Ports have naming style "port@n", we need to select the n */ >> + ret = kstrtou32(parent_name + strlen("port@"), >> + 10, &endpoint->port); > A nit: > > ret = kstrtou32(parent_name + strlen("port@"), 10, &endpoint->port); > > (perhaps you need to adjust your editor settings, this still fits 80) Ah - my bad. Originally instead of parent_name there was swnode->parent->node->name, which didn't fit. When I added the temp variable I forgot to fix the line break - thanks.