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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pwm: axiado: misinterprets constant-high hardware state as 0% duty on readback
--

commit 0be6bd48e49f4eaed6639766408acd48c1985d0f
Author: Petar Stepanovic <pstepanovic@axiado.com>

pwm: add Axiado AX3000 PWM driver

The Axiado AX3000 and AX3005 SoCs include a single-channel PWM
controller. Add a driver supporting period, duty-cycle, and enable-state
configuration through the Linux PWM framework.

> diff --git a/drivers/pwm/pwm-axiado.c b/drivers/pwm/pwm-axiado.c
> new file mode 100644
> index 0000000000000..781053e255f03
> --- /dev/null
> +++ b/drivers/pwm/pwm-axiado.c
[ ... ]
> +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;
> +	}

[Severity: High]
Does this logic misinterpret a constant-high hardware state as a 0% duty
cycle on readback?

The driver's comments indicate that a zero high time is interpreted by the
hardware as a constant high output (100% duty cycle). However, if the
hardware is already programmed with a zero high time (e.g., by the
bootloader), axiado_pwm_read_waveform() will read duty = 0.

When this state is processed here in axiado_pwm_round_waveform_fromhw(),
it fails to map duty = 0 to duty = period, returning a duty_length_ns
of 0 instead of 100% duty as documented by the hardware behavior.

This could cause an abrupt output flip from 100% duty (constant high) to
0% duty (constant low) when a consumer driver probes and inherits the
initial PWM state. If the PWM controls a voltage regulator, this drops the
voltage to 0, which could cause a system crash. For a backlight, the
screen could turn black.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921-axiado-ax3000-pwm-v5-0-202aad28e129@axiado.com?part=2