[PATCH v2] staging: media: atomisp: Use str_on_off() to fix Coccinelle warning

[PATCH v2] staging: media: atomisp: Use str_on_off() to fix Coccinelle warning

[Nick Spooner]

Fixes the following Coccinelle warning reported by string_choices.cocci:

	opportunity for str_on_off(on)

Signed-off-by: Nick Spooner <nicholas.spooner@seagate.com>
---
Changes in v2:
  - Include "media" in the subject line.

 drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
index 4026e98c5845..322eca4a3755 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
@@ -13,6 +13,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/gpio.h>
 #include <linux/platform_device.h>
+#include <linux/string_choices.h>
 #include "../../include/linux/atomisp_platform.h"
 #include "../../include/linux/atomisp_gmin_platform.h"
 
@@ -917,7 +918,7 @@ static int gmin_acpi_pm_ctrl(struct v4l2_subdev *subdev, int on)
 		return 0;
 
 	dev_dbg(subdev->dev, "Setting power state to %s\n",
-		on ? "on" : "off");
+		str_on_off(on));
 
 	if (on)
 		ret = acpi_device_set_power(adev,
@@ -930,7 +931,7 @@ static int gmin_acpi_pm_ctrl(struct v4l2_subdev *subdev, int on)
 		gs->clock_on = on;
 	else
 		dev_err(subdev->dev, "Couldn't set power state to %s\n",
-			on ? "on" : "off");
+			str_on_off(on));
 
 	return ret;
 }
-- 
2.53.0

Re: [PATCH v2] staging: media: atomisp: Use str_on_off() to fix Coccinelle warning

[Sakari Ailus]

Hi Nick,

On Thu, Apr 23, 2026 at 01:47:27PM -0600, Nick Spooner wrote:
> Fixes the following Coccinelle warning reported by string_choices.cocci:
> 
> 	opportunity for str_on_off(on)
> 
> Signed-off-by: Nick Spooner <nicholas.spooner@seagate.com>
> ---
> Changes in v2:
>   - Include "media" in the subject line.

If there's only such a change that you'd do right after submitting a patch,
please wait some time for other review comments first.

> 
>  drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> index 4026e98c5845..322eca4a3755 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> @@ -13,6 +13,7 @@
>  #include <linux/gpio/consumer.h>
>  #include <linux/gpio.h>
>  #include <linux/platform_device.h>
> +#include <linux/string_choices.h>
>  #include "../../include/linux/atomisp_platform.h"
>  #include "../../include/linux/atomisp_gmin_platform.h"
>  
> @@ -917,7 +918,7 @@ static int gmin_acpi_pm_ctrl(struct v4l2_subdev *subdev, int on)
>  		return 0;
>  
>  	dev_dbg(subdev->dev, "Setting power state to %s\n",
> -		on ? "on" : "off");
> +		str_on_off(on));

This fits on the previous line.

>  
>  	if (on)
>  		ret = acpi_device_set_power(adev,
> @@ -930,7 +931,7 @@ static int gmin_acpi_pm_ctrl(struct v4l2_subdev *subdev, int on)
>  		gs->clock_on = on;
>  	else
>  		dev_err(subdev->dev, "Couldn't set power state to %s\n",
> -			on ? "on" : "off");
> +			str_on_off(on));
>  
>  	return ret;
>  }

-- 
Regards,

Sakari Ailus

Re: [PATCH v2] staging: media: atomisp: Use str_on_off() to fix Coccinelle warning

[David Laight]

On Thu, 23 Apr 2026 13:47:27 -0600
Nick Spooner <nicholas.spooner@seagate.com> wrote:

> Fixes the following Coccinelle warning reported by string_choices.cocci:
> 
> 	opportunity for str_on_off(on)
> 
> Signed-off-by: Nick Spooner <nicholas.spooner@seagate.com>
> ---
> Changes in v2:
>   - Include "media" in the subject line.
> 
>  drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> index 4026e98c5845..322eca4a3755 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> @@ -13,6 +13,7 @@
>  #include <linux/gpio/consumer.h>
>  #include <linux/gpio.h>
>  #include <linux/platform_device.h>
> +#include <linux/string_choices.h>
>  #include "../../include/linux/atomisp_platform.h"
>  #include "../../include/linux/atomisp_gmin_platform.h"
>  
> @@ -917,7 +918,7 @@ static int gmin_acpi_pm_ctrl(struct v4l2_subdev *subdev, int on)
>  		return 0;
>  
>  	dev_dbg(subdev->dev, "Setting power state to %s\n",
> -		on ? "on" : "off");
> +		str_on_off(on));

While someone went to the trouble of adding the 'helper' and the Coccinelle
warning; I'm not really convinced it actually improves the code.
It doesn't even do &"off\0on"[on_off * 4] - which might be an excuse
for a helper.

	David

>  
>  	if (on)
>  		ret = acpi_device_set_power(adev,
> @@ -930,7 +931,7 @@ static int gmin_acpi_pm_ctrl(struct v4l2_subdev *subdev, int on)
>  		gs->clock_on = on;
>  	else
>  		dev_err(subdev->dev, "Couldn't set power state to %s\n",
> -			on ? "on" : "off");
> +			str_on_off(on));
>  
>  	return ret;
>  }

Re: [PATCH v2] staging: media: atomisp: Use str_on_off() to fix Coccinelle warning

[Andy Shevchenko]

On Thu, Apr 23, 2026 at 10:41:13PM +0100, David Laight wrote:
> On Thu, 23 Apr 2026 13:47:27 -0600
> Nick Spooner <nicholas.spooner@seagate.com> wrote:

...

> >  	dev_dbg(subdev->dev, "Setting power state to %s\n",
> > -		on ? "on" : "off");
> > +		str_on_off(on));
> 
> While someone went to the trouble of adding the 'helper' and the Coccinelle
> warning; I'm not really convinced it actually improves the code.
> It doesn't even do &"off\0on"[on_off * 4] - which might be an excuse
> for a helper.

The point of the helper mainly not about the code optimisation, it's all about
making unified tokens when printing stuff (same for dates, times, escaped
strings, et cetera). Btw, the code like `&"off\0on"[on_off * 4]` is unreadable
trick. And I dunno if compiler/linker will be able to pack these string literals
as currently done in case several of str_FOO_BAR() [with the same FOO_BAR] are
in the same compilation unit.

-- 
With Best Regards,
Andy Shevchenko