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=-2.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no 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 0D5C4C64E90 for ; Sun, 29 Nov 2020 17:51:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D254D206DF for ; Sun, 29 Nov 2020 17:51:30 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=gmx.net header.i=@gmx.net header.b="Gts6FsyC" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728314AbgK2RvP (ORCPT ); Sun, 29 Nov 2020 12:51:15 -0500 Received: from mout.gmx.net ([212.227.17.20]:42665 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726470AbgK2RvO (ORCPT ); Sun, 29 Nov 2020 12:51:14 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1606672133; bh=q5EfoxNx3GvtONQ7tlxc5ju4+UKEtK3w/Aju5T/lU34=; h=X-UI-Sender-Class:Date:From:To:Cc:Subject:References:In-Reply-To; b=Gts6FsyCRBclIgLeMbu+ykktP0TVR2B5D9ViHP8xcHK/0SV367mIvdY9ji1ZkI9k7 rUmxbpLN17ZnIGzFMpCO6WWWwl+EMyuTicJ13EmKPrS/w8ZSJDCe7ceHDjWJp5t1N3 Dp28OQe31+RnsjaNTGYA2Jm7G/Gfvnpwq8i/zpew= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from longitude ([37.201.214.162]) by mail.gmx.com (mrgmx104 [212.227.17.168]) with ESMTPSA (Nemesis) id 1M26vB-1klLYq2i1h-002VfI; Sun, 29 Nov 2020 18:48:53 +0100 Date: Sun, 29 Nov 2020 18:48:43 +0100 From: Jonathan =?utf-8?Q?Neusch=C3=A4fer?= To: Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= Cc: Jonathan =?utf-8?Q?Neusch=C3=A4fer?= , Mark Brown , Alexandre Belloni , Heiko Stuebner , devicetree@vger.kernel.org, Linus Walleij , Thierry Reding , Sam Ravnborg , linux-rtc@vger.kernel.org, Arnd Bergmann , Mauro Carvalho Chehab , Fabio Estevam , Daniel Palmer , Andy Shevchenko , Andreas Kemnade , NXP Linux Team , linux-pwm@vger.kernel.org, Stephan Gerhold , allen , Sascha Hauer , Lubomir Rintel , Rob Herring , Lee Jones , linux-arm-kernel@lists.infradead.org, Alessandro Zummo , linux-kernel@vger.kernel.org, Pengutronix Kernel Team , Heiko Stuebner , Josua Mayer , Shawn Guo , "David S. Miller" Subject: Re: [PATCH v4 4/7] pwm: ntxec: Add driver for PWM function in Netronix EC Message-ID: <20201129174843.GF456020@latitude> References: <20201122222739.1455132-1-j.neuschaefer@gmx.net> <20201122222739.1455132-5-j.neuschaefer@gmx.net> <20201124082019.vpkr3xnp55arjpnp@pengutronix.de> <20201126231931.GE456020@latitude> <20201127071105.k2rb4iykeqevbao5@pengutronix.de> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="o0ZfoUVt4BxPQnbU" Content-Disposition: inline In-Reply-To: <20201127071105.k2rb4iykeqevbao5@pengutronix.de> X-Provags-ID: V03:K1:yZA5pcBjayKfsaxQ10SfBrM5TtDKdp9YyH3brKYnvC8zSTMCWqx h0UaKAduTVL83Gr5kOuKiX4XYYsAUiszORA1/rugd6Na78VA4FYZ4JTiuhVU7BbuVEsSIpJ 100aI3LX+F30cUiLSNnMzKeZVS5eS+M+e0Rb0ykW2sIKcX/Q1g0V8GhnvF1oDTpO+uWjqh5 fkeIuG8MVZVukVfY2XOyg== X-UI-Out-Filterresults: notjunk:1;V03:K0:M+Qcdze4yRY=:6JxxKN0ysRb87ucwiWXXLY swxGPxXnUJUAs391j0nHMvRjcgdmD0w4+fAauujDWIe1JD9Dd4gc7BhllY+AA1sLumM1IG8EJ RALnpXvJbh2/4ZZ5z9XvGRm4liw1xddf8y2F3svKn3KVefZW86dY/mzPEYyj4J6IFveUK/y3O quY64o3L2y2iTiAY/I8fgmeOEWJ8juNkB//SAZ16DnMF/yyeHO561iMHh8hO5vfr2tGNCy6jK nnI1lyiPdgrp3cIIfvqKGLmBaZJ40U26SVUy2vTCQ+oxg8AzDsl75RiQcJL/xgsIrqEVh/Plo 2+mtlMn/cOaLSV/4ka1QyojdmemXYrVv5oRsNVOzv57/9+johssutH5ll1Dp7XslH+v74mnVR gI79cahkevbD9e8QjJW6wAq7YHiv+mKwr2YD/34yj9bLFYqwyGyWETYoMlARMPsiluHIzSjMv T1kP/C2Jvhk5Wn9hojRMVD2f0t5P6sPh1ZI+Q+EMCcF5PLxxa/nbtKXLy6cNj+LpFg2wJR7ns zIWseKjMWUFnpc7TG+LjKW/V/hJ/oTNmALWsclbC6I1C2CS8//HlPMMKrugBtSGHfqiFVHz0T JsicR7Rkg4R1LChi998PSbZhIo1r/yTMSm1pberGgl03vGMQdi6XqGXaqEPS+SnOMdVZQGp0M /w+0EfPFgeyzmxBlA5ZkDvaR6IwlLI6C5KEmUFeEBeh/0JFLSpKkEiMSscyCh+3at95VRpnuR yUEFJWrWl/j2DDQyh18HIFjeAq4sQVq73iUUjfLCAoJiYQAwfdjFGgtv5Mr9hu5sK+lErpjXj RzIRZbJcbgWM+v512HhnJ03AgF0gZtdqbVuKFa0a5AX8OZsv9997wVVqUtYEnCIkmcXG+j725 S//VM3O+OWENLmLgmdBg== Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --o0ZfoUVt4BxPQnbU Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Nov 27, 2020 at 08:11:05AM +0100, Uwe Kleine-K=C3=B6nig wrote: > Hello Jonathan, >=20 > On Fri, Nov 27, 2020 at 12:19:31AM +0100, Jonathan Neusch=C3=A4fer wrote: > > On Tue, Nov 24, 2020 at 09:20:19AM +0100, Uwe Kleine-K=C3=B6nig wrote: [...] > > > state->duty_cycle and state->period are u64, so you're losing > > > information here. Consider state->duty_cycle =3D 0x100000001 and > > > state->period =3D 0x200000001. > >=20 > > Oh, good point, I didn't notice the truncation. > >=20 > > The reason I picked unsigned int was to avoid a 64-bit division; > > I suppose I can do something like this: > >=20 > > period =3D (u32)period / TIME_BASE_NS; > > duty =3D (u32)duty / TIME_BASE_NS; >=20 > You can do that after you checked period > MAX_PERIOD_NS below, yes. > Something like: >=20 > if (state->polarity !=3D PWM_POLARITY_NORMAL) > return -EINVAL; >=20 > if (state->period > MAX_PERIOD_NS) { > period =3D MAX_PERIOD_NS; > else > period =3D state->period; >=20 > if (state->duty_cycle > period) > duty_cycle =3D period; > else > duty_cycle =3D state->duty_cycle; >=20 > should work with even keeping the local variables as unsigned int. With the min_t() macro, this becomes nice and short: period =3D min_t(u64, state->period, MAX_PERIOD_NS); duty =3D min_t(u64, state->duty_cycle, period); period /=3D TIME_BASE_NS; duty /=3D TIME_BASE_NS; > > > I think I already asked, but I don't remember the reply: What happens= to > > > the output between these writes? A comment here about this would be > > > suitable. > >=20 > > I will add something like the following: > >=20 > > /* > > * Changes to the period and duty cycle take effect as soon as the > > * corresponding low byte is written, so the hardware may be configured > > * to an inconsistent state after the period is written and before the > > * duty cycle is fully written. If, in such a case, the old duty cycle > > * is longer than the new period, the EC will output 100% for a moment. > > */ >=20 > Is the value pair taken over by hardware atomically? That is, is it > really "will" in your last line, or only "might". (E.g. when changing > from duty_cycle, period =3D 1000, 2000 to 500, 800 and a new cycle begins > after reducing period, the new duty_cycle is probably written before the > counter reaches 500. Do we get a 100% cycle here?) I am not sure when exactly a new period or duty cycle value is applied, and I don't have the test equipment to measure it. I'll change the text to "may output 100%". > Other than that the info is fine. Make sure to point this out in the > Limitations paragraph at the top of the driver please, too. Okay. > > > /* > > > * The current state cannot be read out, so there is no .get_state > > > * callback. > > > */ > > >=20 > > > Hmm, at least you could provice a .get_state() callback that reports = the > > > setting that was actually implemented for in the last call to .apply(= )? > >=20 > > Yes... I see two options: > >=20 > > 1. Caching the state in the driver's private struct. I'm not completely > > convinced of the value, given that the information is mostly > > available in the PWM core already (except for the adjustments that > > the driver makes). > >=20 > > 2. Writing the adjusted state back into pwm_dev->state (via pwm_set_*). > > This seems a bit dirty. >=20 > 2. isn't a good option. Maybe regmap caches this stuff anyhow for 1. (or > can be made doing that)? With regmap caching, I'd be concerned that a read operation may slip through and reach the device, producing a bogus result. Not sure if write-only/write-through caching can be configured in regmap. Thanks, Jonathan --o0ZfoUVt4BxPQnbU Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEvHAHGBBjQPVy+qvDCDBEmo7zX9sFAl/D3vAACgkQCDBEmo7z X9t1kA//Rr2iUNp6uHCpqNEZM8RTiPQXN3QbeqMwR17sfoIUcaVl5lT0gD949Ea8 VbzZAp+3xPIlkGjs6JvjpDm9VVEM1jIV78+SVdvbAf7jW6g4WoM+xsSpCz1oD3QK MU2M6v/hkW51EaTB05SKxYWVNkHrEYcxjLv2pjuBE/WP7MATjX1CzksJ8y3QSrYc 4j4os/8/ulznBElr1epcNr+IpMEotlV6JT7W+CQzW1Zj2ASOJjUjy9uUGBcOUUfH v7Sfe/SFS0Dniy6GDaBQj0r13rRGyevR+TQBRUcrrIogEn8rc51KDMZW1FvAp1Sw +a/ZURqwce2YP03FuFxMoJhePJRfqxXIQ82/BG51sE6MwnVwNls4dDIZGlHMzfPD crmjBo0Tk3G2WMQIiPtJoRbKNLqQ3J+RoU4+P0ch0eQcvrOqzn54wJpVAPE1/aHl lWFa5bzXUTdSxVoHzBu5iNcUnlVp4J8USdo59u/l0Sj7ZLnXRtrJQL58Xkf0yh9m VtyK02UJ7m9YfwtCo6j8yqk1/uJ1PlqyABrYgMOMtUDcUXKBLgnjB+vu+x/NLtEu Mwd8vA6dJYL6rATyv4l90Aagh97wjnTy9WjdgydXv/5kyEPzbGmihsGcX07/tcCc qiLeT9MG0S0a2/TKef6MfDMOEBszXiQ9jbCkHIBTQW3nCBGYGRE= =eiSy -----END PGP SIGNATURE----- --o0ZfoUVt4BxPQnbU-- 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=-0.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no 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 24131C3E8C5 for ; Sun, 29 Nov 2020 17:51:10 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 9586F206D8 for ; Sun, 29 Nov 2020 17:51:09 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="ZHQ8cm87"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=gmx.net header.i=@gmx.net header.b="Gts6FsyC" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9586F206D8 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmx.net Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+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=merlin.20170209; h=Sender: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-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=SKBoVE8BX+p15jaTCOmNFVstlqf+WeSxDJib/ura3OI=; b=ZHQ8cm87cX6hz6d6E/2TUzpwg MINtOFnYyLV47UmetMPS8bi5DkNghIv+wAkmSNQwGuxSJ4T0aO3lUOAvWyyRg8SBDCb48IpqXI2Bp 4h1MTOmTkVWOJPUPawm0e4/dL3WXRTUCzuhIIEB9G90gFiGsGMKdt457fLGAHt/xfTiCC8rMb0Rod FVx0sGPkz3KSjhmF4Yf/5/tlgp9SRkOXoFQsWQCVX6dK+woIGUJYkMKKIQP4MnbgbnybkUhv1T5cA 3lS9GK6/eP+cQvCfJhuDgssteL1DIGNR54hmjs4ls6zBIeHUpUG2Qzslt2QcsVJqvo7nkK5jUmipg /RfRaUlBA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kjQpR-0004ip-0P; Sun, 29 Nov 2020 17:49:41 +0000 Received: from mout.gmx.net ([212.227.17.20]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kjQpN-0004iB-PR for linux-arm-kernel@lists.infradead.org; Sun, 29 Nov 2020 17:49:38 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1606672133; bh=q5EfoxNx3GvtONQ7tlxc5ju4+UKEtK3w/Aju5T/lU34=; h=X-UI-Sender-Class:Date:From:To:Cc:Subject:References:In-Reply-To; b=Gts6FsyCRBclIgLeMbu+ykktP0TVR2B5D9ViHP8xcHK/0SV367mIvdY9ji1ZkI9k7 rUmxbpLN17ZnIGzFMpCO6WWWwl+EMyuTicJ13EmKPrS/w8ZSJDCe7ceHDjWJp5t1N3 Dp28OQe31+RnsjaNTGYA2Jm7G/Gfvnpwq8i/zpew= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from longitude ([37.201.214.162]) by mail.gmx.com (mrgmx104 [212.227.17.168]) with ESMTPSA (Nemesis) id 1M26vB-1klLYq2i1h-002VfI; Sun, 29 Nov 2020 18:48:53 +0100 Date: Sun, 29 Nov 2020 18:48:43 +0100 From: Jonathan =?utf-8?Q?Neusch=C3=A4fer?= To: Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= Subject: Re: [PATCH v4 4/7] pwm: ntxec: Add driver for PWM function in Netronix EC Message-ID: <20201129174843.GF456020@latitude> References: <20201122222739.1455132-1-j.neuschaefer@gmx.net> <20201122222739.1455132-5-j.neuschaefer@gmx.net> <20201124082019.vpkr3xnp55arjpnp@pengutronix.de> <20201126231931.GE456020@latitude> <20201127071105.k2rb4iykeqevbao5@pengutronix.de> MIME-Version: 1.0 In-Reply-To: <20201127071105.k2rb4iykeqevbao5@pengutronix.de> X-Provags-ID: V03:K1:yZA5pcBjayKfsaxQ10SfBrM5TtDKdp9YyH3brKYnvC8zSTMCWqx h0UaKAduTVL83Gr5kOuKiX4XYYsAUiszORA1/rugd6Na78VA4FYZ4JTiuhVU7BbuVEsSIpJ 100aI3LX+F30cUiLSNnMzKeZVS5eS+M+e0Rb0ykW2sIKcX/Q1g0V8GhnvF1oDTpO+uWjqh5 fkeIuG8MVZVukVfY2XOyg== X-UI-Out-Filterresults: notjunk:1;V03:K0:M+Qcdze4yRY=:6JxxKN0ysRb87ucwiWXXLY swxGPxXnUJUAs391j0nHMvRjcgdmD0w4+fAauujDWIe1JD9Dd4gc7BhllY+AA1sLumM1IG8EJ RALnpXvJbh2/4ZZ5z9XvGRm4liw1xddf8y2F3svKn3KVefZW86dY/mzPEYyj4J6IFveUK/y3O quY64o3L2y2iTiAY/I8fgmeOEWJ8juNkB//SAZ16DnMF/yyeHO561iMHh8hO5vfr2tGNCy6jK nnI1lyiPdgrp3cIIfvqKGLmBaZJ40U26SVUy2vTCQ+oxg8AzDsl75RiQcJL/xgsIrqEVh/Plo 2+mtlMn/cOaLSV/4ka1QyojdmemXYrVv5oRsNVOzv57/9+johssutH5ll1Dp7XslH+v74mnVR gI79cahkevbD9e8QjJW6wAq7YHiv+mKwr2YD/34yj9bLFYqwyGyWETYoMlARMPsiluHIzSjMv T1kP/C2Jvhk5Wn9hojRMVD2f0t5P6sPh1ZI+Q+EMCcF5PLxxa/nbtKXLy6cNj+LpFg2wJR7ns zIWseKjMWUFnpc7TG+LjKW/V/hJ/oTNmALWsclbC6I1C2CS8//HlPMMKrugBtSGHfqiFVHz0T JsicR7Rkg4R1LChi998PSbZhIo1r/yTMSm1pberGgl03vGMQdi6XqGXaqEPS+SnOMdVZQGp0M /w+0EfPFgeyzmxBlA5ZkDvaR6IwlLI6C5KEmUFeEBeh/0JFLSpKkEiMSscyCh+3at95VRpnuR yUEFJWrWl/j2DDQyh18HIFjeAq4sQVq73iUUjfLCAoJiYQAwfdjFGgtv5Mr9hu5sK+lErpjXj RzIRZbJcbgWM+v512HhnJ03AgF0gZtdqbVuKFa0a5AX8OZsv9997wVVqUtYEnCIkmcXG+j725 S//VM3O+OWENLmLgmdBg== X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201129_124938_113561_FF1BD96B X-CRM114-Status: GOOD ( 36.77 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Alexandre Belloni , Heiko Stuebner , linux-pwm@vger.kernel.org, Linus Walleij , Thierry Reding , Fabio Estevam , linux-rtc@vger.kernel.org, Arnd Bergmann , Mauro Carvalho Chehab , Sam Ravnborg , Daniel Palmer , Andy Shevchenko , Andreas Kemnade , NXP Linux Team , devicetree@vger.kernel.org, Stephan Gerhold , allen , Sascha Hauer , Jonathan =?utf-8?Q?Neusch=C3=A4fer?= , Lubomir Rintel , Rob Herring , Lee Jones , linux-arm-kernel@lists.infradead.org, Alessandro Zummo , linux-kernel@vger.kernel.org, Mark Brown , Pengutronix Kernel Team , Heiko Stuebner , Josua Mayer , Shawn Guo , "David S. Miller" Content-Type: multipart/mixed; boundary="===============8236946604644216750==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============8236946604644216750== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="o0ZfoUVt4BxPQnbU" Content-Disposition: inline --o0ZfoUVt4BxPQnbU Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Nov 27, 2020 at 08:11:05AM +0100, Uwe Kleine-K=C3=B6nig wrote: > Hello Jonathan, >=20 > On Fri, Nov 27, 2020 at 12:19:31AM +0100, Jonathan Neusch=C3=A4fer wrote: > > On Tue, Nov 24, 2020 at 09:20:19AM +0100, Uwe Kleine-K=C3=B6nig wrote: [...] > > > state->duty_cycle and state->period are u64, so you're losing > > > information here. Consider state->duty_cycle =3D 0x100000001 and > > > state->period =3D 0x200000001. > >=20 > > Oh, good point, I didn't notice the truncation. > >=20 > > The reason I picked unsigned int was to avoid a 64-bit division; > > I suppose I can do something like this: > >=20 > > period =3D (u32)period / TIME_BASE_NS; > > duty =3D (u32)duty / TIME_BASE_NS; >=20 > You can do that after you checked period > MAX_PERIOD_NS below, yes. > Something like: >=20 > if (state->polarity !=3D PWM_POLARITY_NORMAL) > return -EINVAL; >=20 > if (state->period > MAX_PERIOD_NS) { > period =3D MAX_PERIOD_NS; > else > period =3D state->period; >=20 > if (state->duty_cycle > period) > duty_cycle =3D period; > else > duty_cycle =3D state->duty_cycle; >=20 > should work with even keeping the local variables as unsigned int. With the min_t() macro, this becomes nice and short: period =3D min_t(u64, state->period, MAX_PERIOD_NS); duty =3D min_t(u64, state->duty_cycle, period); period /=3D TIME_BASE_NS; duty /=3D TIME_BASE_NS; > > > I think I already asked, but I don't remember the reply: What happens= to > > > the output between these writes? A comment here about this would be > > > suitable. > >=20 > > I will add something like the following: > >=20 > > /* > > * Changes to the period and duty cycle take effect as soon as the > > * corresponding low byte is written, so the hardware may be configured > > * to an inconsistent state after the period is written and before the > > * duty cycle is fully written. If, in such a case, the old duty cycle > > * is longer than the new period, the EC will output 100% for a moment. > > */ >=20 > Is the value pair taken over by hardware atomically? That is, is it > really "will" in your last line, or only "might". (E.g. when changing > from duty_cycle, period =3D 1000, 2000 to 500, 800 and a new cycle begins > after reducing period, the new duty_cycle is probably written before the > counter reaches 500. Do we get a 100% cycle here?) I am not sure when exactly a new period or duty cycle value is applied, and I don't have the test equipment to measure it. I'll change the text to "may output 100%". > Other than that the info is fine. Make sure to point this out in the > Limitations paragraph at the top of the driver please, too. Okay. > > > /* > > > * The current state cannot be read out, so there is no .get_state > > > * callback. > > > */ > > >=20 > > > Hmm, at least you could provice a .get_state() callback that reports = the > > > setting that was actually implemented for in the last call to .apply(= )? > >=20 > > Yes... I see two options: > >=20 > > 1. Caching the state in the driver's private struct. I'm not completely > > convinced of the value, given that the information is mostly > > available in the PWM core already (except for the adjustments that > > the driver makes). > >=20 > > 2. Writing the adjusted state back into pwm_dev->state (via pwm_set_*). > > This seems a bit dirty. >=20 > 2. isn't a good option. Maybe regmap caches this stuff anyhow for 1. (or > can be made doing that)? With regmap caching, I'd be concerned that a read operation may slip through and reach the device, producing a bogus result. Not sure if write-only/write-through caching can be configured in regmap. Thanks, Jonathan --o0ZfoUVt4BxPQnbU Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEvHAHGBBjQPVy+qvDCDBEmo7zX9sFAl/D3vAACgkQCDBEmo7z X9t1kA//Rr2iUNp6uHCpqNEZM8RTiPQXN3QbeqMwR17sfoIUcaVl5lT0gD949Ea8 VbzZAp+3xPIlkGjs6JvjpDm9VVEM1jIV78+SVdvbAf7jW6g4WoM+xsSpCz1oD3QK MU2M6v/hkW51EaTB05SKxYWVNkHrEYcxjLv2pjuBE/WP7MATjX1CzksJ8y3QSrYc 4j4os/8/ulznBElr1epcNr+IpMEotlV6JT7W+CQzW1Zj2ASOJjUjy9uUGBcOUUfH v7Sfe/SFS0Dniy6GDaBQj0r13rRGyevR+TQBRUcrrIogEn8rc51KDMZW1FvAp1Sw +a/ZURqwce2YP03FuFxMoJhePJRfqxXIQ82/BG51sE6MwnVwNls4dDIZGlHMzfPD crmjBo0Tk3G2WMQIiPtJoRbKNLqQ3J+RoU4+P0ch0eQcvrOqzn54wJpVAPE1/aHl lWFa5bzXUTdSxVoHzBu5iNcUnlVp4J8USdo59u/l0Sj7ZLnXRtrJQL58Xkf0yh9m VtyK02UJ7m9YfwtCo6j8yqk1/uJ1PlqyABrYgMOMtUDcUXKBLgnjB+vu+x/NLtEu Mwd8vA6dJYL6rATyv4l90Aagh97wjnTy9WjdgydXv/5kyEPzbGmihsGcX07/tcCc qiLeT9MG0S0a2/TKef6MfDMOEBszXiQ9jbCkHIBTQW3nCBGYGRE= =eiSy -----END PGP SIGNATURE----- --o0ZfoUVt4BxPQnbU-- --===============8236946604644216750== 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 --===============8236946604644216750==--