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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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, ®l->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
śr., 27 lis 2019 o 14:59 Marco Felsch <[email protected]> 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 <[email protected]>
> ---
> 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
>
On 19-11-28 11:46, Bartosz Golaszewski wrote:
> śr., 27 lis 2019 o 14:59 Marco Felsch <[email protected]> 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 <[email protected]>
> > ---
> > 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
> >
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 <[email protected]> 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 <[email protected]>
> > > ---
> > > 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/ |
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 <[email protected]> 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 <[email protected]>
> > > > ---
> > > > 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 |
On Wed, Nov 27, 2019 at 2:59 PM Marco Felsch <[email protected]> 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 <[email protected]>
> ---
> 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
Hi Marco,
thanks for your patch!
On Wed, Nov 27, 2019 at 2:59 PM Marco Felsch <[email protected]> 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 <[email protected]>
(...)
> +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
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 <[email protected]> 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 <[email protected]>
> (...)
> > +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 |
On Fri, Nov 29, 2019 at 11:15 AM Marco Felsch <[email protected]> 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
On 19-11-29 11:19, Linus Walleij wrote:
> On Fri, Nov 29, 2019 at 11:15 AM Marco Felsch <[email protected]> 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 |
On Fri, Nov 29, 2019 at 12:36 PM Marco Felsch <[email protected]> wrote:
> On 19-11-29 11:19, Linus Walleij wrote:
> > On Fri, Nov 29, 2019 at 11:15 AM Marco Felsch <[email protected]> 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