From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.9 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_PASS,T_DKIMWL_WL_HIGH,URIBL_BLOCKED,USER_AGENT_NEOMUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 07CF9C04AB1 for ; Thu, 9 May 2019 07:20:46 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id CD88121019 for ; Thu, 9 May 2019 07:20:45 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="Nug8PJ1a" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CD88121019 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=pengutronix.de Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=3IbdOyA9C+OUemA0KUxmKXUKiSBwHYB9JAOOx5IY4JQ=; b=Nug8PJ1aFFJIZC RMy+fKEfqMkptdf84C1FFBbuIfjajLVF3GJCrU8KgpJ/pXJ4VxvNkMRQnvrwNHj5mJh1XVEiOECwU Dpg31gsWkgXWgoHayzFP5/PylP7Y9CJYCtUqxwk6YJoqUXKUHXH+SulOqpGn4JjkiOSlNi99fYZhZ OC7XgwKOQn+wR04D/UtwTvh0ZpAq0Q7+8+90qFoLEU83QDSqslQcmZ5cTR1aP5BYjiAhPm5PwsXzl wLBe7vRCfu83sQPu3vknU7nTxMS6wEe067DsRKPOE7tvHRmXGS1mVjio0Z1iJTnzz9sXMhafwVisp r1Jp+XcQxQ3NYYeiVJUw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1hOdM2-0005xt-Oq; Thu, 09 May 2019 07:20:34 +0000 Received: from metis.ext.pengutronix.de ([2001:67c:670:201:290:27ff:fe1d:cc33]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1hOdLz-0005xS-67 for linux-arm-kernel@lists.infradead.org; Thu, 09 May 2019 07:20:33 +0000 Received: from pty.hi.pengutronix.de ([2001:67c:670:100:1d::c5]) by metis.ext.pengutronix.de with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1hOdLo-0001DW-QJ; Thu, 09 May 2019 09:20:20 +0200 Received: from ukl by pty.hi.pengutronix.de with local (Exim 4.89) (envelope-from ) id 1hOdLm-0007dA-Ad; Thu, 09 May 2019 09:20:18 +0200 Date: Thu, 9 May 2019 09:20:18 +0200 From: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= To: Anson Huang Subject: Re: [PATCH V11 2/5] pwm: Add i.MX TPM PWM driver support Message-ID: <20190509072018.cenumkgbysfba57l@pengutronix.de> References: <1554860547-18237-1-git-send-email-Anson.Huang@nxp.com> <1554860547-18237-3-git-send-email-Anson.Huang@nxp.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1554860547-18237-3-git-send-email-Anson.Huang@nxp.com> User-Agent: NeoMutt/20170113 (1.7.2) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c5 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-arm-kernel@lists.infradead.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190509_002031_469042_E363BA03 X-CRM114-Status: GOOD ( 27.74 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "mark.rutland@arm.com" , "linux-pwm@vger.kernel.org" , Robin Gong , "otavio@ossystems.com.br" , "devicetree@vger.kernel.org" , "festevam@gmail.com" , "s.hauer@pengutronix.de" , "linux@armlinux.org.uk" , "robh+dt@kernel.org" , "linux-kernel@vger.kernel.org" , "thierry.reding@gmail.com" , "stefan@agner.ch" , "kernel@pengutronix.de" , Leonard Crestez , "shawnguo@kernel.org" , "linux-arm-kernel@lists.infradead.org" , dl-linux-imx Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hello, On Wed, Apr 10, 2019 at 01:47:40AM +0000, Anson Huang wrote: > i.MX7ULP has TPM(Low Power Timer/Pulse Width Modulation Module) > inside, it can support multiple PWM channels, all the channels > share same counter and period setting, but each channel can > configure its duty and polarity independently. > = > There are several TPM modules in i.MX7ULP, the number of channels > in TPM modules are different, it can be read from each TPM module's > PARAM register. > = > Signed-off-by: Anson Huang > --- > Changes since V10: > - remove channel private data which is ONLY for storing polarity, just r= ead it from HW register; > - improve pwm_imx_tpm_round_state() and pwm_imx_tpm_apply_hw() parameter= s sequence; > - improve comments for polarity setting; > - refuse polarity change if PWM is active. > --- > drivers/pwm/Kconfig | 11 ++ > drivers/pwm/Makefile | 1 + > drivers/pwm/pwm-imx-tpm.c | 442 ++++++++++++++++++++++++++++++++++++++++= ++++++ > 3 files changed, 454 insertions(+) > create mode 100644 drivers/pwm/pwm-imx-tpm.c > = > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig > index c054bd1..1311b540 100644 > --- a/drivers/pwm/Kconfig > +++ b/drivers/pwm/Kconfig > @@ -210,6 +210,17 @@ config PWM_IMX27 > To compile this driver as a module, choose M here: the module > will be called pwm-imx27. > = > +config PWM_IMX_TPM > + tristate "i.MX TPM PWM support" > + depends on ARCH_MXC || COMPILE_TEST > + depends on HAVE_CLK && HAS_IOMEM > + help > + Generic PWM framework driver for i.MX7ULP TPM module, TPM's full > + name is Low Power Timer/Pulse Width Modulation Module. > + > + To compile this driver as a module, choose M here: the module > + will be called pwm-imx-tpm. > + > config PWM_JZ4740 > tristate "Ingenic JZ47xx PWM support" > depends on MACH_INGENIC > diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile > index 448825e..c368599 100644 > --- a/drivers/pwm/Makefile > +++ b/drivers/pwm/Makefile > @@ -19,6 +19,7 @@ obj-$(CONFIG_PWM_HIBVT) +=3D pwm-hibvt.o > obj-$(CONFIG_PWM_IMG) +=3D pwm-img.o > obj-$(CONFIG_PWM_IMX1) +=3D pwm-imx1.o > obj-$(CONFIG_PWM_IMX27) +=3D pwm-imx27.o > +obj-$(CONFIG_PWM_IMX_TPM) +=3D pwm-imx-tpm.o > obj-$(CONFIG_PWM_JZ4740) +=3D pwm-jz4740.o > obj-$(CONFIG_PWM_LP3943) +=3D pwm-lp3943.o > obj-$(CONFIG_PWM_LPC18XX_SCT) +=3D pwm-lpc18xx-sct.o > diff --git a/drivers/pwm/pwm-imx-tpm.c b/drivers/pwm/pwm-imx-tpm.c > new file mode 100644 > index 0000000..9349f4f > --- /dev/null > +++ b/drivers/pwm/pwm-imx-tpm.c > @@ -0,0 +1,442 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright 2018-2019 NXP. > + * > + * Limitations: > + * - The TPM counter and period counter are shared between > + * multiple channels, so all channels should use same period > + * settings. > + * - Changes to polarity cannot be latched at the time of the > + * next period start. > + * - Changing period and duty cycle together isn't atomic, > + * with the wrong timing it might happen that a period is > + * produced with old duty cycle but new period settings. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define PWM_IMX_TPM_PARAM 0x4 > +#define PWM_IMX_TPM_GLOBAL 0x8 > +#define PWM_IMX_TPM_SC 0x10 > +#define PWM_IMX_TPM_CNT 0x14 > +#define PWM_IMX_TPM_MOD 0x18 > +#define PWM_IMX_TPM_CnSC(n) (0x20 + (n) * 0x8) > +#define PWM_IMX_TPM_CnV(n) (0x24 + (n) * 0x8) > + > +#define PWM_IMX_TPM_PARAM_CHAN GENMASK(7, 0) > + > +#define PWM_IMX_TPM_SC_PS GENMASK(2, 0) > +#define PWM_IMX_TPM_SC_CMOD GENMASK(4, 3) > +#define PWM_IMX_TPM_SC_CMOD_INC_EVERY_CLK FIELD_PREP(PWM_IMX_TPM_SC_CMOD= , 1) > +#define PWM_IMX_TPM_SC_CPWMS BIT(5) > + > +#define PWM_IMX_TPM_CnSC_CHF BIT(7) > +#define PWM_IMX_TPM_CnSC_MSB BIT(5) > +#define PWM_IMX_TPM_CnSC_MSA BIT(4) > + > +/* > + * The reference manual describes this field as two separate bits. The > + * semantic of the two bits isn't orthogonal though, so they are treated > + * together as a 2-bit field here. > + */ > +#define PWM_IMX_TPM_CnSC_ELS GENMASK(3, 2) > +#define PWM_IMX_TPM_CnSC_ELS_INVERSED FIELD_PREP(PWM_IMX_TPM_CnSC_ELS, 1) > +#define PWM_IMX_TPM_CnSC_ELS_NORMAL FIELD_PREP(PWM_IMX_TPM_CnSC_ELS, 2) > + > + > +#define PWM_IMX_TPM_MOD_WIDTH 16 > +#define PWM_IMX_TPM_MOD_MOD GENMASK(PWM_IMX_TPM_MOD_WIDTH - 1, 0) > + > +struct imx_tpm_pwm_chip { > + struct pwm_chip chip; > + struct clk *clk; > + void __iomem *base; > + struct mutex lock; > + u32 user_count; > + u32 enable_count; > + u32 real_period; > +}; > + > +struct imx_tpm_pwm_param { > + u8 prescale; > + u32 mod; > + u32 val; > +}; > + > +static inline struct imx_tpm_pwm_chip *to_imx_tpm_pwm_chip(struct pwm_ch= ip *chip) > +{ > + return container_of(chip, struct imx_tpm_pwm_chip, chip); > +} > + Maybe add a comment here describing the purpose of this function. Something like: /* = * This function determines for a given pwm_state *state that a consumer * might request the pwm_state *realstate that eventually is implemented * by the hardware and the necessary register values (in *p) to achive * this. */ I didn't revalidate all the maths in this driver but assume they are still right from the previous rounds. If you add the comment I suggested above, feel free to also add Reviewed-by: Uwe Kleine-K=F6nig > +static int pwm_imx_tpm_round_state(struct pwm_chip *chip, > + struct imx_tpm_pwm_param *p, > + struct pwm_state *real_state, > + struct pwm_state *state) > +{ > + struct imx_tpm_pwm_chip *tpm =3D to_imx_tpm_pwm_chip(chip); > + u32 rate, prescale, period_count, clock_unit; > + u64 tmp; > + > + rate =3D clk_get_rate(tpm->clk); > + tmp =3D (u64)state->period * rate; > + clock_unit =3D DIV_ROUND_CLOSEST_ULL(tmp, NSEC_PER_SEC); > + if (clock_unit <=3D PWM_IMX_TPM_MOD_MOD) > + prescale =3D 0; > + else > + prescale =3D ilog2(clock_unit) + 1 - PWM_IMX_TPM_MOD_WIDTH; > + > + if ((!FIELD_FIT(PWM_IMX_TPM_SC_PS, prescale))) > + return -ERANGE; It's a bit sad that my ideas for the core concerning a round_rate callback don't go down particularly well on Thierry's side. = With the way I suggested we'd continue with prescale =3D 7 in this case. As of now there is no rule which kind of deviation to accept and which not. :-| (Nothing this patch can change of course.) Best regards Uwe -- = Pengutronix e.K. | Uwe Kleine-K=F6nig | Industrial Linux Solutions | http://www.pengutronix.de/ | _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel