Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753446AbcCaG5o (ORCPT ); Thu, 31 Mar 2016 02:57:44 -0400 Received: from down.free-electrons.com ([37.187.137.238]:57105 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751511AbcCaG5l (ORCPT ); Thu, 31 Mar 2016 02:57:41 -0400 Date: Thu, 31 Mar 2016 08:57:35 +0200 From: Boris Brezillon To: Stephen Boyd Cc: Thierry Reding , linux-pwm@vger.kernel.org, Mike Turquette , linux-clk@vger.kernel.org, Mark Brown , Liam Girdwood , Kamil Debski , lm-sensors@lm-sensors.org, Jean Delvare , Guenter Roeck , Dmitry Torokhov , linux-input@vger.kernel.org, Bryan Wu , Richard Purdie , Jacek Anaszewski , linux-leds@vger.kernel.org, Maxime Ripard , Chen-Yu Tsai , linux-sunxi@googlegroups.com, Joachim Eastwood , Thomas Petazzoni , Heiko Stuebner , linux-rockchip@lists.infradead.org, Jingoo Han , Lee Jones , linux-fbdev@vger.kernel.org, Jean-Christophe Plagniol-Villard , Tomi Valkeinen , Robert Jarzmik , Alexandre Belloni , Kukjin Kim , Krzysztof Kozlowski , linux-samsung-soc@vger.kernel.org, intel-gfx@lists.freedesktop.org, Daniel Vetter , Jani Nikula , Jonathan Corbet , linux-doc@vger.kernel.org, David Airlie , Daniel Vetter , dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Hartley Sweeten , Ryan Mallon , Alexander Shiyan , Milo Kim Subject: Re: [PATCH v5 34/46] clk: pwm: switch to the atomic API Message-ID: <20160331085735.3574970b@bbrezillon> In-Reply-To: <20160330220149.GU18567@codeaurora.org> References: <1459368249-13241-1-git-send-email-boris.brezillon@free-electrons.com> <1459368249-13241-35-git-send-email-boris.brezillon@free-electrons.com> <20160330220149.GU18567@codeaurora.org> X-Mailer: Claws Mail 3.12.0 (GTK+ 2.24.28; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1678 Lines: 53 Hi Stephen, On Wed, 30 Mar 2016 15:01:49 -0700 Stephen Boyd wrote: > On 03/30, Boris Brezillon wrote: > > diff --git a/drivers/clk/clk-pwm.c b/drivers/clk/clk-pwm.c > > index ebcd738..49ec5b1 100644 > > --- a/drivers/clk/clk-pwm.c > > +++ b/drivers/clk/clk-pwm.c > > @@ -28,15 +28,29 @@ static inline struct clk_pwm *to_clk_pwm(struct clk_hw *hw) > > static int clk_pwm_prepare(struct clk_hw *hw) > > { > > struct clk_pwm *clk_pwm = to_clk_pwm(hw); > > + struct pwm_state pstate; > > > > - return pwm_enable(clk_pwm->pwm); > > + pwm_get_state(clk_pwm->pwm, &pstate); > > + if (pstate.enabled) > > + return 0; > > + > > + pstate.enabled = true; > > + > > + return pwm_apply_state(clk_pwm->pwm, &pstate); > > This doesn't seem atomic anymore if we're checking the state and > then not calling apply_state if it's already enabled. But I > assume this doesn't matter because we "own" the pwm here? Yep. Actually it's not atomic in term of concurrency (maybe the 'atomic' word is not appropriate here). Atomicity is here referring to the fact that we're now providing all the PWM parameters in the same request instead of splitting it in pwm_config() + pwm_enable/disable() calls. Concurrent accesses still have to be controlled by the PWM user (which is already the case for this driver, thanks to the locking infrastructure in the CCF). > Otherwise I would think this would be unconditional apply state > and duplicates would be ignored in the pwm framework. > Yep, I'll remove the if (pstate.enabled) branch. Thanks for your review. Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com