Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752046AbcL1WKN (ORCPT ); Wed, 28 Dec 2016 17:10:13 -0500 Received: from 2.mo5.mail-out.ovh.net ([178.33.109.111]:34716 "EHLO 2.mo5.mail-out.ovh.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751159AbcL1WKM (ORCPT ); Wed, 28 Dec 2016 17:10:12 -0500 Date: Wed, 28 Dec 2016 23:01:16 +0100 From: Lukasz Majewski To: Stefan Agner Cc: Thierry Reding , Sascha Hauer , linux-pwm@vger.kernel.org, Bhuvanchandra DV , linux-kernel@vger.kernel.org, Fabio Estevam , Fabio Estevam , Boris Brezillon , Lothar Wassmann , kernel@pengutronix.de, Philipp Zabel Subject: Re: [PATCH v3 RESEND 02/11] pwm: imx: remove ipg clock Message-ID: <20161228230116.4bf078ce@jawa> In-Reply-To: <20161227085401.0115add5@jawa> References: <1477259146-19167-1-git-send-email-l.majewski@majess.pl> <1482792961-12702-1-git-send-email-l.majewski@majess.pl> <1482792961-12702-3-git-send-email-l.majewski@majess.pl> <46d2d434303d416645f27bb20358d46e@agner.ch> <20161227085401.0115add5@jawa> 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_/XmXcs2UN1t7YlEGUCE0uvQp"; protocol="application/pgp-signature" X-Ovh-Tracer-Id: 3909405953177928263 X-VR-SPAMSTATE: OK X-VR-SPAMSCORE: -100 X-VR-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrfeelgedrtddvgdduheegucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuqfggjfdpvefjgfevmfevgfenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddm Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4642 Lines: 145 --Sig_/XmXcs2UN1t7YlEGUCE0uvQp Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Hi Stefan, > Hi Stefan, >=20 > > On 2016-12-26 23:55, Lukasz Majewski wrote: > > > From: Sascha Hauer > > >=20 > > > The use of the ipg clock was introduced with commit 7b27c160c681 > > > ("pwm: i.MX: fix clock lookup"). > > > In the commit message it was claimed that the ipg clock is enabled > > > for register accesses. This is true for the ->config() callback, > > > but not for the ->set_enable() callback. Given that the ipg clock > > > is not consistently enabled for all register accesses we can > > > assume that either it is not required at all or that the current > > > code does not work. Remove the ipg clock code for now so that > > > it's no longer in the way of refactoring the driver. > >=20 > > Hi Lukasz, > >=20 > > Has my concern addressed in any way with this resend? > > https://lkml.org/lkml/2016/11/22/729 >=20 > Unfortunately not, since I don't have iMX7 for testing. >=20 > >=20 > > Breaking hardware is usually not an option :-) >=20 > Yes, I know, but >=20 > Please look on the patch set from my perspective: >=20 > I originally wanted to add polarity inversion to PWM. Then, there was > the request from you and Boris to go with "atomicity" support, so I > converted the driver to support it. >=20 > This patch set has been resent on purpose at the end of merge window, > so we do have some time to fix it if it would be accepted to -next > tree (or any other PWM related one). Moreover, the burden for > preparing patches would be smaller - since we all have agreed that > "atomicity" is a more than welcome feature. >=20 >=20 > >=20 > > I checked the i.MX 7 reference manual again, and in this case the > > peripheral access clock is a clock line named "ipg_clk_s" (Table > > 12-20), with a clock root "PWM1_CLK_ROOT" (Table 5-12). In i.MX 7 > > all clocks are behind a single gate, so in fact it does not matter > > which clock we take. Given that others have peripheral access > > behind the "pwm" gate, I guess we should take the "pwm" gate... >=20 >=20 > If possible please prepare a patch. It would be the best solution. If I might ask - are you willing to prepare patch to fix iMX7 or shall I roll back to the ipg code already present in main line ? Best regards, =C5=81ukasz Majewski >=20 > Thanks in advance, > =C5=81ukasz Majewski >=20 > >=20 > > -- > > Stefan > >=20 > > >=20 > > > Signed-off-by: Sascha Hauer > > > Cc: Philipp Zabel > > > --- > > > [commit message text refactored by Lukasz Majewski > > > ] --- > > > Changes for v3: > > > - New patch > > > --- > > > drivers/pwm/pwm-imx.c | 19 +------------------ > > > 1 file changed, 1 insertion(+), 18 deletions(-) > > >=20 > > > diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c > > > index d600fd5..70609ef2 100644 > > > --- a/drivers/pwm/pwm-imx.c > > > +++ b/drivers/pwm/pwm-imx.c > > > @@ -49,7 +49,6 @@ > > > =20 > > > struct imx_chip { > > > struct clk *clk_per; > > > - struct clk *clk_ipg; > > > =20 > > > void __iomem *mmio_base; > > > =20 > > > @@ -204,17 +203,8 @@ static int imx_pwm_config(struct pwm_chip > > > *chip, struct pwm_device *pwm, int duty_ns, int period_ns) > > > { > > > struct imx_chip *imx =3D to_imx_chip(chip); > > > - int ret; > > > - > > > - ret =3D clk_prepare_enable(imx->clk_ipg); > > > - if (ret) > > > - return ret; > > > =20 > > > - ret =3D imx->config(chip, pwm, duty_ns, period_ns); > > > - > > > - clk_disable_unprepare(imx->clk_ipg); > > > - > > > - return ret; > > > + return imx->config(chip, pwm, duty_ns, period_ns); > > > } > > > =20 > > > static int imx_pwm_enable(struct pwm_chip *chip, struct > > > pwm_device *pwm) @@ -293,13 +283,6 @@ static int > > > imx_pwm_probe(struct platform_device *pdev) return > > > PTR_ERR(imx->clk_per); } > > > =20 > > > - imx->clk_ipg =3D devm_clk_get(&pdev->dev, "ipg"); > > > - if (IS_ERR(imx->clk_ipg)) { > > > - dev_err(&pdev->dev, "getting ipg clock failed > > > with %ld\n", > > > - PTR_ERR(imx->clk_ipg)); > > > - return PTR_ERR(imx->clk_ipg); > > > - } > > > - > > > imx->chip.ops =3D &imx_pwm_ops; > > > imx->chip.dev =3D &pdev->dev; > > > imx->chip.base =3D -1; >=20 --Sig_/XmXcs2UN1t7YlEGUCE0uvQp Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iEYEARECAAYFAlhkNjIACgkQf9/hG2YwgjGOSwCfdgzVSNid+J90RPWHPARq3VmW mMgAn0qphcAz6t6o9rZ5KbJrL6lFVlkD =WVzC -----END PGP SIGNATURE----- --Sig_/XmXcs2UN1t7YlEGUCE0uvQp--