Re: [PATCH v4 4/8] regulator: stpmic1: add stpmic1 regulator driver
On Thu, Oct 25, 2018 at 01:23:08PM +, Pascal PAILLET-LME wrote:
> I have reworked the code so that we don't touch any more to the init_data.
> the new loop to register the regulators is below:
>
> for (i = 0; i < ARRAY_SIZE(stpmic1_regulator_cfgs); i++) {
> ret = stpmic1_regulator_register(pdev, i, _matches[i],
> regul);
> if (ret < 0)
> return ret;
> regul++;
> }
>
> Each regulator is registered, even is it is not described in the
> device-tree.
Looks good.
signature.asc
Description: PGP signature
Re: [PATCH v4 4/8] regulator: stpmic1: add stpmic1 regulator driver
On Thu, Oct 25, 2018 at 01:23:08PM +, Pascal PAILLET-LME wrote:
> I have reworked the code so that we don't touch any more to the init_data.
> the new loop to register the regulators is below:
>
> for (i = 0; i < ARRAY_SIZE(stpmic1_regulator_cfgs); i++) {
> ret = stpmic1_regulator_register(pdev, i, _matches[i],
> regul);
> if (ret < 0)
> return ret;
> regul++;
> }
>
> Each regulator is registered, even is it is not described in the
> device-tree.
Looks good.
signature.asc
Description: PGP signature
Re: [PATCH v4 4/8] regulator: stpmic1: add stpmic1 regulator driver
Hello Mark,
Le 10/24/2018 03:17 PM, Mark Brown a écrit :
> On Wed, Oct 24, 2018 at 12:54:46PM +, Pascal PAILLET-LME wrote:
>
>> I'm sorry, I'm not sure to understand. Would you prefer to not register
>> regulators that
>> are not described in the device-tree ?
> No, I'm saying register all regulators regardless of if they are in the
> device tree - you shouldn't be looking at the init data at all here,
> just let the framework match them using of_match.
I have reworked the code so that we don't touch any more to the init_data.
the new loop to register the regulators is below:
for (i = 0; i < ARRAY_SIZE(stpmic1_regulator_cfgs); i++) {
ret = stpmic1_regulator_register(pdev, i, _matches[i],
regul);
if (ret < 0)
return ret;
regul++;
}
Each regulator is registered, even is it is not described in the
device-tree.
stpmic1_regulator_parse_dt() and stpmic1_regulator_init() are now merged
inside
stpmic1_regulator_register() function.
Thank You,
pascal
Re: [PATCH v4 4/8] regulator: stpmic1: add stpmic1 regulator driver
Hello Mark,
Le 10/24/2018 03:17 PM, Mark Brown a écrit :
> On Wed, Oct 24, 2018 at 12:54:46PM +, Pascal PAILLET-LME wrote:
>
>> I'm sorry, I'm not sure to understand. Would you prefer to not register
>> regulators that
>> are not described in the device-tree ?
> No, I'm saying register all regulators regardless of if they are in the
> device tree - you shouldn't be looking at the init data at all here,
> just let the framework match them using of_match.
I have reworked the code so that we don't touch any more to the init_data.
the new loop to register the regulators is below:
for (i = 0; i < ARRAY_SIZE(stpmic1_regulator_cfgs); i++) {
ret = stpmic1_regulator_register(pdev, i, _matches[i],
regul);
if (ret < 0)
return ret;
regul++;
}
Each regulator is registered, even is it is not described in the
device-tree.
stpmic1_regulator_parse_dt() and stpmic1_regulator_init() are now merged
inside
stpmic1_regulator_register() function.
Thank You,
pascal
Re: [PATCH v4 4/8] regulator: stpmic1: add stpmic1 regulator driver
On Wed, Oct 24, 2018 at 12:54:46PM +, Pascal PAILLET-LME wrote: > I'm sorry, I'm not sure to understand. Would you prefer to not register > regulators that > are not described in the device-tree ? No, I'm saying register all regulators regardless of if they are in the device tree - you shouldn't be looking at the init data at all here, just let the framework match them using of_match. signature.asc Description: PGP signature
Re: [PATCH v4 4/8] regulator: stpmic1: add stpmic1 regulator driver
On Wed, Oct 24, 2018 at 12:54:46PM +, Pascal PAILLET-LME wrote: > I'm sorry, I'm not sure to understand. Would you prefer to not register > regulators that > are not described in the device-tree ? No, I'm saying register all regulators regardless of if they are in the device tree - you shouldn't be looking at the init data at all here, just let the framework match them using of_match. signature.asc Description: PGP signature
Re: [PATCH v4 4/8] regulator: stpmic1: add stpmic1 regulator driver
Hello Mark,
Le 10/19/2018 01:50 PM, Mark Brown a écrit :
> On Thu, Oct 18, 2018 at 09:02:12AM +, Pascal PAILLET-LME wrote:
>
>> +for (i = 0; i < ARRAY_SIZE(stpmic1_regulator_cfgs); i++) {
>> +/* Parse DT & find regulators to register */
>> +init_data = stpmic1_regulators_matches[i].init_data;
>> +if (init_data)
>> +init_data->regulator_init = _regulator_parse_dt;
>> +
>> +rdev = stpmic1_regulator_register(pdev, i, init_data, regul);
>> +if (IS_ERR(rdev))
>> +return PTR_ERR(rdev);
> This looks mostly good, the only big thing is this - the default is to
> just unconditionally register all the regulators that exist rather than
> only those that are configured on that particular platform. This is a
> bit simpler and means that all the readback of the configuration for the
> unconfigured regulators is available for diagnostics. Is there a reason
> not to do that?
I'm sorry, I'm not sure to understand. Would you prefer to not register
regulators that
are not described in the device-tree ?
In that case I could add:
if (!init_data)
continue;
This would permit to keep some regulators unmodified by the kernel. This
can be useful
because we have some regulators configured by boot loaders (for the DDR
at least) and
it would be more simple to not handle them in the kernel.
best regards,
pascal
Re: [PATCH v4 4/8] regulator: stpmic1: add stpmic1 regulator driver
Hello Mark,
Le 10/19/2018 01:50 PM, Mark Brown a écrit :
> On Thu, Oct 18, 2018 at 09:02:12AM +, Pascal PAILLET-LME wrote:
>
>> +for (i = 0; i < ARRAY_SIZE(stpmic1_regulator_cfgs); i++) {
>> +/* Parse DT & find regulators to register */
>> +init_data = stpmic1_regulators_matches[i].init_data;
>> +if (init_data)
>> +init_data->regulator_init = _regulator_parse_dt;
>> +
>> +rdev = stpmic1_regulator_register(pdev, i, init_data, regul);
>> +if (IS_ERR(rdev))
>> +return PTR_ERR(rdev);
> This looks mostly good, the only big thing is this - the default is to
> just unconditionally register all the regulators that exist rather than
> only those that are configured on that particular platform. This is a
> bit simpler and means that all the readback of the configuration for the
> unconfigured regulators is available for diagnostics. Is there a reason
> not to do that?
I'm sorry, I'm not sure to understand. Would you prefer to not register
regulators that
are not described in the device-tree ?
In that case I could add:
if (!init_data)
continue;
This would permit to keep some regulators unmodified by the kernel. This
can be useful
because we have some regulators configured by boot loaders (for the DDR
at least) and
it would be more simple to not handle them in the kernel.
best regards,
pascal
Re: [PATCH v4 4/8] regulator: stpmic1: add stpmic1 regulator driver
On Thu, Oct 18, 2018 at 09:02:12AM +, Pascal PAILLET-LME wrote:
> + for (i = 0; i < ARRAY_SIZE(stpmic1_regulator_cfgs); i++) {
> + /* Parse DT & find regulators to register */
> + init_data = stpmic1_regulators_matches[i].init_data;
> + if (init_data)
> + init_data->regulator_init = _regulator_parse_dt;
> +
> + rdev = stpmic1_regulator_register(pdev, i, init_data, regul);
> + if (IS_ERR(rdev))
> + return PTR_ERR(rdev);
This looks mostly good, the only big thing is this - the default is to
just unconditionally register all the regulators that exist rather than
only those that are configured on that particular platform. This is a
bit simpler and means that all the readback of the configuration for the
unconfigured regulators is available for diagnostics. Is there a reason
not to do that?
signature.asc
Description: PGP signature
Re: [PATCH v4 4/8] regulator: stpmic1: add stpmic1 regulator driver
On Thu, Oct 18, 2018 at 09:02:12AM +, Pascal PAILLET-LME wrote:
> + for (i = 0; i < ARRAY_SIZE(stpmic1_regulator_cfgs); i++) {
> + /* Parse DT & find regulators to register */
> + init_data = stpmic1_regulators_matches[i].init_data;
> + if (init_data)
> + init_data->regulator_init = _regulator_parse_dt;
> +
> + rdev = stpmic1_regulator_register(pdev, i, init_data, regul);
> + if (IS_ERR(rdev))
> + return PTR_ERR(rdev);
This looks mostly good, the only big thing is this - the default is to
just unconditionally register all the regulators that exist rather than
only those that are configured on that particular platform. This is a
bit simpler and means that all the readback of the configuration for the
unconfigured regulators is available for diagnostics. Is there a reason
not to do that?
signature.asc
Description: PGP signature
[PATCH v4 4/8] regulator: stpmic1: add stpmic1 regulator driver
From: pascal paillet
The stpmic1 PMIC embeds several regulators and switches with
different capabilities.
Signed-off-by: pascal paillet
---
changes in v4: nothing
drivers/regulator/Kconfig | 12 +
drivers/regulator/Makefile| 1 +
drivers/regulator/stpmic1_regulator.c | 674 ++
3 files changed, 687 insertions(+)
create mode 100644 drivers/regulator/stpmic1_regulator.c
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 6e96ef1..026d480 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -803,6 +803,18 @@ config REGULATOR_STM32_VREFBUF
This driver can also be built as a module. If so, the module
will be called stm32-vrefbuf.
+config REGULATOR_STPMIC1
+ tristate "STMicroelectronics STPMIC1 PMIC Regulators"
+ depends on MFD_STPMIC1
+ help
+ This driver supports STMicroelectronics STPMIC1 PMIC voltage
+ regulators and switches. The STPMIC1 regulators supply power to
+ an application processor as well as to external system
+ peripherals such as DDR, Flash memories and system devices.
+
+ To compile this driver as a module, choose M here: the
+ module will be called stpmic1_regulator.
+
config REGULATOR_TI_ABB
tristate "TI Adaptive Body Bias on-chip LDO"
depends on ARCH_OMAP
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index eac4d79..300bc4c 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -101,6 +101,7 @@ obj-$(CONFIG_REGULATOR_S5M8767) += s5m8767.o
obj-$(CONFIG_REGULATOR_SC2731) += sc2731-regulator.o
obj-$(CONFIG_REGULATOR_SKY81452) += sky81452-regulator.o
obj-$(CONFIG_REGULATOR_STM32_VREFBUF) += stm32-vrefbuf.o
+obj-$(CONFIG_REGULATOR_STPMIC1) += stpmic1_regulator.o
obj-$(CONFIG_REGULATOR_STW481X_VMMC) += stw481x-vmmc.o
obj-$(CONFIG_REGULATOR_SY8106A) += sy8106a-regulator.o
obj-$(CONFIG_REGULATOR_TI_ABB) += ti-abb-regulator.o
diff --git a/drivers/regulator/stpmic1_regulator.c
b/drivers/regulator/stpmic1_regulator.c
new file mode 100644
index 000..96f1808
--- /dev/null
+++ b/drivers/regulator/stpmic1_regulator.c
@@ -0,0 +1,674 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) STMicroelectronics 2018
+// Author: Pascal Paillet for STMicroelectronics.
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/**
+ * stpmic1 regulator description
+ * @desc: regulator framework description
+ * @mask_reset_reg: mask reset register address
+ * @mask_reset_mask: mask rank and mask reset register mask
+ * @icc_reg: icc register address
+ * @icc_mask: icc register mask
+ */
+struct stpmic1_regulator_cfg {
+ struct regulator_desc desc;
+ u8 mask_reset_reg;
+ u8 mask_reset_mask;
+ u8 icc_reg;
+ u8 icc_mask;
+};
+
+/**
+ * stpmic1 regulator data: this structure is used as driver data
+ * @regul_id: regulator id
+ * @reg_node: DT node of regulator (unused on non-DT platforms)
+ * @cfg: stpmic specific regulator description
+ * @mask_reset: mask_reset bit value
+ * @irq_curlim: current limit interrupt number
+ * @regmap: point to parent regmap structure
+ */
+struct stpmic1_regulator {
+ unsigned int regul_id;
+ struct device_node *reg_node;
+ struct stpmic1_regulator_cfg *cfg;
+ u8 mask_reset;
+ int irq_curlim;
+ struct regmap *regmap;
+};
+
+static int stpmic1_set_mode(struct regulator_dev *rdev, unsigned int mode);
+static unsigned int stpmic1_get_mode(struct regulator_dev *rdev);
+static int stpmic1_set_icc(struct regulator_dev *rdev);
+static int stpmic1_regulator_parse_dt(void *driver_data);
+static unsigned int stpmic1_map_mode(unsigned int mode);
+
+enum {
+ STPMIC1_BUCK1 = 0,
+ STPMIC1_BUCK2 = 1,
+ STPMIC1_BUCK3 = 2,
+ STPMIC1_BUCK4 = 3,
+ STPMIC1_LDO1 = 4,
+ STPMIC1_LDO2 = 5,
+ STPMIC1_LDO3 = 6,
+ STPMIC1_LDO4 = 7,
+ STPMIC1_LDO5 = 8,
+ STPMIC1_LDO6 = 9,
+ STPMIC1_VREF_DDR = 10,
+ STPMIC1_BOOST = 11,
+ STPMIC1_VBUS_OTG = 12,
+ STPMIC1_SW_OUT = 13,
+};
+
+/* Enable time worst case is 5000mV/(2250uV/uS) */
+#define PMIC_ENABLE_TIME_US 2200
+
+#define STPMIC1_BUCK_MODE_NORMAL 0
+#define STPMIC1_BUCK_MODE_LP BUCK_HPLP_ENABLE_MASK
+
+struct regulator_linear_range buck1_ranges[] = {
+ REGULATOR_LINEAR_RANGE(60, 0, 30, 25000),
+ REGULATOR_LINEAR_RANGE(135, 31, 63, 0),
+};
+
+struct regulator_linear_range buck2_ranges[] = {
+ REGULATOR_LINEAR_RANGE(100, 0, 17, 0),
+ REGULATOR_LINEAR_RANGE(105, 18, 19, 0),
+ REGULATOR_LINEAR_RANGE(110, 20, 21, 0),
+ REGULATOR_LINEAR_RANGE(115, 22, 23, 0),
+ REGULATOR_LINEAR_RANGE(120, 24, 25, 0),
+ REGULATOR_LINEAR_RANGE(125, 26, 27, 0),
+ REGULATOR_LINEAR_RANGE(130, 28, 29, 0),
+ REGULATOR_LINEAR_RANGE(135, 30, 31,
[PATCH v4 4/8] regulator: stpmic1: add stpmic1 regulator driver
From: pascal paillet
The stpmic1 PMIC embeds several regulators and switches with
different capabilities.
Signed-off-by: pascal paillet
---
changes in v4: nothing
drivers/regulator/Kconfig | 12 +
drivers/regulator/Makefile| 1 +
drivers/regulator/stpmic1_regulator.c | 674 ++
3 files changed, 687 insertions(+)
create mode 100644 drivers/regulator/stpmic1_regulator.c
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 6e96ef1..026d480 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -803,6 +803,18 @@ config REGULATOR_STM32_VREFBUF
This driver can also be built as a module. If so, the module
will be called stm32-vrefbuf.
+config REGULATOR_STPMIC1
+ tristate "STMicroelectronics STPMIC1 PMIC Regulators"
+ depends on MFD_STPMIC1
+ help
+ This driver supports STMicroelectronics STPMIC1 PMIC voltage
+ regulators and switches. The STPMIC1 regulators supply power to
+ an application processor as well as to external system
+ peripherals such as DDR, Flash memories and system devices.
+
+ To compile this driver as a module, choose M here: the
+ module will be called stpmic1_regulator.
+
config REGULATOR_TI_ABB
tristate "TI Adaptive Body Bias on-chip LDO"
depends on ARCH_OMAP
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index eac4d79..300bc4c 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -101,6 +101,7 @@ obj-$(CONFIG_REGULATOR_S5M8767) += s5m8767.o
obj-$(CONFIG_REGULATOR_SC2731) += sc2731-regulator.o
obj-$(CONFIG_REGULATOR_SKY81452) += sky81452-regulator.o
obj-$(CONFIG_REGULATOR_STM32_VREFBUF) += stm32-vrefbuf.o
+obj-$(CONFIG_REGULATOR_STPMIC1) += stpmic1_regulator.o
obj-$(CONFIG_REGULATOR_STW481X_VMMC) += stw481x-vmmc.o
obj-$(CONFIG_REGULATOR_SY8106A) += sy8106a-regulator.o
obj-$(CONFIG_REGULATOR_TI_ABB) += ti-abb-regulator.o
diff --git a/drivers/regulator/stpmic1_regulator.c
b/drivers/regulator/stpmic1_regulator.c
new file mode 100644
index 000..96f1808
--- /dev/null
+++ b/drivers/regulator/stpmic1_regulator.c
@@ -0,0 +1,674 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) STMicroelectronics 2018
+// Author: Pascal Paillet for STMicroelectronics.
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/**
+ * stpmic1 regulator description
+ * @desc: regulator framework description
+ * @mask_reset_reg: mask reset register address
+ * @mask_reset_mask: mask rank and mask reset register mask
+ * @icc_reg: icc register address
+ * @icc_mask: icc register mask
+ */
+struct stpmic1_regulator_cfg {
+ struct regulator_desc desc;
+ u8 mask_reset_reg;
+ u8 mask_reset_mask;
+ u8 icc_reg;
+ u8 icc_mask;
+};
+
+/**
+ * stpmic1 regulator data: this structure is used as driver data
+ * @regul_id: regulator id
+ * @reg_node: DT node of regulator (unused on non-DT platforms)
+ * @cfg: stpmic specific regulator description
+ * @mask_reset: mask_reset bit value
+ * @irq_curlim: current limit interrupt number
+ * @regmap: point to parent regmap structure
+ */
+struct stpmic1_regulator {
+ unsigned int regul_id;
+ struct device_node *reg_node;
+ struct stpmic1_regulator_cfg *cfg;
+ u8 mask_reset;
+ int irq_curlim;
+ struct regmap *regmap;
+};
+
+static int stpmic1_set_mode(struct regulator_dev *rdev, unsigned int mode);
+static unsigned int stpmic1_get_mode(struct regulator_dev *rdev);
+static int stpmic1_set_icc(struct regulator_dev *rdev);
+static int stpmic1_regulator_parse_dt(void *driver_data);
+static unsigned int stpmic1_map_mode(unsigned int mode);
+
+enum {
+ STPMIC1_BUCK1 = 0,
+ STPMIC1_BUCK2 = 1,
+ STPMIC1_BUCK3 = 2,
+ STPMIC1_BUCK4 = 3,
+ STPMIC1_LDO1 = 4,
+ STPMIC1_LDO2 = 5,
+ STPMIC1_LDO3 = 6,
+ STPMIC1_LDO4 = 7,
+ STPMIC1_LDO5 = 8,
+ STPMIC1_LDO6 = 9,
+ STPMIC1_VREF_DDR = 10,
+ STPMIC1_BOOST = 11,
+ STPMIC1_VBUS_OTG = 12,
+ STPMIC1_SW_OUT = 13,
+};
+
+/* Enable time worst case is 5000mV/(2250uV/uS) */
+#define PMIC_ENABLE_TIME_US 2200
+
+#define STPMIC1_BUCK_MODE_NORMAL 0
+#define STPMIC1_BUCK_MODE_LP BUCK_HPLP_ENABLE_MASK
+
+struct regulator_linear_range buck1_ranges[] = {
+ REGULATOR_LINEAR_RANGE(60, 0, 30, 25000),
+ REGULATOR_LINEAR_RANGE(135, 31, 63, 0),
+};
+
+struct regulator_linear_range buck2_ranges[] = {
+ REGULATOR_LINEAR_RANGE(100, 0, 17, 0),
+ REGULATOR_LINEAR_RANGE(105, 18, 19, 0),
+ REGULATOR_LINEAR_RANGE(110, 20, 21, 0),
+ REGULATOR_LINEAR_RANGE(115, 22, 23, 0),
+ REGULATOR_LINEAR_RANGE(120, 24, 25, 0),
+ REGULATOR_LINEAR_RANGE(125, 26, 27, 0),
+ REGULATOR_LINEAR_RANGE(130, 28, 29, 0),
+ REGULATOR_LINEAR_RANGE(135, 30, 31,
