linux-kernel - Re: [PATCH v2 2/7] pwm: Add Rust driver for T-HEAD TH1520 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <d1523586-82ca-4863-964f-331718bb1f0e@samsung.com>
Date: Thu, 12 Jun 2025 10:14:13 +0200
From: Michal Wilczynski <m.wilczynski@...sung.com>
To: Uwe Kleine-König <ukleinek@...nel.org>
Cc: Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>,
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, Andreas
	Hindborg <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>, Trevor
	Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>, Drew Fustini
	<drew@...7.com>, Guo Ren <guoren@...nel.org>, Fu Wei <wefu@...hat.com>, Rob
	Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor
	Dooley <conor+dt@...nel.org>, Paul Walmsley <paul.walmsley@...ive.com>,
	Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
	Alexandre Ghiti <alex@...ti.fr>, Marek Szyprowski
	<m.szyprowski@...sung.com>, Benno Lossin <lossin@...nel.org>, Michael
	Turquette <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>,
	linux-kernel@...r.kernel.org, linux-pwm@...r.kernel.org,
	rust-for-linux@...r.kernel.org, linux-riscv@...ts.infradead.org,
	devicetree@...r.kernel.org, linux-clk@...r.kernel.org
Subject: Re: [PATCH v2 2/7] pwm: Add Rust driver for T-HEAD TH1520 SoC



On 6/11/25 08:58, Uwe Kleine-König wrote:
> Hello,
> 
> Huh, if you do the newstyle stuff, .get_state() is wrong. It's either
> .round_waveform_tohw() + .round_waveform_fromhw() + .read_waveform() +
> .write_waveform() or .apply() + .get_state(), but don't mix these.

In the process of implementing the full "newstyle" waveform API as you
suggested, I discovered a hardware limitation. After writing new values
to the period and duty cycle registers, reading them back does not
return the programmed values, which makes it impossible to reliably
report the current hardware state.

This appears to be a known quirk of the hardware, as the reference C
driver from T-HEAD [1] also omits the .get_state callback, likely for
the same reason.

Given this, would it be acceptable to provide a write-only driver? My
proposed solution would be to omit the .read_waveform() and
.round_waveform_fromhw() implementations from my PwmOps trait. This
would mean the driver can correctly set the PWM state, but attempting to
read it back via sysfs would fail (e.g., with -EOPNOTSUPP), reflecting
the hardware's capability.

Does this sound like a reasonable approach to you?

[1] - https://github.com/revyos/thead-kernel/blob/lpi4a/drivers/pwm/pwm-light.c

> 
>> +        let data: &Self = chip.drvdata().ok_or(EINVAL)?;
>> +        let hwpwm = pwm.hwpwm();
>> +        let iomem_guard = data.iomem.access(parent_dev)?;
>> +        let iomap = iomem_guard.deref();
>> +        let ctrl = iomap.read32(th1520_pwm_ctrl(hwpwm));
>> +        let period_cycles = iomap.read32(th1520_pwm_per(hwpwm));
>> +        let duty_cycles = iomap.read32(th1520_pwm_fp(hwpwm));
>> +
>> +        state.set_enabled(duty_cycles != 0);
>> +
>> +        let rate_hz = data.clk.rate().as_hz();
>> +        let period_ns = (period_cycles as u64)
>> +            .mul_div(time::NSEC_PER_SEC as u64, rate_hz as u64)
>> +            .unwrap_or(0);

> Best regards
> Uwe

Best regards,
-- 
Michal Wilczynski <m.wilczynski@...sung.com>