lkml.org 
[lkml]   [2022]   [Sep]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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)

\
 
 \ /
  Last update: 2022-09-29 18:06    [W:0.090 / U:0.412 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site