This is the REGULATOR component driver of the Dialog DA9058 PMIC.
This driver is just one component of the whole DA9058 PMIC driver.
It depends on the CORE component driver of the DA9058 MFD.
There are 6 warnings from scripts/checkpatch.pl, but since it seems to
be complaining about variable names such as min_uV are in CamelCase,
when it is obvious that they are not in CamelCase I have ignored them.
Signed-off-by: Anthony Olech <[email protected]>
Signed-off-by: David Dajun Chen <[email protected]>
---
drivers/regulator/Kconfig | 11 ++
drivers/regulator/Makefile | 1 +
drivers/regulator/da9058-regulator.c | 228 ++++++++++++++++++++++++++++++++++
3 files changed, 240 insertions(+)
create mode 100644 drivers/regulator/da9058-regulator.c
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index a5d97ea..3b7b098 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -64,6 +64,17 @@ config REGULATOR_USERSPACE_CONSUMER
If unsure, say no.
+config REGULATOR_DA9058
+ tristate "Support regulators on Dialog Semiconductor DA9058 PMIC"
+ depends on MFD_DA9058
+ help
+ Say y here to support the BUCKs and LDOs regulators found on
+ Dialog Semiconductor DA9058 PMIC.
+
+ This driver can also be built as a module. If so, the module
+ will be called da9058-regulator.
+
+
config REGULATOR_GPIO
tristate "GPIO regulator support"
depends on GENERIC_GPIO
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 6e82503..f8e1784 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_REGULATOR_DB8500_PRCMU) += db8500-prcmu.o
obj-$(CONFIG_REGULATOR_FAN53555) += fan53555.o
obj-$(CONFIG_REGULATOR_GPIO) += gpio-regulator.o
obj-$(CONFIG_REGULATOR_ISL6271A) += isl6271a-regulator.o
+obj-$(CONFIG_REGULATOR_DA9058) += da9058-regulator.o
obj-$(CONFIG_REGULATOR_LP3971) += lp3971.o
obj-$(CONFIG_REGULATOR_LP3972) += lp3972.o
obj-$(CONFIG_REGULATOR_LP872X) += lp872x.o
diff --git a/drivers/regulator/da9058-regulator.c b/drivers/regulator/da9058-regulator.c
new file mode 100644
index 0000000..a033364
--- /dev/null
+++ b/drivers/regulator/da9058-regulator.c
@@ -0,0 +1,228 @@
+/*
+ * Copyright (C) 2012 Dialog Semiconductor Ltd.
+ *
+ * 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.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/driver.h>
+#include <linux/regmap.h>
+#include <linux/mfd/core.h>
+
+#include <linux/mfd/da9058/version.h>
+#include <linux/mfd/da9058/registers.h>
+#include <linux/mfd/da9058/core.h>
+#include <linux/mfd/da9058/regulator.h>
+
+struct da9058_regulator {
+ struct da9058 *da9058;
+ int ramp_register;
+ int ramp_enable_mask;
+ struct platform_device *pdev;
+ struct regulator_dev *reg_dev;
+ struct regulator_desc desc;
+ struct regulator_init_data init;
+};
+
+static int da9058_buck_ramp_voltage(struct regulator_dev *rdev,
+ unsigned int old_selector,
+ unsigned int new_selector)
+{
+ struct da9058_regulator *regulator = rdev_get_drvdata(rdev);
+ struct da9058 *da9058 = regulator->da9058;
+ int ret;
+
+ if (regulator->ramp_register == 0)
+ return -EINVAL;
+
+ if (regulator->ramp_enable_mask == 0)
+ return -EINVAL;
+
+ ret = da9058_set_bits(da9058, regulator->ramp_register,
+ regulator->ramp_enable_mask);
+
+ if (ret)
+ return ret;
+
+ return 2200; /* micro Seconds needed to ramp to new voltage*/
+}
+
+static struct regulator_ops da9058_buck_regulator_ops = {
+ .map_voltage = regulator_map_voltage_linear,
+ .list_voltage = regulator_list_voltage_linear,
+ .set_voltage_time_sel = da9058_buck_ramp_voltage,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+};
+
+static struct regulator_ops da9058_ldo_regulator_ops = {
+ .map_voltage = regulator_map_voltage_linear,
+ .list_voltage = regulator_list_voltage_linear,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+};
+
+static int da9058_regulator_probe(struct platform_device *pdev)
+{
+ struct da9058 *da9058 = dev_get_drvdata(pdev->dev.parent);
+ const struct mfd_cell *cell = mfd_get_cell(pdev);
+ struct da9058_regulator_pdata *rpdata;
+ struct da9058_regulator *reg;
+ struct regulator_dev *rdev;
+ struct regulator_config config = { };
+ int ret;
+ unsigned int val;
+
+ if (cell == NULL) {
+ ret = -ENODEV;
+ goto exit;
+ }
+
+ rpdata = cell->platform_data;
+
+ if (rpdata == NULL) {
+ ret = -EINVAL;
+ goto exit;
+ }
+
+ if (rpdata->control_register == 0) {
+ ret = -EINVAL;
+ goto exit;
+ }
+
+ if (rpdata->control_enable_mask == 0) {
+ ret = -EINVAL;
+ goto exit;
+ }
+
+ reg = devm_kzalloc(&pdev->dev, sizeof(struct da9058_regulator),
+ GFP_KERNEL);
+ if (!reg) {
+ ret = -ENOMEM;
+ goto exit;
+ }
+
+ platform_set_drvdata(pdev, reg);
+
+ reg->da9058 = da9058;
+ reg->pdev = pdev;
+ reg->ramp_register = rpdata->ramp_register;
+ reg->ramp_enable_mask = rpdata->ramp_enable_mask;
+
+ reg->desc.name = rpdata->regulator_name;
+ reg->desc.id = rpdata->regulator_id;
+ reg->desc.type = REGULATOR_VOLTAGE;
+ reg->desc.n_voltages = 1;
+
+ if (rpdata->control_voltage_step > 0)
+ reg->desc.n_voltages += (rpdata->max_uV - rpdata->min_uV) /
+ rpdata->control_voltage_step;
+
+ reg->desc.owner = THIS_MODULE;
+ reg->desc.enable_reg = rpdata->control_register;
+ reg->desc.enable_mask = rpdata->control_enable_mask;
+
+ reg->desc.vsel_reg = rpdata->control_register;
+ reg->desc.vsel_mask = rpdata->control_step_mask;
+ reg->desc.min_uV = rpdata->min_uV;
+ reg->desc.uV_step = rpdata->control_voltage_step;
+
+ if (reg->ramp_register)
+ reg->desc.ops = &da9058_buck_regulator_ops;
+ else
+ reg->desc.ops = &da9058_ldo_regulator_ops;
+
+ reg->init.constraints.name = rpdata->regulator_name;
+ reg->init.constraints.min_uV = rpdata->min_uV;
+ reg->init.constraints.max_uV = rpdata->max_uV;
+ reg->init.constraints.valid_ops_mask = rpdata->valid_ops_mask;
+ reg->init.constraints.valid_modes_mask = rpdata->valid_modes_mask;
+ reg->init.constraints.boot_on = rpdata->boot_on;
+ reg->init.constraints.always_on = rpdata->always_on;
+ reg->init.num_consumer_supplies = rpdata->num_consumer_supplies;
+ reg->init.consumer_supplies = rpdata->consumer_supplies;
+
+ config.dev = pdev->dev.parent;
+ config.init_data = ®->init;
+ config.driver_data = reg;
+ config.regmap = da9058->regmap;
+
+ rdev = regulator_register(®->desc, &config);
+
+ if (IS_ERR(rdev)) {
+ dev_err(&pdev->dev, "failed to register %s\n",
+ rpdata->regulator_name);
+ ret = PTR_ERR(rdev);
+ goto failed_to_register;
+ }
+ reg->reg_dev = rdev;
+
+ /* before we do anything check the lock bit */
+ ret = da9058_reg_read(da9058, DA9058_SUPPLY_REG, &val);
+ if (ret)
+ goto unlock_failed;
+
+ if (val & DA9058_SUPPLY_VLOCK)
+ ret = da9058_clear_bits(da9058, DA9058_SUPPLY_REG,
+ DA9058_SUPPLY_VLOCK);
+ if (ret)
+ goto unlock_failed;
+
+ goto exit;
+
+unlock_failed:
+ regulator_unregister(rdev);
+failed_to_register:
+ platform_set_drvdata(pdev, NULL);
+exit:
+ return ret;
+}
+
+static int da9058_regulator_remove(struct platform_device *pdev)
+{
+ struct regulator_dev *rdev = platform_get_drvdata(pdev);
+
+ regulator_unregister(rdev);
+
+ return 0;
+}
+
+static struct platform_driver da9058_regulator_driver = {
+ .probe = da9058_regulator_probe,
+ .remove = da9058_regulator_remove,
+ .driver = {
+ .name = "da9058-regulator",
+ .owner = THIS_MODULE,
+ },
+};
+
+static int __init da9058_regulator_init(void)
+{
+ return platform_driver_register(&da9058_regulator_driver);
+}
+
+subsys_initcall(da9058_regulator_init);
+
+static void __exit da9058_regulator_exit(void)
+{
+ platform_driver_unregister(&da9058_regulator_driver);
+}
+
+module_exit(da9058_regulator_exit);
+
+MODULE_DESCRIPTION("Dialog DA9058 PMIC voltage and current regulator");
+MODULE_AUTHOR("Anthony Olech <[email protected]>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:da9058-regulator");
--
end-of-patch for NEW DRIVER V4
On Fri, Apr 12, 2013 at 02:05:28PM +0100, Anthony Olech wrote:
This looks good, I assume there's some dependencies on the MFD or other
earlier patches so I won't apply it, let me know if there aren't any and
I will:
Acked-by: Mark Brown <[email protected]>
Please use subject lines reflecting the subsystem.
> +static int da9058_buck_ramp_voltage(struct regulator_dev *rdev,
> + unsigned int old_selector,
> + unsigned int new_selector)
> +{
> + struct da9058_regulator *regulator = rdev_get_drvdata(rdev);
> + struct da9058 *da9058 = regulator->da9058;
> + int ret;
> +
> + if (regulator->ramp_register == 0)
> + return -EINVAL;
> +
> + if (regulator->ramp_enable_mask == 0)
> + return -EINVAL;
> +
> + ret = da9058_set_bits(da9058, regulator->ramp_register,
> + regulator->ramp_enable_mask);
> +
> + if (ret)
> + return ret;
> +
> + return 2200; /* micro Seconds needed to ramp to new voltage*/
> +}
Hrm, this really should be implementable with a generic regmap
operation...
> + rdev = regulator_register(®->desc, &config);
> +
> + if (IS_ERR(rdev)) {
> + dev_err(&pdev->dev, "failed to register %s\n",
> + rpdata->regulator_name);
> + ret = PTR_ERR(rdev);
> + goto failed_to_register;
> + }
In general it's a bit better style to print out the return value to help
with diagnosis but it's no big deal.
On Fri, Apr 12, 2013 at 02:05:28PM +0100, Anthony Olech wrote:
> This is the REGULATOR component driver of the Dialog DA9058 PMIC.
> This driver is just one component of the whole DA9058 PMIC driver.
> It depends on the CORE component driver of the DA9058 MFD.
>
> There are 6 warnings from scripts/checkpatch.pl, but since it seems to
> be complaining about variable names such as min_uV are in CamelCase,
> when it is obvious that they are not in CamelCase I have ignored them.
>
??? min_uV _is_ CamelCase ???
Ok, maybe it is camelcasE, but you are splitting hairs here.
Guenter
> Signed-off-by: Anthony Olech <[email protected]>
> Signed-off-by: David Dajun Chen <[email protected]>
> ---
> drivers/regulator/Kconfig | 11 ++
> drivers/regulator/Makefile | 1 +
> drivers/regulator/da9058-regulator.c | 228 ++++++++++++++++++++++++++++++++++
> 3 files changed, 240 insertions(+)
> create mode 100644 drivers/regulator/da9058-regulator.c
>
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index a5d97ea..3b7b098 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -64,6 +64,17 @@ config REGULATOR_USERSPACE_CONSUMER
>
> If unsure, say no.
>
> +config REGULATOR_DA9058
> + tristate "Support regulators on Dialog Semiconductor DA9058 PMIC"
> + depends on MFD_DA9058
> + help
> + Say y here to support the BUCKs and LDOs regulators found on
> + Dialog Semiconductor DA9058 PMIC.
> +
> + This driver can also be built as a module. If so, the module
> + will be called da9058-regulator.
> +
> +
> config REGULATOR_GPIO
> tristate "GPIO regulator support"
> depends on GENERIC_GPIO
> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index 6e82503..f8e1784 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_REGULATOR_DB8500_PRCMU) += db8500-prcmu.o
> obj-$(CONFIG_REGULATOR_FAN53555) += fan53555.o
> obj-$(CONFIG_REGULATOR_GPIO) += gpio-regulator.o
> obj-$(CONFIG_REGULATOR_ISL6271A) += isl6271a-regulator.o
> +obj-$(CONFIG_REGULATOR_DA9058) += da9058-regulator.o
> obj-$(CONFIG_REGULATOR_LP3971) += lp3971.o
> obj-$(CONFIG_REGULATOR_LP3972) += lp3972.o
> obj-$(CONFIG_REGULATOR_LP872X) += lp872x.o
> diff --git a/drivers/regulator/da9058-regulator.c b/drivers/regulator/da9058-regulator.c
> new file mode 100644
> index 0000000..a033364
> --- /dev/null
> +++ b/drivers/regulator/da9058-regulator.c
> @@ -0,0 +1,228 @@
> +/*
> + * Copyright (C) 2012 Dialog Semiconductor Ltd.
> + *
> + * 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.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/err.h>
> +#include <linux/regulator/machine.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/core.h>
> +
> +#include <linux/mfd/da9058/version.h>
> +#include <linux/mfd/da9058/registers.h>
> +#include <linux/mfd/da9058/core.h>
> +#include <linux/mfd/da9058/regulator.h>
> +
> +struct da9058_regulator {
> + struct da9058 *da9058;
> + int ramp_register;
> + int ramp_enable_mask;
> + struct platform_device *pdev;
> + struct regulator_dev *reg_dev;
> + struct regulator_desc desc;
> + struct regulator_init_data init;
> +};
> +
> +static int da9058_buck_ramp_voltage(struct regulator_dev *rdev,
> + unsigned int old_selector,
> + unsigned int new_selector)
> +{
> + struct da9058_regulator *regulator = rdev_get_drvdata(rdev);
> + struct da9058 *da9058 = regulator->da9058;
> + int ret;
> +
> + if (regulator->ramp_register == 0)
> + return -EINVAL;
> +
> + if (regulator->ramp_enable_mask == 0)
> + return -EINVAL;
> +
> + ret = da9058_set_bits(da9058, regulator->ramp_register,
> + regulator->ramp_enable_mask);
> +
> + if (ret)
> + return ret;
> +
> + return 2200; /* micro Seconds needed to ramp to new voltage*/
> +}
> +
> +static struct regulator_ops da9058_buck_regulator_ops = {
> + .map_voltage = regulator_map_voltage_linear,
> + .list_voltage = regulator_list_voltage_linear,
> + .set_voltage_time_sel = da9058_buck_ramp_voltage,
> + .get_voltage_sel = regulator_get_voltage_sel_regmap,
> + .set_voltage_sel = regulator_set_voltage_sel_regmap,
> + .enable = regulator_enable_regmap,
> + .disable = regulator_disable_regmap,
> + .is_enabled = regulator_is_enabled_regmap,
> +};
> +
> +static struct regulator_ops da9058_ldo_regulator_ops = {
> + .map_voltage = regulator_map_voltage_linear,
> + .list_voltage = regulator_list_voltage_linear,
> + .get_voltage_sel = regulator_get_voltage_sel_regmap,
> + .set_voltage_sel = regulator_set_voltage_sel_regmap,
> + .enable = regulator_enable_regmap,
> + .disable = regulator_disable_regmap,
> + .is_enabled = regulator_is_enabled_regmap,
> +};
> +
> +static int da9058_regulator_probe(struct platform_device *pdev)
> +{
> + struct da9058 *da9058 = dev_get_drvdata(pdev->dev.parent);
> + const struct mfd_cell *cell = mfd_get_cell(pdev);
> + struct da9058_regulator_pdata *rpdata;
> + struct da9058_regulator *reg;
> + struct regulator_dev *rdev;
> + struct regulator_config config = { };
> + int ret;
> + unsigned int val;
> +
> + if (cell == NULL) {
> + ret = -ENODEV;
> + goto exit;
> + }
> +
> + rpdata = cell->platform_data;
> +
> + if (rpdata == NULL) {
> + ret = -EINVAL;
> + goto exit;
> + }
> +
> + if (rpdata->control_register == 0) {
> + ret = -EINVAL;
> + goto exit;
> + }
> +
> + if (rpdata->control_enable_mask == 0) {
> + ret = -EINVAL;
> + goto exit;
> + }
> +
> + reg = devm_kzalloc(&pdev->dev, sizeof(struct da9058_regulator),
> + GFP_KERNEL);
> + if (!reg) {
> + ret = -ENOMEM;
> + goto exit;
> + }
> +
> + platform_set_drvdata(pdev, reg);
> +
> + reg->da9058 = da9058;
> + reg->pdev = pdev;
> + reg->ramp_register = rpdata->ramp_register;
> + reg->ramp_enable_mask = rpdata->ramp_enable_mask;
> +
> + reg->desc.name = rpdata->regulator_name;
> + reg->desc.id = rpdata->regulator_id;
> + reg->desc.type = REGULATOR_VOLTAGE;
> + reg->desc.n_voltages = 1;
> +
> + if (rpdata->control_voltage_step > 0)
> + reg->desc.n_voltages += (rpdata->max_uV - rpdata->min_uV) /
> + rpdata->control_voltage_step;
> +
> + reg->desc.owner = THIS_MODULE;
> + reg->desc.enable_reg = rpdata->control_register;
> + reg->desc.enable_mask = rpdata->control_enable_mask;
> +
> + reg->desc.vsel_reg = rpdata->control_register;
> + reg->desc.vsel_mask = rpdata->control_step_mask;
> + reg->desc.min_uV = rpdata->min_uV;
> + reg->desc.uV_step = rpdata->control_voltage_step;
> +
> + if (reg->ramp_register)
> + reg->desc.ops = &da9058_buck_regulator_ops;
> + else
> + reg->desc.ops = &da9058_ldo_regulator_ops;
> +
> + reg->init.constraints.name = rpdata->regulator_name;
> + reg->init.constraints.min_uV = rpdata->min_uV;
> + reg->init.constraints.max_uV = rpdata->max_uV;
> + reg->init.constraints.valid_ops_mask = rpdata->valid_ops_mask;
> + reg->init.constraints.valid_modes_mask = rpdata->valid_modes_mask;
> + reg->init.constraints.boot_on = rpdata->boot_on;
> + reg->init.constraints.always_on = rpdata->always_on;
> + reg->init.num_consumer_supplies = rpdata->num_consumer_supplies;
> + reg->init.consumer_supplies = rpdata->consumer_supplies;
> +
> + config.dev = pdev->dev.parent;
> + config.init_data = ®->init;
> + config.driver_data = reg;
> + config.regmap = da9058->regmap;
> +
> + rdev = regulator_register(®->desc, &config);
> +
> + if (IS_ERR(rdev)) {
> + dev_err(&pdev->dev, "failed to register %s\n",
> + rpdata->regulator_name);
> + ret = PTR_ERR(rdev);
> + goto failed_to_register;
> + }
> + reg->reg_dev = rdev;
> +
> + /* before we do anything check the lock bit */
> + ret = da9058_reg_read(da9058, DA9058_SUPPLY_REG, &val);
> + if (ret)
> + goto unlock_failed;
> +
> + if (val & DA9058_SUPPLY_VLOCK)
> + ret = da9058_clear_bits(da9058, DA9058_SUPPLY_REG,
> + DA9058_SUPPLY_VLOCK);
> + if (ret)
> + goto unlock_failed;
> +
> + goto exit;
> +
> +unlock_failed:
> + regulator_unregister(rdev);
> +failed_to_register:
> + platform_set_drvdata(pdev, NULL);
> +exit:
> + return ret;
> +}
> +
> +static int da9058_regulator_remove(struct platform_device *pdev)
> +{
> + struct regulator_dev *rdev = platform_get_drvdata(pdev);
> +
> + regulator_unregister(rdev);
> +
> + return 0;
> +}
> +
> +static struct platform_driver da9058_regulator_driver = {
> + .probe = da9058_regulator_probe,
> + .remove = da9058_regulator_remove,
> + .driver = {
> + .name = "da9058-regulator",
> + .owner = THIS_MODULE,
> + },
> +};
> +
> +static int __init da9058_regulator_init(void)
> +{
> + return platform_driver_register(&da9058_regulator_driver);
> +}
> +
> +subsys_initcall(da9058_regulator_init);
> +
> +static void __exit da9058_regulator_exit(void)
> +{
> + platform_driver_unregister(&da9058_regulator_driver);
> +}
> +
> +module_exit(da9058_regulator_exit);
> +
> +MODULE_DESCRIPTION("Dialog DA9058 PMIC voltage and current regulator");
> +MODULE_AUTHOR("Anthony Olech <[email protected]>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:da9058-regulator");
> --
> end-of-patch for NEW DRIVER V4
>
>
> -----Original Message-----
> From: Guenter Roeck [mailto:[email protected]]
> Sent: 12 April 2013 14:32
> To: Opensource [Anthony Olech]
> Cc: Mark Brown; Liam Girdwood; Jean Delvare; Randy Dunlap; LKML; David
> Dajun Chen
> Subject: Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver
>
> On Fri, Apr 12, 2013 at 02:05:28PM +0100, Anthony Olech wrote:
> > This is the REGULATOR component driver of the Dialog DA9058 PMIC.
> > This driver is just one component of the whole DA9058 PMIC driver.
> > It depends on the CORE component driver of the DA9058 MFD.
> >
> > There are 6 warnings from scripts/checkpatch.pl, but since it seems to
> > be complaining about variable names such as min_uV are in CamelCase,
> > when it is obvious that they are not in CamelCase I have ignored them.
> >
> ??? min_uV _is_ CamelCase ???
>
> Ok, maybe it is camelcasE, but you are splitting hairs here.
it is not me splitting hairs, it is scripts/checkpatch.pl
Tony Olech
On Mon, Apr 15, 2013 at 03:00:58PM +0000, Opensource [Anthony Olech] wrote:
>
>
> > -----Original Message-----
> > From: Guenter Roeck [mailto:[email protected]]
> > Sent: 12 April 2013 14:32
> > To: Opensource [Anthony Olech]
> > Cc: Mark Brown; Liam Girdwood; Jean Delvare; Randy Dunlap; LKML; David
> > Dajun Chen
> > Subject: Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver
> >
> > On Fri, Apr 12, 2013 at 02:05:28PM +0100, Anthony Olech wrote:
> > > This is the REGULATOR component driver of the Dialog DA9058 PMIC.
> > > This driver is just one component of the whole DA9058 PMIC driver.
> > > It depends on the CORE component driver of the DA9058 MFD.
> > >
> > > There are 6 warnings from scripts/checkpatch.pl, but since it seems to
> > > be complaining about variable names such as min_uV are in CamelCase,
> > > when it is obvious that they are not in CamelCase I have ignored them.
> > >
> > ??? min_uV _is_ CamelCase ???
> >
> > Ok, maybe it is camelcasE, but you are splitting hairs here.
>
> it is not me splitting hairs, it is scripts/checkpatch.pl
>
Maybe you did not understand what I meant. Per your logic,
MicroVolt is CamelCase
uVolt is ???
uV is not CamelCase
By abbreviating CamelCase to camelCase to cC you make it,
in your opinion, acceptable.
If you want to declare CamelCase variables, just do it,
but don't claim that they are not really CamelCase.
Guenter
> -----Original Message-----
> From: Guenter Roeck [mailto:[email protected]]
> Sent: 15 April 2013 17:36
> To: Opensource [Anthony Olech]
> Cc: LKML
> Subject: Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver
>
> On Mon, Apr 15, 2013 at 03:00:58PM +0000, Opensource [Anthony Olech]
> wrote:
> >
> >
> > > -----Original Message-----
> > > From: Guenter Roeck [mailto:[email protected]]
> > > Sent: 12 April 2013 14:32
> > > To: Opensource [Anthony Olech]
> > > Cc: Mark Brown; Liam Girdwood; Jean Delvare; Randy Dunlap; LKML;
> > > David Dajun Chen
> > > Subject: Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver
> > >
> > > On Fri, Apr 12, 2013 at 02:05:28PM +0100, Anthony Olech wrote:
> > > > This is the REGULATOR component driver of the Dialog DA9058 PMIC.
> > > > This driver is just one component of the whole DA9058 PMIC driver.
> > > > It depends on the CORE component driver of the DA9058 MFD.
> > > >
> > > > There are 6 warnings from scripts/checkpatch.pl, but since it
> > > > seems to be complaining about variable names such as min_uV are in
> > > > CamelCase, when it is obvious that they are not in CamelCase I have
> ignored them.
> > > >
> > > ??? min_uV _is_ CamelCase ???
> > >
> > > Ok, maybe it is camelcasE, but you are splitting hairs here.
> >
> > it is not me splitting hairs, it is scripts/checkpatch.pl
> >
> Maybe you did not understand what I meant. Per your logic,
>
> MicroVolt is CamelCase
> uVolt is ???
> uV is not CamelCase
>
> By abbreviating CamelCase to camelCase to cC you make it, in your opinion,
> acceptable.
>
> If you want to declare CamelCase variables, just do it, but don't claim that they
> are not really CamelCase.
>
> Guenter
I always thought that camel case meant "changing from lower case to upper case the first
letter of each word and then joining the capitalized words together", so by that definition
uV or mW are not camel case because "v" and "w" are not words!
Either way it seems that the algorithm in scripts/checkpatch.pl is wrong! and that was my point.
Tony Olech
On Mon, Apr 15, 2013 at 05:29:13PM +0000, Opensource [Anthony Olech] wrote:
> > -----Original Message-----
> > From: Guenter Roeck [mailto:[email protected]]
> > Sent: 15 April 2013 17:36
> > To: Opensource [Anthony Olech]
> > Cc: LKML
> > Subject: Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver
> >
> > On Mon, Apr 15, 2013 at 03:00:58PM +0000, Opensource [Anthony Olech]
> > wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Guenter Roeck [mailto:[email protected]]
> > > > Sent: 12 April 2013 14:32
> > > > To: Opensource [Anthony Olech]
> > > > Cc: Mark Brown; Liam Girdwood; Jean Delvare; Randy Dunlap; LKML;
> > > > David Dajun Chen
> > > > Subject: Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver
> > > >
> > > > On Fri, Apr 12, 2013 at 02:05:28PM +0100, Anthony Olech wrote:
> > > > > This is the REGULATOR component driver of the Dialog DA9058 PMIC.
> > > > > This driver is just one component of the whole DA9058 PMIC driver.
> > > > > It depends on the CORE component driver of the DA9058 MFD.
> > > > >
> > > > > There are 6 warnings from scripts/checkpatch.pl, but since it
> > > > > seems to be complaining about variable names such as min_uV are in
> > > > > CamelCase, when it is obvious that they are not in CamelCase I have
> > ignored them.
> > > > >
> > > > ??? min_uV _is_ CamelCase ???
> > > >
> > > > Ok, maybe it is camelcasE, but you are splitting hairs here.
> > >
> > > it is not me splitting hairs, it is scripts/checkpatch.pl
> > >
> > Maybe you did not understand what I meant. Per your logic,
> >
> > MicroVolt is CamelCase
> > uVolt is ???
> > uV is not CamelCase
> >
> > By abbreviating CamelCase to camelCase to cC you make it, in your opinion,
> > acceptable.
> >
> > If you want to declare CamelCase variables, just do it, but don't claim that they
> > are not really CamelCase.
> >
> > Guenter
>
> I always thought that camel case meant "changing from lower case to upper case the first
> letter of each word and then joining the capitalized words together", so by that definition
> uV or mW are not camel case because "v" and "w" are not words!
>
> Either way it seems that the algorithm in scripts/checkpatch.pl is wrong! and that was my point.
>
Guess we'll have to agree to disagree here, as I happen to think that checkpatch
is perfectly right.
Guenter
> -----Original Message-----
> From: Guenter Roeck [mailto:[email protected]]
> Sent: 15 April 2013 18:46
> To: Opensource [Anthony Olech]
> Cc: LKML
> Subject: Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver
>
> On Mon, Apr 15, 2013 at 05:29:13PM +0000, Opensource [Anthony Olech]
> wrote:
> > > -----Original Message-----
> > > From: Guenter Roeck [mailto:[email protected]]
> > > Sent: 15 April 2013 17:36
> > > To: Opensource [Anthony Olech]
> > > Cc: LKML
> > > Subject: Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver
> > >
> > > On Mon, Apr 15, 2013 at 03:00:58PM +0000, Opensource [Anthony Olech]
> > > wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Guenter Roeck [mailto:[email protected]]
> > > > > Sent: 12 April 2013 14:32
> > > > > To: Opensource [Anthony Olech]
> > > > > Cc: Mark Brown; Liam Girdwood; Jean Delvare; Randy Dunlap; LKML;
> > > > > David Dajun Chen
> > > > > Subject: Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver
> > > > >
> > > > > On Fri, Apr 12, 2013 at 02:05:28PM +0100, Anthony Olech wrote:
> > > > > > This is the REGULATOR component driver of the Dialog DA9058 PMIC.
> > > > > > This driver is just one component of the whole DA9058 PMIC driver.
> > > > > > It depends on the CORE component driver of the DA9058 MFD.
> > > > > >
> > > > > > There are 6 warnings from scripts/checkpatch.pl, but since it
> > > > > > seems to be complaining about variable names such as min_uV
> > > > > > are in CamelCase, when it is obvious that they are not in
> > > > > > CamelCase I have
> > > ignored them.
> > > > > >
> > > > > ??? min_uV _is_ CamelCase ???
> > > > >
> > > > > Ok, maybe it is camelcasE, but you are splitting hairs here.
> > > >
> > > > it is not me splitting hairs, it is scripts/checkpatch.pl
> > > >
> > > Maybe you did not understand what I meant. Per your logic,
> > >
> > > MicroVolt is CamelCase
> > > uVolt is ???
> > > uV is not CamelCase
> > >
> > > By abbreviating CamelCase to camelCase to cC you make it, in your
> > > opinion, acceptable.
> > >
> > > If you want to declare CamelCase variables, just do it, but don't
> > > claim that they are not really CamelCase.
> > >
> > > Guenter
> >
> > I always thought that camel case meant "changing from lower case to
> > upper case the first letter of each word and then joining the
> > capitalized words together", so by that definition uV or mW are not camel
> case because "v" and "w" are not words!
The definition of CamelCase From Wikipedia, the free encyclopedia is:
"CamelCase (camel case) is a term which refers to the practice of writing
compound words where the first letter of an identifier is lowercase and the
first letter of each subsequent concatenated word is capitalized."
> > Either way it seems that the algorithm in scripts/checkpatch.pl is wrong! and
> that was my point.
>
> Guess we'll have to agree to disagree here, as I happen to think that
> checkpatch is perfectly right.
> Guenter
Hi Guenter,
I am quite happy to accept the algorithm in scripts/checkpatch.pl as the
arbiter for correctly formed linux kernel variable names.
On that basis "old_mV", "new_uA" etc are incorrectly formed variable names.
Could you possibly suggest legal alternatives to "mA", "uV", "kW" ??
Tony Olech
> -----Original Message-----
> From: Mark Brown [mailto:[email protected]]
> Sent: 12 April 2013 14:27
> To: Opensource [Anthony Olech]
> Cc: Liam Girdwood; Guenter Roeck; Jean Delvare; Randy Dunlap; LKML; David
> Dajun Chen
> Subject: Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver
> On Fri, Apr 12, 2013 at 02:05:28PM +0100, Anthony Olech wrote:
> This looks good, I assume there's some dependencies on the MFD or other
> earlier patches so I won't apply it, let me know if there aren't any and I will
I will resubmit later after re-testing, fixing the 3 points you raised in this driver.
> Acked-by: Mark Brown <[email protected]>
> Please use subject lines reflecting the subsystem.
I assume you mean "regulator" for drivers in "drivers/regulator",
"misc" for drivers in "drivers/input/misc" etc etc ??
> > +static int da9058_buck_ramp_voltage(struct regulator_dev *rdev,
> > + unsigned int old_selector,
> > + unsigned int new_selector)
> > +{
> > + struct da9058_regulator *regulator = rdev_get_drvdata(rdev);
> > + struct da9058 *da9058 = regulator->da9058;
> > + int ret;
> > +
> > + if (regulator->ramp_register == 0)
> > + return -EINVAL;
> > +
> > + if (regulator->ramp_enable_mask == 0)
> > + return -EINVAL;
> > +
> > + ret = da9058_set_bits(da9058, regulator->ramp_register,
> > + regulator->ramp_enable_mask);
> > +
> > + if (ret)
> > + return ret;
> > +
> > + return 2200; /* micro Seconds needed to ramp to new voltage*/ }
>
> Hrm, this really should be implementable with a generic regmap operation...
Yes, I did not notice that the generic regmap operation "reulator_set_voltage_sel_regmap()"
supports a trigger/go. I will change to using it.
> > + rdev = regulator_register(®->desc, &config);
> > +
> > + if (IS_ERR(rdev)) {
> > + dev_err(&pdev->dev, "failed to register %s\n",
> > + rpdata->regulator_name);
> > + ret = PTR_ERR(rdev);
> > + goto failed_to_register;
> > + }
>
> In general it's a bit better style to print out the return value to help with
> diagnosis but it's no big deal.
Yes, I will include the value of "ret" in the dev_err message
Tony Olech
On Tue, Apr 16, 2013 at 09:17:27AM +0000, Opensource [Anthony Olech] wrote:
> > -----Original Message-----
> > From: Guenter Roeck [mailto:[email protected]]
> > Sent: 15 April 2013 18:46
> > To: Opensource [Anthony Olech]
> > Cc: LKML
> > Subject: Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver
> >
> > On Mon, Apr 15, 2013 at 05:29:13PM +0000, Opensource [Anthony Olech]
> > wrote:
> > > > -----Original Message-----
> > > > From: Guenter Roeck [mailto:[email protected]]
> > > > Sent: 15 April 2013 17:36
> > > > To: Opensource [Anthony Olech]
> > > > Cc: LKML
> > > > Subject: Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver
> > > >
> > > > On Mon, Apr 15, 2013 at 03:00:58PM +0000, Opensource [Anthony Olech]
> > > > wrote:
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Guenter Roeck [mailto:[email protected]]
> > > > > > Sent: 12 April 2013 14:32
> > > > > > To: Opensource [Anthony Olech]
> > > > > > Cc: Mark Brown; Liam Girdwood; Jean Delvare; Randy Dunlap; LKML;
> > > > > > David Dajun Chen
> > > > > > Subject: Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver
> > > > > >
> > > > > > On Fri, Apr 12, 2013 at 02:05:28PM +0100, Anthony Olech wrote:
> > > > > > > This is the REGULATOR component driver of the Dialog DA9058 PMIC.
> > > > > > > This driver is just one component of the whole DA9058 PMIC driver.
> > > > > > > It depends on the CORE component driver of the DA9058 MFD.
> > > > > > >
> > > > > > > There are 6 warnings from scripts/checkpatch.pl, but since it
> > > > > > > seems to be complaining about variable names such as min_uV
> > > > > > > are in CamelCase, when it is obvious that they are not in
> > > > > > > CamelCase I have
> > > > ignored them.
> > > > > > >
> > > > > > ??? min_uV _is_ CamelCase ???
> > > > > >
> > > > > > Ok, maybe it is camelcasE, but you are splitting hairs here.
> > > > >
> > > > > it is not me splitting hairs, it is scripts/checkpatch.pl
> > > > >
> > > > Maybe you did not understand what I meant. Per your logic,
> > > >
> > > > MicroVolt is CamelCase
> > > > uVolt is ???
> > > > uV is not CamelCase
> > > >
> > > > By abbreviating CamelCase to camelCase to cC you make it, in your
> > > > opinion, acceptable.
> > > >
> > > > If you want to declare CamelCase variables, just do it, but don't
> > > > claim that they are not really CamelCase.
> > > >
> > > > Guenter
> > >
> > > I always thought that camel case meant "changing from lower case to
> > > upper case the first letter of each word and then joining the
> > > capitalized words together", so by that definition uV or mW are not camel
> > case because "v" and "w" are not words!
>
> The definition of CamelCase From Wikipedia, the free encyclopedia is:
>
> "CamelCase (camel case) is a term which refers to the practice of writing
> compound words where the first letter of an identifier is lowercase and the
> first letter of each subsequent concatenated word is capitalized."
>
Maybe the rule should read "don't mix lowercase and uppercase letters in
variable names and defines" to prevent variable names such as cAmelcAse or
cameLcasE, which would be permitted with your logic :).
> > > Either way it seems that the algorithm in scripts/checkpatch.pl is wrong! and
> > that was my point.
> >
> > Guess we'll have to agree to disagree here, as I happen to think that
> > checkpatch is perfectly right.
> > Guenter
>
> Hi Guenter,
>
> I am quite happy to accept the algorithm in scripts/checkpatch.pl as the
> arbiter for correctly formed linux kernel variable names.
>
> On that basis "old_mV", "new_uA" etc are incorrectly formed variable names.
> Could you possibly suggest legal alternatives to "mA", "uV", "kW" ??
>
I just changed it to lowercase in the ntc_thermistor driver. What you use is
really your call as long as it does not mix uppercase and lowercase letters.
Guenter
> -----Original Message-----
> From: Guenter Roeck [mailto:[email protected]]
> Sent: 16 April 2013 14:36
> To: Opensource [Anthony Olech]
> Cc: LKML
> Subject: Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver
>
> On Tue, Apr 16, 2013 at 09:17:27AM +0000, Opensource [Anthony Olech]
> wrote:
> > > -----Original Message-----
> > > From: Guenter Roeck [mailto:[email protected]]
> > > Sent: 15 April 2013 18:46
> > > To: Opensource [Anthony Olech]
> > > Cc: LKML
> > > Subject: Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver
> > >
> > > On Mon, Apr 15, 2013 at 05:29:13PM +0000, Opensource [Anthony Olech]
> > > wrote:
> > > > > -----Original Message-----
> > > > > From: Guenter Roeck [mailto:[email protected]]
> > > > > Sent: 15 April 2013 17:36
> > > > > To: Opensource [Anthony Olech]
> > > > > Cc: LKML
> > > > > Subject: Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver
> > > > >
> > > > > On Mon, Apr 15, 2013 at 03:00:58PM +0000, Opensource [Anthony
> > > > > Olech]
> > > > > wrote:
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Guenter Roeck [mailto:[email protected]]
> > > > > > > Sent: 12 April 2013 14:32
> > > > > > > To: Opensource [Anthony Olech]
> > > > > > > Cc: Mark Brown; Liam Girdwood; Jean Delvare; Randy Dunlap;
> > > > > > > LKML; David Dajun Chen
> > > > > > > Subject: Re: [NEW DRIVER V4 7/7] DA9058 REGULATOR driver
> > > > > > >
> > > > > > > On Fri, Apr 12, 2013 at 02:05:28PM +0100, Anthony Olech wrote:
> > > > > > > > This is the REGULATOR component driver of the Dialog DA9058
> PMIC.
> > > > > > > > This driver is just one component of the whole DA9058 PMIC
> driver.
> > > > > > > > It depends on the CORE component driver of the DA9058 MFD.
> > > > > > > >
> > > > > > > > There are 6 warnings from scripts/checkpatch.pl, but since
> > > > > > > > it seems to be complaining about variable names such as
> > > > > > > > min_uV are in CamelCase, when it is obvious that they are
> > > > > > > > not in CamelCase I have
> > > > > ignored them.
> > > > > > > >
> > > > > > > ??? min_uV _is_ CamelCase ???
> > > > > > >
> > > > > > > Ok, maybe it is camelcasE, but you are splitting hairs here.
> > > > > >
> > > > > > it is not me splitting hairs, it is scripts/checkpatch.pl
> > > > > >
> > > > > Maybe you did not understand what I meant. Per your logic,
> > > > >
> > > > > MicroVolt is CamelCase
> > > > > uVolt is ???
> > > > > uV is not CamelCase
> > > > >
> > > > > By abbreviating CamelCase to camelCase to cC you make it, in
> > > > > your opinion, acceptable.
> > > > >
> > > > > If you want to declare CamelCase variables, just do it, but
> > > > > don't claim that they are not really CamelCase.
> > > > >
> > > > > Guenter
> > > >
> > > > I always thought that camel case meant "changing from lower case
> > > > to upper case the first letter of each word and then joining the
> > > > capitalized words together", so by that definition uV or mW are
> > > > not camel
> > > case because "v" and "w" are not words!
> >
> > The definition of CamelCase From Wikipedia, the free encyclopedia is:
> >
> > "CamelCase (camel case) is a term which refers to the practice of
> > writing compound words where the first letter of an identifier is
> > lowercase and the first letter of each subsequent concatenated word is
> capitalized."
> >
> Maybe the rule should read "don't mix lowercase and uppercase letters in
> variable names and defines" to prevent variable names such as cAmelcAse or
> cameLcasE, which would be permitted with your logic :).
It is really good to have a definition that anyone can work with!
> > > > Either way it seems that the algorithm in scripts/checkpatch.pl is
> > > > wrong! and
> > > that was my point.
> > >
> > > Guess we'll have to agree to disagree here, as I happen to think
> > > that checkpatch is perfectly right.
> > > Guenter
> >
> > Hi Guenter,
> >
> > I am quite happy to accept the algorithm in scripts/checkpatch.pl as
> > the arbiter for correctly formed linux kernel variable names.
> >
> > On that basis "old_mV", "new_uA" etc are incorrectly formed variable names.
> > Could you possibly suggest legal alternatives to "mA", "uV", "kW" ??
> >
> I just changed it to lowercase in the ntc_thermistor driver. What you use is
> really your call as long as it does not mix uppercase and lowercase letters.
>
> Guenter
many thanks, I will do the same.
Tony Olech