[PATCH v4 0/4] regulator: Add support for Allwinner D1 system LDOs

[PATCH v4 0/4] regulator: Add support for Allwinner D1 system LDOs

[Samuel Holland]

This series adds the binding and driver for one of the two pairs of LDOs
inside the Allwinner D1 SoC. I am splitting up the series to unblock
merging the SoC devicetree; the analog LDOs are not required to boot.

A binding and driver change is required for the SRAM controller, to
accept the regulators device as its child node.

The example for the regulator device binding is in SRAM controller
binding document, per Rob's request to keep MFD examples in one place.

Changes in v4:
 - Fix the order of the maintainer/description sections
 - Replace unevaluatedProperties with "additionalProperties: false"
 - Drop the analog LDOs until the codec binding is ready
 - Drop the analog LDOs until the codec binding is ready
 - Remove unevaluatedProperties from regulators schema reference
 - Check the compatible string instead of the node name

Changes in v3:
 - Add "reg" property to bindings
 - Add "unevaluatedProperties: true" to regulator nodes
 - Minor changes to regulator node name patterns
 - Remove system-ldos example (now added in the parent binding)
 - Adjust control flow in sun20i_regulator_get_regmap() for clarity
 - Require the regulators node to have a unit address
 - Reference the regulator schema from the SRAM controller schema
 - Move the system LDOs example to the SRAM controller schema
 - Reorder the patches so the example passes validation

Changes in v2:
 - Remove syscon property from bindings
 - Update binding examples to fix warnings and provide context
 - Use decimal numbers for .n_voltages instead of field widths
 - Get the regmap from the parent device instead of a property/phandle

Samuel Holland (4):
  regulator: dt-bindings: Add Allwinner D1 system LDOs
  regulator: sun20i: Add Allwinner D1 LDOs driver
  dt-bindings: sram: sunxi-sram: Add regulators child
  soc: sunxi: sram: Only iterate over SRAM children

 .../allwinner,sun20i-d1-system-ldos.yaml      |  37 +++++
 .../allwinner,sun4i-a10-system-control.yaml   |  28 ++++
 drivers/regulator/Kconfig                     |   8 +
 drivers/regulator/Makefile                    |   1 +
 drivers/regulator/sun20i-regulator.c          | 150 ++++++++++++++++++
 drivers/soc/sunxi/sunxi_sram.c                |   3 +
 6 files changed, 227 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.yaml
 create mode 100644 drivers/regulator/sun20i-regulator.c

-- 
2.37.4

[PATCH v4 0/4] regulator: Add support for Allwinner D1 system LDOs

[Samuel Holland]

This series adds the binding and driver for one of the two pairs of LDOs
inside the Allwinner D1 SoC. I am splitting up the series to unblock
merging the SoC devicetree; the analog LDOs are not required to boot.

A binding and driver change is required for the SRAM controller, to
accept the regulators device as its child node.

The example for the regulator device binding is in SRAM controller
binding document, per Rob's request to keep MFD examples in one place.

Changes in v4:
 - Fix the order of the maintainer/description sections
 - Replace unevaluatedProperties with "additionalProperties: false"
 - Drop the analog LDOs until the codec binding is ready
 - Drop the analog LDOs until the codec binding is ready
 - Remove unevaluatedProperties from regulators schema reference
 - Check the compatible string instead of the node name

Changes in v3:
 - Add "reg" property to bindings
 - Add "unevaluatedProperties: true" to regulator nodes
 - Minor changes to regulator node name patterns
 - Remove system-ldos example (now added in the parent binding)
 - Adjust control flow in sun20i_regulator_get_regmap() for clarity
 - Require the regulators node to have a unit address
 - Reference the regulator schema from the SRAM controller schema
 - Move the system LDOs example to the SRAM controller schema
 - Reorder the patches so the example passes validation

Changes in v2:
 - Remove syscon property from bindings
 - Update binding examples to fix warnings and provide context
 - Use decimal numbers for .n_voltages instead of field widths
 - Get the regmap from the parent device instead of a property/phandle

Samuel Holland (4):
  regulator: dt-bindings: Add Allwinner D1 system LDOs
  regulator: sun20i: Add Allwinner D1 LDOs driver
  dt-bindings: sram: sunxi-sram: Add regulators child
  soc: sunxi: sram: Only iterate over SRAM children

 .../allwinner,sun20i-d1-system-ldos.yaml      |  37 +++++
 .../allwinner,sun4i-a10-system-control.yaml   |  28 ++++
 drivers/regulator/Kconfig                     |   8 +
 drivers/regulator/Makefile                    |   1 +
 drivers/regulator/sun20i-regulator.c          | 150 ++++++++++++++++++
 drivers/soc/sunxi/sunxi_sram.c                |   3 +
 6 files changed, 227 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.yaml
 create mode 100644 drivers/regulator/sun20i-regulator.c

-- 
2.37.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 1/4] regulator: dt-bindings: Add Allwinner D1 system LDOs

[Samuel Holland]

The Allwinner D1 SoC contains two pairs of in-package LDOs. The pair of
"system" LDOs is for general purpose use. LDOA generally powers the
board's 1.8 V rail. LDOB powers the in-package DRAM, where applicable.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

Changes in v4:
 - Fix the order of the maintainer/description sections
 - Replace unevaluatedProperties with "additionalProperties: false"
 - Drop the analog LDOs until the codec binding is ready

Changes in v3:
 - Add "reg" property to bindings
 - Add "unevaluatedProperties: true" to regulator nodes
 - Minor changes to regulator node name patterns
 - Remove system-ldos example (now added in the parent binding)

Changes in v2:
 - Remove syscon property from bindings
 - Update binding examples to fix warnings and provide context

 .../allwinner,sun20i-d1-system-ldos.yaml      | 37 +++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.yaml

diff --git a/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.yaml b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.yaml
new file mode 100644
index 000000000000..ec6695c8d2e3
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.yaml
@@ -0,0 +1,37 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/allwinner,sun20i-d1-system-ldos.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner D1 System LDOs
+
+maintainers:
+  - Samuel Holland <samuel@sholland.org>
+
+description:
+  Allwinner D1 contains a pair of general-purpose LDOs which are designed to
+  supply power inside and outside the SoC. They are controlled by a register
+  within the system control MMIO space.
+
+properties:
+  compatible:
+    enum:
+      - allwinner,sun20i-d1-system-ldos
+
+  reg:
+    maxItems: 1
+
+patternProperties:
+  "^ldo[ab]$":
+    type: object
+    $ref: regulator.yaml#
+    unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+...
-- 
2.37.4

[PATCH v4 1/4] regulator: dt-bindings: Add Allwinner D1 system LDOs

[Samuel Holland]

The Allwinner D1 SoC contains two pairs of in-package LDOs. The pair of
"system" LDOs is for general purpose use. LDOA generally powers the
board's 1.8 V rail. LDOB powers the in-package DRAM, where applicable.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

Changes in v4:
 - Fix the order of the maintainer/description sections
 - Replace unevaluatedProperties with "additionalProperties: false"
 - Drop the analog LDOs until the codec binding is ready

Changes in v3:
 - Add "reg" property to bindings
 - Add "unevaluatedProperties: true" to regulator nodes
 - Minor changes to regulator node name patterns
 - Remove system-ldos example (now added in the parent binding)

Changes in v2:
 - Remove syscon property from bindings
 - Update binding examples to fix warnings and provide context

 .../allwinner,sun20i-d1-system-ldos.yaml      | 37 +++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.yaml

diff --git a/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.yaml b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.yaml
new file mode 100644
index 000000000000..ec6695c8d2e3
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.yaml
@@ -0,0 +1,37 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/allwinner,sun20i-d1-system-ldos.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner D1 System LDOs
+
+maintainers:
+  - Samuel Holland <samuel@sholland.org>
+
+description:
+  Allwinner D1 contains a pair of general-purpose LDOs which are designed to
+  supply power inside and outside the SoC. They are controlled by a register
+  within the system control MMIO space.
+
+properties:
+  compatible:
+    enum:
+      - allwinner,sun20i-d1-system-ldos
+
+  reg:
+    maxItems: 1
+
+patternProperties:
+  "^ldo[ab]$":
+    type: object
+    $ref: regulator.yaml#
+    unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+...
-- 
2.37.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 2/4] regulator: sun20i: Add Allwinner D1 LDOs driver

[Samuel Holland]

D1 contains two pairs of LDOs, "analog" LDOs and "system" LDOs. They are
similar and can share a driver, but only the system LDOs have a DT
binding defined so far.

The system LDOs have a single linear range. The voltage step is not an
integer, so a custom .list_voltage is needed to get the rounding right.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

Changes in v4:
 - Drop the analog LDOs until the codec binding is ready

Changes in v3:
 - Adjust control flow in sun20i_regulator_get_regmap() for clarity

Changes in v2:
 - Use decimal numbers for .n_voltages instead of field widths
 - Get the regmap from the parent device instead of a property/phandle

 drivers/regulator/Kconfig            |   8 ++
 drivers/regulator/Makefile           |   1 +
 drivers/regulator/sun20i-regulator.c | 150 +++++++++++++++++++++++++++
 3 files changed, 159 insertions(+)
 create mode 100644 drivers/regulator/sun20i-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 070e4403c6c2..8480532114c1 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1280,6 +1280,14 @@ config REGULATOR_STW481X_VMMC
 	  This driver supports the internal VMMC regulator in the STw481x
 	  PMIC chips.
 
+config REGULATOR_SUN20I
+	tristate "Allwinner D1 internal LDOs"
+	depends on ARCH_SUNXI || COMPILE_TEST
+	select MFD_SYSCON
+	default ARCH_SUNXI
+	help
+	  This driver supports the internal LDOs in the Allwinner D1 SoC.
+
 config REGULATOR_SY7636A
 	tristate "Silergy SY7636A voltage regulator"
 	depends on MFD_SY7636A
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 5962307e1130..8e9b5a21123d 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -150,6 +150,7 @@ 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_SUN20I) += sun20i-regulator.o
 obj-$(CONFIG_REGULATOR_SY7636A) += sy7636a-regulator.o
 obj-$(CONFIG_REGULATOR_SY8106A) += sy8106a-regulator.o
 obj-$(CONFIG_REGULATOR_SY8824X) += sy8824x.o
diff --git a/drivers/regulator/sun20i-regulator.c b/drivers/regulator/sun20i-regulator.c
new file mode 100644
index 000000000000..031bcc3dee50
--- /dev/null
+++ b/drivers/regulator/sun20i-regulator.c
@@ -0,0 +1,150 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// Copyright (c) 2021-2022 Samuel Holland <samuel@sholland.org>
+//
+
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+
+#define SUN20I_SYS_LDO_CTRL_REG		0x150
+
+struct sun20i_regulator_data {
+	const struct regulator_desc	*descs;
+	unsigned int			ndescs;
+};
+
+/* regulator_list_voltage_linear() modified for the non-integral uV_step. */
+static int sun20i_d1_system_ldo_list_voltage(struct regulator_dev *rdev,
+					     unsigned int selector)
+{
+	const struct regulator_desc *desc = rdev->desc;
+	unsigned int uV;
+
+	if (selector >= desc->n_voltages)
+		return -EINVAL;
+
+	uV = desc->min_uV + (desc->uV_step * selector);
+
+	/* Produce correctly-rounded absolute voltages. */
+	return uV + ((selector + 1 + (desc->min_uV % 4)) / 3);
+}
+
+static const struct regulator_ops sun20i_d1_system_ldo_ops = {
+	.list_voltage		= sun20i_d1_system_ldo_list_voltage,
+	.map_voltage		= regulator_map_voltage_ascend,
+	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
+	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
+};
+
+static const struct regulator_desc sun20i_d1_system_ldo_descs[] = {
+	{
+		.name		= "ldoa",
+		.supply_name	= "ldo-in",
+		.of_match	= "ldoa",
+		.ops		= &sun20i_d1_system_ldo_ops,
+		.type		= REGULATOR_VOLTAGE,
+		.owner		= THIS_MODULE,
+		.n_voltages	= 32,
+		.min_uV		= 1600000,
+		.uV_step	= 13333, /* repeating */
+		.vsel_reg	= SUN20I_SYS_LDO_CTRL_REG,
+		.vsel_mask	= GENMASK(7, 0),
+	},
+	{
+		.name		= "ldob",
+		.supply_name	= "ldo-in",
+		.of_match	= "ldob",
+		.ops		= &sun20i_d1_system_ldo_ops,
+		.type		= REGULATOR_VOLTAGE,
+		.owner		= THIS_MODULE,
+		.n_voltages	= 64,
+		.min_uV		= 1166666,
+		.uV_step	= 13333, /* repeating */
+		.vsel_reg	= SUN20I_SYS_LDO_CTRL_REG,
+		.vsel_mask	= GENMASK(15, 8),
+	},
+};
+
+static const struct sun20i_regulator_data sun20i_d1_system_ldos = {
+	.descs	= sun20i_d1_system_ldo_descs,
+	.ndescs	= ARRAY_SIZE(sun20i_d1_system_ldo_descs),
+};
+
+static struct regmap *sun20i_regulator_get_regmap(struct device *dev)
+{
+	struct regmap *regmap;
+
+	/*
+	 * First try the syscon interface. The system control device is not
+	 * compatible with "syscon", so fall back to getting the regmap from
+	 * its platform device. This is ugly, but required for devicetree
+	 * backward compatibility.
+	 */
+	regmap = syscon_node_to_regmap(dev->parent->of_node);
+	if (!IS_ERR(regmap))
+		return regmap;
+
+	regmap = dev_get_regmap(dev->parent, NULL);
+	if (regmap)
+		return regmap;
+
+	return ERR_PTR(-EPROBE_DEFER);
+}
+
+static int sun20i_regulator_probe(struct platform_device *pdev)
+{
+	const struct sun20i_regulator_data *data;
+	struct device *dev = &pdev->dev;
+	struct regulator_config config;
+	struct regmap *regmap;
+
+	data = of_device_get_match_data(dev);
+	if (!data)
+		return -EINVAL;
+
+	regmap = sun20i_regulator_get_regmap(dev);
+	if (IS_ERR(regmap))
+		return dev_err_probe(dev, PTR_ERR(regmap), "Failed to get regmap\n");
+
+	config = (struct regulator_config) {
+		.dev	= dev,
+		.regmap	= regmap,
+	};
+
+	for (unsigned int i = 0; i < data->ndescs; ++i) {
+		const struct regulator_desc *desc = &data->descs[i];
+		struct regulator_dev *rdev;
+
+		rdev = devm_regulator_register(dev, desc, &config);
+		if (IS_ERR(rdev))
+			return PTR_ERR(rdev);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id sun20i_regulator_of_match[] = {
+	{
+		.compatible = "allwinner,sun20i-d1-system-ldos",
+		.data = &sun20i_d1_system_ldos,
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, sun20i_regulator_of_match);
+
+static struct platform_driver sun20i_regulator_driver = {
+	.probe	= sun20i_regulator_probe,
+	.driver	= {
+		.name		= "sun20i-regulator",
+		.of_match_table	= sun20i_regulator_of_match,
+	},
+};
+module_platform_driver(sun20i_regulator_driver);
+
+MODULE_AUTHOR("Samuel Holland <samuel@sholland.org>");
+MODULE_DESCRIPTION("Allwinner D1 internal LDO driver");
+MODULE_LICENSE("GPL");
-- 
2.37.4

[PATCH v4 2/4] regulator: sun20i: Add Allwinner D1 LDOs driver

[Samuel Holland]

D1 contains two pairs of LDOs, "analog" LDOs and "system" LDOs. They are
similar and can share a driver, but only the system LDOs have a DT
binding defined so far.

The system LDOs have a single linear range. The voltage step is not an
integer, so a custom .list_voltage is needed to get the rounding right.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

Changes in v4:
 - Drop the analog LDOs until the codec binding is ready

Changes in v3:
 - Adjust control flow in sun20i_regulator_get_regmap() for clarity

Changes in v2:
 - Use decimal numbers for .n_voltages instead of field widths
 - Get the regmap from the parent device instead of a property/phandle

 drivers/regulator/Kconfig            |   8 ++
 drivers/regulator/Makefile           |   1 +
 drivers/regulator/sun20i-regulator.c | 150 +++++++++++++++++++++++++++
 3 files changed, 159 insertions(+)
 create mode 100644 drivers/regulator/sun20i-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 070e4403c6c2..8480532114c1 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1280,6 +1280,14 @@ config REGULATOR_STW481X_VMMC
 	  This driver supports the internal VMMC regulator in the STw481x
 	  PMIC chips.
 
+config REGULATOR_SUN20I
+	tristate "Allwinner D1 internal LDOs"
+	depends on ARCH_SUNXI || COMPILE_TEST
+	select MFD_SYSCON
+	default ARCH_SUNXI
+	help
+	  This driver supports the internal LDOs in the Allwinner D1 SoC.
+
 config REGULATOR_SY7636A
 	tristate "Silergy SY7636A voltage regulator"
 	depends on MFD_SY7636A
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 5962307e1130..8e9b5a21123d 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -150,6 +150,7 @@ 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_SUN20I) += sun20i-regulator.o
 obj-$(CONFIG_REGULATOR_SY7636A) += sy7636a-regulator.o
 obj-$(CONFIG_REGULATOR_SY8106A) += sy8106a-regulator.o
 obj-$(CONFIG_REGULATOR_SY8824X) += sy8824x.o
diff --git a/drivers/regulator/sun20i-regulator.c b/drivers/regulator/sun20i-regulator.c
new file mode 100644
index 000000000000..031bcc3dee50
--- /dev/null
+++ b/drivers/regulator/sun20i-regulator.c
@@ -0,0 +1,150 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// Copyright (c) 2021-2022 Samuel Holland <samuel@sholland.org>
+//
+
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+
+#define SUN20I_SYS_LDO_CTRL_REG		0x150
+
+struct sun20i_regulator_data {
+	const struct regulator_desc	*descs;
+	unsigned int			ndescs;
+};
+
+/* regulator_list_voltage_linear() modified for the non-integral uV_step. */
+static int sun20i_d1_system_ldo_list_voltage(struct regulator_dev *rdev,
+					     unsigned int selector)
+{
+	const struct regulator_desc *desc = rdev->desc;
+	unsigned int uV;
+
+	if (selector >= desc->n_voltages)
+		return -EINVAL;
+
+	uV = desc->min_uV + (desc->uV_step * selector);
+
+	/* Produce correctly-rounded absolute voltages. */
+	return uV + ((selector + 1 + (desc->min_uV % 4)) / 3);
+}
+
+static const struct regulator_ops sun20i_d1_system_ldo_ops = {
+	.list_voltage		= sun20i_d1_system_ldo_list_voltage,
+	.map_voltage		= regulator_map_voltage_ascend,
+	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
+	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
+};
+
+static const struct regulator_desc sun20i_d1_system_ldo_descs[] = {
+	{
+		.name		= "ldoa",
+		.supply_name	= "ldo-in",
+		.of_match	= "ldoa",
+		.ops		= &sun20i_d1_system_ldo_ops,
+		.type		= REGULATOR_VOLTAGE,
+		.owner		= THIS_MODULE,
+		.n_voltages	= 32,
+		.min_uV		= 1600000,
+		.uV_step	= 13333, /* repeating */
+		.vsel_reg	= SUN20I_SYS_LDO_CTRL_REG,
+		.vsel_mask	= GENMASK(7, 0),
+	},
+	{
+		.name		= "ldob",
+		.supply_name	= "ldo-in",
+		.of_match	= "ldob",
+		.ops		= &sun20i_d1_system_ldo_ops,
+		.type		= REGULATOR_VOLTAGE,
+		.owner		= THIS_MODULE,
+		.n_voltages	= 64,
+		.min_uV		= 1166666,
+		.uV_step	= 13333, /* repeating */
+		.vsel_reg	= SUN20I_SYS_LDO_CTRL_REG,
+		.vsel_mask	= GENMASK(15, 8),
+	},
+};
+
+static const struct sun20i_regulator_data sun20i_d1_system_ldos = {
+	.descs	= sun20i_d1_system_ldo_descs,
+	.ndescs	= ARRAY_SIZE(sun20i_d1_system_ldo_descs),
+};
+
+static struct regmap *sun20i_regulator_get_regmap(struct device *dev)
+{
+	struct regmap *regmap;
+
+	/*
+	 * First try the syscon interface. The system control device is not
+	 * compatible with "syscon", so fall back to getting the regmap from
+	 * its platform device. This is ugly, but required for devicetree
+	 * backward compatibility.
+	 */
+	regmap = syscon_node_to_regmap(dev->parent->of_node);
+	if (!IS_ERR(regmap))
+		return regmap;
+
+	regmap = dev_get_regmap(dev->parent, NULL);
+	if (regmap)
+		return regmap;
+
+	return ERR_PTR(-EPROBE_DEFER);
+}
+
+static int sun20i_regulator_probe(struct platform_device *pdev)
+{
+	const struct sun20i_regulator_data *data;
+	struct device *dev = &pdev->dev;
+	struct regulator_config config;
+	struct regmap *regmap;
+
+	data = of_device_get_match_data(dev);
+	if (!data)
+		return -EINVAL;
+
+	regmap = sun20i_regulator_get_regmap(dev);
+	if (IS_ERR(regmap))
+		return dev_err_probe(dev, PTR_ERR(regmap), "Failed to get regmap\n");
+
+	config = (struct regulator_config) {
+		.dev	= dev,
+		.regmap	= regmap,
+	};
+
+	for (unsigned int i = 0; i < data->ndescs; ++i) {
+		const struct regulator_desc *desc = &data->descs[i];
+		struct regulator_dev *rdev;
+
+		rdev = devm_regulator_register(dev, desc, &config);
+		if (IS_ERR(rdev))
+			return PTR_ERR(rdev);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id sun20i_regulator_of_match[] = {
+	{
+		.compatible = "allwinner,sun20i-d1-system-ldos",
+		.data = &sun20i_d1_system_ldos,
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, sun20i_regulator_of_match);
+
+static struct platform_driver sun20i_regulator_driver = {
+	.probe	= sun20i_regulator_probe,
+	.driver	= {
+		.name		= "sun20i-regulator",
+		.of_match_table	= sun20i_regulator_of_match,
+	},
+};
+module_platform_driver(sun20i_regulator_driver);
+
+MODULE_AUTHOR("Samuel Holland <samuel@sholland.org>");
+MODULE_DESCRIPTION("Allwinner D1 internal LDO driver");
+MODULE_LICENSE("GPL");
-- 
2.37.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 3/4] dt-bindings: sram: sunxi-sram: Add regulators child

[Samuel Holland]

Some sunxi SoCs have in-package regulators controlled by a register in
the system control MMIO block. Allow a child node for this regulator
device in addition to SRAM child nodes.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

Changes in v4:
 - Remove unevaluatedProperties from regulators schema reference

Changes in v3:
 - Require the regulators node to have a unit address
 - Reference the regulator schema from the SRAM controller schema
 - Move the system LDOs example to the SRAM controller schema
 - Reorder the patches so the example passes validation

Changes in v2:
 - New patch for v2

 .../allwinner,sun4i-a10-system-control.yaml   | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml b/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml
index 98a7dc7f467d..c487ac9521e9 100644
--- a/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml
+++ b/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml
@@ -56,6 +56,9 @@ properties:
   ranges: true
 
 patternProperties:
+  "^regulators@[0-9a-f]+$":
+    $ref: /schemas/regulator/allwinner,sun20i-d1-system-ldos.yaml#
+
   "^sram@[a-z0-9]+":
     type: object
 
@@ -130,3 +133,28 @@ examples:
         };
       };
     };
+
+  - |
+    syscon@3000000 {
+      compatible = "allwinner,sun20i-d1-system-control";
+      reg = <0x3000000 0x1000>;
+      ranges;
+      #address-cells = <1>;
+      #size-cells = <1>;
+
+      regulators@3000150 {
+        compatible = "allwinner,sun20i-d1-system-ldos";
+        reg = <0x3000150 0x4>;
+
+        reg_ldoa: ldoa {
+          regulator-min-microvolt = <1800000>;
+          regulator-max-microvolt = <1800000>;
+        };
+
+        reg_ldob: ldob {
+          regulator-name = "vcc-dram";
+          regulator-min-microvolt = <1500000>;
+          regulator-max-microvolt = <1500000>;
+        };
+      };
+    };
-- 
2.37.4

[PATCH v4 3/4] dt-bindings: sram: sunxi-sram: Add regulators child

[Samuel Holland]

Some sunxi SoCs have in-package regulators controlled by a register in
the system control MMIO block. Allow a child node for this regulator
device in addition to SRAM child nodes.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

Changes in v4:
 - Remove unevaluatedProperties from regulators schema reference

Changes in v3:
 - Require the regulators node to have a unit address
 - Reference the regulator schema from the SRAM controller schema
 - Move the system LDOs example to the SRAM controller schema
 - Reorder the patches so the example passes validation

Changes in v2:
 - New patch for v2

 .../allwinner,sun4i-a10-system-control.yaml   | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml b/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml
index 98a7dc7f467d..c487ac9521e9 100644
--- a/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml
+++ b/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml
@@ -56,6 +56,9 @@ properties:
   ranges: true
 
 patternProperties:
+  "^regulators@[0-9a-f]+$":
+    $ref: /schemas/regulator/allwinner,sun20i-d1-system-ldos.yaml#
+
   "^sram@[a-z0-9]+":
     type: object
 
@@ -130,3 +133,28 @@ examples:
         };
       };
     };
+
+  - |
+    syscon@3000000 {
+      compatible = "allwinner,sun20i-d1-system-control";
+      reg = <0x3000000 0x1000>;
+      ranges;
+      #address-cells = <1>;
+      #size-cells = <1>;
+
+      regulators@3000150 {
+        compatible = "allwinner,sun20i-d1-system-ldos";
+        reg = <0x3000150 0x4>;
+
+        reg_ldoa: ldoa {
+          regulator-min-microvolt = <1800000>;
+          regulator-max-microvolt = <1800000>;
+        };
+
+        reg_ldob: ldob {
+          regulator-name = "vcc-dram";
+          regulator-min-microvolt = <1500000>;
+          regulator-max-microvolt = <1500000>;
+        };
+      };
+    };
-- 
2.37.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 4/4] soc: sunxi: sram: Only iterate over SRAM children

[Samuel Holland]

Now that a regulators child is accepted by the controller binding, the
debugfs show routine must be explicitly limited to mmio-sram children.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

Changes in v4:
 - Check the compatible string instead of the node name

Changes in v2:
 - New patch for v2

 drivers/soc/sunxi/sunxi_sram.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/soc/sunxi/sunxi_sram.c b/drivers/soc/sunxi/sunxi_sram.c
index 92f9186c1c42..f09918c59042 100644
--- a/drivers/soc/sunxi/sunxi_sram.c
+++ b/drivers/soc/sunxi/sunxi_sram.c
@@ -120,6 +120,9 @@ static int sunxi_sram_show(struct seq_file *s, void *data)
 	seq_puts(s, "--------------------\n\n");
 
 	for_each_child_of_node(sram_dev->of_node, sram_node) {
+		if (!of_device_is_compatible(sram_node, "mmio-sram"))
+			continue;
+
 		sram_addr_p = of_get_address(sram_node, 0, NULL, NULL);
 
 		seq_printf(s, "sram@%08x\n",
-- 
2.37.4

[PATCH v4 4/4] soc: sunxi: sram: Only iterate over SRAM children

[Samuel Holland]

Now that a regulators child is accepted by the controller binding, the
debugfs show routine must be explicitly limited to mmio-sram children.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

Changes in v4:
 - Check the compatible string instead of the node name

Changes in v2:
 - New patch for v2

 drivers/soc/sunxi/sunxi_sram.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/soc/sunxi/sunxi_sram.c b/drivers/soc/sunxi/sunxi_sram.c
index 92f9186c1c42..f09918c59042 100644
--- a/drivers/soc/sunxi/sunxi_sram.c
+++ b/drivers/soc/sunxi/sunxi_sram.c
@@ -120,6 +120,9 @@ static int sunxi_sram_show(struct seq_file *s, void *data)
 	seq_puts(s, "--------------------\n\n");
 
 	for_each_child_of_node(sram_dev->of_node, sram_node) {
+		if (!of_device_is_compatible(sram_node, "mmio-sram"))
+			continue;
+
 		sram_addr_p = of_get_address(sram_node, 0, NULL, NULL);
 
 		seq_printf(s, "sram@%08x\n",
-- 
2.37.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 1/4] regulator: dt-bindings: Add Allwinner D1 system LDOs

[Krzysztof Kozlowski]

On 25/11/2022 05:01, Samuel Holland wrote:
> The Allwinner D1 SoC contains two pairs of in-package LDOs. The pair of
> "system" LDOs is for general purpose use. LDOA generally powers the
> board's 1.8 V rail. LDOB powers the in-package DRAM, where applicable.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v4 1/4] regulator: dt-bindings: Add Allwinner D1 system LDOs

[Krzysztof Kozlowski]

On 25/11/2022 05:01, Samuel Holland wrote:
> The Allwinner D1 SoC contains two pairs of in-package LDOs. The pair of
> "system" LDOs is for general purpose use. LDOA generally powers the
> board's 1.8 V rail. LDOB powers the in-package DRAM, where applicable.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 3/4] dt-bindings: sram: sunxi-sram: Add regulators child

[Krzysztof Kozlowski]

On 25/11/2022 05:01, Samuel Holland wrote:
> Some sunxi SoCs have in-package regulators controlled by a register in
> the system control MMIO block. Allow a child node for this regulator
> device in addition to SRAM child nodes.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> 
> Changes in v4:
>  - Remove unevaluatedProperties from regulators schema reference
> 
> Changes in v3:
>  - Require the regulators node to have a unit address
>  - Reference the regulator schema from the SRAM controller schema
>  - Move the system LDOs example to the SRAM controller schema
>  - Reorder the patches so the example passes validation
> 
> Changes in v2:
>  - New patch for v2
> 
>  .../allwinner,sun4i-a10-system-control.yaml   | 28 +++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml b/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml
> index 98a7dc7f467d..c487ac9521e9 100644
> --- a/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml
> +++ b/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml
> @@ -56,6 +56,9 @@ properties:
>    ranges: true
>  
>  patternProperties:
> +  "^regulators@[0-9a-f]+$":
> +    $ref: /schemas/regulator/allwinner,sun20i-d1-system-ldos.yaml#
> +
>    "^sram@[a-z0-9]+":
>      type: object

You could add allOf:if:not:...then: clause making regulators as false
for all variants which do not support it. This would make it more specific.

Anyway:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH v4 3/4] dt-bindings: sram: sunxi-sram: Add regulators child

[Krzysztof Kozlowski]

On 25/11/2022 05:01, Samuel Holland wrote:
> Some sunxi SoCs have in-package regulators controlled by a register in
> the system control MMIO block. Allow a child node for this regulator
> device in addition to SRAM child nodes.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> 
> Changes in v4:
>  - Remove unevaluatedProperties from regulators schema reference
> 
> Changes in v3:
>  - Require the regulators node to have a unit address
>  - Reference the regulator schema from the SRAM controller schema
>  - Move the system LDOs example to the SRAM controller schema
>  - Reorder the patches so the example passes validation
> 
> Changes in v2:
>  - New patch for v2
> 
>  .../allwinner,sun4i-a10-system-control.yaml   | 28 +++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml b/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml
> index 98a7dc7f467d..c487ac9521e9 100644
> --- a/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml
> +++ b/Documentation/devicetree/bindings/sram/allwinner,sun4i-a10-system-control.yaml
> @@ -56,6 +56,9 @@ properties:
>    ranges: true
>  
>  patternProperties:
> +  "^regulators@[0-9a-f]+$":
> +    $ref: /schemas/regulator/allwinner,sun20i-d1-system-ldos.yaml#
> +
>    "^sram@[a-z0-9]+":
>      type: object

You could add allOf:if:not:...then: clause making regulators as false
for all variants which do not support it. This would make it more specific.

Anyway:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 2/4] regulator: sun20i: Add Allwinner D1 LDOs driver

[Andre Przywara]

On Thu, 24 Nov 2022 22:01:10 -0600
Samuel Holland <samuel@sholland.org> wrote:

Hi Samuel,

> D1 contains two pairs of LDOs, "analog" LDOs and "system" LDOs. They are
> similar and can share a driver, but only the system LDOs have a DT
> binding defined so far.
> 
> The system LDOs have a single linear range. The voltage step is not an
> integer, so a custom .list_voltage is needed to get the rounding right.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> 
> Changes in v4:
>  - Drop the analog LDOs until the codec binding is ready
> 
> Changes in v3:
>  - Adjust control flow in sun20i_regulator_get_regmap() for clarity
> 
> Changes in v2:
>  - Use decimal numbers for .n_voltages instead of field widths
>  - Get the regmap from the parent device instead of a property/phandle
> 
>  drivers/regulator/Kconfig            |   8 ++
>  drivers/regulator/Makefile           |   1 +
>  drivers/regulator/sun20i-regulator.c | 150 +++++++++++++++++++++++++++
>  3 files changed, 159 insertions(+)
>  create mode 100644 drivers/regulator/sun20i-regulator.c
> 
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index 070e4403c6c2..8480532114c1 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -1280,6 +1280,14 @@ config REGULATOR_STW481X_VMMC
>  	  This driver supports the internal VMMC regulator in the STw481x
>  	  PMIC chips.
>  
> +config REGULATOR_SUN20I
> +	tristate "Allwinner D1 internal LDOs"
> +	depends on ARCH_SUNXI || COMPILE_TEST
> +	select MFD_SYSCON
> +	default ARCH_SUNXI
> +	help
> +	  This driver supports the internal LDOs in the Allwinner D1 SoC.
> +
>  config REGULATOR_SY7636A
>  	tristate "Silergy SY7636A voltage regulator"
>  	depends on MFD_SY7636A
> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index 5962307e1130..8e9b5a21123d 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -150,6 +150,7 @@ 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_SUN20I) += sun20i-regulator.o
>  obj-$(CONFIG_REGULATOR_SY7636A) += sy7636a-regulator.o
>  obj-$(CONFIG_REGULATOR_SY8106A) += sy8106a-regulator.o
>  obj-$(CONFIG_REGULATOR_SY8824X) += sy8824x.o
> diff --git a/drivers/regulator/sun20i-regulator.c b/drivers/regulator/sun20i-regulator.c
> new file mode 100644
> index 000000000000..031bcc3dee50
> --- /dev/null
> +++ b/drivers/regulator/sun20i-regulator.c
> @@ -0,0 +1,150 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// Copyright (c) 2021-2022 Samuel Holland <samuel@sholland.org>
> +//
> +
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/driver.h>
> +
> +#define SUN20I_SYS_LDO_CTRL_REG		0x150
> +
> +struct sun20i_regulator_data {
> +	const struct regulator_desc	*descs;
> +	unsigned int			ndescs;
> +};
> +
> +/* regulator_list_voltage_linear() modified for the non-integral uV_step. */
> +static int sun20i_d1_system_ldo_list_voltage(struct regulator_dev *rdev,
> +					     unsigned int selector)
> +{
> +	const struct regulator_desc *desc = rdev->desc;
> +	unsigned int uV;
> +
> +	if (selector >= desc->n_voltages)
> +		return -EINVAL;
> +
> +	uV = desc->min_uV + (desc->uV_step * selector);
> +
> +	/* Produce correctly-rounded absolute voltages. */
> +	return uV + ((selector + 1 + (desc->min_uV % 4)) / 3);
> +}
> +
> +static const struct regulator_ops sun20i_d1_system_ldo_ops = {
> +	.list_voltage		= sun20i_d1_system_ldo_list_voltage,
> +	.map_voltage		= regulator_map_voltage_ascend,
> +	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
> +	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
> +};
> +
> +static const struct regulator_desc sun20i_d1_system_ldo_descs[] = {
> +	{
> +		.name		= "ldoa",
> +		.supply_name	= "ldo-in",
> +		.of_match	= "ldoa",
> +		.ops		= &sun20i_d1_system_ldo_ops,
> +		.type		= REGULATOR_VOLTAGE,
> +		.owner		= THIS_MODULE,
> +		.n_voltages	= 32,
> +		.min_uV		= 1600000,
> +		.uV_step	= 13333, /* repeating */

So while I see that those values are probably the closest we can with a
simple linear algorithm, they first two values seem to be slightly off
from those values in the manual:
sel diff algor  manual
 0:   7 (1.600 - 1.593)
 1:   6 (1.613 - 1.607)
Oddly enough the rest of the values are spot on.
I don't know if this really matters, or if the LDOs are actually
accurate enough to that level of precision, or if it's a manual bug, or
if we really care at all, but it might warrant some comment, I guess?
I just got triggered by the min value not being the first value in the
list.

> +		.vsel_reg	= SUN20I_SYS_LDO_CTRL_REG,
> +		.vsel_mask	= GENMASK(7, 0),
> +	},
> +	{
> +		.name		= "ldob",
> +		.supply_name	= "ldo-in",
> +		.of_match	= "ldob",
> +		.ops		= &sun20i_d1_system_ldo_ops,
> +		.type		= REGULATOR_VOLTAGE,
> +		.owner		= THIS_MODULE,
> +		.n_voltages	= 64,
> +		.min_uV		= 1166666,
> +		.uV_step	= 13333, /* repeating */

For LDOB it seems to be worse, as the second half is constantly off by
what looks like 6.666mV:
sel diff algor  manual
...
32:   0 (1.593 - 1.593)
33:   0 (1.607 - 1.607)
34:  -7 (1.620 - 1.627)
35:  -7 (1.633 - 1.64)
36:  -6 (1.647 - 1.653)
...
63:  -6 (2.007 - 2.013)
The first half is correct, though. Closer inspection reveals that
everything with bit 5 set is exactly the same as LDOA. Maybe we can use
that to our advantage?

Cheers,
Andre

> +		.vsel_reg	= SUN20I_SYS_LDO_CTRL_REG,
> +		.vsel_mask	= GENMASK(15, 8),
> +	},
> +};
> +
> +static const struct sun20i_regulator_data sun20i_d1_system_ldos = {
> +	.descs	= sun20i_d1_system_ldo_descs,
> +	.ndescs	= ARRAY_SIZE(sun20i_d1_system_ldo_descs),
> +};
> +
> +static struct regmap *sun20i_regulator_get_regmap(struct device *dev)
> +{
> +	struct regmap *regmap;
> +
> +	/*
> +	 * First try the syscon interface. The system control device is not
> +	 * compatible with "syscon", so fall back to getting the regmap from
> +	 * its platform device. This is ugly, but required for devicetree
> +	 * backward compatibility.
> +	 */
> +	regmap = syscon_node_to_regmap(dev->parent->of_node);
> +	if (!IS_ERR(regmap))
> +		return regmap;
> +
> +	regmap = dev_get_regmap(dev->parent, NULL);
> +	if (regmap)
> +		return regmap;
> +
> +	return ERR_PTR(-EPROBE_DEFER);
> +}
> +
> +static int sun20i_regulator_probe(struct platform_device *pdev)
> +{
> +	const struct sun20i_regulator_data *data;
> +	struct device *dev = &pdev->dev;
> +	struct regulator_config config;
> +	struct regmap *regmap;
> +
> +	data = of_device_get_match_data(dev);
> +	if (!data)
> +		return -EINVAL;
> +
> +	regmap = sun20i_regulator_get_regmap(dev);
> +	if (IS_ERR(regmap))
> +		return dev_err_probe(dev, PTR_ERR(regmap), "Failed to get regmap\n");
> +
> +	config = (struct regulator_config) {
> +		.dev	= dev,
> +		.regmap	= regmap,
> +	};
> +
> +	for (unsigned int i = 0; i < data->ndescs; ++i) {
> +		const struct regulator_desc *desc = &data->descs[i];
> +		struct regulator_dev *rdev;
> +
> +		rdev = devm_regulator_register(dev, desc, &config);
> +		if (IS_ERR(rdev))
> +			return PTR_ERR(rdev);
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id sun20i_regulator_of_match[] = {
> +	{
> +		.compatible = "allwinner,sun20i-d1-system-ldos",
> +		.data = &sun20i_d1_system_ldos,
> +	},
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, sun20i_regulator_of_match);
> +
> +static struct platform_driver sun20i_regulator_driver = {
> +	.probe	= sun20i_regulator_probe,
> +	.driver	= {
> +		.name		= "sun20i-regulator",
> +		.of_match_table	= sun20i_regulator_of_match,
> +	},
> +};
> +module_platform_driver(sun20i_regulator_driver);
> +
> +MODULE_AUTHOR("Samuel Holland <samuel@sholland.org>");
> +MODULE_DESCRIPTION("Allwinner D1 internal LDO driver");
> +MODULE_LICENSE("GPL");

Re: [PATCH v4 2/4] regulator: sun20i: Add Allwinner D1 LDOs driver

[Andre Przywara]

On Thu, 24 Nov 2022 22:01:10 -0600
Samuel Holland <samuel@sholland.org> wrote:

Hi Samuel,

> D1 contains two pairs of LDOs, "analog" LDOs and "system" LDOs. They are
> similar and can share a driver, but only the system LDOs have a DT
> binding defined so far.
> 
> The system LDOs have a single linear range. The voltage step is not an
> integer, so a custom .list_voltage is needed to get the rounding right.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> 
> Changes in v4:
>  - Drop the analog LDOs until the codec binding is ready
> 
> Changes in v3:
>  - Adjust control flow in sun20i_regulator_get_regmap() for clarity
> 
> Changes in v2:
>  - Use decimal numbers for .n_voltages instead of field widths
>  - Get the regmap from the parent device instead of a property/phandle
> 
>  drivers/regulator/Kconfig            |   8 ++
>  drivers/regulator/Makefile           |   1 +
>  drivers/regulator/sun20i-regulator.c | 150 +++++++++++++++++++++++++++
>  3 files changed, 159 insertions(+)
>  create mode 100644 drivers/regulator/sun20i-regulator.c
> 
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index 070e4403c6c2..8480532114c1 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -1280,6 +1280,14 @@ config REGULATOR_STW481X_VMMC
>  	  This driver supports the internal VMMC regulator in the STw481x
>  	  PMIC chips.
>  
> +config REGULATOR_SUN20I
> +	tristate "Allwinner D1 internal LDOs"
> +	depends on ARCH_SUNXI || COMPILE_TEST
> +	select MFD_SYSCON
> +	default ARCH_SUNXI
> +	help
> +	  This driver supports the internal LDOs in the Allwinner D1 SoC.
> +
>  config REGULATOR_SY7636A
>  	tristate "Silergy SY7636A voltage regulator"
>  	depends on MFD_SY7636A
> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index 5962307e1130..8e9b5a21123d 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -150,6 +150,7 @@ 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_SUN20I) += sun20i-regulator.o
>  obj-$(CONFIG_REGULATOR_SY7636A) += sy7636a-regulator.o
>  obj-$(CONFIG_REGULATOR_SY8106A) += sy8106a-regulator.o
>  obj-$(CONFIG_REGULATOR_SY8824X) += sy8824x.o
> diff --git a/drivers/regulator/sun20i-regulator.c b/drivers/regulator/sun20i-regulator.c
> new file mode 100644
> index 000000000000..031bcc3dee50
> --- /dev/null
> +++ b/drivers/regulator/sun20i-regulator.c
> @@ -0,0 +1,150 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// Copyright (c) 2021-2022 Samuel Holland <samuel@sholland.org>
> +//
> +
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/driver.h>
> +
> +#define SUN20I_SYS_LDO_CTRL_REG		0x150
> +
> +struct sun20i_regulator_data {
> +	const struct regulator_desc	*descs;
> +	unsigned int			ndescs;
> +};
> +
> +/* regulator_list_voltage_linear() modified for the non-integral uV_step. */
> +static int sun20i_d1_system_ldo_list_voltage(struct regulator_dev *rdev,
> +					     unsigned int selector)
> +{
> +	const struct regulator_desc *desc = rdev->desc;
> +	unsigned int uV;
> +
> +	if (selector >= desc->n_voltages)
> +		return -EINVAL;
> +
> +	uV = desc->min_uV + (desc->uV_step * selector);
> +
> +	/* Produce correctly-rounded absolute voltages. */
> +	return uV + ((selector + 1 + (desc->min_uV % 4)) / 3);
> +}
> +
> +static const struct regulator_ops sun20i_d1_system_ldo_ops = {
> +	.list_voltage		= sun20i_d1_system_ldo_list_voltage,
> +	.map_voltage		= regulator_map_voltage_ascend,
> +	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
> +	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
> +};
> +
> +static const struct regulator_desc sun20i_d1_system_ldo_descs[] = {
> +	{
> +		.name		= "ldoa",
> +		.supply_name	= "ldo-in",
> +		.of_match	= "ldoa",
> +		.ops		= &sun20i_d1_system_ldo_ops,
> +		.type		= REGULATOR_VOLTAGE,
> +		.owner		= THIS_MODULE,
> +		.n_voltages	= 32,
> +		.min_uV		= 1600000,
> +		.uV_step	= 13333, /* repeating */

So while I see that those values are probably the closest we can with a
simple linear algorithm, they first two values seem to be slightly off
from those values in the manual:
sel diff algor  manual
 0:   7 (1.600 - 1.593)
 1:   6 (1.613 - 1.607)
Oddly enough the rest of the values are spot on.
I don't know if this really matters, or if the LDOs are actually
accurate enough to that level of precision, or if it's a manual bug, or
if we really care at all, but it might warrant some comment, I guess?
I just got triggered by the min value not being the first value in the
list.

> +		.vsel_reg	= SUN20I_SYS_LDO_CTRL_REG,
> +		.vsel_mask	= GENMASK(7, 0),
> +	},
> +	{
> +		.name		= "ldob",
> +		.supply_name	= "ldo-in",
> +		.of_match	= "ldob",
> +		.ops		= &sun20i_d1_system_ldo_ops,
> +		.type		= REGULATOR_VOLTAGE,
> +		.owner		= THIS_MODULE,
> +		.n_voltages	= 64,
> +		.min_uV		= 1166666,
> +		.uV_step	= 13333, /* repeating */

For LDOB it seems to be worse, as the second half is constantly off by
what looks like 6.666mV:
sel diff algor  manual
...
32:   0 (1.593 - 1.593)
33:   0 (1.607 - 1.607)
34:  -7 (1.620 - 1.627)
35:  -7 (1.633 - 1.64)
36:  -6 (1.647 - 1.653)
...
63:  -6 (2.007 - 2.013)
The first half is correct, though. Closer inspection reveals that
everything with bit 5 set is exactly the same as LDOA. Maybe we can use
that to our advantage?

Cheers,
Andre

> +		.vsel_reg	= SUN20I_SYS_LDO_CTRL_REG,
> +		.vsel_mask	= GENMASK(15, 8),
> +	},
> +};
> +
> +static const struct sun20i_regulator_data sun20i_d1_system_ldos = {
> +	.descs	= sun20i_d1_system_ldo_descs,
> +	.ndescs	= ARRAY_SIZE(sun20i_d1_system_ldo_descs),
> +};
> +
> +static struct regmap *sun20i_regulator_get_regmap(struct device *dev)
> +{
> +	struct regmap *regmap;
> +
> +	/*
> +	 * First try the syscon interface. The system control device is not
> +	 * compatible with "syscon", so fall back to getting the regmap from
> +	 * its platform device. This is ugly, but required for devicetree
> +	 * backward compatibility.
> +	 */
> +	regmap = syscon_node_to_regmap(dev->parent->of_node);
> +	if (!IS_ERR(regmap))
> +		return regmap;
> +
> +	regmap = dev_get_regmap(dev->parent, NULL);
> +	if (regmap)
> +		return regmap;
> +
> +	return ERR_PTR(-EPROBE_DEFER);
> +}
> +
> +static int sun20i_regulator_probe(struct platform_device *pdev)
> +{
> +	const struct sun20i_regulator_data *data;
> +	struct device *dev = &pdev->dev;
> +	struct regulator_config config;
> +	struct regmap *regmap;
> +
> +	data = of_device_get_match_data(dev);
> +	if (!data)
> +		return -EINVAL;
> +
> +	regmap = sun20i_regulator_get_regmap(dev);
> +	if (IS_ERR(regmap))
> +		return dev_err_probe(dev, PTR_ERR(regmap), "Failed to get regmap\n");
> +
> +	config = (struct regulator_config) {
> +		.dev	= dev,
> +		.regmap	= regmap,
> +	};
> +
> +	for (unsigned int i = 0; i < data->ndescs; ++i) {
> +		const struct regulator_desc *desc = &data->descs[i];
> +		struct regulator_dev *rdev;
> +
> +		rdev = devm_regulator_register(dev, desc, &config);
> +		if (IS_ERR(rdev))
> +			return PTR_ERR(rdev);
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id sun20i_regulator_of_match[] = {
> +	{
> +		.compatible = "allwinner,sun20i-d1-system-ldos",
> +		.data = &sun20i_d1_system_ldos,
> +	},
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, sun20i_regulator_of_match);
> +
> +static struct platform_driver sun20i_regulator_driver = {
> +	.probe	= sun20i_regulator_probe,
> +	.driver	= {
> +		.name		= "sun20i-regulator",
> +		.of_match_table	= sun20i_regulator_of_match,
> +	},
> +};
> +module_platform_driver(sun20i_regulator_driver);
> +
> +MODULE_AUTHOR("Samuel Holland <samuel@sholland.org>");
> +MODULE_DESCRIPTION("Allwinner D1 internal LDO driver");
> +MODULE_LICENSE("GPL");


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 2/4] regulator: sun20i: Add Allwinner D1 LDOs driver

[Samuel Holland]

Hi Andre,

On 11/25/22 18:22, Andre Przywara wrote:
> On Thu, 24 Nov 2022 22:01:10 -0600
> Samuel Holland <samuel@sholland.org> wrote:
> 
> Hi Samuel,
> 
>> D1 contains two pairs of LDOs, "analog" LDOs and "system" LDOs. They are
>> similar and can share a driver, but only the system LDOs have a DT
>> binding defined so far.
>>
>> The system LDOs have a single linear range. The voltage step is not an
>> integer, so a custom .list_voltage is needed to get the rounding right.
>>
>> Signed-off-by: Samuel Holland <samuel@sholland.org>
>> ---
>>
>> Changes in v4:
>>  - Drop the analog LDOs until the codec binding is ready
>>
>> Changes in v3:
>>  - Adjust control flow in sun20i_regulator_get_regmap() for clarity
>>
>> Changes in v2:
>>  - Use decimal numbers for .n_voltages instead of field widths
>>  - Get the regmap from the parent device instead of a property/phandle
>>
>>  drivers/regulator/Kconfig            |   8 ++
>>  drivers/regulator/Makefile           |   1 +
>>  drivers/regulator/sun20i-regulator.c | 150 +++++++++++++++++++++++++++
>>  3 files changed, 159 insertions(+)
>>  create mode 100644 drivers/regulator/sun20i-regulator.c
>>
>> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
>> index 070e4403c6c2..8480532114c1 100644
>> --- a/drivers/regulator/Kconfig
>> +++ b/drivers/regulator/Kconfig
>> @@ -1280,6 +1280,14 @@ config REGULATOR_STW481X_VMMC
>>  	  This driver supports the internal VMMC regulator in the STw481x
>>  	  PMIC chips.
>>  
>> +config REGULATOR_SUN20I
>> +	tristate "Allwinner D1 internal LDOs"
>> +	depends on ARCH_SUNXI || COMPILE_TEST
>> +	select MFD_SYSCON
>> +	default ARCH_SUNXI
>> +	help
>> +	  This driver supports the internal LDOs in the Allwinner D1 SoC.
>> +
>>  config REGULATOR_SY7636A
>>  	tristate "Silergy SY7636A voltage regulator"
>>  	depends on MFD_SY7636A
>> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
>> index 5962307e1130..8e9b5a21123d 100644
>> --- a/drivers/regulator/Makefile
>> +++ b/drivers/regulator/Makefile
>> @@ -150,6 +150,7 @@ 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_SUN20I) += sun20i-regulator.o
>>  obj-$(CONFIG_REGULATOR_SY7636A) += sy7636a-regulator.o
>>  obj-$(CONFIG_REGULATOR_SY8106A) += sy8106a-regulator.o
>>  obj-$(CONFIG_REGULATOR_SY8824X) += sy8824x.o
>> diff --git a/drivers/regulator/sun20i-regulator.c b/drivers/regulator/sun20i-regulator.c
>> new file mode 100644
>> index 000000000000..031bcc3dee50
>> --- /dev/null
>> +++ b/drivers/regulator/sun20i-regulator.c
>> @@ -0,0 +1,150 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +//
>> +// Copyright (c) 2021-2022 Samuel Holland <samuel@sholland.org>
>> +//
>> +
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/regulator/driver.h>
>> +
>> +#define SUN20I_SYS_LDO_CTRL_REG		0x150
>> +
>> +struct sun20i_regulator_data {
>> +	const struct regulator_desc	*descs;
>> +	unsigned int			ndescs;
>> +};
>> +
>> +/* regulator_list_voltage_linear() modified for the non-integral uV_step. */
>> +static int sun20i_d1_system_ldo_list_voltage(struct regulator_dev *rdev,
>> +					     unsigned int selector)
>> +{
>> +	const struct regulator_desc *desc = rdev->desc;
>> +	unsigned int uV;
>> +
>> +	if (selector >= desc->n_voltages)
>> +		return -EINVAL;
>> +
>> +	uV = desc->min_uV + (desc->uV_step * selector);
>> +
>> +	/* Produce correctly-rounded absolute voltages. */
>> +	return uV + ((selector + 1 + (desc->min_uV % 4)) / 3);
>> +}
>> +
>> +static const struct regulator_ops sun20i_d1_system_ldo_ops = {
>> +	.list_voltage		= sun20i_d1_system_ldo_list_voltage,
>> +	.map_voltage		= regulator_map_voltage_ascend,
>> +	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
>> +	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
>> +};
>> +
>> +static const struct regulator_desc sun20i_d1_system_ldo_descs[] = {
>> +	{
>> +		.name		= "ldoa",
>> +		.supply_name	= "ldo-in",
>> +		.of_match	= "ldoa",
>> +		.ops		= &sun20i_d1_system_ldo_ops,
>> +		.type		= REGULATOR_VOLTAGE,
>> +		.owner		= THIS_MODULE,
>> +		.n_voltages	= 32,
>> +		.min_uV		= 1600000,
>> +		.uV_step	= 13333, /* repeating */
> 
> So while I see that those values are probably the closest we can with a
> simple linear algorithm, they first two values seem to be slightly off
> from those values in the manual:
> sel diff algor  manual
>  0:   7 (1.600 - 1.593)
>  1:   6 (1.613 - 1.607)
> Oddly enough the rest of the values are spot on.
> I don't know if this really matters, or if the LDOs are actually
> accurate enough to that level of precision, or if it's a manual bug, or
> if we really care at all, but it might warrant some comment, I guess?
> I just got triggered by the min value not being the first value in the
> list.

Whoops, I did the math based on the default 1.8 V and after some spot
checks assumed the range was linear. I should have noticed this.

Conveniently, LDOB is unused on most boards, so I can measure the range
directly. The INA219 I'm using gives me a 4 mV LSB in its most precise
mode, which I'm averaging to get 2 mV precision.

reg     measure manual  diff
============================
0x00:   1.160   1.167   -7
0x08:   1.266   1.273   -7
0x09:   1.280   1.287   -7
0x0a:   1.292   1.300   -8
0x0b:   1.306   1.313   -7
0x0c:   1.320   1.327   -7
0x0d:   1.332   1.340   -8
0x0e:   1.346   1.353   -7
0x0f:   1.360   1.367   -7
0x10:   1.372   1.380   -8
0x18:   1.480   1.487   -7
0x1c:   1.532   1.540   -8
0x1d:   1.546   1.553   -7
0x1e:   1.560   1.567   -7
0x1f:   1.572   1.580   -8
0x20:   1.586   1.593   -7 <<
0x21:   1.600   1.607   -7 <<
0x22:   1.620   1.627   -7 <<
0x23:   1.632   1.640   -8 <<
0x24:   1.646   1.653   -7
0x25:   1.660   1.667   -7
0x26:   1.672   1.680   -8
0x27:   1.686   1.693   -7
0x28:   1.700   1.707   -7
0x30:   1.806   1.813   -7
0x38:   1.912   1.920   -8
0x3e:   1.992   2.000   -8
0x3f:   2.006   2.013   -7

So it looks like the LDOs are accurate, and the manual is quite correct.

I'm willing to blame the constant offset on my crude test setup.

>> +		.vsel_reg	= SUN20I_SYS_LDO_CTRL_REG,
>> +		.vsel_mask	= GENMASK(7, 0),
>> +	},
>> +	{
>> +		.name		= "ldob",
>> +		.supply_name	= "ldo-in",
>> +		.of_match	= "ldob",
>> +		.ops		= &sun20i_d1_system_ldo_ops,
>> +		.type		= REGULATOR_VOLTAGE,
>> +		.owner		= THIS_MODULE,
>> +		.n_voltages	= 64,
>> +		.min_uV		= 1166666,
>> +		.uV_step	= 13333, /* repeating */
> 
> For LDOB it seems to be worse, as the second half is constantly off by
> what looks like 6.666mV:
> sel diff algor  manual
> ...
> 32:   0 (1.593 - 1.593)
> 33:   0 (1.607 - 1.607)
> 34:  -7 (1.620 - 1.627)
> 35:  -7 (1.633 - 1.64)
> 36:  -6 (1.647 - 1.653)
> ...
> 63:  -6 (2.007 - 2.013)
> The first half is correct, though. Closer inspection reveals that
> everything with bit 5 set is exactly the same as LDOA. Maybe we can use
> that to our advantage?

Since I already have a custom .list_voltage implementation, I think the
simplest solution is:

	if (uV >= 1620000)
		uV += 7000;

or similar to get the rounding right.

Regards,
Samuel

> Cheers,
> Andre
> 
>> +		.vsel_reg	= SUN20I_SYS_LDO_CTRL_REG,
>> +		.vsel_mask	= GENMASK(15, 8),
>> +	},
>> +};
>> +
>> +static const struct sun20i_regulator_data sun20i_d1_system_ldos = {
>> +	.descs	= sun20i_d1_system_ldo_descs,
>> +	.ndescs	= ARRAY_SIZE(sun20i_d1_system_ldo_descs),
>> +};
>> +
>> +static struct regmap *sun20i_regulator_get_regmap(struct device *dev)
>> +{
>> +	struct regmap *regmap;
>> +
>> +	/*
>> +	 * First try the syscon interface. The system control device is not
>> +	 * compatible with "syscon", so fall back to getting the regmap from
>> +	 * its platform device. This is ugly, but required for devicetree
>> +	 * backward compatibility.
>> +	 */
>> +	regmap = syscon_node_to_regmap(dev->parent->of_node);
>> +	if (!IS_ERR(regmap))
>> +		return regmap;
>> +
>> +	regmap = dev_get_regmap(dev->parent, NULL);
>> +	if (regmap)
>> +		return regmap;
>> +
>> +	return ERR_PTR(-EPROBE_DEFER);
>> +}
>> +
>> +static int sun20i_regulator_probe(struct platform_device *pdev)
>> +{
>> +	const struct sun20i_regulator_data *data;
>> +	struct device *dev = &pdev->dev;
>> +	struct regulator_config config;
>> +	struct regmap *regmap;
>> +
>> +	data = of_device_get_match_data(dev);
>> +	if (!data)
>> +		return -EINVAL;
>> +
>> +	regmap = sun20i_regulator_get_regmap(dev);
>> +	if (IS_ERR(regmap))
>> +		return dev_err_probe(dev, PTR_ERR(regmap), "Failed to get regmap\n");
>> +
>> +	config = (struct regulator_config) {
>> +		.dev	= dev,
>> +		.regmap	= regmap,
>> +	};
>> +
>> +	for (unsigned int i = 0; i < data->ndescs; ++i) {
>> +		const struct regulator_desc *desc = &data->descs[i];
>> +		struct regulator_dev *rdev;
>> +
>> +		rdev = devm_regulator_register(dev, desc, &config);
>> +		if (IS_ERR(rdev))
>> +			return PTR_ERR(rdev);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id sun20i_regulator_of_match[] = {
>> +	{
>> +		.compatible = "allwinner,sun20i-d1-system-ldos",
>> +		.data = &sun20i_d1_system_ldos,
>> +	},
>> +	{ },
>> +};
>> +MODULE_DEVICE_TABLE(of, sun20i_regulator_of_match);
>> +
>> +static struct platform_driver sun20i_regulator_driver = {
>> +	.probe	= sun20i_regulator_probe,
>> +	.driver	= {
>> +		.name		= "sun20i-regulator",
>> +		.of_match_table	= sun20i_regulator_of_match,
>> +	},
>> +};
>> +module_platform_driver(sun20i_regulator_driver);
>> +
>> +MODULE_AUTHOR("Samuel Holland <samuel@sholland.org>");
>> +MODULE_DESCRIPTION("Allwinner D1 internal LDO driver");
>> +MODULE_LICENSE("GPL");
> 

Re: [PATCH v4 2/4] regulator: sun20i: Add Allwinner D1 LDOs driver

[Samuel Holland]

Hi Andre,

On 11/25/22 18:22, Andre Przywara wrote:
> On Thu, 24 Nov 2022 22:01:10 -0600
> Samuel Holland <samuel@sholland.org> wrote:
> 
> Hi Samuel,
> 
>> D1 contains two pairs of LDOs, "analog" LDOs and "system" LDOs. They are
>> similar and can share a driver, but only the system LDOs have a DT
>> binding defined so far.
>>
>> The system LDOs have a single linear range. The voltage step is not an
>> integer, so a custom .list_voltage is needed to get the rounding right.
>>
>> Signed-off-by: Samuel Holland <samuel@sholland.org>
>> ---
>>
>> Changes in v4:
>>  - Drop the analog LDOs until the codec binding is ready
>>
>> Changes in v3:
>>  - Adjust control flow in sun20i_regulator_get_regmap() for clarity
>>
>> Changes in v2:
>>  - Use decimal numbers for .n_voltages instead of field widths
>>  - Get the regmap from the parent device instead of a property/phandle
>>
>>  drivers/regulator/Kconfig            |   8 ++
>>  drivers/regulator/Makefile           |   1 +
>>  drivers/regulator/sun20i-regulator.c | 150 +++++++++++++++++++++++++++
>>  3 files changed, 159 insertions(+)
>>  create mode 100644 drivers/regulator/sun20i-regulator.c
>>
>> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
>> index 070e4403c6c2..8480532114c1 100644
>> --- a/drivers/regulator/Kconfig
>> +++ b/drivers/regulator/Kconfig
>> @@ -1280,6 +1280,14 @@ config REGULATOR_STW481X_VMMC
>>  	  This driver supports the internal VMMC regulator in the STw481x
>>  	  PMIC chips.
>>  
>> +config REGULATOR_SUN20I
>> +	tristate "Allwinner D1 internal LDOs"
>> +	depends on ARCH_SUNXI || COMPILE_TEST
>> +	select MFD_SYSCON
>> +	default ARCH_SUNXI
>> +	help
>> +	  This driver supports the internal LDOs in the Allwinner D1 SoC.
>> +
>>  config REGULATOR_SY7636A
>>  	tristate "Silergy SY7636A voltage regulator"
>>  	depends on MFD_SY7636A
>> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
>> index 5962307e1130..8e9b5a21123d 100644
>> --- a/drivers/regulator/Makefile
>> +++ b/drivers/regulator/Makefile
>> @@ -150,6 +150,7 @@ 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_SUN20I) += sun20i-regulator.o
>>  obj-$(CONFIG_REGULATOR_SY7636A) += sy7636a-regulator.o
>>  obj-$(CONFIG_REGULATOR_SY8106A) += sy8106a-regulator.o
>>  obj-$(CONFIG_REGULATOR_SY8824X) += sy8824x.o
>> diff --git a/drivers/regulator/sun20i-regulator.c b/drivers/regulator/sun20i-regulator.c
>> new file mode 100644
>> index 000000000000..031bcc3dee50
>> --- /dev/null
>> +++ b/drivers/regulator/sun20i-regulator.c
>> @@ -0,0 +1,150 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +//
>> +// Copyright (c) 2021-2022 Samuel Holland <samuel@sholland.org>
>> +//
>> +
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/regulator/driver.h>
>> +
>> +#define SUN20I_SYS_LDO_CTRL_REG		0x150
>> +
>> +struct sun20i_regulator_data {
>> +	const struct regulator_desc	*descs;
>> +	unsigned int			ndescs;
>> +};
>> +
>> +/* regulator_list_voltage_linear() modified for the non-integral uV_step. */
>> +static int sun20i_d1_system_ldo_list_voltage(struct regulator_dev *rdev,
>> +					     unsigned int selector)
>> +{
>> +	const struct regulator_desc *desc = rdev->desc;
>> +	unsigned int uV;
>> +
>> +	if (selector >= desc->n_voltages)
>> +		return -EINVAL;
>> +
>> +	uV = desc->min_uV + (desc->uV_step * selector);
>> +
>> +	/* Produce correctly-rounded absolute voltages. */
>> +	return uV + ((selector + 1 + (desc->min_uV % 4)) / 3);
>> +}
>> +
>> +static const struct regulator_ops sun20i_d1_system_ldo_ops = {
>> +	.list_voltage		= sun20i_d1_system_ldo_list_voltage,
>> +	.map_voltage		= regulator_map_voltage_ascend,
>> +	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
>> +	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
>> +};
>> +
>> +static const struct regulator_desc sun20i_d1_system_ldo_descs[] = {
>> +	{
>> +		.name		= "ldoa",
>> +		.supply_name	= "ldo-in",
>> +		.of_match	= "ldoa",
>> +		.ops		= &sun20i_d1_system_ldo_ops,
>> +		.type		= REGULATOR_VOLTAGE,
>> +		.owner		= THIS_MODULE,
>> +		.n_voltages	= 32,
>> +		.min_uV		= 1600000,
>> +		.uV_step	= 13333, /* repeating */
> 
> So while I see that those values are probably the closest we can with a
> simple linear algorithm, they first two values seem to be slightly off
> from those values in the manual:
> sel diff algor  manual
>  0:   7 (1.600 - 1.593)
>  1:   6 (1.613 - 1.607)
> Oddly enough the rest of the values are spot on.
> I don't know if this really matters, or if the LDOs are actually
> accurate enough to that level of precision, or if it's a manual bug, or
> if we really care at all, but it might warrant some comment, I guess?
> I just got triggered by the min value not being the first value in the
> list.

Whoops, I did the math based on the default 1.8 V and after some spot
checks assumed the range was linear. I should have noticed this.

Conveniently, LDOB is unused on most boards, so I can measure the range
directly. The INA219 I'm using gives me a 4 mV LSB in its most precise
mode, which I'm averaging to get 2 mV precision.

reg     measure manual  diff
============================
0x00:   1.160   1.167   -7
0x08:   1.266   1.273   -7
0x09:   1.280   1.287   -7
0x0a:   1.292   1.300   -8
0x0b:   1.306   1.313   -7
0x0c:   1.320   1.327   -7
0x0d:   1.332   1.340   -8
0x0e:   1.346   1.353   -7
0x0f:   1.360   1.367   -7
0x10:   1.372   1.380   -8
0x18:   1.480   1.487   -7
0x1c:   1.532   1.540   -8
0x1d:   1.546   1.553   -7
0x1e:   1.560   1.567   -7
0x1f:   1.572   1.580   -8
0x20:   1.586   1.593   -7 <<
0x21:   1.600   1.607   -7 <<
0x22:   1.620   1.627   -7 <<
0x23:   1.632   1.640   -8 <<
0x24:   1.646   1.653   -7
0x25:   1.660   1.667   -7
0x26:   1.672   1.680   -8
0x27:   1.686   1.693   -7
0x28:   1.700   1.707   -7
0x30:   1.806   1.813   -7
0x38:   1.912   1.920   -8
0x3e:   1.992   2.000   -8
0x3f:   2.006   2.013   -7

So it looks like the LDOs are accurate, and the manual is quite correct.

I'm willing to blame the constant offset on my crude test setup.

>> +		.vsel_reg	= SUN20I_SYS_LDO_CTRL_REG,
>> +		.vsel_mask	= GENMASK(7, 0),
>> +	},
>> +	{
>> +		.name		= "ldob",
>> +		.supply_name	= "ldo-in",
>> +		.of_match	= "ldob",
>> +		.ops		= &sun20i_d1_system_ldo_ops,
>> +		.type		= REGULATOR_VOLTAGE,
>> +		.owner		= THIS_MODULE,
>> +		.n_voltages	= 64,
>> +		.min_uV		= 1166666,
>> +		.uV_step	= 13333, /* repeating */
> 
> For LDOB it seems to be worse, as the second half is constantly off by
> what looks like 6.666mV:
> sel diff algor  manual
> ...
> 32:   0 (1.593 - 1.593)
> 33:   0 (1.607 - 1.607)
> 34:  -7 (1.620 - 1.627)
> 35:  -7 (1.633 - 1.64)
> 36:  -6 (1.647 - 1.653)
> ...
> 63:  -6 (2.007 - 2.013)
> The first half is correct, though. Closer inspection reveals that
> everything with bit 5 set is exactly the same as LDOA. Maybe we can use
> that to our advantage?

Since I already have a custom .list_voltage implementation, I think the
simplest solution is:

	if (uV >= 1620000)
		uV += 7000;

or similar to get the rounding right.

Regards,
Samuel

> Cheers,
> Andre
> 
>> +		.vsel_reg	= SUN20I_SYS_LDO_CTRL_REG,
>> +		.vsel_mask	= GENMASK(15, 8),
>> +	},
>> +};
>> +
>> +static const struct sun20i_regulator_data sun20i_d1_system_ldos = {
>> +	.descs	= sun20i_d1_system_ldo_descs,
>> +	.ndescs	= ARRAY_SIZE(sun20i_d1_system_ldo_descs),
>> +};
>> +
>> +static struct regmap *sun20i_regulator_get_regmap(struct device *dev)
>> +{
>> +	struct regmap *regmap;
>> +
>> +	/*
>> +	 * First try the syscon interface. The system control device is not
>> +	 * compatible with "syscon", so fall back to getting the regmap from
>> +	 * its platform device. This is ugly, but required for devicetree
>> +	 * backward compatibility.
>> +	 */
>> +	regmap = syscon_node_to_regmap(dev->parent->of_node);
>> +	if (!IS_ERR(regmap))
>> +		return regmap;
>> +
>> +	regmap = dev_get_regmap(dev->parent, NULL);
>> +	if (regmap)
>> +		return regmap;
>> +
>> +	return ERR_PTR(-EPROBE_DEFER);
>> +}
>> +
>> +static int sun20i_regulator_probe(struct platform_device *pdev)
>> +{
>> +	const struct sun20i_regulator_data *data;
>> +	struct device *dev = &pdev->dev;
>> +	struct regulator_config config;
>> +	struct regmap *regmap;
>> +
>> +	data = of_device_get_match_data(dev);
>> +	if (!data)
>> +		return -EINVAL;
>> +
>> +	regmap = sun20i_regulator_get_regmap(dev);
>> +	if (IS_ERR(regmap))
>> +		return dev_err_probe(dev, PTR_ERR(regmap), "Failed to get regmap\n");
>> +
>> +	config = (struct regulator_config) {
>> +		.dev	= dev,
>> +		.regmap	= regmap,
>> +	};
>> +
>> +	for (unsigned int i = 0; i < data->ndescs; ++i) {
>> +		const struct regulator_desc *desc = &data->descs[i];
>> +		struct regulator_dev *rdev;
>> +
>> +		rdev = devm_regulator_register(dev, desc, &config);
>> +		if (IS_ERR(rdev))
>> +			return PTR_ERR(rdev);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id sun20i_regulator_of_match[] = {
>> +	{
>> +		.compatible = "allwinner,sun20i-d1-system-ldos",
>> +		.data = &sun20i_d1_system_ldos,
>> +	},
>> +	{ },
>> +};
>> +MODULE_DEVICE_TABLE(of, sun20i_regulator_of_match);
>> +
>> +static struct platform_driver sun20i_regulator_driver = {
>> +	.probe	= sun20i_regulator_probe,
>> +	.driver	= {
>> +		.name		= "sun20i-regulator",
>> +		.of_match_table	= sun20i_regulator_of_match,
>> +	},
>> +};
>> +module_platform_driver(sun20i_regulator_driver);
>> +
>> +MODULE_AUTHOR("Samuel Holland <samuel@sholland.org>");
>> +MODULE_DESCRIPTION("Allwinner D1 internal LDO driver");
>> +MODULE_LICENSE("GPL");
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 4/4] soc: sunxi: sram: Only iterate over SRAM children

[Jernej Škrabec]

Dne petek, 25. november 2022 ob 05:01:12 CET je Samuel Holland napisal(a):
> Now that a regulators child is accepted by the controller binding, the
> debugfs show routine must be explicitly limited to mmio-sram children.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

Re: [PATCH v4 4/4] soc: sunxi: sram: Only iterate over SRAM children

[Jernej Škrabec]

Dne petek, 25. november 2022 ob 05:01:12 CET je Samuel Holland napisal(a):
> Now that a regulators child is accepted by the controller binding, the
> debugfs show routine must be explicitly limited to mmio-sram children.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel