Re: [PATCH v4 1/1] pwm: pca9685: fix gpio-only operation.

From: Sven Van Asbroeck
Date: Tue Apr 18 2017 - 11:53:34 EST


Thanks for the feedback Andy !!

> I would go with
>
> /* Wait for @sleep microseconds for the oscillator to be back up */
> if (sleep)
> udelay(sleep);
>
> Otherwise int sleep is oddly here.
>
> Or
>
> bool sleep
>
> /* Wait 500us ... */
> if (sleep)
> udelay(500);
>
>> +}

I think you may be getting confused between:
- the chip's SLEEP bit (int sleep)
- the amount of time to delay after chip comes _out of_ sleep.
(always 500 us)

If it's confusing for you, it might be confusing for others?
Perhaps change the parameter to 'bool sleep_bit' or 'bool do_sleep'
to make the distinction clearer?

> __maybe_unused and remove ugly #ifdef:ery.

If this works on non- CONFIG_PM systems, I'm all for it !
Grepping the drivers/ directory, I see that some drivers use
#ifdef CONFIG_PM, some use __maybe_unused for runtime_pm.

Mika and Thierry, thoughts ?