Re: [PATCH v3 2/2] leds: Add PWM multicolor driver

From: Sven Schwermer <sven@svenschwermer.de>
To: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: "Linux LED Subsystem" <linux-leds@vger.kernel.org>,
	devicetree <devicetree@vger.kernel.org>,
	linux-pwm@vger.kernel.org,
	"Sven Schwermer" <sven.schwermer@disruptive-technologies.com>,
	"Pavel Machek" <pavel@ucw.cz>, "Rob Herring" <robh+dt@kernel.org>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Lee Jones" <lee.jones@linaro.org>,
	post@lespocky.de
Subject: Re: [PATCH v3 2/2] leds: Add PWM multicolor driver
Date: Sun, 6 Feb 2022 10:17:51 +0100	[thread overview]
Message-ID: <20f95cfd-a851-af4f-1c60-45f2ca238e10@svenschwermer.de> (raw)
In-Reply-To: <CAHp75VfMTCvgib__PhnfB_g7xLhyNws5TDRyMVyzuAkT1ydY_w@mail.gmail.com>

Hi Andy,

Thanks for looking through my patch. I have a couple of follow-up 
questions about your feedback:

On 2/2/22 13:33, Andy Shevchenko wrote:
>> +                       ret = PTR_ERR(pwmled->pwm);
>> +                       dev_err(&pdev->dev, "unable to request PWM: %d\n", ret);
>> +                       fwnode_handle_put(fwnode);
>> +                       goto destroy_mutex;
> 
> fwnode_handle_put();
> return dev_err_probe(...);
This would skip the destruction of the mutex and releasing of mcnode. 
Isn't that problematic? The same goes for all of your comments of this kind.

>> +destroy_mutex:
>> +       mutex_destroy(&priv->lock);
> 
> Wrong ordering here and in ->remove().
> 
> Don't mix devm_* with non-devm_* calls.
What do you mean by this?

Best regards,
Sven