From patchwork Thu Jul 1 07:29:25 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Uwe_Kleine-K=C3=B6nig?= X-Patchwork-Id: 1499419 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=linux-pwm-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 4GFqbg6j6Sz9sVb for ; Thu, 1 Jul 2021 17:29:35 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234829AbhGAHcE (ORCPT ); Thu, 1 Jul 2021 03:32:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52742 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229906AbhGAHcE (ORCPT ); Thu, 1 Jul 2021 03:32:04 -0400 Received: from metis.ext.pengutronix.de (metis.ext.pengutronix.de [IPv6:2001:67c:670:201:290:27ff:fe1d:cc33]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EF892C061756 for ; Thu, 1 Jul 2021 00:29:32 -0700 (PDT) Received: from ptx.hi.pengutronix.de ([2001:67c:670:100:1d::c0]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lyr8c-00038y-Ou; Thu, 01 Jul 2021 09:29:30 +0200 Received: from ukl by ptx.hi.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1lyr8b-0006y1-OU; Thu, 01 Jul 2021 09:29:29 +0200 From: =?utf-8?q?Uwe_Kleine-K=C3=B6nig?= To: Thierry Reding , Lee Jones Cc: linux-pwm@vger.kernel.org, kernel@pengutronix.de, Geert Uytterhoeven Subject: [PATCH 1/3] pwm: Move legacy driver handling into a dedicated function Date: Thu, 1 Jul 2021 09:29:25 +0200 Message-Id: <20210701072927.328254-2-u.kleine-koenig@pengutronix.de> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210701072927.328254-1-u.kleine-koenig@pengutronix.de> References: <20210701072927.328254-1-u.kleine-koenig@pengutronix.de> MIME-Version: 1.0 X-Patch-Hashes: v=1; h=sha256; i=VLywSRiHGKnQZ+sS60XRoAG+wlQ7ZanrKLCkgOrOEtI=; m=QQPqrse405A0JmOj99AyliymNJHPDcGyWK2Z6TGu7H8=; p=UfM8u/A5kO1ZoEeAdooxrxFd7zYtC0Qf5wAU/XvdwmU=; g=66d12b77bcf010abc5b380ec6f11cccdd7875858 X-Patch-Sig: m=pgp; i=uwe@kleine-koenig.org; s=0x0D2511F322BFAB1C1580266BE2DCDD9132669BD6; b=iQEzBAABCgAdFiEEfnIqFpAYrP8+dKQLwfwUeK3K7AkFAmDdbsoACgkQwfwUeK3K7AnDUwf9Gd/ R/7bk0u93wRx4bEYcaWnLy+qtDgUX816nORB4cJZtdclCrw4a5zQmXgMXLskoUmJ8j2OPnJ8AM1n8 2wnDX8J7Rs8ZFq7Og9+1BmB5CRBb4wmi9GAf9AJJmbk4xE8pmKFENVcLEYyiAk1KWX3VCsXAnk+/P eKnR69iqUO8ND1TDEOrBOvZ73TnlyKYxWH2Cz7aMtpqBFs2cLdmlBIhyFZhJyy0tmC9IVuuYG1Voj O6i2HfDUuRCJ7n0qzVqaoTTv4JVPX0dxDP1a/ZMMTNoLk7zwKp6pshezy5dyYwapb1ZfSNcyvkjIU AQmJ18qWJHwevSgyU2EbgS4aiWwD27g== X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c0 X-SA-Exim-Mail-From: ukl@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-pwm@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pwm@vger.kernel.org There is no change in behaviour, only some code is moved from pwm_apply_state to a separate function. Signed-off-by: Uwe Kleine-König --- drivers/pwm/core.c | 130 ++++++++++++++++++++++++--------------------- 1 file changed, 70 insertions(+), 60 deletions(-) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index c4d5c0667137..3c72f8963073 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -535,6 +535,64 @@ static void pwm_apply_state_debug(struct pwm_device *pwm, } } +static int pwm_apply_legacy(struct pwm_chip *chip, struct pwm_device *pwm, + const struct pwm_state *state) +{ + int err; + + /* + * FIXME: restore the initial state in case of error. + */ + if (state->polarity != pwm->state.polarity) { + if (!chip->ops->set_polarity) + return -EINVAL; + + /* + * Changing the polarity of a running PWM is only allowed when + * the PWM driver implements ->apply(). + */ + if (pwm->state.enabled) { + chip->ops->disable(chip, pwm); + + /* + * Update pwm->state already here in case + * .set_polarity() or another callback depend on that. + */ + pwm->state.enabled = false; + } + + err = chip->ops->set_polarity(chip, pwm, state->polarity); + if (err) + return err; + + pwm->state.polarity = state->polarity; + } + + if (state->period != pwm->state.period || + state->duty_cycle != pwm->state.duty_cycle) { + err = chip->ops->config(pwm->chip, pwm, + state->duty_cycle, + state->period); + if (err) + return err; + + pwm->state.period = state->period; + pwm->state.duty_cycle = state->duty_cycle; + } + + if (state->enabled != pwm->state.enabled) { + if (!pwm->state.enabled) { + err = chip->ops->enable(chip, pwm); + if (err) + return err; + } else { + chip->ops->disable(chip, pwm); + } + } + + return 0; +} + /** * pwm_apply_state() - atomically apply a new state to a PWM device * @pwm: PWM device @@ -557,70 +615,22 @@ int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state) state->enabled == pwm->state.enabled) return 0; - if (chip->ops->apply) { + if (chip->ops->apply) err = chip->ops->apply(chip, pwm, state); - if (err) - return err; - - trace_pwm_apply(pwm, state); - - pwm->state = *state; - - /* - * only do this after pwm->state was applied as some - * implementations of .get_state depend on this - */ - pwm_apply_state_debug(pwm, state); - } else { - /* - * FIXME: restore the initial state in case of error. - */ - if (state->polarity != pwm->state.polarity) { - if (!chip->ops->set_polarity) - return -EINVAL; - - /* - * Changing the polarity of a running PWM is - * only allowed when the PWM driver implements - * ->apply(). - */ - if (pwm->state.enabled) { - chip->ops->disable(chip, pwm); - pwm->state.enabled = false; - } - - err = chip->ops->set_polarity(chip, pwm, - state->polarity); - if (err) - return err; - - pwm->state.polarity = state->polarity; - } - - if (state->period != pwm->state.period || - state->duty_cycle != pwm->state.duty_cycle) { - err = chip->ops->config(pwm->chip, pwm, - state->duty_cycle, - state->period); - if (err) - return err; + else + err = pwm_apply_legacy(chip, pwm, state); + if (err) + return err; - pwm->state.duty_cycle = state->duty_cycle; - pwm->state.period = state->period; - } + trace_pwm_apply(pwm, state); - if (state->enabled != pwm->state.enabled) { - if (state->enabled) { - err = chip->ops->enable(chip, pwm); - if (err) - return err; - } else { - chip->ops->disable(chip, pwm); - } + pwm->state = *state; - pwm->state.enabled = state->enabled; - } - } + /* + * only do this after pwm->state was applied as some + * implementations of .get_state depend on this + */ + pwm_apply_state_debug(pwm, state); return 0; } From patchwork Thu Jul 1 07:29:26 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Uwe_Kleine-K=C3=B6nig?= X-Patchwork-Id: 1499422 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=linux-pwm-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 4GFqbh6Hqtz9sVb for ; Thu, 1 Jul 2021 17:29:36 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234635AbhGAHcE (ORCPT ); Thu, 1 Jul 2021 03:32:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52746 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234824AbhGAHcE (ORCPT ); Thu, 1 Jul 2021 03:32:04 -0400 Received: from metis.ext.pengutronix.de (metis.ext.pengutronix.de [IPv6:2001:67c:670:201:290:27ff:fe1d:cc33]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 28B6CC0617AD for ; Thu, 1 Jul 2021 00:29:33 -0700 (PDT) Received: from ptx.hi.pengutronix.de ([2001:67c:670:100:1d::c0]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lyr8c-00038z-Ou; Thu, 01 Jul 2021 09:29:30 +0200 Received: from ukl by ptx.hi.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1lyr8b-0006y4-V8; Thu, 01 Jul 2021 09:29:29 +0200 From: =?utf-8?q?Uwe_Kleine-K=C3=B6nig?= To: Thierry Reding , Lee Jones Cc: linux-pwm@vger.kernel.org, kernel@pengutronix.de, Geert Uytterhoeven Subject: [PATCH 2/3] pwm: Prevent a glitch for legacy drivers Date: Thu, 1 Jul 2021 09:29:26 +0200 Message-Id: <20210701072927.328254-3-u.kleine-koenig@pengutronix.de> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210701072927.328254-1-u.kleine-koenig@pengutronix.de> References: <20210701072927.328254-1-u.kleine-koenig@pengutronix.de> MIME-Version: 1.0 X-Patch-Hashes: v=1; h=sha256; i=3jp+KNgBvhQphIgkyTxKed1TpNAQoyExL9GSbIVWWQw=; m=rP85FNWtFgXD27m7WY/IxRngNXRoyZYDgl17dsyh6Vo=; p=L5ZhrPYspHz4XDsRRnjWAFPVp9jL9aqm3h/Sc7WnMiw=; g=6b0f80e136ce6a82845a5ee3ecc159fe1446a3b4 X-Patch-Sig: m=pgp; i=uwe@kleine-koenig.org; s=0x0D2511F322BFAB1C1580266BE2DCDD9132669BD6; b=iQEzBAABCgAdFiEEfnIqFpAYrP8+dKQLwfwUeK3K7AkFAmDdbqkACgkQwfwUeK3K7AnCegf/cwR Jz9bOUzVELWnBWpzAOr57vKCfJH298VRn5JLVuZm2lYPrVQwlJ2EgLolq6UZsCIj5CYl41FI0jALc JLQON5qHOQF3y+wV3i4gwPOwQDvT6AdAF/HYUcj+RIiceqvRyVoc6L8ehwth6YYrEe36IvOIOTvpD h/pcOqaMMLIgd/aJmsIc0WZpFJMaGObCIn8fMiVJzUbBsHgSru752Qix+ShzGB21P8cmvenWoPwAe P0GVHVXNSiYDKvTXWPTVfpo/qp7Qqh8mSpGRz1e57zpuMaWTNLcIg1gn8+RTqBjGpgqmWsRKfyecy qPK+XoKq8+o5FgsZISUVwpaxrv5QOEA== X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c0 X-SA-Exim-Mail-From: ukl@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-pwm@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pwm@vger.kernel.org If a running PWM is reconfigured to disabled calling the ->config() callback before disabling the hardware might result in a glitch where the (maybe) new period and duty_cycle are visible on the output before disabling the hardware. So handle disabling before calling ->config(). Also exit early in this case which is possible because period and duty_cycle don't matter for disabled PWMs. In return however ->config has to be called even if state->period == pwm->state.period && state->duty_cycle != pwm->state.duty_cycle because setting these might have been skipped in the previous call. Signed-off-by: Uwe Kleine-König --- drivers/pwm/core.c | 41 ++++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index 3c72f8963073..20afe6d0bc5e 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -568,26 +568,33 @@ static int pwm_apply_legacy(struct pwm_chip *chip, struct pwm_device *pwm, pwm->state.polarity = state->polarity; } - if (state->period != pwm->state.period || - state->duty_cycle != pwm->state.duty_cycle) { - err = chip->ops->config(pwm->chip, pwm, - state->duty_cycle, - state->period); - if (err) - return err; + if (!state->enabled) { + if (pwm->state.enabled) + chip->ops->disable(chip, pwm); - pwm->state.period = state->period; - pwm->state.duty_cycle = state->duty_cycle; + return 0; } - if (state->enabled != pwm->state.enabled) { - if (!pwm->state.enabled) { - err = chip->ops->enable(chip, pwm); - if (err) - return err; - } else { - chip->ops->disable(chip, pwm); - } + /* + * We cannot skip calling ->config even if state->period == + * pwm->state.period && state->duty_cycle == pwm->state.duty_cycle + * because we might have exited early in the last call to + * pwm_apply_state because of !state->enabled and so the two values in + * pwm->state might not be configured in hardware. + */ + err = chip->ops->config(pwm->chip, pwm, + state->duty_cycle, + state->period); + if (err) + return err; + + pwm->state.period = state->period; + pwm->state.duty_cycle = state->duty_cycle; + + if (!pwm->state.enabled) { + err = chip->ops->enable(chip, pwm); + if (err) + return err; } return 0; From patchwork Thu Jul 1 07:29:27 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Uwe_Kleine-K=C3=B6nig?= X-Patchwork-Id: 1499421 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=linux-pwm-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 4GFqbh3g8bz9sX5 for ; Thu, 1 Jul 2021 17:29:36 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234733AbhGAHcE (ORCPT ); Thu, 1 Jul 2021 03:32:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52748 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234635AbhGAHcE (ORCPT ); Thu, 1 Jul 2021 03:32:04 -0400 Received: from metis.ext.pengutronix.de (metis.ext.pengutronix.de [IPv6:2001:67c:670:201:290:27ff:fe1d:cc33]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2FAD0C0617AE for ; Thu, 1 Jul 2021 00:29:33 -0700 (PDT) Received: from ptx.hi.pengutronix.de ([2001:67c:670:100:1d::c0]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lyr8c-000391-Ou; Thu, 01 Jul 2021 09:29:30 +0200 Received: from ukl by ptx.hi.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1lyr8c-0006yA-5B; Thu, 01 Jul 2021 09:29:30 +0200 From: =?utf-8?q?Uwe_Kleine-K=C3=B6nig?= To: Thierry Reding , Lee Jones Cc: linux-pwm@vger.kernel.org, kernel@pengutronix.de, Geert Uytterhoeven Subject: [PATCH 3/3] pwm: Restore initial state if a legacy callback fails Date: Thu, 1 Jul 2021 09:29:27 +0200 Message-Id: <20210701072927.328254-4-u.kleine-koenig@pengutronix.de> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210701072927.328254-1-u.kleine-koenig@pengutronix.de> References: <20210701072927.328254-1-u.kleine-koenig@pengutronix.de> MIME-Version: 1.0 X-Patch-Hashes: v=1; h=sha256; i=NKl7qBUDZ4t5WRIxEdUoKhwmGiKzejmC7NB0eOp7sJM=; m=VkgWLiZafRShbBgwl+M9ZkcM1pqbPA2BmrjkiU0Ckrc=; p=qrgcSIeKy6cUIOC+EtdUMUjaGm9f9sUXBDJRKZszYuQ=; g=3631a5b206800449c9da6c96dba155575e518015 X-Patch-Sig: m=pgp; i=uwe@kleine-koenig.org; s=0x0D2511F322BFAB1C1580266BE2DCDD9132669BD6; b=iQEzBAABCgAdFiEEfnIqFpAYrP8+dKQLwfwUeK3K7AkFAmDdbn8ACgkQwfwUeK3K7AlRAwf+NzP EnzoTbHITOg/hYZFVb11X4f+S6HfrPY/fHhD1KD3xbGCYmlz1iZhMRP79I3o+vi3TznM40mR8XTub bVZYW3Z2u4x7MdsssLNVR1E5GQYfkoFjWXIzaNkUFIo05je6yndc1TpE6qeKhj+/IzghJjuQmEvgM 2ggm0v99WYlp9Mzpk9m2VK4XMectn1vVVyl22R5OzlvXRu4MJIe0cKLEtDu2qiXJRN3S10diAr3V2 951ScNxhXRA9sn1c9PJRBDV+RdDvHIeHbEhFZP5bQHUg94l/kl1XpA0pSqfCirWNeZlGLnrlSdaMy +6G0yyub26fYhVVuz9ujfkLxVN2jLig== X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c0 X-SA-Exim-Mail-From: ukl@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-pwm@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pwm@vger.kernel.org It is not entirely accurate to go back to the initial state after e.g. .enable() failed, as .config() still modified the hardware, but this same inconsistency exists for drivers that implement .apply(). Signed-off-by: Uwe Kleine-König --- drivers/pwm/core.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index 20afe6d0bc5e..6e30ef9b9b79 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -539,10 +539,8 @@ static int pwm_apply_legacy(struct pwm_chip *chip, struct pwm_device *pwm, const struct pwm_state *state) { int err; + struct pwm_state initial_state = pwm->state; - /* - * FIXME: restore the initial state in case of error. - */ if (state->polarity != pwm->state.polarity) { if (!chip->ops->set_polarity) return -EINVAL; @@ -563,7 +561,7 @@ static int pwm_apply_legacy(struct pwm_chip *chip, struct pwm_device *pwm, err = chip->ops->set_polarity(chip, pwm, state->polarity); if (err) - return err; + goto rollback; pwm->state.polarity = state->polarity; } @@ -586,7 +584,7 @@ static int pwm_apply_legacy(struct pwm_chip *chip, struct pwm_device *pwm, state->duty_cycle, state->period); if (err) - return err; + goto rollback; pwm->state.period = state->period; pwm->state.duty_cycle = state->duty_cycle; @@ -594,10 +592,14 @@ static int pwm_apply_legacy(struct pwm_chip *chip, struct pwm_device *pwm, if (!pwm->state.enabled) { err = chip->ops->enable(chip, pwm); if (err) - return err; + goto rollback; } return 0; + +rollback: + pwm->state = initial_state; + return err; } /**