Re: [PATCH 02/10] pwm: Add SI-EN SN3112 PWM support - Uwe Kleine-König

From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Xilin Wu <wuxilin123@gmail.com>,
	 Konrad Dybcio <konrad.dybcio@linaro.org>
Cc: Rob Herring <robh@kernel.org>,
	 Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	 Junhao Xie <bigfoot@classfun.cn>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	 Jessica Zhang <quic_jesszhan@quicinc.com>,
	Sam Ravnborg <sam@ravnborg.org>,
	 David Airlie <airlied@gmail.com>,
	Daniel Vetter <daniel@ffwll.ch>,
	 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	 Thomas Zimmermann <tzimmermann@suse.de>,
	Bjorn Andersson <andersson@kernel.org>,
	 Tengfei Fan <quic_tengfan@quicinc.com>,
	Molly Sophia <mollysophia379@gmail.com>,
	 linux-pwm@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,  dri-devel@lists.freedesktop.org,
	linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH 02/10] pwm: Add SI-EN SN3112 PWM support
Date: Thu, 25 Apr 2024 08:14:39 +0200	[thread overview]
Message-ID: <nuq56pinjqonlo4crfibtb2ddlwvldurkphomsyyqp6iupqpbk@u6kun3qvstw2> (raw)
In-Reply-To: <af62742c-8d6d-4fa9-b2e4-f83253e6e388@linaro.org>

[-- Attachment #1: Type: text/plain, Size: 1007 bytes --]

Hello,

On Wed, Apr 24, 2024 at 09:37:25PM +0200, Konrad Dybcio wrote:
> On 4/24/24 17:29, Xilin Wu via B4 Relay wrote:
> > +
> > +	/* use random value to apply changes */
> > +	ret = sn3112_write_reg(priv, SN3112_REG_APPLY, 0x66);
> 
> "a random value"? sounds suspicious..

I smiled about that one, too, remembering https://xkcd.com/221/

> [...]
> > +#if IS_ENABLED(CONFIG_GPIOLIB)
> > +	/* enable hardware shutdown pin */
> > +	if (priv->sdb)
> > +		gpiod_set_value(priv->sdb, 1);
> > +#endif
> > +
> > +	/* power-off sn5112 power vdd */
> > +	regulator_disable(priv->vdd);
> > +
> > +	pwmchip_remove(chip);
> 
> devm_pwmchip_add?

Note using devm_xyz only works if all requests before are also using
devm. (There are a few exceptions, but these need proper thinking and
extensive commenting.)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]