* [PATCH 2/2] pwm: ensure pwm_apply_state() doesn't modify the state argument
[not found] ` <20190312083537.23748-1-u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2019-03-12 8:35 ` [PATCH 1/2] pwm: rockchip: Don't update the state for the caller of pwm_apply_state() Uwe Kleine-König
@ 2019-03-12 8:35 ` Uwe Kleine-König
[not found] ` <20190312083537.23748-3-u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
1 sibling, 1 reply; 5+ messages in thread
From: Uwe Kleine-König @ 2019-03-12 8:35 UTC (permalink / raw)
To: Thierry Reding, Heiko Stuebner
Cc: linux-pwm-u79uwXL29TY76Z2rM5mHXA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
It is surprising for a PWM consumer when the variable holding the
requested state is modified by pwm_apply_state(). Consider for example a
driver doing:
#define PERIOD 5000000
#define DUTY_LITTLE 10
...
struct pwm_state state = {
.period = PERIOD,
.duty_cycle = DUTY_LITTLE,
.polarity = PWM_POLARITY_NORMAL,
.enabled = true,
};
pwm_apply_state(mypwm, &state);
...
state.duty_cycle = PERIOD / 2;
pwm_apply_state(mypwm, &state);
For sure the second call to pwm_apply_state() should still have
state.period = PERIOD and not something the hardware driver chose for a
reason that doesn't necessarily apply to the second call.
So declare the state argument as a pointer to a const type and adapt all
drivers' .apply callbacks.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/pwm/core.c | 2 +-
drivers/pwm/pwm-atmel-hlcdc.c | 2 +-
drivers/pwm/pwm-atmel.c | 2 +-
drivers/pwm/pwm-bcm-iproc.c | 2 +-
drivers/pwm/pwm-cros-ec.c | 2 +-
drivers/pwm/pwm-hibvt.c | 2 +-
drivers/pwm/pwm-imx27.c | 2 +-
drivers/pwm/pwm-lpss.c | 2 +-
drivers/pwm/pwm-meson.c | 2 +-
drivers/pwm/pwm-rcar.c | 2 +-
drivers/pwm/pwm-rockchip.c | 4 ++--
drivers/pwm/pwm-stm32-lp.c | 2 +-
drivers/pwm/pwm-sun4i.c | 10 ++--------
drivers/pwm/pwm-zx.c | 2 +-
include/linux/pwm.h | 4 ++--
15 files changed, 18 insertions(+), 24 deletions(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 3149204567f3..e547c3217fca 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -464,7 +464,7 @@ EXPORT_SYMBOL_GPL(pwm_free);
* if the requested config is not achievable, for example,
* ->duty_cycle and ->period might be approximated.
*/
-int pwm_apply_state(struct pwm_device *pwm, struct pwm_state *state)
+int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state)
{
int err;
diff --git a/drivers/pwm/pwm-atmel-hlcdc.c b/drivers/pwm/pwm-atmel-hlcdc.c
index 54c6633d9b5d..20f3feb6ce4a 100644
--- a/drivers/pwm/pwm-atmel-hlcdc.c
+++ b/drivers/pwm/pwm-atmel-hlcdc.c
@@ -50,7 +50,7 @@ static inline struct atmel_hlcdc_pwm *to_atmel_hlcdc_pwm(struct pwm_chip *chip)
}
static int atmel_hlcdc_pwm_apply(struct pwm_chip *c, struct pwm_device *pwm,
- struct pwm_state *state)
+ const struct pwm_state *state)
{
struct atmel_hlcdc_pwm *chip = to_atmel_hlcdc_pwm(c);
struct atmel_hlcdc *hlcdc = chip->hlcdc;
diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
index a9fd6f0d408c..c185d3d73d4e 100644
--- a/drivers/pwm/pwm-atmel.c
+++ b/drivers/pwm/pwm-atmel.c
@@ -210,7 +210,7 @@ static void atmel_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm,
}
static int atmel_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
- struct pwm_state *state)
+ const struct pwm_state *state)
{
struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
struct pwm_state cstate;
diff --git a/drivers/pwm/pwm-bcm-iproc.c b/drivers/pwm/pwm-bcm-iproc.c
index d961a8207b1c..56c38cfae92c 100644
--- a/drivers/pwm/pwm-bcm-iproc.c
+++ b/drivers/pwm/pwm-bcm-iproc.c
@@ -115,7 +115,7 @@ static void iproc_pwmc_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
}
static int iproc_pwmc_apply(struct pwm_chip *chip, struct pwm_device *pwm,
- struct pwm_state *state)
+ const struct pwm_state *state)
{
unsigned long prescale = IPROC_PWM_PRESCALE_MIN;
struct iproc_pwmc *ip = to_iproc_pwmc(chip);
diff --git a/drivers/pwm/pwm-cros-ec.c b/drivers/pwm/pwm-cros-ec.c
index 98f6ac6cf6ab..db5faa79c33f 100644
--- a/drivers/pwm/pwm-cros-ec.c
+++ b/drivers/pwm/pwm-cros-ec.c
@@ -93,7 +93,7 @@ static int cros_ec_pwm_get_duty(struct cros_ec_device *ec, u8 index)
}
static int cros_ec_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
- struct pwm_state *state)
+ const struct pwm_state *state)
{
struct cros_ec_pwm_device *ec_pwm = pwm_to_cros_ec_pwm(chip);
int duty_cycle;
diff --git a/drivers/pwm/pwm-hibvt.c b/drivers/pwm/pwm-hibvt.c
index a0b09603d13d..086dfd3850e8 100644
--- a/drivers/pwm/pwm-hibvt.c
+++ b/drivers/pwm/pwm-hibvt.c
@@ -161,7 +161,7 @@ static void hibvt_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
}
static int hibvt_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
- struct pwm_state *state)
+ const struct pwm_state *state)
{
struct hibvt_pwm_chip *hi_pwm_chip = to_hibvt_pwm_chip(chip);
diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c
index 806130654211..eefc69133fa4 100644
--- a/drivers/pwm/pwm-imx27.c
+++ b/drivers/pwm/pwm-imx27.c
@@ -205,7 +205,7 @@ static void pwm_imx27_wait_fifo_slot(struct pwm_chip *chip,
}
static int pwm_imx27_apply(struct pwm_chip *chip, struct pwm_device *pwm,
- struct pwm_state *state)
+ const struct pwm_state *state)
{
unsigned long period_cycles, duty_cycles, prescale;
struct pwm_imx27_chip *imx = to_pwm_imx27_chip(chip);
diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c
index 2ac3a2aa9e53..3b7e7ee0da8c 100644
--- a/drivers/pwm/pwm-lpss.c
+++ b/drivers/pwm/pwm-lpss.c
@@ -125,7 +125,7 @@ static inline void pwm_lpss_cond_enable(struct pwm_device *pwm, bool cond)
}
static int pwm_lpss_apply(struct pwm_chip *chip, struct pwm_device *pwm,
- struct pwm_state *state)
+ const struct pwm_state *state)
{
struct pwm_lpss_chip *lpwm = to_lpwm(chip);
int ret;
diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
index c1ed641b3e26..72def8772593 100644
--- a/drivers/pwm/pwm-meson.c
+++ b/drivers/pwm/pwm-meson.c
@@ -292,7 +292,7 @@ static void meson_pwm_disable(struct meson_pwm *meson, unsigned int id)
}
static int meson_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
- struct pwm_state *state)
+ const struct pwm_state *state)
{
struct meson_pwm_channel *channel = pwm_get_chip_data(pwm);
struct meson_pwm *meson = to_meson_pwm(chip);
diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c
index cfe7dd1b448e..91ab1889eb66 100644
--- a/drivers/pwm/pwm-rcar.c
+++ b/drivers/pwm/pwm-rcar.c
@@ -158,7 +158,7 @@ static void rcar_pwm_disable(struct rcar_pwm_chip *rp)
}
static int rcar_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
- struct pwm_state *state)
+ const struct pwm_state *state)
{
struct rcar_pwm_chip *rp = to_rcar_pwm_chip(chip);
struct pwm_state cur_state;
diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index 16186bcd99e0..1501541d801e 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -102,7 +102,7 @@ static void rockchip_pwm_get_state(struct pwm_chip *chip,
}
static void rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
- struct pwm_state *state)
+ const struct pwm_state *state)
{
struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
unsigned long period, duty;
@@ -186,7 +186,7 @@ static int rockchip_pwm_enable(struct pwm_chip *chip,
}
static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
- struct pwm_state *state)
+ const struct pwm_state *state)
{
struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
struct pwm_state curstate;
diff --git a/drivers/pwm/pwm-stm32-lp.c b/drivers/pwm/pwm-stm32-lp.c
index 0059b24cfdc3..11c5295c6a86 100644
--- a/drivers/pwm/pwm-stm32-lp.c
+++ b/drivers/pwm/pwm-stm32-lp.c
@@ -31,7 +31,7 @@ static inline struct stm32_pwm_lp *to_stm32_pwm_lp(struct pwm_chip *chip)
#define STM32_LPTIM_MAX_PRESCALER 128
static int stm32_pwm_lp_apply(struct pwm_chip *chip, struct pwm_device *pwm,
- struct pwm_state *state)
+ const struct pwm_state *state)
{
struct stm32_pwm_lp *priv = to_stm32_pwm_lp(chip);
unsigned long long prd, div, dty;
diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index 470d4f71e7eb..52dace9f739e 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -146,7 +146,7 @@ static void sun4i_pwm_get_state(struct pwm_chip *chip,
}
static int sun4i_pwm_calculate(struct sun4i_pwm_chip *sun4i_pwm,
- struct pwm_state *state,
+ const struct pwm_state *state,
u32 *dty, u32 *prd, unsigned int *prsclr)
{
u64 clk_rate, div = 0;
@@ -193,17 +193,11 @@ static int sun4i_pwm_calculate(struct sun4i_pwm_chip *sun4i_pwm,
*dty = div;
*prsclr = prescaler;
- div = (u64)pval * NSEC_PER_SEC * *prd;
- state->period = DIV_ROUND_CLOSEST_ULL(div, clk_rate);
-
- div = (u64)pval * NSEC_PER_SEC * *dty;
- state->duty_cycle = DIV_ROUND_CLOSEST_ULL(div, clk_rate);
-
return 0;
}
static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
- struct pwm_state *state)
+ const struct pwm_state *state)
{
struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
struct pwm_state cstate;
diff --git a/drivers/pwm/pwm-zx.c b/drivers/pwm/pwm-zx.c
index 5d27c16edfb1..d6da939a2e57 100644
--- a/drivers/pwm/pwm-zx.c
+++ b/drivers/pwm/pwm-zx.c
@@ -151,7 +151,7 @@ static int zx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
}
static int zx_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
- struct pwm_state *state)
+ const struct pwm_state *state)
{
struct zx_pwm_chip *zpc = to_zx_pwm_chip(chip);
struct pwm_state cstate;
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index b628abfffacc..7525787985bb 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -262,7 +262,7 @@ struct pwm_ops {
int (*capture)(struct pwm_chip *chip, struct pwm_device *pwm,
struct pwm_capture *result, unsigned long timeout);
int (*apply)(struct pwm_chip *chip, struct pwm_device *pwm,
- struct pwm_state *state);
+ const struct pwm_state *state);
void (*get_state)(struct pwm_chip *chip, struct pwm_device *pwm,
struct pwm_state *state);
struct module *owner;
@@ -316,7 +316,7 @@ struct pwm_capture {
/* PWM user APIs */
struct pwm_device *pwm_request(int pwm_id, const char *label);
void pwm_free(struct pwm_device *pwm);
-int pwm_apply_state(struct pwm_device *pwm, struct pwm_state *state);
+int pwm_apply_state(struct pwm_device *pwm, const struct pwm_state *state);
int pwm_adjust_config(struct pwm_device *pwm);
/**
--
2.20.1
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 5+ messages in thread