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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGngYiWj1jBsgYAL31Qb2Ne+Te34vpjWD14xbV+u9vRS0i8kkQ@mail.gmail.com>
Date:   Tue, 18 Apr 2017 11:52:49 -0400
From:   Sven Van Asbroeck <thesven73@...il.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     Thierry Reding <thierry.reding@...il.com>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>, linux-pwm@...r.kernel.org,
        linux-kernel@...r.kernel.org, clemens.gruber@...ruber.com,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Sven Van Asbroeck <TheSven73@...glemail.com>
Subject: Re: [PATCH v4 1/1] pwm: pca9685: fix gpio-only operation.

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 ?