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@xxxxxxxxxx>
> > ---
> > 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 [李琼斯]
