Messages in this thread | | | Date | Thu, 29 Sep 2022 12:06:32 -0400 | From | Steven Rostedt <> | Subject | Re: [PATCH v1 1/1] test_printf: Refactor fwnode_pointer() to make it more readable |
| |
On Wed, 24 Aug 2022 20:05:42 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> Converting fwnode_pointer() to use better swnode API allows to > make code more readable. > > While at it, rename full_name to full_name_third to show exact > relation in the hierarchy. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > lib/test_printf.c | 26 ++++++++++++-------------- > 1 file changed, 12 insertions(+), 14 deletions(-) > > diff --git a/lib/test_printf.c b/lib/test_printf.c > index fe13de1bed5f..6f7f179dd8f4 100644 > --- a/lib/test_printf.c > +++ b/lib/test_printf.c > @@ -704,31 +704,29 @@ flags(void) > > static void __init fwnode_pointer(void) > { > - const struct software_node softnodes[] = { > - { .name = "first", }, > - { .name = "second", .parent = &softnodes[0], }, > - { .name = "third", .parent = &softnodes[1], }, > - { NULL /* Guardian */ } > - }; > - const char * const full_name = "first/second/third"; > + const struct software_node first = { .name = "first" }; > + const struct software_node second = { .name = "second", .parent = &first }; > + const struct software_node third = { .name = "third", .parent = &second };
I personally do not find the above more readable, but honestly, I'm not attached to this code at all.
> + const struct software_node *group[] = { &first, &second, &third, NULL };
Could this just be:
const struct software_node *group[] = { &softnodes[0], &softnodes[1], &softnodes[2], NULL };
> const char * const full_name_second = "first/second"; > + const char * const full_name_third = "first/second/third"; > const char * const second_name = "second"; > const char * const third_name = "third"; > int rval; > > - rval = software_node_register_nodes(softnodes); > + rval = software_node_register_node_group(group); > if (rval) { > pr_warn("cannot register softnodes; rval %d\n", rval); > return; > } > > - test(full_name_second, "%pfw", software_node_fwnode(&softnodes[1])); > - test(full_name, "%pfw", software_node_fwnode(&softnodes[2])); > - test(full_name, "%pfwf", software_node_fwnode(&softnodes[2])); > - test(second_name, "%pfwP", software_node_fwnode(&softnodes[1])); > - test(third_name, "%pfwP", software_node_fwnode(&softnodes[2])); > + test(full_name_second, "%pfw", software_node_fwnode(&second)); > + test(full_name_third, "%pfw", software_node_fwnode(&third)); > + test(full_name_third, "%pfwf", software_node_fwnode(&third)); > + test(second_name, "%pfwP", software_node_fwnode(&second)); > + test(third_name, "%pfwP", software_node_fwnode(&third));
Then the above doesn't need to change.
But again, I'm not maintaining this code, so I'm not attached. Just adding my $0.02 to this (as I'm triaging my inbox and found this email).
-- Steve
> > - software_node_unregister_nodes(softnodes); > + software_node_unregister_node_group(group); > } > > static void __init fourcc_pointer(void)
| |