Hi Kever,
On 11 April 2017 at 02:35, Kever Yang <kever.yang at rock-chips.com> wrote:
> The latest kernel PWM drivers enable the polarity settings. When system
> run from U-Boot to kerenl, if there are differences in polarity set or
> duty cycle, the PMW will re-init:
> close -> set polarity and duty cycle -> enable the PWM.
> The power supply controled by pwm regulator may have voltage shaking,
> which lead to the system not stable.
>
> Signed-off-by: Elaine Zhang <zhangqing at rock-chips.com>
> Signed-off-by: Kever Yang <kever.yang at rock-chips.com>
> ---
>
> Changes in v2:
> - use pwm_set_invert() instead of pwm_set_init()
> - add comment for polarity
>
> drivers/power/regulator/pwm_regulator.c | 16 ++++++++++++++--
> drivers/pwm/pwm-uclass.c | 10 ++++++++++
> drivers/pwm/rk_pwm.c | 17 ++++++++++++++++-
> include/pwm.h | 9 +++++++++
> 4 files changed, 49 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/power/regulator/pwm_regulator.c b/drivers/power/regulator/pwm_regulator.c
> index 4875238..3ffe3a1 100644
> --- a/drivers/power/regulator/pwm_regulator.c
> +++ b/drivers/power/regulator/pwm_regulator.c
> @@ -24,6 +24,12 @@ struct pwm_regulator_info {
> int pwm_id;
> /* the period of one PWM cycle */
> int period_ns;
> + /*
> + * the polarity of one PWM
> + * 0: normal polarity
> + * 1: inverted polarity
> + */
> + int polarity;
> struct udevice *pwm;
> /* initialize voltage of regulator */
> unsigned int init_voltage;
> @@ -49,7 +55,7 @@ static int pwm_voltage_to_duty_cycle_percentage(struct udevice *dev, int req_uV)
> int max_uV = priv->max_voltage;
> int diff = max_uV - min_uV;
>
> - return 100 - (((req_uV * 100) - (min_uV * 100)) / diff);
> + return ((req_uV * 100) - (min_uV * 100)) / diff;
> }
>
> static int pwm_regulator_get_voltage(struct udevice *dev)
> @@ -67,6 +73,12 @@ static int pwm_regulator_set_voltage(struct udevice *dev, int uvolt)
>
> duty_cycle = pwm_voltage_to_duty_cycle_percentage(dev, uvolt);
>
> + ret = pwm_set_invert(priv->pwm, priv->pwm_id, priv->polarity);
> + if (ret) {
> + dev_err(dev, "Failed to init PWM\n");
> + return ret;
> + }
> +
> ret = pwm_set_config(priv->pwm, priv->pwm_id,
> (priv->period_ns / 100) * duty_cycle, priv->period_ns);
> if (ret) {
> @@ -97,9 +109,9 @@ static int pwm_regulator_ofdata_to_platdata(struct udevice *dev)
> debug("%s: Cannot get PWM phandle: ret=%d\n", __func__, ret);
> return ret;
> }
> - /* TODO: pwm_id here from device tree if needed */
>
> priv->period_ns = args.args[1];
> + priv->polarity = args.args[2];
>
> priv->init_voltage = fdtdec_get_int(blob, node,
> "regulator-init-microvolt", -1);
> diff --git a/drivers/pwm/pwm-uclass.c b/drivers/pwm/pwm-uclass.c
> index c2200af..d1d7fce 100644
> --- a/drivers/pwm/pwm-uclass.c
> +++ b/drivers/pwm/pwm-uclass.c
> @@ -9,6 +9,16 @@
> #include <dm.h>
> #include <pwm.h>
>
> +int pwm_set_invert(struct udevice *dev, uint channel, uint polarity)
> +{
> + struct pwm_ops *ops = pwm_get_ops(dev);
> +
> + if (!ops->set_invert)
> + return -ENOSYS;
> +
> + return ops->set_invert(dev, channel, polarity);
> +}
> +
> int pwm_set_config(struct udevice *dev, uint channel, uint period_ns,
> uint duty_ns)
> {
> diff --git a/drivers/pwm/rk_pwm.c b/drivers/pwm/rk_pwm.c
> index 9254f5b..f003d09 100644
> --- a/drivers/pwm/rk_pwm.c
> +++ b/drivers/pwm/rk_pwm.c
> @@ -21,8 +21,22 @@ DECLARE_GLOBAL_DATA_PTR;
> struct rk_pwm_priv {
> struct rk3288_pwm *regs;
> ulong freq;
> + uint enable_conf;
> };
>
> +static int rk_pwm_set_invert(struct udevice *dev, uint channel, uint polarity)
> +{
> + struct rk_pwm_priv *priv = dev_get_priv(dev);
> +
> + debug("%s: polarity=%u\n", __func__, polarity);
> + if (polarity)
> + priv->enable_conf |= PWM_DUTY_NEGATIVE | PWM_INACTIVE_POSTIVE;
> + else
> + priv->enable_conf |= PWM_DUTY_POSTIVE | PWM_INACTIVE_NEGATIVE;
> +
> + return 0;
> +}
> +
> static int rk_pwm_set_config(struct udevice *dev, uint channel, uint period_ns,
> uint duty_ns)
> {
> @@ -32,7 +46,7 @@ static int rk_pwm_set_config(struct udevice *dev, uint channel, uint period_ns,
>
> debug("%s: period_ns=%u, duty_ns=%u\n", __func__, period_ns, duty_ns);
> writel(PWM_SEL_SRC_CLK | PWM_OUTPUT_LEFT | PWM_LP_DISABLE |
> - PWM_CONTINUOUS | PWM_DUTY_POSTIVE | PWM_INACTIVE_POSTIVE |
> + PWM_CONTINUOUS | priv->enable_conf |
> RK_PWM_DISABLE,
> ®s->ctrl);
>
> @@ -83,6 +97,7 @@ static int rk_pwm_probe(struct udevice *dev)
> }
>
> static const struct pwm_ops rk_pwm_ops = {
> + .set_invert = rk_pwm_set_invert,
> .set_config = rk_pwm_set_config,
> .set_enable = rk_pwm_set_enable,
> };
> diff --git a/include/pwm.h b/include/pwm.h
> index 851915e..66c6bf0 100644
> --- a/include/pwm.h
> +++ b/include/pwm.h
> @@ -14,6 +14,15 @@
> /* struct pwm_ops: Operations for the PWM uclass */
> struct pwm_ops {
> /**
> + * set_invert() - Set the PWM invert
> + *
> + * @dev: PWM device to update
> + * @channel: PWM channel to update
> + * @polarity: PWM invert polarity
Please document the meaning of this parameter.
> + * @return 0 if OK, -ve on error
> + */
> + int (*set_invert)(struct udevice *dev, uint channel, uint polarity);
> + /**
> * set_config() - Set the PWM configuration
> *
> * @dev: PWM device to update
> --
> 1.9.1
>
Can you also please write a test in test/dm/pwm.c with a sandbox pwm
driver? A recent example is:
http://patchwork.ozlabs.org/patch/750796/
Regards,
Simon
