[U-Boot] [PATCH 3/7] power: regulator: add pwm regulator
Kever Yang
kever.yang at rock-chips.com
Tue Sep 6 12:03:57 CEST 2016
Hi Simon,
On 09/06/2016 09:03 AM, Simon Glass wrote:
> Hi Kever,
>
> On 29 August 2016 at 21:02, Kever Yang <kever.yang at rock-chips.com> wrote:
>> This driver add support for pwm regulator.
>>
>> Signed-off-by: Elaine Zhang <zhangqing at rock-chips.com>
>> Signed-off-by: Kever Yang <kever.yang at rock-chips.com>
>> ---
>>
>> drivers/power/regulator/Kconfig | 9 ++
>> drivers/power/regulator/Makefile | 1 +
>> drivers/power/regulator/pwm_regulator.c | 157 ++++++++++++++++++++++++++++++++
>> 3 files changed, 167 insertions(+)
>> create mode 100644 drivers/power/regulator/pwm_regulator.c
>>
>> diff --git a/drivers/power/regulator/Kconfig b/drivers/power/regulator/Kconfig
>> index 17f22dd..c2eaa84 100644
>> --- a/drivers/power/regulator/Kconfig
>> +++ b/drivers/power/regulator/Kconfig
>> @@ -42,6 +42,15 @@ config DM_REGULATOR_PFUZE100
>> features for REGULATOR PFUZE100. The driver implements get/set api for:
>> value, enable and mode.
>>
>> +config REGULATOR_PWM
>> + bool "Enable driver for PWM regulators"
>> + depends on DM_REGULATOR
>> + ---help---
>> + Enable support for the regulator functions of the PWM. The
> Does a PWM have regulator functions? Do you mean a board that uses a
> PWM to control a regulator?
Yes, the PWM control the regulator, the voltage is depend on the PWM
duty ratio.
The PWM regulator is used in some of rockchip board.
>
>> + driver implements get/set api for the various BUCKS.
>> + This driver is controlled by a device tree node
>> + which includes voltage limits.
>> +
>> config DM_REGULATOR_MAX77686
>> bool "Enable Driver Model for REGULATOR MAX77686"
>> depends on DM_REGULATOR && DM_PMIC_MAX77686
>> diff --git a/drivers/power/regulator/Makefile b/drivers/power/regulator/Makefile
>> index 1590d85..ab461ec 100644
>> --- a/drivers/power/regulator/Makefile
>> +++ b/drivers/power/regulator/Makefile
>> @@ -9,6 +9,7 @@ obj-$(CONFIG_$(SPL_)DM_REGULATOR) += regulator-uclass.o
>> obj-$(CONFIG_REGULATOR_ACT8846) += act8846.o
>> obj-$(CONFIG_DM_REGULATOR_MAX77686) += max77686.o
>> obj-$(CONFIG_DM_REGULATOR_PFUZE100) += pfuze100.o
>> +obj-$(CONFIG_REGULATOR_PWM) += pwm_regulator.o
>> obj-$(CONFIG_$(SPL_)DM_REGULATOR_FIXED) += fixed.o
>> obj-$(CONFIG_REGULATOR_RK808) += rk808.o
>> obj-$(CONFIG_REGULATOR_S5M8767) += s5m8767.o
>> diff --git a/drivers/power/regulator/pwm_regulator.c b/drivers/power/regulator/pwm_regulator.c
>> new file mode 100644
>> index 0000000..f75731b
>> --- /dev/null
>> +++ b/drivers/power/regulator/pwm_regulator.c
>> @@ -0,0 +1,157 @@
>> +/*
>> + * Copyright (C) 2016 Rockchip Electronics Co., Ltd
>> + *
>> + * Based on kernel drivers/regulator/pwm-regulator.c
>> + * Copyright (C) 2014 - STMicroelectronics Inc.
>> + * Author: Lee Jones <lee.jones at linaro.org>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <errno.h>
>> +#include <pwm.h>
>> +#include <power/regulator.h>
>> +#include <libfdt.h>
>> +#include <fdt_support.h>
>> +#include <fdtdec.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +struct pwm_regulator_info {
>> + int pwm_id;
> Please add a comment for members
OK,
>
>> + int period;
> period_ms is better, if this is milliseconds. Or period_us?
Yes, period_ns will be fine, which is used in pwm driver.
>
>> + struct udevice *pwm;
>> + unsigned int init_voltage;
>> + unsigned int max_voltage;
>> + unsigned int min_voltage;
>> + unsigned int boot_on;
> bool?
I think this could be removed in next version for it is not used right now.
>
>> + unsigned int volt_uV;
>> +};
>> +
>> +static int pwm_regulator_enable(struct udevice *dev, bool enable)
>> +{
>> + struct pwm_regulator_info *priv = dev_get_priv(dev);
>> +
>> + return pwm_set_enable(priv->pwm, priv->pwm_id, enable);
>> +}
>> +
>> +static int pwm_voltage_to_duty_cycle_percentage(struct udevice *dev, int req_uV)
>> +{
>> + struct pwm_regulator_info *priv = dev_get_priv(dev);
>> + int min_uV = priv->min_voltage;
>> + int max_uV = priv->max_voltage;
>> + int diff = max_uV - min_uV;
>> +
>> + return 100 - (((req_uV * 100) - (min_uV * 100)) / diff);
>> +}
>> +
>> +static int pwm_regulator_get_voltage(struct udevice *dev)
>> +{
>> + struct pwm_regulator_info *priv = dev_get_priv(dev);
>> +
>> + return priv->volt_uV;
>> +}
>> +
>> +static int pwm_regulator_set_voltage(struct udevice *dev, int uvolt)
>> +{
>> + struct pwm_regulator_info *priv = dev_get_priv(dev);
>> + int duty_cycle;
>> + int ret = 0;
>> +
>> + duty_cycle = pwm_voltage_to_duty_cycle_percentage(dev, uvolt);
>> +
>> + ret = pwm_set_config(priv->pwm, priv->pwm_id,
>> + (priv->period / 100) * duty_cycle, priv->period);
>> + if (ret) {
>> + dev_err(dev, "Failed to configure PWM\n");
>> + return ret;
>> + }
>> +
>> + ret = pwm_set_enable(priv->pwm, priv->pwm_id, true);
>> + if (ret) {
>> + dev_err(dev, "Failed to enable PWM\n");
>> + return ret;
>> + }
>> + priv->volt_uV = uvolt;
>> + return ret;
>> +}
>> +
>> +static int pwm_regulator_ofdata_to_platdata(struct udevice *dev)
>> +{
>> + struct pwm_regulator_info *priv = dev_get_priv(dev);
>> + struct fdtdec_phandle_args args;
>> + const void *blob = gd->fdt_blob;
>> + int node = dev->of_offset;
>> + int ret;
>> +
>> + ret = fdtdec_parse_phandle_with_args(blob, node, "pwms", "#pwm-cells",
>> + 0, 0, &args);
>> + if (ret) {
>> + debug("%s: Cannot get PWM phandle: ret=%d\n", __func__, ret);
>> + return ret;
>> + }
>> +
>> + priv->period = args.args[1];
>> +
>> + /* rkpwm do not use the pwm_id, set it to 0 */
>> + priv->pwm_id = 0;
> But this is not a rockchip driver. Also private data starts at 0 so
> you can skip this.
This can be removed for rkpwm does not use it, some other SoC need to
fill it with DT parse if they need it.
>
>> +
>> + priv->init_voltage = fdtdec_get_int(blob, node,
>> + "regulator-init-microvolt", 0);
>> + if (priv->init_voltage < 0)
>> + printf("Cannot find regulator pwm init_voltage\n");
> debug()
>
> If that is an error, please return -EINVAL. The error seems wrong
> also. If the property is missing then fdtdec_get_int() will return
> your default value (0). So perhaps the error should be 'invalid
> voltage < 0'. But actually I cannot see why such a check is useful.
> People should not make such mistakes in device-tree data.
How about using -1 us default value to get an error and then we can return
error code.
>
>> +
>> + ret = uclass_get_device_by_of_offset(UCLASS_PWM, args.node, &priv->pwm);
>> + if (ret) {
>> + debug("%s: Cannot get PWM: ret=%d\n", __func__, ret);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int pwm_regulator_probe(struct udevice *dev)
>> +{
>> + struct pwm_regulator_info *priv = dev_get_priv(dev);
>> + struct dm_regulator_uclass_platdata *uc_pdata;
>> +
>> + uc_pdata = dev_get_uclass_platdata(dev);
>> +
>> + uc_pdata->type = REGULATOR_TYPE_BUCK;
>> + uc_pdata->mode_count = 0;
>> + priv->max_voltage = uc_pdata->max_uV;
>> + priv->min_voltage = uc_pdata->min_uV;
>> +
>> + if (priv->init_voltage)
>> + pwm_regulator_set_voltage(dev, priv->init_voltage);
>> +
>> + if (priv->boot_on)
>> + pwm_regulator_enable(dev, 1);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct dm_regulator_ops pwm_regulator_ops = {
>> + .get_value = pwm_regulator_get_voltage,
>> + .set_value = pwm_regulator_set_voltage,
>> + .set_enable = pwm_regulator_enable,
>> +};
>> +
>> +static const struct udevice_id pwm_regulator_ids[] = {
>> + { .compatible = "pwm-regulator" },
>> + { .compatible = "rockchip_pwm_regulator" },
> Should that be rockchip,pwm-regulator ?
Yep, I will remove this in next version because the only compatible is
enough now.
Thanks,
- Kever
>
>> + { }
>> +};
>> +
>> +U_BOOT_DRIVER(pwm_regulator) = {
>> + .name = "pwm_regulator",
>> + .id = UCLASS_REGULATOR,
>> + .ops = &pwm_regulator_ops,
>> + .probe = pwm_regulator_probe,
>> + .of_match = pwm_regulator_ids,
>> + .ofdata_to_platdata = pwm_regulator_ofdata_to_platdata,
>> + .priv_auto_alloc_size = sizeof(struct pwm_regulator_info),
>> +};
>> +
>> --
>> 1.9.1
>>
> Regards,
> Simon
>
>
>
More information about the U-Boot
mailing list