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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 3B32EC38A2D for ; Thu, 27 Oct 2022 09:24:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: In-Reply-To:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date: Reply-To:Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date :Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=15hfWsh0yaQY9GcS4LSxrIKDrYWhK91iIRnykVDOCS0=; b=rvW8Eh1i8W+BitXY7t3XfwxBSK /nspxY2srG51ieNsKvFsqQHxHBOw0mxUtH0/ECpYZwy4YLWm4f6sM86tnVE4/X/fhGkSNQ1vE5YcT KScfr1589PYn14s+U8rI9Ycvyao1a5xVmCYYTIWQemXL5hwL0IbWmm8mt7TUDhEtyLhSXSpOfzP8o glqiLrgxiaIhRNavaLwVAOKnPGkT150J5JD2V5xAIvsBTAV7Qbukcv6Ru47nIGvLk4rpvgRMDqNYq jPnYq4r84aEtNQIsKY5kefLmVmTxLV8UEdXo8hqzwa8D2HeWML4wwa1HQO5BKnsNnOoe3FNQ1A6Fn hOzXD7eQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1onz69-00Ca5w-9A; Thu, 27 Oct 2022 09:22:49 +0000 Received: from metis.ext.pengutronix.de ([2001:67c:670:201:290:27ff:fe1d:cc33]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1onz5y-00Ca2C-GZ for linux-arm-kernel@lists.infradead.org; Thu, 27 Oct 2022 09:22:40 +0000 Received: from drehscheibe.grey.stw.pengutronix.de ([2a0a:edc0:0:c01:1d::a2]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1onz5p-0007LF-Or; Thu, 27 Oct 2022 11:22:29 +0200 Received: from [2a0a:edc0:0:900:1d::77] (helo=ptz.office.stw.pengutronix.de) by drehscheibe.grey.stw.pengutronix.de with esmtp (Exim 4.94.2) (envelope-from ) id 1onz5p-000fr5-4o; Thu, 27 Oct 2022 11:22:28 +0200 Received: from ukl by ptz.office.stw.pengutronix.de with local (Exim 4.94.2) (envelope-from ) id 1onz5n-00B4Xo-IF; Thu, 27 Oct 2022 11:22:27 +0200 Date: Thu, 27 Oct 2022 11:22:27 +0200 From: Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= To: Romain Perier Cc: Thierry Reding , Lee Jones , Daniel Palmer , Rob Herring , Mark Brown , linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, linux-pwm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 2/5] pwm: Add support for the MSTAR MSC313 PWM Message-ID: <20221027092227.wkxjsw6mep3o3auc@pengutronix.de> References: <20220907131241.31941-1-romain.perier@gmail.com> <20220907131241.31941-3-romain.perier@gmail.com> <20220927163314.rzfld7sqchsdfvfg@pengutronix.de> MIME-Version: 1.0 In-Reply-To: X-SA-Exim-Connect-IP: 2a0a:edc0:0:c01:1d::a2 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-20221027_022238_744235_AE069FF6 X-CRM114-Status: GOOD ( 47.25 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: multipart/mixed; boundary="===============8046560404995670714==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============8046560404995670714== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="gok45hmzf5nviwgp" Content-Disposition: inline --gok45hmzf5nviwgp Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hello Romain, On Thu, Oct 27, 2022 at 10:36:10AM +0200, Romain Perier wrote: > Le mar. 27 sept. 2022 =E0 18:33, Uwe Kleine-K=F6nig > a =E9crit : > > > > Hello Romain, hello Daniel, > > > > adding Mark Brown to Cc: for the regmap stuff. > > > > On Wed, Sep 07, 2022 at 03:12:38PM +0200, Romain Perier wrote: > > > From: Daniel Palmer > > > > > > This adds support for the PWM block on the Mstar MSC313e SoCs and new= er. > > > > > > Signed-off-by: Daniel Palmer > > > Co-developed-by: Romain Perier > > > Signed-off-by: Romain Perier > > > --- > > > MAINTAINERS | 1 + > > > drivers/pwm/Kconfig | 9 ++ > > > drivers/pwm/Makefile | 1 + > > > drivers/pwm/pwm-msc313e.c | 269 ++++++++++++++++++++++++++++++++++++= ++ > > > 4 files changed, 280 insertions(+) > > > create mode 100644 drivers/pwm/pwm-msc313e.c > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index 9d7f64dc0efe..c3b39b09097c 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -2439,6 +2439,7 @@ F: arch/arm/mach-mstar/ > > > F: drivers/clk/mstar/ > > > F: drivers/clocksource/timer-msc313e.c > > > F: drivers/gpio/gpio-msc313.c > > > +F: drivers/pwm/pwm-msc313e.c > > > F: drivers/rtc/rtc-msc313.c > > > F: drivers/watchdog/msc313e_wdt.c > > > F: include/dt-bindings/clock/mstar-* > > > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig > > > index 60d13a949bc5..8049fd03a821 100644 > > > --- a/drivers/pwm/Kconfig > > > +++ b/drivers/pwm/Kconfig > > > @@ -372,6 +372,15 @@ config PWM_MESON > > > To compile this driver as a module, choose M here: the module > > > will be called pwm-meson. > > > > > > +config PWM_MSC313E > > > + tristate "MStar MSC313e PWM support" > > > + depends on ARCH_MSTARV7 || COMPILE_TEST > > > + help > > > + Generic PWM framework driver for MSTAR MSC313e. > > > + > > > + To compile this driver as a module, choose M here: the module > > > + will be called pwm-msc313e. > > > + > > > config PWM_MTK_DISP > > > tristate "MediaTek display PWM driver" > > > depends on ARCH_MEDIATEK || COMPILE_TEST > > > diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile > > > index 7bf1a29f02b8..bc285c054f2a 100644 > > > --- a/drivers/pwm/Makefile > > > +++ b/drivers/pwm/Makefile > > > @@ -62,4 +62,5 @@ obj-$(CONFIG_PWM_TWL) +=3D pwm-twl.o > > > obj-$(CONFIG_PWM_TWL_LED) +=3D pwm-twl-led.o > > > obj-$(CONFIG_PWM_VISCONTI) +=3D pwm-visconti.o > > > obj-$(CONFIG_PWM_VT8500) +=3D pwm-vt8500.o > > > +obj-$(CONFIG_PWM_MSC313E) +=3D pwm-msc313e.o > > > obj-$(CONFIG_PWM_XILINX) +=3D pwm-xilinx.o > > > diff --git a/drivers/pwm/pwm-msc313e.c b/drivers/pwm/pwm-msc313e.c > > > new file mode 100644 > > > index 000000000000..a71f39ba66c3 > > > --- /dev/null > > > +++ b/drivers/pwm/pwm-msc313e.c > > > @@ -0,0 +1,269 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Copyright (C) 2021 Daniel Palmer > > > + * Copyright (C) 2022 Romain Perier > > > + */ > > > + > > > +#include > > > +#include > > > +#include > > > +#include > > > + > > > +#define DRIVER_NAME "msc313e-pwm" > > > + > > > +#define CHANNEL_OFFSET 0x80 > > > +#define REG_DUTY 0x8 > > > +#define REG_PERIOD 0x10 > > > +#define REG_DIV 0x18 > > > +#define REG_CTRL 0x1c > > > +#define REG_SWRST 0x1fc > > > + > > > +struct msc313e_pwm_channel { > > > + struct regmap_field *clkdiv; > > > + struct regmap_field *polarity; > > > + struct regmap_field *dutyl; > > > + struct regmap_field *dutyh; > > > + struct regmap_field *periodl; > > > + struct regmap_field *periodh; > > > + struct regmap_field *swrst; > > > +}; > > > + > > > +struct msc313e_pwm { > > > + struct regmap *regmap; > > > + struct pwm_chip pwmchip; > > > + struct clk *clk; > > > + struct msc313e_pwm_channel channels[]; > > > +}; > > > + > > > +struct msc313e_pwm_info { > > > + unsigned int channels; > > > +}; > > > + > > > +#define to_msc313e_pwm(ptr) container_of(ptr, struct msc313e_pwm, pw= mchip) > > > + > > > +static const struct regmap_config msc313e_pwm_regmap_config =3D { > > > + .reg_bits =3D 16, > > > + .val_bits =3D 16, > > > + .reg_stride =3D 4, > > > +}; > > > + > > > +static const struct msc313e_pwm_info msc313e_data =3D { > > > + .channels =3D 8, > > > +}; > > > + > > > +static const struct msc313e_pwm_info ssd20xd_data =3D { > > > + .channels =3D 4, > > > +}; > > > + > > > +static void msc313e_pwm_writecounter(struct regmap_field *low, struc= t regmap_field *high, u32 value) > > > +{ > > > + /* The bus that connects the CPU to the peripheral registers sp= lits 32 bit registers into > > > > Please fix the comment style to use /* on a line for itself. Also for > > comments staying below 80 chars per line is appreciated. >=20 > even if check-patch.pl --strict passed ? ^^ I also already wondered about check-patch not demanding this. *shrug* > > > + * two 16bit registers placed 4 bytes apart. It's the hardware = design they used. The counter > > > + * we are about to write has this contrainst. > > > > s/contrainst/contraint/ > > > > I wonder if that could be abstracted by regmap?! >=20 > I had the same thought, not from what I have read/found, but perhaps > the regmap maintainer has an opinion. >=20 > > > > > + */ > > > + regmap_field_write(low, value & 0xffff); > > > + regmap_field_write(high, value >> 16); > > > +} > > > + > > > +static void msc313e_pwm_readcounter(struct regmap_field *low, struct= regmap_field *high, u32 *value) > > > +{ > > > + unsigned int val =3D 0; > > > + > > > + regmap_field_read(low, &val); > > > + *value =3D val; > > > + regmap_field_read(high, &val); > > > + *value =3D (val << 16) | *value; > > > +} > > > + > > > +static int msc313e_pwm_config(struct pwm_chip *chip, struct pwm_devi= ce *device, > > > + int duty_ns, int period_ns) > > > +{ > > > + struct msc313e_pwm *pwm =3D to_msc313e_pwm(chip); > > > + unsigned long long nspertick =3D DIV_ROUND_DOWN_ULL(NSEC_PER_SE= C, clk_get_rate(pwm->clk)); > > > + struct msc313e_pwm_channel *channel =3D &pwm->channels[device->= hwpwm]; > > > + unsigned long long div =3D 1; > > > + > > > + /* Fit the period into the period register by prescaling the cl= k */ > > > + while (DIV_ROUND_DOWN_ULL(period_ns, nspertick) > 0x3ffff) { > > > > dividing by the result of a division looses precision. Also rounding > > down both divisions looks wrong. >=20 > Such cases are not supposed to be covered by PWM_DEBUG ? (because > everything passed with PWM_DEBUG) Note that PWM_DEBUG being silent isn't an indicator that everything is fine. It cannot catch everything and so doesn't replace human review. If you tell me what clk_get_rate() returns for you, I might be able to tell you a procedure that makes PWM_DEBUG unhappy. Best regards Uwe --=20 Pengutronix e.K. | Uwe Kleine-K=F6nig | Industrial Linux Solutions | https://www.pengutronix.de/ | --gok45hmzf5nviwgp Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCgAdFiEEfnIqFpAYrP8+dKQLwfwUeK3K7AkFAmNaTdAACgkQwfwUeK3K 7AnMqgf8DzFsEd9BI2kFodRUCfC6mjEpPasgkBKB6PUykVv8BpgpneGcod1ZuGaf mFa8bCcs+2m/KIBKGaO6EXjWd5/8v1jwfTUMi6tArh2KXYcODvyWBDkAFenw5jqX 3K+dpScgpoD+tBam1wzgkCsDBqH7aS7tk0yTEJZEQd1U0kTiKUTFLHarnmlqQcAZ ZaTQdSuA4DXfKVEIsXo+PxMF6VUuW96tvnktN15Q1qe1baW7hymbe5w3sGOLYI3c AxQI5QeEshwegQ+CmJNYFE+s96MuMG2tA+Y9C7Jyzb2iooqkIPB65sS3DnJaa4Gh NT22oQSFzN+YTPBRGIjPos0kq84n2A== =Umol -----END PGP SIGNATURE----- --gok45hmzf5nviwgp-- --===============8046560404995670714== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --===============8046560404995670714==--