Re: [PATCH v3 5/8] pwm: Add support for pwmchip devices for faster and easier userspace access

[Kent Gibson <warthog618@gmail.com>]

On Wed, Aug 07, 2024 at 08:04:22AM +0200, Uwe Kleine-König wrote:
> Hello Kent,
>
> On Wed, Aug 07, 2024 at 10:34:33AM +0800, Kent Gibson wrote:
> > On Mon, Jul 29, 2024 at 04:34:21PM +0200, Uwe Kleine-König wrote:
> > > diff --git a/include/uapi/linux/pwm.h b/include/uapi/linux/pwm.h
> > > new file mode 100644
> > > index 000000000000..c89ba3e3def8
> > > --- /dev/null
> > > +++ b/include/uapi/linux/pwm.h
> > > @@ -0,0 +1,25 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */
> > > +
> > > +#ifndef _UAPI_PWM_H_
> > > +#define _UAPI_PWM_H_
> > > +
> > > +#include <linux/ioctl.h>
> > > +#include <linux/types.h>
> > > +
> > > +struct pwmchip_waveform {
> > > +	unsigned int hwpwm;
> > > +	unsigned int __pad; /* padding, must be zero */
> > > +	__u64 period_length_ns;
> > > +	__u64 duty_length_ns;
> > > +	__u64 duty_offset_ns;
> > > +};
> > > +
> >
> > I would go with __u32, rather than unsigned int, to be absolutely clear
> > on sizing.
>
> Hmm, the upside of using unsigned int is that it matches struct
> pwm_device::hwpwm. To the best of my knowledge all Linux platforms have
> sizeof(int) == 4, so a change would have no effect on binary
> representation, but only help the human reader (which is good).
>

The uapi headers define ABI, a machine interface, so I would give
priority to precision over human readibility.
The size of __u32 is guaranteed, while unsigned int is not.

FWIW, __u32 is used more frequently in include/uapi/linux/*.h than unsigned int:

$ grep "unsigned int" include/uapi/linux/*.h | wc -l
441
$ grep __u32 include/uapi/linux/*.h | wc -l
4234

> I don't know yet, but I will consider the suggestion.
>

No problem - just a thought.
Sorry for not reviewing earlier.

Cheers,
Kent.