[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20231012091602.GD8314@google.com>
Date: Thu, 12 Oct 2023 10:16:02 +0100
From: Lee Jones <lee@...nel.org>
To: Baolin Wang <baolin.wang@...ux.alibaba.com>
Cc: Chunyan Zhang <chunyan.zhang@...soc.com>,
Pavel Machek <pavel@....cz>, linux-leds@...r.kernel.org,
Orson Zhai <orsonzhai@...il.com>,
Chunyan Zhang <zhang.lyra@...il.com>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RESEND PATCH V2] leds: sc27xx: Move mutex_init() to the end of
probe
On Thu, 12 Oct 2023, Baolin Wang wrote:
>
>
> On 10/12/2023 11:47 AM, Chunyan Zhang wrote:
> > Move the mutex_init() to avoid redundant mutex_destroy() calls after
> > that for each time the probe fails.
> >
> > Signed-off-by: Chunyan Zhang <chunyan.zhang@...soc.com>
> > ---
> > Rebased onto linux-next.
> >
> > V2:
> > - Move the mutex_init() to the end of .probe() instead of adding
> > mutex_destroy() according to Lee's comments.
> > ---
> > drivers/leds/leds-sc27xx-bltc.c | 9 ++++-----
> > 1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/leds/leds-sc27xx-bltc.c b/drivers/leds/leds-sc27xx-bltc.c
> > index af1f00a2f328..ef57e57ecf07 100644
> > --- a/drivers/leds/leds-sc27xx-bltc.c
> > +++ b/drivers/leds/leds-sc27xx-bltc.c
> > @@ -296,7 +296,6 @@ static int sc27xx_led_probe(struct platform_device *pdev)
> > return -ENOMEM;
> > platform_set_drvdata(pdev, priv);
> > - mutex_init(&priv->lock);
> > priv->base = base;
> > priv->regmap = dev_get_regmap(dev->parent, NULL);
> > if (!priv->regmap) {
> > @@ -309,13 +308,11 @@ static int sc27xx_led_probe(struct platform_device *pdev)
> > err = of_property_read_u32(child, "reg", ®);
> > if (err) {
> > of_node_put(child);
> > - mutex_destroy(&priv->lock);
> > return err;
> > }
> > if (reg >= SC27XX_LEDS_MAX || priv->leds[reg].active) {
> > of_node_put(child);
> > - mutex_destroy(&priv->lock);
> > return -EINVAL;
> > }
> > @@ -325,9 +322,11 @@ static int sc27xx_led_probe(struct platform_device *pdev)
> > err = sc27xx_led_register(dev, priv);
> > if (err)
> > - mutex_destroy(&priv->lock);
> > + return err;
> > - return err;
> > + mutex_init(&priv->lock);
>
> I think it is better to prepare all the required resources before
> registering the led device, what I mean is moving mutex_init() before
> calling sc27xx_led_register().
Is the mutex used before this point?
If not, I don't see any reason to initialise it sooner.
--
Lee Jones [李琼斯]
Powered by blists - more mailing lists