Re: [PATCH 6/7] counter: Add rockchip-pwm-capture driver

[William Breathitt Gray <wbg@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2592 bytes --]

On Tue, Apr 08, 2025 at 02:32:18PM +0200, Nicolas Frattaroli wrote:
> Among many other things, Rockchip's new PWMv4 IP in the RK3576 supports
> PWM capture functionality.
> 
> Add a basic driver for this that works to capture period and duty cycle
> values and return them as nanoseconds to the user. It's quite basic, but
> works well enough to demonstrate the device function exclusion stuff
> that mfpwm does, in order to eventually support all the functions of
> this device in drivers within their appropriate subsystems, without them
> interfering with each other.
> 
> Once enabled, the counter driver waits for enough high-to-low and
> low-to-high interrupt signals to arrive, and then writes the cycle count
> register values into some atomic members of the driver instance's state
> struct. The read callback can then do the conversion from cycle count to
> the more useful period and duty cycle nanosecond values, which require
> knowledge of the clock rate, which requires a call that the interrupt
> handler cannot make itself because said call may sleep.
> 
> To detect the condition of a PWM signal disappearing, i.e. turning off,
> we modify the delay value of a delayed worker whose job it is to simply
> set those atomic members to zero. Should the "timeout" so to speak be
> reached, we assume the PWM signal is off. This isn't perfect; it
> obviously introduces a latency between it being off and the counter
> reporting it as such. Additionally, periods longer than the timeout
> value will cause the count to "flicker" between the correct period and
> duty cycle values, and zero. This is because there doesn't appear to be
> a way to reset the hardware's internal counters, even when writing to
> the registers.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

Hi Nicolas,

I just want to give you a heads-up that I'm looking over this but it's
going to take me a couple more weeks or so; this hardware is a little
weird so I want to properly grok it before I Ack such a driver. In
particular, I'm not sure yet that the counter subsystem is necessarily
the right place for this functionality if you're ultimately after values
in units of time (sounds more like a clk framework feature) -- but we'll
determine so together.

That being said, please continue on to a version 2 of this patchset if
you have other changes ready -- I don't want the counter driver
bottlenecking the development of the rest of this series when progress
can be made independent of it.

Thanks,

William Breathitt Gray

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]