linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 1/1] test_printf: Refactor fwnode_pointer() to make it more readable
@ 2022-08-24 17:05 Andy Shevchenko
  2022-08-31 14:16 ` Andy Shevchenko
  2022-09-29 16:06 ` Steven Rostedt
  0 siblings, 2 replies; 6+ messages in thread
From: Andy Shevchenko @ 2022-08-24 17:05 UTC (permalink / raw)
  To: Petr Mladek, linux-kernel
  Cc: Steven Rostedt, Sergey Senozhatsky, Andy Shevchenko, Rasmus Villemoes

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 };
+	const struct software_node *group[] = { &first, &second, &third, 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));
 
-	software_node_unregister_nodes(softnodes);
+	software_node_unregister_node_group(group);
 }
 
 static void __init fourcc_pointer(void)
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 1/1] test_printf: Refactor fwnode_pointer() to make it more readable
  2022-08-24 17:05 [PATCH v1 1/1] test_printf: Refactor fwnode_pointer() to make it more readable Andy Shevchenko
@ 2022-08-31 14:16 ` Andy Shevchenko
  2022-11-01 13:01   ` Petr Mladek
  2022-09-29 16:06 ` Steven Rostedt
  1 sibling, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2022-08-31 14:16 UTC (permalink / raw)
  To: Petr Mladek, linux-kernel
  Cc: Steven Rostedt, Sergey Senozhatsky, Rasmus Villemoes

On Wed, Aug 24, 2022 at 08:05:42PM +0300, Andy Shevchenko 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.

Any comments?

Note, I would like to reduce exported swnode APIs and this will
help to do so.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 1/1] test_printf: Refactor fwnode_pointer() to make it more readable
  2022-08-24 17:05 [PATCH v1 1/1] test_printf: Refactor fwnode_pointer() to make it more readable Andy Shevchenko
  2022-08-31 14:16 ` Andy Shevchenko
@ 2022-09-29 16:06 ` Steven Rostedt
  2022-09-29 17:04   ` Andy Shevchenko
  1 sibling, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2022-09-29 16:06 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Petr Mladek, linux-kernel, Sergey Senozhatsky, Rasmus Villemoes

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)


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 1/1] test_printf: Refactor fwnode_pointer() to make it more readable
  2022-09-29 16:06 ` Steven Rostedt
@ 2022-09-29 17:04   ` Andy Shevchenko
  0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2022-09-29 17:04 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Petr Mladek, linux-kernel, Sergey Senozhatsky, Rasmus Villemoes

On Thu, Sep 29, 2022 at 12:06:32PM -0400, Steven Rostedt wrote:
> On Wed, 24 Aug 2022 20:05:42 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

Thank you for review, my answers below.

...

> > +	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 };

It could, but the issue is that it will loose the self-explanatory naming
scheme. It's much easier to see what we test and what we expect in the below
calls...

> >  	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));

...here.

> Then the above doesn't need to change.

And that's why I want to change them.

> 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).

> > -	software_node_unregister_nodes(softnodes);
> > +	software_node_unregister_node_group(group);
> >  }
> >  
> >  static void __init fourcc_pointer(void)

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 1/1] test_printf: Refactor fwnode_pointer() to make it more readable
  2022-08-31 14:16 ` Andy Shevchenko
@ 2022-11-01 13:01   ` Petr Mladek
  2022-11-01 14:40     ` Andy Shevchenko
  0 siblings, 1 reply; 6+ messages in thread
From: Petr Mladek @ 2022-11-01 13:01 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-kernel, Steven Rostedt, Sergey Senozhatsky, Rasmus Villemoes

On Wed 2022-08-31 17:16:44, Andy Shevchenko wrote:
> On Wed, Aug 24, 2022 at 08:05:42PM +0300, Andy Shevchenko 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.
> 
> Any comments?
> 
> Note, I would like to reduce exported swnode APIs and this will
> help to do so.

JFYI, I have just pushed the patch into printk/linux.git, branch
for-6.2.

I am sorry that it took so long. I have got a long backlog of mails
to proceed.

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 1/1] test_printf: Refactor fwnode_pointer() to make it more readable
  2022-11-01 13:01   ` Petr Mladek
@ 2022-11-01 14:40     ` Andy Shevchenko
  0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2022-11-01 14:40 UTC (permalink / raw)
  To: Petr Mladek
  Cc: linux-kernel, Steven Rostedt, Sergey Senozhatsky, Rasmus Villemoes

On Tue, Nov 01, 2022 at 02:01:32PM +0100, Petr Mladek wrote:
> On Wed 2022-08-31 17:16:44, Andy Shevchenko wrote:
> > On Wed, Aug 24, 2022 at 08:05:42PM +0300, Andy Shevchenko 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.
> > 
> > Any comments?
> > 
> > Note, I would like to reduce exported swnode APIs and this will
> > help to do so.
> 
> JFYI, I have just pushed the patch into printk/linux.git, branch
> for-6.2.

Thank you!

> I am sorry that it took so long. I have got a long backlog of mails
> to proceed.

No problem.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-11-01 14:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-24 17:05 [PATCH v1 1/1] test_printf: Refactor fwnode_pointer() to make it more readable Andy Shevchenko
2022-08-31 14:16 ` Andy Shevchenko
2022-11-01 13:01   ` Petr Mladek
2022-11-01 14:40     ` Andy Shevchenko
2022-09-29 16:06 ` Steven Rostedt
2022-09-29 17:04   ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).