linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] media: i2c: imx290: fix conditional function defintions
@ 2023-02-07 16:13 Arnd Bergmann
  2023-02-07 16:40 ` Laurent Pinchart
  2023-03-20  9:22 ` Manivannan Sadhasivam
  0 siblings, 2 replies; 8+ messages in thread
From: Arnd Bergmann @ 2023-02-07 16:13 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Mauro Carvalho Chehab, Sakari Ailus,
	Alexander Stein, Laurent Pinchart
  Cc: Arnd Bergmann, Dave Stevenson, linux-media, linux-kernel

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>
---
 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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] media: i2c: imx290: fix conditional function defintions
  2023-02-07 16:13 [PATCH] media: i2c: imx290: fix conditional function defintions Arnd Bergmann
@ 2023-02-07 16:40 ` Laurent Pinchart
  2023-03-20  9:18   ` Linux regression tracking (Thorsten Leemhuis)
  2023-03-20  9:22 ` Manivannan Sadhasivam
  1 sibling, 1 reply; 8+ messages in thread
From: Laurent Pinchart @ 2023-02-07 16:40 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Manivannan Sadhasivam, Mauro Carvalho Chehab, Sakari Ailus,
	Alexander Stein, Arnd Bergmann, Dave Stevenson, linux-media,
	linux-kernel

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,
>  	},
>  };
>  

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] media: i2c: imx290: fix conditional function defintions
  2023-02-07 16:40 ` Laurent Pinchart
@ 2023-03-20  9:18   ` Linux regression tracking (Thorsten Leemhuis)
  2023-03-20  9:32     ` Sakari Ailus
  0 siblings, 1 reply; 8+ messages in thread
From: Linux regression tracking (Thorsten Leemhuis) @ 2023-03-20  9:18 UTC (permalink / raw)
  To: Sakari Ailus, Manivannan Sadhasivam
  Cc: Mauro Carvalho Chehab, Alexander Stein, Arnd Bergmann,
	Dave Stevenson, linux-media, linux-kernel, Laurent Pinchart,
	Arnd Bergmann, Linux kernel regressions list

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,
>>  	},
>>  };
>>  
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] media: i2c: imx290: fix conditional function defintions
  2023-02-07 16:13 [PATCH] media: i2c: imx290: fix conditional function defintions Arnd Bergmann
  2023-02-07 16:40 ` Laurent Pinchart
@ 2023-03-20  9:22 ` Manivannan Sadhasivam
  1 sibling, 0 replies; 8+ messages in thread
From: Manivannan Sadhasivam @ 2023-03-20  9:22 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Mauro Carvalho Chehab, Sakari Ailus, Alexander Stein,
	Laurent Pinchart, Arnd Bergmann, Dave Stevenson, linux-media,
	linux-kernel

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
> 

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] media: i2c: imx290: fix conditional function defintions
  2023-03-20  9:18   ` Linux regression tracking (Thorsten Leemhuis)
@ 2023-03-20  9:32     ` Sakari Ailus
  2023-03-20  9:42       ` Linux regression tracking (Thorsten Leemhuis)
  0 siblings, 1 reply; 8+ messages in thread
From: Sakari Ailus @ 2023-03-20  9:32 UTC (permalink / raw)
  To: Linux regressions mailing list
  Cc: Manivannan Sadhasivam, Mauro Carvalho Chehab, Alexander Stein,
	Arnd Bergmann, Dave Stevenson, linux-media, linux-kernel,
	Laurent Pinchart, Arnd Bergmann

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.

-- 
Kind regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] media: i2c: imx290: fix conditional function defintions
  2023-03-20  9:32     ` Sakari Ailus
@ 2023-03-20  9:42       ` Linux regression tracking (Thorsten Leemhuis)
  2023-03-20  9:46         ` Thorsten Leemhuis
  0 siblings, 1 reply; 8+ messages in thread
From: Linux regression tracking (Thorsten Leemhuis) @ 2023-03-20  9:42 UTC (permalink / raw)
  To: Sakari Ailus, Linux regressions mailing list
  Cc: Manivannan Sadhasivam, Mauro Carvalho Chehab, Alexander Stein,
	Arnd Bergmann, Dave Stevenson, linux-media, linux-kernel,
	Laurent Pinchart, Arnd Bergmann

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] media: i2c: imx290: fix conditional function defintions
  2023-03-20  9:42       ` Linux regression tracking (Thorsten Leemhuis)
@ 2023-03-20  9:46         ` Thorsten Leemhuis
  2023-03-20  9:51           ` Sakari Ailus
  0 siblings, 1 reply; 8+ messages in thread
From: Thorsten Leemhuis @ 2023-03-20  9:46 UTC (permalink / raw)
  To: Sakari Ailus, Linux regressions mailing list
  Cc: Manivannan Sadhasivam, Mauro Carvalho Chehab, Alexander Stein,
	Arnd Bergmann, Dave Stevenson, linux-media, linux-kernel,
	Laurent Pinchart, Arnd Bergmann



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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] media: i2c: imx290: fix conditional function defintions
  2023-03-20  9:46         ` Thorsten Leemhuis
@ 2023-03-20  9:51           ` Sakari Ailus
  0 siblings, 0 replies; 8+ messages in thread
From: Sakari Ailus @ 2023-03-20  9:51 UTC (permalink / raw)
  To: Linux regressions mailing list
  Cc: Manivannan Sadhasivam, Mauro Carvalho Chehab, Alexander Stein,
	Arnd Bergmann, Dave Stevenson, linux-media, linux-kernel,
	Laurent Pinchart, Arnd Bergmann

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!

-- 
Sakari Ailus

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-03-20  9:51 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-07 16:13 [PATCH] media: i2c: imx290: fix conditional function defintions Arnd Bergmann
2023-02-07 16:40 ` Laurent Pinchart
2023-03-20  9:18   ` Linux regression tracking (Thorsten Leemhuis)
2023-03-20  9:32     ` Sakari Ailus
2023-03-20  9:42       ` Linux regression tracking (Thorsten Leemhuis)
2023-03-20  9:46         ` Thorsten Leemhuis
2023-03-20  9:51           ` Sakari Ailus
2023-03-20  9:22 ` Manivannan Sadhasivam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).