From mboxrd@z Thu Jan 1 00:00:00 1970 From: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= Subject: Re: [PATCH v3 5/6] pwm: fsl-ftm: Don't update the state for the caller of pwm_apply_state() Date: Tue, 3 Sep 2019 20:48:00 +0200 Message-ID: <20190903184800.2fmmvwyzbwbsaf6y@pengutronix.de> References: <20190824153707.13746-1-uwe@kleine-koenig.org> <20190824153707.13746-6-uwe@kleine-koenig.org> <20190902142709.wxrjsfzorozgeiuh@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+glpar-linux-rockchip=m.gmane.org-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org To: Doug Anderson Cc: linux-pwm , Heiko Stuebner , Maxime Ripard , Patrick Havelange , "open list:ARM/Rockchip SoC..." , Chen-Yu Tsai , Thierry Reding , Sascha Hauer List-Id: linux-pwm@vger.kernel.org Hello, On Tue, Sep 03, 2019 at 09:54:37AM -0700, Doug Anderson wrote: > On Mon, Sep 2, 2019 at 7:27 AM Uwe Kleine-K=F6nig > wrote: > > On Fri, Aug 30, 2019 at 10:39:16AM -0700, Doug Anderson wrote: > > > On Sat, Aug 24, 2019 at 8:37 AM Uwe Kleine-K=F6nig wrote: > > > > > > > > The pwm-fsl-ftm driver is one of only three PWM drivers which updat= es > > > > the state for the caller of pwm_apply_state(). This might have > > > > surprising results if the caller reuses the values expecting them to > > > > still represent the same state. > > > > > > > > Signed-off-by: Uwe Kleine-K=F6nig > > > > --- > > > > drivers/pwm/pwm-fsl-ftm.c | 4 ---- > > > > 1 file changed, 4 deletions(-) > > > > > > Presumably this patch could break something since the pwm-fsl-ftm > > > driver doesn't appear to implement the get_state() function. ...or > > > did I miss it? > > > > I don't expect breakage. We have more than 50 pwm drivers and only three > > of them made use of adapting the passed state. So unless you do > > something special with the PWM (i.e. more than backlight, LED or fan > > control) I don't think a consumer might care. But it might well be that > > I miss something so feel free to prove me wrong. > = > I don't have this hardware so I can't prove you wrong. ...but > presumably someone added the code to return the state on purpose? > = > Maybe you could implement get_state() for this driver in your series? Sure, I could. But I don't have hardware either and so I'm not in a better position than anybody else on this list. I suggest to apply as is during the merge window, and let affected user report problems (or patches) if there really is an issue. Guessing what people might suffer from and trying to cure this with untested patches won't help I think. Best regards Uwe -- = Pengutronix e.K. | Uwe Kleine-K=F6nig | Industrial Linux Solutions | http://www.pengutronix.de/ |