linux-kernel - Re: [PATCH v23] pwm: airoha: Add support for EN7581 SoC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <xsblhw36y3corxx3pxe6223auirrsqr3efovfnrm5lbo4xy3lf@wf3ytlivzv6g>
Date: Fri, 1 Aug 2025 11:15:41 +0200
From: Uwe Kleine-König <ukleinek@...nel.org>
To: Christian Marangi <ansuelsmth@...il.com>
Cc: linux-kernel@...r.kernel.org, linux-pwm@...r.kernel.org, 
	Benjamin Larsson <benjamin.larsson@...exis.eu>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, Lorenzo Bianconi <lorenzo@...nel.org>, 
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Subject: Re: [PATCH v23] pwm: airoha: Add support for EN7581 SoC

Hello Christian,

On Tue, Jul 08, 2025 at 04:50:52PM +0200, Christian Marangi wrote:
> +static u32 airoha_pwm_get_period_ticks_from_ns(u32 period_ns)
> +{
> +	return period_ns / AIROHA_PWM_PERIOD_TICK_NS;
> +}
> +
> +static u32 airoha_pwm_get_duty_ticks_from_ns(u32 period_ns, u32 duty_ns)
> +{
> +	return mul_u64_u32_div(duty_ns, AIROHA_PWM_DUTY_FULL, period_ns);
> +}
> +
> [...]
> +static int airoha_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> +			    const struct pwm_state *state)
> +{
> [...]
> +	/*
> +	 * Period goes at 4ns step, normalize it to check if we can

4 ms or 4 ns?

> +	 * share a generator.
> +	 */
> +	period_ns = rounddown(period_ns, AIROHA_PWM_PERIOD_TICK_NS);
> +
> +	/* Convert ns to ticks */
> +	period_ticks = airoha_pwm_get_period_ticks_from_ns(period_ns);

Rounding down to the next multiple of 4ns isn't needed for
airoha_pwm_get_period_ticks_from_ns() which is just a division by
AIROHA_PWM_PERIOD_TICK_NS.

> +	duty_ticks = airoha_pwm_get_duty_ticks_from_ns(period_ns, duty_ns);

As duty_ticks depends on the selected period_ticks, I think the bucket
selection algorithm is still wrong.

Consider a request to implement

	period_ns = 256 ms
	duty_ns = 128 ms

which at first correctly results in

	period_ticks = 64
	duty_ticks = 127

If however all buckets are used and we only find one with say 62 period
ticks we get period_ns = 248 and with that duty_ticks should better be
131 and not 127.

> +
> +	return airoha_pwm_config(pc, pwm, period_ticks, duty_ticks);
> +}

Best regards
Uwe

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)