[PATCH 8/9] regulator: bd71837: BD71837 PMIC regulator driver

[PATCH 8/9] regulator: bd71837: BD71837 PMIC regulator driver

[Matti Vaittinen]

Support for controlling the 8 bucks and 7 LDOs the PMIC contains.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
 drivers/regulator/bd71837-regulator.c | 683 ++++++++++++++++++++++++++++++++++
 1 file changed, 683 insertions(+)
 create mode 100644 drivers/regulator/bd71837-regulator.c

diff --git a/drivers/regulator/bd71837-regulator.c b/drivers/regulator/bd71837-regulator.c
new file mode 100644
index 000000000000..e6c3fa770755
--- /dev/null
+++ b/drivers/regulator/bd71837-regulator.c
@@ -0,0 +1,683 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2018 ROHM Semiconductors */
+/*
+ * bd71837-regulator.c ROHM BD71837MWV regulator driver
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+#include <linux/gpio.h>
+#include <linux/mfd/bd71837.h>
+#include <linux/regulator/of_regulator.h>
+
+struct bd71837_pmic {
+	struct regulator_desc descs[BD71837_REGULATOR_CNT];
+	struct bd71837 *mfd;
+	struct platform_device *pdev;
+	struct regulator_dev *rdev[BD71837_REGULATOR_CNT];
+	struct mutex mtx;
+};
+
+/*
+ * BUCK1/2/3/4
+ * BUCK1RAMPRATE[1:0] BUCK1 DVS ramp rate setting
+ * 00: 10.00mV/usec 10mV 1uS
+ * 01: 5.00mV/usec	10mV 2uS
+ * 10: 2.50mV/usec	10mV 4uS
+ * 11: 1.25mV/usec	10mV 8uS
+ */
+static int bd71837_buck1234_set_ramp_delay(struct regulator_dev *rdev,
+					   int ramp_delay)
+{
+	struct bd71837_pmic *pmic = rdev_get_drvdata(rdev);
+	struct bd71837 *mfd = pmic->mfd;
+	int id = rdev->desc->id;
+	unsigned int ramp_value = BUCK1_RAMPRATE_10P00MV;
+
+	dev_dbg(&(pmic->pdev->dev), "Buck[%d] Set Ramp = %d\n", id + 1,
+		ramp_delay);
+	switch (ramp_delay) {
+	case 1 ... 1250:
+		ramp_value = BUCK1_RAMPRATE_1P25MV;
+		break;
+	case 1251 ... 2500:
+		ramp_value = BUCK1_RAMPRATE_2P50MV;
+		break;
+	case 2501 ... 5000:
+		ramp_value = BUCK1_RAMPRATE_5P00MV;
+		break;
+	case 5001 ... 10000:
+		ramp_value = BUCK1_RAMPRATE_10P00MV;
+		break;
+	default:
+		ramp_value = BUCK1_RAMPRATE_10P00MV;
+		dev_err(&pmic->pdev->dev,
+			"%s: ramp_delay: %d not supported, setting 10000mV//us\n",
+			rdev->desc->name, ramp_delay);
+	}
+
+	return regmap_update_bits(mfd->regmap, BD71837_REG_BUCK1_CTRL + id,
+				  BUCK1_RAMPRATE_MASK, ramp_value << 6);
+}
+
+static int bd71837_regulator_set_regmap(struct regulator_dev *rdev, int set)
+{
+	int ret = -EINVAL;
+	struct bd71837_pmic *pmic = rdev->reg_data;
+
+	if (pmic) {
+		mutex_lock(&pmic->mtx);
+		if (!set)
+			ret = regulator_disable_regmap(rdev);
+		else
+			ret = regulator_enable_regmap(rdev);
+		mutex_unlock(&pmic->mtx);
+	}
+	return ret;
+}
+
+static int bd71837_regulator_enable_regmap(struct regulator_dev *rdev)
+{
+	return bd71837_regulator_set_regmap(rdev, 1);
+}
+
+static int bd71837_regulator_disable_regmap(struct regulator_dev *rdev)
+{
+	return bd71837_regulator_set_regmap(rdev, 0);
+}
+
+static int bd71837_regulator_set_voltage_sel_regmap(struct regulator_dev *rdev,
+						    unsigned int sel)
+{
+	int ret = -EINVAL;
+	struct bd71837_pmic *pmic = rdev->reg_data;
+	int do_disable;
+
+	if (pmic) {
+		mutex_lock(&pmic->mtx);
+		ret = regulator_is_enabled_regmap(rdev);
+		if (!ret || 1 == ret) {
+			do_disable = ret;
+			ret = 0;
+			if (do_disable)
+				ret = regulator_disable_regmap(rdev);
+			if (!ret) {
+				ret =
+				    regulator_set_voltage_sel_regmap(rdev, sel);
+				if (do_disable)
+					ret = regulator_enable_regmap(rdev);
+			}
+		}
+		mutex_unlock(&pmic->mtx);
+	}
+	return ret;
+}
+
+static struct regulator_ops bd71837_ldo_regulator_ops = {
+	.enable = bd71837_regulator_enable_regmap,
+	.disable = bd71837_regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.list_voltage = regulator_list_voltage_linear_range,
+	.set_voltage_sel = bd71837_regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+};
+
+static struct regulator_ops bd71837_ldo_regulator_nolinear_ops = {
+	.enable = bd71837_regulator_enable_regmap,
+	.disable = bd71837_regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.list_voltage = regulator_list_voltage_table,
+	.set_voltage_sel = bd71837_regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+};
+
+static struct regulator_ops bd71837_buck_regulator_ops = {
+	.enable = bd71837_regulator_enable_regmap,
+	.disable = bd71837_regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.list_voltage = regulator_list_voltage_linear_range,
+	.set_voltage_sel = bd71837_regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_voltage_time_sel = regulator_set_voltage_time_sel,
+};
+
+static struct regulator_ops bd71837_buck_regulator_nolinear_ops = {
+	.enable = bd71837_regulator_enable_regmap,
+	.disable = bd71837_regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.list_voltage = regulator_list_voltage_table,
+	.set_voltage_sel = bd71837_regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_voltage_time_sel = regulator_set_voltage_time_sel,
+};
+
+static struct regulator_ops bd71837_buck1234_regulator_ops = {
+	.enable = bd71837_regulator_enable_regmap,
+	.disable = bd71837_regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.list_voltage = regulator_list_voltage_linear_range,
+	.set_voltage_sel = bd71837_regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_voltage_time_sel = regulator_set_voltage_time_sel,
+	.set_ramp_delay = bd71837_buck1234_set_ramp_delay,
+};
+
+/*
+ * BUCK1/2/3/4
+ * 0.70 to 1.30V (10mV step)
+ */
+static const struct regulator_linear_range bd71837_buck1234_voltage_ranges[] = {
+	REGULATOR_LINEAR_RANGE(700000, 0x00, 0x3C, 10000),
+	REGULATOR_LINEAR_RANGE(1300000, 0x3D, 0x3F, 0),
+};
+
+/*
+ * BUCK5
+ * 0.9V to 1.35V ()
+ */
+static const struct regulator_linear_range bd71837_buck5_voltage_ranges[] = {
+	REGULATOR_LINEAR_RANGE(700000, 0x00, 0x03, 100000),
+	REGULATOR_LINEAR_RANGE(1050000, 0x04, 0x05, 50000),
+	REGULATOR_LINEAR_RANGE(1200000, 0x06, 0x07, 150000),
+};
+
+/*
+ * BUCK6
+ * 3.0V to 3.3V (step 100mV)
+ */
+static const struct regulator_linear_range bd71837_buck6_voltage_ranges[] = {
+	REGULATOR_LINEAR_RANGE(3000000, 0x00, 0x03, 100000),
+};
+
+/*
+ * BUCK7
+ * 000 = 1.605V
+ * 001 = 1.695V
+ * 010 = 1.755V
+ * 011 = 1.8V (Initial)
+ * 100 = 1.845V
+ * 101 = 1.905V
+ * 110 = 1.95V
+ * 111 = 1.995V
+ */
+const unsigned int buck_7_volts[] = {
+	1605000, 1695000, 1755000, 1800000, 1845000, 1905000, 1950000, 1995000
+};
+
+/*
+ * BUCK8
+ * 0.8V to 1.40V (step 10mV)
+ */
+static const struct regulator_linear_range bd71837_buck8_voltage_ranges[] = {
+	REGULATOR_LINEAR_RANGE(800000, 0x00, 0x3C, 10000),
+	REGULATOR_LINEAR_RANGE(1400000, 0x3D, 0x3F, 0),
+};
+
+/*
+ * LDO1
+ * 3.0 to 3.3V (100mV step)
+ */
+static const struct regulator_linear_range bd71837_ldo1_voltage_ranges[] = {
+	REGULATOR_LINEAR_RANGE(3000000, 0x00, 0x03, 100000),
+};
+
+/*
+ * LDO2
+ * 0.8 or 0.9V
+ */
+const unsigned int ldo_2_volts[] = {
+	900000, 800000
+};
+
+/*
+ * LDO3
+ * 1.8 to 3.3V (100mV step)
+ */
+static const struct regulator_linear_range bd71837_ldo3_voltage_ranges[] = {
+	REGULATOR_LINEAR_RANGE(1800000, 0x00, 0x0F, 100000),
+};
+
+/*
+ * LDO4
+ * 0.9 to 1.8V (100mV step)
+ */
+static const struct regulator_linear_range bd71837_ldo4_voltage_ranges[] = {
+	REGULATOR_LINEAR_RANGE(900000, 0x00, 0x09, 100000),
+	REGULATOR_LINEAR_RANGE(1800000, 0x0A, 0x0F, 0),
+};
+
+/*
+ * LDO5
+ * 1.8 to 3.3V (100mV step)
+ */
+static const struct regulator_linear_range bd71837_ldo5_voltage_ranges[] = {
+	REGULATOR_LINEAR_RANGE(1800000, 0x00, 0x0F, 100000),
+};
+
+/*
+ * LDO6
+ * 0.9 to 1.8V (100mV step)
+ */
+static const struct regulator_linear_range bd71837_ldo6_voltage_ranges[] = {
+	REGULATOR_LINEAR_RANGE(900000, 0x00, 0x09, 100000),
+	REGULATOR_LINEAR_RANGE(1800000, 0x0A, 0x0F, 0),
+};
+
+/*
+ * LDO7
+ * 1.8 to 3.3V (100mV step)
+ */
+static const struct regulator_linear_range bd71837_ldo7_voltage_ranges[] = {
+	REGULATOR_LINEAR_RANGE(1800000, 0x00, 0x0F, 100000),
+};
+
+static const struct regulator_desc bd71837_regulators[] = {
+	{
+	 .name = "BUCK1",
+	 .of_match = of_match_ptr("BUCK1"),
+	 .id = BD71837_BUCK1,
+	 .ops = &bd71837_buck1234_regulator_ops,
+	 .type = REGULATOR_VOLTAGE,
+	 .n_voltages = BD71837_BUCK1_VOLTAGE_NUM,
+	 .linear_ranges = bd71837_buck1234_voltage_ranges,
+	 .n_linear_ranges = ARRAY_SIZE(bd71837_buck1234_voltage_ranges),
+	 .vsel_reg = BD71837_REG_BUCK1_VOLT_RUN,
+	 .vsel_mask = BUCK1_RUN_MASK,
+	 .enable_reg = BD71837_REG_BUCK1_CTRL,
+	 .enable_mask = BD71837_BUCK_EN,
+	 .owner = THIS_MODULE,
+	 },
+	{
+	 .name = "BUCK2",
+	 .of_match = of_match_ptr("BUCK2"),
+	 .id = BD71837_BUCK2,
+	 .ops = &bd71837_buck1234_regulator_ops,
+	 .type = REGULATOR_VOLTAGE,
+	 .n_voltages = BD71837_BUCK2_VOLTAGE_NUM,
+	 .linear_ranges = bd71837_buck1234_voltage_ranges,
+	 .n_linear_ranges = ARRAY_SIZE(bd71837_buck1234_voltage_ranges),
+	 .vsel_reg = BD71837_REG_BUCK2_VOLT_RUN,
+	 .vsel_mask = BUCK2_RUN_MASK,
+	 .enable_reg = BD71837_REG_BUCK2_CTRL,
+	 .enable_mask = BD71837_BUCK_EN,
+	 .owner = THIS_MODULE,
+	 },
+	{
+	 .name = "BUCK3",
+	 .of_match = of_match_ptr("BUCK3"),
+	 .id = BD71837_BUCK3,
+	 .ops = &bd71837_buck1234_regulator_ops,
+	 .type = REGULATOR_VOLTAGE,
+	 .n_voltages = BD71837_BUCK3_VOLTAGE_NUM,
+	 .linear_ranges = bd71837_buck1234_voltage_ranges,
+	 .n_linear_ranges = ARRAY_SIZE(bd71837_buck1234_voltage_ranges),
+	 .vsel_reg = BD71837_REG_BUCK3_VOLT_RUN,
+	 .vsel_mask = BUCK3_RUN_MASK,
+	 .enable_reg = BD71837_REG_BUCK3_CTRL,
+	 .enable_mask = BD71837_BUCK_EN,
+	 .owner = THIS_MODULE,
+	 },
+	{
+	 .name = "BUCK4",
+	 .of_match = of_match_ptr("BUCK4"),
+	 .id = BD71837_BUCK4,
+	 .ops = &bd71837_buck1234_regulator_ops,
+	 .type = REGULATOR_VOLTAGE,
+	 .n_voltages = BD71837_BUCK4_VOLTAGE_NUM,
+	 .linear_ranges = bd71837_buck1234_voltage_ranges,
+	 .n_linear_ranges = ARRAY_SIZE(bd71837_buck1234_voltage_ranges),
+	 .vsel_reg = BD71837_REG_BUCK4_VOLT_RUN,
+	 .vsel_mask = BUCK4_RUN_MASK,
+	 .enable_reg = BD71837_REG_BUCK4_CTRL,
+	 .enable_mask = BD71837_BUCK_EN,
+	 .owner = THIS_MODULE,
+	 },
+	{
+	 .name = "BUCK5",
+	 .of_match = of_match_ptr("BUCK5"),
+	 .id = BD71837_BUCK5,
+	 .ops = &bd71837_buck_regulator_ops,
+	 .type = REGULATOR_VOLTAGE,
+	 .n_voltages = BD71837_BUCK5_VOLTAGE_NUM,
+	 .linear_ranges = bd71837_buck5_voltage_ranges,
+	 .n_linear_ranges = ARRAY_SIZE(bd71837_buck5_voltage_ranges),
+	 .vsel_reg = BD71837_REG_BUCK5_VOLT,
+	 .vsel_mask = BUCK5_MASK,
+	 .enable_reg = BD71837_REG_BUCK5_CTRL,
+	 .enable_mask = BD71837_BUCK_EN,
+	 .owner = THIS_MODULE,
+	 },
+	{
+	 .name = "BUCK6",
+	 .of_match = of_match_ptr("BUCK6"),
+	 .id = BD71837_BUCK6,
+	 .ops = &bd71837_buck_regulator_ops,
+	 .type = REGULATOR_VOLTAGE,
+	 .n_voltages = BD71837_BUCK6_VOLTAGE_NUM,
+	 .linear_ranges = bd71837_buck6_voltage_ranges,
+	 .n_linear_ranges = ARRAY_SIZE(bd71837_buck6_voltage_ranges),
+	 .vsel_reg = BD71837_REG_BUCK6_VOLT,
+	 .vsel_mask = BUCK6_MASK,
+	 .enable_reg = BD71837_REG_BUCK6_CTRL,
+	 .enable_mask = BD71837_BUCK_EN,
+	 .owner = THIS_MODULE,
+	 },
+	{
+	 .name = "BUCK7",
+	 .of_match = of_match_ptr("BUCK7"),
+	 .id = BD71837_BUCK7,
+	 .ops = &bd71837_buck_regulator_nolinear_ops,
+	 .type = REGULATOR_VOLTAGE,
+	 .volt_table = &buck_7_volts[0],
+	 .n_voltages = ARRAY_SIZE(buck_7_volts),
+	 .vsel_reg = BD71837_REG_BUCK7_VOLT,
+	 .vsel_mask = BUCK7_MASK,
+	 .enable_reg = BD71837_REG_BUCK7_CTRL,
+	 .enable_mask = BD71837_BUCK_EN,
+	 .owner = THIS_MODULE,
+	 },
+	{
+	 .name = "BUCK8",
+	 .of_match = of_match_ptr("BUCK8"),
+	 .id = BD71837_BUCK8,
+	 .ops = &bd71837_buck_regulator_ops,
+	 .type = REGULATOR_VOLTAGE,
+	 .n_voltages = BD71837_BUCK8_VOLTAGE_NUM,
+	 .linear_ranges = bd71837_buck8_voltage_ranges,
+	 .n_linear_ranges = ARRAY_SIZE(bd71837_buck8_voltage_ranges),
+	 .vsel_reg = BD71837_REG_BUCK8_VOLT,
+	 .vsel_mask = BUCK8_MASK,
+	 .enable_reg = BD71837_REG_BUCK8_CTRL,
+	 .enable_mask = BD71837_BUCK_EN,
+	 .owner = THIS_MODULE,
+	 },
+	{
+	 .name = "LDO1",
+	 .of_match = of_match_ptr("LDO1"),
+	 .id = BD71837_LDO1,
+	 .ops = &bd71837_ldo_regulator_ops,
+	 .type = REGULATOR_VOLTAGE,
+	 .n_voltages = BD71837_LDO1_VOLTAGE_NUM,
+	 .linear_ranges = bd71837_ldo1_voltage_ranges,
+	 .n_linear_ranges = ARRAY_SIZE(bd71837_ldo1_voltage_ranges),
+	 .vsel_reg = BD71837_REG_LDO1_VOLT,
+	 .vsel_mask = LDO1_MASK,
+	 .enable_reg = BD71837_REG_LDO1_VOLT,
+	 .enable_mask = BD71837_LDO_EN,
+	 .owner = THIS_MODULE,
+	 },
+	{
+	 .name = "LDO2",
+	 .of_match = of_match_ptr("LDO2"),
+	 .id = BD71837_LDO2,
+	 .ops = &bd71837_ldo_regulator_nolinear_ops,
+	 .type = REGULATOR_VOLTAGE,
+	 .volt_table = &ldo_2_volts[0],
+	 .vsel_reg = BD71837_REG_LDO2_VOLT,
+	 .vsel_mask = LDO2_MASK,
+	 .n_voltages = ARRAY_SIZE(ldo_2_volts),
+	 .n_voltages = BD71837_LDO2_VOLTAGE_NUM,
+	 .enable_reg = BD71837_REG_LDO2_VOLT,
+	 .enable_mask = BD71837_LDO_EN,
+	 .owner = THIS_MODULE,
+	 },
+	{
+	 .name = "LDO3",
+	 .of_match = of_match_ptr("LDO3"),
+	 .id = BD71837_LDO3,
+	 .ops = &bd71837_ldo_regulator_ops,
+	 .type = REGULATOR_VOLTAGE,
+	 .n_voltages = BD71837_LDO3_VOLTAGE_NUM,
+	 .linear_ranges = bd71837_ldo3_voltage_ranges,
+	 .n_linear_ranges = ARRAY_SIZE(bd71837_ldo3_voltage_ranges),
+	 .vsel_reg = BD71837_REG_LDO3_VOLT,
+	 .vsel_mask = LDO3_MASK,
+	 .enable_reg = BD71837_REG_LDO3_VOLT,
+	 .enable_mask = BD71837_LDO_EN,
+	 .owner = THIS_MODULE,
+	 },
+	{
+	 .name = "LDO4",
+	 .of_match = of_match_ptr("LDO4"),
+	 .id = BD71837_LDO4,
+	 .ops = &bd71837_ldo_regulator_ops,
+	 .type = REGULATOR_VOLTAGE,
+	 .n_voltages = BD71837_LDO4_VOLTAGE_NUM,
+	 .linear_ranges = bd71837_ldo4_voltage_ranges,
+	 .n_linear_ranges = ARRAY_SIZE(bd71837_ldo4_voltage_ranges),
+	 .vsel_reg = BD71837_REG_LDO4_VOLT,
+	 .vsel_mask = LDO4_MASK,
+	 .enable_reg = BD71837_REG_LDO4_VOLT,
+	 .enable_mask = BD71837_LDO_EN,
+	 .owner = THIS_MODULE,
+	 },
+	{
+	 .name = "LDO5",
+	 .of_match = of_match_ptr("LDO5"),
+	 .id = BD71837_LDO5,
+	 .ops = &bd71837_ldo_regulator_ops,
+	 .type = REGULATOR_VOLTAGE,
+	 .n_voltages = BD71837_LDO5_VOLTAGE_NUM,
+	 .linear_ranges = bd71837_ldo5_voltage_ranges,
+	 .n_linear_ranges = ARRAY_SIZE(bd71837_ldo5_voltage_ranges),
+	 /* LDO5 is supplied by buck6 */
+	 .supply_name = "buck6",
+	 .vsel_reg = BD71837_REG_LDO5_VOLT,
+	 .vsel_mask = LDO5_MASK,
+	 .enable_reg = BD71837_REG_LDO5_VOLT,
+	 .enable_mask = BD71837_LDO_EN,
+	 .owner = THIS_MODULE,
+	 },
+	{
+	 .name = "LDO6",
+	 .of_match = of_match_ptr("LDO6"),
+	 .id = BD71837_LDO6,
+	 .ops = &bd71837_ldo_regulator_ops,
+	 .type = REGULATOR_VOLTAGE,
+	 .n_voltages = BD71837_LDO6_VOLTAGE_NUM,
+	 .linear_ranges = bd71837_ldo6_voltage_ranges,
+	 .n_linear_ranges = ARRAY_SIZE(bd71837_ldo6_voltage_ranges),
+	 /* LDO6 is supplied by buck7 */
+	 .supply_name = "buck7",
+	 .vsel_reg = BD71837_REG_LDO6_VOLT,
+	 .vsel_mask = LDO6_MASK,
+	 .enable_reg = BD71837_REG_LDO6_VOLT,
+	 .enable_mask = BD71837_LDO_EN,
+	 .owner = THIS_MODULE,
+	 },
+	{
+	 .name = "LDO7",
+	 .of_match = of_match_ptr("LDO7"),
+	 .id = BD71837_LDO7,
+	 .ops = &bd71837_ldo_regulator_ops,
+	 .type = REGULATOR_VOLTAGE,
+	 .n_voltages = BD71837_LDO7_VOLTAGE_NUM,
+	 .linear_ranges = bd71837_ldo7_voltage_ranges,
+	 .n_linear_ranges = ARRAY_SIZE(bd71837_ldo7_voltage_ranges),
+	 .vsel_reg = BD71837_REG_LDO7_VOLT,
+	 .vsel_mask = LDO7_MASK,
+	 .enable_reg = BD71837_REG_LDO7_VOLT,
+	 .enable_mask = BD71837_LDO_EN,
+	 .owner = THIS_MODULE,
+	 },
+};
+
+struct reg_init {
+	unsigned int reg;
+	unsigned int mask;
+};
+
+static int bd71837_probe(struct platform_device *pdev)
+{
+	struct bd71837_pmic *pmic;
+	struct bd71837_board *pdata;
+	struct regulator_config config = { 0 };
+	struct reg_init pmic_regulator_inits[] = {
+		{
+		 .reg = BD71837_REG_BUCK1_CTRL,
+		 .mask = BD71837_BUCK_SEL,
+		}, {
+		 .reg = BD71837_REG_BUCK2_CTRL,
+		 .mask = BD71837_BUCK_SEL,
+		}, {
+		 .reg = BD71837_REG_BUCK3_CTRL,
+		 .mask = BD71837_BUCK_SEL,
+		}, {
+		 .reg = BD71837_REG_BUCK4_CTRL,
+		 .mask = BD71837_BUCK_SEL,
+		}, {
+		 .reg = BD71837_REG_BUCK5_CTRL,
+		 .mask = BD71837_BUCK_SEL,
+		}, {
+		 .reg = BD71837_REG_BUCK6_CTRL,
+		 .mask = BD71837_BUCK_SEL,
+		}, {
+		 .reg = BD71837_REG_BUCK7_CTRL,
+		 .mask = BD71837_BUCK_SEL,
+		}, {
+		 .reg = BD71837_REG_BUCK8_CTRL,
+		 .mask = BD71837_BUCK_SEL,
+		}, {
+		 .reg = BD71837_REG_LDO1_VOLT,
+		 .mask = BD71837_LDO_SEL,
+		}, {
+		 .reg = BD71837_REG_LDO2_VOLT,
+		 .mask = BD71837_LDO_SEL,
+		}, {
+		 .reg = BD71837_REG_LDO3_VOLT,
+		 .mask = BD71837_LDO_SEL,
+		}, {
+		 .reg = BD71837_REG_LDO4_VOLT,
+		 .mask = BD71837_LDO_SEL,
+		}, {
+		 .reg = BD71837_REG_LDO5_VOLT,
+		 .mask = BD71837_LDO_SEL,
+		}, {
+		 .reg = BD71837_REG_LDO6_VOLT,
+		 .mask = BD71837_LDO_SEL,
+		}, {
+		 .reg = BD71837_REG_LDO7_VOLT,
+		 .mask = BD71837_LDO_SEL,
+		}
+	};
+
+	int i = 0, err;
+
+	pmic = kzalloc(sizeof(struct bd71837_pmic), GFP_KERNEL);
+	if (!pmic)
+		return -ENOMEM;
+
+	mutex_init(&pmic->mtx);
+
+	memcpy(pmic->descs, bd71837_regulators, sizeof(pmic->descs));
+
+	pmic->pdev = pdev;
+	pmic->mfd = dev_get_drvdata(pdev->dev.parent);
+
+	if (!pmic->mfd) {
+		dev_err(&pdev->dev, "No MFD driver data\n");
+		err = -EINVAL;
+		goto err;
+	}
+	platform_set_drvdata(pdev, pmic);
+	pdata = dev_get_platdata(pmic->mfd->dev);
+
+	/* Register LOCK release */
+	err =
+	    regmap_update_bits(pmic->mfd->regmap, BD71837_REG_REGLOCK,
+			       (REGLOCK_PWRSEQ | REGLOCK_VREG), 0);
+	if (err) {
+		dev_err(&pmic->pdev->dev, "Failed to unlock PMIC (%d)\n", err);
+		goto err;
+	} else
+		dev_dbg(&pmic->pdev->dev, "%s: Unlocked lock register 0x%x\n",
+			__func__, BD71837_REG_REGLOCK);
+
+	for (i = 0; i < ARRAY_SIZE(pmic_regulator_inits); i++) {
+
+		struct regulator_desc *desc;
+		struct regulator_dev *rdev;
+
+		desc = &pmic->descs[i];
+
+		if (pdata)
+			config.init_data = pdata->init_data[i];
+
+		config.dev = &(pmic->pdev->dev);
+		config.driver_data = pmic;
+		config.regmap = pmic->mfd->regmap;
+
+		rdev = regulator_register(desc, &config);
+		if (IS_ERR(rdev)) {
+			dev_err(pmic->mfd->dev,
+				"failed to register %s regulator\n",
+				desc->name);
+			err = PTR_ERR(rdev);
+			goto err;
+		}
+		/* Regulator register gets the regulator constraints and
+		 * applies them (set_machine_constraints). This should have
+		 * turned the control register(s) to correct values and we
+		 * can now switch the control from PMIC state machine to the
+		 * register interface
+		 */
+		err =
+		    regmap_update_bits(pmic->mfd->regmap,
+				       pmic_regulator_inits[i].reg,
+				       pmic_regulator_inits[i].mask,
+				       0xFFFFFFFF);
+		if (err) {
+			dev_err(&pmic->pdev->dev,
+				"Failed to write BUCK/LDO SEL bit for (%s)\n",
+				desc->name);
+			goto err;
+		}
+
+		pmic->rdev[i] = rdev;
+	}
+
+	return 0;
+
+err:
+	while (--i >= 0)
+		regulator_unregister(pmic->rdev[i]);
+
+	kfree(pmic);
+	return err;
+}
+
+static int bd71837_remove(struct platform_device *pdev)
+{
+	struct bd71837_pmic *pmic = platform_get_drvdata(pdev);
+	int i;
+
+	if (pmic) {
+		for (i = 0; i < BD71837_REGULATOR_CNT; i++)
+			regulator_unregister(pmic->rdev[i]);
+
+		kfree(pmic);
+	}
+	return 0;
+}
+
+static struct platform_driver bd71837_regulator = {
+	.driver = {
+		   .name = "bd71837-pmic",
+		   .owner = THIS_MODULE,
+		   },
+	.probe = bd71837_probe,
+	.remove = bd71837_remove,
+};
+
+module_platform_driver(bd71837_regulator);
+
+MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>");
+MODULE_DESCRIPTION("BD71837 voltage regulator driver");
+MODULE_LICENSE("GPL");
-- 
2.14.3

Re: [PATCH 8/9] regulator: bd71837: BD71837 PMIC regulator driver

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1668 bytes --]

On Thu, May 24, 2018 at 09:00:36AM +0300, Matti Vaittinen wrote:

> @@ -0,0 +1,683 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (C) 2018 ROHM Semiconductors */
> +/*
> + * bd71837-regulator.c ROHM BD71837MWV regulator driver
> + */
> +#include <linux/kernel.h>

Make the entire comment block a C++ comment so it looks more intentional
and add a blank line before the headers for legibility.

> +static int bd71837_regulator_set_regmap(struct regulator_dev *rdev, int set)
> +{
> +	int ret = -EINVAL;
> +	struct bd71837_pmic *pmic = rdev->reg_data;
> +
> +	if (pmic) {
> +		mutex_lock(&pmic->mtx);
> +		if (!set)
> +			ret = regulator_disable_regmap(rdev);
> +		else
> +			ret = regulator_enable_regmap(rdev);
> +		mutex_unlock(&pmic->mtx);
> +	}

This looks very weird - why might we not have a parent PMIC, what is the
lock doing and what is this wrapper function intended to do?  Similar
issues apply to the voltage functions, if there's any need for this it
needs to be better documented but it really doesn't look like a good
idea.

> +	err =
> +	    regmap_update_bits(pmic->mfd->regmap, BD71837_REG_REGLOCK,
> +			       (REGLOCK_PWRSEQ | REGLOCK_VREG), 0);
> +	if (err) {
> +		dev_err(&pmic->pdev->dev, "Failed to unlock PMIC (%d)\n", err);
> +		goto err;
> +	} else
> +		dev_dbg(&pmic->pdev->dev, "%s: Unlocked lock register 0x%x\n",
> +			__func__, BD71837_REG_REGLOCK);

There's loads of coding style problems with this code, please refer to
the coding style - indentation is weird and if there's { } on one side
of an else it should be on both.

> +		rdev = regulator_register(desc, &config);
> +		if (IS_ERR(rdev)) {

devm_regulator_regster()

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: [PATCH 8/9] regulator: bd71837: BD71837 PMIC regulator driver

[Vaittinen, Matti]

From: Mark Brown [broonie@kernel.org]
> Sent: Thursday, May 24, 2018 5:14 PM
>
> On Thu, May 24, 2018 at 09:00:36AM +0300, Matti Vaittinen wrote:
>
> > @@ -0,0 +1,683 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (C) 2018 ROHM Semiconductors */
> > +/*
> > + * bd71837-regulator.c ROHM BD71837MWV regulator driver
> > + */
> > +#include <linux/kernel.h>
>
> Make the entire comment block a C++ comment so it looks more intentional

I will change this to C++ - but the verison of checkpatch.pl I used did complain
if I used C++ style comments in C-files or if I used C-style comments in header.
I guess the sscript should be fixed unless it is already done.

> and add a blank line before the headers for legibility.
Will do.

> > +static int bd71837_regulator_set_regmap(struct regulator_dev *rdev, int set)
> > +{
> > +     int ret = -EINVAL;
> > +     struct bd71837_pmic *pmic = rdev->reg_data;
> > +
> > +     if (pmic) {
> > +             mutex_lock(&pmic->mtx);
> > +             if (!set)
> > +                     ret = regulator_disable_regmap(rdev);
> > +             else
> > +                     ret = regulator_enable_regmap(rdev);
> > +             mutex_unlock(&pmic->mtx);
> > +     }
>
> This looks very weird - why might we not have a parent PMIC,
I'll remove the check. I've just adopted habit of adding NULL checks
for pointers "jsut in case".

> what is the lock doing and what is this wrapper function intended to do?
This was the other spot which I was unsure how to handle. Datasheet for
the chip says that if voltage is to be changed, the regulator must be
disabled. Thus my voltage changing function checks if regulator is enabled
and disables it for duration of voltage change (if it was enabled). This lock
is used to protect the voltage change so that no one enables the regulator
during voltage change. I don't know what would have been correct way of
doing this, or if disabling regulator for voltage change is Ok - but this was
the only way I could think of. I am again grateful for any tips.

> Similar
> issues apply to the voltage functions, if there's any need for this it
> needs to be better documented but it really doesn't look like a good
> idea.
If the solution with lock and wrapper (to prevent race during state check
and voltage change) is Ok, then I will add comment which explains this,

> > +     err =
> > +         regmap_update_bits(pmic->mfd->regmap, BD71837_REG_REGLOCK,
> > +                            (REGLOCK_PWRSEQ | REGLOCK_VREG), 0);
> > +     if (err) {
> > +             dev_err(&pmic->pdev->dev, "Failed to unlock PMIC (%d)\n", err);
> > +             goto err;
> > +     } else
> > +             dev_dbg(&pmic->pdev->dev, "%s: Unlocked lock register 0x%x\n",
> > +                     __func__, BD71837_REG_REGLOCK);
>
> There's loads of coding style problems with this code, please refer to
> the coding style - indentation is weird and if there's { } on one side
> of an else it should be on both.

Will fix it.

> > +             rdev = regulator_register(desc, &config);
> > +             if (IS_ERR(rdev)) {
>
> devm_regulator_regster()

Makes sense. Thanks


Best Regards
    Matti Vaittinen

RE: [PATCH 8/9] regulator: bd71837: BD71837 PMIC regulator driver

[Vaittinen, Matti]

=0A=
From: Mark Brown [broonie@kernel.org]=0A=
> Sent: Thursday, May 24, 2018 5:14 PM=0A=
>=0A=
> On Thu, May 24, 2018 at 09:00:36AM +0300, Matti Vaittinen wrote:=0A=
>=0A=
> > @@ -0,0 +1,683 @@=0A=
> > +// SPDX-License-Identifier: GPL-2.0=0A=
> > +/* Copyright (C) 2018 ROHM Semiconductors */=0A=
> > +/*=0A=
> > + * bd71837-regulator.c ROHM BD71837MWV regulator driver=0A=
> > + */=0A=
> > +#include <linux/kernel.h>=0A=
>=0A=
> Make the entire comment block a C++ comment so it looks more intentional=
=0A=
=0A=
I will change this to C++ - but the verison of checkpatch.pl I used did com=
plain=0A=
if I used C++ style comments in C-files or if I used C-style comments in he=
ader.=0A=
I guess the sscript should be fixed unless it is already done.=0A=
=0A=
> and add a blank line before the headers for legibility.=0A=
Will do.=0A=
=0A=
> > +static int bd71837_regulator_set_regmap(struct regulator_dev *rdev, in=
t set)=0A=
> > +{=0A=
> > +     int ret =3D -EINVAL;=0A=
> > +     struct bd71837_pmic *pmic =3D rdev->reg_data;=0A=
> > +=0A=
> > +     if (pmic) {=0A=
> > +             mutex_lock(&pmic->mtx);=0A=
> > +             if (!set)=0A=
> > +                     ret =3D regulator_disable_regmap(rdev);=0A=
> > +             else=0A=
> > +                     ret =3D regulator_enable_regmap(rdev);=0A=
> > +             mutex_unlock(&pmic->mtx);=0A=
> > +     }=0A=
>=0A=
> This looks very weird - why might we not have a parent PMIC,=0A=
I'll remove the check. I've just adopted habit of adding NULL checks=0A=
for pointers "jsut in case".=0A=
=0A=
> what is the lock doing and what is this wrapper function intended to do?=
=0A=
This was the other spot which I was unsure how to handle. Datasheet for=0A=
the chip says that if voltage is to be changed, the regulator must be=0A=
disabled. Thus my voltage changing function checks if regulator is enabled=
=0A=
and disables it for duration of voltage change (if it was enabled). This lo=
ck=0A=
is used to protect the voltage change so that no one enables the regulator=
=0A=
during voltage change. I don't know what would have been correct way of=0A=
doing this, or if disabling regulator for voltage change is Ok - but this w=
as=0A=
the only way I could think of. I am again grateful for any tips.=0A=
=0A=
> Similar=0A=
> issues apply to the voltage functions, if there's any need for this it=0A=
> needs to be better documented but it really doesn't look like a good=0A=
> idea.=0A=
If the solution with lock and wrapper (to prevent race during state check=
=0A=
and voltage change) is Ok, then I will add comment which explains this,=0A=
=0A=
> > +     err =3D=0A=
> > +         regmap_update_bits(pmic->mfd->regmap, BD71837_REG_REGLOCK,=0A=
> > +                            (REGLOCK_PWRSEQ | REGLOCK_VREG), 0);=0A=
> > +     if (err) {=0A=
> > +             dev_err(&pmic->pdev->dev, "Failed to unlock PMIC (%d)\n",=
 err);=0A=
> > +             goto err;=0A=
> > +     } else=0A=
> > +             dev_dbg(&pmic->pdev->dev, "%s: Unlocked lock register 0x%=
x\n",=0A=
> > +                     __func__, BD71837_REG_REGLOCK);=0A=
>=0A=
> There's loads of coding style problems with this code, please refer to=0A=
> the coding style - indentation is weird and if there's { } on one side=0A=
> of an else it should be on both.=0A=
=0A=
Will fix it.=0A=
=0A=
> > +             rdev =3D regulator_register(desc, &config);=0A=
> > +             if (IS_ERR(rdev)) {=0A=
>=0A=
> devm_regulator_regster()=0A=
=0A=
Makes sense. Thanks=0A=
=0A=
=0A=
Best Regards=0A=
    Matti Vaittinen=0A=
=0A=

Re: [PATCH 8/9] regulator: bd71837: BD71837 PMIC regulator driver

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 756 bytes --]

On Thu, May 24, 2018 at 05:51:27PM +0000, Vaittinen, Matti wrote:

> > what is the lock doing and what is this wrapper function intended to do?

> This was the other spot which I was unsure how to handle. Datasheet for
> the chip says that if voltage is to be changed, the regulator must be
> disabled. Thus my voltage changing function checks if regulator is enabled

Ugh, this chip is not very good is it?  Don't bounce the supply to
change the voltage silently, that's clearly a bad idea - the devices
using the supply are going to get very upset when the power gets removed
just because they changed the voltage.  Instead implement a custom set
operation that returns an error if the user attempts to change the
voltage while the regualtor is enabled.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 8/9] regulator: bd71837: BD71837 PMIC regulator driver

[Matti Vaittinen]

On Thu, May 24, 2018 at 06:59:40PM +0100, Mark Brown wrote:
> On Thu, May 24, 2018 at 05:51:27PM +0000, Vaittinen, Matti wrote:
> 
> > > what is the lock doing and what is this wrapper function intended to do?
> 
> > This was the other spot which I was unsure how to handle. Datasheet for
> > the chip says that if voltage is to be changed, the regulator must be
> > disabled. Thus my voltage changing function checks if regulator is enabled
> 
> Ugh, this chip is not very good is it?
I am not the correct guy to judge that as I don't have too wide
experience on PMICs. (This is first PMIC I have been working with).
Probably this chip has some other advantages and is thus used.

>  Don't bounce the supply to
> change the voltage silently, that's clearly a bad idea - the devices
> using the supply are going to get very upset when the power gets removed
> just because they changed the voltage.  Instead implement a custom set
> operation that returns an error if the user attempts to change the
> voltage while the regualtor is enabled.

Makes perfect sense. I will change the operation to this.

Br,
    Matti Vaittinen

Re: [PATCH 8/9] regulator: bd71837: BD71837 PMIC regulator driver

[Matti Vaittinen]

On Thu, May 24, 2018 at 05:51:27PM +0000, Vaittinen, Matti wrote:
> 
> From: Mark Brown [broonie@kernel.org]
> > Sent: Thursday, May 24, 2018 5:14 PM
> > On Thu, May 24, 2018 at 09:00:36AM +0300, Matti Vaittinen wrote:
> > > +             rdev = regulator_register(desc, &config);
> > > +             if (IS_ERR(rdev)) {
> >
> > devm_regulator_regster()
> 
> Makes sense. Thanks

I was going to do 

-       pmic = kzalloc(sizeof(struct bd71837_pmic), GFP_KERNEL);
+       pmic = devm_kzalloc(&pdev->dev, sizeof(struct bd71837_pmic),
+                           GFP_KERNEL);

and

-               rdev = regulator_register(desc, &config);
+               rdev = devm_regulator_register(&pdev->dev, desc, &config);

but is there now a race regarding freeing the pmic structure and
unregistering the regulator?

Best Regards
    Matti Vaittinen

Re: [PATCH 8/9] regulator: bd71837: BD71837 PMIC regulator driver

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 757 bytes --]

On Fri, May 25, 2018 at 10:22:53AM +0300, Matti Vaittinen wrote:
> On Thu, May 24, 2018 at 05:51:27PM +0000, Vaittinen, Matti wrote:

> > > devm_regulator_regster()

> > Makes sense. Thanks

> I was going to do 

> -       pmic = kzalloc(sizeof(struct bd71837_pmic), GFP_KERNEL);
> +       pmic = devm_kzalloc(&pdev->dev, sizeof(struct bd71837_pmic),
> +                           GFP_KERNEL);

> and

> -               rdev = regulator_register(desc, &config);
> +               rdev = devm_regulator_register(&pdev->dev, desc, &config);
> 
> but is there now a race regarding freeing the pmic structure and
> unregistering the regulator?

Why?  devm_ stuff gets unwound in the opposite order to the order in
which it was allocated.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 484 bytes --]

Re: [PATCH 8/9] regulator: bd71837: BD71837 PMIC regulator driver

[Matti Vaittinen]

On Fri, May 25, 2018 at 11:14:23AM +0100, Mark Brown wrote:
> On Fri, May 25, 2018 at 10:22:53AM +0300, Matti Vaittinen wrote:
> > On Thu, May 24, 2018 at 05:51:27PM +0000, Vaittinen, Matti wrote:
> 
> > > > devm_regulator_regster()
> 
> > > Makes sense. Thanks
> 
> > I was going to do 
> 
> > -       pmic = kzalloc(sizeof(struct bd71837_pmic), GFP_KERNEL);
> > +       pmic = devm_kzalloc(&pdev->dev, sizeof(struct bd71837_pmic),
> > +                           GFP_KERNEL);
> 
> > and
> 
> > -               rdev = regulator_register(desc, &config);
> > +               rdev = devm_regulator_register(&pdev->dev, desc, &config);
> > 
> > but is there now a race regarding freeing the pmic structure and
> > unregistering the regulator?
> 
> Why?  devm_ stuff gets unwound in the opposite order to the order in
> which it was allocated.
Allright. Then there's no problems. I'll do the change.