Message ID | 20230803085734.340-1-nylon.chen@sifive.com |
---|---|
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:9f41:0:b0:3e4:2afc:c1 with SMTP id v1csp1114945vqx; Thu, 3 Aug 2023 05:32:37 -0700 (PDT) X-Google-Smtp-Source: APBJJlGpuce92Shx7eoTX9HsQ1jjNZxDUfmCUnXCCXX5eLc87f1Y2Bt7FdH08oaGRdH6UiDPJJpr X-Received: by 2002:a17:90a:ba18:b0:268:68da:63fe with SMTP id s24-20020a17090aba1800b0026868da63femr16658366pjr.38.1691065956914; Thu, 03 Aug 2023 05:32:36 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1691065956; cv=none; d=google.com; s=arc-20160816; b=q4WME9zfZqV5V6JwlasvgEHDVf0+ypXoLDQ7KdDLJKqRXG5jnmzR4oHWzOmP6zIUtL FjKyAd2BuQjIKTQ+6KTha7p3ui/ZtwhRtvAaW9A2tyr5ye/YO9XhFL4EOSCeRl1pijbB kbFXK2XtA7/SYj0etoaBhcskRXGtAifK25PBlO1Gg/3NFeN0GHdy/ieFxGagf2CH4Ykb ppqGmyFfeZya0dph8JnhV8GQiL55PD1dXaZ+0rluzu5e5IGACQ2IhxOemgY4WOLbtNyo APtOFgQTScCceyv2+KnJvGwGxUJiHgeM8mH30yXZ7ggxQBRAnJi551U7WBBag/ifwG/Q lVwA== 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 :message-id:date:subject:cc:to:from:dkim-signature; bh=yLuMaVJQRwFXzrj2KAyjh6dQRHn4ylta+xeHAhU0hZc=; fh=ykrlczGktD/RozskNxGNltSE+PZUoKNV/g8GF10y7zI=; b=0Hqrnca8Z/1pnwPBuyHAYj+yDsU3WwZxUzAnk8k+gdQOLAyCqFa5dW0iPNT2f21adY e135dTNKnUCNsr1XX33MsP3C1uLB1e8cTdj/ReJK1AP8K7RMZrU6b15jiVwVxkRSRpIC Lzk4YG9E16ndA4kzAyJ+RcL5jknzjKxwKNchiMGM9rJ546fajZg/4e7HSXlfiT6jL8l2 japdxlCmUXsEPcxz1egg6W1j7pVgcWOzI29vEnbji4b4iRJKpuEUqyLmd6ghGVgZKW6K gz1AO347hR9oE69uXrwAuO1eIuoi+V8CTr9DQAHKUXLKrEzYI7h1Vwxmw10yHUzyk22F dzxw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sifive.com header.s=google header.b=Bu0D3tGj; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 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 (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id pg5-20020a17090b1e0500b002683f507990si3197716pjb.109.2023.08.03.05.32.11; Thu, 03 Aug 2023 05:32:36 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@sifive.com header.s=google header.b=Bu0D3tGj; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=sifive.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234884AbjHCI6W (ORCPT <rfc822;jeff.pang.chn@gmail.com> + 99 others); Thu, 3 Aug 2023 04:58:22 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42650 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234911AbjHCI5x (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 3 Aug 2023 04:57:53 -0400 Received: from mail-pj1-x102c.google.com (mail-pj1-x102c.google.com [IPv6:2607:f8b0:4864:20::102c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CFE8B1FC2 for <linux-kernel@vger.kernel.org>; Thu, 3 Aug 2023 01:57:42 -0700 (PDT) Received: by mail-pj1-x102c.google.com with SMTP id 98e67ed59e1d1-268b3ddc894so357847a91.1 for <linux-kernel@vger.kernel.org>; Thu, 03 Aug 2023 01:57:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sifive.com; s=google; t=1691053061; x=1691657861; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=yLuMaVJQRwFXzrj2KAyjh6dQRHn4ylta+xeHAhU0hZc=; b=Bu0D3tGjQV9Pc4NF3RI5B5EiGg4B5cv6DLumG8NB9wnCVaSTXNEZzxw48JRUczYBZg zia2PWyNqRQVA1lY1qZBnJ95b8Kr3cIHSKrJ+uLCY5rcKIWSx6IDWM8IryDShuLSGoE5 pn9BtS0M7KZ4IzyfSh0JZ0j6l5cXaQojDx7yqnjlwtlmOnpvMJnlEs6kfTLnZCkEqAiU D14t5UbsxeVCvzkkUSbJhYQ8kVoIOwIZDDmcEvkSF3lTBYlUBm0nnfSfKvF+pX3pa0gD Dl1uFzTOlCW3eVyT7CNUSofcEAwnHXRh5EUHt9kC4E0DQEhfSNDOz8JZAG2r6AqJmufg +xHw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691053061; x=1691657861; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=yLuMaVJQRwFXzrj2KAyjh6dQRHn4ylta+xeHAhU0hZc=; b=Eh9PtNYuXZHbY082P/X8KMj13lJKJmdkXu17SHbrZJe3SQDr7R5cjmp4PBjOF0YFvU d6E8y0zsaAhocvAP7XtEqOdnZEJV9eSEdfQTrPFscQ5OpxyKQFWCY2FLqdDbgEpfrZzc k7EUKKkdfuPsZHl1YQ/7lFjc1hPlFb/QuU+FA+1362XNgTxNON0bK8F7ziBxks7hDK0m U6h95QmSw2XA2LHzaIutUWfSnJ/4IxdU86nPIOWn+B4VB7ZAHaLi0ewFS7L4Hw4R0AJE nG2yFDnImUSPJz44u/iUTD4u+/vROrDPecBsAbE/JaCrzj5Erie4M23d7FXBVm9NP5ET sF/A== X-Gm-Message-State: ABy/qLbR9NTPaSgI84ajqM1xFClBw2B5N6ZE8rGD74FUkU+UbjX285Qk 43MDmJdPBrsTyEwLcC5B9cHgsulIKtUxfK2ulIbHclw/zRc/U4XZM3x5zlqnzH714YVDmjcgWqn GqjLZFhdCqt9wvZevUSJlYIV7i0RnghS8nTth/kcCg6f3LyqvKB4sQZbpCzyKuITASrhiuRo9ni A2U4PkQW2J3g== X-Received: by 2002:a17:90a:a592:b0:268:daa4:3a70 with SMTP id b18-20020a17090aa59200b00268daa43a70mr8066701pjq.32.1691053061337; Thu, 03 Aug 2023 01:57:41 -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 i6-20020a633c46000000b0056334a7b9b2sm12941735pgn.33.2023.08.03.01.57.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 03 Aug 2023 01:57:40 -0700 (PDT) From: Nylon Chen <nylon.chen@sifive.com> To: linux-kernel@vger.kernel.org Cc: devicetree@vger.kernel.org, linux-riscv@lists.infradead.org, geert+renesas@glider.be, pavel@ucw.cz, vincent.chen@sifive.com, nylon.chen@sifive.com, emil.renner.berthing@canonical.com, aou@eecs.berkeley.edu, palmer@dabbelt.com, paul.walmsley@sifive.com, krzysztof.kozlowski+dt@linaro.org, robh+dt@kernel.org, conor@kernel.org, zong.li@sifive.com Subject: [PATCH v4 0/1] Change PWM-controlled LED pin active mode and algorithm Date: Thu, 3 Aug 2023 16:57:33 +0800 Message-Id: <20230803085734.340-1-nylon.chen@sifive.com> X-Mailer: git-send-email 2.40.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1773205383829115196 X-GMAIL-MSGID: 1773211177229761685 |
Series |
Change PWM-controlled LED pin active mode and algorithm
|
|
Message
Nylon Chen
Aug. 3, 2023, 8:57 a.m. UTC
According to the circuit diagram of User LEDs - RGB described in themanual hifive-unleashed-a00.pdf[0] and hifive-unmatched-schematics-v3.pdf[1]. The behavior of PWM is acitve-high. Removed patches: 1 New patches: (none) Links: - [0]: https://sifive.cdn.prismic.io/sifive/c52a8e32-05ce-4aaf-95c8-7bf8453f8698_hifive-unleashed-a00-schematics-1.pdf - [1]: https://sifive.cdn.prismic.io/sifive/6a06d6c0-6e66-49b5-8e9e-e68ce76f4192_hifive-unmatched-schematics-v3.pdf - [2]: https://sifive.cdn.prismic.io/sifive/1a82e600-1f93-4f41-b2d8-86ed8b16acba_fu740-c000-manual-v1p6.pdf Changed in v4: - Remove previous updates to the PWM algorithm. Changed in v3: - Convert the reference link to standard link. - Move the inverted function before taking the minimum value. - Change polarity check condition(high and low). - Pick the biggest period length possible that is not bigger than the requested period. Changed in v2: - Convert the reference link to standard link. - Fix typo: s/sifive unmatched:/sifive: unmatched:/. - Remove active-low from hifive-unleashed-a00.dts. - Include this reference link in the dts and pwm commit messages. Nylon Chen (1): riscv: dts: sifive: unleashed/unmatched: Remove PWM controlled LED's active-low properties arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts | 4 ---- arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts | 4 ---- 2 files changed, 8 deletions(-)
Comments
Hi Conor, Conor Dooley <conor.dooley@microchip.com> 於 2023年8月3日 週四 下午5:44寫道: > > Hey Nylon, > > (I yoinked the reply to 1/1 to here, as it makes more sense in this > context) > > > On Thu, Aug 03, 2023 at 10:15:10AM +0100, Conor Dooley wrote: > > > On Thu, Aug 03, 2023 at 04:57:33PM +0800, Nylon Chen wrote: > > > > According to the circuit diagram of User LEDs - RGB described in themanual hifive-unleashed-a00.pdf[0] and hifive-unmatched-schematics-v3.pdf[1]. > > > > > > > > The behavior of PWM is acitve-high. > > > > > > > > Removed patches: 1 > > > > New patches: (none) > > > > > > > > Links: > > > > - [0]: https://sifive.cdn.prismic.io/sifive/c52a8e32-05ce-4aaf-95c8-7bf8453f8698_hifive-unleashed-a00-schematics-1.pdf > > > > - [1]: https://sifive.cdn.prismic.io/sifive/6a06d6c0-6e66-49b5-8e9e-e68ce76f4192_hifive-unmatched-schematics-v3.pdf > > > > - [2]: https://sifive.cdn.prismic.io/sifive/1a82e600-1f93-4f41-b2d8-86ed8b16acba_fu740-c000-manual-v1p6.pdf > > > > > > > > Changed in v4: > > > > - Remove previous updates to the PWM algorithm. > > > > > > Why? I don't recall the conclusion on the previous version being that > > > that patch was not needed. > > > > I apologize for forgetting about this update earlier. Just now, > > I tried to pull rebase master and noticed that other developers seem > > to have made some fixes to the algorithm. Upon closer inspection, I > > found that they addressed the part we previously discussed with Emil > > and Uwe, such as "first pwm_apply_state." > > > > Therefore, my instinct tells me that they have already taken care of > > the issues we discussed before. > > I didn't see anything in linux-next that would solve this problem of > inversion. The last meaningful change is: > commit 334c7b13d38321e47d1a51dba0bef9f4c403ec75 > Author: Emil Renner Berthing <emil.renner.berthing@canonical.com> > AuthorDate: Wed Nov 9 12:37:24 2022 +0100 > Commit: Thierry Reding <thierry.reding@gmail.com> > CommitDate: Mon Jan 30 16:42:45 2023 +0100 > > pwm: sifive: Always let the first pwm_apply_state succeed > > which predates your v3 by quite a bit. > > > I will review the conflicting parts in the pwm-sifive.c code in my v4 > > version once again to ensure there are no omissions. If I find any, I > > will submit v5 accordingly. > > And if this patch is okay in isolation, please reply here explaining > which commit fixed the algorithm, so that I can pick it up. This patch needs to be accompanied by modifications to the pwm_sifive_apply() function to make sense. I will double-check and review the previous discussions to ensure that. Thank you for your response. > > Thanks, > Conor.
Hi Conor, Thank you for patiently giving me advice. I appreciate your help. Not long ago, I said, "This patch needs to be accompanied by modifications to the pwm_sifive_apply() function to make sense." I recently reviewed the v3 version, and after discussing it with Emil, there are several areas that require modification. I will provide the necessary changes for each of them: 1. polarity check. (Suggestion from Uwe) - if (state->polarity != PWM_POLARITY_INVERSED) + if (state->polarity != PWM_POLARITY_NORMAL) 2. avoid using old periodperiod, not state->period - period = max(state->period, ddata->approx_period); - frac = DIV64_U64_ROUND_CLOSEST(num, state->period); + frac = DIV64_U64_ROUND_CLOSEST(num, period); 3. add a conditional check can be added in the code to set ddata->approx_period to state->period when state->period is smaller than ddata->approx_period if (state->period != ddata->approx_period) { ... + if (state->period < ddata->approx_period) { + ddata->approx_period = state->period; + } - ddata->approx_period = state->period; + period = ddata->approx_period; I will use 'unmatched' on my end to verify again. If there are any other errors, feel free to point them out. Thank you. Nylon Chen <nylon.chen@sifive.com> 於 2023年8月4日 週五 上午9:42寫道: > > Hi Conor, > > Conor Dooley <conor.dooley@microchip.com> 於 2023年8月3日 週四 下午5:44寫道: > > > > Hey Nylon, > > > > (I yoinked the reply to 1/1 to here, as it makes more sense in this > > context) > > > > > On Thu, Aug 03, 2023 at 10:15:10AM +0100, Conor Dooley wrote: > > > > On Thu, Aug 03, 2023 at 04:57:33PM +0800, Nylon Chen wrote: > > > > > According to the circuit diagram of User LEDs - RGB described in themanual hifive-unleashed-a00.pdf[0] and hifive-unmatched-schematics-v3.pdf[1]. > > > > > > > > > > The behavior of PWM is acitve-high. > > > > > > > > > > Removed patches: 1 > > > > > New patches: (none) > > > > > > > > > > Links: > > > > > - [0]: https://sifive.cdn.prismic.io/sifive/c52a8e32-05ce-4aaf-95c8-7bf8453f8698_hifive-unleashed-a00-schematics-1.pdf > > > > > - [1]: https://sifive.cdn.prismic.io/sifive/6a06d6c0-6e66-49b5-8e9e-e68ce76f4192_hifive-unmatched-schematics-v3.pdf > > > > > - [2]: https://sifive.cdn.prismic.io/sifive/1a82e600-1f93-4f41-b2d8-86ed8b16acba_fu740-c000-manual-v1p6.pdf > > > > > > > > > > Changed in v4: > > > > > - Remove previous updates to the PWM algorithm. > > > > > > > > Why? I don't recall the conclusion on the previous version being that > > > > that patch was not needed. > > > > > > I apologize for forgetting about this update earlier. Just now, > > > I tried to pull rebase master and noticed that other developers seem > > > to have made some fixes to the algorithm. Upon closer inspection, I > > > found that they addressed the part we previously discussed with Emil > > > and Uwe, such as "first pwm_apply_state." > > > > > > Therefore, my instinct tells me that they have already taken care of > > > the issues we discussed before. > > > > I didn't see anything in linux-next that would solve this problem of > > inversion. The last meaningful change is: > > commit 334c7b13d38321e47d1a51dba0bef9f4c403ec75 > > Author: Emil Renner Berthing <emil.renner.berthing@canonical.com> > > AuthorDate: Wed Nov 9 12:37:24 2022 +0100 > > Commit: Thierry Reding <thierry.reding@gmail.com> > > CommitDate: Mon Jan 30 16:42:45 2023 +0100 > > > > pwm: sifive: Always let the first pwm_apply_state succeed > > > > which predates your v3 by quite a bit. > > > > > I will review the conflicting parts in the pwm-sifive.c code in my v4 > > > version once again to ensure there are no omissions. If I find any, I > > > will submit v5 accordingly. > > > > And if this patch is okay in isolation, please reply here explaining > > which commit fixed the algorithm, so that I can pick it up. > This patch needs to be accompanied by modifications to the > pwm_sifive_apply() function to make sense. > > I will double-check and review the previous discussions to ensure > that. Thank you for your response. > > > > Thanks, > > Conor.
On Fri, Aug 04, 2023 at 02:54:33PM +0800, Nylon Chen wrote: > Hi Conor, > > Thank you for patiently giving me advice. I appreciate your help. > > Not long ago, I said, "This patch needs to be accompanied by > modifications to the pwm_sifive_apply() function to make sense." > > I recently reviewed the v3 version, and after discussing it with Emil, > there are several areas that require modification. I will provide the > necessary changes for each of them: > > 1. polarity check. (Suggestion from Uwe) > - if (state->polarity != PWM_POLARITY_INVERSED) > + if (state->polarity != PWM_POLARITY_NORMAL) > 2. avoid using old periodperiod, not state->period > - period = max(state->period, ddata->approx_period); > - frac = DIV64_U64_ROUND_CLOSEST(num, state->period); > + frac = DIV64_U64_ROUND_CLOSEST(num, period); > 3. add a conditional check can be added in the code to set > ddata->approx_period to state->period when state->period is smaller > than ddata->approx_period > if (state->period != ddata->approx_period) { > ... > + if (state->period < ddata->approx_period) { > + ddata->approx_period = state->period; > + } > - ddata->approx_period = state->period; > + period = ddata->approx_period; > > I will use 'unmatched' on my end to verify again. If there are any > other errors, feel free to point them out. Thank you. I'm not sure of the driver details without going and looking into the code itself, but this sounds like it makes a lot more sense than just flipping the polarity in the dts. Thanks for taking another look!