From: Tzung-Bi Shih <tzungbi@kernel.org>
To: "Uwe Kleine-König" <u.kleine-koenig@baylibre.com>
Cc: Benson Leung <bleung@chromium.org>,
Guenter Roeck <groeck@chromium.org>,
linux-pwm@vger.kernel.org, chrome-platform@lists.linux.dev,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] pwm: cros-ec: Don't care about consumers in .get_state()
Date: Wed, 12 Jun 2024 06:27:14 +0000 [thread overview]
Message-ID: <Zmk_wolV5vK4JPCV@google.com> (raw)
In-Reply-To: <dbygoq4rzxnzforpdsvuy5jze2rxqszi6qxtx6q37yxwjo36o6@qfoc6iz2nbay>
On Tue, Jun 11, 2024 at 12:39:44PM +0200, Uwe Kleine-König wrote:
> On Tue, Jun 11, 2024 at 08:50:44AM +0000, Tzung-Bi Shih wrote:
> > On Fri, Jun 07, 2024 at 10:44:15AM +0200, Uwe Kleine-König wrote:
> > > The get_state() callback is never called (in a visible way) after there
> > > is a consumer for a pwm device. The core handles loosing the information
> > > about duty_cycle just fine.
> >
> > ChromeOS EC has no separated "enabled" state, it sees `duty == 0` as
> > "disabled"[1]. 1db37f9561b2 ("pwm: cros-ec: Cache duty cycle value")
> > caches the value in kernel side so that it can retrieve the original duty
> > value even if (struct pwm_state *)->enabled is false.
>
> There is no need to cache, so the following would work:
Ack.
> > To make sure I understand, did you mean the original duty value could be less
> > important because:
> > - We are less caring as it is in a debug context at [2]?
> > - At [3], the PWM device is still initializing.
>
> It doesn't really matter that this is about debug or initialisation. The
> key here is that the core can handle the PWM using duty_cycle 0 (or
> anything else) when it was requested to be disabled.
>
>
> > [1]: https://crrev.com/0e16954460a08133b2557150e0897014ea2b9672/common/pwm.c#66
> > [2]: https://elixir.bootlin.com/linux/v6.10-rc3/source/drivers/pwm/core.c#L52
> > [3]: https://elixir.bootlin.com/linux/v6.10-rc3/source/drivers/pwm/core.c#L371
I was trying to understand the description in the commit message:
: The get_state() callback is never called (in a visible way) after there
: is a consumer for a pwm device.
I guess I understood; the core reads the duty value via get_state() when:
- Initializing the device for the intial value.
- Debugging for checking if apply() really takes effect.
What 1db37f9561b2 worried about is already addressed by the core[4].
[4]: https://elixir.bootlin.com/linux/v6.10-rc3/source/drivers/pwm/core.c#L495
Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
next prev parent reply other threads:[~2024-06-12 6:27 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-07 8:44 [PATCH 0/3] pwm: cros-ec: Some simplifications Uwe Kleine-König
2024-06-07 8:44 ` [PATCH 1/3] pwm: cros-ec: Don't care about consumers in .get_state() Uwe Kleine-König
2024-06-07 16:43 ` Uwe Kleine-König
2024-06-08 14:24 ` kernel test robot
2024-06-11 8:50 ` Tzung-Bi Shih
2024-06-11 10:39 ` Uwe Kleine-König
2024-06-12 6:27 ` Tzung-Bi Shih [this message]
2024-06-13 6:07 ` Uwe Kleine-König
2024-06-07 8:44 ` [PATCH 2/3] pwm: cros-ec: Simplify device tree xlation Uwe Kleine-König
2024-06-12 6:27 ` Tzung-Bi Shih
2024-06-07 8:44 ` [PATCH 3/3] pwm: Make pwm_request_from_chip() private to the core Uwe Kleine-König
2024-06-12 6:27 ` Tzung-Bi Shih
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=Zmk_wolV5vK4JPCV@google.com \
--to=tzungbi@kernel.org \
--cc=bleung@chromium.org \
--cc=chrome-platform@lists.linux.dev \
--cc=groeck@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pwm@vger.kernel.org \
--cc=u.kleine-koenig@baylibre.com \
/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.