From: Sven Van Asbroeck <thesven73@gmail.com>
To: Clemens Gruber <clemens.gruber@pqgruber.com>
Cc: linux-pwm@vger.kernel.org,
"Thierry Reding" <thierry.reding@gmail.com>,
"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
"Lee Jones" <lee.jones@linaro.org>,
"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
"Mika Westerberg" <mika.westerberg@linux.intel.com>,
"David Jander" <david@protonic.nl>
Subject: Re: [PATCH 1/3] pwm: pca9685: Switch to atomic API
Date: Thu, 19 Nov 2020 12:29:26 -0500 [thread overview]
Message-ID: <CAGngYiUELShMgFnvq6XzF0v=2UAwj7gJsPmbdGkmyAbzhM8OLA@mail.gmail.com> (raw)
In-Reply-To: <20201119160013.GA217674@workstation.tuxnet>
On Thu, Nov 19, 2020 at 11:00 AM Clemens Gruber
<clemens.gruber@pqgruber.com> wrote:
>
> One thing I noticed: The driver currently assumes that it comes out of
> POR in "active" state (comment at end of probe and PM calls).
> However, the SLEEP bit is set by default / after POR.
Very good point, the comment is probably not correct.
It would be wrong to assume that the chip is in any particular
state when probe() runs. There is no reset pin, so the CPU running
Linux could have reset while manipulating the chip, there could even
be leftover state from a bootloader talking to the chip, etc...
I remember running into this myself at the time.
However, we want to make sure that the runtime pm puts the chip to sleep,
no matter its initial state. So the current code is correct, but the
comment can be changed to:
/*
* Chip activity state unknown. Tell the runtime pm that the chip is
* active, so pm_runtime_enable() will force it into suspend.
* Which is what we want as all outputs are disabled at this point.
*/
> 2) If !CONFIG_PM: Clear the SLEEP bit in .probe
Is anyone likely to use this driver without CONFIG_PM? My kernel won't even
build without it...
If you personally have no use for it, then I would not bother with the
!CONFIG_PM case. Just formalize in Kconfig that the driver needs PM.
I think we can add "depends on PM" or "select PM", I am not sure which one
to use here.
Sven
next prev parent reply other threads:[~2020-11-19 17:29 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-18 17:44 [PATCH 1/3] pwm: pca9685: Switch to atomic API Clemens Gruber
2020-11-18 17:44 ` [PATCH 2/3] pwm: pca9685: Set full OFF bits in probe Clemens Gruber
2020-11-18 17:44 ` [PATCH 3/3] pwm: pca9685: Support staggered output ON times Clemens Gruber
2020-11-19 0:18 ` [PATCH 1/3] pwm: pca9685: Switch to atomic API Sven Van Asbroeck
2020-11-19 10:00 ` Clemens Gruber
2020-11-19 14:58 ` Sven Van Asbroeck
2020-11-19 16:00 ` Clemens Gruber
2020-11-19 17:29 ` Sven Van Asbroeck [this message]
2020-11-21 14:58 ` Clemens Gruber
2020-11-21 22:00 ` Sven Van Asbroeck
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAGngYiUELShMgFnvq6XzF0v=2UAwj7gJsPmbdGkmyAbzhM8OLA@mail.gmail.com' \
--to=thesven73@gmail.com \
--cc=clemens.gruber@pqgruber.com \
--cc=david@protonic.nl \
--cc=lee.jones@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pwm@vger.kernel.org \
--cc=mika.westerberg@linux.intel.com \
--cc=thierry.reding@gmail.com \
--cc=u.kleine-koenig@pengutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.