Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1269910 > unrolled thread

[PATCH v4 00/24] pwm: add support for atomic update

Back to article view | Back to linux.kernel

Contents

  [PATCH v4 00/24] pwm: add support for atomic update Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-11-16 10:00 +0100
    [PATCH v4 19/24] pwm: add the PWM initial state retrieval infra Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-11-16 10:00 +0100
    [PATCH v4 02/24] pwm: use pwm_get_xxx() helpers where appropriate Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-11-16 10:00 +0100
      Re: [PATCH v4 02/24] pwm: use pwm_get_xxx() helpers where appropriate Joachim Eastwood <manabian@gmail.com> - 2015-11-16 18:50 +0100
        Re: [PATCH v4 02/24] pwm: use pwm_get_xxx() helpers where  appropriate Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-11-16 19:20 +0100
    [PATCH v4 13/24] hwmon: pwm-fan: use pwm_get/set_default_xxx() helpers where appropriate Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-11-16 10:10 +0100
      Re: [PATCH v4 13/24] hwmon: pwm-fan: use pwm_get/set_default_xxx()  helpers where appropriate Guenter Roeck <linux@roeck-us.net> - 2015-11-16 17:10 +0100
    [PATCH v4 16/24] pwm: move the enabled/disabled info to pwm_state struct Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-11-16 10:10 +0100
    [PATCH v4 10/24] backlight: pwm: use pwm_get/set_default_xxx() helpers where appropriate Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-11-16 10:10 +0100
    [PATCH v4 11/24] fbdev: use pwm_get/set_default_xxx() helpers where appropriate Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-11-16 10:10 +0100
    [PATCH v4 08/24] leds: pwm: use pwm_get/set_default_xxx() helpers where appropriate Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-11-16 10:10 +0100
    [PATCH v4 15/24] pwm: define a new pwm_state struct Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-11-16 10:10 +0100
    [PATCH v4 14/24] clk: pwm: use pwm_get/set_default_xxx() helpers where appropriate Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-11-16 10:10 +0100
    [PATCH v4 09/24] regulator: pwm: use pwm_get/set_default_xxx() helpers where appropriate Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-11-16 10:10 +0100
      Re: [PATCH v4 09/24] regulator: pwm: use pwm_get/set_default_xxx()  helpers where appropriate Mark Brown <broonie@kernel.org> - 2015-11-16 12:00 +0100
        Re: [PATCH v4 09/24] regulator: pwm: use pwm_get/set_default_xxx()  helpers where appropriate Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-11-16 13:30 +0100
          Re: [PATCH v4 09/24] regulator: pwm: use pwm_get/set_default_xxx()  helpers where appropriate Mark Brown <broonie@kernel.org> - 2015-11-16 19:50 +0100
            Re: [PATCH v4 09/24] regulator: pwm: use pwm_get/set_default_xxx()  helpers where appropriate Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-11-16 20:30 +0100
    [PATCH v4 07/24] pwm: use pwm_get/set_default_xxx() helpers where appropriate Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-11-16 10:10 +0100
    [PATCH v4 06/24] pwm: introduce default period and polarity concepts Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-11-16 10:10 +0100
    [PATCH v4 03/24] clk: pwm: use pwm_get_xxx() helpers where appropriate Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-11-16 10:10 +0100
    [PATCH v4 17/24] backlight: pwm_bl: remove useless call to pwm_set_period Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-11-16 10:10 +0100
    [PATCH v4 12/24] misc: max77693: use pwm_get/set_default_xxx() helpers where appropriate Boris Brezillon <boris.brezillon@free-electrons.com> - 2015-11-16 10:10 +0100
    Re: [PATCH v4 00/24] pwm: add support for atomic update Heiko Stübner <heiko@sntech.de> - 2015-11-16 15:50 +0100

Page 1 of 2  [1] 2  Next page →

#1269910 — [PATCH v4 00/24] pwm: add support for atomic update

Hello,

This series adds support for atomic PWM update, or IOW, the capability
to update all the parameters of a PWM device (enabled/disabled, period,
duty and polarity) in one go.

Best Regards,

Boris

Changes since v3:
- rebased on pwm/for-next after pulling 4.4-rc1
- replace direct access to pwm fields by pwm_get/set_xxx() helpers, thus
  fixing some build errors
- split changes to allow each maintainer to review/ack or take the
  modification through its subsystem

Changes since v2:
- rebased on top of 4.3-rc2
- reintroduced pwm-regulator patches

Changes since v1:
- dropped applied patches
- squashed Heiko's fixes into the rockchip driver changes
- made a few cosmetic changes
- added kerneldoc comments
- added Heiko's patch to display more information in debugfs
- dropped pwm-regulator patches (should be submitted separately)

Boris Brezillon (23):
  pwm: rcar: make use of pwm_is_enabled()
  pwm: use pwm_get_xxx() helpers where appropriate
  clk: pwm: use pwm_get_xxx() helpers where appropriate
  hwmon: pwm-fan: use pwm_get_xxx() helpers where appropriate
  misc: max77693-haptic: use pwm_get_xxx() helpers where appropriate
  pwm: introduce default period and polarity concepts
  pwm: use pwm_get/set_default_xxx() helpers where appropriate
  leds: pwm: use pwm_get/set_default_xxx() helpers where appropriate
  regulator: pwm: use pwm_get/set_default_xxx() helpers where
    appropriate
  backlight: pwm: use pwm_get/set_default_xxx() helpers where
    appropriate
  fbdev: use pwm_get/set_default_xxx() helpers where appropriate
  misc: max77693: use pwm_get/set_default_xxx() helpers where
    appropriate
  hwmon: pwm-fan: use pwm_get/set_default_xxx() helpers where
    appropriate
  clk: pwm: use pwm_get/set_default_xxx() helpers where appropriate
  pwm: define a new pwm_state struct
  pwm: move the enabled/disabled info to pwm_state struct
  backlight: pwm_bl: remove useless call to pwm_set_period
  pwm: declare a default PWM state
  pwm: add the PWM initial state retrieval infra
  pwm: add the core infrastructure to allow atomic update
  pwm: rockchip: add initial state retrieval
  pwm: rockchip: add support for atomic update
  regulator: pwm: properly initialize the ->state field

Heiko Stübner (1):
  pwm: add information about polarity, duty cycle and period to debugfs

 drivers/clk/clk-pwm.c                |  11 +--
 drivers/hwmon/pwm-fan.c              |  16 ++--
 drivers/input/misc/max77693-haptic.c |   9 +-
 drivers/leds/leds-pwm.c              |   2 +-
 drivers/pwm/core.c                   | 169 +++++++++++++++++++++++++++++++----
 drivers/pwm/pwm-crc.c                |   2 +-
 drivers/pwm/pwm-lpc18xx-sct.c        |   2 +-
 drivers/pwm/pwm-pxa.c                |   2 +-
 drivers/pwm/pwm-rcar.c               |   2 +-
 drivers/pwm/pwm-rockchip.c           | 119 +++++++++++++++++++-----
 drivers/pwm/pwm-sun4i.c              |   3 +-
 drivers/regulator/pwm-regulator.c    |  30 ++++++-
 drivers/video/backlight/lm3630a_bl.c |   4 +-
 drivers/video/backlight/pwm_bl.c     |  10 ++-
 drivers/video/fbdev/ssd1307fb.c      |   2 +-
 include/linux/pwm.h                  |  89 +++++++++++++++---
 16 files changed, 386 insertions(+), 86 deletions(-)

-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1269911 — [PATCH v4 19/24] pwm: add the PWM initial state retrieval infra

Add a ->reset_state() function to the pwm_ops struct to let PWM drivers
initialize the PWM state attached to a PWM device.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/pwm/core.c  | 3 +++
 include/linux/pwm.h | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 6bbda6c..ce8b373 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -271,6 +271,9 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip,
 		pwm_set_default_polarity(pwm, polarity);
 		mutex_init(&pwm->lock);
 
+		if (chip->ops->reset_state)
+			chip->ops->reset_state(chip, pwm);
+
 		radix_tree_insert(&pwm_tree, pwm->pwm, pwm);
 	}
 
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 292e7da..caa0ffe 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -182,6 +182,9 @@ static inline enum pwm_polarity pwm_get_polarity(const struct pwm_device *pwm)
  * @set_polarity: configure the polarity of this PWM
  * @enable: enable PWM output toggling
  * @disable: disable PWM output toggling
+ * @reset_state: reset the current PWM state (pwm->state) to the actual
+ *		 hardware state. This function is only called once per
+ *		 PWM device when the PWM chip is registered.
  * @dbg_show: optional routine to show contents in debugfs
  * @owner: helps prevent removal of modules exporting active PWMs
  */
@@ -194,6 +197,7 @@ struct pwm_ops {
 			    enum pwm_polarity polarity);
 	int (*enable)(struct pwm_chip *chip, struct pwm_device *pwm);
 	void (*disable)(struct pwm_chip *chip, struct pwm_device *pwm);
+	void (*reset_state)(struct pwm_chip *chip, struct pwm_device *pwm);
 #ifdef CONFIG_DEBUG_FS
 	void (*dbg_show)(struct pwm_chip *chip, struct seq_file *s);
 #endif
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1269912 — [PATCH v4 02/24] pwm: use pwm_get_xxx() helpers where appropriate

Use pwm_get_xxx() helpers instead of directly accessing the pwm->xxx field.
Doing that will ease adaptation of the PWM framework to support atomic
update.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
Patch generated with the following coccinelle script:

--->8---
virtual patch

@@
struct pwm_device *p;
expression e;
@@
(
-(p)->polarity = e;
+pwm_set_polarity((p), e);
|
-(p)->polarity
+pwm_get_polarity((p))
|
-(p)->period = e;
+pwm_set_period((p), e);
|
-(p)->period
+pwm_get_period((p))
|
-(p)->duty_cycle = e;
+pwm_set_duty_cycle((p), e);
|
-(p)->duty_cycle
+pwm_get_duty_cycle((p))
)
--->8---
---
 drivers/pwm/pwm-crc.c         | 2 +-
 drivers/pwm/pwm-lpc18xx-sct.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-crc.c b/drivers/pwm/pwm-crc.c
index 7101c70..2f88543 100644
--- a/drivers/pwm/pwm-crc.c
+++ b/drivers/pwm/pwm-crc.c
@@ -75,7 +75,7 @@ static int crc_pwm_config(struct pwm_chip *c, struct pwm_device *pwm,
 		return -EINVAL;
 	}
 
-	if (pwm->period != period_ns) {
+	if (pwm_get_period((pwm)) != period_ns) {
 		int clk_div;
 
 		/* changing the clk divisor, need to disable fisrt */
diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c
index 9163085..091fa13 100644
--- a/drivers/pwm/pwm-lpc18xx-sct.c
+++ b/drivers/pwm/pwm-lpc18xx-sct.c
@@ -249,7 +249,7 @@ static int lpc18xx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 			   LPC18XX_PWM_EVSTATEMSK(lpc18xx_data->duty_event),
 			   LPC18XX_PWM_EVSTATEMSK_ALL);
 
-	if (pwm->polarity == PWM_POLARITY_NORMAL) {
+	if (pwm_get_polarity((pwm)) == PWM_POLARITY_NORMAL) {
 		set_event = lpc18xx_pwm->period_event;
 		clear_event = lpc18xx_data->duty_event;
 		res_action = LPC18XX_PWM_RES_SET;
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1270419 — Re: [PATCH v4 02/24] pwm: use pwm_get_xxx() helpers where appropriate

Hi Boris

(Adding Ariel for pwm-lpc18xx-sct)

On 16 November 2015 at 09:56, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> Use pwm_get_xxx() helpers instead of directly accessing the pwm->xxx field.
> Doing that will ease adaptation of the PWM framework to support atomic
> update.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> Patch generated with the following coccinelle script:
>
> --->8---
> virtual patch
>
> @@
> struct pwm_device *p;
> expression e;
> @@
> (
> -(p)->polarity = e;
> +pwm_set_polarity((p), e);
> |
> -(p)->polarity
> +pwm_get_polarity((p))
> |
> -(p)->period = e;
> +pwm_set_period((p), e);
> |
> -(p)->period
> +pwm_get_period((p))
> |
> -(p)->duty_cycle = e;
> +pwm_set_duty_cycle((p), e);
> |
> -(p)->duty_cycle
> +pwm_get_duty_cycle((p))
> )
> --->8---
> ---
>  drivers/pwm/pwm-crc.c         | 2 +-
>  drivers/pwm/pwm-lpc18xx-sct.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pwm/pwm-crc.c b/drivers/pwm/pwm-crc.c
> index 7101c70..2f88543 100644
> --- a/drivers/pwm/pwm-crc.c
> +++ b/drivers/pwm/pwm-crc.c
> @@ -75,7 +75,7 @@ static int crc_pwm_config(struct pwm_chip *c, struct pwm_device *pwm,
>                 return -EINVAL;
>         }
>
> -       if (pwm->period != period_ns) {
> +       if (pwm_get_period((pwm)) != period_ns) {
>                 int clk_div;
>
>                 /* changing the clk divisor, need to disable fisrt */
> diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c
> index 9163085..091fa13 100644
> --- a/drivers/pwm/pwm-lpc18xx-sct.c
> +++ b/drivers/pwm/pwm-lpc18xx-sct.c
> @@ -249,7 +249,7 @@ static int lpc18xx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
>                            LPC18XX_PWM_EVSTATEMSK(lpc18xx_data->duty_event),
>                            LPC18XX_PWM_EVSTATEMSK_ALL);
>
> -       if (pwm->polarity == PWM_POLARITY_NORMAL) {
> +       if (pwm_get_polarity((pwm)) == PWM_POLARITY_NORMAL) {

What is the deal with the double parentheses?

Think I saw that in some of the other patches as well.


regards,
Joachim Eastwood
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1270445 — Re: [PATCH v4 02/24] pwm: use pwm_get_xxx() helpers where appropriate

Hi Joachim,

On Mon, 16 Nov 2015 18:46:44 +0100
Joachim  Eastwood <manabian@gmail.com> wrote:

> Hi Boris
> 
> (Adding Ariel for pwm-lpc18xx-sct)
> 
> On 16 November 2015 at 09:56, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > Use pwm_get_xxx() helpers instead of directly accessing the pwm->xxx field.
> > Doing that will ease adaptation of the PWM framework to support atomic
> > update.
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > ---
> > Patch generated with the following coccinelle script:
> >
> > --->8---
> > virtual patch
> >
> > @@
> > struct pwm_device *p;
> > expression e;
> > @@
> > (
> > -(p)->polarity = e;
> > +pwm_set_polarity((p), e);
> > |
> > -(p)->polarity
> > +pwm_get_polarity((p))
> > |
> > -(p)->period = e;
> > +pwm_set_period((p), e);
> > |
> > -(p)->period
> > +pwm_get_period((p))
> > |
> > -(p)->duty_cycle = e;
> > +pwm_set_duty_cycle((p), e);
> > |
> > -(p)->duty_cycle
> > +pwm_get_duty_cycle((p))
> > )
> > --->8---
> > ---
> >  drivers/pwm/pwm-crc.c         | 2 +-
> >  drivers/pwm/pwm-lpc18xx-sct.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pwm/pwm-crc.c b/drivers/pwm/pwm-crc.c
> > index 7101c70..2f88543 100644
> > --- a/drivers/pwm/pwm-crc.c
> > +++ b/drivers/pwm/pwm-crc.c
> > @@ -75,7 +75,7 @@ static int crc_pwm_config(struct pwm_chip *c, struct pwm_device *pwm,
> >                 return -EINVAL;
> >         }
> >
> > -       if (pwm->period != period_ns) {
> > +       if (pwm_get_period((pwm)) != period_ns) {
> >                 int clk_div;
> >
> >                 /* changing the clk divisor, need to disable fisrt */
> > diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c
> > index 9163085..091fa13 100644
> > --- a/drivers/pwm/pwm-lpc18xx-sct.c
> > +++ b/drivers/pwm/pwm-lpc18xx-sct.c
> > @@ -249,7 +249,7 @@ static int lpc18xx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> >                            LPC18XX_PWM_EVSTATEMSK(lpc18xx_data->duty_event),
> >                            LPC18XX_PWM_EVSTATEMSK_ALL);
> >
> > -       if (pwm->polarity == PWM_POLARITY_NORMAL) {
> > +       if (pwm_get_polarity((pwm)) == PWM_POLARITY_NORMAL) {
> 
> What is the deal with the double parentheses?
> 
> Think I saw that in some of the other patches as well.

It comes from a typo in my coccinelle script. I already fixed it and
regenerated the faulty patches, so please ignore this aspect while
reviewing (this will be addressed in the next version).

Thanks,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1269913 — [PATCH v4 13/24] hwmon: pwm-fan: use pwm_get/set_default_xxx() helpers where appropriate

pwm_set/get_default_xxx() helpers have been introduced to differentiate
the default PWM states (those retrieved through DT, PWM lookup table or
statically assigned by the driver) and the current ones.
Make use of those helpers where appropriate.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/hwmon/pwm-fan.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
index 105b964..989d7b4 100644
--- a/drivers/hwmon/pwm-fan.c
+++ b/drivers/hwmon/pwm-fan.c
@@ -47,8 +47,9 @@ static int  __set_pwm(struct pwm_fan_ctx *ctx, unsigned long pwm)
 	if (ctx->pwm_value == pwm)
 		goto exit_set_pwm_err;
 
-	duty = DIV_ROUND_UP(pwm * (pwm_get_period((ctx->pwm)) - 1), MAX_PWM);
-	ret = pwm_config(ctx->pwm, duty, pwm_get_period((ctx->pwm)));
+	duty = DIV_ROUND_UP(pwm * (pwm_get_default_period((ctx->pwm)) - 1),
+			    MAX_PWM);
+	ret = pwm_config(ctx->pwm, duty, pwm_get_default_period((ctx->pwm)));
 	if (ret)
 		goto exit_set_pwm_err;
 
@@ -234,10 +235,11 @@ static int pwm_fan_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, ctx);
 
 	/* Set duty cycle to maximum allowed */
-	duty_cycle = pwm_get_period((ctx->pwm)) - 1;
+	duty_cycle = pwm_get_default_period((ctx->pwm)) - 1;
 	ctx->pwm_value = MAX_PWM;
 
-	ret = pwm_config(ctx->pwm, duty_cycle, pwm_get_period((ctx->pwm)));
+	ret = pwm_config(ctx->pwm, duty_cycle,
+			 pwm_get_default_period((ctx->pwm)));
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to configure PWM\n");
 		return ret;
@@ -309,9 +311,10 @@ static int pwm_fan_resume(struct device *dev)
 	if (ctx->pwm_value == 0)
 		return 0;
 
-	duty = DIV_ROUND_UP(ctx->pwm_value * (pwm_get_period((ctx->pwm)) - 1),
+	duty = DIV_ROUND_UP(ctx->pwm_value *
+			    (pwm_get_default_period((ctx->pwm)) - 1),
 			    MAX_PWM);
-	ret = pwm_config(ctx->pwm, duty, pwm_get_period((ctx->pwm)));
+	ret = pwm_config(ctx->pwm, duty, pwm_get_default_period((ctx->pwm)));
 	if (ret)
 		return ret;
 	return pwm_enable(ctx->pwm);
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1270323 — Re: [PATCH v4 13/24] hwmon: pwm-fan: use pwm_get/set_default_xxx() helpers where appropriate

On 11/16/2015 12:56 AM, Boris Brezillon wrote:
> pwm_set/get_default_xxx() helpers have been introduced to differentiate
> the default PWM states (those retrieved through DT, PWM lookup table or
> statically assigned by the driver) and the current ones.
> Make use of those helpers where appropriate.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
>   drivers/hwmon/pwm-fan.c | 15 +++++++++------
>   1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
> index 105b964..989d7b4 100644
> --- a/drivers/hwmon/pwm-fan.c
> +++ b/drivers/hwmon/pwm-fan.c
> @@ -47,8 +47,9 @@ static int  __set_pwm(struct pwm_fan_ctx *ctx, unsigned long pwm)
>   	if (ctx->pwm_value == pwm)
>   		goto exit_set_pwm_err;
>
> -	duty = DIV_ROUND_UP(pwm * (pwm_get_period((ctx->pwm)) - 1), MAX_PWM);
> -	ret = pwm_config(ctx->pwm, duty, pwm_get_period((ctx->pwm)));
> +	duty = DIV_ROUND_UP(pwm * (pwm_get_default_period((ctx->pwm)) - 1),
> +			    MAX_PWM);
> +	ret = pwm_config(ctx->pwm, duty, pwm_get_default_period((ctx->pwm)));

Same here - please no unnecessary ( ) [ which were first introduced by
the other patch ].

Guenter

>   	if (ret)
>   		goto exit_set_pwm_err;
>
> @@ -234,10 +235,11 @@ static int pwm_fan_probe(struct platform_device *pdev)
>   	platform_set_drvdata(pdev, ctx);
>
>   	/* Set duty cycle to maximum allowed */
> -	duty_cycle = pwm_get_period((ctx->pwm)) - 1;
> +	duty_cycle = pwm_get_default_period((ctx->pwm)) - 1;
>   	ctx->pwm_value = MAX_PWM;
>
> -	ret = pwm_config(ctx->pwm, duty_cycle, pwm_get_period((ctx->pwm)));
> +	ret = pwm_config(ctx->pwm, duty_cycle,
> +			 pwm_get_default_period((ctx->pwm)));
>   	if (ret) {
>   		dev_err(&pdev->dev, "Failed to configure PWM\n");
>   		return ret;
> @@ -309,9 +311,10 @@ static int pwm_fan_resume(struct device *dev)
>   	if (ctx->pwm_value == 0)
>   		return 0;
>
> -	duty = DIV_ROUND_UP(ctx->pwm_value * (pwm_get_period((ctx->pwm)) - 1),
> +	duty = DIV_ROUND_UP(ctx->pwm_value *
> +			    (pwm_get_default_period((ctx->pwm)) - 1),
>   			    MAX_PWM);
> -	ret = pwm_config(ctx->pwm, duty, pwm_get_period((ctx->pwm)));
> +	ret = pwm_config(ctx->pwm, duty, pwm_get_default_period((ctx->pwm)));
>   	if (ret)
>   		return ret;
>   	return pwm_enable(ctx->pwm);
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1269914 — [PATCH v4 16/24] pwm: move the enabled/disabled info to pwm_state struct

Prepare the transition to PWM atomic update by moving the enabled/disabled
state into the pwm_state struct. This way we can easily update the whole
PWM state by copying the new state in the ->state field.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/pwm/core.c  | 17 +++++++++++++----
 include/linux/pwm.h |  7 ++++---
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index f1c6769..6bbda6c 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -508,10 +508,10 @@ int pwm_enable(struct pwm_device *pwm)
 
 	mutex_lock(&pwm->lock);
 
-	if (!test_and_set_bit(PWMF_ENABLED, &pwm->flags)) {
+	if (!pwm_is_enabled(pwm)) {
 		err = pwm->chip->ops->enable(pwm->chip, pwm);
-		if (err)
-			clear_bit(PWMF_ENABLED, &pwm->flags);
+		if (!err)
+			pwm->state.enabled = true;
 	}
 
 	mutex_unlock(&pwm->lock);
@@ -526,8 +526,17 @@ EXPORT_SYMBOL_GPL(pwm_enable);
  */
 void pwm_disable(struct pwm_device *pwm)
 {
-	if (pwm && test_and_clear_bit(PWMF_ENABLED, &pwm->flags))
+	if (!pwm)
+		return;
+
+	mutex_lock(&pwm->lock);
+
+	if (pwm_is_enabled(pwm)) {
 		pwm->chip->ops->disable(pwm->chip, pwm);
+		pwm->state.enabled = false;
+	}
+
+	mutex_unlock(&pwm->lock);
 }
 EXPORT_SYMBOL_GPL(pwm_disable);
 
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index af42299..398c58c 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -76,8 +76,7 @@ enum pwm_polarity {
 
 enum {
 	PWMF_REQUESTED = 1 << 0,
-	PWMF_ENABLED = 1 << 1,
-	PWMF_EXPORTED = 1 << 2,
+	PWMF_EXPORTED = 1 << 1,
 };
 
 /*
@@ -85,11 +84,13 @@ enum {
  * @period: PWM period (in nanoseconds)
  * @duty_cycle: PWM duty cycle (in nanoseconds)
  * @polarity: PWM polarity
+ * @enabled: PWM enabled status
  */
 struct pwm_state {
 	unsigned int period;
 	unsigned int duty_cycle;
 	enum pwm_polarity polarity;
+	bool enabled;
 };
 
 /**
@@ -117,7 +118,7 @@ struct pwm_device {
 
 static inline bool pwm_is_enabled(const struct pwm_device *pwm)
 {
-	return test_bit(PWMF_ENABLED, &pwm->flags);
+	return pwm->state.enabled;
 }
 
 static inline void pwm_set_period(struct pwm_device *pwm, unsigned int period)
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1269915 — [PATCH v4 10/24] backlight: pwm: use pwm_get/set_default_xxx() helpers where appropriate

pwm_set/get_default_xxx() helpers have been introduced to differentiate
the default PWM states (those retrieved through DT, PWM lookup table or
statically assigned by the driver) and the current ones.
Make use of those helpers where appropriate.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/video/backlight/lm3630a_bl.c | 4 ++--
 drivers/video/backlight/pwm_bl.c     | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
index 35fe482..449ebc3 100644
--- a/drivers/video/backlight/lm3630a_bl.c
+++ b/drivers/video/backlight/lm3630a_bl.c
@@ -162,7 +162,7 @@ static int lm3630a_intr_config(struct lm3630a_chip *pchip)
 
 static void lm3630a_pwm_ctrl(struct lm3630a_chip *pchip, int br, int br_max)
 {
-	unsigned int period = pwm_get_period(pchip->pwmd);
+	unsigned int period = pwm_get_default_period(pchip->pwmd);
 	unsigned int duty = br * period / br_max;
 
 	pwm_config(pchip->pwmd, duty, period);
@@ -425,7 +425,7 @@ static int lm3630a_probe(struct i2c_client *client,
 			return PTR_ERR(pchip->pwmd);
 		}
 	}
-	pchip->pwmd->period = pdata->pwm_period;
+	pwm_set_default_period(pchip->pwmd, pdata->pwm_period);
 
 	/* interrupt enable  : irq 0 is not allowed */
 	pchip->irq = client->irq;
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index ae3c6b6..54d5a03 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -293,7 +293,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
 	 * set the period from platform data if it has not already been set
 	 * via the PWM lookup table.
 	 */
-	pb->period = pwm_get_period(pb->pwm);
+	pb->period = pwm_get_default_period(pb->pwm);
 	if (!pb->period && (data->pwm_period_ns > 0)) {
 		pb->period = data->pwm_period_ns;
 		pwm_set_period(pb->pwm, data->pwm_period_ns);
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1269917 — [PATCH v4 11/24] fbdev: use pwm_get/set_default_xxx() helpers where appropriate

pwm_set/get_default_xxx() helpers have been introduced to differentiate
the default PWM states (those retrieved through DT, PWM lookup table or
statically assigned by the driver) and the current ones.
Make use of those helpers where appropriate.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/video/fbdev/ssd1307fb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index fa34808..81f4885 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -294,7 +294,7 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
 			return PTR_ERR(par->pwm);
 		}
 
-		par->pwm_period = pwm_get_period(par->pwm);
+		par->pwm_period = pwm_get_default_period(par->pwm);
 		/* Enable the PWM */
 		pwm_config(par->pwm, par->pwm_period / 2, par->pwm_period);
 		pwm_enable(par->pwm);
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1269918 — [PATCH v4 08/24] leds: pwm: use pwm_get/set_default_xxx() helpers where appropriate

pwm_set/get_default_xxx() helpers have been introduced to differentiate
the default PWM states (those retrieved through DT, PWM lookup table or
statically assigned by the driver) and the current ones.
Make use of those helpers where appropriate.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Acked-by: Jacek Anaszewski <j.anaszewski@samsung.com>
---
 drivers/leds/leds-pwm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index 1d07e3e..2c564d1 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -125,7 +125,7 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
 	if (led_data->can_sleep)
 		INIT_WORK(&led_data->work, led_pwm_work);
 
-	led_data->period = pwm_get_period(led_data->pwm);
+	led_data->period = pwm_get_default_period(led_data->pwm);
 	if (!led_data->period && (led->pwm_period_ns > 0))
 		led_data->period = led->pwm_period_ns;
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1269919 — [PATCH v4 15/24] pwm: define a new pwm_state struct

The PWM state, represented by its period, duty_cycle and polarity,
is currently directly stored in the PWM device.
Declare a pwm_state structure embedding those field so that we can later
use this struct to atomically update all the PWM parameters at once.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/pwm/core.c  |  6 +++---
 include/linux/pwm.h | 30 +++++++++++++++++++-----------
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 59073a1..f1c6769 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -447,8 +447,8 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
 	if (err)
 		return err;
 
-	pwm->duty_cycle = duty_ns;
-	pwm->period = period_ns;
+	pwm->state.duty_cycle = duty_ns;
+	pwm->state.period = period_ns;
 
 	return 0;
 }
@@ -485,7 +485,7 @@ int pwm_set_polarity(struct pwm_device *pwm, enum pwm_polarity polarity)
 	if (err)
 		goto unlock;
 
-	pwm->polarity = polarity;
+	pwm->state.polarity = polarity;
 
 unlock:
 	mutex_unlock(&pwm->lock);
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 8ba57fc..af42299 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -80,6 +80,18 @@ enum {
 	PWMF_EXPORTED = 1 << 2,
 };
 
+/*
+ * struct pwm_state - state of a PWM channel
+ * @period: PWM period (in nanoseconds)
+ * @duty_cycle: PWM duty cycle (in nanoseconds)
+ * @polarity: PWM polarity
+ */
+struct pwm_state {
+	unsigned int period;
+	unsigned int duty_cycle;
+	enum pwm_polarity polarity;
+};
+
 /**
  * struct pwm_device - PWM channel object
  * @label: name of the PWM device
@@ -89,9 +101,7 @@ enum {
  * @chip: PWM chip providing this PWM device
  * @chip_data: chip-private data associated with the PWM device
  * @lock: used to serialize accesses to the PWM device where necessary
- * @period: period of the PWM signal (in nanoseconds)
- * @duty_cycle: duty cycle of the PWM signal (in nanoseconds)
- * @polarity: polarity of the PWM signal
+ * @state: curent PWM channel state
  */
 struct pwm_device {
 	const char *label;
@@ -102,9 +112,7 @@ struct pwm_device {
 	void *chip_data;
 	struct mutex lock;
 
-	unsigned int period;
-	unsigned int duty_cycle;
-	enum pwm_polarity polarity;
+	struct pwm_state state;
 };
 
 static inline bool pwm_is_enabled(const struct pwm_device *pwm)
@@ -115,7 +123,7 @@ static inline bool pwm_is_enabled(const struct pwm_device *pwm)
 static inline void pwm_set_period(struct pwm_device *pwm, unsigned int period)
 {
 	if (pwm)
-		pwm->period = period;
+		pwm->state.period = period;
 }
 
 static inline void pwm_set_default_period(struct pwm_device *pwm,
@@ -126,7 +134,7 @@ static inline void pwm_set_default_period(struct pwm_device *pwm,
 
 static inline unsigned int pwm_get_period(const struct pwm_device *pwm)
 {
-	return pwm ? pwm->period : 0;
+	return pwm ? pwm->state.period : 0;
 }
 
 static inline unsigned int pwm_get_default_period(const struct pwm_device *pwm)
@@ -137,12 +145,12 @@ static inline unsigned int pwm_get_default_period(const struct pwm_device *pwm)
 static inline void pwm_set_duty_cycle(struct pwm_device *pwm, unsigned int duty)
 {
 	if (pwm)
-		pwm->duty_cycle = duty;
+		pwm->state.duty_cycle = duty;
 }
 
 static inline unsigned int pwm_get_duty_cycle(const struct pwm_device *pwm)
 {
-	return pwm ? pwm->duty_cycle : 0;
+	return pwm ? pwm->state.duty_cycle : 0;
 }
 
 /*
@@ -158,7 +166,7 @@ static inline void pwm_set_default_polarity(struct pwm_device *pwm,
 
 static inline enum pwm_polarity pwm_get_polarity(const struct pwm_device *pwm)
 {
-	return pwm ? pwm->polarity : PWM_POLARITY_NORMAL;
+	return pwm ? pwm->state.polarity : PWM_POLARITY_NORMAL;
 }
 
 /**
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1269920 — [PATCH v4 14/24] clk: pwm: use pwm_get/set_default_xxx() helpers where appropriate

pwm_set/get_default_xxx() helpers have been introduced to differentiate
the default PWM states (those retrieved through DT, PWM lookup table or
statically assigned by the driver) and the current ones.
Make use of those helpers where appropriate.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/clk/clk-pwm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/clk-pwm.c b/drivers/clk/clk-pwm.c
index b6306a2..642a49a 100644
--- a/drivers/clk/clk-pwm.c
+++ b/drivers/clk/clk-pwm.c
@@ -71,23 +71,23 @@ static int clk_pwm_probe(struct platform_device *pdev)
 	if (IS_ERR(pwm))
 		return PTR_ERR(pwm);
 
-	if (!pwm_get_period((pwm))) {
+	if (!pwm_get_default_period((pwm))) {
 		dev_err(&pdev->dev, "invalid PWM period\n");
 		return -EINVAL;
 	}
 
 	if (of_property_read_u32(node, "clock-frequency", &clk_pwm->fixed_rate))
-		clk_pwm->fixed_rate = NSEC_PER_SEC / pwm_get_period((pwm));
+		clk_pwm->fixed_rate = NSEC_PER_SEC / pwm_get_default_period((pwm));
 
-	if (pwm_get_period((pwm)) != NSEC_PER_SEC / clk_pwm->fixed_rate &&
-	    pwm_get_period((pwm)) != DIV_ROUND_UP(NSEC_PER_SEC, clk_pwm->fixed_rate)) {
+	if (pwm_get_default_period((pwm)) != NSEC_PER_SEC / clk_pwm->fixed_rate &&
+	    pwm_get_default_period((pwm)) != DIV_ROUND_UP(NSEC_PER_SEC, clk_pwm->fixed_rate)) {
 		dev_err(&pdev->dev,
 			"clock-frequency does not match PWM period\n");
 		return -EINVAL;
 	}
 
-	ret = pwm_config(pwm, (pwm_get_period((pwm)) + 1) >> 1,
-			 pwm_get_period((pwm)));
+	ret = pwm_config(pwm, (pwm_get_default_period((pwm)) + 1) >> 1,
+			 pwm_get_default_period((pwm)));
 	if (ret < 0)
 		return ret;
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1269921 — [PATCH v4 09/24] regulator: pwm: use pwm_get/set_default_xxx() helpers where appropriate

pwm_set/get_default_xxx() helpers have been introduced to differentiate
the default PWM states (those retrieved through DT, PWM lookup table or
statically assigned by the driver) and the current ones.
Make use of those helpers where appropriate.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/regulator/pwm-regulator.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index 3aca067b..9ffdbd6 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -56,7 +56,7 @@ static int pwm_regulator_set_voltage_sel(struct regulator_dev *rdev,
 	int dutycycle;
 	int ret;
 
-	pwm_reg_period = pwm_get_period(drvdata->pwm);
+	pwm_reg_period = pwm_get_default_period(drvdata->pwm);
 
 	dutycycle = (pwm_reg_period *
 		    drvdata->duty_cycle_table[selector].dutycycle) / 100;
@@ -131,7 +131,7 @@ static int pwm_regulator_set_voltage(struct regulator_dev *rdev,
 {
 	struct pwm_regulator_data *drvdata = rdev_get_drvdata(rdev);
 	unsigned int ramp_delay = rdev->constraints->ramp_delay;
-	unsigned int period = pwm_get_period(drvdata->pwm);
+	unsigned int period = pwm_get_default_period(drvdata->pwm);
 	int duty_cycle;
 	int ret;
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1269997 — Re: [PATCH v4 09/24] regulator: pwm: use pwm_get/set_default_xxx() helpers where appropriate

[Multipart message — attachments visible in raw view] — view raw

On Mon, Nov 16, 2015 at 09:56:32AM +0100, Boris Brezillon wrote:

> +++ b/drivers/regulator/pwm-regulator.c
> @@ -56,7 +56,7 @@ static int pwm_regulator_set_voltage_sel(struct regulator_dev *rdev,
>  	int dutycycle;
>  	int ret;
>  
> -	pwm_reg_period = pwm_get_period(drvdata->pwm);
> +	pwm_reg_period = pwm_get_default_period(drvdata->pwm);
>  
>  	dutycycle = (pwm_reg_period *
>  		    drvdata->duty_cycle_table[selector].dutycycle) / 100;

It's not clear to me that we're not looking for the current period here
or in the other use.  Won't configuring based on a period other than the
one that has been set give the wrong answer?

[toc] | [prev] | [next] | [standalone]

#1270065 — Re: [PATCH v4 09/24] regulator: pwm: use pwm_get/set_default_xxx() helpers where appropriate

Hi Mark,

On Mon, 16 Nov 2015 10:55:58 +0000
Mark Brown <broonie@kernel.org> wrote:

> On Mon, Nov 16, 2015 at 09:56:32AM +0100, Boris Brezillon wrote:
> 
> > +++ b/drivers/regulator/pwm-regulator.c
> > @@ -56,7 +56,7 @@ static int pwm_regulator_set_voltage_sel(struct regulator_dev *rdev,
> >  	int dutycycle;
> >  	int ret;
> >  
> > -	pwm_reg_period = pwm_get_period(drvdata->pwm);
> > +	pwm_reg_period = pwm_get_default_period(drvdata->pwm);
> >  
> >  	dutycycle = (pwm_reg_period *
> >  		    drvdata->duty_cycle_table[selector].dutycycle) / 100;
> 
> It's not clear to me that we're not looking for the current period here
> or in the other use.  Won't configuring based on a period other than the
> one that has been set give the wrong answer?

Hm, maybe that's naming problem. What I call the 'default' period here
is actually the period configured in your board file (using a PWM lookup
table) or your DT. This value represent the period requested by the PWM
user not a default value specified by the PWM chip driver.

The reason we're not using the 'current' period value is because it may
have been set by the bootloader, and may be inappropriate for our use
case (ie. the period may be to small to represent the different
voltages).
ITOH, we're using the current period value when calculating the current
voltage, because we want to get the correct voltage value, and the PWM
device may still use the configuration set by the bootloader (not the
default one specified in your board or DT files).

I hope this clarifies the differences between the current and default
period, and why we should use the default value here.

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1270473 — Re: [PATCH v4 09/24] regulator: pwm: use pwm_get/set_default_xxx() helpers where appropriate

[Multipart message — attachments visible in raw view] — view raw

On Mon, Nov 16, 2015 at 01:23:59PM +0100, Boris Brezillon wrote:
> Mark Brown <broonie@kernel.org> wrote:
> > On Mon, Nov 16, 2015 at 09:56:32AM +0100, Boris Brezillon wrote:

> > > -	pwm_reg_period = pwm_get_period(drvdata->pwm);
> > > +	pwm_reg_period = pwm_get_default_period(drvdata->pwm);

> > It's not clear to me that we're not looking for the current period here
> > or in the other use.  Won't configuring based on a period other than the
> > one that has been set give the wrong answer?

> Hm, maybe that's naming problem. What I call the 'default' period here
> is actually the period configured in your board file (using a PWM lookup
> table) or your DT. This value represent the period requested by the PWM
> user not a default value specified by the PWM chip driver.

> The reason we're not using the 'current' period value is because it may
> have been set by the bootloader, and may be inappropriate for our use
> case (ie. the period may be to small to represent the different
> voltages).

> ITOH, we're using the current period value when calculating the current
> voltage, because we want to get the correct voltage value, and the PWM
> device may still use the configuration set by the bootloader (not the
> default one specified in your board or DT files).

> I hope this clarifies the differences between the current and default
> period, and why we should use the default value here.

To be honest I'm still a bit confused here.  When do we actually apply
the default setting and why do we keep on having to constantly override
it rather than doing this once at boot?  It feels wrong to be using it
every time we set anything.  I'd expect it to be something we only need
to do at probe time or which would automatically be handled by the PWM
framework (but that'd have issues changing the state and potentially
breaking things if done in an uncoordiated fashion).

[toc] | [prev] | [next] | [standalone]

#1270504 — Re: [PATCH v4 09/24] regulator: pwm: use pwm_get/set_default_xxx() helpers where appropriate

On Mon, 16 Nov 2015 18:42:38 +0000
Mark Brown <broonie@kernel.org> wrote:

> On Mon, Nov 16, 2015 at 01:23:59PM +0100, Boris Brezillon wrote:
> > Mark Brown <broonie@kernel.org> wrote:
> > > On Mon, Nov 16, 2015 at 09:56:32AM +0100, Boris Brezillon wrote:
> 
> > > > -	pwm_reg_period = pwm_get_period(drvdata->pwm);
> > > > +	pwm_reg_period = pwm_get_default_period(drvdata->pwm);
> 
> > > It's not clear to me that we're not looking for the current period here
> > > or in the other use.  Won't configuring based on a period other than the
> > > one that has been set give the wrong answer?
> 
> > Hm, maybe that's naming problem. What I call the 'default' period here
> > is actually the period configured in your board file (using a PWM lookup
> > table) or your DT. This value represent the period requested by the PWM
> > user not a default value specified by the PWM chip driver.
> 
> > The reason we're not using the 'current' period value is because it may
> > have been set by the bootloader, and may be inappropriate for our use
> > case (ie. the period may be to small to represent the different
> > voltages).
> 
> > ITOH, we're using the current period value when calculating the current
> > voltage, because we want to get the correct voltage value, and the PWM
> > device may still use the configuration set by the bootloader (not the
> > default one specified in your board or DT files).
> 
> > I hope this clarifies the differences between the current and default
> > period, and why we should use the default value here.
> 
> To be honest I'm still a bit confused here.  When do we actually apply
> the default setting and why do we keep on having to constantly override
> it rather than doing this once at boot?

That's why I said the 'default' name may be inappropriate. The
default values are actually never directly applied by the PWM framework.
It's the default value for a specific PWM user, so it can be applied by
the PWM user when he wants. It's more here as a reference, nothing
forces the PWM user to use this specific value.

> It feels wrong to be using it
> every time we set anything.  I'd expect it to be something we only need
> to do at probe time or which would automatically be handled by the PWM
> framework (but that'd have issues changing the state and potentially
> breaking things if done in an uncoordiated fashion).

The whole point of this series is to smoothly take over the bootloader
config. This is why we are keeping the PWM untouched until someone
really wants to change the regulator output. We should be able to apply
the 'default' PWM period when probing the device, but this means first
extracting the current voltage from the PWM state and then applying a
new dutycycle and the default period in a single operation. Not sure
it's worth the trouble.

Doing it in the PWM framework is not really possible, because the PWM
lookup table and DT definitions are only defining the 'default' period
value not the 'default' dutycycle, and applying that automatically when
requesting the PWM means generating a glitch on the PWM signal
(dutycycle will be set to 0 until the user changes it using
pwm_config() or pwm_apply_state()) which is exactly what we're trying to
solve here.

Also, note that you have to pass the period anyway when configuring the
PWM, so passing the default one or the current one should be pretty
much the same in term of performances (unless the PWM driver is able
to optimize its setting if the period does not change).

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1269925 — [PATCH v4 07/24] pwm: use pwm_get/set_default_xxx() helpers where appropriate

pwm_set/get_default_xxx() helpers have been introduced to differentiate
the default PWM states (those retrieved through DT, PWM lookup table or
statically assigned by the driver) and the current PWM ones.
Make use of those helpers where appropriate.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/pwm/core.c      | 14 +++++++-------
 drivers/pwm/pwm-pxa.c   |  2 +-
 drivers/pwm/pwm-sun4i.c |  3 ++-
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index d24ca5f..59073a1 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -146,12 +146,12 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args)
 	if (IS_ERR(pwm))
 		return pwm;
 
-	pwm_set_period(pwm, args->args[1]);
+	pwm_set_default_period(pwm, args->args[1]);
 
 	if (args->args[2] & PWM_POLARITY_INVERTED)
-		pwm_set_polarity(pwm, PWM_POLARITY_INVERSED);
+		pwm_set_default_polarity(pwm, PWM_POLARITY_INVERSED);
 	else
-		pwm_set_polarity(pwm, PWM_POLARITY_NORMAL);
+		pwm_set_default_polarity(pwm, PWM_POLARITY_NORMAL);
 
 	return pwm;
 }
@@ -172,7 +172,7 @@ of_pwm_simple_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
 	if (IS_ERR(pwm))
 		return pwm;
 
-	pwm_set_period(pwm, args->args[1]);
+	pwm_set_default_period(pwm, args->args[1]);
 
 	return pwm;
 }
@@ -268,7 +268,7 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip,
 		pwm->chip = chip;
 		pwm->pwm = chip->base + i;
 		pwm->hwpwm = i;
-		pwm->polarity = polarity;
+		pwm_set_default_polarity(pwm, polarity);
 		mutex_init(&pwm->lock);
 
 		radix_tree_insert(&pwm_tree, pwm->pwm, pwm);
@@ -751,8 +751,8 @@ struct pwm_device *pwm_get(struct device *dev, const char *con_id)
 	if (IS_ERR(pwm))
 		goto out;
 
-	pwm_set_period(pwm, chosen->period);
-	pwm_set_polarity(pwm, chosen->polarity);
+	pwm_set_default_period(pwm, chosen->period);
+	pwm_set_default_polarity(pwm, chosen->polarity);
 
 out:
 	mutex_unlock(&pwm_lookup_lock);
diff --git a/drivers/pwm/pwm-pxa.c b/drivers/pwm/pwm-pxa.c
index cb2f702..65b80aa 100644
--- a/drivers/pwm/pwm-pxa.c
+++ b/drivers/pwm/pwm-pxa.c
@@ -160,7 +160,7 @@ pxa_pwm_of_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
 	if (IS_ERR(pwm))
 		return pwm;
 
-	pwm_set_period(pwm, args->args[0]);
+	pwm_set_default_period(pwm, args->args[0]);
 
 	return pwm;
 }
diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index 67af9f6..f6a7451 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -354,7 +354,8 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
 	val = sun4i_pwm_readl(pwm, PWM_CTRL_REG);
 	for (i = 0; i < pwm->chip.npwm; i++)
 		if (!(val & BIT_CH(PWM_ACT_STATE, i)))
-			pwm->chip.pwms[i].polarity = PWM_POLARITY_INVERSED;
+			pwm_set_default_polarity(&pwm->chip.pwms[i],
+						 PWM_POLARITY_INVERSED);
 	clk_disable_unprepare(pwm->clk);
 
 	return 0;
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1269926 — [PATCH v4 06/24] pwm: introduce default period and polarity concepts

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.

PWM users might call the pwm_get_default_period to query the default
period value. There is currently no helper to query the default
polarity, but it might be added later on if there is a need for it.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
---
 include/linux/pwm.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index cfc3ed4..8ba57fc 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -118,11 +118,22 @@ static inline void pwm_set_period(struct pwm_device *pwm, unsigned int period)
 		pwm->period = period;
 }
 
+static inline void pwm_set_default_period(struct pwm_device *pwm,
+					  unsigned int period)
+{
+	pwm_set_period(pwm, period);
+}
+
 static inline unsigned int pwm_get_period(const struct pwm_device *pwm)
 {
 	return pwm ? pwm->period : 0;
 }
 
+static inline unsigned int pwm_get_default_period(const struct pwm_device *pwm)
+{
+	return pwm_get_period(pwm);
+}
+
 static inline void pwm_set_duty_cycle(struct pwm_device *pwm, unsigned int duty)
 {
 	if (pwm)
@@ -139,6 +150,12 @@ static inline unsigned int pwm_get_duty_cycle(const struct pwm_device *pwm)
  */
 int pwm_set_polarity(struct pwm_device *pwm, enum pwm_polarity polarity);
 
+static inline void pwm_set_default_polarity(struct pwm_device *pwm,
+					    enum pwm_polarity polarity)
+{
+	pwm_set_polarity(pwm, polarity);
+}
+
 static inline enum pwm_polarity pwm_get_polarity(const struct pwm_device *pwm)
 {
 	return pwm ? pwm->polarity : PWM_POLARITY_NORMAL;
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web