[v5,11/11] lib/test_printf: Add tests for %pfw printk modifier
diff mbox series

Message ID 20190902135732.23455-12-sakari.ailus@linux.intel.com
State Superseded, archived
Headers show
Series
  • Device property improvements, add %pfw format specifier
Related show

Commit Message

Sakari Ailus Sept. 2, 2019, 1:57 p.m. UTC
Add a test for the %pfw printk modifier using software nodes.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 lib/test_printf.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

Comments

Andy Shevchenko Sept. 2, 2019, 4:13 p.m. UTC | #1
On Mon, Sep 02, 2019 at 04:57:32PM +0300, Sakari Ailus wrote:
> Add a test for the %pfw printk modifier using software nodes.

> +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 */ },

Comma is still here :-)

> +	};

> +	test(full_name_second, "%pfw",
> +	     software_node_fwnode(&softnodes[ARRAY_SIZE(softnodes) - 3]));
> +	test(full_name, "%pfw",
> +	     software_node_fwnode(&softnodes[ARRAY_SIZE(softnodes) - 2]));
> +	test(full_name, "%pfwf",
> +	     software_node_fwnode(&softnodes[ARRAY_SIZE(softnodes) - 2]));
> +	test(second_name, "%pfwP",
> +	     software_node_fwnode(&softnodes[ARRAY_SIZE(softnodes) - 3]));
> +	test(third_name, "%pfwP",
> +	     software_node_fwnode(&softnodes[ARRAY_SIZE(softnodes) - 2]));

I have another thought about these. The test cases will fail in either of
adding, inserting or removing items in softnodes array. So, using the above
"protective" scheme doesn't bring any value except making readability worse.
Sakari Ailus Sept. 4, 2019, 4:10 p.m. UTC | #2
Hi Andy,

On Mon, Sep 02, 2019 at 07:13:52PM +0300, Andy Shevchenko wrote:
> On Mon, Sep 02, 2019 at 04:57:32PM +0300, Sakari Ailus wrote:
> > Add a test for the %pfw printk modifier using software nodes.
> 
> > +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 */ },
> 
> Comma is still here :-)

Oops. I ended up removing the comma in a wrong patch which wasn't submitted
to the list. Will fix for v6.

> 
> > +	};
> 
> > +	test(full_name_second, "%pfw",
> > +	     software_node_fwnode(&softnodes[ARRAY_SIZE(softnodes) - 3]));
> > +	test(full_name, "%pfw",
> > +	     software_node_fwnode(&softnodes[ARRAY_SIZE(softnodes) - 2]));
> > +	test(full_name, "%pfwf",
> > +	     software_node_fwnode(&softnodes[ARRAY_SIZE(softnodes) - 2]));
> > +	test(second_name, "%pfwP",
> > +	     software_node_fwnode(&softnodes[ARRAY_SIZE(softnodes) - 3]));
> > +	test(third_name, "%pfwP",
> > +	     software_node_fwnode(&softnodes[ARRAY_SIZE(softnodes) - 2]));
> 
> I have another thought about these. The test cases will fail in either of
> adding, inserting or removing items in softnodes array. So, using the above
> "protective" scheme doesn't bring any value except making readability worse.

Agreed, to be addressed in v6.
Andy Shevchenko Sept. 4, 2019, 5:22 p.m. UTC | #3
On Wed, Sep 04, 2019 at 07:10:51PM +0300, Sakari Ailus wrote:
> On Mon, Sep 02, 2019 at 07:13:52PM +0300, Andy Shevchenko wrote:
> > On Mon, Sep 02, 2019 at 04:57:32PM +0300, Sakari Ailus wrote:
> > > Add a test for the %pfw printk modifier using software nodes.
> > 
> > > +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 */ },
> > 
> > Comma is still here :-)
> 
> Oops. I ended up removing the comma in a wrong patch which wasn't submitted
> to the list. Will fix for v6.

Also you may remove NULL there since it's default.
Sakari Ailus Sept. 6, 2019, 7 a.m. UTC | #4
On Wed, Sep 04, 2019 at 08:22:22PM +0300, Andy Shevchenko wrote:
> On Wed, Sep 04, 2019 at 07:10:51PM +0300, Sakari Ailus wrote:
> > On Mon, Sep 02, 2019 at 07:13:52PM +0300, Andy Shevchenko wrote:
> > > On Mon, Sep 02, 2019 at 04:57:32PM +0300, Sakari Ailus wrote:
> > > > Add a test for the %pfw printk modifier using software nodes.
> > > 
> > > > +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 */ },
> > > 
> > > Comma is still here :-)
> > 
> > Oops. I ended up removing the comma in a wrong patch which wasn't submitted
> > to the list. Will fix for v6.
> 
> Also you may remove NULL there since it's default.

Then it'd become GCC specific. Albeit I'm not sure that's any kind of a
problem in practice. I guess Clang must cope with that, too? Still, I
prefer not to use compiler specific syntax if there's no need to.

Patch
diff mbox series

diff --git a/lib/test_printf.c b/lib/test_printf.c
index 944eb50f38625..9c6d716979fb1 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -22,6 +22,8 @@ 
 #include <linux/gfp.h>
 #include <linux/mm.h>
 
+#include <linux/property.h>
+
 #include "../tools/testing/selftests/kselftest_module.h"
 
 #define BUF_SIZE 256
@@ -588,6 +590,40 @@  flags(void)
 	kfree(cmp_buffer);
 }
 
+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 = "/second/third";
+	const char * const full_name_second = "/second";
+	const char * const second_name = "second";
+	const char * const third_name = "third";
+	int rval;
+
+	rval = software_node_register_nodes(softnodes);
+	if (rval) {
+		pr_warn("cannot register softnodes; rval %d\n", rval);
+		return;
+	}
+
+	test(full_name_second, "%pfw",
+	     software_node_fwnode(&softnodes[ARRAY_SIZE(softnodes) - 3]));
+	test(full_name, "%pfw",
+	     software_node_fwnode(&softnodes[ARRAY_SIZE(softnodes) - 2]));
+	test(full_name, "%pfwf",
+	     software_node_fwnode(&softnodes[ARRAY_SIZE(softnodes) - 2]));
+	test(second_name, "%pfwP",
+	     software_node_fwnode(&softnodes[ARRAY_SIZE(softnodes) - 3]));
+	test(third_name, "%pfwP",
+	     software_node_fwnode(&softnodes[ARRAY_SIZE(softnodes) - 2]));
+
+	software_node_unregister_nodes(softnodes);
+}
+
 static void __init
 test_pointer(void)
 {
@@ -610,6 +646,7 @@  test_pointer(void)
 	bitmap();
 	netdev_features();
 	flags();
+	fwnode_pointer();
 }
 
 static void __init selftest(void)