[PATCH v2 0/5] DA9062 PMIC features

[PATCH v2 0/5] DA9062 PMIC features

[Marco Felsch]

Hi,

this series address all comments made on [1]. Patch "regulator: da9062:
fix suspend_enable/disable preparation" was applied mainline so I
droppped it. Patch "gpio: add support to get local gpio number" is new
and based on Linus suggestion. The v2 has no compile dependency like
[1] but you need to apply [2] else the features can't be used.

[1] https://lkml.org/lkml/2019/9/17/411
[2] https://patchwork.ozlabs.org/cover/1201549/

Marco Felsch (5):
  gpio: add support to get local gpio number
  dt-bindings: mfd: da9062: add regulator voltage selection
    documentation
  regulator: da9062: add voltage selection gpio support
  dt-bindings: mfd: da9062: add regulator gpio enable/disable
    documentation
  regulator: da9062: add gpio based regulator dis-/enable support

 .../devicetree/bindings/mfd/da9062.txt        |  17 ++
 drivers/gpio/gpiolib.c                        |   6 +
 drivers/regulator/da9062-regulator.c          | 244 ++++++++++++++++++
 include/linux/gpio/consumer.h                 |  10 +
 4 files changed, 277 insertions(+)

-- 
2.20.1

[PATCH v2 1/5] gpio: add support to get local gpio number

[Marco Felsch]

Sometimes consumers needs to know the gpio-chip local gpio number of a
'struct gpio_desc' for further configuration. This is often the case for
mfd devices.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/gpio/gpiolib.c        |  6 ++++++
 include/linux/gpio/consumer.h | 10 ++++++++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 104ed299d5ea..7709648313fc 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4377,6 +4377,12 @@ int gpiod_count(struct device *dev, const char *con_id)
 }
 EXPORT_SYMBOL_GPL(gpiod_count);
 
+int gpiod_to_offset(struct gpio_desc *desc)
+{
+	return gpio_chip_hwgpio(desc);
+}
+EXPORT_SYMBOL_GPL(gpiod_to_offset);
+
 /**
  * gpiod_get - obtain a GPIO for a given GPIO function
  * @dev:	GPIO consumer, can be NULL for system-global GPIOs
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index b70af921c614..e2178c3bf7fd 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -60,6 +60,9 @@ enum gpiod_flags {
 /* Return the number of GPIOs associated with a device / function */
 int gpiod_count(struct device *dev, const char *con_id);
 
+/* Get the local chip offset from a global desc */
+int gpiod_to_offset(struct gpio_desc *desc);
+
 /* Acquire and dispose GPIOs */
 struct gpio_desc *__must_check gpiod_get(struct device *dev,
 					 const char *con_id,
@@ -189,6 +192,13 @@ static inline int gpiod_count(struct device *dev, const char *con_id)
 	return 0;
 }
 
+static inline int gpiod_to_offset(struct gpio_desc *desc)
+{
+	/* GPIO can never have been requested */
+	WARN_ON(desc);
+	return 0;
+}
+
 static inline struct gpio_desc *__must_check gpiod_get(struct device *dev,
 						       const char *con_id,
 						       enum gpiod_flags flags)
-- 
2.20.1

[PATCH v2 2/5] dt-bindings: mfd: da9062: add regulator voltage selection documentation

[Marco Felsch]

Add the documentation which describe the voltage selection gpio support.
This property can be applied to each subnode within the 'regulators'
node so each regulator can be configured differently.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 Documentation/devicetree/bindings/mfd/da9062.txt | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/da9062.txt b/Documentation/devicetree/bindings/mfd/da9062.txt
index edca653a5777..7e780fa6f57d 100644
--- a/Documentation/devicetree/bindings/mfd/da9062.txt
+++ b/Documentation/devicetree/bindings/mfd/da9062.txt
@@ -66,6 +66,15 @@ Sub-nodes:
   details of individual regulator device can be found in:
   Documentation/devicetree/bindings/regulator/regulator.txt
 
+  Optional regulator device-specific properties:
+  - dlg,vsel-sense-gpios : The GPIO reference which should be used by the
+    regulator to switch the voltage between active/suspend voltage settings. If
+    the signal is active the active-settings are applied else the suspend
+    settings are applied. Attention: Sharing the same gpio for other purposes
+    or across multiple regulators is possible but the gpio settings must be the
+    same. Also the gpio phandle must refer to the mfd root node other gpios are
+    not allowed and make no sense.
+
 - rtc : This node defines settings required for the Real-Time Clock associated
   with the DA9062. There are currently no entries in this binding, however
   compatible = "dlg,da9062-rtc" should be added if a node is created.
-- 
2.20.1

[PATCH v2 3/5] regulator: da9062: add voltage selection gpio support

[Marco Felsch]

The DA9062/1 devices can switch their regulator voltages between
voltage-A (active) and voltage-B (suspend) settings. Switching the
voltages can be controlled by ther internal state-machine or by a gpio
input signal and can be configured for each individual regulator. This
commit adds the gpio-based voltage switching support.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
Changelog:

v2:
- use new public api gpiod_to_offset()
- add -ENOENT error check to mimic devm_gpio_optional
- add local gpio check for hardening the code

 drivers/regulator/da9062-regulator.c | 164 +++++++++++++++++++++++++++
 1 file changed, 164 insertions(+)

diff --git a/drivers/regulator/da9062-regulator.c b/drivers/regulator/da9062-regulator.c
index 710e67081d53..f545ae39352e 100644
--- a/drivers/regulator/da9062-regulator.c
+++ b/drivers/regulator/da9062-regulator.c
@@ -7,6 +7,7 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/err.h>
+#include <linux/gpio/consumer.h>
 #include <linux/slab.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
@@ -50,6 +51,7 @@ struct da9062_regulator_info {
 	struct reg_field sleep;
 	struct reg_field suspend_sleep;
 	unsigned int suspend_vsel_reg;
+	struct reg_field vsel_gpi;
 	/* Event detection bit */
 	struct reg_field oc_event;
 };
@@ -65,6 +67,7 @@ struct da9062_regulator {
 	struct regmap_field			*suspend;
 	struct regmap_field			*sleep;
 	struct regmap_field			*suspend_sleep;
+	struct regmap_field			*vsel_gpi;
 };
 
 /* Encapsulates all information for the regulators driver */
@@ -351,6 +354,81 @@ static const struct regulator_ops da9062_ldo_ops = {
 	.set_suspend_mode	= da9062_ldo_set_suspend_mode,
 };
 
+static int da9062_config_gpi(struct device_node *np,
+			     const struct regulator_desc *desc,
+			     struct regulator_config *cfg, const char *gpi_id)
+{
+	struct da9062_regulator *regl = cfg->driver_data;
+	struct device *hw = regl->hw->dev;
+	struct device_node *gpio_np;
+	struct gpio_desc *gpi;
+	unsigned int nr;
+	int ret;
+	char *prop, *label;
+
+	prop = kasprintf(GFP_KERNEL, "dlg,%s-sense-gpios", gpi_id);
+	if (!prop)
+		return -ENOMEM;
+
+	label = kasprintf(GFP_KERNEL, "%s-%s-gpi", desc->name, gpi_id);
+	if (!label) {
+		ret = -ENOMEM;
+		goto free;
+	}
+
+	/*
+	 * We only must ensure that the gpio device is probed before the
+	 * regulator driver so no need to store the reference global. Luckily
+	 * devm_* releases the gpio upon a remove action. The gpio's are
+	 * optional so we need to check for ENOENT. Also we need to check for
+	 * the GPIOLIB support. Do nothing if the property or the gpiolib is
+	 * missing.
+	 */
+	gpi = devm_gpiod_get_from_of_node(cfg->dev, np, prop, 0, GPIOD_IN |
+					  GPIOD_FLAGS_BIT_NONEXCLUSIVE, label);
+	if (IS_ERR(gpi)) {
+		ret = PTR_ERR(gpi);
+		if (ret == -ENOENT || ret == -ENOSYS)
+			ret = 0;
+		goto free;
+	}
+
+	/*
+	 * Only local gpios are valid. The gpio-controller is within the
+	 * mfd-root node.
+	 */
+	gpio_np = of_parse_phandle(np, prop, 0);
+	if (gpio_np != hw->of_node) {
+		of_node_put(gpio_np);
+		dev_err(hw, "Failed to request %s.\n", prop);
+		ret = -EINVAL;
+		goto free;
+	}
+	of_node_put(gpio_np);
+
+	/* We need the local number */
+	nr = gpiod_to_offset(gpi);
+	if (nr < 1 || nr > 3) {
+		ret = -EINVAL;
+		goto free;
+	}
+
+	ret = regmap_field_write(regl->vsel_gpi, nr);
+
+free:
+	kfree(prop);
+	kfree(label);
+
+	return ret;
+}
+
+static int da9062_parse_dt(struct device_node *np,
+			   const struct regulator_desc *desc,
+			   struct regulator_config *cfg)
+{
+	return da9062_config_gpi(np, desc, cfg, "vsel");
+}
+
 /* DA9061 Regulator information */
 static const struct da9062_regulator_info local_da9061_regulator_info[] = {
 	{
@@ -358,6 +436,7 @@ static const struct da9062_regulator_info local_da9061_regulator_info[] = {
 		.desc.name = "DA9061 BUCK1",
 		.desc.of_match = of_match_ptr("buck1"),
 		.desc.regulators_node = of_match_ptr("regulators"),
+		.desc.of_parse_cb = da9062_parse_dt,
 		.desc.ops = &da9062_buck_ops,
 		.desc.min_uV = (300) * 1000,
 		.desc.uV_step = (10) * 1000,
@@ -388,12 +467,17 @@ static const struct da9062_regulator_info local_da9061_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_BUCK1_CONF_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_BUCK1_CONF_MASK) - 1),
+		.vsel_gpi = REG_FIELD(DA9062AA_BUCK1_CONT,
+			__builtin_ffs((int)DA9062AA_VBUCK1_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_VBUCK1_GPI_MASK) - 1),
 	},
 	{
 		.desc.id = DA9061_ID_BUCK2,
 		.desc.name = "DA9061 BUCK2",
 		.desc.of_match = of_match_ptr("buck2"),
 		.desc.regulators_node = of_match_ptr("regulators"),
+		.desc.of_parse_cb = da9062_parse_dt,
 		.desc.ops = &da9062_buck_ops,
 		.desc.min_uV = (800) * 1000,
 		.desc.uV_step = (20) * 1000,
@@ -424,12 +508,17 @@ static const struct da9062_regulator_info local_da9061_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_BUCK3_CONF_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_BUCK3_CONF_MASK) - 1),
+		.vsel_gpi = REG_FIELD(DA9062AA_BUCK3_CONT,
+			__builtin_ffs((int)DA9062AA_VBUCK3_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_VBUCK3_GPI_MASK) - 1),
 	},
 	{
 		.desc.id = DA9061_ID_BUCK3,
 		.desc.name = "DA9061 BUCK3",
 		.desc.of_match = of_match_ptr("buck3"),
 		.desc.regulators_node = of_match_ptr("regulators"),
+		.desc.of_parse_cb = da9062_parse_dt,
 		.desc.ops = &da9062_buck_ops,
 		.desc.min_uV = (530) * 1000,
 		.desc.uV_step = (10) * 1000,
@@ -460,12 +549,17 @@ static const struct da9062_regulator_info local_da9061_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_BUCK4_CONF_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_BUCK4_CONF_MASK) - 1),
+		.vsel_gpi = REG_FIELD(DA9062AA_BUCK4_CONT,
+			__builtin_ffs((int)DA9062AA_VBUCK4_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_VBUCK4_GPI_MASK) - 1),
 	},
 	{
 		.desc.id = DA9061_ID_LDO1,
 		.desc.name = "DA9061 LDO1",
 		.desc.of_match = of_match_ptr("ldo1"),
 		.desc.regulators_node = of_match_ptr("regulators"),
+		.desc.of_parse_cb = da9062_parse_dt,
 		.desc.ops = &da9062_ldo_ops,
 		.desc.min_uV = (900) * 1000,
 		.desc.uV_step = (50) * 1000,
@@ -489,6 +583,10 @@ static const struct da9062_regulator_info local_da9061_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_LDO1_CONF_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_LDO1_CONF_MASK) - 1),
+		.vsel_gpi = REG_FIELD(DA9062AA_LDO1_CONT,
+			__builtin_ffs((int)DA9062AA_VLDO1_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_VLDO1_GPI_MASK) - 1),
 		.oc_event = REG_FIELD(DA9062AA_STATUS_D,
 			__builtin_ffs((int)DA9062AA_LDO1_ILIM_MASK) - 1,
 			sizeof(unsigned int) * 8 -
@@ -499,6 +597,7 @@ static const struct da9062_regulator_info local_da9061_regulator_info[] = {
 		.desc.name = "DA9061 LDO2",
 		.desc.of_match = of_match_ptr("ldo2"),
 		.desc.regulators_node = of_match_ptr("regulators"),
+		.desc.of_parse_cb = da9062_parse_dt,
 		.desc.ops = &da9062_ldo_ops,
 		.desc.min_uV = (900) * 1000,
 		.desc.uV_step = (50) * 1000,
@@ -522,6 +621,10 @@ static const struct da9062_regulator_info local_da9061_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_LDO2_CONF_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_LDO2_CONF_MASK) - 1),
+		.vsel_gpi = REG_FIELD(DA9062AA_LDO2_CONT,
+			__builtin_ffs((int)DA9062AA_VLDO2_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_VLDO2_GPI_MASK) - 1),
 		.oc_event = REG_FIELD(DA9062AA_STATUS_D,
 			__builtin_ffs((int)DA9062AA_LDO2_ILIM_MASK) - 1,
 			sizeof(unsigned int) * 8 -
@@ -532,6 +635,7 @@ static const struct da9062_regulator_info local_da9061_regulator_info[] = {
 		.desc.name = "DA9061 LDO3",
 		.desc.of_match = of_match_ptr("ldo3"),
 		.desc.regulators_node = of_match_ptr("regulators"),
+		.desc.of_parse_cb = da9062_parse_dt,
 		.desc.ops = &da9062_ldo_ops,
 		.desc.min_uV = (900) * 1000,
 		.desc.uV_step = (50) * 1000,
@@ -555,6 +659,10 @@ static const struct da9062_regulator_info local_da9061_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_LDO3_CONF_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_LDO3_CONF_MASK) - 1),
+		.vsel_gpi = REG_FIELD(DA9062AA_LDO3_CONT,
+			__builtin_ffs((int)DA9062AA_VLDO3_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_VLDO3_GPI_MASK) - 1),
 		.oc_event = REG_FIELD(DA9062AA_STATUS_D,
 			__builtin_ffs((int)DA9062AA_LDO3_ILIM_MASK) - 1,
 			sizeof(unsigned int) * 8 -
@@ -565,6 +673,7 @@ static const struct da9062_regulator_info local_da9061_regulator_info[] = {
 		.desc.name = "DA9061 LDO4",
 		.desc.of_match = of_match_ptr("ldo4"),
 		.desc.regulators_node = of_match_ptr("regulators"),
+		.desc.of_parse_cb = da9062_parse_dt,
 		.desc.ops = &da9062_ldo_ops,
 		.desc.min_uV = (900) * 1000,
 		.desc.uV_step = (50) * 1000,
@@ -588,6 +697,10 @@ static const struct da9062_regulator_info local_da9061_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_LDO4_CONF_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_LDO4_CONF_MASK) - 1),
+		.vsel_gpi = REG_FIELD(DA9062AA_LDO4_CONT,
+			__builtin_ffs((int)DA9062AA_VLDO4_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_VLDO4_GPI_MASK) - 1),
 		.oc_event = REG_FIELD(DA9062AA_STATUS_D,
 			__builtin_ffs((int)DA9062AA_LDO4_ILIM_MASK) - 1,
 			sizeof(unsigned int) * 8 -
@@ -602,6 +715,7 @@ static const struct da9062_regulator_info local_da9062_regulator_info[] = {
 		.desc.name = "DA9062 BUCK1",
 		.desc.of_match = of_match_ptr("buck1"),
 		.desc.regulators_node = of_match_ptr("regulators"),
+		.desc.of_parse_cb = da9062_parse_dt,
 		.desc.ops = &da9062_buck_ops,
 		.desc.min_uV = (300) * 1000,
 		.desc.uV_step = (10) * 1000,
@@ -632,12 +746,17 @@ static const struct da9062_regulator_info local_da9062_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_BUCK1_CONF_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_BUCK1_CONF_MASK) - 1),
+		.vsel_gpi = REG_FIELD(DA9062AA_BUCK1_CONT,
+			__builtin_ffs((int)DA9062AA_VBUCK1_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_VBUCK1_GPI_MASK) - 1),
 	},
 	{
 		.desc.id = DA9062_ID_BUCK2,
 		.desc.name = "DA9062 BUCK2",
 		.desc.of_match = of_match_ptr("buck2"),
 		.desc.regulators_node = of_match_ptr("regulators"),
+		.desc.of_parse_cb = da9062_parse_dt,
 		.desc.ops = &da9062_buck_ops,
 		.desc.min_uV = (300) * 1000,
 		.desc.uV_step = (10) * 1000,
@@ -668,12 +787,17 @@ static const struct da9062_regulator_info local_da9062_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_BUCK2_CONF_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_BUCK2_CONF_MASK) - 1),
+		.vsel_gpi = REG_FIELD(DA9062AA_BUCK2_CONT,
+			__builtin_ffs((int)DA9062AA_VBUCK2_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_VBUCK2_GPI_MASK) - 1),
 	},
 	{
 		.desc.id = DA9062_ID_BUCK3,
 		.desc.name = "DA9062 BUCK3",
 		.desc.of_match = of_match_ptr("buck3"),
 		.desc.regulators_node = of_match_ptr("regulators"),
+		.desc.of_parse_cb = da9062_parse_dt,
 		.desc.ops = &da9062_buck_ops,
 		.desc.min_uV = (800) * 1000,
 		.desc.uV_step = (20) * 1000,
@@ -704,12 +828,17 @@ static const struct da9062_regulator_info local_da9062_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_BUCK3_CONF_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_BUCK3_CONF_MASK) - 1),
+		.vsel_gpi = REG_FIELD(DA9062AA_BUCK3_CONT,
+			__builtin_ffs((int)DA9062AA_VBUCK3_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_VBUCK3_GPI_MASK) - 1),
 	},
 	{
 		.desc.id = DA9062_ID_BUCK4,
 		.desc.name = "DA9062 BUCK4",
 		.desc.of_match = of_match_ptr("buck4"),
 		.desc.regulators_node = of_match_ptr("regulators"),
+		.desc.of_parse_cb = da9062_parse_dt,
 		.desc.ops = &da9062_buck_ops,
 		.desc.min_uV = (530) * 1000,
 		.desc.uV_step = (10) * 1000,
@@ -740,12 +869,17 @@ static const struct da9062_regulator_info local_da9062_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_BUCK4_CONF_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_BUCK4_CONF_MASK) - 1),
+		.vsel_gpi = REG_FIELD(DA9062AA_BUCK4_CONT,
+			__builtin_ffs((int)DA9062AA_VBUCK4_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_VBUCK4_GPI_MASK) - 1),
 	},
 	{
 		.desc.id = DA9062_ID_LDO1,
 		.desc.name = "DA9062 LDO1",
 		.desc.of_match = of_match_ptr("ldo1"),
 		.desc.regulators_node = of_match_ptr("regulators"),
+		.desc.of_parse_cb = da9062_parse_dt,
 		.desc.ops = &da9062_ldo_ops,
 		.desc.min_uV = (900) * 1000,
 		.desc.uV_step = (50) * 1000,
@@ -769,6 +903,10 @@ static const struct da9062_regulator_info local_da9062_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_LDO1_CONF_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_LDO1_CONF_MASK) - 1),
+		.vsel_gpi = REG_FIELD(DA9062AA_LDO1_CONT,
+			__builtin_ffs((int)DA9062AA_VLDO1_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_VLDO1_GPI_MASK) - 1),
 		.oc_event = REG_FIELD(DA9062AA_STATUS_D,
 			__builtin_ffs((int)DA9062AA_LDO1_ILIM_MASK) - 1,
 			sizeof(unsigned int) * 8 -
@@ -779,6 +917,7 @@ static const struct da9062_regulator_info local_da9062_regulator_info[] = {
 		.desc.name = "DA9062 LDO2",
 		.desc.of_match = of_match_ptr("ldo2"),
 		.desc.regulators_node = of_match_ptr("regulators"),
+		.desc.of_parse_cb = da9062_parse_dt,
 		.desc.ops = &da9062_ldo_ops,
 		.desc.min_uV = (900) * 1000,
 		.desc.uV_step = (50) * 1000,
@@ -802,6 +941,10 @@ static const struct da9062_regulator_info local_da9062_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_LDO2_CONF_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_LDO2_CONF_MASK) - 1),
+		.vsel_gpi = REG_FIELD(DA9062AA_LDO2_CONT,
+			__builtin_ffs((int)DA9062AA_VLDO2_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_VLDO2_GPI_MASK) - 1),
 		.oc_event = REG_FIELD(DA9062AA_STATUS_D,
 			__builtin_ffs((int)DA9062AA_LDO2_ILIM_MASK) - 1,
 			sizeof(unsigned int) * 8 -
@@ -812,6 +955,7 @@ static const struct da9062_regulator_info local_da9062_regulator_info[] = {
 		.desc.name = "DA9062 LDO3",
 		.desc.of_match = of_match_ptr("ldo3"),
 		.desc.regulators_node = of_match_ptr("regulators"),
+		.desc.of_parse_cb = da9062_parse_dt,
 		.desc.ops = &da9062_ldo_ops,
 		.desc.min_uV = (900) * 1000,
 		.desc.uV_step = (50) * 1000,
@@ -835,6 +979,10 @@ static const struct da9062_regulator_info local_da9062_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_LDO3_CONF_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_LDO3_CONF_MASK) - 1),
+		.vsel_gpi = REG_FIELD(DA9062AA_LDO3_CONT,
+			__builtin_ffs((int)DA9062AA_VLDO3_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_VLDO3_GPI_MASK) - 1),
 		.oc_event = REG_FIELD(DA9062AA_STATUS_D,
 			__builtin_ffs((int)DA9062AA_LDO3_ILIM_MASK) - 1,
 			sizeof(unsigned int) * 8 -
@@ -845,6 +993,7 @@ static const struct da9062_regulator_info local_da9062_regulator_info[] = {
 		.desc.name = "DA9062 LDO4",
 		.desc.of_match = of_match_ptr("ldo4"),
 		.desc.regulators_node = of_match_ptr("regulators"),
+		.desc.of_parse_cb = da9062_parse_dt,
 		.desc.ops = &da9062_ldo_ops,
 		.desc.min_uV = (900) * 1000,
 		.desc.uV_step = (50) * 1000,
@@ -868,6 +1017,10 @@ static const struct da9062_regulator_info local_da9062_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_LDO4_CONF_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_LDO4_CONF_MASK) - 1),
+		.vsel_gpi = REG_FIELD(DA9062AA_LDO4_CONT,
+			__builtin_ffs((int)DA9062AA_VLDO4_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_VLDO4_GPI_MASK) - 1),
 		.oc_event = REG_FIELD(DA9062AA_STATUS_D,
 			__builtin_ffs((int)DA9062AA_LDO4_ILIM_MASK) - 1,
 			sizeof(unsigned int) * 8 -
@@ -988,6 +1141,15 @@ static int da9062_regulator_probe(struct platform_device *pdev)
 				return PTR_ERR(regl->suspend_sleep);
 		}
 
+		if (regl->info->vsel_gpi.reg) {
+			regl->vsel_gpi = devm_regmap_field_alloc(
+					&pdev->dev,
+					chip->regmap,
+					regl->info->vsel_gpi);
+			if (IS_ERR(regl->vsel_gpi))
+				return PTR_ERR(regl->vsel_gpi);
+		}
+
 		/* Register regulator */
 		memset(&config, 0, sizeof(config));
 		config.dev = chip->dev;
@@ -997,6 +1159,8 @@ static int da9062_regulator_probe(struct platform_device *pdev)
 		regl->rdev = devm_regulator_register(&pdev->dev, &regl->desc,
 						     &config);
 		if (IS_ERR(regl->rdev)) {
+			if (PTR_ERR(regl->rdev) == -EPROBE_DEFER)
+				return -EPROBE_DEFER;
 			dev_err(&pdev->dev,
 				"Failed to register %s regulator\n",
 				regl->desc.name);
-- 
2.20.1

[PATCH v2 4/5] dt-bindings: mfd: da9062: add regulator gpio enable/disable documentation

[Marco Felsch]

At the gpio-based regulator enable/disable documentation. This property
can be applied to each subnode within the 'regulators' node so each
regulator can be configured differently.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 Documentation/devicetree/bindings/mfd/da9062.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/da9062.txt b/Documentation/devicetree/bindings/mfd/da9062.txt
index 7e780fa6f57d..417e836e273a 100644
--- a/Documentation/devicetree/bindings/mfd/da9062.txt
+++ b/Documentation/devicetree/bindings/mfd/da9062.txt
@@ -75,6 +75,13 @@ Sub-nodes:
     same. Also the gpio phandle must refer to the mfd root node other gpios are
     not allowed and make no sense.
 
+  - dlg,ena-sense-gpios : The GPIO reference which should be used by the
+    regulator to enable/disable the output. If the signal is active the
+    regulator is on else it is off. Attention: Sharing the same gpio for other
+    purposes or across multiple regulators is possible but the gpio settings
+    must be the same. Also the gpio phandle must refer to the mfd root node
+    other gpios are not allowed and make no sense.
+
 - rtc : This node defines settings required for the Real-Time Clock associated
   with the DA9062. There are currently no entries in this binding, however
   compatible = "dlg,da9062-rtc" should be added if a node is created.
@@ -112,6 +119,7 @@ Example:
 				regulator-min-microvolt = <900000>;
 				regulator-max-microvolt = <3600000>;
 				regulator-boot-on;
+				dlg,ena-sense-gpios = <&pmic0 2 GPIO_ACTIVE_LOW>;
 			};
 		};
 	};
-- 
2.20.1

[PATCH v2 5/5] regulator: da9062: add gpio based regulator dis-/enable support

[Marco Felsch]

Each regulator can be enabeld/disabled by the internal pmic state
machine or by a gpio input signal. Typically the OTP configures the
regulators to be enabled/disabled on a specific sequence number which is
most the time fine. Sometimes we need to reconfigure that due to a PCB
bug. This patch adds the support to disable/enable the regulator based
on a gpio input signal.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/regulator/da9062-regulator.c | 84 +++++++++++++++++++++++++++-
 1 file changed, 82 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/da9062-regulator.c b/drivers/regulator/da9062-regulator.c
index f545ae39352e..484b5c9a1d7e 100644
--- a/drivers/regulator/da9062-regulator.c
+++ b/drivers/regulator/da9062-regulator.c
@@ -52,6 +52,7 @@ struct da9062_regulator_info {
 	struct reg_field suspend_sleep;
 	unsigned int suspend_vsel_reg;
 	struct reg_field vsel_gpi;
+	struct reg_field ena_gpi;
 	/* Event detection bit */
 	struct reg_field oc_event;
 };
@@ -68,6 +69,7 @@ struct da9062_regulator {
 	struct regmap_field			*sleep;
 	struct regmap_field			*suspend_sleep;
 	struct regmap_field			*vsel_gpi;
+	struct regmap_field			*ena_gpi;
 };
 
 /* Encapsulates all information for the regulators driver */
@@ -413,7 +415,10 @@ static int da9062_config_gpi(struct device_node *np,
 		goto free;
 	}
 
-	ret = regmap_field_write(regl->vsel_gpi, nr);
+	if (!strncmp(gpi_id, "ena", 3))
+		ret = regmap_field_write(regl->ena_gpi, nr);
+	else
+		ret = regmap_field_write(regl->vsel_gpi, nr);
 
 free:
 	kfree(prop);
@@ -426,7 +431,13 @@ static int da9062_parse_dt(struct device_node *np,
 			   const struct regulator_desc *desc,
 			   struct regulator_config *cfg)
 {
-	return da9062_config_gpi(np, desc, cfg, "vsel");
+	int error;
+
+	error = da9062_config_gpi(np, desc, cfg, "vsel");
+	if (error)
+		return error;
+
+	return da9062_config_gpi(np, desc, cfg, "ena");
 }
 
 /* DA9061 Regulator information */
@@ -471,6 +482,10 @@ static const struct da9062_regulator_info local_da9061_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_VBUCK1_GPI_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_VBUCK1_GPI_MASK) - 1),
+		.ena_gpi = REG_FIELD(DA9062AA_BUCK1_CONT,
+			__builtin_ffs((int)DA9062AA_BUCK1_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_BUCK1_GPI_MASK) - 1),
 	},
 	{
 		.desc.id = DA9061_ID_BUCK2,
@@ -512,6 +527,10 @@ static const struct da9062_regulator_info local_da9061_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_VBUCK3_GPI_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_VBUCK3_GPI_MASK) - 1),
+		.ena_gpi = REG_FIELD(DA9062AA_BUCK3_CONT,
+			__builtin_ffs((int)DA9062AA_BUCK3_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_BUCK3_GPI_MASK) - 1),
 	},
 	{
 		.desc.id = DA9061_ID_BUCK3,
@@ -553,6 +572,10 @@ static const struct da9062_regulator_info local_da9061_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_VBUCK4_GPI_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_VBUCK4_GPI_MASK) - 1),
+		.ena_gpi = REG_FIELD(DA9062AA_BUCK4_CONT,
+			__builtin_ffs((int)DA9062AA_BUCK4_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_BUCK4_GPI_MASK) - 1),
 	},
 	{
 		.desc.id = DA9061_ID_LDO1,
@@ -587,6 +610,10 @@ static const struct da9062_regulator_info local_da9061_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_VLDO1_GPI_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_VLDO1_GPI_MASK) - 1),
+		.ena_gpi = REG_FIELD(DA9062AA_LDO1_CONT,
+			__builtin_ffs((int)DA9062AA_LDO1_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_LDO1_GPI_MASK) - 1),
 		.oc_event = REG_FIELD(DA9062AA_STATUS_D,
 			__builtin_ffs((int)DA9062AA_LDO1_ILIM_MASK) - 1,
 			sizeof(unsigned int) * 8 -
@@ -625,6 +652,10 @@ static const struct da9062_regulator_info local_da9061_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_VLDO2_GPI_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_VLDO2_GPI_MASK) - 1),
+		.ena_gpi = REG_FIELD(DA9062AA_LDO2_CONT,
+			__builtin_ffs((int)DA9062AA_LDO2_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_LDO2_GPI_MASK) - 1),
 		.oc_event = REG_FIELD(DA9062AA_STATUS_D,
 			__builtin_ffs((int)DA9062AA_LDO2_ILIM_MASK) - 1,
 			sizeof(unsigned int) * 8 -
@@ -663,6 +694,10 @@ static const struct da9062_regulator_info local_da9061_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_VLDO3_GPI_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_VLDO3_GPI_MASK) - 1),
+		.ena_gpi = REG_FIELD(DA9062AA_LDO3_CONT,
+			__builtin_ffs((int)DA9062AA_LDO3_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_LDO3_GPI_MASK) - 1),
 		.oc_event = REG_FIELD(DA9062AA_STATUS_D,
 			__builtin_ffs((int)DA9062AA_LDO3_ILIM_MASK) - 1,
 			sizeof(unsigned int) * 8 -
@@ -701,6 +736,10 @@ static const struct da9062_regulator_info local_da9061_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_VLDO4_GPI_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_VLDO4_GPI_MASK) - 1),
+		.ena_gpi = REG_FIELD(DA9062AA_LDO4_CONT,
+			__builtin_ffs((int)DA9062AA_LDO4_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_LDO4_GPI_MASK) - 1),
 		.oc_event = REG_FIELD(DA9062AA_STATUS_D,
 			__builtin_ffs((int)DA9062AA_LDO4_ILIM_MASK) - 1,
 			sizeof(unsigned int) * 8 -
@@ -750,6 +789,10 @@ static const struct da9062_regulator_info local_da9062_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_VBUCK1_GPI_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_VBUCK1_GPI_MASK) - 1),
+		.ena_gpi = REG_FIELD(DA9062AA_BUCK1_CONT,
+			__builtin_ffs((int)DA9062AA_BUCK1_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_BUCK1_GPI_MASK) - 1),
 	},
 	{
 		.desc.id = DA9062_ID_BUCK2,
@@ -791,6 +834,10 @@ static const struct da9062_regulator_info local_da9062_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_VBUCK2_GPI_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_VBUCK2_GPI_MASK) - 1),
+		.ena_gpi = REG_FIELD(DA9062AA_BUCK2_CONT,
+			__builtin_ffs((int)DA9062AA_BUCK2_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_BUCK2_GPI_MASK) - 1),
 	},
 	{
 		.desc.id = DA9062_ID_BUCK3,
@@ -832,6 +879,10 @@ static const struct da9062_regulator_info local_da9062_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_VBUCK3_GPI_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_VBUCK3_GPI_MASK) - 1),
+		.ena_gpi = REG_FIELD(DA9062AA_BUCK3_CONT,
+			__builtin_ffs((int)DA9062AA_BUCK3_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_BUCK3_GPI_MASK) - 1),
 	},
 	{
 		.desc.id = DA9062_ID_BUCK4,
@@ -873,6 +924,10 @@ static const struct da9062_regulator_info local_da9062_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_VBUCK4_GPI_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_VBUCK4_GPI_MASK) - 1),
+		.ena_gpi = REG_FIELD(DA9062AA_BUCK4_CONT,
+			__builtin_ffs((int)DA9062AA_BUCK4_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_BUCK4_GPI_MASK) - 1),
 	},
 	{
 		.desc.id = DA9062_ID_LDO1,
@@ -907,6 +962,10 @@ static const struct da9062_regulator_info local_da9062_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_VLDO1_GPI_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_VLDO1_GPI_MASK) - 1),
+		.ena_gpi = REG_FIELD(DA9062AA_LDO1_CONT,
+			__builtin_ffs((int)DA9062AA_LDO1_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_LDO1_GPI_MASK) - 1),
 		.oc_event = REG_FIELD(DA9062AA_STATUS_D,
 			__builtin_ffs((int)DA9062AA_LDO1_ILIM_MASK) - 1,
 			sizeof(unsigned int) * 8 -
@@ -945,6 +1004,10 @@ static const struct da9062_regulator_info local_da9062_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_VLDO2_GPI_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_VLDO2_GPI_MASK) - 1),
+		.ena_gpi = REG_FIELD(DA9062AA_LDO2_CONT,
+			__builtin_ffs((int)DA9062AA_LDO2_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_LDO2_GPI_MASK) - 1),
 		.oc_event = REG_FIELD(DA9062AA_STATUS_D,
 			__builtin_ffs((int)DA9062AA_LDO2_ILIM_MASK) - 1,
 			sizeof(unsigned int) * 8 -
@@ -983,6 +1046,10 @@ static const struct da9062_regulator_info local_da9062_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_VLDO3_GPI_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_VLDO3_GPI_MASK) - 1),
+		.ena_gpi = REG_FIELD(DA9062AA_LDO3_CONT,
+			__builtin_ffs((int)DA9062AA_LDO3_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_LDO3_GPI_MASK) - 1),
 		.oc_event = REG_FIELD(DA9062AA_STATUS_D,
 			__builtin_ffs((int)DA9062AA_LDO3_ILIM_MASK) - 1,
 			sizeof(unsigned int) * 8 -
@@ -1021,6 +1088,10 @@ static const struct da9062_regulator_info local_da9062_regulator_info[] = {
 			__builtin_ffs((int)DA9062AA_VLDO4_GPI_MASK) - 1,
 			sizeof(unsigned int) * 8 -
 			__builtin_clz(DA9062AA_VLDO4_GPI_MASK) - 1),
+		.ena_gpi = REG_FIELD(DA9062AA_LDO4_CONT,
+			__builtin_ffs((int)DA9062AA_LDO4_GPI_MASK) - 1,
+			sizeof(unsigned int) * 8 -
+			__builtin_clz(DA9062AA_LDO4_GPI_MASK) - 1),
 		.oc_event = REG_FIELD(DA9062AA_STATUS_D,
 			__builtin_ffs((int)DA9062AA_LDO4_ILIM_MASK) - 1,
 			sizeof(unsigned int) * 8 -
@@ -1150,6 +1221,15 @@ static int da9062_regulator_probe(struct platform_device *pdev)
 				return PTR_ERR(regl->vsel_gpi);
 		}
 
+		if (regl->info->ena_gpi.reg) {
+			regl->ena_gpi = devm_regmap_field_alloc(
+					&pdev->dev,
+					chip->regmap,
+					regl->info->ena_gpi);
+			if (IS_ERR(regl->ena_gpi))
+				return PTR_ERR(regl->ena_gpi);
+		}
+
 		/* Register regulator */
 		memset(&config, 0, sizeof(config));
 		config.dev = chip->dev;
-- 
2.20.1

Re: [PATCH v2 1/5] gpio: add support to get local gpio number

[Bartosz Golaszewski]

śr., 27 lis 2019 o 14:59 Marco Felsch <m.felsch@pengutronix.de> napisał(a):
>
> Sometimes consumers needs to know the gpio-chip local gpio number of a
> 'struct gpio_desc' for further configuration. This is often the case for
> mfd devices.
>

We already have this support. It's just a matter of exporting it, so
maybe adjust the commit message to not be confusing.

That being said: I'm not really a fan of this - the whole idea of gpio
descriptors was to make them opaque and their hardware offsets
irrelevant. :(

> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  drivers/gpio/gpiolib.c        |  6 ++++++
>  include/linux/gpio/consumer.h | 10 ++++++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 104ed299d5ea..7709648313fc 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -4377,6 +4377,12 @@ int gpiod_count(struct device *dev, const char *con_id)
>  }
>  EXPORT_SYMBOL_GPL(gpiod_count);
>
> +int gpiod_to_offset(struct gpio_desc *desc)

Maybe call it: gpiod_desc_to_offset()?

> +{
> +       return gpio_chip_hwgpio(desc);
> +}
> +EXPORT_SYMBOL_GPL(gpiod_to_offset);
> +
>  /**
>   * gpiod_get - obtain a GPIO for a given GPIO function
>   * @dev:       GPIO consumer, can be NULL for system-global GPIOs
> diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
> index b70af921c614..e2178c3bf7fd 100644
> --- a/include/linux/gpio/consumer.h
> +++ b/include/linux/gpio/consumer.h
> @@ -60,6 +60,9 @@ enum gpiod_flags {
>  /* Return the number of GPIOs associated with a device / function */
>  int gpiod_count(struct device *dev, const char *con_id);
>
> +/* Get the local chip offset from a global desc */
> +int gpiod_to_offset(struct gpio_desc *desc);
> +
>  /* Acquire and dispose GPIOs */
>  struct gpio_desc *__must_check gpiod_get(struct device *dev,
>                                          const char *con_id,
> @@ -189,6 +192,13 @@ static inline int gpiod_count(struct device *dev, const char *con_id)
>         return 0;
>  }
>
> +static inline int gpiod_to_offset(struct gpio_desc *desc)
> +{
> +       /* GPIO can never have been requested */
> +       WARN_ON(desc);
> +       return 0;
> +}
> +
>  static inline struct gpio_desc *__must_check gpiod_get(struct device *dev,
>                                                        const char *con_id,
>                                                        enum gpiod_flags flags)
> --
> 2.20.1
>

Re: [PATCH v2 1/5] gpio: add support to get local gpio number

[Marco Felsch]

On 19-11-28 11:46, Bartosz Golaszewski wrote:
> śr., 27 lis 2019 o 14:59 Marco Felsch <m.felsch@pengutronix.de> napisał(a):
> >
> > Sometimes consumers needs to know the gpio-chip local gpio number of a
> > 'struct gpio_desc' for further configuration. This is often the case for
> > mfd devices.
> >
> 
> We already have this support. It's just a matter of exporting it, so
> maybe adjust the commit message to not be confusing.

Therefore I mentioned the consumers.

> That being said: I'm not really a fan of this - the whole idea of gpio
> descriptors was to make them opaque and their hardware offsets
> irrelevant. :(

I know therefore I added a driver local helper but this wasn't the way
Linus wanted to go..

> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >  drivers/gpio/gpiolib.c        |  6 ++++++
> >  include/linux/gpio/consumer.h | 10 ++++++++++
> >  2 files changed, 16 insertions(+)
> >
> > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > index 104ed299d5ea..7709648313fc 100644
> > --- a/drivers/gpio/gpiolib.c
> > +++ b/drivers/gpio/gpiolib.c
> > @@ -4377,6 +4377,12 @@ int gpiod_count(struct device *dev, const char *con_id)
> >  }
> >  EXPORT_SYMBOL_GPL(gpiod_count);
> >
> > +int gpiod_to_offset(struct gpio_desc *desc)
> 
> Maybe call it: gpiod_desc_to_offset()?

The function name is proposed by Linus too so Linus what's your
oppinion?

Regards,
  Marco

> > +{
> > +       return gpio_chip_hwgpio(desc);
> > +}
> > +EXPORT_SYMBOL_GPL(gpiod_to_offset);
> > +
> >  /**
> >   * gpiod_get - obtain a GPIO for a given GPIO function
> >   * @dev:       GPIO consumer, can be NULL for system-global GPIOs
> > diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
> > index b70af921c614..e2178c3bf7fd 100644
> > --- a/include/linux/gpio/consumer.h
> > +++ b/include/linux/gpio/consumer.h
> > @@ -60,6 +60,9 @@ enum gpiod_flags {
> >  /* Return the number of GPIOs associated with a device / function */
> >  int gpiod_count(struct device *dev, const char *con_id);
> >
> > +/* Get the local chip offset from a global desc */
> > +int gpiod_to_offset(struct gpio_desc *desc);
> > +
> >  /* Acquire and dispose GPIOs */
> >  struct gpio_desc *__must_check gpiod_get(struct device *dev,
> >                                          const char *con_id,
> > @@ -189,6 +192,13 @@ static inline int gpiod_count(struct device *dev, const char *con_id)
> >         return 0;
> >  }
> >
> > +static inline int gpiod_to_offset(struct gpio_desc *desc)
> > +{
> > +       /* GPIO can never have been requested */
> > +       WARN_ON(desc);
> > +       return 0;
> > +}
> > +
> >  static inline struct gpio_desc *__must_check gpiod_get(struct device *dev,
> >                                                        const char *con_id,
> >                                                        enum gpiod_flags flags)
> > --
> > 2.20.1
> >

Re: [PATCH v2 1/5] gpio: add support to get local gpio number

[Uwe Kleine-König]

On Thu, Nov 28, 2019 at 01:49:42PM +0100, Marco Felsch wrote:
> On 19-11-28 11:46, Bartosz Golaszewski wrote:
> > śr., 27 lis 2019 o 14:59 Marco Felsch <m.felsch@pengutronix.de> napisał(a):
> > >
> > > Sometimes consumers needs to know the gpio-chip local gpio number of a
> > > 'struct gpio_desc' for further configuration. This is often the case for
> > > mfd devices.
> > >
> > 
> > We already have this support. It's just a matter of exporting it, so
> > maybe adjust the commit message to not be confusing.
> 
> Therefore I mentioned the consumers.
> 
> > That being said: I'm not really a fan of this - the whole idea of gpio
> > descriptors was to make them opaque and their hardware offsets
> > irrelevant. :(
> 
> I know therefore I added a driver local helper but this wasn't the way
> Linus wanted to go..
> 
> > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > ---
> > >  drivers/gpio/gpiolib.c        |  6 ++++++
> > >  include/linux/gpio/consumer.h | 10 ++++++++++
> > >  2 files changed, 16 insertions(+)
> > >
> > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > > index 104ed299d5ea..7709648313fc 100644
> > > --- a/drivers/gpio/gpiolib.c
> > > +++ b/drivers/gpio/gpiolib.c
> > > @@ -4377,6 +4377,12 @@ int gpiod_count(struct device *dev, const char *con_id)
> > >  }
> > >  EXPORT_SYMBOL_GPL(gpiod_count);
> > >
> > > +int gpiod_to_offset(struct gpio_desc *desc)
> > 
> > Maybe call it: gpiod_desc_to_offset()?
> 
> The function name is proposed by Linus too so Linus what's your
> oppinion?

INAL (I'm not a Linus) but I wonder what the 'd' in gpiod stands for.
Assuming it already meand "desc" I'd prefer gpiod_to_offset.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Re: [PATCH v2 1/5] gpio: add support to get local gpio number

[Marco Felsch]

On 19-11-29 08:45, Uwe Kleine-König wrote:
> On Thu, Nov 28, 2019 at 01:49:42PM +0100, Marco Felsch wrote:
> > On 19-11-28 11:46, Bartosz Golaszewski wrote:
> > > śr., 27 lis 2019 o 14:59 Marco Felsch <m.felsch@pengutronix.de> napisał(a):
> > > >
> > > > Sometimes consumers needs to know the gpio-chip local gpio number of a
> > > > 'struct gpio_desc' for further configuration. This is often the case for
> > > > mfd devices.
> > > >
> > > 
> > > We already have this support. It's just a matter of exporting it, so
> > > maybe adjust the commit message to not be confusing.
> > 
> > Therefore I mentioned the consumers.
> > 
> > > That being said: I'm not really a fan of this - the whole idea of gpio
> > > descriptors was to make them opaque and their hardware offsets
> > > irrelevant. :(
> > 
> > I know therefore I added a driver local helper but this wasn't the way
> > Linus wanted to go..
> > 
> > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > > ---
> > > >  drivers/gpio/gpiolib.c        |  6 ++++++
> > > >  include/linux/gpio/consumer.h | 10 ++++++++++
> > > >  2 files changed, 16 insertions(+)
> > > >
> > > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > > > index 104ed299d5ea..7709648313fc 100644
> > > > --- a/drivers/gpio/gpiolib.c
> > > > +++ b/drivers/gpio/gpiolib.c
> > > > @@ -4377,6 +4377,12 @@ int gpiod_count(struct device *dev, const char *con_id)
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(gpiod_count);
> > > >
> > > > +int gpiod_to_offset(struct gpio_desc *desc)
> > > 
> > > Maybe call it: gpiod_desc_to_offset()?
> > 
> > The function name is proposed by Linus too so Linus what's your
> > oppinion?
> 
> INAL (I'm not a Linus) but I wonder what the 'd' in gpiod stands for.
> Assuming it already meand "desc" I'd prefer gpiod_to_offset.

Yes, that was my assumption too.

Regards,
  Marco

> Best regards
> Uwe
> 
> -- 
> Pengutronix e.K.                           | Uwe Kleine-König            |
> Industrial Linux Solutions                 | https://www.pengutronix.de/ |
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v2 5/5] regulator: da9062: add gpio based regulator dis-/enable support

[Linus Walleij]

On Wed, Nov 27, 2019 at 2:59 PM Marco Felsch <m.felsch@pengutronix.de> wrote:

> Each regulator can be enabeld/disabled by the internal pmic state
> machine or by a gpio input signal. Typically the OTP configures the
> regulators to be enabled/disabled on a specific sequence number which is
> most the time fine. Sometimes we need to reconfigure that due to a PCB
> bug. This patch adds the support to disable/enable the regulator based
> on a gpio input signal.
>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

Overall I think this is fine, it's a solid use case that need to be
supported.

> +       struct reg_field ena_gpi;

Maybe just add some doc comment to this struct member?

IIUC it is a general purpose input that can be configured
such that it will enable one of the DA9062 regulators when
asserted. (Correct?)

Yours,
Linus Walleij

Re: [PATCH v2 3/5] regulator: da9062: add voltage selection gpio support

[Linus Walleij]

On Wed, Nov 27, 2019 at 2:59 PM Marco Felsch <m.felsch@pengutronix.de> wrote:

> The DA9062/1 devices can switch their regulator voltages between
> voltage-A (active) and voltage-B (suspend) settings. Switching the
> voltages can be controlled by ther internal state-machine or by a gpio
> input signal and can be configured for each individual regulator. This
> commit adds the gpio-based voltage switching support.
>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
> Changelog:
>
> v2:
> - use new public api gpiod_to_offset()

OK this is better in my opinion, at least it is a lesser evil than
the hacks I've seen.

> +       struct reg_field vsel_gpi;

Again add some comments to the code describing what this is
about please. A general purpose input that can be configured
such that it is not a general purpose input anymore, but instead
looped back internally to control a voltage on the DA9062.

Part of me wonder if these lines are really "general purpose"
but I suppose software could use them.

Yours,
Linus Walleij

Re: [PATCH v2 4/5] dt-bindings: mfd: da9062: add regulator gpio enable/disable documentation

[Linus Walleij]

On Wed, Nov 27, 2019 at 2:59 PM Marco Felsch <m.felsch@pengutronix.de> wrote:

> At the gpio-based regulator enable/disable documentation. This property
> can be applied to each subnode within the 'regulators' node so each
> regulator can be configured differently.
>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
(...)
> +  - dlg,ena-sense-gpios : The GPIO reference which should be used by the
> +    regulator to enable/disable the output. If the signal is active the
> +    regulator is on else it is off. Attention: Sharing the same gpio for other
> +    purposes or across multiple regulators is possible but the gpio settings
> +    must be the same. Also the gpio phandle must refer to the mfd root node
> +    other gpios are not allowed and make no sense.

- Point out that this concerns references to the GPI general purpose
input on the DA9062 itself, and not just "any" GPIO. The last sentence
tries to say this but it should be stated more clearly.

- Clarify which "gpio settings" must be the same, e.g. polarity I guess.

Yours,
Linus Walleij

Re: [PATCH v2 5/5] regulator: da9062: add gpio based regulator dis-/enable support

[Marco Felsch]

On 19-11-29 09:25, Linus Walleij wrote:
> On Wed, Nov 27, 2019 at 2:59 PM Marco Felsch <m.felsch@pengutronix.de> wrote:
> 
> > Each regulator can be enabeld/disabled by the internal pmic state
> > machine or by a gpio input signal. Typically the OTP configures the
> > regulators to be enabled/disabled on a specific sequence number which is
> > most the time fine. Sometimes we need to reconfigure that due to a PCB
> > bug. This patch adds the support to disable/enable the regulator based
> > on a gpio input signal.
> >
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> 
> Overall I think this is fine, it's a solid use case that need to be
> supported.
> 
> > +       struct reg_field ena_gpi;
> 
> Maybe just add some doc comment to this struct member?

Yes, I can do that.

> IIUC it is a general purpose input that can be configured
> such that it will enable one of the DA9062 regulators when
> asserted. (Correct?)

Yes that's correct.

Regards,
  Marco

> Yours,
> Linus Walleij
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v2 1/5] gpio: add support to get local gpio number

[Linus Walleij]

Hi Marco,

thanks for your patch!

On Wed, Nov 27, 2019 at 2:59 PM Marco Felsch <m.felsch@pengutronix.de> wrote:

> Sometimes consumers needs to know the gpio-chip local gpio number of a
> 'struct gpio_desc' for further configuration. This is often the case for
> mfd devices.
>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
(...)
> +int gpiod_to_offset(struct gpio_desc *desc)
> +{
> +       return gpio_chip_hwgpio(desc);
> +}
> +EXPORT_SYMBOL_GPL(gpiod_to_offset);

That seems like an unnecessary wrapper.

What about renaming gpio_chip_hwgpio() everywhere
to gpiod_to_offet(), remove it from drivers/gpio/gpiolib.h
and export it in <linux/gpio/consumer.h> instead?

I suppose this is what Bartosz is indicating, not sure though.

Indeed it is a bit of a worrysome thing to export and we need
to be very specific about its usecase, so I'd also like some
nice to-the-point kerneldoc on the export site so that it is
clear what corner cases this function is for. (Like in this
specific driver.)

Yours,
Linus Walleij

Re: [PATCH v2 1/5] gpio: add support to get local gpio number

[Marco Felsch]

Hi Linus,

On 19-11-29 10:32, Linus Walleij wrote:
> Hi Marco,
> 
> thanks for your patch!
> 
> On Wed, Nov 27, 2019 at 2:59 PM Marco Felsch <m.felsch@pengutronix.de> wrote:
> 
> > Sometimes consumers needs to know the gpio-chip local gpio number of a
> > 'struct gpio_desc' for further configuration. This is often the case for
> > mfd devices.
> >
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> (...)
> > +int gpiod_to_offset(struct gpio_desc *desc)
> > +{
> > +       return gpio_chip_hwgpio(desc);
> > +}
> > +EXPORT_SYMBOL_GPL(gpiod_to_offset);
> 
> That seems like an unnecessary wrapper.

I went this way due to minimal changes..

> What about renaming gpio_chip_hwgpio() everywhere
> to gpiod_to_offet(), remove it from drivers/gpio/gpiolib.h
> and export it in <linux/gpio/consumer.h> instead?

That's also possible but then we have to include the consumer.h header
within the gpiolib.c and this seems to be wrong. But since I'm not the
maintainer it is up to you and Bart. Both ways are possible,

> I suppose this is what Bartosz is indicating, not sure though.
> 
> Indeed it is a bit of a worrysome thing to export and we need
> to be very specific about its usecase, so I'd also like some
> nice to-the-point kerneldoc on the export site so that it is
> clear what corner cases this function is for. (Like in this
> specific driver.)

Absolutly, I missed the kerneldoc.. but where should I put the kerneldoc
if we go the 'wrapper'-way?

Regards,
  Marco

> Yours,
> Linus Walleij
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v2 1/5] gpio: add support to get local gpio number

[Linus Walleij]

On Fri, Nov 29, 2019 at 11:15 AM Marco Felsch <m.felsch@pengutronix.de> wrote:

> > What about renaming gpio_chip_hwgpio() everywhere
> > to gpiod_to_offet(), remove it from drivers/gpio/gpiolib.h
> > and export it in <linux/gpio/consumer.h> instead?
>
> That's also possible but then we have to include the consumer.h header
> within the gpiolib.c and this seems to be wrong. But since I'm not the
> maintainer it is up to you and Bart. Both ways are possible,

What about following the pattern by the clk subsystem and
create <linux/gpio/private.h> and put it there?

It should be an indication to people to not use these features
lightly. We can decorate the header file with some warnings.

Yours,
Linus Walleij

Re: [PATCH v2 1/5] gpio: add support to get local gpio number

[Marco Felsch]

On 19-11-29 11:19, Linus Walleij wrote:
> On Fri, Nov 29, 2019 at 11:15 AM Marco Felsch <m.felsch@pengutronix.de> wrote:
> 
> > > What about renaming gpio_chip_hwgpio() everywhere
> > > to gpiod_to_offet(), remove it from drivers/gpio/gpiolib.h
> > > and export it in <linux/gpio/consumer.h> instead?
> >
> > That's also possible but then we have to include the consumer.h header
> > within the gpiolib.c and this seems to be wrong. But since I'm not the
> > maintainer it is up to you and Bart. Both ways are possible,
> 
> What about following the pattern by the clk subsystem and
> create <linux/gpio/private.h> and put it there?
> 
> It should be an indication to people to not use these features
> lightly. We can decorate the header file with some warnings.

That's a good idea. So the following points should be done:
  - rename gpio_chip_hwgpio() to gpiod_to_offset() or gpiod_to_local_offset()
  - move the new helper to <linux/gpio/private.h>
  - add kerneldoc
  - add warnings into the header

Regards,
  Marco

> Yours,
> Linus Walleij
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v2 1/5] gpio: add support to get local gpio number

[Linus Walleij]

On Fri, Nov 29, 2019 at 12:36 PM Marco Felsch <m.felsch@pengutronix.de> wrote:
> On 19-11-29 11:19, Linus Walleij wrote:
> > On Fri, Nov 29, 2019 at 11:15 AM Marco Felsch <m.felsch@pengutronix.de> wrote:
> >
> > > > What about renaming gpio_chip_hwgpio() everywhere
> > > > to gpiod_to_offet(), remove it from drivers/gpio/gpiolib.h
> > > > and export it in <linux/gpio/consumer.h> instead?
> > >
> > > That's also possible but then we have to include the consumer.h header
> > > within the gpiolib.c and this seems to be wrong. But since I'm not the
> > > maintainer it is up to you and Bart. Both ways are possible,
> >
> > What about following the pattern by the clk subsystem and
> > create <linux/gpio/private.h> and put it there?
> >
> > It should be an indication to people to not use these features
> > lightly. We can decorate the header file with some warnings.
>
> That's a good idea. So the following points should be done:
>   - rename gpio_chip_hwgpio() to gpiod_to_offset() or gpiod_to_local_offset()
>   - move the new helper to <linux/gpio/private.h>
>   - add kerneldoc
>   - add warnings into the header

Ack!

Linus