Message-ID: <20250720002024.696040-1-wbg@kernel.org>
Date: Sun, 20 Jul 2025 09:20:15 +0900
From: William Breathitt Gray <wbg@...nel.org>
To: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
Cc: William Breathitt Gray <wbg@...nel.org>,
	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>,
	Yury Norov <yury.norov@...il.com>,
	Rasmus Villemoes <linux@...musvillemoes.dk>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Dave Ertman <david.m.ertman@...el.com>,
	Ira Weiny <ira.weiny@...el.com>,
	Leon Romanovsky <leon@...nel.org>,
	Lee Jones <lee@...nel.org>,
	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 v2 6/7] counter: Add rockchip-pwm-capture driver

On Mon, Jun 02, 2025 at 06:19:17PM +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. Because there isn't a way to reset the internal
> double-buffered cycle count in the hardware, we filter out unreliable
> periods above the timeout value in the counter read callback.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>

Hi Nicolas,

Would you help me understand the computations in this driver?

If I understand the purpose of this driver correctly, it's meant to
compute the period and duty cycle of a PWM signal. What do LPC and HPC
represent? I'm guessing they are the low period count (LPC) and the high
period count (HPC). So then you calculate the total period by adding
LPC and HPC, whereas the duty cycle derives from HPC.

Am I understanding the algorithm correctly? What are the units of HPC
and LPC; are they ticks from the core clock? Are PWMV4_INT_LPC and
PWM4_INT_HPC the change-of-state interrupts for LPC and HPC
respectively?

The Counter subsystem can be used to derive the period and duty cycle of
a signal, but I believe there's a more idiomatic way to implement this.
Existing counter drivers such as microchip-tcb-capture achieve this by
leveraging Counter events exposed via the Counter chrdev interface.

The basic idea would be:
    * Expose LPC and HPC as count0 and count1;
    * Push the PWMV4_INT_LPC and PWMV4_INT_HPC interrupts as
      COUNTER_EVENT_CHANGE_OF_STATE events on channel 0 and channel 1
      respectively;
    * Register Counter watches in userspace to capture LPC and HPC on
      each interrupt;

The Counter chrdev interface records a timestamp in nanoseconds with
each event capture. So to compute period and duty cycle, you would
subtract the difference between two HPC/LPC captures; the difference in
the timestamps gives you the elapsed time between the two captures in
nanoseconds.

Would that design work for your use case?

William Breathitt Gray

Powered by blists - more mailing lists