| Message ID | 20190902135732.23455-12-sakari.ailus@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Device property improvements, add %pfw format specifier | expand |
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.
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.
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.
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.
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)