This patch set adds support for the MediaTek PMIC mt6311 regulator driver,
which adds mt6311 related buck/ldo voltage data to the driver,
and creates the regulator_desc table.
Changes in v3:
- Removed the struct mt6311 since the members of mt6311 were not be necessary.
Changes in v2:
- Re-ordered the sequence of makefile and Kconfig.
- Specifying the regulator DT bindings by providing of_match and
regulators_node in the regulator_desc and delete the DT handling code.
Henry Chen (2):
dt-bindings: regulator: mt6311: Add document for mt6311 regulator
regulator: mt6311: Add support for mt6311 regulator
.../bindings/regulator/mt6311-regulator.txt | 35 ++++
drivers/regulator/Kconfig | 9 +
drivers/regulator/Makefile | 1 +
drivers/regulator/mt6311-regulator.c | 183 +++++++++++++++++++++
drivers/regulator/mt6311-regulator.h | 65 ++++++++
include/linux/regulator/mt6311.h | 29 ++++
6 files changed, 322 insertions(+)
create mode 100644 Documentation/devicetree/bindings/regulator/mt6311-regulator.txt
create mode 100644 drivers/regulator/mt6311-regulator.c
create mode 100644 drivers/regulator/mt6311-regulator.h
create mode 100644 include/linux/regulator/mt6311.h
--
1.8.1.1.dirty
This patch adds a list of supported regulator names to the devicetree
binding documentation for Mediatek MT6311 PMIC.
Signed-off-by: Henry Chen <[email protected]>
---
.../bindings/regulator/mt6311-regulator.txt | 35 ++++++++++++++++++++++
1 file changed, 35 insertions(+)
create mode 100644 Documentation/devicetree/bindings/regulator/mt6311-regulator.txt
diff --git a/Documentation/devicetree/bindings/regulator/mt6311-regulator.txt b/Documentation/devicetree/bindings/regulator/mt6311-regulator.txt
new file mode 100644
index 0000000..9387d6d
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/mt6311-regulator.txt
@@ -0,0 +1,35 @@
+Mediatek MT6311 Regulator Driver
+
+Required properties:
+- compatible: "mediatek,mt6311-regulator"
+- reg: I2C slave address, usually 0x6b.
+- regulators: List of regulators provided by this controller. It is named
+ to VDVFS and VBIASN.
+ The definition for each of these nodes is defined using the standard binding
+ for regulators at Documentation/devicetree/bindings/regulator/regulator.txt.
+
+The valid names for regulators are:
+BUCK:
+ VDVFS
+LDO:
+ VBIASN
+
+Example:
+ pmic: mt6311@6B {
+ compatible = "mediatek,mt6311-regulator";
+ reg = <0x6B>;
+
+ regulators {
+ mt6311_vcpu_reg: VDVFS {
+ regulator-name = "VDVFS";
+ regulator-min-microvolt = < 600000>;
+ regulator-max-microvolt = <1400000>;
+ regulator-ramp-delay = <10000>;
+ };
+ mt6311_ldo_reg: VBIASN {
+ regulator-name = "VBIASN";
+ regulator-min-microvolt = <200000>;
+ regulator-max-microvolt = <800000>;
+ };
+ };
+ };
--
1.8.1.1.dirty
Add regulator support for mt6311.
It has 2 regulaotrs - Buck and LDO, provide the related buck/ldo voltage
data to the driver, and creates the regulator_desc table. Supported
operations for Buck are enabled/disabled and voltage change, only
enabled/disabled for LDO.
Signed-off-by: Henry Chen <[email protected]>
---
drivers/regulator/Kconfig | 9 ++
drivers/regulator/Makefile | 1 +
drivers/regulator/mt6311-regulator.c | 183 +++++++++++++++++++++++++++++++++++
drivers/regulator/mt6311-regulator.h | 65 +++++++++++++
include/linux/regulator/mt6311.h | 29 ++++++
5 files changed, 287 insertions(+)
create mode 100644 drivers/regulator/mt6311-regulator.c
create mode 100644 drivers/regulator/mt6311-regulator.h
create mode 100644 include/linux/regulator/mt6311.h
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index bef3bde..aab09ac 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -451,6 +451,15 @@ config REGULATOR_MC13892
Say y here to support the regulators found on the Freescale MC13892
PMIC.
+config REGULATOR_MT6311
+ tristate "MediaTek MT6311 PMIC"
+ depends on I2C
+ help
+ Say y here to select this option to enable the power regulator of
+ MediaTek MT6311 PMIC.
+ This driver supports the control of different power rails of device
+ through regulator interface.
+
config REGULATOR_MT6397
tristate "MediaTek MT6397 PMIC"
depends on MFD_MT6397
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 91bf762..45e790f 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -60,6 +60,7 @@ obj-$(CONFIG_REGULATOR_MAX77843) += max77843.o
obj-$(CONFIG_REGULATOR_MC13783) += mc13783-regulator.o
obj-$(CONFIG_REGULATOR_MC13892) += mc13892-regulator.o
obj-$(CONFIG_REGULATOR_MC13XXX_CORE) += mc13xxx-regulator-core.o
+obj-$(CONFIG_REGULATOR_MT6311) += mt6311-regulator.o
obj-$(CONFIG_REGULATOR_MT6397) += mt6397-regulator.o
obj-$(CONFIG_REGULATOR_QCOM_RPM) += qcom_rpm-regulator.o
obj-$(CONFIG_REGULATOR_QCOM_SPMI) += qcom_spmi-regulator.o
diff --git a/drivers/regulator/mt6311-regulator.c b/drivers/regulator/mt6311-regulator.c
new file mode 100644
index 0000000..3c926a2
--- /dev/null
+++ b/drivers/regulator/mt6311-regulator.c
@@ -0,0 +1,183 @@
+/*
+ * Copyright (c) 2015 MediaTek Inc.
+ * Author: Henry Chen <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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.
+ */
+
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/regulator/mt6311.h>
+#include <linux/slab.h>
+#include "mt6311-regulator.h"
+
+static const struct regmap_config mt6311_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = MT6311_FQMTR_CON4,
+};
+
+/* Default limits measured in millivolts and milliamps */
+#define MT6311_MIN_UV 600000
+#define MT6311_MAX_UV 1400000
+#define MT6311_STEP_UV 6250
+
+static const struct regulator_linear_range buck_volt_range[] = {
+ REGULATOR_LINEAR_RANGE(MT6311_MIN_UV, 0, 0x7f, MT6311_STEP_UV),
+};
+
+static struct regulator_ops mt6311_buck_ops = {
+ .list_voltage = regulator_list_voltage_linear_range,
+ .map_voltage = regulator_map_voltage_linear_range,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .set_voltage_time_sel = regulator_set_voltage_time_sel,
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+};
+
+static struct regulator_ops mt6311_ldo_ops = {
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+};
+
+#define MT6311_BUCK(_id) \
+{\
+ .name = #_id,\
+ .ops = &mt6311_buck_ops,\
+ .of_match = of_match_ptr(#_id),\
+ .regulators_node = of_match_ptr("regulators"),\
+ .type = REGULATOR_VOLTAGE,\
+ .id = MT6311_ID_##_id,\
+ .n_voltages = (MT6311_MAX_UV - MT6311_MIN_UV) / MT6311_STEP_UV + 1,\
+ .min_uV = MT6311_MIN_UV,\
+ .uV_step = MT6311_STEP_UV,\
+ .owner = THIS_MODULE,\
+ .linear_ranges = buck_volt_range, \
+ .n_linear_ranges = ARRAY_SIZE(buck_volt_range), \
+ .enable_reg = MT6311_VDVFS11_CON9,\
+ .enable_mask = MT6311_PMIC_VDVFS11_EN_MASK,\
+ .vsel_reg = MT6311_VDVFS11_CON12,\
+ .vsel_mask = MT6311_PMIC_VDVFS11_VOSEL_MASK,\
+}
+
+#define MT6311_LDO(_id) \
+{\
+ .name = #_id,\
+ .ops = &mt6311_ldo_ops,\
+ .of_match = of_match_ptr(#_id),\
+ .regulators_node = of_match_ptr("regulators"),\
+ .type = REGULATOR_VOLTAGE,\
+ .id = MT6311_ID_##_id,\
+ .owner = THIS_MODULE,\
+ .enable_reg = MT6311_LDO_CON3,\
+ .enable_mask = MT6311_PMIC_RG_VBIASN_EN_MASK,\
+}
+
+static struct regulator_desc mt6311_regulators[] = {
+ MT6311_BUCK(VDVFS),
+ MT6311_LDO(VBIASN),
+};
+
+/*
+ * I2C driver interface functions
+ */
+static int mt6311_i2c_probe(struct i2c_client *i2c,
+ const struct i2c_device_id *id)
+{
+ struct regulator_config config = { };
+ struct regulator_dev *rdev;
+ struct regmap *regmap;
+ int error, i, ret;
+ unsigned int data;
+
+ regmap = devm_regmap_init_i2c(i2c, &mt6311_regmap_config);
+ if (IS_ERR(regmap)) {
+ error = PTR_ERR(regmap);
+ dev_err(&i2c->dev, "Failed to allocate register map: %d\n",
+ error);
+ return error;
+ }
+
+ ret = regmap_read(regmap, MT6311_SWCID, &data);
+ if (ret < 0) {
+ dev_err(&i2c->dev, "Failed to read DEVICE_ID reg: %d\n", ret);
+ return ret;
+ }
+
+ switch (data) {
+ case MT6311_E1_CID_CODE:
+ case MT6311_E2_CID_CODE:
+ case MT6311_E3_CID_CODE:
+ break;
+ default:
+ dev_err(&i2c->dev, "Unsupported device id = 0x%x.\n", data);
+ return -ENODEV;
+ }
+
+ for (i = 0; i < MT6311_MAX_REGULATORS; i++) {
+ config.dev = &i2c->dev;
+ config.regmap = regmap;
+
+ rdev = devm_regulator_register(&i2c->dev,
+ &mt6311_regulators[i], &config);
+ if (IS_ERR(rdev)) {
+ dev_err(&i2c->dev,
+ "Failed to register MT6311 regulator\n");
+ return PTR_ERR(rdev);
+ }
+ }
+
+ if (ret < 0)
+ dev_err(&i2c->dev, "Failed to initialize regulator: %d\n", ret);
+
+ return ret;
+}
+
+static const struct i2c_device_id mt6311_i2c_id[] = {
+ {"mt6311", 0},
+ {},
+};
+MODULE_DEVICE_TABLE(i2c, mt6311_i2c_id);
+
+#ifdef CONFIG_OF
+static const struct of_device_id mt6311_dt_ids[] = {
+ { .compatible = "mediatek,mt6311-regulator",
+ .data = &mt6311_i2c_id[0] },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mt6311_dt_ids);
+#endif
+
+static struct i2c_driver mt6311_regulator_driver = {
+ .driver = {
+ .name = "mt6311",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(mt6311_dt_ids),
+ },
+ .probe = mt6311_i2c_probe,
+ .id_table = mt6311_i2c_id,
+};
+
+module_i2c_driver(mt6311_regulator_driver);
+
+MODULE_AUTHOR("Henry Chen <[email protected]>");
+MODULE_DESCRIPTION("Regulator device driver for Mediatek MT6311");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/regulator/mt6311-regulator.h b/drivers/regulator/mt6311-regulator.h
new file mode 100644
index 0000000..5218db4
--- /dev/null
+++ b/drivers/regulator/mt6311-regulator.h
@@ -0,0 +1,65 @@
+/*
+ * Copyright (c) 2015 MediaTek Inc.
+ * Author: Henry Chen <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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.
+ */
+
+#ifndef __MT6311_REGULATOR_H__
+#define __MT6311_REGULATOR_H__
+
+#define MT6311_SWCID 0x01
+
+#define MT6311_TOP_INT_CON 0x18
+#define MT6311_TOP_INT_MON 0x19
+
+#define MT6311_VDVFS11_CON0 0x87
+#define MT6311_VDVFS11_CON7 0x88
+#define MT6311_VDVFS11_CON8 0x89
+#define MT6311_VDVFS11_CON9 0x8A
+#define MT6311_VDVFS11_CON10 0x8B
+#define MT6311_VDVFS11_CON11 0x8C
+#define MT6311_VDVFS11_CON12 0x8D
+#define MT6311_VDVFS11_CON13 0x8E
+#define MT6311_VDVFS11_CON14 0x8F
+#define MT6311_VDVFS11_CON15 0x90
+#define MT6311_VDVFS11_CON16 0x91
+#define MT6311_VDVFS11_CON17 0x92
+#define MT6311_VDVFS11_CON18 0x93
+#define MT6311_VDVFS11_CON19 0x94
+
+#define MT6311_LDO_CON0 0xCC
+#define MT6311_LDO_OCFB0 0xCD
+#define MT6311_LDO_CON2 0xCE
+#define MT6311_LDO_CON3 0xCF
+#define MT6311_LDO_CON4 0xD0
+#define MT6311_FQMTR_CON0 0xD1
+#define MT6311_FQMTR_CON1 0xD2
+#define MT6311_FQMTR_CON2 0xD3
+#define MT6311_FQMTR_CON3 0xD4
+#define MT6311_FQMTR_CON4 0xD5
+
+#define MT6311_PMIC_RG_INT_POL_MASK 0x1
+#define MT6311_PMIC_RG_INT_EN_MASK 0x2
+#define MT6311_PMIC_RG_BUCK_OC_INT_STATUS_MASK 0x10
+
+#define MT6311_PMIC_VDVFS11_EN_CTRL_MASK 0x1
+#define MT6311_PMIC_VDVFS11_VOSEL_CTRL_MASK 0x2
+#define MT6311_PMIC_VDVFS11_EN_SEL_MASK 0x3
+#define MT6311_PMIC_VDVFS11_VOSEL_SEL_MASK 0xc
+#define MT6311_PMIC_VDVFS11_EN_MASK 0x1
+#define MT6311_PMIC_VDVFS11_VOSEL_MASK 0x7F
+#define MT6311_PMIC_VDVFS11_VOSEL_ON_MASK 0x7F
+#define MT6311_PMIC_VDVFS11_VOSEL_SLEEP_MASK 0x7F
+#define MT6311_PMIC_NI_VDVFS11_VOSEL_MASK 0x7F
+
+#define MT6311_PMIC_RG_VBIASN_EN_MASK 0x1
+
+#endif
diff --git a/include/linux/regulator/mt6311.h b/include/linux/regulator/mt6311.h
new file mode 100644
index 0000000..8473259
--- /dev/null
+++ b/include/linux/regulator/mt6311.h
@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2015 MediaTek Inc.
+ * Author: Henry Chen <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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.
+ */
+
+#ifndef __LINUX_REGULATOR_MT6311_H
+#define __LINUX_REGULATOR_MT6311_H
+
+#define MT6311_MAX_REGULATORS 2
+
+enum {
+ MT6311_ID_VDVFS = 0,
+ MT6311_ID_VBIASN,
+};
+
+#define MT6311_E1_CID_CODE 0x10
+#define MT6311_E2_CID_CODE 0x20
+#define MT6311_E3_CID_CODE 0x30
+
+#endif /* __LINUX_REGULATOR_MT6311_H */
--
1.8.1.1.dirty
Hello Henry,
On Thu, Jul 23, 2015 at 5:39 AM, Henry Chen <[email protected]> wrote:
> This patch adds a list of supported regulator names to the devicetree
> binding documentation for Mediatek MT6311 PMIC.
>
> Signed-off-by: Henry Chen <[email protected]>
> ---
> .../bindings/regulator/mt6311-regulator.txt | 35 ++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/regulator/mt6311-regulator.txt
>
> diff --git a/Documentation/devicetree/bindings/regulator/mt6311-regulator.txt b/Documentation/devicetree/bindings/regulator/mt6311-regulator.txt
> new file mode 100644
> index 0000000..9387d6d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/mt6311-regulator.txt
> @@ -0,0 +1,35 @@
> +Mediatek MT6311 Regulator Driver
> +
> +Required properties:
> +- compatible: "mediatek,mt6311-regulator"
> +- reg: I2C slave address, usually 0x6b.
> +- regulators: List of regulators provided by this controller. It is named
> + to VDVFS and VBIASN.
> + The definition for each of these nodes is defined using the standard binding
> + for regulators at Documentation/devicetree/bindings/regulator/regulator.txt.
> +
> +The valid names for regulators are:
> +BUCK:
> + VDVFS
> +LDO:
> + VBIASN
> +
> +Example:
Sorry, I missed this in the previous revision but the ePAPR document
says "the name of a node should be somewhat generic, reflecting the
function of the device and not its precise programming model."
> + pmic: mt6311@6B {
So this should be:
mt6311: pmic@
> + compatible = "mediatek,mt6311-regulator";
> + reg = <0x6B>;
> +
> + regulators {
> + mt6311_vcpu_reg: VDVFS {
> + regulator-name = "VDVFS";
> + regulator-min-microvolt = < 600000>;
> + regulator-max-microvolt = <1400000>;
> + regulator-ramp-delay = <10000>;
> + };
> + mt6311_ldo_reg: VBIASN {
> + regulator-name = "VBIASN";
> + regulator-min-microvolt = <200000>;
> + regulator-max-microvolt = <800000>;
> + };
> + };
> + };
> --
Besides the comment above, it looks good to me.
Reviewed-by: Javier Martinez Canillas <[email protected]>
Best regards,
Javier
Hello Henry,
Driver looks mostly good for me now but I just found some things I
missed in the last revision.
On Thu, Jul 23, 2015 at 5:39 AM, Henry Chen <[email protected]> wrote:
> Add regulator support for mt6311.
> It has 2 regulaotrs - Buck and LDO, provide the related buck/ldo voltage
> data to the driver, and creates the regulator_desc table. Supported
> operations for Buck are enabled/disabled and voltage change, only
> enabled/disabled for LDO.
>
> Signed-off-by: Henry Chen <[email protected]>
> ---
> drivers/regulator/Kconfig | 9 ++
> drivers/regulator/Makefile | 1 +
> drivers/regulator/mt6311-regulator.c | 183 +++++++++++++++++++++++++++++++++++
> drivers/regulator/mt6311-regulator.h | 65 +++++++++++++
> include/linux/regulator/mt6311.h | 29 ++++++
> 5 files changed, 287 insertions(+)
> create mode 100644 drivers/regulator/mt6311-regulator.c
> create mode 100644 drivers/regulator/mt6311-regulator.h
> create mode 100644 include/linux/regulator/mt6311.h
>
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index bef3bde..aab09ac 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -451,6 +451,15 @@ config REGULATOR_MC13892
> Say y here to support the regulators found on the Freescale MC13892
> PMIC.
>
> +config REGULATOR_MT6311
> + tristate "MediaTek MT6311 PMIC"
> + depends on I2C
> + help
> + Say y here to select this option to enable the power regulator of
> + MediaTek MT6311 PMIC.
> + This driver supports the control of different power rails of device
> + through regulator interface.
> +
> config REGULATOR_MT6397
> tristate "MediaTek MT6397 PMIC"
> depends on MFD_MT6397
> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index 91bf762..45e790f 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -60,6 +60,7 @@ obj-$(CONFIG_REGULATOR_MAX77843) += max77843.o
> obj-$(CONFIG_REGULATOR_MC13783) += mc13783-regulator.o
> obj-$(CONFIG_REGULATOR_MC13892) += mc13892-regulator.o
> obj-$(CONFIG_REGULATOR_MC13XXX_CORE) += mc13xxx-regulator-core.o
> +obj-$(CONFIG_REGULATOR_MT6311) += mt6311-regulator.o
> obj-$(CONFIG_REGULATOR_MT6397) += mt6397-regulator.o
> obj-$(CONFIG_REGULATOR_QCOM_RPM) += qcom_rpm-regulator.o
> obj-$(CONFIG_REGULATOR_QCOM_SPMI) += qcom_spmi-regulator.o
> diff --git a/drivers/regulator/mt6311-regulator.c b/drivers/regulator/mt6311-regulator.c
> new file mode 100644
> index 0000000..3c926a2
> --- /dev/null
> +++ b/drivers/regulator/mt6311-regulator.c
> @@ -0,0 +1,183 @@
> +/*
> + * Copyright (c) 2015 MediaTek Inc.
> + * Author: Henry Chen <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * 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.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/gpio.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/machine.h>
> +#include <linux/regulator/of_regulator.h>
> +#include <linux/regulator/mt6311.h>
> +#include <linux/slab.h>
> +#include "mt6311-regulator.h"
> +
> +static const struct regmap_config mt6311_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = MT6311_FQMTR_CON4,
> +};
> +
> +/* Default limits measured in millivolts and milliamps */
> +#define MT6311_MIN_UV 600000
> +#define MT6311_MAX_UV 1400000
> +#define MT6311_STEP_UV 6250
> +
> +static const struct regulator_linear_range buck_volt_range[] = {
> + REGULATOR_LINEAR_RANGE(MT6311_MIN_UV, 0, 0x7f, MT6311_STEP_UV),
> +};
> +
> +static struct regulator_ops mt6311_buck_ops = {
> + .list_voltage = regulator_list_voltage_linear_range,
> + .map_voltage = regulator_map_voltage_linear_range,
> + .set_voltage_sel = regulator_set_voltage_sel_regmap,
> + .get_voltage_sel = regulator_get_voltage_sel_regmap,
> + .set_voltage_time_sel = regulator_set_voltage_time_sel,
> + .enable = regulator_enable_regmap,
> + .disable = regulator_disable_regmap,
> + .is_enabled = regulator_is_enabled_regmap,
> +};
> +
> +static struct regulator_ops mt6311_ldo_ops = {
> + .enable = regulator_enable_regmap,
> + .disable = regulator_disable_regmap,
> + .is_enabled = regulator_is_enabled_regmap,
> +};
> +
> +#define MT6311_BUCK(_id) \
> +{\
> + .name = #_id,\
> + .ops = &mt6311_buck_ops,\
> + .of_match = of_match_ptr(#_id),\
> + .regulators_node = of_match_ptr("regulators"),\
> + .type = REGULATOR_VOLTAGE,\
> + .id = MT6311_ID_##_id,\
> + .n_voltages = (MT6311_MAX_UV - MT6311_MIN_UV) / MT6311_STEP_UV + 1,\
> + .min_uV = MT6311_MIN_UV,\
> + .uV_step = MT6311_STEP_UV,\
> + .owner = THIS_MODULE,\
> + .linear_ranges = buck_volt_range, \
> + .n_linear_ranges = ARRAY_SIZE(buck_volt_range), \
> + .enable_reg = MT6311_VDVFS11_CON9,\
> + .enable_mask = MT6311_PMIC_VDVFS11_EN_MASK,\
> + .vsel_reg = MT6311_VDVFS11_CON12,\
> + .vsel_mask = MT6311_PMIC_VDVFS11_VOSEL_MASK,\
> +}
> +
> +#define MT6311_LDO(_id) \
> +{\
> + .name = #_id,\
> + .ops = &mt6311_ldo_ops,\
> + .of_match = of_match_ptr(#_id),\
> + .regulators_node = of_match_ptr("regulators"),\
> + .type = REGULATOR_VOLTAGE,\
> + .id = MT6311_ID_##_id,\
> + .owner = THIS_MODULE,\
> + .enable_reg = MT6311_LDO_CON3,\
> + .enable_mask = MT6311_PMIC_RG_VBIASN_EN_MASK,\
> +}
> +
> +static struct regulator_desc mt6311_regulators[] = {
> + MT6311_BUCK(VDVFS),
> + MT6311_LDO(VBIASN),
> +};
> +
> +/*
> + * I2C driver interface functions
> + */
> +static int mt6311_i2c_probe(struct i2c_client *i2c,
> + const struct i2c_device_id *id)
> +{
> + struct regulator_config config = { };
> + struct regulator_dev *rdev;
> + struct regmap *regmap;
> + int error, i, ret;
> + unsigned int data;
> +
> + regmap = devm_regmap_init_i2c(i2c, &mt6311_regmap_config);
> + if (IS_ERR(regmap)) {
> + error = PTR_ERR(regmap);
> + dev_err(&i2c->dev, "Failed to allocate register map: %d\n",
> + error);
> + return error;
> + }
> +
> + ret = regmap_read(regmap, MT6311_SWCID, &data);
> + if (ret < 0) {
> + dev_err(&i2c->dev, "Failed to read DEVICE_ID reg: %d\n", ret);
> + return ret;
> + }
> +
> + switch (data) {
> + case MT6311_E1_CID_CODE:
> + case MT6311_E2_CID_CODE:
> + case MT6311_E3_CID_CODE:
> + break;
> + default:
> + dev_err(&i2c->dev, "Unsupported device id = 0x%x.\n", data);
> + return -ENODEV;
> + }
> +
> + for (i = 0; i < MT6311_MAX_REGULATORS; i++) {
> + config.dev = &i2c->dev;
> + config.regmap = regmap;
> +
> + rdev = devm_regulator_register(&i2c->dev,
> + &mt6311_regulators[i], &config);
> + if (IS_ERR(rdev)) {
> + dev_err(&i2c->dev,
> + "Failed to register MT6311 regulator\n");
> + return PTR_ERR(rdev);
> + }
> + }
> +
> + if (ret < 0)
> + dev_err(&i2c->dev, "Failed to initialize regulator: %d\n", ret);
> +
I don't think this is necessary, you are already adding logs for all
the error conditions.
> + return ret;
> +}
> +
> +static const struct i2c_device_id mt6311_i2c_id[] = {
> + {"mt6311", 0},
> + {},
> +};
> +MODULE_DEVICE_TABLE(i2c, mt6311_i2c_id);
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id mt6311_dt_ids[] = {
> + { .compatible = "mediatek,mt6311-regulator",
> + .data = &mt6311_i2c_id[0] },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, mt6311_dt_ids);
> +#endif
> +
> +static struct i2c_driver mt6311_regulator_driver = {
> + .driver = {
> + .name = "mt6311",
> + .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(mt6311_dt_ids),
of_match_ptr() is NULL when CONFIG_OF is not defined so you don't need
the additional #ifdef
CONFIG_OF around mt6311_dt_ids.
> + },
> + .probe = mt6311_i2c_probe,
> + .id_table = mt6311_i2c_id,
> +};
> +
> +module_i2c_driver(mt6311_regulator_driver);
> +
> +MODULE_AUTHOR("Henry Chen <[email protected]>");
> +MODULE_DESCRIPTION("Regulator device driver for Mediatek MT6311");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/regulator/mt6311-regulator.h b/drivers/regulator/mt6311-regulator.h
> new file mode 100644
> index 0000000..5218db4
> --- /dev/null
> +++ b/drivers/regulator/mt6311-regulator.h
> @@ -0,0 +1,65 @@
> +/*
> + * Copyright (c) 2015 MediaTek Inc.
> + * Author: Henry Chen <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * 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.
> + */
> +
> +#ifndef __MT6311_REGULATOR_H__
> +#define __MT6311_REGULATOR_H__
> +
> +#define MT6311_SWCID 0x01
> +
> +#define MT6311_TOP_INT_CON 0x18
> +#define MT6311_TOP_INT_MON 0x19
> +
> +#define MT6311_VDVFS11_CON0 0x87
> +#define MT6311_VDVFS11_CON7 0x88
> +#define MT6311_VDVFS11_CON8 0x89
> +#define MT6311_VDVFS11_CON9 0x8A
> +#define MT6311_VDVFS11_CON10 0x8B
> +#define MT6311_VDVFS11_CON11 0x8C
> +#define MT6311_VDVFS11_CON12 0x8D
> +#define MT6311_VDVFS11_CON13 0x8E
> +#define MT6311_VDVFS11_CON14 0x8F
> +#define MT6311_VDVFS11_CON15 0x90
> +#define MT6311_VDVFS11_CON16 0x91
> +#define MT6311_VDVFS11_CON17 0x92
> +#define MT6311_VDVFS11_CON18 0x93
> +#define MT6311_VDVFS11_CON19 0x94
> +
> +#define MT6311_LDO_CON0 0xCC
> +#define MT6311_LDO_OCFB0 0xCD
> +#define MT6311_LDO_CON2 0xCE
> +#define MT6311_LDO_CON3 0xCF
> +#define MT6311_LDO_CON4 0xD0
> +#define MT6311_FQMTR_CON0 0xD1
> +#define MT6311_FQMTR_CON1 0xD2
> +#define MT6311_FQMTR_CON2 0xD3
> +#define MT6311_FQMTR_CON3 0xD4
> +#define MT6311_FQMTR_CON4 0xD5
> +
> +#define MT6311_PMIC_RG_INT_POL_MASK 0x1
> +#define MT6311_PMIC_RG_INT_EN_MASK 0x2
> +#define MT6311_PMIC_RG_BUCK_OC_INT_STATUS_MASK 0x10
> +
> +#define MT6311_PMIC_VDVFS11_EN_CTRL_MASK 0x1
> +#define MT6311_PMIC_VDVFS11_VOSEL_CTRL_MASK 0x2
> +#define MT6311_PMIC_VDVFS11_EN_SEL_MASK 0x3
> +#define MT6311_PMIC_VDVFS11_VOSEL_SEL_MASK 0xc
> +#define MT6311_PMIC_VDVFS11_EN_MASK 0x1
> +#define MT6311_PMIC_VDVFS11_VOSEL_MASK 0x7F
> +#define MT6311_PMIC_VDVFS11_VOSEL_ON_MASK 0x7F
> +#define MT6311_PMIC_VDVFS11_VOSEL_SLEEP_MASK 0x7F
> +#define MT6311_PMIC_NI_VDVFS11_VOSEL_MASK 0x7F
> +
> +#define MT6311_PMIC_RG_VBIASN_EN_MASK 0x1
> +
> +#endif
> diff --git a/include/linux/regulator/mt6311.h b/include/linux/regulator/mt6311.h
> new file mode 100644
> index 0000000..8473259
> --- /dev/null
> +++ b/include/linux/regulator/mt6311.h
> @@ -0,0 +1,29 @@
> +/*
> + * Copyright (c) 2015 MediaTek Inc.
> + * Author: Henry Chen <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * 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.
> + */
> +
> +#ifndef __LINUX_REGULATOR_MT6311_H
> +#define __LINUX_REGULATOR_MT6311_H
> +
> +#define MT6311_MAX_REGULATORS 2
> +
> +enum {
> + MT6311_ID_VDVFS = 0,
> + MT6311_ID_VBIASN,
> +};
> +
> +#define MT6311_E1_CID_CODE 0x10
> +#define MT6311_E2_CID_CODE 0x20
> +#define MT6311_E3_CID_CODE 0x30
> +
> +#endif /* __LINUX_REGULATOR_MT6311_H */
> --
Besides those comments, it looks good to me now.
Reviewed-by: Javier Martinez Canillas <[email protected]>
Best regards,
Javier
On Thu, 2015-07-23 at 11:07 +0200, Javier Martinez Canillas wrote:
> > +
> > + if (ret < 0)
> > + dev_err(&i2c->dev, "Failed to initialize regulator: %d\n", ret);
> > +
>
> I don't think this is necessary, you are already adding logs for all
> the error conditions.
Yes, I will remove it.
>
> > + return ret;
> > +}
> > +
> > +static const struct i2c_device_id mt6311_i2c_id[] = {
> > + {"mt6311", 0},
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(i2c, mt6311_i2c_id);
> > +
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id mt6311_dt_ids[] = {
> > + { .compatible = "mediatek,mt6311-regulator",
> > + .data = &mt6311_i2c_id[0] },
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(of, mt6311_dt_ids);
> > +#endif
> > +
> > +static struct i2c_driver mt6311_regulator_driver = {
> > + .driver = {
> > + .name = "mt6311",
> > + .owner = THIS_MODULE,
> > + .of_match_table = of_match_ptr(mt6311_dt_ids),
>
> of_match_ptr() is NULL when CONFIG_OF is not defined so you don't need
> the additional #ifdef
> CONFIG_OF around mt6311_dt_ids.
If CONFIG_OF is not defined and without #ifdef CONFIG_OF around
mt6311_dt_ids, it will allocate a struct mt6311_dt_ids but no one used
on here, right?
Henry
> > + },
> > + .probe = mt6311_i2c_probe,
> > + .id_table = mt6311_i2c_id,
> > +};
> > +
> > +module_i2c_driver(mt6311_regulator_driver);
> > +
> > +MODULE_AUTHOR("Henry Chen <[email protected]>");
> > +MODULE_DESCRIPTION("Regulator device driver for Mediatek MT6311");
> > +MODULE_LICENSE("GPL v2");
> > diff --git a/drivers/regulator/mt6311-regulator.h b/drivers/regulator/mt6311-regulator.h
> > new file mode 100644
> > index 0000000..5218db4
> > --- /dev/null
> > +++ b/drivers/regulator/mt6311-regulator.h
> > @@ -0,0 +1,65 @@
> > +/*
> > + * Copyright (c) 2015 MediaTek Inc.
> > + * Author: Henry Chen <[email protected]>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * 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.
> > + */
> > +
> > +#ifndef __MT6311_REGULATOR_H__
> > +#define __MT6311_REGULATOR_H__
> > +
> > +#define MT6311_SWCID 0x01
> > +
> > +#define MT6311_TOP_INT_CON 0x18
> > +#define MT6311_TOP_INT_MON 0x19
> > +
> > +#define MT6311_VDVFS11_CON0 0x87
> > +#define MT6311_VDVFS11_CON7 0x88
> > +#define MT6311_VDVFS11_CON8 0x89
> > +#define MT6311_VDVFS11_CON9 0x8A
> > +#define MT6311_VDVFS11_CON10 0x8B
> > +#define MT6311_VDVFS11_CON11 0x8C
> > +#define MT6311_VDVFS11_CON12 0x8D
> > +#define MT6311_VDVFS11_CON13 0x8E
> > +#define MT6311_VDVFS11_CON14 0x8F
> > +#define MT6311_VDVFS11_CON15 0x90
> > +#define MT6311_VDVFS11_CON16 0x91
> > +#define MT6311_VDVFS11_CON17 0x92
> > +#define MT6311_VDVFS11_CON18 0x93
> > +#define MT6311_VDVFS11_CON19 0x94
> > +
> > +#define MT6311_LDO_CON0 0xCC
> > +#define MT6311_LDO_OCFB0 0xCD
> > +#define MT6311_LDO_CON2 0xCE
> > +#define MT6311_LDO_CON3 0xCF
> > +#define MT6311_LDO_CON4 0xD0
> > +#define MT6311_FQMTR_CON0 0xD1
> > +#define MT6311_FQMTR_CON1 0xD2
> > +#define MT6311_FQMTR_CON2 0xD3
> > +#define MT6311_FQMTR_CON3 0xD4
> > +#define MT6311_FQMTR_CON4 0xD5
> > +
> > +#define MT6311_PMIC_RG_INT_POL_MASK 0x1
> > +#define MT6311_PMIC_RG_INT_EN_MASK 0x2
> > +#define MT6311_PMIC_RG_BUCK_OC_INT_STATUS_MASK 0x10
> > +
> > +#define MT6311_PMIC_VDVFS11_EN_CTRL_MASK 0x1
> > +#define MT6311_PMIC_VDVFS11_VOSEL_CTRL_MASK 0x2
> > +#define MT6311_PMIC_VDVFS11_EN_SEL_MASK 0x3
> > +#define MT6311_PMIC_VDVFS11_VOSEL_SEL_MASK 0xc
> > +#define MT6311_PMIC_VDVFS11_EN_MASK 0x1
> > +#define MT6311_PMIC_VDVFS11_VOSEL_MASK 0x7F
> > +#define MT6311_PMIC_VDVFS11_VOSEL_ON_MASK 0x7F
> > +#define MT6311_PMIC_VDVFS11_VOSEL_SLEEP_MASK 0x7F
> > +#define MT6311_PMIC_NI_VDVFS11_VOSEL_MASK 0x7F
> > +
> > +#define MT6311_PMIC_RG_VBIASN_EN_MASK 0x1
> > +
> > +#endif
> > diff --git a/include/linux/regulator/mt6311.h b/include/linux/regulator/mt6311.h
> > new file mode 100644
> > index 0000000..8473259
> > --- /dev/null
> > +++ b/include/linux/regulator/mt6311.h
> > @@ -0,0 +1,29 @@
> > +/*
> > + * Copyright (c) 2015 MediaTek Inc.
> > + * Author: Henry Chen <[email protected]>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * 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.
> > + */
> > +
> > +#ifndef __LINUX_REGULATOR_MT6311_H
> > +#define __LINUX_REGULATOR_MT6311_H
> > +
> > +#define MT6311_MAX_REGULATORS 2
> > +
> > +enum {
> > + MT6311_ID_VDVFS = 0,
> > + MT6311_ID_VBIASN,
> > +};
> > +
> > +#define MT6311_E1_CID_CODE 0x10
> > +#define MT6311_E2_CID_CODE 0x20
> > +#define MT6311_E3_CID_CODE 0x30
> > +
> > +#endif /* __LINUX_REGULATOR_MT6311_H */
> > --
>
> Besides those comments, it looks good to me now.
>
> Reviewed-by: Javier Martinez Canillas <[email protected]>
>
> Best regards,
> Javier
Hi Henry,
On Thu, Jul 23, 2015 at 7:04 PM, Henry Chen <[email protected]> wrote:
>
> On Thu, 2015-07-23 at 11:07 +0200, Javier Martinez Canillas wrote:
>
> > > +
> > > + if (ret < 0)
> > > + dev_err(&i2c->dev, "Failed to initialize regulator: %d\n", ret);
> > > +
> >
> > I don't think this is necessary, you are already adding logs for all
> > the error conditions.
>
> Yes, I will remove it.
>
> >
> > > + return ret;
> > > +}
> > > +
> > > +static const struct i2c_device_id mt6311_i2c_id[] = {
> > > + {"mt6311", 0},
> > > + {},
> > > +};
> > > +MODULE_DEVICE_TABLE(i2c, mt6311_i2c_id);
> > > +
> > > +#ifdef CONFIG_OF
> > > +static const struct of_device_id mt6311_dt_ids[] = {
> > > + { .compatible = "mediatek,mt6311-regulator",
> > > + .data = &mt6311_i2c_id[0] },
> > > + {},
> > > +};
> > > +MODULE_DEVICE_TABLE(of, mt6311_dt_ids);
> > > +#endif
> > > +
> > > +static struct i2c_driver mt6311_regulator_driver = {
> > > + .driver = {
> > > + .name = "mt6311",
> > > + .owner = THIS_MODULE,
> > > + .of_match_table = of_match_ptr(mt6311_dt_ids),
> >
> > of_match_ptr() is NULL when CONFIG_OF is not defined so you don't need
> > the additional #ifdef
> > CONFIG_OF around mt6311_dt_ids.
>
> If CONFIG_OF is not defined and without #ifdef CONFIG_OF around
> mt6311_dt_ids, it will allocate a struct mt6311_dt_ids but no one used
> on here, right?
Correct, I agree with Henry that it is better to keep the #ifdef
around mt6311_dt_ids table.
-Daniel
>
> Henry
>
> > > + },
> > > + .probe = mt6311_i2c_probe,
> > > + .id_table = mt6311_i2c_id,
> > > +};
> > > +
> > > +module_i2c_driver(mt6311_regulator_driver);
> > > +
> > > +MODULE_AUTHOR("Henry Chen <[email protected]>");
> > > +MODULE_DESCRIPTION("Regulator device driver for Mediatek MT6311");
> > > +MODULE_LICENSE("GPL v2");
> > > diff --git a/drivers/regulator/mt6311-regulator.h b/drivers/regulator/mt6311-regulator.h
> > > new file mode 100644
> > > index 0000000..5218db4
> > > --- /dev/null
> > > +++ b/drivers/regulator/mt6311-regulator.h
> > > @@ -0,0 +1,65 @@
> > > +/*
> > > + * Copyright (c) 2015 MediaTek Inc.
> > > + * Author: Henry Chen <[email protected]>
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License version 2 as
> > > + * published by the Free Software Foundation.
> > > + *
> > > + * 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.
> > > + */
> > > +
> > > +#ifndef __MT6311_REGULATOR_H__
> > > +#define __MT6311_REGULATOR_H__
> > > +
> > > +#define MT6311_SWCID 0x01
> > > +
> > > +#define MT6311_TOP_INT_CON 0x18
> > > +#define MT6311_TOP_INT_MON 0x19
> > > +
> > > +#define MT6311_VDVFS11_CON0 0x87
> > > +#define MT6311_VDVFS11_CON7 0x88
> > > +#define MT6311_VDVFS11_CON8 0x89
> > > +#define MT6311_VDVFS11_CON9 0x8A
> > > +#define MT6311_VDVFS11_CON10 0x8B
> > > +#define MT6311_VDVFS11_CON11 0x8C
> > > +#define MT6311_VDVFS11_CON12 0x8D
> > > +#define MT6311_VDVFS11_CON13 0x8E
> > > +#define MT6311_VDVFS11_CON14 0x8F
> > > +#define MT6311_VDVFS11_CON15 0x90
> > > +#define MT6311_VDVFS11_CON16 0x91
> > > +#define MT6311_VDVFS11_CON17 0x92
> > > +#define MT6311_VDVFS11_CON18 0x93
> > > +#define MT6311_VDVFS11_CON19 0x94
> > > +
> > > +#define MT6311_LDO_CON0 0xCC
> > > +#define MT6311_LDO_OCFB0 0xCD
> > > +#define MT6311_LDO_CON2 0xCE
> > > +#define MT6311_LDO_CON3 0xCF
> > > +#define MT6311_LDO_CON4 0xD0
> > > +#define MT6311_FQMTR_CON0 0xD1
> > > +#define MT6311_FQMTR_CON1 0xD2
> > > +#define MT6311_FQMTR_CON2 0xD3
> > > +#define MT6311_FQMTR_CON3 0xD4
> > > +#define MT6311_FQMTR_CON4 0xD5
> > > +
> > > +#define MT6311_PMIC_RG_INT_POL_MASK 0x1
> > > +#define MT6311_PMIC_RG_INT_EN_MASK 0x2
> > > +#define MT6311_PMIC_RG_BUCK_OC_INT_STATUS_MASK 0x10
> > > +
> > > +#define MT6311_PMIC_VDVFS11_EN_CTRL_MASK 0x1
> > > +#define MT6311_PMIC_VDVFS11_VOSEL_CTRL_MASK 0x2
> > > +#define MT6311_PMIC_VDVFS11_EN_SEL_MASK 0x3
> > > +#define MT6311_PMIC_VDVFS11_VOSEL_SEL_MASK 0xc
> > > +#define MT6311_PMIC_VDVFS11_EN_MASK 0x1
> > > +#define MT6311_PMIC_VDVFS11_VOSEL_MASK 0x7F
> > > +#define MT6311_PMIC_VDVFS11_VOSEL_ON_MASK 0x7F
> > > +#define MT6311_PMIC_VDVFS11_VOSEL_SLEEP_MASK 0x7F
> > > +#define MT6311_PMIC_NI_VDVFS11_VOSEL_MASK 0x7F
> > > +
> > > +#define MT6311_PMIC_RG_VBIASN_EN_MASK 0x1
> > > +
> > > +#endif
> > > diff --git a/include/linux/regulator/mt6311.h b/include/linux/regulator/mt6311.h
> > > new file mode 100644
> > > index 0000000..8473259
> > > --- /dev/null
> > > +++ b/include/linux/regulator/mt6311.h
> > > @@ -0,0 +1,29 @@
> > > +/*
> > > + * Copyright (c) 2015 MediaTek Inc.
> > > + * Author: Henry Chen <[email protected]>
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License version 2 as
> > > + * published by the Free Software Foundation.
> > > + *
> > > + * 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.
> > > + */
> > > +
> > > +#ifndef __LINUX_REGULATOR_MT6311_H
> > > +#define __LINUX_REGULATOR_MT6311_H
> > > +
> > > +#define MT6311_MAX_REGULATORS 2
> > > +
> > > +enum {
> > > + MT6311_ID_VDVFS = 0,
> > > + MT6311_ID_VBIASN,
> > > +};
> > > +
> > > +#define MT6311_E1_CID_CODE 0x10
> > > +#define MT6311_E2_CID_CODE 0x20
> > > +#define MT6311_E3_CID_CODE 0x30
> > > +
> > > +#endif /* __LINUX_REGULATOR_MT6311_H */
> > > --
> >
> > Besides those comments, it looks good to me now.
> >
> > Reviewed-by: Javier Martinez Canillas <[email protected]>
> >
> > Best regards,
> > Javier
>
>
> --
> 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/
Hello Henry,
On Thu, Jul 23, 2015 at 1:04 PM, Henry Chen <[email protected]> wrote:
> On Thu, 2015-07-23 at 11:07 +0200, Javier Martinez Canillas wrote:
>
>> > +
>> > + if (ret < 0)
>> > + dev_err(&i2c->dev, "Failed to initialize regulator: %d\n", ret);
>> > +
>>
>> I don't think this is necessary, you are already adding logs for all
>> the error conditions.
>
> Yes, I will remove it.
>
>>
>> > + return ret;
>> > +}
>> > +
>> > +static const struct i2c_device_id mt6311_i2c_id[] = {
>> > + {"mt6311", 0},
>> > + {},
>> > +};
>> > +MODULE_DEVICE_TABLE(i2c, mt6311_i2c_id);
>> > +
>> > +#ifdef CONFIG_OF
>> > +static const struct of_device_id mt6311_dt_ids[] = {
>> > + { .compatible = "mediatek,mt6311-regulator",
>> > + .data = &mt6311_i2c_id[0] },
>> > + {},
>> > +};
>> > +MODULE_DEVICE_TABLE(of, mt6311_dt_ids);
>> > +#endif
>> > +
>> > +static struct i2c_driver mt6311_regulator_driver = {
>> > + .driver = {
>> > + .name = "mt6311",
>> > + .owner = THIS_MODULE,
>> > + .of_match_table = of_match_ptr(mt6311_dt_ids),
>>
>> of_match_ptr() is NULL when CONFIG_OF is not defined so you don't need
>> the additional #ifdef
>> CONFIG_OF around mt6311_dt_ids.
>
> If CONFIG_OF is not defined and without #ifdef CONFIG_OF around
> mt6311_dt_ids, it will allocate a struct mt6311_dt_ids but no one used
> on here, right?
>
You are right, sorry for the noise.
> Henry
>
Best regards,
Javier
Hi Javier,
On Thu, 2015-07-23 at 10:43 +0200, Javier Martinez Canillas wrote:
> Hello Henry,
>
> On Thu, Jul 23, 2015 at 5:39 AM, Henry Chen <[email protected]> wrote:
> > This patch adds a list of supported regulator names to the devicetree
> > binding documentation for Mediatek MT6311 PMIC.
> >
> > Signed-off-by: Henry Chen <[email protected]>
> > ---
> > .../bindings/regulator/mt6311-regulator.txt | 35 ++++++++++++++++++++++
> > 1 file changed, 35 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/regulator/mt6311-regulator.txt
> >
> > diff --git a/Documentation/devicetree/bindings/regulator/mt6311-regulator.txt b/Documentation/devicetree/bindings/regulator/mt6311-regulator.txt
> > new file mode 100644
> > index 0000000..9387d6d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/regulator/mt6311-regulator.txt
> > @@ -0,0 +1,35 @@
> > +Mediatek MT6311 Regulator Driver
> > +
> > +Required properties:
> > +- compatible: "mediatek,mt6311-regulator"
> > +- reg: I2C slave address, usually 0x6b.
> > +- regulators: List of regulators provided by this controller. It is named
> > + to VDVFS and VBIASN.
> > + The definition for each of these nodes is defined using the standard binding
> > + for regulators at Documentation/devicetree/bindings/regulator/regulator.txt.
> > +
> > +The valid names for regulators are:
> > +BUCK:
> > + VDVFS
> > +LDO:
> > + VBIASN
> > +
> > +Example:
>
> Sorry, I missed this in the previous revision but the ePAPR document
> says "the name of a node should be somewhat generic, reflecting the
> function of the device and not its precise programming model."
>
> > + pmic: mt6311@6B {
>
> So this should be:
>
> mt6311: pmic@
Ok, I will change it, thanks for reviewing.
Henry
>
> > + compatible = "mediatek,mt6311-regulator";
> > + reg = <0x6B>;
> > +
> > + regulators {
> > + mt6311_vcpu_reg: VDVFS {
> > + regulator-name = "VDVFS";
> > + regulator-min-microvolt = < 600000>;
> > + regulator-max-microvolt = <1400000>;
> > + regulator-ramp-delay = <10000>;
> > + };
> > + mt6311_ldo_reg: VBIASN {
> > + regulator-name = "VBIASN";
> > + regulator-min-microvolt = <200000>;
> > + regulator-max-microvolt = <800000>;
> > + };
> > + };
> > + };
> > --
>
> Besides the comment above, it looks good to me.
>
> Reviewed-by: Javier Martinez Canillas <[email protected]>
>
> Best regards,
> Javier