[PATCH 1/2] dt-bindings: regulator: Add stm32mp1 pwr regulators

[PATCH 1/2] dt-bindings: regulator: Add stm32mp1 pwr regulators

[Pascal PAILLET-LME]

Document devicetree bindings for stm32mp1 pwr regulators.

Signed-off-by: Pascal Paillet <p.paillet@st.com>
---
 .../bindings/regulator/st,stm32mp1-pwr-reg.txt     | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/st,stm32mp1-pwr-reg.txt

diff --git a/Documentation/devicetree/bindings/regulator/st,stm32mp1-pwr-reg.txt b/Documentation/devicetree/bindings/regulator/st,stm32mp1-pwr-reg.txt
new file mode 100644
index 0000000..e372dd3
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/st,stm32mp1-pwr-reg.txt
@@ -0,0 +1,43 @@
+STM32MP1 PWR Regulators
+-----------------------
+
+Available Regulators in STM32MP1 PWR block are:
+  - reg11 for regulator 1V1
+  - reg18 for regulator 1V8
+  - usb33 for the swtich USB3V3
+
+Required properties:
+- compatible: Must be "st,stm32mp1,pwr-reg"
+- list of child nodes that specify the regulator reg11, reg18 or usb33
+  initialization data for defined regulators. The definition for each of
+  these nodes is defined using the standard binding for regulators found at
+  Documentation/devicetree/bindings/regulator/regulator.txt.
+- vdd-supply: phandle to the parent supply/regulator node for vdd input
+- vdd_3v3_usbfs-supply: phandle to the parent supply/regulator node for usb33
+
+Example:
+
+pwr_regulators: pwr@50001000 {
+	compatible = "st,stm32mp1,pwr-reg";
+	reg = <0x50001000 0x10>;
+	vdd-supply = <&vdd>;
+	vdd_3v3_usbfs-supply = <&vdd_usb>;
+
+	reg11: reg11 {
+		regulator-name = "reg11";
+		regulator-min-microvolt = <1100000>;
+		regulator-max-microvolt = <1100000>;
+	};
+
+	reg18: reg18 {
+		regulator-name = "reg18";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+	};
+
+	usb33: usb33 {
+		regulator-name = "usb33";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+	};
+};
-- 
1.9.1

[PATCH 0/2] Add support for STM32MP1 power regulators

[Pascal PAILLET-LME]

The POWER block of the STM32MP1 integrates 3 fixed regulators with a ready
status bit for each one. The goal of this patchset is to add support those
regulators.

Pascal Paillet (2):
  dt-bindings: regulator: Add stm32mp1 pwr regulators
  regulator: Add support for stm32 power regulators

 .../bindings/regulator/st,stm32mp1-pwr-reg.txt     |  43 +++++
 drivers/regulator/Kconfig                          |   7 +
 drivers/regulator/Makefile                         |   1 +
 drivers/regulator/stm32-pwr.c                      | 193 +++++++++++++++++++++
 4 files changed, 244 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/st,stm32mp1-pwr-reg.txt
 create mode 100644 drivers/regulator/stm32-pwr.c

-- 
1.9.1

[PATCH 2/2] regulator: Add support for stm32 power regulators

[Pascal PAILLET-LME]

Add support for 1V1 1V8 USB3V3 power regulators.

Signed-off-by: Pascal Paillet <p.paillet@st.com>
---
 drivers/regulator/Kconfig     |   7 ++
 drivers/regulator/Makefile    |   1 +
 drivers/regulator/stm32-pwr.c | 193 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 201 insertions(+)
 create mode 100644 drivers/regulator/stm32-pwr.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index c4dc2bc..26e6e26 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -840,6 +840,13 @@ config REGULATOR_STM32_VREFBUF
 	  This driver can also be built as a module. If so, the module
 	  will be called stm32-vrefbuf.
 
+config REGULATOR_STM32_PWR
+	bool "STMicroelectronics STM32 PWR"
+	depends on ARCH_STM32
+	help
+	  This driver supports internal regulators (1V1, 1V8, 3V3) in the
+	  STMicroelectronics STM32 chips.
+
 config REGULATOR_STPMIC1
 	tristate "STMicroelectronics STPMIC1 PMIC Regulators"
 	depends on MFD_STPMIC1
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 1169f8a..93f5384 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -105,6 +105,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_STM32_PWR) += stm32-pwr.o
 obj-$(CONFIG_REGULATOR_STPMIC1) += stpmic1_regulator.o
 obj-$(CONFIG_REGULATOR_STW481X_VMMC) += stw481x-vmmc.o
 obj-$(CONFIG_REGULATOR_SY8106A) += sy8106a-regulator.o
diff --git a/drivers/regulator/stm32-pwr.c b/drivers/regulator/stm32-pwr.c
new file mode 100644
index 0000000..166dc44
--- /dev/null
+++ b/drivers/regulator/stm32-pwr.c
@@ -0,0 +1,193 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) STMicroelectronics 2019
+// Authors: Gabriel Fernandez <gabriel.fernandez@st.com>
+//          Pascal Paillet <p.paillet@st.com>.
+
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+
+/*
+ * Registers description
+ */
+#define REG_PWR_CR3 0x0C
+
+#define USB_3_3_EN BIT(24)
+#define USB_3_3_RDY BIT(26)
+#define REG_1_8_EN BIT(28)
+#define REG_1_8_RDY BIT(29)
+#define REG_1_1_EN BIT(30)
+#define REG_1_1_RDY BIT(31)
+
+/* list of supported regulators */
+enum {
+	PWR_REG11,
+	PWR_REG18,
+	PWR_USB33,
+	STM32PWR_REG_NUM_REGS
+};
+
+u32 ready_mask_table[STM32PWR_REG_NUM_REGS] = {
+	[PWR_REG11] = REG_1_1_RDY,
+	[PWR_REG18] = REG_1_8_RDY,
+	[PWR_USB33] = USB_3_3_RDY,
+};
+
+struct stm32_pwr_reg {
+	void __iomem *base;
+	const struct regulator_desc *desc;
+	u32 ready_mask;
+};
+
+int stm32_pwr_reg_is_enabled(struct regulator_dev *rdev)
+{
+	struct stm32_pwr_reg *priv = rdev_get_drvdata(rdev);
+	u32 val;
+
+	val = readl_relaxed(priv->base + REG_PWR_CR3);
+
+	return (val & priv->ready_mask);
+}
+
+static int stm32_pwr_reg_enable(struct regulator_dev *rdev)
+{
+	struct stm32_pwr_reg *priv = rdev_get_drvdata(rdev);
+	int ret;
+	u32 val;
+
+	val = readl_relaxed(priv->base + REG_PWR_CR3);
+	val |= priv->desc->enable_mask;
+	writel_relaxed(val, priv->base + REG_PWR_CR3);
+
+	/* use an arbitrary timeout of 20ms */
+	ret = readx_poll_timeout(stm32_pwr_reg_is_enabled, rdev, val, val,
+				 100, 20 * 1000);
+	if (ret)
+		dev_err(&rdev->dev, "regulator enable timed out!\n");
+
+	return ret;
+}
+
+static int stm32_pwr_reg_disable(struct regulator_dev *rdev)
+{
+	struct stm32_pwr_reg *priv = rdev_get_drvdata(rdev);
+	int ret;
+	u32 val;
+
+	val = readl_relaxed(priv->base + REG_PWR_CR3);
+	val &= ~priv->desc->enable_mask;
+	writel_relaxed(val, priv->base + REG_PWR_CR3);
+
+	/* use an arbitrary timeout of 20ms */
+	ret = readx_poll_timeout(stm32_pwr_reg_is_enabled, rdev, val, !val,
+				 100, 20 * 1000);
+	if (ret)
+		dev_err(&rdev->dev, "regulator disable timed out!\n");
+
+	return ret;
+}
+
+static const struct regulator_ops stm32_pwr_reg_ops = {
+	.list_voltage	= regulator_list_voltage_linear,
+	.enable		= stm32_pwr_reg_enable,
+	.disable	= stm32_pwr_reg_disable,
+	.is_enabled	= stm32_pwr_reg_is_enabled,
+};
+
+#define PWR_REG(_id, _name, _volt, _en, _supply) \
+	[_id] = { \
+		.id = _id, \
+		.name = _name, \
+		.n_voltages = 1, \
+		.type = REGULATOR_VOLTAGE, \
+		.min_uV = _volt, \
+		.fixed_uV = _volt, \
+		.ops = &stm32_pwr_reg_ops, \
+		.enable_mask = _en, \
+		.owner = THIS_MODULE, \
+		.supply_name = _supply, \
+	} \
+
+static const struct regulator_desc stm32_pwr_desc[] = {
+	PWR_REG(PWR_REG11, "reg11", 1100000, REG_1_1_EN, "vdd"),
+	PWR_REG(PWR_REG18, "reg18", 1800000, REG_1_8_EN, "vdd"),
+	PWR_REG(PWR_USB33, "usb33", 3300000, USB_3_3_EN, "vdd_3v3_usbfs"),
+};
+
+static struct of_regulator_match stm32_pwr_reg_matches[] = {
+	{ .name = "reg11", },
+	{ .name = "reg18", },
+	{ .name = "usb33", },
+};
+
+static int stm32_pwr_regulator_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct stm32_pwr_reg *priv;
+	void __iomem *base;
+	struct regulator_dev *rdev;
+	struct regulator_config config = { };
+	int i, ret = 0;
+
+	base = of_iomap(np, 0);
+	if (IS_ERR(base)) {
+		dev_err(&pdev->dev, "Unable to map IO memory\n");
+		return PTR_ERR(base);
+	}
+
+	of_regulator_match(&pdev->dev, np, stm32_pwr_reg_matches,
+			   STM32PWR_REG_NUM_REGS);
+
+	config.dev = &pdev->dev;
+
+	for (i = 0; i < STM32PWR_REG_NUM_REGS; i++) {
+		struct of_regulator_match *match = &stm32_pwr_reg_matches[i];
+
+		priv = devm_kzalloc(&pdev->dev, sizeof(struct stm32_pwr_reg),
+				    GFP_KERNEL);
+		if (!priv)
+			return -ENOMEM;
+		priv->base = base;
+		priv->desc = &stm32_pwr_desc[i];
+		priv->ready_mask = ready_mask_table[i];
+
+		config.init_data = match->init_data;
+		config.of_node = match->of_node;
+		config.driver_data = priv;
+
+		rdev = devm_regulator_register(&pdev->dev,
+					       &stm32_pwr_desc[i],
+					       &config);
+		if (IS_ERR(rdev)) {
+			ret = PTR_ERR(rdev);
+			dev_err(&pdev->dev,
+				"Failed to register regulator: %d\n", ret);
+			break;
+		}
+	}
+	return ret;
+}
+
+static const struct of_device_id stm32_pwr_of_match[] = {
+	{ .compatible = "st,stm32mp1,pwr-reg", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, stm32_pwr_of_match);
+
+static struct platform_driver stm32_pwr_driver = {
+	.probe = stm32_pwr_regulator_probe,
+	.driver = {
+		.name  = "stm32-pwr-regulator",
+		.of_match_table = of_match_ptr(stm32_pwr_of_match),
+	},
+};
+module_platform_driver(stm32_pwr_driver);
+
+MODULE_DESCRIPTION("STM32MP1 PWR voltage regulator driver");
+MODULE_AUTHOR("Gabriel Fernandez <gabriel.fernandez@st.com>");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

Re: [PATCH 2/2] regulator: Add support for stm32 power regulators

[Mark Brown]

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

On Mon, Apr 08, 2019 at 03:24:08PM +0000, Pascal PAILLET-LME wrote:

> +config REGULATOR_STM32_PWR
> +	bool "STMicroelectronics STM32 PWR"
> +	depends on ARCH_STM32

There's no build time dependency here, please also add an || COMPILE_TEST to 
help with build coverage.

> +int stm32_pwr_reg_is_enabled(struct regulator_dev *rdev)
> +{
> +	struct stm32_pwr_reg *priv = rdev_get_drvdata(rdev);
> +	u32 val;
> +
> +	val = readl_relaxed(priv->base + REG_PWR_CR3);
> +
> +	return (val & priv->ready_mask);
> +}

This looks like it's reading back a status bit from the hardware not the
state requested by software - I'd expect an _is_enabled() to reference
the same bit that's updated by the enable/disable functions.  This looks
like a better fit for _get_status().

> +	of_regulator_match(&pdev->dev, np, stm32_pwr_reg_matches,
> +			   STM32PWR_REG_NUM_REGS);

You don't need to open code the matching any more, just set of_match and
regulators_node in the regulator_desc and the core will do it for you.

> +MODULE_DESCRIPTION("STM32MP1 PWR voltage regulator driver");
> +MODULE_AUTHOR("Gabriel Fernandez <gabriel.fernandez@st.com>");
> +MODULE_LICENSE("GPL v2");

No signoff from Gabriel?

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