Re: [PATCH v5 2/2] pwm: add Axiado AX3000 PWM driver

[Petar Stepanovic <pstepanovic@axiado.com>]

On 9/21/2026 4:48 PM, Uwe Kleine-König wrote:
> On Mon, Sep 21, 2026 at 12:21:31AM -0700, Petar Stepanovic wrote:
>> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
>> index 0dc0d2b69025..4466a29e780a 100644
>> --- a/drivers/pwm/Makefile
>> +++ b/drivers/pwm/Makefile
>> @@ -8,6 +8,7 @@ obj-$(CONFIG_PWM_ARGON_FAN_HAT)	+= pwm-argon-fan-hat.o
>>  obj-$(CONFIG_PWM_ATMEL)		+= pwm-atmel.o
>>  obj-$(CONFIG_PWM_ATMEL_HLCDC_PWM)	+= pwm-atmel-hlcdc.o
>>  obj-$(CONFIG_PWM_ATMEL_TCB)	+= pwm-atmel-tcb.o
>> +obj-$(CONFIG_PWM_AXIADO)	+= pwm-axiado.o
>>  obj-$(CONFIG_PWM_AXI_PWMGEN)	+= pwm-axi-pwmgen.o
>>  obj-$(CONFIG_PWM_BCM2835)	+= pwm-bcm2835.o
>>  obj-$(CONFIG_PWM_BCM_IPROC)	+= pwm-bcm-iproc.o
>> diff --git a/drivers/pwm/pwm-axiado.c b/drivers/pwm/pwm-axiado.c
>> new file mode 100644
>> index 000000000000..781053e255f0
>> --- /dev/null
>> +++ b/drivers/pwm/pwm-axiado.c
>> @@ -0,0 +1,277 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * SPDX-FileCopyrightText: 2021-2026 Axiado Corporation.
>> + */
>> +
> Is there a public reference manual for that hardware? If so, adding a
> link here would be awesome.

Unfortunately, there is currently no publicly available reference manual for this hardware.

>
>> +/*
>> + * Limitations:
>> + * - Only normal polarity is supported.
>> + * - The hardware has no shadow registers, so configuration changes take
>> + *   effect immediately without waiting for the current period to complete.
>> + * - The output is driven high while the hardware is disabled.
>> + * - Supported period range: 2 through 0xfffffffe PWM input clock cycles;
>> + *   0xffffffff is reserved by the hardware for a constant low output.
>> + *   Longer periods are clamped to the maximum.
>> + * - A 0% duty cycle is emitted using the constant low encoding because the
>> + *   hardware interprets a zero high time as a constant high output. The
>> + *   requested period is lost in that case and is reported as a single clock
>> + *   cycle.
>> + * - A 100% duty cycle is supported and emits a constant high output.
>> + */
>> [...]
>> +static int
>> +axiado_pwm_round_waveform_tohw(struct pwm_chip *chip,
>> +			       struct pwm_device *pwm,
>> +			       const struct pwm_waveform *wf,
>> +			       void *_wfhw)
>> +{
>> +	struct axiado_pwm_chip *axpwm = pwmchip_get_drvdata(chip);
>> +	struct axiado_pwm_waveform *wfhw = _wfhw;
>> +	u64 period;
>> +	u64 duty;
>> +	int ret = 0;
>> +
>> +	/* Encode a disabled request as a zeroed hardware waveform. */
>> +	if (!wf->period_length_ns) {
>> +		*wfhw = (struct axiado_pwm_waveform) { };
>> +
>> +		return 0;
>> +	}
>> +
>> +	period = mul_u64_u64_div_u64(wf->period_length_ns, axpwm->rate,
>> +				     NSEC_PER_SEC);
>> +	duty = mul_u64_u64_div_u64(wf->duty_length_ns, axpwm->rate,
>> +				   NSEC_PER_SEC);
>> +
>> +	if (!duty) {
>> +		/*
>> +		 * A zero high time makes the hardware emit a constant high
>> +		 * output, so use the constant low period encoding for a 0%
>> +		 * duty cycle. The high time must stay non-zero because it
>> +		 * takes precedence over that encoding. The requested period
>> +		 * is lost, .round_waveform_fromhw() reports a single clock
>> +		 * cycle instead.
>> +		 */
>> +		*wfhw = (struct axiado_pwm_waveform) {
>> +			.period = AXIADO_PWM_PERIOD_CONST_LOW,
>> +			.duty = AXIADO_PWM_DUTY_MIN,
>> +			.enabled = true,
>> +		};
>> +
>> +		/* That single cycle is longer than a sub-cycle request. */
>> +		return period ? 0 : 1;
>> +	}
>> +
>> +	if (period < AXIADO_PWM_PERIOD_MIN) {
>> +		period = AXIADO_PWM_PERIOD_MIN;
>> +		ret = 1;
>> +	} else if (period > AXIADO_PWM_PERIOD_MAX) {
>> +		period = AXIADO_PWM_PERIOD_MAX;
>> +	}
>> +
>> +	/*
>> +	 * Period clamping can leave the converted duty greater than the
>> +	 * final hardware period. In that case, clamp it to 100% duty.
>> +	 */
>> +	if (duty > period)
>> +		duty = period;
> So with period in [AXIADO_PWM_PERIOD_MIN, AXIADO_PWM_PERIOD_MAX], duty =
> period yields a constant high output? If so, why is there a special
> encoding for const high?

Yes. With a normal period value, |duty == period| produces a constant-high
output.

There isn't a separate constant-high encoding used by the driver. The unusual
hardware behavior is that a zero high time also leaves the output high.

To generate a constant-low output, the hardware provides a special encoding:
the period register is set to |0xffffffff| while the high-time register remains
non-zero. The driver uses that encoding for a 0% duty cycle because |duty == 0|
would otherwise leave the output high.

>
>> +	*wfhw = (struct axiado_pwm_waveform) {
>> +		.period = period,
>> +		.duty = duty,
>> +		.enabled = true,
>> +	};
>> +
>> +	return ret;
>> +}
>> +
>> +static int
>> +axiado_pwm_round_waveform_fromhw(struct pwm_chip *chip,
>> +				 struct pwm_device *pwm,
>> +				 const void *_wfhw,
>> +				 struct pwm_waveform *wf)
>> +{
>> +	struct axiado_pwm_chip *axpwm = pwmchip_get_drvdata(chip);
>> +	const struct axiado_pwm_waveform *wfhw = _wfhw;
>> +	u32 period = wfhw->period;
>> +	u32 duty = wfhw->duty;
>> +
>> +	if (!wfhw->enabled) {
>> +		*wf = (struct pwm_waveform) {
>> +			.period_length_ns = 0,
>> +		};
>> +
>> +		return 0;
>> +	}
>> +
>> +	/*
>> +	 * The constant low encoding doesn't hold a period. Report the
>> +	 * shortest one, the output is low for all of it either way.
>> +	 */
>> +	if (period == AXIADO_PWM_PERIOD_CONST_LOW) {
>> +		period = 1;
>> +		duty = 0;
>> +	} else if (duty > period) {
>> +		duty = period;
>> +	}
> You don't handle wfhw->duty == 0 here. I think this is what Sashiko
> pointed out. While .round_waveform_tohw() doens't produce this setting,
> it would still be good to handle that in case the driver is loaded with
> this setting applied in hardware.

Yes, you're right. A zero high-time value results in a constant-high output, so
|.round_waveform_fromhw()|should handle that case as well in case the hardware
was configured before the driver was loaded.

I'll update it to interpret |duty == 0|as a 100% duty cycle. I'll also check
this before the constant-low period encoding, since a zero high time takes
precedence over that encoding.

>> +	*wf = (struct pwm_waveform) {
>> +		.period_length_ns =
>> +			mul_u64_u64_div_u64_roundup(period, NSEC_PER_SEC,
>> +						    axpwm->rate),
>> +		.duty_length_ns =
>> +			mul_u64_u64_div_u64_roundup(duty, NSEC_PER_SEC,
>> +						    axpwm->rate),
>> +		.duty_offset_ns = 0,
>> +	};
>> +
>> +	return 0;
>> +}

Best regards,
Petar