From: Yadwinder Singh Brar <[email protected]>
Add support for PMIC/regulator portion of MAX77686 multifunction device.
MAX77686 provides LDOs[1-26] and BUCKs[1-9]. This is initial release of driv
which supports setting and getting the voltage of a regulator with I2C
interface.
Signed-off-by: Yadwinder Singh Brar <[email protected]>
---
drivers/regulator/Kconfig | 9 +
drivers/regulator/Makefile | 1 +
drivers/regulator/max77686.c | 387 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 397 insertions(+), 0 deletions(-)
create mode 100644 drivers/regulator/max77686.c
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index c86b886..e8f9417 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -195,6 +195,15 @@ config REGULATOR_MAX8998
via I2C bus. The provided regulator is suitable for S3C6410
and S5PC1XX chips to control VCC_CORE and VCC_USIM voltages.
+config REGULATOR_MAX77686
+ tristate "Maxim 77686 regulator"
+ depends on MFD_MAX77686
+ help
+ This driver controls a Maxim 77686 voltage regulator via I2C
+ bus. The provided regulator is suitable for Exynos5 chips to
+ control VDD_ARM and VDD_INT voltages.It supports LDOs[1-26]
+ and BUCKs[1-9].
+
config REGULATOR_PCAP
tristate "Motorola PCAP2 regulator driver"
depends on EZX_PCAP
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 977fd46..d854453 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_REGULATOR_MAX8925) += max8925-regulator.o
obj-$(CONFIG_REGULATOR_MAX8952) += max8952.o
obj-$(CONFIG_REGULATOR_MAX8997) += max8997.o
obj-$(CONFIG_REGULATOR_MAX8998) += max8998.o
+obj-$(CONFIG_REGULATOR_MAX77686) += max77686.o
obj-$(CONFIG_REGULATOR_MC13783) += mc13783-regulator.o
obj-$(CONFIG_REGULATOR_MC13892) += mc13892-regulator.o
obj-$(CONFIG_REGULATOR_MC13XXX_CORE) += mc13xxx-regulator-core.o
diff --git a/drivers/regulator/max77686.c b/drivers/regulator/max77686.c
new file mode 100644
index 0000000..98dbd50
--- /dev/null
+++ b/drivers/regulator/max77686.c
@@ -0,0 +1,387 @@
+/*
+ * max77686.c - Regulator driver for the Maxim 77686
+ *
+ * Copyright (C) 2012 Samsung Electronics Co. Ltd.
+ * Chiwoong Byun <[email protected]>
+ * Yadwinder Singh Brar <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * This driver is based on max8997.c
+ */
+
+#include <linux/module.h>
+#include <linux/bug.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/mfd/max77686.h>
+#include <linux/mfd/max77686-private.h>
+
+struct max77686_data {
+ struct device *dev;
+ struct max77686_dev *iodev;
+ int num_regulators;
+ struct regulator_dev **rdev;
+ int ramp_delay; /* index of ramp_delay */
+
+ /*GPIO-DVS feature is not enabled with the
+ *current version of MAX77686 driver.*/
+};
+
+static int max77686_voltage_dvs_buck_time_sel(struct regulator_dev *rdev,
+ unsigned int old_sel,
+ unsigned int new_sel)
+{
+ struct max77686_data *max77686 = rdev_get_drvdata(rdev);
+ int ramp[] = {13, 27, 55, 100}; /* ramp_rate in mV/uS */
+
+ return DIV_ROUND_UP(rdev->desc->uV_step *
+ abs(new_sel - old_sel),
+ ramp[max77686->ramp_delay]);
+}
+
+static int max77686_voltage_time_sel(struct regulator_dev *rdev,
+ unsigned int old_sel,
+ unsigned int new_sel)
+{
+ return DIV_ROUND_UP(rdev->desc->uV_step *
+ abs(new_sel - old_sel),
+ 100);
+}
+
+static struct regulator_ops max77686_ops = {
+ .map_voltage = regulator_map_voltage_linear,
+ .list_voltage = regulator_list_voltage_linear,
+ .is_enabled = regulator_is_enabled_regmap,
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .set_voltage_time_sel = max77686_voltage_time_sel,
+};
+
+static struct regulator_ops max77686_buck_ops = {
+ .map_voltage = regulator_map_voltage_linear,
+ .list_voltage = regulator_list_voltage_linear,
+ .is_enabled = regulator_is_enabled_regmap,
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .set_voltage_time_sel = max77686_voltage_dvs_buck_time_sel,
+};
+
+#define regulator_desc_ldo(num) { \
+ .name = "LDO"#num, \
+ .id = MAX77686_LDO##num, \
+ .ops = &max77686_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .owner = THIS_MODULE, \
+ .min_uV = 800000, \
+ .uV_step = 50000, \
+ .n_voltages = 64, \
+ .vsel_reg = MAX77686_REG_LDO1CTRL1 + num - 1, \
+ .vsel_mask = 0x3f, \
+ .enable_reg = MAX77686_REG_LDO1CTRL1 + num - 1, \
+ .enable_mask = 0x0c, \
+}
+#define regulator_desc_ldo_low_vol(num) { \
+ .name = "LDO"#num, \
+ .id = MAX77686_LDO##num, \
+ .ops = &max77686_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .owner = THIS_MODULE, \
+ .min_uV = 800000, \
+ .uV_step = 25000, \
+ .n_voltages = 64, \
+ .vsel_reg = MAX77686_REG_LDO1CTRL1 + num - 1, \
+ .vsel_mask = 0x3f, \
+ .enable_reg = MAX77686_REG_LDO1CTRL1 + num - 1, \
+ .enable_mask = 0x0c, \
+}
+#define regulator_desc_buck(num) { \
+ .name = "BUCK"#num, \
+ .id = MAX77686_BUCK##num, \
+ .ops = &max77686_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .owner = THIS_MODULE, \
+ .min_uV = 750000, \
+ .uV_step = 50000, \
+ .n_voltages = 64, \
+ .vsel_reg = MAX77686_REG_BUCK5OUT + (num - 5) * 2, \
+ .vsel_mask = 0x3f, \
+ .enable_reg = MAX77686_REG_BUCK5CTRL + (num - 5) * 2, \
+ .enable_mask = 0x03, \
+}
+#define regulator_desc_buck1(num) { \
+ .name = "BUCK"#num, \
+ .id = MAX77686_BUCK##num, \
+ .ops = &max77686_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .owner = THIS_MODULE, \
+ .min_uV = 750000, \
+ .uV_step = 50000, \
+ .n_voltages = 64, \
+ .vsel_reg = MAX77686_REG_BUCK1OUT, \
+ .vsel_mask = 0x3f, \
+ .enable_reg = MAX77686_REG_BUCK1CTRL, \
+ .enable_mask = 0x03, \
+}
+#define regulator_desc_buck_dvs(num) { \
+ .name = "BUCK"#num, \
+ .id = MAX77686_BUCK##num, \
+ .ops = &max77686_buck_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .owner = THIS_MODULE, \
+ .min_uV = 600000, \
+ .uV_step = 12500, \
+ .n_voltages = 256, \
+ .vsel_reg = MAX77686_REG_BUCK2DVS1 + (num - 2) * 10, \
+ .vsel_mask = 0xff, \
+ .enable_reg = MAX77686_REG_BUCK2CTRL1 + (num - 2) * 10, \
+ .enable_mask = 0x30, \
+}
+
+static struct regulator_desc regulators[] = {
+ regulator_desc_ldo_low_vol(1),
+ regulator_desc_ldo_low_vol(2),
+ regulator_desc_ldo(3),
+ regulator_desc_ldo(4),
+ regulator_desc_ldo(5),
+ regulator_desc_ldo_low_vol(6),
+ regulator_desc_ldo_low_vol(7),
+ regulator_desc_ldo_low_vol(8),
+ regulator_desc_ldo(9),
+ regulator_desc_ldo(10),
+ regulator_desc_ldo(11),
+ regulator_desc_ldo(12),
+ regulator_desc_ldo(13),
+ regulator_desc_ldo(14),
+ regulator_desc_ldo(15),
+ regulator_desc_ldo(16),
+ regulator_desc_ldo(17),
+ regulator_desc_ldo(18),
+ regulator_desc_ldo(19),
+ regulator_desc_ldo(20),
+ regulator_desc_ldo(21),
+ regulator_desc_ldo(22),
+ regulator_desc_ldo(23),
+ regulator_desc_ldo(24),
+ regulator_desc_ldo(25),
+ regulator_desc_ldo(26),
+ regulator_desc_buck1(1),
+ regulator_desc_buck_dvs(2),
+ regulator_desc_buck_dvs(3),
+ regulator_desc_buck_dvs(4),
+ regulator_desc_buck(5),
+ regulator_desc_buck(6),
+ regulator_desc_buck(7),
+ regulator_desc_buck(8),
+ regulator_desc_buck(9),
+};
+
+#ifdef CONFIG_OF
+static int max77686_pmic_dt_parse_pdata(struct max77686_dev *iodev,
+ struct max77686_platform_data *pdata)
+{
+ struct device_node *pmic_np, *regulators_np;
+ struct of_regulator_match *rdata;
+ unsigned int i, ret;
+
+ pmic_np = iodev->dev->of_node;
+ if (!pmic_np) {
+ dev_err(iodev->dev, "could not find pmic sub-node\n");
+ return -ENODEV;
+ }
+
+ regulators_np = of_find_node_by_name(pmic_np, "voltage-regulators");
+ if (!regulators_np) {
+ dev_err(iodev->dev, "could not find regulators sub-node\n");
+ return -EINVAL;
+ }
+
+ /* count the number of regulators to be supported in pmic */
+ pdata->num_regulators = ARRAY_SIZE(regulators);
+
+ rdata = devm_kzalloc(iodev->dev, sizeof(*rdata) *
+ (pdata->num_regulators), GFP_KERNEL);
+ if (!rdata) {
+ dev_err(iodev->dev,
+ "could not allocate memory for regulator data\n");
+ return -ENOMEM;
+ }
+
+ for (i = 0; i < pdata->num_regulators; i++)
+ rdata[i].name = regulators[i].name;
+
+ ret = of_regulator_match(iodev->dev, regulators_np, rdata,
+ pdata->num_regulators);
+
+ if (ret < 0)
+ dev_err(iodev->dev, "Parsing DT for regulators failed\n");
+ else
+ dev_info(iodev->dev, "regulators found in device tree : %d\n"
+ , ret);
+
+ pdata->regulators = rdata;
+
+ if (of_property_read_u32(pmic_np, "max77686,buck_ramp_delay", &i))
+ pdata->ramp_delay = i & 0xff;
+
+ return 0;
+}
+#else
+static int max77686_pmic_dt_parse_pdata(struct max77686_dev *iodev,
+ struct max77686_platform_data *pdata)
+{
+ return 0;
+}
+#endif /* CONFIG_OF */
+
+static __devinit int max77686_pmic_probe(struct platform_device *pdev)
+{
+ struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
+ struct max77686_platform_data *pdata = iodev->pdata;
+ struct regulator_dev **rdev;
+ struct max77686_data *max77686;
+ struct i2c_client *i2c = iodev->i2c;
+ struct regulator_config config = { };
+ int i, ret, size;
+
+ if (iodev->dev->of_node) {
+ ret = max77686_pmic_dt_parse_pdata(iodev, pdata);
+ if (ret)
+ return ret;
+ } else { /* pdata from machine-setup file */
+ if (!pdata) {
+ dev_err(&pdev->dev, "platform data not found\n");
+ return -ENODEV;
+ } else {
+ if (pdata->num_regulators != ARRAY_SIZE(regulators)) {
+ dev_err(&pdev->dev,
+ "incomplete regulator list\n");
+ return -ENODEV;
+ }
+ }
+ }
+
+ max77686 = devm_kzalloc(&pdev->dev, sizeof(struct max77686_data),
+ GFP_KERNEL);
+ if (!max77686)
+ return -ENOMEM;
+
+ size = sizeof(struct regulator_dev *) * pdata->num_regulators;
+ max77686->rdev = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
+ if (!max77686->rdev) {
+ kfree(max77686);
+ return -ENOMEM;
+ }
+
+ rdev = max77686->rdev;
+
+ max77686->dev = &pdev->dev;
+ max77686->iodev = iodev;
+ max77686->num_regulators = pdata->num_regulators;
+
+ if (pdata->ramp_delay < MAX77686_RAMP_RATE_13MV ||
+ pdata->ramp_delay > MAX77686_RAMP_RATE_100MV)
+ pdata->ramp_delay = MAX77686_RAMP_RATE_27MV; /* default */
+
+ max77686->ramp_delay = pdata->ramp_delay - 1;
+ max77686_update_reg(i2c, MAX77686_REG_BUCK2CTRL1,
+ max77686->ramp_delay << 6, RAMP_MASK);
+ max77686_update_reg(i2c, MAX77686_REG_BUCK3CTRL1,
+ max77686->ramp_delay << 6, RAMP_MASK);
+ max77686_update_reg(i2c, MAX77686_REG_BUCK4CTRL1,
+ max77686->ramp_delay << 6, RAMP_MASK);
+
+ platform_set_drvdata(pdev, max77686);
+
+ for (i = 0; i < pdata->num_regulators; i++) {
+ config.dev = max77686->dev;
+ config.init_data = pdata->regulators[i].init_data;
+ config.driver_data = max77686;
+ config.regmap = iodev->regmap;
+
+ rdev[i] = regulator_register(®ulators[i], &config);
+ if (IS_ERR(rdev[i])) {
+ ret = PTR_ERR(rdev[i]);
+ dev_err(max77686->dev,
+ "regulator init failed for id : %d\n", i);
+ rdev[i] = NULL;
+ goto err;
+ }
+ }
+
+ return 0;
+ err:
+ for (i = 0; i < max77686->num_regulators; i++)
+ if (rdev[i])
+ regulator_unregister(rdev[i]);
+
+ return ret;
+}
+
+static int __devexit max77686_pmic_remove(struct platform_device *pdev)
+{
+ struct max77686_data *max77686 = platform_get_drvdata(pdev);
+ struct regulator_dev **rdev = max77686->rdev;
+ int i;
+
+ for (i = 0; i < max77686->num_regulators; i++)
+ if (rdev[i])
+ regulator_unregister(rdev[i]);
+
+ return 0;
+}
+
+static const struct platform_device_id max77686_pmic_id[] = {
+ {"max77686-pmic", 0},
+ {},
+};
+MODULE_DEVICE_TABLE(platform, max77686_pmic_id);
+
+static struct platform_driver max77686_pmic_driver = {
+ .driver = {
+ .name = "max77686-pmic",
+ .owner = THIS_MODULE,
+ },
+ .probe = max77686_pmic_probe,
+ .remove = __devexit_p(max77686_pmic_remove),
+ .id_table = max77686_pmic_id,
+};
+
+static int __init max77686_pmic_init(void)
+{
+ return platform_driver_register(&max77686_pmic_driver);
+}
+subsys_initcall(max77686_pmic_init);
+
+static void __exit max77686_pmic_cleanup(void)
+{
+ platform_driver_unregister(&max77686_pmic_driver);
+}
+module_exit(max77686_pmic_cleanup);
+
+MODULE_DESCRIPTION("MAXIM 77686 Regulator Driver");
+MODULE_AUTHOR("Chiwoong Byun <[email protected]>");
+MODULE_AUTHOR("Yadwinder Singh Brar <[email protected]>");
+MODULE_LICENSE("GPL");
--
1.7.0.4
Hi, Yadwinder,
As you know, both of us, recently, had competition for one driver
whether you intend or not. And now, i think it is time to stop this and
to find appropriate goal. From now on, i won't update this driver no
more. I recommend you to review my patch and apply feature that you can
apply. And also check comments that i wrote below.
On 2012년 05월 22일 14:57, [email protected] wrote:
> From: Yadwinder Singh Brar <[email protected]>
>
> Add support for PMIC/regulator portion of MAX77686 multifunction device.
> MAX77686 provides LDOs[1-26] and BUCKs[1-9]. This is initial release of driv
> which supports setting and getting the voltage of a regulator with I2C
> interface.
>
> Signed-off-by: Yadwinder Singh Brar <[email protected]>
> ---
> drivers/regulator/Kconfig | 9 +
> drivers/regulator/Makefile | 1 +
> drivers/regulator/max77686.c | 387 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 397 insertions(+), 0 deletions(-)
> create mode 100644 drivers/regulator/max77686.c
>
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index c86b886..e8f9417 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -195,6 +195,15 @@ config REGULATOR_MAX8998
> via I2C bus. The provided regulator is suitable for S3C6410
> and S5PC1XX chips to control VCC_CORE and VCC_USIM voltages.
>
> +config REGULATOR_MAX77686
> + tristate "Maxim 77686 regulator"
> + depends on MFD_MAX77686
> + help
> + This driver controls a Maxim 77686 voltage regulator via I2C
> + bus. The provided regulator is suitable for Exynos5 chips to
> + control VDD_ARM and VDD_INT voltages.It supports LDOs[1-26]
> + and BUCKs[1-9].
> +
> config REGULATOR_PCAP
> tristate "Motorola PCAP2 regulator driver"
> depends on EZX_PCAP
> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index 977fd46..d854453 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -30,6 +30,7 @@ obj-$(CONFIG_REGULATOR_MAX8925) += max8925-regulator.o
> obj-$(CONFIG_REGULATOR_MAX8952) += max8952.o
> obj-$(CONFIG_REGULATOR_MAX8997) += max8997.o
> obj-$(CONFIG_REGULATOR_MAX8998) += max8998.o
> +obj-$(CONFIG_REGULATOR_MAX77686) += max77686.o
> obj-$(CONFIG_REGULATOR_MC13783) += mc13783-regulator.o
> obj-$(CONFIG_REGULATOR_MC13892) += mc13892-regulator.o
> obj-$(CONFIG_REGULATOR_MC13XXX_CORE) += mc13xxx-regulator-core.o
> diff --git a/drivers/regulator/max77686.c b/drivers/regulator/max77686.c
> new file mode 100644
> index 0000000..98dbd50
> --- /dev/null
> +++ b/drivers/regulator/max77686.c
> @@ -0,0 +1,387 @@
> +/*
> + * max77686.c - Regulator driver for the Maxim 77686
> + *
> + * Copyright (C) 2012 Samsung Electronics Co. Ltd.
> + * Chiwoong Byun <[email protected]>
> + * Yadwinder Singh Brar <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + *
> + * This driver is based on max8997.c
> + */
> +
> +#include <linux/module.h>
> +#include <linux/bug.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/gpio.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/machine.h>
> +#include <linux/regulator/of_regulator.h>
> +#include <linux/mfd/max77686.h>
> +#include <linux/mfd/max77686-private.h>
> +
> +struct max77686_data {
> + struct device *dev;
> + struct max77686_dev *iodev;
> + int num_regulators;
> + struct regulator_dev **rdev;
> + int ramp_delay; /* index of ramp_delay */
> +
> + /*GPIO-DVS feature is not enabled with the
> + *current version of MAX77686 driver.*/
> +};
> +
> +static int max77686_voltage_dvs_buck_time_sel(struct regulator_dev *rdev,
> + unsigned int old_sel,
> + unsigned int new_sel)
> +{
> + struct max77686_data *max77686 = rdev_get_drvdata(rdev);
> + int ramp[] = {13, 27, 55, 100}; /* ramp_rate in mV/uS */
> +
> + return DIV_ROUND_UP(rdev->desc->uV_step *
> + abs(new_sel - old_sel),
> + ramp[max77686->ramp_delay]);
> +}
> +
> +static int max77686_voltage_time_sel(struct regulator_dev *rdev,
> + unsigned int old_sel,
> + unsigned int new_sel)
> +{
> + return DIV_ROUND_UP(rdev->desc->uV_step *
> + abs(new_sel - old_sel),
> + 100);
> +}
> +
Does LDO also need waiting for voltage change? I afraid it's not.
> +static struct regulator_ops max77686_ops = {
> + .map_voltage = regulator_map_voltage_linear,
> + .list_voltage = regulator_list_voltage_linear,
> + .is_enabled = regulator_is_enabled_regmap,
> + .enable = regulator_enable_regmap,
> + .disable = regulator_disable_regmap,
> + .get_voltage_sel = regulator_get_voltage_sel_regmap,
> + .set_voltage_sel = regulator_set_voltage_sel_regmap,
> + .set_voltage_time_sel = max77686_voltage_time_sel,
> +};
> +
> +static struct regulator_ops max77686_buck_ops = {
> + .map_voltage = regulator_map_voltage_linear,
> + .list_voltage = regulator_list_voltage_linear,
> + .is_enabled = regulator_is_enabled_regmap,
> + .enable = regulator_enable_regmap,
> + .disable = regulator_disable_regmap,
> + .get_voltage_sel = regulator_get_voltage_sel_regmap,
> + .set_voltage_sel = regulator_set_voltage_sel_regmap,
> + .set_voltage_time_sel = max77686_voltage_dvs_buck_time_sel,
> +};
> +
> +#define regulator_desc_ldo(num) { \
> + .name = "LDO"#num, \
> + .id = MAX77686_LDO##num, \
> + .ops = &max77686_ops, \
> + .type = REGULATOR_VOLTAGE, \
> + .owner = THIS_MODULE, \
> + .min_uV = 800000, \
> + .uV_step = 50000, \
> + .n_voltages = 64, \
> + .vsel_reg = MAX77686_REG_LDO1CTRL1 + num - 1, \
> + .vsel_mask = 0x3f, \
> + .enable_reg = MAX77686_REG_LDO1CTRL1 + num - 1, \
> + .enable_mask = 0x0c, \
> +}
> +#define regulator_desc_ldo_low_vol(num) { \
> + .name = "LDO"#num, \
> + .id = MAX77686_LDO##num, \
> + .ops = &max77686_ops, \
> + .type = REGULATOR_VOLTAGE, \
> + .owner = THIS_MODULE, \
> + .min_uV = 800000, \
> + .uV_step = 25000, \
> + .n_voltages = 64, \
> + .vsel_reg = MAX77686_REG_LDO1CTRL1 + num - 1, \
> + .vsel_mask = 0x3f, \
> + .enable_reg = MAX77686_REG_LDO1CTRL1 + num - 1, \
> + .enable_mask = 0x0c, \
> +}
> +#define regulator_desc_buck(num) { \
> + .name = "BUCK"#num, \
> + .id = MAX77686_BUCK##num, \
> + .ops = &max77686_ops, \
> + .type = REGULATOR_VOLTAGE, \
> + .owner = THIS_MODULE, \
> + .min_uV = 750000, \
> + .uV_step = 50000, \
> + .n_voltages = 64, \
> + .vsel_reg = MAX77686_REG_BUCK5OUT + (num - 5) * 2, \
> + .vsel_mask = 0x3f, \
> + .enable_reg = MAX77686_REG_BUCK5CTRL + (num - 5) * 2, \
> + .enable_mask = 0x03, \
> +}
> +#define regulator_desc_buck1(num) { \
> + .name = "BUCK"#num, \
> + .id = MAX77686_BUCK##num, \
> + .ops = &max77686_ops, \
> + .type = REGULATOR_VOLTAGE, \
> + .owner = THIS_MODULE, \
> + .min_uV = 750000, \
> + .uV_step = 50000, \
> + .n_voltages = 64, \
> + .vsel_reg = MAX77686_REG_BUCK1OUT, \
> + .vsel_mask = 0x3f, \
> + .enable_reg = MAX77686_REG_BUCK1CTRL, \
> + .enable_mask = 0x03, \
> +}
> +#define regulator_desc_buck_dvs(num) { \
> + .name = "BUCK"#num, \
> + .id = MAX77686_BUCK##num, \
> + .ops = &max77686_buck_ops, \
> + .type = REGULATOR_VOLTAGE, \
> + .owner = THIS_MODULE, \
> + .min_uV = 600000, \
> + .uV_step = 12500, \
> + .n_voltages = 256, \
> + .vsel_reg = MAX77686_REG_BUCK2DVS1 + (num - 2) * 10, \
> + .vsel_mask = 0xff, \
> + .enable_reg = MAX77686_REG_BUCK2CTRL1 + (num - 2) * 10, \
> + .enable_mask = 0x30, \
> +}
> +
> +static struct regulator_desc regulators[] = {
> + regulator_desc_ldo_low_vol(1),
> + regulator_desc_ldo_low_vol(2),
> + regulator_desc_ldo(3),
> + regulator_desc_ldo(4),
> + regulator_desc_ldo(5),
> + regulator_desc_ldo_low_vol(6),
> + regulator_desc_ldo_low_vol(7),
> + regulator_desc_ldo_low_vol(8),
> + regulator_desc_ldo(9),
> + regulator_desc_ldo(10),
> + regulator_desc_ldo(11),
> + regulator_desc_ldo(12),
> + regulator_desc_ldo(13),
> + regulator_desc_ldo(14),
> + regulator_desc_ldo(15),
> + regulator_desc_ldo(16),
> + regulator_desc_ldo(17),
> + regulator_desc_ldo(18),
> + regulator_desc_ldo(19),
> + regulator_desc_ldo(20),
> + regulator_desc_ldo(21),
> + regulator_desc_ldo(22),
> + regulator_desc_ldo(23),
> + regulator_desc_ldo(24),
> + regulator_desc_ldo(25),
> + regulator_desc_ldo(26),
> + regulator_desc_buck1(1),
> + regulator_desc_buck_dvs(2),
> + regulator_desc_buck_dvs(3),
> + regulator_desc_buck_dvs(4),
> + regulator_desc_buck(5),
> + regulator_desc_buck(6),
> + regulator_desc_buck(7),
> + regulator_desc_buck(8),
> + regulator_desc_buck(9),
> +};
> +
> +#ifdef CONFIG_OF
> +static int max77686_pmic_dt_parse_pdata(struct max77686_dev *iodev,
> + struct max77686_platform_data *pdata)
> +{
> + struct device_node *pmic_np, *regulators_np;
> + struct of_regulator_match *rdata;
> + unsigned int i, ret;
> +
> + pmic_np = iodev->dev->of_node;
> + if (!pmic_np) {
> + dev_err(iodev->dev, "could not find pmic sub-node\n");
> + return -ENODEV;
> + }
> +
> + regulators_np = of_find_node_by_name(pmic_np, "voltage-regulators");
> + if (!regulators_np) {
> + dev_err(iodev->dev, "could not find regulators sub-node\n");
> + return -EINVAL;
> + }
> +
> + /* count the number of regulators to be supported in pmic */
> + pdata->num_regulators = ARRAY_SIZE(regulators);
> +
> + rdata = devm_kzalloc(iodev->dev, sizeof(*rdata) *
> + (pdata->num_regulators), GFP_KERNEL);
> + if (!rdata) {
> + dev_err(iodev->dev,
> + "could not allocate memory for regulator data\n");
> + return -ENOMEM;
> + }
> +
> + for (i = 0; i < pdata->num_regulators; i++)
> + rdata[i].name = regulators[i].name;
> +
> + ret = of_regulator_match(iodev->dev, regulators_np, rdata,
> + pdata->num_regulators);
> +
> + if (ret < 0)
> + dev_err(iodev->dev, "Parsing DT for regulators failed\n");
> + else
> + dev_info(iodev->dev, "regulators found in device tree : %d\n"
> + , ret);
> +
> + pdata->regulators = rdata;
> +
> + if (of_property_read_u32(pmic_np, "max77686,buck_ramp_delay", &i))
> + pdata->ramp_delay = i & 0xff;
> +
> + return 0;
> +}
> +#else
> +static int max77686_pmic_dt_parse_pdata(struct max77686_dev *iodev,
> + struct max77686_platform_data *pdata)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_OF */
> +
> +static __devinit int max77686_pmic_probe(struct platform_device *pdev)
> +{
> + struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
> + struct max77686_platform_data *pdata = iodev->pdata;
> + struct regulator_dev **rdev;
> + struct max77686_data *max77686;
> + struct i2c_client *i2c = iodev->i2c;
> + struct regulator_config config = { };
> + int i, ret, size;
> +
> + if (iodev->dev->of_node) {
> + ret = max77686_pmic_dt_parse_pdata(iodev, pdata);
> + if (ret)
> + return ret;
> + } else { /* pdata from machine-setup file */
> + if (!pdata) {
> + dev_err(&pdev->dev, "platform data not found\n");
> + return -ENODEV;
> + } else {
> + if (pdata->num_regulators != ARRAY_SIZE(regulators)) {
> + dev_err(&pdev->dev,
> + "incomplete regulator list\n");
> + return -ENODEV;
> + }
> + }
> + }
> +
> + max77686 = devm_kzalloc(&pdev->dev, sizeof(struct max77686_data),
> + GFP_KERNEL);
> + if (!max77686)
> + return -ENOMEM;
> +
> + size = sizeof(struct regulator_dev *) * pdata->num_regulators;
> + max77686->rdev = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
> + if (!max77686->rdev) {
> + kfree(max77686);
> + return -ENOMEM;
> + }
> +
> + rdev = max77686->rdev;
> +
> + max77686->dev = &pdev->dev;
> + max77686->iodev = iodev;
> + max77686->num_regulators = pdata->num_regulators;
> +
> + if (pdata->ramp_delay < MAX77686_RAMP_RATE_13MV ||
> + pdata->ramp_delay > MAX77686_RAMP_RATE_100MV)
> + pdata->ramp_delay = MAX77686_RAMP_RATE_27MV; /* default */
> +
> + max77686->ramp_delay = pdata->ramp_delay - 1;
I think it is better to check pdata->ramp_delay is available.
If pdata doesn't have ramp_delay member it might be error.
> + max77686_update_reg(i2c, MAX77686_REG_BUCK2CTRL1,
> + max77686->ramp_delay << 6, RAMP_MASK);
> + max77686_update_reg(i2c, MAX77686_REG_BUCK3CTRL1,
> + max77686->ramp_delay << 6, RAMP_MASK);
> + max77686_update_reg(i2c, MAX77686_REG_BUCK4CTRL1,
> + max77686->ramp_delay << 6, RAMP_MASK);
> +
Why do you use i2c client still? If you registered regmap you can use
its API. I recommend you to use regmap_update_bits() directly.
> + platform_set_drvdata(pdev, max77686);
> +
> + for (i = 0; i < pdata->num_regulators; i++) {
> + config.dev = max77686->dev;
> + config.init_data = pdata->regulators[i].init_data;
> + config.driver_data = max77686;
> + config.regmap = iodev->regmap;
> +
> + rdev[i] = regulator_register(®ulators[i], &config);
> + if (IS_ERR(rdev[i])) {
> + ret = PTR_ERR(rdev[i]);
> + dev_err(max77686->dev,
> + "regulator init failed for id : %d\n", i);
> + rdev[i] = NULL;
> + goto err;
> + }
> + }
> +
> + return 0;
> + err:
> + for (i = 0; i < max77686->num_regulators; i++)
> + if (rdev[i])
> + regulator_unregister(rdev[i]);
> +
> + return ret;
> +}
> +
> +static int __devexit max77686_pmic_remove(struct platform_device *pdev)
> +{
> + struct max77686_data *max77686 = platform_get_drvdata(pdev);
> + struct regulator_dev **rdev = max77686->rdev;
> + int i;
> +
> + for (i = 0; i < max77686->num_regulators; i++)
> + if (rdev[i])
> + regulator_unregister(rdev[i]);
> +
> + return 0;
> +}
> +
> +static const struct platform_device_id max77686_pmic_id[] = {
> + {"max77686-pmic", 0},
> + {},
> +};
> +MODULE_DEVICE_TABLE(platform, max77686_pmic_id);
> +
> +static struct platform_driver max77686_pmic_driver = {
> + .driver = {
> + .name = "max77686-pmic",
> + .owner = THIS_MODULE,
> + },
> + .probe = max77686_pmic_probe,
> + .remove = __devexit_p(max77686_pmic_remove),
> + .id_table = max77686_pmic_id,
> +};
> +
> +static int __init max77686_pmic_init(void)
> +{
> + return platform_driver_register(&max77686_pmic_driver);
> +}
> +subsys_initcall(max77686_pmic_init);
> +
> +static void __exit max77686_pmic_cleanup(void)
> +{
> + platform_driver_unregister(&max77686_pmic_driver);
> +}
> +module_exit(max77686_pmic_cleanup);
> +
> +MODULE_DESCRIPTION("MAXIM 77686 Regulator Driver");
> +MODULE_AUTHOR("Chiwoong Byun <[email protected]>");
> +MODULE_AUTHOR("Yadwinder Singh Brar <[email protected]>");
> +MODULE_LICENSE("GPL");
MAX77686 has crystal oscillator in it. And original version of this
driver which was written by Chiwoon Byun, registers it as a regulator.
As Mark says, we have to change it to use generic clock API. Where do
you think should we put them into? In my opinion, it is proper that just
leave them in regulator driver because this driver is almost core of
PMIC. I already applied generic API in my local repository but i
couldn't test yet. Because it crashed with SOC's private clock API.
Anyway if you register 32khz clock with generic API ,DEFINE_CLK_GATE()
will help you out which defined in linux/clk-private.h.
Thanks.
Hi, again.
On 2012년 05월 22일 14:57, [email protected] wrote:
> +static __devinit int max77686_pmic_probe(struct platform_device *pdev)
> +{
> +
> + for (i = 0; i < pdata->num_regulators; i++) {
> + config.dev = max77686->dev;
> + config.init_data = pdata->regulators[i].init_data;
> + config.driver_data = max77686;
> + config.regmap = iodev->regmap;
> +
> + rdev[i] = regulator_register(®ulators[i], &config);
I'm sorry that i missed one. You have to register all regulators
unconditionally. Mark brown commented about this to my former patch.
'No, you should unconditionally register all regulators the device
physically has. This is useful for debug and simplifies the code.'
- from Mark Brown
Thanks.
(adding Kyungmin Park and Samuel Ortiz)
Hi,
Yes, It happened unintentionally. I didn't know about your patch
before submitting
the initial version of my patches. I agree with you, I will review
your patches and
will try to incorporate extra features from your patches.
On Wed, May 23, 2012 at 7:10 AM, <[email protected]> wrote:
> Hi, Yadwinder,
> As you know, both of us, recently, had competition for one driver
> whether you intend or not. And now, i think it is time to stop this and
> to find appropriate goal. From now on, i won't update this driver no
> more. I recommend you to review my patch and apply feature that you can
> apply. And also check comments that i wrote below.
>
> On 2012년 05월 22일 14:57, [email protected] wrote:
>
>> From: Yadwinder Singh Brar <[email protected]>
>>
>> Add support for PMIC/regulator portion of MAX77686 multifunction device.
>> MAX77686 provides LDOs[1-26] and BUCKs[1-9]. This is initial release of driv
>> which supports setting and getting the voltage of a regulator with I2C
>> interface.
>> + return DIV_ROUND_UP(rdev->desc->uV_step *
>> + abs(new_sel - old_sel),
>> + 100);
>> +}
>> +
>
>
> Does LDO also need waiting for voltage change? I afraid it's not.
>
Yes, according to technical reference manual which I have, ramp rate for
LDOs is also 100mV/us.
>> +
>> + if (pdata->ramp_delay < MAX77686_RAMP_RATE_13MV ||
>> + pdata->ramp_delay > MAX77686_RAMP_RATE_100MV)
>> + pdata->ramp_delay = MAX77686_RAMP_RATE_27MV; /* default */
If pdata doesn't have proper ramp_delay, it will get value
MAX77686_RAMP_RATE_27MV.
>> +
>> + max77686->ramp_delay = pdata->ramp_delay - 1;
>
>
> I think it is better to check pdata->ramp_delay is available.
> If pdata doesn't have ramp_delay member it might be error.
>
Yes, we have taken care of this case above before setting value of
max77686->ramp_delay.
>> + max77686_update_reg(i2c, MAX77686_REG_BUCK2CTRL1,
>> + max77686->ramp_delay << 6, RAMP_MASK);
>> + max77686_update_reg(i2c, MAX77686_REG_BUCK3CTRL1,
>> + max77686->ramp_delay << 6, RAMP_MASK);
>> + max77686_update_reg(i2c, MAX77686_REG_BUCK4CTRL1,
>> + max77686->ramp_delay << 6, RAMP_MASK);
>> +
>
>
> Why do you use i2c client still? If you registered regmap you can use
> its API. I recommend you to use regmap_update_bits() directly.
>
>
Yes, we are using regmap_update_bits(). max77686_update_reg() is just
a wrapper over it.
>> + platform_set_drvdata(pdev, max77686);
>> +
>
> MAX77686 has crystal oscillator in it. And original version of this
> driver which was written by Chiwoon Byun, registers it as a regulator.
> As Mark says, we have to change it to use generic clock API. Where do
> you think should we put them into? In my opinion, it is proper that just
> leave them in regulator driver because this driver is almost core of
> PMIC. I already applied generic API in my local repository but i
> couldn't test yet. Because it crashed with SOC's private clock API.
> Anyway if you register 32khz clock with generic API ,DEFINE_CLK_GATE()
> will help you out which defined in linux/clk-private.h.
>
Yes, I was also thinking about where to put it. I am not sure whether this
is a proper place to put them. Anyway I will again think about it.
Thanks,
Yadwinder.
Hi,
On Wed, May 23, 2012 at 7:20 AM, <[email protected]> wrote:
> Hi, again.
> On 2012년 05월 22일 14:57, [email protected] wrote:
>
>
>> +static __devinit int max77686_pmic_probe(struct platform_device *pdev)
>> +{
>
>> +
>> + for (i = 0; i < pdata->num_regulators; i++) {
>> + config.dev = max77686->dev;
>> + config.init_data = pdata->regulators[i].init_data;
>> + config.driver_data = max77686;
>> + config.regmap = iodev->regmap;
>> +
>> + rdev[i] = regulator_register(®ulators[i], &config);
>
>
> I'm sorry that i missed one. You have to register all regulators
> unconditionally. Mark brown commented about this to my former patch.
>
> 'No, you should unconditionally register all regulators the device
> physically has. This is useful for debug and simplifies the code.'
> - from Mark Brown
>
Yes, we are registering all regulators here.
As pdata->num_regulators will be equal to ARRAY_SIZE(regulators)
Thanks,
Yadwinder.
On 2012년 05월 23일 13:16, Yadwinder Singh Brar wrote:
>>> + max77686_update_reg(i2c, MAX77686_REG_BUCK2CTRL1,
>>> + max77686->ramp_delay << 6, RAMP_MASK);
>>> + max77686_update_reg(i2c, MAX77686_REG_BUCK3CTRL1,
>>> + max77686->ramp_delay << 6, RAMP_MASK);
>>> + max77686_update_reg(i2c, MAX77686_REG_BUCK4CTRL1,
>>> + max77686->ramp_delay << 6, RAMP_MASK);
>>> +
>>
>>
>> Why do you use i2c client still? If you registered regmap you can use
>> its API. I recommend you to use regmap_update_bits() directly.
>>
>>
>
> Yes, we are using regmap_update_bits(). max77686_update_reg() is just
> a wrapper over it.
>
Yes, i know what you mean. However it doesn't need max77686_update_reg()
any more since it uses regmap API. Why don't you just pass iodev->regmap
to regmap_update_bits(). It is clear that there is no reason for using
i2c client as a medium. Please check regulator and mfd driver of my
previous patch.
Thanks.
On Wed, May 23, 2012 at 10:10 AM, <[email protected]> wrote:
> On 2012년 05월 23일 13:16, Yadwinder Singh Brar wrote:
>
>>>> + max77686_update_reg(i2c, MAX77686_REG_BUCK2CTRL1,
>>>> + max77686->ramp_delay << 6, RAMP_MASK);
>>>> + max77686_update_reg(i2c, MAX77686_REG_BUCK3CTRL1,
>>>> + max77686->ramp_delay << 6, RAMP_MASK);
>>>> + max77686_update_reg(i2c, MAX77686_REG_BUCK4CTRL1,
>>>> + max77686->ramp_delay << 6, RAMP_MASK);
>>>> +
>>>
>>>
>>> Why do you use i2c client still? If you registered regmap you can use
>>> its API. I recommend you to use regmap_update_bits() directly.
>>>
>>>
>>
>> Yes, we are using regmap_update_bits(). max77686_update_reg() is just
>> a wrapper over it.
>>
>
>
> Yes, i know what you mean. However it doesn't need max77686_update_reg()
> any more since it uses regmap API. Why don't you just pass iodev->regmap
> to regmap_update_bits(). It is clear that there is no reason for using
> i2c client as a medium. Please check regulator and mfd driver of my
> previous patch.
>
I agree with you we can use directly regmap API. But I preferred
max77686_update_reg() because its a common practice to use
common read/write API which we define in mfd driver to access
that particular mfd device from other drivers.
Regards,
Yadwinder.
On 2012년 05월 23일 14:23, Yadwinder Singh Brar wrote:
> On Wed, May 23, 2012 at 10:10 AM, <[email protected]> wrote:
>> On 2012년 05월 23일 13:16, Yadwinder Singh Brar wrote:
>>
>>>>> + max77686_update_reg(i2c, MAX77686_REG_BUCK2CTRL1,
>>>>> + max77686->ramp_delay << 6, RAMP_MASK);
>>>>> + max77686_update_reg(i2c, MAX77686_REG_BUCK3CTRL1,
>>>>> + max77686->ramp_delay << 6, RAMP_MASK);
>>>>> + max77686_update_reg(i2c, MAX77686_REG_BUCK4CTRL1,
>>>>> + max77686->ramp_delay << 6, RAMP_MASK);
>>>>> +
>>>>
>>>>
>>>> Why do you use i2c client still? If you registered regmap you can use
>>>> its API. I recommend you to use regmap_update_bits() directly.
>>>>
>>>>
>>>
>>> Yes, we are using regmap_update_bits(). max77686_update_reg() is just
>>> a wrapper over it.
>>>
>>
>>
>> Yes, i know what you mean. However it doesn't need max77686_update_reg()
>> any more since it uses regmap API. Why don't you just pass iodev->regmap
>> to regmap_update_bits(). It is clear that there is no reason for using
>> i2c client as a medium. Please check regulator and mfd driver of my
>> previous patch.
>>
>
> I agree with you we can use directly regmap API. But I preferred
> max77686_update_reg() because its a common practice to use
> common read/write API which we define in mfd driver to access
> that particular mfd device from other drivers.
>
> Regards,
> Yadwinder.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
I inform you my mfd driver has been confirmed by Samuel Oritz and there
is no mfd private API. This situation looks unusual that we registers
mfd driver and regulator driver separately. But how should we do? For
corporation , i'm asking you to consider my suggestion.
Thanks.
Thanks.
On Wed, May 23, 2012 at 9:46 AM, Yadwinder Singh Brar
<[email protected]> wrote:
> (adding Kyungmin Park and Samuel Ortiz)
>
> Hi,
>
> Yes, It happened unintentionally. I didn't know about your patch
> before submitting
> the initial version of my patches. I agree with you, I will review
> your patches and
> will try to incorporate extra features from your patches.
>
Now I have seen your patches for mfd and regulator drivers.
Apparently, it seems that mostly we same features in our patches.
Their is no extra feature to be incorporated form your patches.
Rather I found device tree support is additional in our patches
and mainly their are some differences related to DVS_GPIO and
opmode stuff in our patches:
1- Since we are not implementing and using DVS feature
through GPIOs, so all(incomplete) stuff related to dvs_gpio
is not required currently in our mfd driver presently.
2- Since presently, we are not implementing suspend_enable/disable
callbacks in regulator driver, So we don't need opmode related
stuff now because I think regulators should come up in normal mode
only through .enable callback function.
> On Wed, May 23, 2012 at 7:10 AM, ?<[email protected]> wrote:
>> Hi, Yadwinder,
>> As you know, both of us, recently, had competition for one driver
>> whether you intend or not. And now, i think it is time to stop this and
>> to find appropriate goal. From now on, i won't update this driver no
>> more. I recommend you to review my patch and apply feature that you can
>> apply. And also check comments that i wrote below.
>>
Thanks,
Yadwinder.
On Wed, May 23, 2012 at 02:33:28PM +0900, [email protected] wrote:
> I inform you my mfd driver has been confirmed by Samuel Oritz and there
> is no mfd private API. This situation looks unusual that we registers
> mfd driver and regulator driver separately. But how should we do? For
> corporation , i'm asking you to consider my suggestion.
Given all this discussion I'm going to ignore this patch for now and
wait for a repost after you've come to an agreement.
Hi Jonghwa,
I didn't know about confirmation of your mfd driver by Samuel Oritz.
So please feel free to revise and submit your patch set.
Anyways, I am interested only in getting the driver for max77686 in mainline.
Thanks,
Yadwinder.
On Wed, May 23, 2012 at 3:48 PM, Mark Brown
<[email protected]> wrote:
> On Wed, May 23, 2012 at 02:33:28PM +0900, [email protected] wrote:
>
>> I inform you my mfd driver has been confirmed by Samuel Oritz and there
>> is no mfd private API. This situation looks unusual that we registers
>> mfd driver and regulator driver separately. But how should we do? For
>> corporation , i'm asking you to consider my suggestion.
>
> Given all this discussion I'm going to ignore this patch for now and
> wait for a repost after you've come to an agreement.