Re: [PATCH v4 05/24] misc: max77693-haptic: use pwm_get_xxx() helpers where appropriate

[Guenter Roeck]

On 11/16/2015 05:55 AM, Boris Brezillon wrote:
> Hi Krzysztof,
>
> On Mon, 16 Nov 2015 22:10:40 +0900
> Krzysztof Kozlowski <k.kozlowski@xxxxxxxxxxx> wrote:
>
> > W dniu 16.11.2015 o 17:56, Boris Brezillon pisze:
> > > Use pwm_get_xxx() helpers instead of directly accessing the pwm->xxx field.
> > > Doing that will ease adaptation of the PWM framework to support atomic
> > > update.
> > >
> > > Signed-off-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx>
> > > ---
> > > Patch generated with the following coccinelle script:
> > >
> > > --->8---
> > > virtual patch
> > >
> > > @@
> > > struct pwm_device *p;
> > > expression e;
> > > @@
> > > (
> > > -(p)->polarity = e;
> > > +pwm_set_polarity((p), e);
> > > |
> > > -(p)->polarity
> > > +pwm_get_polarity((p))
> > > |
> > > -(p)->period = e;
> > > +pwm_set_period((p), e);
> > > |
> > > -(p)->period
> > > +pwm_get_period((p))
> > > |
> > > -(p)->duty_cycle = e;
> > > +pwm_set_duty_cycle((p), e);
> > > |
> > > -(p)->duty_cycle
> > > +pwm_get_duty_cycle((p))
> > > )
> > > --->8---
> > > ---
> > >   drivers/input/misc/max77693-haptic.c | 7 ++++---
> > >   1 file changed, 4 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/input/misc/max77693-haptic.c b/drivers/input/misc/max77693-haptic.c
> > > index 6d96bff..a038fb3 100644
> > > --- a/drivers/input/misc/max77693-haptic.c
> > > +++ b/drivers/input/misc/max77693-haptic.c
> > > @@ -70,10 +70,11 @@ struct max77693_haptic {
> > >
> > >   static int max77693_haptic_set_duty_cycle(struct max77693_haptic *haptic)
> > >   {
> > > -	int delta = (haptic->pwm_dev->period + haptic->pwm_duty) / 2;
> > > +	int delta = (pwm_get_period((haptic->pwm_dev)) + haptic->pwm_duty) / 2;
> >
> > Double parentheses over argument are not needed so just:
> > pwm_get_period(haptic->pwm_dev) + ...
>
> Actually it was generated with coccinelle, hence I didn't fix existing
> coding style issues, but I have no problem fixing them.
There was no existing coding style issue. Your coccinelle script introduces it.
You might want to consider updating your script and remove the unnecessary (( ))
from it.

Guenter

> Thanks,
>
> Boris
>
> > Beside that patch looks good, so with removing parentheses here and below:
> >
> > Reviewed-by: Krzysztof Kozlowski <k.kozlowski@xxxxxxxxxxx>
> >
> > Best regards,
> > Krzysztof
> >
> > >   	int error;
> > >
> > > -	error = pwm_config(haptic->pwm_dev, delta, haptic->pwm_dev->period);
> > > +	error = pwm_config(haptic->pwm_dev, delta,
> > > +			   pwm_get_period((haptic->pwm_dev)));
> > >   	if (error) {
> > >   		dev_err(haptic->dev, "failed to configure pwm: %d\n", error);
> > >   		return error;
> > > @@ -245,7 +246,7 @@ static int max77693_haptic_play_effect(struct input_dev *dev, void *data,
> > >   	 * The formula to convert magnitude to pwm_duty as follows:
> > >   	 * - pwm_duty = (magnitude * pwm_period) / MAX_MAGNITUDE(0xFFFF)
> > >   	 */
> > > -	period_mag_multi = (u64)haptic->pwm_dev->period * haptic->magnitude;
> > > +	period_mag_multi = (u64)pwm_get_period((haptic->pwm_dev)) * haptic->magnitude;
> > >   	haptic->pwm_duty = (unsigned int)(period_mag_multi >>
> > >   						MAX_MAGNITUDE_SHIFT);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/