On Mon, Jul 20, 2015 at 12:01:16PM +0200, Boris Brezillon wrote: > On Mon, 20 Jul 2015 10:04:59 +0200 > Thierry Reding wrote: > > > On Wed, Jul 01, 2015 at 10:21:52AM +0200, Boris Brezillon wrote: > > [...] > > > diff --git a/include/linux/pwm.h b/include/linux/pwm.h > > [...] > > > +struct pwm_state { > > > + unsigned int period; /* in nanoseconds */ > > > + unsigned int duty_cycle; /* in nanoseconds */ > > > + enum pwm_polarity polarity; > > > +}; > > > > No need for the extra padding here. > > What do you mean by "extra padding" ? > I just reused the indentation used in the pwm_device struct. Yeah, I have a local patch to fix that up. I find it useless to pad things like this, and it has the downside that it will become totally inconsistent (or cause a lot of churn by reformatting) if ever you add a field that extends beyond the padding. Single spaces don't have any such drawbacks and, in my opinion, look just as good. > Would you prefer something like that ? > > struct pwm_state { > unsigned int period; /* in nanoseconds */ > unsigned int duty_cycle; /* in nanoseconds */ > enum pwm_polarity polarity; > }; Yeah. I'd say even the comments would be more suited in a kerneldoc- style comment: /** * struct pwm_state - state of a PWM channel * @period: PWM period (in nanoseconds) * @duty_cycle: PWM duty cycle (in nanoseconds) * @polarity: PWM polarity */ struct pwm_state { unsigned int period; unsigned int duty_cycle; enum pwm_polarity polarity; }; This is something that users will need to deal with, so eventually somebody might look at this via some DocBook generated HTML or PDF. Thierry -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: