public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed
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 -

  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