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 <[email protected]>
> ---
> 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)
>
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
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 <[email protected]>
Best regards,
Jernej
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 <[email protected]>
Thank you for the review!
--
With Best Regards,
Andy Shevchenko