From: Baruch Siach <baruch@tkos.co.il>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Thierry Reding <thierry.reding@gmail.com>,
Lee Jones <lee.jones@linaro.org>, Andy Gross <agross@kernel.org>,
Bjorn Andersson <bjorn.andersson@linaro.org>,
Balaji Prakash J <bjagadee@codeaurora.org>,
Rob Herring <robh+dt@kernel.org>,
Robert Marko <robert.marko@sartura.hr>,
Kathiravan T <kathirav@codeaurora.org>,
linux-pwm@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-msm@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, kernel@pengutronix.de
Subject: Re: [PATCH v8 2/4] pwm: driver for qualcomm ipq6018 pwm block
Date: Tue, 14 Dec 2021 18:05:08 +0200 [thread overview]
Message-ID: <87pmpzmaf1.fsf@tarshish> (raw)
In-Reply-To: <20210914124959.spwjiifvysposhls@pengutronix.de>
Hi Uwe,
On Tue, Sep 14 2021, Uwe Kleine-König wrote:
> On Mon, Aug 30, 2021 at 02:46:25PM +0300, Baruch Siach wrote:
>> + for (; pre_div <= IPQ_PWM_MAX_DIV; pre_div++) {
>> + pwm_div = DIV64_U64_ROUND_UP(period_ns * rate,
>> + (u64)NSEC_PER_SEC * (pre_div + 1));
>> + pwm_div--;
>
> Can it happen that pwm_div is zero before it is decreased by one? Also
> you need to round down here; with rounding up the resulting period is
> bigger than the requested period (unless the division yields an exact
> integer).
I followed your round down advice on v9, but it turned out to be
wrong. Round down means that the divider is smaller so the period is
larger. This means that 'diff' below can not be positive. So only exact
match (diff == 0) works. When there is no exact match, best_* values are
left in their initial setting.
I'll fix that in v10 along with another bug I introduced in v9.
baruch
>> + if (pre_div > pwm_div)
>> + break;
>
> A comment here why we can end the search would be good.
>
>> + /*
>> + * Make sure we can do 100% duty cycle where
>> + * hi_dur == pwm_div + 1
>> + */
>> + if (pwm_div > IPQ_PWM_MAX_DIV - 1)
>> + continue;
>> +
>> + diff = ((uint64_t)freq * (pre_div + 1) * (pwm_div + 1))
>> + - (uint64_t)rate;
>> +
>> + if (diff < 0) /* period larger than requested */
>> + continue;
>> + if (diff == 0) { /* bingo */
>> + best_pre_div = pre_div;
>> + best_pwm_div = pwm_div;
>> + break;
>> + }
>> + if (diff < min_diff) {
>> + min_diff = diff;
>> + best_pre_div = pre_div;
>> + best_pwm_div = pwm_div;
>> + }
>> + }
>> +
>> + /* config divider values for the closest possible frequency */
>> + config_div_and_duty(pwm, best_pre_div, best_pwm_div,
>> + rate, duty_ns, state->enabled);
>> +
>> + return 0;
>> +}
--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
next prev parent reply other threads:[~2021-12-14 16:20 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-30 11:46 [PATCH v8 1/4] dt-bindings: mfd: qcom,tcsr: document ipq6018 compatible Baruch Siach
2021-08-30 11:46 ` [PATCH v8 2/4] pwm: driver for qualcomm ipq6018 pwm block Baruch Siach
2021-09-14 12:49 ` Uwe Kleine-König
2021-12-14 16:05 ` Baruch Siach [this message]
2021-08-30 11:46 ` [PATCH v8 3/4] dt-bindings: pwm: add IPQ6018 binding Baruch Siach
2021-08-31 19:02 ` Rob Herring
2021-08-30 11:46 ` [PATCH v8 4/4] arm64: dts: ipq6018: add pwm node Baruch Siach
2021-09-22 14:10 ` [PATCH v8 1/4] dt-bindings: mfd: qcom,tcsr: document ipq6018 compatible Lee Jones
2021-11-04 9:27 ` Uwe Kleine-König
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87pmpzmaf1.fsf@tarshish \
--to=baruch@tkos.co.il \
--cc=agross@kernel.org \
--cc=bjagadee@codeaurora.org \
--cc=bjorn.andersson@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=kathirav@codeaurora.org \
--cc=kernel@pengutronix.de \
--cc=lee.jones@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-pwm@vger.kernel.org \
--cc=robert.marko@sartura.hr \
--cc=robh+dt@kernel.org \
--cc=thierry.reding@gmail.com \
--cc=u.kleine-koenig@pengutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox