[PATCH v1 1/1] leds: sun50i-a100: Use match_string() helper to simplify the code

[PATCH v1 1/1] leds: sun50i-a100: Use match_string() helper to simplify the code

[Andy Shevchenko]

match_string() returns the array index of a matching string.
Use it instead of the open-coded implementation.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/leds/leds-sun50i-a100.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/leds/leds-sun50i-a100.c b/drivers/leds/leds-sun50i-a100.c
index 62d21c3a3575..119eff9471f0 100644
--- a/drivers/leds/leds-sun50i-a100.c
+++ b/drivers/leds/leds-sun50i-a100.c
@@ -252,18 +252,16 @@ static int sun50i_a100_ledc_parse_format(struct device *dev,
 					 struct sun50i_a100_ledc *priv)
 {
 	const char *format = "grb";
-	u32 i;
+	int i;
 
 	device_property_read_string(dev, "allwinner,pixel-format", &format);
 
-	for (i = 0; i < ARRAY_SIZE(sun50i_a100_ledc_formats); i++) {
-		if (!strcmp(format, sun50i_a100_ledc_formats[i])) {
-			priv->format = i;
-			return 0;
-		}
-	}
+	i = match_string(sun50i_a100_ledc_formats, ARRAY_SIZE(sun50i_a100_ledc_formats), format);
+	if (i < 0)
+		return dev_err_probe(dev, i, "Bad pixel format '%s'\n", format);
 
-	return dev_err_probe(dev, -EINVAL, "Bad pixel format '%s'\n", format);
+	priv->format = i;
+	return 0;
 }
 
 static void sun50i_a100_ledc_set_format(struct sun50i_a100_ledc *priv)
-- 
2.43.0.rc1.1336.g36b5255a03ac


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/1] leds: sun50i-a100: Use match_string() helper to simplify the code

[Jernej Škrabec]

Dne petek, 26. april 2024 ob 17:25:15 GMT +2 je Andy Shevchenko napisal(a):
> match_string() returns the array index of a matching string.
> Use it instead of the open-coded implementation.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/leds/leds-sun50i-a100.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/leds/leds-sun50i-a100.c b/drivers/leds/leds-sun50i-a100.c
> index 62d21c3a3575..119eff9471f0 100644
> --- a/drivers/leds/leds-sun50i-a100.c
> +++ b/drivers/leds/leds-sun50i-a100.c
> @@ -252,18 +252,16 @@ static int sun50i_a100_ledc_parse_format(struct device *dev,
>  					 struct sun50i_a100_ledc *priv)
>  {
>  	const char *format = "grb";
> -	u32 i;
> +	int i;
>  
>  	device_property_read_string(dev, "allwinner,pixel-format", &format);
>  
> -	for (i = 0; i < ARRAY_SIZE(sun50i_a100_ledc_formats); i++) {
> -		if (!strcmp(format, sun50i_a100_ledc_formats[i])) {
> -			priv->format = i;
> -			return 0;
> -		}
> -	}
> +	i = match_string(sun50i_a100_ledc_formats, ARRAY_SIZE(sun50i_a100_ledc_formats), format);
> +	if (i < 0)
> +		return dev_err_probe(dev, i, "Bad pixel format '%s'\n", format);

I know that old code used dev_err_probe() without reason, but could you change
it to ordinary dev_err()? dev_err_probe() is useful only when return code could
be -EPROBE_DEFER. This is clearly not the case here.

Best regards,
Jernej

>  
> -	return dev_err_probe(dev, -EINVAL, "Bad pixel format '%s'\n", format);
> +	priv->format = i;
> +	return 0;
>  }
>  
>  static void sun50i_a100_ledc_set_format(struct sun50i_a100_ledc *priv)
> 





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/1] leds: sun50i-a100: Use match_string() helper to simplify the code

[Andy Shevchenko]

On Fri, Apr 26, 2024 at 05:37:42PM +0200, Jernej Škrabec wrote:
> Dne petek, 26. april 2024 ob 17:25:15 GMT +2 je Andy Shevchenko napisal(a):

...

> > +		return dev_err_probe(dev, i, "Bad pixel format '%s'\n", format);
> 
> I know that old code used dev_err_probe() without reason, but could you change
> it to ordinary dev_err()?

First of all, it's out of scope of _this_ patch.

> dev_err_probe() is useful only when return code could be -EPROBE_DEFER.

This is simply not true. We are trying to have a uniform output in ->probe()
and even documentation for dev_err_probe() was changed long time ago to
encourage using it for non deferred probe cases.

> This is clearly not the case here.

Is it a problem?

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/1] leds: sun50i-a100: Use match_string() helper to simplify the code

[Jernej Škrabec]

Dne petek, 26. april 2024 ob 17:45:14 GMT +2 je Andy Shevchenko napisal(a):
> On Fri, Apr 26, 2024 at 05:37:42PM +0200, Jernej Škrabec wrote:
> > Dne petek, 26. april 2024 ob 17:25:15 GMT +2 je Andy Shevchenko napisal(a):
> 
> ...
> 
> > > +		return dev_err_probe(dev, i, "Bad pixel format '%s'\n", format);
> > 
> > I know that old code used dev_err_probe() without reason, but could you change
> > it to ordinary dev_err()?
> 
> First of all, it's out of scope of _this_ patch.
> 
> > dev_err_probe() is useful only when return code could be -EPROBE_DEFER.
> 
> This is simply not true. We are trying to have a uniform output in ->probe()
> and even documentation for dev_err_probe() was changed long time ago to
> encourage using it for non deferred probe cases.
> 
> > This is clearly not the case here.
> 
> Is it a problem?

Sorry, I missed added note for non -EPROBE_DEFER cases.

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/1] leds: sun50i-a100: Use match_string() helper to simplify the code

[Andy Shevchenko]

On Fri, Apr 26, 2024 at 05:55:22PM +0200, Jernej Škrabec wrote:
> Dne petek, 26. april 2024 ob 17:45:14 GMT +2 je Andy Shevchenko napisal(a):
> > On Fri, Apr 26, 2024 at 05:37:42PM +0200, Jernej Škrabec wrote:
> > > Dne petek, 26. april 2024 ob 17:25:15 GMT +2 je Andy Shevchenko napisal(a):

...

> > > > +		return dev_err_probe(dev, i, "Bad pixel format '%s'\n", format);
> > > 
> > > I know that old code used dev_err_probe() without reason, but could you change
> > > it to ordinary dev_err()?
> > 
> > First of all, it's out of scope of _this_ patch.
> > 
> > > dev_err_probe() is useful only when return code could be -EPROBE_DEFER.
> > 
> > This is simply not true. We are trying to have a uniform output in ->probe()
> > and even documentation for dev_err_probe() was changed long time ago to
> > encourage using it for non deferred probe cases.
> > 
> > > This is clearly not the case here.
> > 
> > Is it a problem?
> 
> Sorry, I missed added note for non -EPROBE_DEFER cases.

No problem.

> Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Thank you for the review!

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: (subset) [PATCH v1 1/1] leds: sun50i-a100: Use match_string() helper to simplify the code

[Lee Jones]

On Fri, 26 Apr 2024 18:25:15 +0300, Andy Shevchenko wrote:
> match_string() returns the array index of a matching string.
> Use it instead of the open-coded implementation.
> 
> 

Applied, thanks!

[1/1] leds: sun50i-a100: Use match_string() helper to simplify the code
      commit: 3b29c7b9f701e5afbe6b536eb2744acb25cf5bfd

--
Lee Jones [李琼斯]


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel