linux-kernel - Re: [PATCH v7] pwm: Add support for pwmchip devices for faster and easier userspace access

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <147fa354-e1fe-46ec-9c47-59794f37bfb7@baylibre.com>
Date: Wed, 16 Apr 2025 11:29:50 -0500
From: David Lechner <dlechner@...libre.com>
To: Uwe Kleine-König <u.kleine-koenig@...libre.com>,
 linux-pwm@...r.kernel.org
Cc: Kent Gibson <warthog618@...il.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7] pwm: Add support for pwmchip devices for faster and
 easier userspace access

On 4/16/25 4:43 AM, Uwe Kleine-König wrote:
> With this change each pwmchip defining the new-style waveform callbacks
> can be accessed from userspace via a character device. Compared to the
> sysfs-API this is faster and allows to pass the whole configuration in a
> single ioctl allowing atomic application and thus reducing glitches.

Didn't do a full review yet, but I have some confusion on when 1 should actually
be returned from ioctl calls...

> +/*
> + * Modifies the passed wf according to hardware constraints. All parameters are
> + * rounded down to the next possible value, unless there is no such value, then
> + * values are rounded up. Note that zero isn't considered for rounding down
> + * period_length_ns.
> + */
> +#define PWM_IOCTL_ROUNDWF	_IOWR(0x75, 3, struct pwmchip_waveform)

Should this return 1 if exact could not be met to match the other functions?

> +
> +/* Get the currently implemented waveform */
> +#define PWM_IOCTL_GETWF		_IOWR(0x75, 4, struct pwmchip_waveform)
> +
> +/* Like PWM_IOCTL_ROUNDWF + PWM_IOCTL_SETEXACTWF in one go. */
> +#define PWM_IOCTL_SETROUNDEDWF	_IOW(0x75, 5, struct pwmchip_waveform)
> +
> +/*
> + * Program the PWM to emit exactly the passed waveform, subject only to rounding
> + * down each value less than 1 ns. Returns 0 on success, 1 if the waveform
> + * cannot be implemented exactly, or other negative error codes.

It doesn't make sense to me that PWM_IOCTL_SETEXACTWF could return 1 meaning
that the exact request could not be met. Isn't that the point of
PWM_IOCTL_SETEXACTWF? To either do exactly as requested (with 1 ns precision)
or return negative error code without changing the output state?

It seems like only PWM_IOCTL_SETROUNDEDWF should be able to return 1. My natural
expectation is that negative error would mean that the hardware output was not
modified and non-negative value means that hardware output was modified.

> + */
> +#define PWM_IOCTL_SETEXACTWF	_IOW(0x75, 6, struct pwmchip_waveform)
> +
> +#endif /* _UAPI_PWM_H_ */
> 
> base-commit: bde5547f2e87e6c71db79dc41e56aff3061e39a9