* [PATCH] of: Drop redundant check in linker section OF match table @ 2019-03-20 10:49 Mukesh Ojha 2019-03-20 19:55 ` Frank Rowand 0 siblings, 1 reply; 3+ messages in thread From: Mukesh Ojha @ 2019-03-20 10:49 UTC (permalink / raw) To: linux-kernel Cc: Mukesh Ojha, Rob Herring, Frank Rowand, Pantelis Antoniou, devicetree Existing check of `fn` against NULL inside OF match table is redundant. Remove the check. Signed-off-by: Mukesh Ojha <mojha@codeaurora.org> Cc: Rob Herring <robh@kernel.org> Cc: Frank Rowand <frowand.list@gmail.com> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> Cc: devicetree@vger.kernel.org --- include/linux/of.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/of.h b/include/linux/of.h index e240992..b86c00a 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -1283,13 +1283,13 @@ static inline int of_get_available_child_count(const struct device_node *np) static const struct of_device_id __of_table_##name \ __used __section(__##table##_of_table) \ = { .compatible = compat, \ - .data = (fn == (fn_type)NULL) ? fn : fn } + .data = fn } #else #define _OF_DECLARE(table, name, compat, fn, fn_type) \ static const struct of_device_id __of_table_##name \ __attribute__((unused)) \ = { .compatible = compat, \ - .data = (fn == (fn_type)NULL) ? fn : fn } + .data = fn } #endif typedef int (*of_init_fn_2)(struct device_node *, struct device_node *); -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] of: Drop redundant check in linker section OF match table 2019-03-20 10:49 [PATCH] of: Drop redundant check in linker section OF match table Mukesh Ojha @ 2019-03-20 19:55 ` Frank Rowand 2019-03-21 7:39 ` Mukesh Ojha 0 siblings, 1 reply; 3+ messages in thread From: Frank Rowand @ 2019-03-20 19:55 UTC (permalink / raw) To: Mukesh Ojha, linux-kernel; +Cc: Rob Herring, Pantelis Antoniou, devicetree On 3/20/19 3:49 AM, Mukesh Ojha wrote: > Existing check of `fn` against NULL inside OF match table > is redundant. Remove the check. > > Signed-off-by: Mukesh Ojha <mojha@codeaurora.org> > Cc: Rob Herring <robh@kernel.org> > Cc: Frank Rowand <frowand.list@gmail.com> > Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> > Cc: devicetree@vger.kernel.org > --- > include/linux/of.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/linux/of.h b/include/linux/of.h > index e240992..b86c00a 100644 > --- a/include/linux/of.h > +++ b/include/linux/of.h > @@ -1283,13 +1283,13 @@ static inline int of_get_available_child_count(const struct device_node *np) > static const struct of_device_id __of_table_##name \ > __used __section(__##table##_of_table) \ > = { .compatible = compat, \ > - .data = (fn == (fn_type)NULL) ? fn : fn } > + .data = fn } > #else > #define _OF_DECLARE(table, name, compat, fn, fn_type) \ > static const struct of_device_id __of_table_##name \ > __attribute__((unused)) \ > = { .compatible = compat, \ > - .data = (fn == (fn_type)NULL) ? fn : fn } > + .data = fn } > #endif > > typedef int (*of_init_fn_2)(struct device_node *, struct device_node *); > The check is not redundant and does serve a purpose. The purpose is not very obvious on the surface, but it is checking that the function fn() is of the proper type. An example of a compiler warning with a bad function type is created by applying the following patch: drivers/of/unittest.c:62:1: warning: comparison of distinct pointer types lacks a cast [enabled by default] Note that you need to have CONFIG_UNITTEST enabled to compile unittest.c. Line 62 is OF_DECLARE_1(__unittest_of_table, unittest_setup_2_bad, "unittest_compat", --- drivers/of/unittest.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) Index: b/drivers/of/unittest.c =================================================================== --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -45,6 +45,23 @@ static struct unittest_results { failed; \ }) +struct of_device_id __unittest_of_table; + +static void __init unittest_setup_1_good(struct device_node *np) +{ +} + +static void __init unittest_setup_2_bad(struct device_node *np_1, + struct device_node *np_2) +{ +} + +OF_DECLARE_1(__unittest_of_table, unittest_setup_1_good, "unittest_compat", + unittest_setup_1_good); + +OF_DECLARE_1(__unittest_of_table, unittest_setup_2_bad, "unittest_compat", + unittest_setup_2_bad); + static void __init of_unittest_find_node_by_name(void) { struct device_node *np; ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] of: Drop redundant check in linker section OF match table 2019-03-20 19:55 ` Frank Rowand @ 2019-03-21 7:39 ` Mukesh Ojha 0 siblings, 0 replies; 3+ messages in thread From: Mukesh Ojha @ 2019-03-21 7:39 UTC (permalink / raw) To: Frank Rowand, linux-kernel; +Cc: Rob Herring, Pantelis Antoniou, devicetree On 3/21/2019 1:25 AM, Frank Rowand wrote: > On 3/20/19 3:49 AM, Mukesh Ojha wrote: >> Existing check of `fn` against NULL inside OF match table >> is redundant. Remove the check. >> >> Signed-off-by: Mukesh Ojha <mojha@codeaurora.org> >> Cc: Rob Herring <robh@kernel.org> >> Cc: Frank Rowand <frowand.list@gmail.com> >> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com> >> Cc: devicetree@vger.kernel.org >> --- >> include/linux/of.h | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/include/linux/of.h b/include/linux/of.h >> index e240992..b86c00a 100644 >> --- a/include/linux/of.h >> +++ b/include/linux/of.h >> @@ -1283,13 +1283,13 @@ static inline int of_get_available_child_count(const struct device_node *np) >> static const struct of_device_id __of_table_##name \ >> __used __section(__##table##_of_table) \ >> = { .compatible = compat, \ >> - .data = (fn == (fn_type)NULL) ? fn : fn } >> + .data = fn } >> #else >> #define _OF_DECLARE(table, name, compat, fn, fn_type) \ >> static const struct of_device_id __of_table_##name \ >> __attribute__((unused)) \ >> = { .compatible = compat, \ >> - .data = (fn == (fn_type)NULL) ? fn : fn } >> + .data = fn } >> #endif >> >> typedef int (*of_init_fn_2)(struct device_node *, struct device_node *); >> > The check is not redundant and does serve a purpose. > > The purpose is not very obvious on the surface, but it is checking that > the function fn() is of the proper type. Frank, Thanks for taking out time to explain the stuff, Yeah some miscellaneous driver can do this type of mistake. -Mukesh > An example of a compiler warning with a bad function type is created > by applying the following patch: > > drivers/of/unittest.c:62:1: warning: comparison of distinct pointer types lacks a cast [enabled by default] > > Note that you need to have CONFIG_UNITTEST enabled to compile unittest.c. > > Line 62 is > OF_DECLARE_1(__unittest_of_table, unittest_setup_2_bad, "unittest_compat", > > --- > drivers/of/unittest.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > Index: b/drivers/of/unittest.c > =================================================================== > --- a/drivers/of/unittest.c > +++ b/drivers/of/unittest.c > @@ -45,6 +45,23 @@ static struct unittest_results { > failed; \ > }) > > +struct of_device_id __unittest_of_table; > + > +static void __init unittest_setup_1_good(struct device_node *np) > +{ > +} > + > +static void __init unittest_setup_2_bad(struct device_node *np_1, > + struct device_node *np_2) > +{ > +} > + > +OF_DECLARE_1(__unittest_of_table, unittest_setup_1_good, "unittest_compat", > + unittest_setup_1_good); > + > +OF_DECLARE_1(__unittest_of_table, unittest_setup_2_bad, "unittest_compat", > + unittest_setup_2_bad); > + > static void __init of_unittest_find_node_by_name(void) > { > struct device_node *np; ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-03-21 7:39 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-20 10:49 [PATCH] of: Drop redundant check in linker section OF match table Mukesh Ojha 2019-03-20 19:55 ` Frank Rowand 2019-03-21 7:39 ` Mukesh Ojha
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.