Message-ID: <aBseFo-EC-PzxiqM@ishi>
Date: Wed, 7 May 2025 17:47:18 +0900
From: William Breathitt Gray <wbg@...nel.org>
To: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
Cc: Linus Walleij <linus.walleij@...aro.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Heiko Stuebner <heiko@...ech.de>,
	Uwe Kleine-König <ukleinek@...nel.org>,
	Sebastian Reichel <sebastian.reichel@...labora.com>,
	Kever Yang <kever.yang@...k-chips.com>, linux-gpio@...r.kernel.org,
	devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org,
	linux-pwm@...r.kernel.org, linux-iio@...r.kernel.org,
	kernel@...labora.com, Jonas Karlman <jonas@...boo.se>,
	Detlev Casanova <detlev.casanova@...labora.com>
Subject: Re: [PATCH 6/7] counter: Add rockchip-pwm-capture driver

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@...labora.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

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

Powered by blists - more mailing lists