Re: [PATCH] leds: st1202: Fix an error handling path in st1202_probe()

[Christophe JAILLET]

Le 20/02/2025 à 16:58, Lee Jones a écrit :
> On Wed, 19 Feb 2025, Christophe JAILLET wrote:
>
> > devm_mutex_init() may return -ENOMEM.
> > So this error should be handled in st1202_probe().
>
> The start of a new sentence shouldn't warrant a line break.
>
> > Fixes: 259230378c65 ("leds: Add LED1202 I2C driver")
> > Signed-off-by: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>
> > ---
> >   drivers/leds/leds-st1202.c | 5 ++++-
> >   1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c
> > index b691c4886993..4fc17d518292 100644
> > --- a/drivers/leds/leds-st1202.c
> > +++ b/drivers/leds/leds-st1202.c
> > @@ -356,7 +356,10 @@ static int st1202_probe(struct i2c_client *client)
> >   	if (!chip)
> >   		return -ENOMEM;
> >   
> > -	devm_mutex_init(&client->dev, &chip->lock);
> > +	ret = devm_mutex_init(&client->dev, &chip->lock);
> > +	if (ret < 0)
>
> My assumption is that anything but 0 would be bad, thus:
>
> 	if (ret)

Matter of taste. All other tests in this driver are "if (ret < 0)" or 
"if (ret != 0)".

What do you prefer: consistency or concision? (my own choice goes to 
consistency)

If you confirm concision, I'll send a v2 that also fix your other 
comment above.

CJ

> > +		return ret;
> > +
> >   	chip->client = client;
> >   
> >   	ret = st1202_dt_init(chip);
> > --
> > 2.48.1