Re: [PATCH v2 01/13] pwm: Add new helpers to create/manipulate PWM states

[Thierry Reding]

On Fri, Jun 10, 2016 at 06:29:42PM +0200, Boris Brezillon wrote:
> On Fri, 10 Jun 2016 17:21:09 +0200, Thierry Reding <thierry.reding@xxxxxxxxx> wrote:
> > On Wed, Jun 08, 2016 at 06:34:36PM +0200, Boris Brezillon wrote:
[...]
> > > + * according to your need before calling pwm_apply_state().  
> > 
> > Maybe mention that the ->duty_cycle field is explicitly zeroed. Then
> > again, do we really need it? If users are going to overwrite it anyway,
> > do we even need to bother? I suppose it makes some sense because the
> > current duty cycle is stale when the ->period gets set to the value from
> > args. I think the documentation should mention this in some way.
> 
> Yes, if we keep the current duty_cycle it can exceed the period value.
> I'm fine dropping the ->duty_cycle = 0 assignment and documenting the
> behavior.

Actually what I was trying to suggest is that we keep the code as-is but
document the behaviour (and rationale behind it).

I think it's fine to zero out the value precisely because it could
become invalid after the function (and there's no other reasonable value
to set it to). Just wanted to make sure it's all properly documented.

Thierry

Attachment: signature.asc
Description: PGP signature