| Message ID | 20230207161316.293923-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: i2c: imx290: fix conditional function defintions | expand |
Hi Arnd, Thank you for the patch. On Tue, Feb 07, 2023 at 05:13:12PM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > The runtime suspend/resume functions are only referenced from the > dev_pm_ops, but they use the old SET_RUNTIME_PM_OPS() helper > that requires a __maybe_unused annotation to avoid a warning: > > drivers/media/i2c/imx290.c:1082:12: error: unused function 'imx290_runtime_resume' [-Werror,-Wunused-function] > static int imx290_runtime_resume(struct device *dev) > ^ > drivers/media/i2c/imx290.c:1090:12: error: unused function 'imx290_runtime_suspend' [-Werror,-Wunused-function] > static int imx290_runtime_suspend(struct device *dev) > ^ > > Convert this to the new RUNTIME_PM_OPS() helper that so this > is not required. To improve this further, also use the pm_ptr() > helper that lets the dev_pm_ops get dropped entirely when > CONFIG_PM is disabled. > > A related mistake happened in the of_match_ptr() macro here, which > like SET_RUNTIME_PM_OPS() requires the match table to be marked > as __maybe_unused, though I could not reproduce building this without > CONFIG_OF. Remove the of_match_ptr() here as there is no point in > dropping the match table in configurations without CONFIG_OF. > > Fixes: 02852c01f654 ("media: i2c: imx290: Initialize runtime PM before subdev") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/media/i2c/imx290.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c > index 49d6c8bdec41..48ae2e0adf9e 100644 > --- a/drivers/media/i2c/imx290.c > +++ b/drivers/media/i2c/imx290.c > @@ -1098,7 +1098,7 @@ static int imx290_runtime_suspend(struct device *dev) > } > > static const struct dev_pm_ops imx290_pm_ops = { > - SET_RUNTIME_PM_OPS(imx290_runtime_suspend, imx290_runtime_resume, NULL) > + RUNTIME_PM_OPS(imx290_runtime_suspend, imx290_runtime_resume, NULL) > }; > > /* ---------------------------------------------------------------------------- > @@ -1362,8 +1362,8 @@ static struct i2c_driver imx290_i2c_driver = { > .remove = imx290_remove, > .driver = { > .name = "imx290", > - .pm = &imx290_pm_ops, > - .of_match_table = of_match_ptr(imx290_of_match), > + .pm = pm_ptr(&imx290_pm_ops), > + .of_match_table = imx290_of_match, > }, > }; >
On 07.02.23 17:40, Laurent Pinchart wrote: > On Tue, Feb 07, 2023 at 05:13:12PM +0100, Arnd Bergmann wrote: >> From: Arnd Bergmann <arnd@arndb.de> >> >> The runtime suspend/resume functions are only referenced from the >> dev_pm_ops, but they use the old SET_RUNTIME_PM_OPS() helper >> that requires a __maybe_unused annotation to avoid a warning: >> >> drivers/media/i2c/imx290.c:1082:12: error: unused function 'imx290_runtime_resume' [-Werror,-Wunused-function] >> static int imx290_runtime_resume(struct device *dev) >> ^ >> drivers/media/i2c/imx290.c:1090:12: error: unused function 'imx290_runtime_suspend' [-Werror,-Wunused-function] >> static int imx290_runtime_suspend(struct device *dev) >> ^ >> I might be missing something (if so, please tell me), but to me it looks this fix for a build issue in 6.3-rc (which shows up in Guenters weekly reports to Linus) didn't make any progress in the past few weeks. Is there a reason why? Who actually needs to pick it up and send it towards mainline? Manivannan Sadhasivam? Sakari Ailus? Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page. #regzbot poke >> Convert this to the new RUNTIME_PM_OPS() helper that so this >> is not required. To improve this further, also use the pm_ptr() >> helper that lets the dev_pm_ops get dropped entirely when >> CONFIG_PM is disabled. >> >> A related mistake happened in the of_match_ptr() macro here, which >> like SET_RUNTIME_PM_OPS() requires the match table to be marked >> as __maybe_unused, though I could not reproduce building this without >> CONFIG_OF. Remove the of_match_ptr() here as there is no point in >> dropping the match table in configurations without CONFIG_OF. >> >> Fixes: 02852c01f654 ("media: i2c: imx290: Initialize runtime PM before subdev") >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> --- >> drivers/media/i2c/imx290.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c >> index 49d6c8bdec41..48ae2e0adf9e 100644 >> --- a/drivers/media/i2c/imx290.c >> +++ b/drivers/media/i2c/imx290.c >> @@ -1098,7 +1098,7 @@ static int imx290_runtime_suspend(struct device *dev) >> } >> >> static const struct dev_pm_ops imx290_pm_ops = { >> - SET_RUNTIME_PM_OPS(imx290_runtime_suspend, imx290_runtime_resume, NULL) >> + RUNTIME_PM_OPS(imx290_runtime_suspend, imx290_runtime_resume, NULL) >> }; >> >> /* ---------------------------------------------------------------------------- >> @@ -1362,8 +1362,8 @@ static struct i2c_driver imx290_i2c_driver = { >> .remove = imx290_remove, >> .driver = { >> .name = "imx290", >> - .pm = &imx290_pm_ops, >> - .of_match_table = of_match_ptr(imx290_of_match), >> + .pm = pm_ptr(&imx290_pm_ops), >> + .of_match_table = imx290_of_match, >> }, >> }; >> >
On Tue, Feb 07, 2023 at 05:13:12PM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > The runtime suspend/resume functions are only referenced from the > dev_pm_ops, but they use the old SET_RUNTIME_PM_OPS() helper > that requires a __maybe_unused annotation to avoid a warning: > > drivers/media/i2c/imx290.c:1082:12: error: unused function 'imx290_runtime_resume' [-Werror,-Wunused-function] > static int imx290_runtime_resume(struct device *dev) > ^ > drivers/media/i2c/imx290.c:1090:12: error: unused function 'imx290_runtime_suspend' [-Werror,-Wunused-function] > static int imx290_runtime_suspend(struct device *dev) > ^ > > Convert this to the new RUNTIME_PM_OPS() helper that so this > is not required. To improve this further, also use the pm_ptr() > helper that lets the dev_pm_ops get dropped entirely when > CONFIG_PM is disabled. > > A related mistake happened in the of_match_ptr() macro here, which > like SET_RUNTIME_PM_OPS() requires the match table to be marked > as __maybe_unused, though I could not reproduce building this without > CONFIG_OF. Remove the of_match_ptr() here as there is no point in > dropping the match table in configurations without CONFIG_OF. > > Fixes: 02852c01f654 ("media: i2c: imx290: Initialize runtime PM before subdev") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Reviewed-by: Manivannan Sadhasivam <mani@kernel.org> Thanks, Mani > --- > drivers/media/i2c/imx290.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c > index 49d6c8bdec41..48ae2e0adf9e 100644 > --- a/drivers/media/i2c/imx290.c > +++ b/drivers/media/i2c/imx290.c > @@ -1098,7 +1098,7 @@ static int imx290_runtime_suspend(struct device *dev) > } > > static const struct dev_pm_ops imx290_pm_ops = { > - SET_RUNTIME_PM_OPS(imx290_runtime_suspend, imx290_runtime_resume, NULL) > + RUNTIME_PM_OPS(imx290_runtime_suspend, imx290_runtime_resume, NULL) > }; > > /* ---------------------------------------------------------------------------- > @@ -1362,8 +1362,8 @@ static struct i2c_driver imx290_i2c_driver = { > .remove = imx290_remove, > .driver = { > .name = "imx290", > - .pm = &imx290_pm_ops, > - .of_match_table = of_match_ptr(imx290_of_match), > + .pm = pm_ptr(&imx290_pm_ops), > + .of_match_table = imx290_of_match, > }, > }; > > -- > 2.39.1 >
Hello, On Mon, Mar 20, 2023 at 10:18:23AM +0100, Linux regression tracking (Thorsten Leemhuis) wrote: > On 07.02.23 17:40, Laurent Pinchart wrote: > > On Tue, Feb 07, 2023 at 05:13:12PM +0100, Arnd Bergmann wrote: > >> From: Arnd Bergmann <arnd@arndb.de> > >> > >> The runtime suspend/resume functions are only referenced from the > >> dev_pm_ops, but they use the old SET_RUNTIME_PM_OPS() helper > >> that requires a __maybe_unused annotation to avoid a warning: > >> > >> drivers/media/i2c/imx290.c:1082:12: error: unused function 'imx290_runtime_resume' [-Werror,-Wunused-function] > >> static int imx290_runtime_resume(struct device *dev) > >> ^ > >> drivers/media/i2c/imx290.c:1090:12: error: unused function 'imx290_runtime_suspend' [-Werror,-Wunused-function] > >> static int imx290_runtime_suspend(struct device *dev) > >> ^ > >> > > I might be missing something (if so, please tell me), but to me it looks > this fix for a build issue in 6.3-rc (which shows up in Guenters weekly > reports to Linus) didn't make any progress in the past few weeks. Is > there a reason why? Who actually needs to pick it up and send it towards > mainline? Manivannan Sadhasivam? Sakari Ailus? Mauro has recently pulled my PR including this and I understand it's in Mauro's tree now.
On 20.03.23 10:32, Sakari Ailus wrote: > On Mon, Mar 20, 2023 at 10:18:23AM +0100, Linux regression tracking (Thorsten Leemhuis) wrote: >> On 07.02.23 17:40, Laurent Pinchart wrote: >>> On Tue, Feb 07, 2023 at 05:13:12PM +0100, Arnd Bergmann wrote: >>>> From: Arnd Bergmann <arnd@arndb.de> >>>> >>>> The runtime suspend/resume functions are only referenced from the >>>> dev_pm_ops, but they use the old SET_RUNTIME_PM_OPS() helper >>>> that requires a __maybe_unused annotation to avoid a warning: >>>> >>>> drivers/media/i2c/imx290.c:1082:12: error: unused function 'imx290_runtime_resume' [-Werror,-Wunused-function] >>>> static int imx290_runtime_resume(struct device *dev) >>>> ^ >>>> drivers/media/i2c/imx290.c:1090:12: error: unused function 'imx290_runtime_suspend' [-Werror,-Wunused-function] >>>> static int imx290_runtime_suspend(struct device *dev) >>>> ^ >>>> >> >> I might be missing something (if so, please tell me), but to me it looks >> this fix for a build issue in 6.3-rc (which shows up in Guenters weekly >> reports to Linus) didn't make any progress in the past few weeks. Is >> there a reason why? Who actually needs to pick it up and send it towards >> mainline? Manivannan Sadhasivam? Sakari Ailus? > > Mauro has recently pulled my PR including this and I understand it's in > Mauro's tree now. Ahh, great, many thx. I checked Linux-next before writing my mail, but didn't sport it there. :-/ Ciao, Thorsten
On 20.03.23 10:42, Linux regression tracking (Thorsten Leemhuis) wrote: > On 20.03.23 10:32, Sakari Ailus wrote: >> On Mon, Mar 20, 2023 at 10:18:23AM +0100, Linux regression tracking (Thorsten Leemhuis) wrote: >>> On 07.02.23 17:40, Laurent Pinchart wrote: >>>> On Tue, Feb 07, 2023 at 05:13:12PM +0100, Arnd Bergmann wrote: >>>>> From: Arnd Bergmann <arnd@arndb.de> >>>>> >>>>> The runtime suspend/resume functions are only referenced from the >>>>> dev_pm_ops, but they use the old SET_RUNTIME_PM_OPS() helper >>>>> that requires a __maybe_unused annotation to avoid a warning: >>>>> >>>>> drivers/media/i2c/imx290.c:1082:12: error: unused function 'imx290_runtime_resume' [-Werror,-Wunused-function] >>>>> static int imx290_runtime_resume(struct device *dev) >>>>> ^ >>>>> drivers/media/i2c/imx290.c:1090:12: error: unused function 'imx290_runtime_suspend' [-Werror,-Wunused-function] >>>>> static int imx290_runtime_suspend(struct device *dev) >>>>> ^ >>>>> >>> >>> I might be missing something (if so, please tell me), but to me it looks >>> this fix for a build issue in 6.3-rc (which shows up in Guenters weekly >>> reports to Linus) didn't make any progress in the past few weeks. Is >>> there a reason why? Who actually needs to pick it up and send it towards >>> mainline? Manivannan Sadhasivam? Sakari Ailus? >> >> Mauro has recently pulled my PR including this and I understand it's in >> Mauro's tree now. > > Ahh, great, many thx. > > I checked Linux-next before writing my mail, but didn't sport it there. :-/ Argh, ohh no, it's there. Sorry for the noise. Second time something like this happens today. Seems today is not my best day. :-/ Ciao, Thorsten
On Mon, Mar 20, 2023 at 10:46:45AM +0100, Thorsten Leemhuis wrote: > Argh, ohh no, it's there. Sorry for the noise. Second time something > like this happens today. Seems today is not my best day. :-/ No worries!
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c index 49d6c8bdec41..48ae2e0adf9e 100644 --- a/drivers/media/i2c/imx290.c +++ b/drivers/media/i2c/imx290.c @@ -1098,7 +1098,7 @@ static int imx290_runtime_suspend(struct device *dev) } static const struct dev_pm_ops imx290_pm_ops = { - SET_RUNTIME_PM_OPS(imx290_runtime_suspend, imx290_runtime_resume, NULL) + RUNTIME_PM_OPS(imx290_runtime_suspend, imx290_runtime_resume, NULL) }; /* ---------------------------------------------------------------------------- @@ -1362,8 +1362,8 @@ static struct i2c_driver imx290_i2c_driver = { .remove = imx290_remove, .driver = { .name = "imx290", - .pm = &imx290_pm_ops, - .of_match_table = of_match_ptr(imx290_of_match), + .pm = pm_ptr(&imx290_pm_ops), + .of_match_table = imx290_of_match, }, };