Re: [PATCH v3 4/5] counter: Add rockchip-pwm-capture driver

Damon Ding
2025-10-28T11:06:03Z

Hi Nicolas,

On 10/28/2025 1:11 AM, 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 expose HPC/LPC counts and
> state change events to userspace through the counter framework. 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.
>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---

......

> +
> +/*
> + * Channel 0 receives a state changed notification whenever the LPC interrupt
> + * fires.
> + *
> + * Channel 1 receives a state changed notification whenever the HPC interrupt
> + * fires.
> + */
> +static struct counter_signal rkpwmc_signals[] = {
> + {
> + .id = 0,
> + .name = "Channel 0"
> + },
> + {
> + .id = 1,
> + .name = "Channel 1"
> + },
> +};
> +
> +static const enum counter_synapse_action rkpwmc_hpc_lpc_actions[] = {
> + COUNTER_SYNAPSE_ACTION_BOTH_EDGES,
> +

Redundant blank line. ;-)

If the dclk cycle is regarded as the 'input signal', then the HPC/LPC
counters should be COUNTER_SYNAPSE_ACTION_BOTH_EDGES.

If to deal with the interrupt for HPC or LPC, the LPC interrupt occurs
in several pclk cycles after the rising edge while the HPC interrupt is
generated similarly after the falling edge. Shall we distinguish the
synapses between LPC and HPC? LPC is COUNTER_SYNAPSE_ACTION_RISING_EDGE
and HPC is COUNTER_SYNAPSE_ACTION_FALLING_EDGE.

> +};
> +
> +static struct counter_synapse rkpwmc_pwm_synapses[] = {
> + {
> + .actions_list = rkpwmc_hpc_lpc_actions,
> + .num_actions = ARRAY_SIZE(rkpwmc_hpc_lpc_actions),
> + .signal = &rkpwmc_signals[0]
> + },
> + {
> + .actions_list = rkpwmc_hpc_lpc_actions,
> + .num_actions = ARRAY_SIZE(rkpwmc_hpc_lpc_actions),
> + .signal = &rkpwmc_signals[1]
> + },
> +};
> +
> +static const enum counter_function rkpwmc_functions[] = {
> + COUNTER_FUNCTION_INCREASE,
> +};
> +

......

> +
> +static struct counter_comp rkpwmc_ext[] = {
> + COUNTER_COMP_ENABLE(rkpwmc_enable_read, rkpwmc_enable_write),
> +};
> +
> +enum rkpwmc_count_id {
> + COUNT_LPC = 0,
> + COUNT_HPC = 1,
> +};
> +
> +static struct counter_count rkpwmc_counts[] = {
> + {
> + .id = COUNT_LPC,
> + .name = "PWM core clock cycles during which the signal was low",
> + .functions_list = rkpwmc_functions,
> + .num_functions = ARRAY_SIZE(rkpwmc_functions),
> + .synapses = rkpwmc_pwm_synapses,
> + .num_synapses = ARRAY_SIZE(rkpwmc_pwm_synapses),
> + .ext = rkpwmc_ext,
> + .num_ext = ARRAY_SIZE(rkpwmc_ext),
> + },
> + {
> + .id = COUNT_HPC,
> + .name = "PWM core clock cycles during which the signal was high",
> + .functions_list = rkpwmc_functions,
> + .num_functions = ARRAY_SIZE(rkpwmc_functions),
> + .synapses = rkpwmc_pwm_synapses,
> + .num_synapses = ARRAY_SIZE(rkpwmc_pwm_synapses),
> + .ext = rkpwmc_ext,
> + .num_ext = ARRAY_SIZE(rkpwmc_ext),
> + },
> +};
> +
>

Additionally, I test the capture by connecting pwm0_ch0 and pwm2_ch0,
and the capture results are correct.

I think the counter/frequency meter/biphasic counter functions can also
be adapted to the counter framework. And they will be available soon in
the future. :-)

Best regards,
Damon