| Message ID | 20220310065045.24772-1-yuehaibing@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,-next] perf/marvell: cn10k Fix build error without CONFIG_OF | expand |
On Thu, Mar 10, 2022 at 7:50 AM YueHaibing <yuehaibing@huawei.com> wrote: > > drivers/perf/marvell_cn10k_ddr_pmu.c:723:21: error: ‘cn10k_ddr_pmu_of_match’ undeclared here (not in a function); did you mean ‘cn10k_ddr_pmu_driver’? > .of_match_table = cn10k_ddr_pmu_of_match, > ^~~~~~~~~~~~~~~~~~~~~~ > > Remove the #ifdef around the match table, because CONFIG_OF is always enabled on arm64. > > Fixes: 7cf83e222bce ("perf/marvell: CN10k DDR performance monitor support") > Signed-off-by: YueHaibing <yuehaibing@huawei.com> > --- > v2: Remove the #ifdef macro as Arnd suggested Reviewed-by: Arnd Bergmann <arnd@arndb.de>
On Thu, Mar 10, 2022 at 02:50:45PM +0800, YueHaibing wrote: > drivers/perf/marvell_cn10k_ddr_pmu.c:723:21: error: ‘cn10k_ddr_pmu_of_match’ undeclared here (not in a function); did you mean ‘cn10k_ddr_pmu_driver’? > .of_match_table = cn10k_ddr_pmu_of_match, > ^~~~~~~~~~~~~~~~~~~~~~ > > Remove the #ifdef around the match table, because CONFIG_OF is always enabled on arm64. > > Fixes: 7cf83e222bce ("perf/marvell: CN10k DDR performance monitor support") > Signed-off-by: YueHaibing <yuehaibing@huawei.com> > --- > v2: Remove the #ifdef macro as Arnd suggested > --- > drivers/perf/marvell_cn10k_ddr_pmu.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/perf/marvell_cn10k_ddr_pmu.c b/drivers/perf/marvell_cn10k_ddr_pmu.c > index 7f3146e71f99..bef0cee3a46a 100644 > --- a/drivers/perf/marvell_cn10k_ddr_pmu.c > +++ b/drivers/perf/marvell_cn10k_ddr_pmu.c > @@ -709,13 +709,11 @@ static int cn10k_ddr_perf_remove(struct platform_device *pdev) > return 0; > } > > -#ifdef CONFIG_OF > static const struct of_device_id cn10k_ddr_pmu_of_match[] = { > { .compatible = "marvell,cn10k-ddr-pmu", }, > { }, > }; > MODULE_DEVICE_TABLE(of, cn10k_ddr_pmu_of_match); > -#endif Ah, sorry, I already fixed this when the conflict was first reported: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?h=for-next/perf&id=6676a42f1e4f1b8ec166b723a3801b7113c25a0e However, I thought this driver could be compile-tested on architectures without OF and then we'd get some report from that? At least, I'm certain I've _added_ these ifdefs to other PMU drivers in the past. Will
On Thu, Mar 10, 2022 at 10:58 AM Will Deacon <will@kernel.org> wrote: > On Thu, Mar 10, 2022 at 02:50:45PM +0800, YueHaibing wrote: > > Ah, sorry, I already fixed this when the conflict was first reported: > > https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?h=for-next/perf&id=6676a42f1e4f1b8ec166b723a3801b7113c25a0e > > However, I thought this driver could be compile-tested on architectures > without OF and then we'd get some report from that? At least, I'm certain > I've _added_ these ifdefs to other PMU drivers in the past. The #ifdefs are never really needed, the only reason to have them is to save a few bytes on architectures that don't normally use DT, at the expense of making it slightly less readable. For compile-testing purposes we don't care about the size of the module, and compiling in the table unconditionally is easier. Arnd
On Thu, Mar 10, 2022 at 11:04:21AM +0100, Arnd Bergmann wrote: > On Thu, Mar 10, 2022 at 10:58 AM Will Deacon <will@kernel.org> wrote: > > On Thu, Mar 10, 2022 at 02:50:45PM +0800, YueHaibing wrote: > > > > Ah, sorry, I already fixed this when the conflict was first reported: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?h=for-next/perf&id=6676a42f1e4f1b8ec166b723a3801b7113c25a0e > > > > However, I thought this driver could be compile-tested on architectures > > without OF and then we'd get some report from that? At least, I'm certain > > I've _added_ these ifdefs to other PMU drivers in the past. > > The #ifdefs are never really needed, the only reason to have them is > to save a few bytes on architectures that don't normally use DT, > at the expense of making it slightly less readable. > > For compile-testing purposes we don't care about the size of the module, > and compiling in the table unconditionally is easier. I think the problem is when the #ifdefs are removed but the use of of_match_ptr() remains, leading to reports from the robot: https://lore.kernel.org/r/202201041700.01KZEzhb-lkp@intel.com Should we therefore remove of_match_ptr() altogether? It seems like it's leading people in the wrong direction here. Will
On Thu, Mar 10, 2022 at 11:21 AM Will Deacon <will@kernel.org> wrote: > On Thu, Mar 10, 2022 at 11:04:21AM +0100, Arnd Bergmann wrote: > > On Thu, Mar 10, 2022 at 10:58 AM Will Deacon <will@kernel.org> wrote: > > I think the problem is when the #ifdefs are removed but the use of > of_match_ptr() remains, leading to reports from the robot: > > https://lore.kernel.org/r/202201041700.01KZEzhb-lkp@intel.com > > Should we therefore remove of_match_ptr() altogether? It seems like it's > leading people in the wrong direction here. There may be valid uses for it, e.g. if the array referenced by it is in a separate, and conditionally compiled file, or if the #ifdef block covers more than just the table. Removing 1563 instances of it also takes a lot of work. Almost all of them are probably useless, and quite a lot of them are actively wrong, because they refer to arrays without an #ifdef. Maybe coccinelle can help here. Arnd
diff --git a/drivers/perf/marvell_cn10k_ddr_pmu.c b/drivers/perf/marvell_cn10k_ddr_pmu.c index 7f3146e71f99..bef0cee3a46a 100644 --- a/drivers/perf/marvell_cn10k_ddr_pmu.c +++ b/drivers/perf/marvell_cn10k_ddr_pmu.c @@ -709,13 +709,11 @@ static int cn10k_ddr_perf_remove(struct platform_device *pdev) return 0; } -#ifdef CONFIG_OF static const struct of_device_id cn10k_ddr_pmu_of_match[] = { { .compatible = "marvell,cn10k-ddr-pmu", }, { }, }; MODULE_DEVICE_TABLE(of, cn10k_ddr_pmu_of_match); -#endif static struct platform_driver cn10k_ddr_pmu_driver = { .driver = {
drivers/perf/marvell_cn10k_ddr_pmu.c:723:21: error: ‘cn10k_ddr_pmu_of_match’ undeclared here (not in a function); did you mean ‘cn10k_ddr_pmu_driver’? .of_match_table = cn10k_ddr_pmu_of_match, ^~~~~~~~~~~~~~~~~~~~~~ Remove the #ifdef around the match table, because CONFIG_OF is always enabled on arm64. Fixes: 7cf83e222bce ("perf/marvell: CN10k DDR performance monitor support") Signed-off-by: YueHaibing <yuehaibing@huawei.com> --- v2: Remove the #ifdef macro as Arnd suggested --- drivers/perf/marvell_cn10k_ddr_pmu.c | 2 -- 1 file changed, 2 deletions(-)