Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758289AbcJYG4O (ORCPT ); Tue, 25 Oct 2016 02:56:14 -0400 Received: from 7.mo3.mail-out.ovh.net ([46.105.57.200]:35520 "EHLO 7.mo3.mail-out.ovh.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752446AbcJYG4I (ORCPT ); Tue, 25 Oct 2016 02:56:08 -0400 X-Greylist: delayed 119392 seconds by postgrey-1.27 at vger.kernel.org; Tue, 25 Oct 2016 02:56:07 EDT Date: Tue, 25 Oct 2016 08:55:01 +0200 From: Lukasz Majewski To: Sascha Hauer Cc: Thierry Reding , Stefan Agner , Boris Brezillon , linux-pwm@vger.kernel.org, Bhuvanchandra DV , linux-kernel@vger.kernel.org, kernel@pengutronix.de, Fabio Estevam , Fabio Estevam , Lothar Wassmann Subject: Re: [PATCH 1/6] pwm: imx: Rewrite imx_pwm_*_v1 code to facilitate switch to atomic pwm operation Message-ID: <20161025085501.1488b322@jawa> In-Reply-To: <20161025055454.emytytmcg6d23jdd@pengutronix.de> References: <1477259146-19167-1-git-send-email-l.majewski@majess.pl> <1477259146-19167-2-git-send-email-l.majewski@majess.pl> <20161025055454.emytytmcg6d23jdd@pengutronix.de> X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.25; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; boundary="Sig_/DJ0/uns5R.a.RUv92.kAiPY"; protocol="application/pgp-signature" X-Ovh-Tracer-Id: 133700614178128528 X-VR-SPAMSTATE: OK X-VR-SPAMSCORE: -100 X-VR-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrfeelvddrieeigdduudeiucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuqfggjfdpvefjgfevmfevgfenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddm Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3326 Lines: 109 --Sig_/DJ0/uns5R.a.RUv92.kAiPY Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Hi Sascha, > On Sun, Oct 23, 2016 at 11:45:41PM +0200, Lukasz Majewski wrote: > > The code has been rewritten to remove "generic" calls to > > imx_pwm_{enable|disable|config}. > >=20 > > Such approach would facilitate switch to atomic PWM (a.k.a > > ->apply()) implementation. > >=20 > > Suggested-by: Stefan Agner > > Suggested-by: Boris Brezillon > > Signed-off-by: Lukasz Majewski > > --- > > drivers/pwm/pwm-imx.c | 46 > > ++++++++++++++++++++++++++++++++++------------ 1 file changed, 34 > > insertions(+), 12 deletions(-) > >=20 > > diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c > > index c37d223..83e43d5 100644 > > --- a/drivers/pwm/pwm-imx.c > > +++ b/drivers/pwm/pwm-imx.c > > @@ -66,8 +66,6 @@ struct imx_chip { > > static int imx_pwm_config_v1(struct pwm_chip *chip, > > struct pwm_device *pwm, int duty_ns, int period_ns) > > { > > - struct imx_chip *imx =3D to_imx_chip(chip); > > - > > /* > > * The PWM subsystem allows for exact frequencies. However, > > * I cannot connect a scope on my device to the PWM line > > and @@ -85,26 +83,52 @@ static int imx_pwm_config_v1(struct > > pwm_chip *chip, > > * both the prescaler (/1 .. /128) and then by CLKSEL > > * (/2 .. /16). > > */ > > + struct imx_chip *imx =3D to_imx_chip(chip); > > u32 max =3D readl(imx->mmio_base + MX1_PWMP); > > u32 p =3D max * duty_ns / period_ns; > > + int ret; > > + > > + ret =3D clk_prepare_enable(imx->clk_ipg); > > + if (ret) > > + return ret; > > + > > writel(max - p, imx->mmio_base + MX1_PWMS); > > =20 > > + clk_disable_unprepare(imx->clk_ipg); > > + > > return 0; > > } > > =20 > > -static void imx_pwm_set_enable_v1(struct pwm_chip *chip, bool > > enable) +static int imx_pwm_enable_v1(struct pwm_chip *chip, struct > > pwm_device *pwm) { > > struct imx_chip *imx =3D to_imx_chip(chip); > > + int ret; > > u32 val; > > =20 > > + ret =3D clk_prepare_enable(imx->clk_ipg); > > + if (ret) > > + return ret; > > + > > val =3D readl(imx->mmio_base + MX1_PWMC); > > + val |=3D MX1_PWMC_EN; > > + writel(val, imx->mmio_base + MX1_PWMC); > > =20 > > - if (enable) > > - val |=3D MX1_PWMC_EN; > > - else > > - val &=3D ~MX1_PWMC_EN; > > + clk_disable_unprepare(imx->clk_per); >=20 > This looks wrong. You start by enabling clk_ipg which is needed for > register access, but then end with disabling clk_per which is needed > for driving the actual PWM output. This function should probably > enable clk_per on entry and enable clk_ipg while accessing registers. This was my omission. You are right, there are two clocks. One is feeding PWM block and another allows access to registers. Similar scheme is used with PWMv2. I will correct this when preparing version 2 of this patch set. Thanks for feedback, =C5=81ukasz Majewski >=20 > Sascha >=20 --Sig_/DJ0/uns5R.a.RUv92.kAiPY Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iEYEARECAAYFAlgPAcwACgkQf9/hG2YwgjHBKwCgnFVWjQxKpEwzzIwxLQWNuQ67 Sf8AoLL3cavGhTBcWsqScY9pq4NB2vKH =HojC -----END PGP SIGNATURE----- --Sig_/DJ0/uns5R.a.RUv92.kAiPY--