Messages in this thread | | | Date | Mon, 28 Dec 2020 18:41:01 +0200 | From | Sakari Ailus <> | Subject | Re: [PATCH v3 07/14] software_node: Add support for fwnode_graph*() family of functions |
| |
Hi Daniel,
On Thu, Dec 24, 2020 at 02:21:15PM +0000, Daniel Scally wrote: > Hi Andy, Laurent > > > On Thu, Dec 24, 2020 at 2:55 PM Laurent Pinchart > > <laurent.pinchart@ideasonboard.com> wrote: > >> On Thu, Dec 24, 2020 at 02:24:12PM +0200, Andy Shevchenko wrote: > >>> On Thu, Dec 24, 2020 at 3:14 AM Daniel Scally wrote: > > > > ... > > > >>>> + if (!strncmp(to_swnode(port)->node->name, "port@", > >>> > >>> You may use here corresponding _FMT macro. > >>> > >>>> + FWNODE_GRAPH_PORT_NAME_PREFIX_LEN)) > >>>> + return port; > > > > ... > > > >>>> + /* Ports have naming style "port@n", we need to select the n */ > >>> > >>>> + ret = kstrtou32(swnode->parent->node->name + FWNODE_GRAPH_PORT_NAME_PREFIX_LEN, > >>> > >>> Maybe a temporary variable? > >>> > >>> unsigned int prefix_len = FWNODE_GRAPH_PORT_NAME_PREFIX_LEN; > >>> ... > >>> ret = kstrtou32(swnode->parent->node->name + prefix_len, > >> > >> Honestly I'm wondering if those macros don't hinder readability. I'd > >> rather write > >> > >> + strlen("port@") > > > > Works for me, since the compiler optimizes this away to be a plain constant. > > Well, how about instead of the LEN macro, we have: > > #define FWNODE_GRAPH_PORT_NAME_PREFIX "port@" > #define FWNODE_GRAPH_PORT_NAME_FMT FWNODE_GRAPH_PORT_NAME_PREFIX "%u" > > And then it's still maintainable in one place but (I think) slightly > less clunky, since we can do strlen(FWNODE_GRAPH_PORT_NAME_PREFIX) > > Or we can do strlen("port@"), I'm good either way :)
I'd be in favour of using strlen("port@") here.
At least for now. I think refactoring the use of such strings could be a separate set at another time, if that's seen as the way to go.
-- Kind regards,
Sakari Ailus
| |