On Mon, 20 Jul 2015 12:09:26 +0200 Thierry Reding <[email protected]> wrote:
> On Mon, Jul 20, 2015 at 12:01:16PM +0200, Boris Brezillon wrote: > > On Mon, 20 Jul 2015 10:04:59 +0200 > > Thierry Reding <[email protected]> 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. I prefer the single space approach too, so I won't complain ;-). > > > 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. I agree. -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
