Message ID | 20231024101902.6689-3-nylon.chen@sifive.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:ce89:0:b0:403:3b70:6f57 with SMTP id p9csp1840032vqx; Tue, 24 Oct 2023 03:19:37 -0700 (PDT) X-Google-Smtp-Source: AGHT+IE880iLWWoTQoUq9yFtJKeOJ2YmnmvsBidr2V0nnZLV469LYYoaLrNPYRaxGjwdil6luskv X-Received: by 2002:a05:6359:1c0e:b0:168:eea8:64b0 with SMTP id us14-20020a0563591c0e00b00168eea864b0mr2502976rwb.22.1698142777036; Tue, 24 Oct 2023 03:19:37 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1698142776; cv=none; d=google.com; s=arc-20160816; b=0NpVyfyfGF/tfuBUr/Pn7fx3oVK9G3hq1ynibRM5wr1gBee4mRp6hYYj27dql0slbH EI07XRHUMl7a20XMMAlG9RIvsrRGWnSOZ5SInDL/LU5xQnYkQ0KLBf2lkbHImwH29Eo0 X6eD1/NROem64VDGJ58eIk6CKsbEJlUn3OsbX//V2uwOhiqa55I1jye2bF3AIAApdoMo Sdk7zi6Z4/Jkn63m5KVoP5WraDq9sVMEi//kkg2MJT3JNk74wUmLgQ4X/s8kYSooOK2Q TqNChceoeA9bxhMAU0nZOW4pbrmAfRmK/eegR67GX8BNAJsyT+V899XeageKjCH3v4tI Q2Vg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=a+QZY3JTroucCQn86akp4c5eSM8t3MeXGfG/H9g9b2Y=; fh=vzn9DV0ufbr7wYsjTsRdOZVrIZLib+mpPLCJoIeO9uA=; b=qGjvjOqkDc+YPbSvdIprUqJjnI9bIAKeYIAKf8nng2Fa+1DB2FzTgCwaHYo70ERPqO 7385BzR7n7zFrsIGFJ+REWu0tdlFuugbZNWLfdmXC9rVbrAx0acV2ivDznpkcHOAv+PL oO5SZwwExs1rns9sxaE8K6xUiV7kAAPJMseX1qIJX2sisWW3mu7x4Ao8XPCsQFVVdKn6 VC2j/GRygezbsOmDCJ9yxhk81bNxUFpAG0gjJqSHB9bz9XVuOF94XpOwytM3h/CCooiC /qM/bBNtPW5uGChb4JZJHhq2SLDtzcvVvjqgZhEGE86aTpVOUaqtwQru2NUCf6o8sLKy MbfA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sifive.com header.s=google header.b=T2xp4Zbc; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=sifive.com Received: from lipwig.vger.email (lipwig.vger.email. [2620:137:e000::3:3]) by mx.google.com with ESMTPS id w8-20020a63f508000000b005b64e8336casi8523713pgh.464.2023.10.24.03.19.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 24 Oct 2023 03:19:36 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3; Authentication-Results: mx.google.com; dkim=pass header.i=@sifive.com header.s=google header.b=T2xp4Zbc; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=sifive.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by lipwig.vger.email (Postfix) with ESMTP id 9457A80CE7C4; Tue, 24 Oct 2023 03:19:34 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at lipwig.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234465AbjJXKTY (ORCPT <rfc822;a1648639935@gmail.com> + 26 others); Tue, 24 Oct 2023 06:19:24 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57536 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234440AbjJXKTV (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 24 Oct 2023 06:19:21 -0400 Received: from mail-pf1-x42b.google.com (mail-pf1-x42b.google.com [IPv6:2607:f8b0:4864:20::42b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BBF7EDD for <linux-kernel@vger.kernel.org>; Tue, 24 Oct 2023 03:19:19 -0700 (PDT) Received: by mail-pf1-x42b.google.com with SMTP id d2e1a72fcca58-6bd96cfb99cso3401746b3a.2 for <linux-kernel@vger.kernel.org>; Tue, 24 Oct 2023 03:19:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sifive.com; s=google; t=1698142759; x=1698747559; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=a+QZY3JTroucCQn86akp4c5eSM8t3MeXGfG/H9g9b2Y=; b=T2xp4ZbcoN2/12TwNePmsUhIH7cKeqiI0b8NSjGBrwM/SxubEG8rlZo9IWNnCr0Lmw Sn0Px5A5Dg43SU9tLImZyFMLYUfX+9qQ27DCr7MMIXTjYaNp2a0tjWDl4wNYi0/fnK6g 9bEvJcha3RXbkTwZzcy8Crz1ZIG93QUEaqWyT/+iKxKwDJDKCOgL339iovypdYEQXvDJ tGdvPZn3z5Zezdlk7R/mgQLNT+s/W4H9+YOtNuk0/knP01Y/8KXgZFXZYVeibJG46yIj ppz0XR2p1Y7dhsoSWvBD8YCq2wt0bRQVqH45SDHOQ7urNaWZY88OUlJ4j4Hk9GQzA2bq oWnA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698142759; x=1698747559; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=a+QZY3JTroucCQn86akp4c5eSM8t3MeXGfG/H9g9b2Y=; b=oRwGXmqvcDflB44bbgqMDTWWiYtFUvxN9M4e9agtut9bPyAIJTKKcqwFegh8dTxqpR tlL8HsbNMktOP+9GP+YsA/i/odvsBdCEt84w3xmQpjHerhAs2VZALX7opTUj1KDpge0F SJoyHmFseklZ/305M1xRP+3qoi1s+J9Uq+R1XvXCFPKpfEr9GSbyc/bdk1RBKnGovB9X +heF9lJ1LrE1i/krG1dVgGvgasawMhyi9kxWnuzQ0B17EeKkg/WFJ7/Xcl75LqlVSxc+ spG0QPblR1DlGPwimg4rvr4zOTI2HeMHsiVE37UoDKhEHlavvBJP9lNPzQqdb7qThax9 Nh1g== X-Gm-Message-State: AOJu0Yyuj7Q0yMxnlKqjOtXoF60K+DULHyEadHH9pYOsP0XZVZusU6s1 e0JMy2HAAnb5D7Q9o5gd93ZLF3zGwHo9COL5YQWn9g== X-Received: by 2002:a05:6a00:27a0:b0:690:c306:151a with SMTP id bd32-20020a056a0027a000b00690c306151amr9848589pfb.0.1698142759118; Tue, 24 Oct 2023 03:19:19 -0700 (PDT) Received: from hsinchu15.internal.sifive.com (59-124-168-89.hinet-ip.hinet.net. [59.124.168.89]) by smtp.gmail.com with ESMTPSA id u202-20020a6279d3000000b006b3dc56c944sm7708372pfc.133.2023.10.24.03.19.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 24 Oct 2023 03:19:18 -0700 (PDT) From: Nylon Chen <nylon.chen@sifive.com> To: linux-pwm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org, devicetree@vger.kernel.org, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, paul.walmsley@sifive.com, palmer@dabbelt.com, aou@eecs.berkeley.edu, thierry.reding@gmail.com, u.kleine-koenig@pengutronix.de, emil.renner.berthing@canonical.com, vincent.chen@sifive.com Cc: nylon.chen@sifive.com, greentime.hu@sifive.com, zong.li@sifive.com, nylon7717@gmail.com Subject: [v5 2/2] pwm: sifive: change the PWM controlled LED algorithm Date: Tue, 24 Oct 2023 18:19:02 +0800 Message-ID: <20231024101902.6689-3-nylon.chen@sifive.com> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231024101902.6689-1-nylon.chen@sifive.com> References: <20231024101902.6689-1-nylon.chen@sifive.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Tue, 24 Oct 2023 03:19:34 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1780631760402449957 X-GMAIL-MSGID: 1780631760402449957 |
Series |
Change PWM-controlled LED pin active mode and algorithm
|
|
Commit Message
Nylon Chen
Oct. 24, 2023, 10:19 a.m. UTC
The `frac` variable represents the pulse inactive time, and the result of this algorithm is the pulse active time. Therefore, we must reverse the result. The reference is SiFive FU740-C000 Manual[0] Link: https://sifive.cdn.prismic.io/sifive/1a82e600-1f93-4f41-b2d8-86ed8b16acba_fu740-c000-manual-v1p6.pdf [0] Signed-off-by: Nylon Chen <nylon.chen@sifive.com> Signed-off-by: Vincent Chen <vincent.chen@sifive.com> --- drivers/pwm/pwm-sifive.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
Comments
Hi, Ping on the series. Uwe, is there anything more I can do to push the process forward? Nylon Chen <nylon.chen@sifive.com> 於 2023年10月24日 週二 下午6:19寫道: > > The `frac` variable represents the pulse inactive time, and the result > of this algorithm is the pulse active time. Therefore, we must reverse the result. > > The reference is SiFive FU740-C000 Manual[0] > > Link: https://sifive.cdn.prismic.io/sifive/1a82e600-1f93-4f41-b2d8-86ed8b16acba_fu740-c000-manual-v1p6.pdf [0] > > Signed-off-by: Nylon Chen <nylon.chen@sifive.com> > Signed-off-by: Vincent Chen <vincent.chen@sifive.com> > --- > drivers/pwm/pwm-sifive.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c > index eabddb7c7820..353c2342fbf1 100644 > --- a/drivers/pwm/pwm-sifive.c > +++ b/drivers/pwm/pwm-sifive.c > @@ -101,7 +101,7 @@ static void pwm_sifive_update_clock(struct pwm_sifive_ddata *ddata, > > /* As scale <= 15 the shift operation cannot overflow. */ > num = (unsigned long long)NSEC_PER_SEC << (PWM_SIFIVE_CMPWIDTH + scale); > - ddata->real_period = div64_ul(num, rate); > + ddata->real_period = DIV_ROUND_UP_ULL(num, rate); > dev_dbg(ddata->chip.dev, > "New real_period = %u ns\n", ddata->real_period); > } > @@ -121,13 +121,14 @@ static int pwm_sifive_get_state(struct pwm_chip *chip, struct pwm_device *pwm, > state->enabled = false; > > state->period = ddata->real_period; > + > + duty = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - duty; > state->duty_cycle = > (u64)duty * ddata->real_period >> PWM_SIFIVE_CMPWIDTH; > - state->polarity = PWM_POLARITY_INVERSED; > + state->polarity = PWM_POLARITY_NORMAL; > > return 0; > } > - > static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm, > const struct pwm_state *state) > { > @@ -139,7 +140,7 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm, > int ret = 0; > u32 frac; > > - if (state->polarity != PWM_POLARITY_INVERSED) > + if (state->polarity != PWM_POLARITY_NORMAL) > return -EINVAL; > > cur_state = pwm->state; > @@ -158,6 +159,7 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm, > num = (u64)duty_cycle * (1U << PWM_SIFIVE_CMPWIDTH); > frac = DIV64_U64_ROUND_CLOSEST(num, state->period); > /* The hardware cannot generate a 100% duty cycle */ > + frac = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - frac; > frac = min(frac, (1U << PWM_SIFIVE_CMPWIDTH) - 1); > > mutex_lock(&ddata->lock); > -- > 2.42.0 >
On 09/11/2023 08:02, Nylon Chen wrote: > Hi, Ping on the series. > > Uwe, is there anything more I can do to push the process forward? It's merge window. What do you exactly expect to happen? Best regards, Krzysztof
Hi Ping on the series. The merge window should have ended. Is there anything more I can do to push the process forward? Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 於 2023年11月9日 週四 下午4:22寫道: > > On 09/11/2023 08:02, Nylon Chen wrote: > > Hi, Ping on the series. > > > > Uwe, is there anything more I can do to push the process forward? > > It's merge window. What do you exactly expect to happen? > > Best regards, > Krzysztof >
Hello Nylon, On Tue, Oct 24, 2023 at 06:19:02PM +0800, Nylon Chen wrote: > The `frac` variable represents the pulse inactive time, and the result > of this algorithm is the pulse active time. Therefore, we must reverse the result. > > The reference is SiFive FU740-C000 Manual[0] > > Link: https://sifive.cdn.prismic.io/sifive/1a82e600-1f93-4f41-b2d8-86ed8b16acba_fu740-c000-manual-v1p6.pdf [0] > > Signed-off-by: Nylon Chen <nylon.chen@sifive.com> > Signed-off-by: Vincent Chen <vincent.chen@sifive.com> > --- > drivers/pwm/pwm-sifive.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c > index eabddb7c7820..353c2342fbf1 100644 > --- a/drivers/pwm/pwm-sifive.c > +++ b/drivers/pwm/pwm-sifive.c > @@ -101,7 +101,7 @@ static void pwm_sifive_update_clock(struct pwm_sifive_ddata *ddata, > > /* As scale <= 15 the shift operation cannot overflow. */ > num = (unsigned long long)NSEC_PER_SEC << (PWM_SIFIVE_CMPWIDTH + scale); > - ddata->real_period = div64_ul(num, rate); > + ddata->real_period = DIV_ROUND_UP_ULL(num, rate); It's unclear to me, why you changed that. > dev_dbg(ddata->chip.dev, > "New real_period = %u ns\n", ddata->real_period); > } > @@ -121,13 +121,14 @@ static int pwm_sifive_get_state(struct pwm_chip *chip, struct pwm_device *pwm, > state->enabled = false; > > state->period = ddata->real_period; > + > + duty = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - duty; I would have placed that directly after duty = readl(...); which then also influences state->enabled = duty > 0; (as it should?). > state->duty_cycle = > (u64)duty * ddata->real_period >> PWM_SIFIVE_CMPWIDTH; > - state->polarity = PWM_POLARITY_INVERSED; > + state->polarity = PWM_POLARITY_NORMAL; > > return 0; > } > - > static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm, > const struct pwm_state *state) > { > @@ -139,7 +140,7 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm, > int ret = 0; > u32 frac; > > - if (state->polarity != PWM_POLARITY_INVERSED) > + if (state->polarity != PWM_POLARITY_NORMAL) > return -EINVAL; > > cur_state = pwm->state; > @@ -158,6 +159,7 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm, > num = (u64)duty_cycle * (1U << PWM_SIFIVE_CMPWIDTH); > frac = DIV64_U64_ROUND_CLOSEST(num, state->period); > /* The hardware cannot generate a 100% duty cycle */ > + frac = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - frac; > frac = min(frac, (1U << PWM_SIFIVE_CMPWIDTH) - 1); frac can only be > (1U << PWM_SIFIVE_CMPWIDTH) - 1 if an overflow happend the line above. Is that what you want here? > mutex_lock(&ddata->lock); Best regards Uwe
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> 於 2023年12月12日 週二 上午4:50寫道: > > Hello Nylon, Hi Uwe, thanks for your feedback. > > > > On Tue, Oct 24, 2023 at 06:19:02PM +0800, Nylon Chen wrote: > > The `frac` variable represents the pulse inactive time, and the result > > of this algorithm is the pulse active time. Therefore, we must reverse the result. > > > > The reference is SiFive FU740-C000 Manual[0] > > > > Link: https://sifive.cdn.prismic.io/sifive/1a82e600-1f93-4f41-b2d8-86ed8b16acba_fu740-c000-manual-v1p6.pdf [0] > > > > Signed-off-by: Nylon Chen <nylon.chen@sifive.com> > > Signed-off-by: Vincent Chen <vincent.chen@sifive.com> > > --- > > drivers/pwm/pwm-sifive.c | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c > > index eabddb7c7820..353c2342fbf1 100644 > > --- a/drivers/pwm/pwm-sifive.c > > +++ b/drivers/pwm/pwm-sifive.c > > @@ -101,7 +101,7 @@ static void pwm_sifive_update_clock(struct pwm_sifive_ddata *ddata, > > > > /* As scale <= 15 the shift operation cannot overflow. */ > > num = (unsigned long long)NSEC_PER_SEC << (PWM_SIFIVE_CMPWIDTH + scale); > > - ddata->real_period = div64_ul(num, rate); > > + ddata->real_period = DIV_ROUND_UP_ULL(num, rate); > > It's unclear to me, why you changed that. Because there is a gap in idempotent tests. e.g. root@unmatched:~# echo 110 > /sys/devices/platform/led-controller-1/leds/d12/brightness [ 706.987712] .apply is not idempotent (ena=1 pol=0 1739692/4032985) -> (ena=1 pol=0 1739630/4032985) root@unmatched:~# echo 120 > /sys/devices/platform/led-controller-1/leds/d12/brightness [ 709.817554] .apply is not idempotent (ena=1 pol=0 1897846/4032985) -> (ena=1 pol=0 1897784/4032985) Round the result to the nearest whole number. This ensures that real_period is always a reasonable integer that is not lower than the actual value. After modification, idempotent errors can be avoided. > > > > dev_dbg(ddata->chip.dev, > > "New real_period = %u ns\n", ddata->real_period); > > } > > @@ -121,13 +121,14 @@ static int pwm_sifive_get_state(struct pwm_chip *chip, struct pwm_device *pwm, > > state->enabled = false; > > > > state->period = ddata->real_period; > > + > > + duty = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - duty; > > I would have placed that directly after > > duty = readl(...); > > which then also influences > > state->enabled = duty > 0; > > (as it should?). > Yes, you are right. I will make relevant corrections. ... duty = readl(ddata->regs + PWM_SIFIVE_PWMCMP(pwm->hwpwm)); + duty = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - duty; - state->enabled = duty <= 65535; + state->enabled = duty > 0; ... state->period = ddata->real_period; - duty = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - duty; > > > state->duty_cycle = > > (u64)duty * ddata->real_period >> PWM_SIFIVE_CMPWIDTH; > > - state->polarity = PWM_POLARITY_INVERSED; > > + state->polarity = PWM_POLARITY_NORMAL; > > > > return 0; > > } > > - > > static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm, > > const struct pwm_state *state) > > { > > @@ -139,7 +140,7 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm, > > int ret = 0; > > u32 frac; > > > > - if (state->polarity != PWM_POLARITY_INVERSED) > > + if (state->polarity != PWM_POLARITY_NORMAL) > > return -EINVAL; > > > > cur_state = pwm->state; > > @@ -158,6 +159,7 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm, > > num = (u64)duty_cycle * (1U << PWM_SIFIVE_CMPWIDTH); > > frac = DIV64_U64_ROUND_CLOSEST(num, state->period); > > /* The hardware cannot generate a 100% duty cycle */ > > + frac = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - frac; > > frac = min(frac, (1U << PWM_SIFIVE_CMPWIDTH) - 1); > > frac can only be > (1U << PWM_SIFIVE_CMPWIDTH) - 1 if an overflow > happend the line above. Is that what you want here? I made a mistake, I pushed the wrong changes. I want to invert it after taking the minimum value, which makes sense to me. frac = DIV64_U64_ROUND_CLOSEST(num, state->period); /* The hardware cannot generate a 100% duty cycle */ frac = min(frac, (1U << PWM_SIFIVE_CMPWIDTH) - 1); + frac = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - frac; > > > mutex_lock(&ddata->lock); > > Best regards > Uwe Best regards Nylon > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello Nylon, On Mon, Jan 08, 2024 at 04:27:40PM +0800, Nylon Chen wrote: > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> 於 2023年12月12日 週二 上午4:50寫道: > > On Tue, Oct 24, 2023 at 06:19:02PM +0800, Nylon Chen wrote: > > > diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c > > > index eabddb7c7820..353c2342fbf1 100644 > > > --- a/drivers/pwm/pwm-sifive.c > > > +++ b/drivers/pwm/pwm-sifive.c > > > @@ -101,7 +101,7 @@ static void pwm_sifive_update_clock(struct pwm_sifive_ddata *ddata, > > > > > > /* As scale <= 15 the shift operation cannot overflow. */ > > > num = (unsigned long long)NSEC_PER_SEC << (PWM_SIFIVE_CMPWIDTH + scale); > > > - ddata->real_period = div64_ul(num, rate); > > > + ddata->real_period = DIV_ROUND_UP_ULL(num, rate); > > > > It's unclear to me, why you changed that. > Because there is a gap in idempotent tests. > e.g. > root@unmatched:~# echo 110 > > /sys/devices/platform/led-controller-1/leds/d12/brightness > [ 706.987712] .apply is not idempotent (ena=1 pol=0 1739692/4032985) > -> (ena=1 pol=0 1739630/4032985) > root@unmatched:~# echo 120 > > /sys/devices/platform/led-controller-1/leds/d12/brightness > [ 709.817554] .apply is not idempotent (ena=1 pol=0 1897846/4032985) > -> (ena=1 pol=0 1897784/4032985) > > Round the result to the nearest whole number. This ensures that > real_period is always a reasonable integer that is not lower than the > actual value. > > After modification, idempotent errors can be avoided. That's very welcome, however I think this should be a separate change. I'll think about the rest of your changes when you send a new patch. Best regards Uwe
diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c index eabddb7c7820..353c2342fbf1 100644 --- a/drivers/pwm/pwm-sifive.c +++ b/drivers/pwm/pwm-sifive.c @@ -101,7 +101,7 @@ static void pwm_sifive_update_clock(struct pwm_sifive_ddata *ddata, /* As scale <= 15 the shift operation cannot overflow. */ num = (unsigned long long)NSEC_PER_SEC << (PWM_SIFIVE_CMPWIDTH + scale); - ddata->real_period = div64_ul(num, rate); + ddata->real_period = DIV_ROUND_UP_ULL(num, rate); dev_dbg(ddata->chip.dev, "New real_period = %u ns\n", ddata->real_period); } @@ -121,13 +121,14 @@ static int pwm_sifive_get_state(struct pwm_chip *chip, struct pwm_device *pwm, state->enabled = false; state->period = ddata->real_period; + + duty = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - duty; state->duty_cycle = (u64)duty * ddata->real_period >> PWM_SIFIVE_CMPWIDTH; - state->polarity = PWM_POLARITY_INVERSED; + state->polarity = PWM_POLARITY_NORMAL; return 0; } - static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm, const struct pwm_state *state) { @@ -139,7 +140,7 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm, int ret = 0; u32 frac; - if (state->polarity != PWM_POLARITY_INVERSED) + if (state->polarity != PWM_POLARITY_NORMAL) return -EINVAL; cur_state = pwm->state; @@ -158,6 +159,7 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm, num = (u64)duty_cycle * (1U << PWM_SIFIVE_CMPWIDTH); frac = DIV64_U64_ROUND_CLOSEST(num, state->period); /* The hardware cannot generate a 100% duty cycle */ + frac = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - frac; frac = min(frac, (1U << PWM_SIFIVE_CMPWIDTH) - 1); mutex_lock(&ddata->lock);