[U-Boot] [PATCH] power: regulator: pwm: support pwm polarity setting

[Elaine Zhang]

On 04/07/2017 07:28 PM, Jaehoon Chung wrote:
> Hi Kever,
>
> On 04/07/2017 08:02 PM, Kever Yang 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.yang at 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(-)
>>
>> 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;
>>   	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);
>> +	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];
>>
>>   	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)
>> +{
>
> "channel" is need?
Don't need.
>
>> +	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);
>
> Is there a reason about using int type? It's always return 0...
> Otherwise do you have a plan to use "int" type?
>
No particular reason.
just follow as set_config and set_enable.

> Best Regards,
> Jaehoon Chung
>
>> +	/**
>>   	 * set_config() - Set the PWM configuration
>>   	 *
>>   	 * @dev:	PWM device to update
>>
>
>
>
>