On 04/10/2017 03:28 AM, Simon Glass wrote:
Hi,

On 7 April 2017 at 05:02, Kever Yang <kever.y...@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: Kever Yang <kever.y...@rock-chips.com>
---

  drivers/power/regulator/pwm_regulator.c | 12 ++++++++++--
  drivers/pwm/pwm-uclass.c                | 10 ++++++++++
  drivers/pwm/rk_pwm.c                    | 17 ++++++++++++++++-
  include/pwm.h                           |  9 +++++++++
  4 files changed, 45 insertions(+), 3 deletions(-)

If the generic uclass change and the rk change can be separated, it is
good to do it.

Also we should have a test for pwm (test/dm/pwm.c). Are you able to do
that? If not I could give it a try.
we have no plan to do it.


diff --git a/drivers/power/regulator/pwm_regulator.c 
b/drivers/power/regulator/pwm_regulator.c
index 4875238..f8a6712 100644
--- a/drivers/power/regulator/pwm_regulator.c
+++ b/drivers/power/regulator/pwm_regulator.c
@@ -24,6 +24,8 @@ struct pwm_regulator_info {
         int pwm_id;
         /* the period of one PWM cycle */
         int period_ns;
+       /* the polarity of one PWM */
+       int polarity;

Can you update the comment to indicate what the values mean? E.g. is 0
the normal polarity and 1 inverted?
0 : normal polarity
1 : inverted polarity
I will update the comment next version.

         struct udevice *pwm;
         /* initialize voltage of regulator */
         unsigned int init_voltage;
@@ -49,7 +51,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 +69,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_init(priv->pwm, priv->pwm_id, priv->polarity);

I wonder if it would be better to combine the polarity into the
pwm_set_config() call? Then we can do everything in one call. If not
then I think pwm_set_invert() would be a better name.
The polarity set only once, so we set it in pwm_set_init() call.
Using pwm_set_invert, of course, is also possible.

+       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 +105,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];

Does this mean that the binding has this argument and we have been ignoring it?

Can you bring in the DT binding file from Linux to U-Boot also?


         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..287a7f3 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_init(struct udevice *dev, uint channel, uint polarity)
+{
+       struct pwm_ops *ops = pwm_get_ops(dev);
+
+       if (!ops->set_init)
+               return -ENOSYS;
+
+       return ops->set_init(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..5ff1e00 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_init(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,
                 &regs->ctrl);

@@ -83,6 +97,7 @@ static int rk_pwm_probe(struct udevice *dev)
  }

  static const struct pwm_ops rk_pwm_ops = {
+       .set_init       = rk_pwm_set_init,
         .set_config     = rk_pwm_set_config,
         .set_enable     = rk_pwm_set_enable,
  };
diff --git a/include/pwm.h b/include/pwm.h
index 851915e..a66ee1f 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_init() - Set the PWM invert
+        *
+        * @dev:        PWM device to update
+        * @channel:    PWM channel to update
+        * @polarity:  PWM invert polarity
+        * @return 0 if OK, -ve on error
+        */
+       int (*set_init)(struct udevice *dev, uint channel, uint polarity);
+       /**
          * set_config() - Set the PWM configuration
          *
          * @dev:        PWM device to update
--
1.9.1


Regards,
Simon




_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to