On Mon, 20 Jul 2015 12:09:26 +0200
Thierry Reding <thierry.reding@xxxxxxxxx> 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 <thierry.reding@xxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
