From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kever Yang Date: Tue, 06 Sep 2016 18:03:57 +0800 Subject: [U-Boot] [PATCH 3/7] power: regulator: add pwm regulator In-Reply-To: References: <1472526134-26965-1-git-send-email-kever.yang@rock-chips.com> <1472526134-26965-4-git-send-email-kever.yang@rock-chips.com> Message-ID: <57CE948D.4030607@rock-chips.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Simon, On 09/06/2016 09:03 AM, Simon Glass wrote: > Hi Kever, > > On 29 August 2016 at 21:02, Kever Yang wrote: >> This driver add support for pwm regulator. >> >> Signed-off-by: Elaine Zhang >> Signed-off-by: Kever Yang >> --- >> >> 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 >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +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 > > >