All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: boris.brezillon@free-electrons.com (Boris Brezillon)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH 05/15] pwm: introduce default period and polarity concepts
Date: Mon, 20 Jul 2015 10:14:43 +0200	[thread overview]
Message-ID: <20150720101443.180ebddb@bbrezillon> (raw)
In-Reply-To: <20150720080313.GF29614@ulmo>

Hi Thierry,

On Mon, 20 Jul 2015 10:03:14 +0200
Thierry Reding <thierry.reding@gmail.com> wrote:

> On Thu, Jul 02, 2015 at 09:49:55AM +0200, Boris Brezillon wrote:
> > On Thu, 2 Jul 2015 08:44:45 +0200
> > Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de> wrote:
> > 
> > > On Wed, Jul 01, 2015 at 10:21:51AM +0200, Boris Brezillon wrote:
> > > > When requested by a user, the PWM is assigned a default period and polarity
> > > > extracted from the DT, the platform data or statically set by the driver.
> > > > Those default values are currently stored in the period and polarity
> > > > fields of the pwm_device struct, but they will be stored somewhere else
> > > > once we have introduced the architecture allowing for hardware state
> > > > retrieval.
> > > > 
> > > > The pwm_set_default_polarity and pwm_set_default_period should only be
> > > > used by PWM drivers or the PWM core infrastructure to specify the
> > > > default period and polarity values.
> > > Would it make sense to put the prototypes of
> > > pwm_set_default_p{olarity,eriod} into (say) drivers/pwm/pwm-private.h
> > > then?
> > > 
> > 
> > Yes, definitely. I was thinking about moving those functions/prototypes
> > into include/linux/pwm-provider.h, but I'm fine with
> > drivers/pwm/pwm-private.h too.
> > 
> > Thierry, any opinion ?
> 
> I'm not sure I see the need for this. If they are the default values and
> drivers have no need to change them, then storing them in the regular
> period and polarity fields seems just fine (they'll be propagated into
> new state objects as they get created).
> 
> And if the driver has a need to change them, then why would it ever care
> about the default values?

Because the period is often directly extracted from the DT, and this
extracted period may not match the one configured by the bootloader.

If the driver wants to display the current status without changing the
PWM state, then the driver will use the current state. ITOH, if it
has to apply a new config, the driver will use the default period
value (extracted from the DT) and change the duty-cycle depending on its
needs.
This is the case we have with the pwm-regulator driver: we want to
display the initial voltage value without changing the PWM config, and
when someone decides to change the voltage, we want to use the default
period instead of keeping the one configured by the bootloader.

Best Regards,

Boris


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

WARNING: multiple messages have this Message-ID (diff)
From: Boris Brezillon <boris.brezillon@free-electrons.com>
To: Thierry Reding <thierry.reding@gmail.com>
Cc: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	linux-pwm@vger.kernel.org, "Alexandre Courbot" <gnurou@gmail.com>,
	linux-fbdev@vger.kernel.org, "Lee Jones" <lee.jones@linaro.org>,
	"Stephen Warren" <swarren@wwwdotorg.org>,
	"Tomi Valkeinen" <tomi.valkeinen@ti.com>,
	"Bryan Wu" <cooloney@gmail.com>,
	"Liam Girdwood" <lgirdwood@gmail.com>,
	"Doug Anderson" <dianders@google.com>,
	"Mark Brown" <broonie@kernel.org>,
	"Richard Purdie" <rpurdie@rpsys.net>,
	linux-arm-kernel@lists.infradead.org,
	"Jingoo Han" <jingoohan1@gmail.com>,
	linux-tegra@vger.kernel.org,
	"Maxime Ripard" <maxime.ripard@free-electrons.com>,
	"Jean-Christophe Plagniol-Villard" <plagnioj@jcrosoft.com>,
	"Jacek Anaszewski" <j.anaszewski@samsung.com>,
	linux-leds@vger.kernel.org
Subject: Re: [RFC PATCH 05/15] pwm: introduce default period and polarity concepts
Date: Mon, 20 Jul 2015 10:14:43 +0200	[thread overview]
Message-ID: <20150720101443.180ebddb@bbrezillon> (raw)
In-Reply-To: <20150720080313.GF29614@ulmo>

Hi Thierry,

On Mon, 20 Jul 2015 10:03:14 +0200
Thierry Reding <thierry.reding@gmail.com> wrote:

> On Thu, Jul 02, 2015 at 09:49:55AM +0200, Boris Brezillon wrote:
> > On Thu, 2 Jul 2015 08:44:45 +0200
> > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> > 
> > > On Wed, Jul 01, 2015 at 10:21:51AM +0200, Boris Brezillon wrote:
> > > > When requested by a user, the PWM is assigned a default period and polarity
> > > > extracted from the DT, the platform data or statically set by the driver.
> > > > Those default values are currently stored in the period and polarity
> > > > fields of the pwm_device struct, but they will be stored somewhere else
> > > > once we have introduced the architecture allowing for hardware state
> > > > retrieval.
> > > > 
> > > > The pwm_set_default_polarity and pwm_set_default_period should only be
> > > > used by PWM drivers or the PWM core infrastructure to specify the
> > > > default period and polarity values.
> > > Would it make sense to put the prototypes of
> > > pwm_set_default_p{olarity,eriod} into (say) drivers/pwm/pwm-private.h
> > > then?
> > > 
> > 
> > Yes, definitely. I was thinking about moving those functions/prototypes
> > into include/linux/pwm-provider.h, but I'm fine with
> > drivers/pwm/pwm-private.h too.
> > 
> > Thierry, any opinion ?
> 
> I'm not sure I see the need for this. If they are the default values and
> drivers have no need to change them, then storing them in the regular
> period and polarity fields seems just fine (they'll be propagated into
> new state objects as they get created).
> 
> And if the driver has a need to change them, then why would it ever care
> about the default values?

Because the period is often directly extracted from the DT, and this
extracted period may not match the one configured by the bootloader.

If the driver wants to display the current status without changing the
PWM state, then the driver will use the current state. ITOH, if it
has to apply a new config, the driver will use the default period
value (extracted from the DT) and change the duty-cycle depending on its
needs.
This is the case we have with the pwm-regulator driver: we want to
display the initial voltage value without changing the PWM config, and
when someone decides to change the voltage, we want to use the default
period instead of keeping the one configured by the bootloader.

Best Regards,

Boris


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

WARNING: multiple messages have this Message-ID (diff)
From: Boris Brezillon <boris.brezillon@free-electrons.com>
To: linux-arm-kernel@lists.infradead.org
Subject: Re: [RFC PATCH 05/15] pwm: introduce default period and polarity concepts
Date: Mon, 20 Jul 2015 08:14:43 +0000	[thread overview]
Message-ID: <20150720101443.180ebddb@bbrezillon> (raw)
In-Reply-To: <20150720080313.GF29614@ulmo>

Hi Thierry,

On Mon, 20 Jul 2015 10:03:14 +0200
Thierry Reding <thierry.reding@gmail.com> wrote:

> On Thu, Jul 02, 2015 at 09:49:55AM +0200, Boris Brezillon wrote:
> > On Thu, 2 Jul 2015 08:44:45 +0200
> > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> > 
> > > On Wed, Jul 01, 2015 at 10:21:51AM +0200, Boris Brezillon wrote:
> > > > When requested by a user, the PWM is assigned a default period and polarity
> > > > extracted from the DT, the platform data or statically set by the driver.
> > > > Those default values are currently stored in the period and polarity
> > > > fields of the pwm_device struct, but they will be stored somewhere else
> > > > once we have introduced the architecture allowing for hardware state
> > > > retrieval.
> > > > 
> > > > The pwm_set_default_polarity and pwm_set_default_period should only be
> > > > used by PWM drivers or the PWM core infrastructure to specify the
> > > > default period and polarity values.
> > > Would it make sense to put the prototypes of
> > > pwm_set_default_p{olarity,eriod} into (say) drivers/pwm/pwm-private.h
> > > then?
> > > 
> > 
> > Yes, definitely. I was thinking about moving those functions/prototypes
> > into include/linux/pwm-provider.h, but I'm fine with
> > drivers/pwm/pwm-private.h too.
> > 
> > Thierry, any opinion ?
> 
> I'm not sure I see the need for this. If they are the default values and
> drivers have no need to change them, then storing them in the regular
> period and polarity fields seems just fine (they'll be propagated into
> new state objects as they get created).
> 
> And if the driver has a need to change them, then why would it ever care
> about the default values?

Because the period is often directly extracted from the DT, and this
extracted period may not match the one configured by the bootloader.

If the driver wants to display the current status without changing the
PWM state, then the driver will use the current state. ITOH, if it
has to apply a new config, the driver will use the default period
value (extracted from the DT) and change the duty-cycle depending on its
needs.
This is the case we have with the pwm-regulator driver: we want to
display the initial voltage value without changing the PWM config, and
when someone decides to change the voltage, we want to use the default
period instead of keeping the one configured by the bootloader.

Best Regards,

Boris


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

  reply	other threads:[~2015-07-20  8:14 UTC|newest]

Thread overview: 204+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-01  8:21 [RFC PATCH 00/15] pwm: add support for atomic update Boris Brezillon
2015-07-01  8:21 ` Boris Brezillon
2015-07-01  8:21 ` Boris Brezillon
2015-07-01  8:21 ` [RFC PATCH 01/15] pwm: add the pwm_is_enabled() helper Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-20  7:47   ` Thierry Reding
2015-07-20  7:47     ` Thierry Reding
2015-07-20  7:47     ` Thierry Reding
2015-07-01  8:21 ` [RFC PATCH 02/15] pwm: fix pwm_get_period and pwm_get_duty_cycle prototypes Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-20  7:50   ` Thierry Reding
2015-07-20  7:50     ` Thierry Reding
2015-07-20  7:50     ` Thierry Reding
2015-07-01  8:21 ` [RFC PATCH 03/15] pwm: add pwm_get_polarity helper function Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-20  7:52   ` Thierry Reding
2015-07-20  7:52     ` Thierry Reding
2015-07-20  7:52     ` Thierry Reding
2015-07-01  8:21 ` [RFC PATCH 04/15] pwm: make use of pwm_get_xxx helpers where appropriate Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-20  8:00   ` Thierry Reding
2015-07-20  8:00     ` Thierry Reding
2015-07-20  8:00     ` Thierry Reding
2015-07-01  8:21 ` [RFC PATCH 05/15] pwm: introduce default period and polarity concepts Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-02  6:44   ` Uwe Kleine-König
2015-07-02  6:44     ` Uwe Kleine-König
2015-07-02  6:44     ` Uwe Kleine-König
2015-07-02  7:49     ` Boris Brezillon
2015-07-02  7:49       ` Boris Brezillon
2015-07-02  7:49       ` Boris Brezillon
2015-07-20  8:03       ` Thierry Reding
2015-07-20  8:03         ` Thierry Reding
2015-07-20  8:03         ` Thierry Reding
2015-07-20  8:14         ` Boris Brezillon [this message]
2015-07-20  8:14           ` Boris Brezillon
2015-07-20  8:14           ` Boris Brezillon
2015-07-20  8:22           ` Thierry Reding
2015-07-20  8:22             ` Thierry Reding
2015-07-20  8:22             ` Thierry Reding
2015-07-20  8:32             ` Boris Brezillon
2015-07-20  8:32               ` Boris Brezillon
2015-07-20  8:32               ` Boris Brezillon
2015-07-01  8:21 ` [RFC PATCH 06/15] pwm: define a new pwm_state struct Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-20  8:04   ` Thierry Reding
2015-07-20  8:04     ` Thierry Reding
2015-07-20  8:04     ` Thierry Reding
2015-07-20 10:01     ` Boris Brezillon
2015-07-20 10:01       ` Boris Brezillon
2015-07-20 10:01       ` Boris Brezillon
2015-07-20 10:09       ` Thierry Reding
2015-07-20 10:09         ` Thierry Reding
2015-07-20 10:09         ` Thierry Reding
2015-07-20 10:12         ` Boris Brezillon
2015-07-20 10:12           ` Boris Brezillon
2015-07-20 10:12           ` Boris Brezillon
2015-07-01  8:21 ` [RFC PATCH 07/15] pwm: move the enabled/disabled info to " Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-20  8:11   ` Thierry Reding
2015-07-20  8:11     ` Thierry Reding
2015-07-20  8:11     ` Thierry Reding
2015-07-01  8:21 ` [RFC PATCH 08/15] backlight: pwm_bl: remove useless call to pwm_set_period Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-20  8:16   ` Thierry Reding
2015-07-20  8:16     ` Thierry Reding
2015-07-20  8:16     ` Thierry Reding
2015-07-20  8:21     ` Boris Brezillon
2015-07-20  8:21       ` Boris Brezillon
2015-07-20  8:21       ` Boris Brezillon
2015-07-20  8:36       ` Thierry Reding
2015-07-20  8:36         ` Thierry Reding
2015-07-20  8:36         ` Thierry Reding
2015-07-20  8:50         ` Boris Brezillon
2015-07-20  8:50           ` Boris Brezillon
2015-07-20  8:50           ` Boris Brezillon
2015-07-20  9:10           ` Thierry Reding
2015-07-20  9:10             ` Thierry Reding
2015-07-20  9:10             ` Thierry Reding
2015-07-20  9:57             ` Boris Brezillon
2015-07-20  9:57               ` Boris Brezillon
2015-07-20  9:57               ` Boris Brezillon
2015-07-20 10:01               ` Thierry Reding
2015-07-20 10:01                 ` Thierry Reding
2015-07-20 10:01                 ` Thierry Reding
2015-07-01  8:21 ` [RFC PATCH 09/15] pwm: declare a default PWM state Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-01  8:21 ` [RFC PATCH 10/15] pwm: add the PWM initial state retrieval infra Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-20  9:01   ` Thierry Reding
2015-07-20  9:01     ` Thierry Reding
2015-07-20  9:01     ` Thierry Reding
2015-07-20  9:42     ` Boris Brezillon
2015-07-20  9:42       ` Boris Brezillon
2015-07-20  9:42       ` Boris Brezillon
2015-07-01  8:21 ` [RFC PATCH 11/15] pwm: add the core infrastructure to allow atomic update Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-20  8:59   ` Thierry Reding
2015-07-20  8:59     ` Thierry Reding
2015-07-20  8:59     ` Thierry Reding
2015-07-20  9:48     ` Boris Brezillon
2015-07-20  9:48       ` Boris Brezillon
2015-07-20  9:48       ` Boris Brezillon
2015-07-20 10:04       ` Thierry Reding
2015-07-20 10:04         ` Thierry Reding
2015-07-20 10:04         ` Thierry Reding
2015-07-01  8:21 ` [RFC PATCH 12/15] pwm: rockchip: add initial state retrieval Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-01 21:44   ` Heiko Stübner
2015-07-01 21:44     ` Heiko Stübner
2015-07-01 21:44     ` Heiko Stübner
2015-07-02  7:46     ` Boris Brezillon
2015-07-02  7:46       ` Boris Brezillon
2015-07-02  7:46       ` Boris Brezillon
2015-07-01  8:21 ` [RFC PATCH 13/15] pwm: rockchip: add support for atomic update Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-01  8:21   ` Boris Brezillon
2015-07-01 21:48   ` Heiko Stübner
2015-07-01 21:48     ` Heiko Stübner
2015-07-01 21:48     ` Heiko Stübner
2015-07-02  7:43     ` Boris Brezillon
2015-07-02  7:43       ` Boris Brezillon
2015-07-02  7:43       ` Boris Brezillon
2015-07-01  8:22 ` [RFC PATCH 14/15] regulator: pwm: implement ->enable(), ->disable() and ->is_enabled methods Boris Brezillon
2015-07-01  8:22   ` Boris Brezillon
2015-07-01  8:22   ` Boris Brezillon
2015-07-01 11:58   ` Heiko Stübner
2015-07-01 11:58     ` Heiko Stübner
2015-07-01 11:58     ` Heiko Stübner
2015-07-01 12:05     ` Boris Brezillon
2015-07-01 12:05       ` Boris Brezillon
2015-07-01 12:05       ` Boris Brezillon
2015-07-01 12:08       ` Heiko Stübner
2015-07-01 12:08         ` Heiko Stübner
2015-07-01 12:08         ` Heiko Stübner
2015-07-01 12:19         ` Boris Brezillon
2015-07-01 12:19           ` Boris Brezillon
2015-07-01 12:19           ` Boris Brezillon
2015-07-14 10:50   ` Mark Brown
2015-07-14 10:50     ` Mark Brown
2015-07-14 10:50     ` Mark Brown
2015-07-14 11:02     ` Boris Brezillon
2015-07-14 11:02       ` Boris Brezillon
2015-07-14 11:02       ` Boris Brezillon
2015-07-14 11:08       ` Mark Brown
2015-07-14 11:08         ` Mark Brown
2015-07-14 11:08         ` Mark Brown
2015-07-14 11:16         ` Boris Brezillon
2015-07-14 11:16           ` Boris Brezillon
2015-07-14 11:16           ` Boris Brezillon
2015-07-01  8:22 ` [RFC PATCH 15/15] regulator: pwm: properly initialize the ->state field Boris Brezillon
2015-07-01  8:22   ` Boris Brezillon
2015-07-01  8:22   ` Boris Brezillon
2015-07-14 10:51   ` Mark Brown
2015-07-14 10:51     ` Mark Brown
2015-07-14 10:51     ` Mark Brown
2015-07-14 11:03     ` Boris Brezillon
2015-07-14 11:03       ` Boris Brezillon
2015-07-14 11:03       ` Boris Brezillon
2015-07-01 21:50 ` [RFC PATCH 16/15] pwm: add informations about polarity, duty cycle and period to debugfs Heiko Stübner
2015-07-01 21:50   ` Heiko Stübner
2015-07-01 21:50   ` Heiko Stübner
2015-07-02 13:01   ` Boris Brezillon
2015-07-02 13:01     ` Boris Brezillon
2015-07-02 13:01     ` Boris Brezillon
2015-07-03  8:43     ` [PATCH] " Heiko Stübner
2015-07-03  8:43       ` Heiko Stübner
2015-07-03  8:43       ` Heiko Stübner
2015-07-01 21:57 ` [RFC PATCH 00/15] pwm: add support for atomic update Heiko Stübner
2015-07-01 21:57   ` Heiko Stübner
2015-07-01 21:57   ` Heiko Stübner
2015-07-02  7:55   ` Boris Brezillon
2015-07-02  7:55     ` Boris Brezillon
2015-07-02  7:55     ` Boris Brezillon
2015-07-02  7:03 ` Uwe Kleine-König
2015-07-02  7:03   ` Uwe Kleine-König
2015-07-02  7:03   ` Uwe Kleine-König
2015-07-02  7:17   ` Tomi Valkeinen
2015-07-02  7:17     ` Tomi Valkeinen
2015-07-02  7:17     ` Tomi Valkeinen
2015-07-02  7:42     ` Uwe Kleine-König
2015-07-02  7:42       ` Uwe Kleine-König
2015-07-02  7:42       ` Uwe Kleine-König
2015-07-02  7:30   ` Boris Brezillon
2015-07-02  7:30     ` Boris Brezillon
2015-07-02  7:30     ` Boris Brezillon
2015-07-20  7:16 ` Boris Brezillon
2015-07-20  7:16   ` Boris Brezillon
2015-07-20  7:16   ` Boris Brezillon
2015-07-20  7:43 ` Thierry Reding
2015-07-20  7:43   ` Thierry Reding
2015-07-20  7:43   ` Thierry Reding

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=20150720101443.180ebddb@bbrezillon \
    --to=boris.brezillon@free-electrons.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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.