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

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

[Samuel Holland]

This series adds bindings and a driver for the two pairs of LDOs
inside the Allwinner D1 SoC.

A preparatory binding and driver change is required for the SRAM
controller, so the regulators device can be its child node.

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):
  dt-bindings: sram: sunxi-sram: Add optional regulators child
  soc: sunxi: sram: Only iterate over SRAM children
  regulator: dt-bindings: Add Allwinner D1 LDOs
  regulator: sun20i: Add support for Allwinner D1 LDOs

 .../allwinner,sun20i-d1-analog-ldos.yaml      |  65 +++++
 .../allwinner,sun20i-d1-system-ldos.yaml      |  57 +++++
 .../allwinner,sun4i-a10-system-control.yaml   |   3 +
 drivers/regulator/Kconfig                     |   8 +
 drivers/regulator/Makefile                    |   1 +
 drivers/regulator/sun20i-regulator.c          | 232 ++++++++++++++++++
 drivers/soc/sunxi/sunxi_sram.c                |   3 +
 7 files changed, 369 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog-ldos.yaml
 create mode 100644 Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.yaml
 create mode 100644 drivers/regulator/sun20i-regulator.c

-- 
2.35.1


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

[PATCH v2 1/4] dt-bindings: sram: sunxi-sram: Add optional 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 these regulators
in addition to SRAM child nodes.

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

Changes in v2:
 - New patch for v2

 .../bindings/sram/allwinner,sun4i-a10-system-control.yaml      | 3 +++
 1 file changed, 3 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 1c426c211e36..cc57836b2906 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
 
+  regulators:
+    type: object
+
 patternProperties:
   "^sram@[a-z0-9]+":
     type: object
-- 
2.35.1


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

[PATCH v2 2/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 "sram" children.
Otherwise, it will crash because the "regulators" node has no address.

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

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 a8f3876963a0..b3016b9698fb 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_node_name_eq(sram_node, "sram"))
+			continue;
+
 		sram_addr_p = of_get_address(sram_node, 0, NULL, NULL);
 
 		seq_printf(s, "sram@%08x\n",
-- 
2.35.1


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

[PATCH v2 3/4] regulator: dt-bindings: Add Allwinner D1 LDOs

[Samuel Holland]

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

The other pair of LDOs powers the analog power domains inside the SoC,
including the audio codec, thermal sensor, and ADCs. These LDOs require
a 0.9 V bandgap voltage reference. The calibration value for the voltage
reference is stored in an eFuse, accessed via an NVMEM cell.

Neither LDO control register is in its own MMIO range; instead, each
regulator device relies on a regmap/syscon exported by its parent.

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

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

 .../allwinner,sun20i-d1-analog-ldos.yaml      | 65 +++++++++++++++++++
 .../allwinner,sun20i-d1-system-ldos.yaml      | 57 ++++++++++++++++
 2 files changed, 122 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog-ldos.yaml
 create mode 100644 Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.yaml

diff --git a/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog-ldos.yaml b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog-ldos.yaml
new file mode 100644
index 000000000000..19c984ef4e53
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog-ldos.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/allwinner,sun20i-d1-analog-ldos.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner D1 Analog LDOs
+
+description:
+  Allwinner D1 contains a set of LDOs which are designed to supply analog power
+  inside and outside the SoC. They are controlled by a register within the audio
+  codec MMIO space, but which is not part of the audio codec clock/reset domain.
+
+maintainers:
+  - Samuel Holland <samuel@sholland.org>
+
+properties:
+  compatible:
+    enum:
+      - allwinner,sun20i-d1-analog-ldos
+
+  nvmem-cells:
+    items:
+      - description: NVMEM cell for the calibrated bandgap reference trim value
+
+  nvmem-cell-names:
+    items:
+      - const: bg_trim
+
+patternProperties:
+  "^(aldo|hpldo)$":
+    type: object
+    $ref: regulator.yaml#
+
+required:
+  - compatible
+  - nvmem-cells
+  - nvmem-cell-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    audio-codec@2030000 {
+        compatible = "simple-mfd", "syscon";
+        reg = <0x2030000 0x1000>;
+
+        regulators {
+            compatible = "allwinner,sun20i-d1-analog-ldos";
+            nvmem-cells = <&bg_trim>;
+            nvmem-cell-names = "bg_trim";
+
+            reg_aldo: aldo {
+                regulator-min-microvolt = <1800000>;
+                regulator-max-microvolt = <1800000>;
+            };
+
+            reg_hpldo: hpldo {
+                regulator-min-microvolt = <1800000>;
+                regulator-max-microvolt = <1800000>;
+            };
+        };
+    };
+
+...
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..c95030a827d6
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.yaml
@@ -0,0 +1,57 @@
+# 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
+
+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.
+
+maintainers:
+  - Samuel Holland <samuel@sholland.org>
+
+properties:
+  compatible:
+    enum:
+      - allwinner,sun20i-d1-system-ldos
+
+patternProperties:
+  "^(ldoa|ldob)$":
+    type: object
+    $ref: regulator.yaml#
+
+required:
+  - compatible
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    syscon@3000000 {
+        compatible = "allwinner,sun20i-d1-system-control";
+        reg = <0x3000000 0x1000>;
+        ranges;
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        regulators {
+            compatible = "allwinner,sun20i-d1-system-ldos";
+
+            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.35.1


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

[PATCH v2 4/4] regulator: sun20i: Add support for Allwinner D1 LDOs

[Samuel Holland]

D1 contains two pairs of LDOs. Since they have similar bindings, and
they always exist together, put them in a single driver.

The analog LDOs are relatively boring, with a single linear range. Their
one quirk is that a bandgap reference must be calibrated for them to
produce the correct voltage.

The system LDOs have the complication that their 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 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 | 232 +++++++++++++++++++++++++++
 3 files changed, 241 insertions(+)
 create mode 100644 drivers/regulator/sun20i-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index cbe0f96ca342..20a22f900bb2 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1234,6 +1234,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
+	depends on MFD_SYSCON && NVMEM
+	default ARCH_SUNXI
+	help
+	  This driver supports the internal LDOs in the Allwinner D1 SoC.
+
 config REGULATOR_SY7636A
 	tristate "Silergy SY7636A voltage regulator"
 	help
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 8d3ee8b6d41d..cb3ac9290fc3 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -145,6 +145,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..789a68829630
--- /dev/null
+++ b/drivers/regulator/sun20i-regulator.c
@@ -0,0 +1,232 @@
+// 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/nvmem-consumer.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+
+#define SUN20I_POWER_REG		0x348
+
+#define SUN20I_SYS_LDO_CTRL_REG		0x150
+
+struct sun20i_regulator_data {
+	int				(*init)(struct device *dev,
+						struct regmap *regmap);
+	const struct regulator_desc	*descs;
+	unsigned int			ndescs;
+};
+
+static int sun20i_d1_analog_ldos_init(struct device *dev, struct regmap *regmap)
+{
+	u8 bg_trim;
+	int ret;
+
+	ret = nvmem_cell_read_u8(dev, "bg_trim", &bg_trim);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to get bg_trim value\n");
+
+	/* The default value corresponds to 900 mV. */
+	if (!bg_trim)
+		bg_trim = 0x19;
+
+	return regmap_update_bits(regmap, SUN20I_POWER_REG,
+				  GENMASK(7, 0), bg_trim);
+}
+
+static const struct regulator_ops sun20i_d1_analog_ldo_ops = {
+	.list_voltage		= regulator_list_voltage_linear,
+	.map_voltage		= regulator_map_voltage_linear,
+	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
+	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
+	.enable			= regulator_enable_regmap,
+	.disable		= regulator_disable_regmap,
+	.is_enabled		= regulator_is_enabled_regmap,
+};
+
+static const struct regulator_desc sun20i_d1_analog_ldo_descs[] = {
+	{
+		.name		= "aldo",
+		.supply_name	= "vdd33",
+		.of_match	= "aldo",
+		.ops		= &sun20i_d1_analog_ldo_ops,
+		.type		= REGULATOR_VOLTAGE,
+		.owner		= THIS_MODULE,
+		.n_voltages	= 8,
+		.min_uV		= 1650000,
+		.uV_step	= 50000,
+		.vsel_reg	= SUN20I_POWER_REG,
+		.vsel_mask	= GENMASK(14, 12),
+		.enable_reg	= SUN20I_POWER_REG,
+		.enable_mask	= BIT(31),
+	},
+	{
+		.name		= "hpldo",
+		.supply_name	= "hpldoin",
+		.of_match	= "hpldo",
+		.ops		= &sun20i_d1_analog_ldo_ops,
+		.type		= REGULATOR_VOLTAGE,
+		.owner		= THIS_MODULE,
+		.n_voltages	= 8,
+		.min_uV		= 1650000,
+		.uV_step	= 50000,
+		.vsel_reg	= SUN20I_POWER_REG,
+		.vsel_mask	= GENMASK(10, 8),
+		.enable_reg	= SUN20I_POWER_REG,
+		.enable_mask	= BIT(30),
+	},
+};
+
+static const struct sun20i_regulator_data sun20i_d1_analog_ldos = {
+	.init	= sun20i_d1_analog_ldos_init,
+	.descs	= sun20i_d1_analog_ldo_descs,
+	.ndescs	= ARRAY_SIZE(sun20i_d1_analog_ldo_descs),
+};
+
+/* 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 const struct of_device_id sun20i_regulator_of_match[] = {
+	{
+		.compatible = "allwinner,sun20i-d1-analog-ldos",
+		.data = &sun20i_d1_analog_ldos,
+	},
+	{
+		.compatible = "allwinner,sun20i-d1-system-ldos",
+		.data = &sun20i_d1_system_ldos,
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, sun20i_regulator_of_match);
+
+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 ERR_PTR(-EPROBE_DEFER);
+
+	return regmap;
+}
+
+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;
+	int ret;
+
+	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");
+
+	if (data->init) {
+		ret = data->init(dev, regmap);
+		if (ret)
+			return ret;
+	}
+
+	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 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.35.1


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

Re: [PATCH v2 3/4] regulator: dt-bindings: Add Allwinner D1 LDOs

[Rob Herring]

On Tue, 02 Aug 2022 00:32:12 -0500, Samuel Holland wrote:
> The Allwinner D1 SoC contains two pairs of in-package LDOs. One pair is
> for general purpose use. LDOA generally powers the board's 1.8 V rail.
> LDOB generally powers the in-package DRAM, where applicable.
> 
> The other pair of LDOs powers the analog power domains inside the SoC,
> including the audio codec, thermal sensor, and ADCs. These LDOs require
> a 0.9 V bandgap voltage reference. The calibration value for the voltage
> reference is stored in an eFuse, accessed via an NVMEM cell.
> 
> Neither LDO control register is in its own MMIO range; instead, each
> regulator device relies on a regmap/syscon exported by its parent.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> 
> Changes in v2:
>  - Remove syscon property from bindings
>  - Update binding examples to fix warnings and provide context
> 
>  .../allwinner,sun20i-d1-analog-ldos.yaml      | 65 +++++++++++++++++++
>  .../allwinner,sun20i-d1-system-ldos.yaml      | 57 ++++++++++++++++
>  2 files changed, 122 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog-ldos.yaml
>  create mode 100644 Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.example.dtb:0:0: /example-0/syscon@3000000: failed to match any schema with compatible: ['allwinner,sun20i-d1-system-control']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


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

Re: [PATCH v2 3/4] regulator: dt-bindings: Add Allwinner D1 LDOs

[Rob Herring]

On Tue, Aug 02, 2022 at 12:32:12AM -0500, Samuel Holland wrote:
> The Allwinner D1 SoC contains two pairs of in-package LDOs. One pair is
> for general purpose use. LDOA generally powers the board's 1.8 V rail.
> LDOB generally powers the in-package DRAM, where applicable.
> 
> The other pair of LDOs powers the analog power domains inside the SoC,
> including the audio codec, thermal sensor, and ADCs. These LDOs require
> a 0.9 V bandgap voltage reference. The calibration value for the voltage
> reference is stored in an eFuse, accessed via an NVMEM cell.
> 
> Neither LDO control register is in its own MMIO range; instead, each
> regulator device relies on a regmap/syscon exported by its parent.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> 
> Changes in v2:
>  - Remove syscon property from bindings
>  - Update binding examples to fix warnings and provide context
> 
>  .../allwinner,sun20i-d1-analog-ldos.yaml      | 65 +++++++++++++++++++
>  .../allwinner,sun20i-d1-system-ldos.yaml      | 57 ++++++++++++++++
>  2 files changed, 122 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog-ldos.yaml
>  create mode 100644 Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.yaml
> 
> diff --git a/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog-ldos.yaml b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog-ldos.yaml
> new file mode 100644
> index 000000000000..19c984ef4e53
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog-ldos.yaml
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/allwinner,sun20i-d1-analog-ldos.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Allwinner D1 Analog LDOs
> +
> +description:
> +  Allwinner D1 contains a set of LDOs which are designed to supply analog power
> +  inside and outside the SoC. They are controlled by a register within the audio
> +  codec MMIO space, but which is not part of the audio codec clock/reset domain.
> +
> +maintainers:
> +  - Samuel Holland <samuel@sholland.org>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - allwinner,sun20i-d1-analog-ldos
> +
> +  nvmem-cells:
> +    items:
> +      - description: NVMEM cell for the calibrated bandgap reference trim value
> +
> +  nvmem-cell-names:
> +    items:
> +      - const: bg_trim
> +
> +patternProperties:
> +  "^(aldo|hpldo)$":

'^(a|hp)ldo$'

> +    type: object
> +    $ref: regulator.yaml#

       unevaluatedProperties: false

> +
> +required:
> +  - compatible
> +  - nvmem-cells
> +  - nvmem-cell-names
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    audio-codec@2030000 {
> +        compatible = "simple-mfd", "syscon";

Needs a specific compatible. Obviously there's some other functionality 
here if this is an 'audio-codec'.

'simple-mfd' also means the child nodes have zero dependence on the 
parent node and its resources.

> +        reg = <0x2030000 0x1000>;
> +
> +        regulators {
> +            compatible = "allwinner,sun20i-d1-analog-ldos";

Is there a defined set of registers for these regulators? If so, put 
them into 'reg'.

> +            nvmem-cells = <&bg_trim>;
> +            nvmem-cell-names = "bg_trim";
> +
> +            reg_aldo: aldo {
> +                regulator-min-microvolt = <1800000>;
> +                regulator-max-microvolt = <1800000>;
> +            };
> +
> +            reg_hpldo: hpldo {
> +                regulator-min-microvolt = <1800000>;
> +                regulator-max-microvolt = <1800000>;
> +            };
> +        };
> +    };
> +
> +...
> 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..c95030a827d6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.yaml
> @@ -0,0 +1,57 @@
> +# 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
> +
> +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.
> +
> +maintainers:
> +  - Samuel Holland <samuel@sholland.org>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - allwinner,sun20i-d1-system-ldos
> +
> +patternProperties:
> +  "^(ldoa|ldob)$":

'^ldo[ab]$'

Any reason the naming is not consistent with 'ldo' as the prefix or 
suffix?

> +    type: object
> +    $ref: regulator.yaml#

       unevaluatedProperties: false

> +
> +required:
> +  - compatible
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    syscon@3000000 {
> +        compatible = "allwinner,sun20i-d1-system-control";
> +        reg = <0x3000000 0x1000>;
> +        ranges;
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        regulators {
> +            compatible = "allwinner,sun20i-d1-system-ldos";
> +
> +            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.35.1
> 
> 

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

Re: [PATCH v2 1/4] dt-bindings: sram: sunxi-sram: Add optional regulators child

[Rob Herring]

On Tue, Aug 02, 2022 at 12:32:10AM -0500, 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 these regulators
> in addition to SRAM child nodes.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> 
> Changes in v2:
>  - New patch for v2
> 
>  .../bindings/sram/allwinner,sun4i-a10-system-control.yaml      | 3 +++
>  1 file changed, 3 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 1c426c211e36..cc57836b2906 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
>  
> +  regulators:
> +    type: object

This should reference the regulator schema.

Also, it's preferred to have 1 complete example here rather than 
piecemeal examples in each child schema.

> +
>  patternProperties:
>    "^sram@[a-z0-9]+":
>      type: object
> -- 
> 2.35.1
> 
> 

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

Re: [PATCH v2 3/4] regulator: dt-bindings: Add Allwinner D1 LDOs

[Samuel Holland]

Hi Rob,

Thanks again for your review.

On 8/2/22 10:04 AM, Rob Herring wrote:
> On Tue, Aug 02, 2022 at 12:32:12AM -0500, Samuel Holland wrote:
>> The Allwinner D1 SoC contains two pairs of in-package LDOs. One pair is
>> for general purpose use. LDOA generally powers the board's 1.8 V rail.
>> LDOB generally powers the in-package DRAM, where applicable.
>>
>> The other pair of LDOs powers the analog power domains inside the SoC,
>> including the audio codec, thermal sensor, and ADCs. These LDOs require
>> a 0.9 V bandgap voltage reference. The calibration value for the voltage
>> reference is stored in an eFuse, accessed via an NVMEM cell.
>>
>> Neither LDO control register is in its own MMIO range; instead, each
>> regulator device relies on a regmap/syscon exported by its parent.
>>
>> Signed-off-by: Samuel Holland <samuel@sholland.org>
>> ---
>>
>> Changes in v2:
>>  - Remove syscon property from bindings
>>  - Update binding examples to fix warnings and provide context
>>
>>  .../allwinner,sun20i-d1-analog-ldos.yaml      | 65 +++++++++++++++++++
>>  .../allwinner,sun20i-d1-system-ldos.yaml      | 57 ++++++++++++++++
>>  2 files changed, 122 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog-ldos.yaml
>>  create mode 100644 Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog-ldos.yaml b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog-ldos.yaml
>> new file mode 100644
>> index 000000000000..19c984ef4e53
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog-ldos.yaml
>> @@ -0,0 +1,65 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/regulator/allwinner,sun20i-d1-analog-ldos.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Allwinner D1 Analog LDOs
>> +
>> +description:
>> +  Allwinner D1 contains a set of LDOs which are designed to supply analog power
>> +  inside and outside the SoC. They are controlled by a register within the audio
>> +  codec MMIO space, but which is not part of the audio codec clock/reset domain.
>> +
>> +maintainers:
>> +  - Samuel Holland <samuel@sholland.org>
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - allwinner,sun20i-d1-analog-ldos
>> +
>> +  nvmem-cells:
>> +    items:
>> +      - description: NVMEM cell for the calibrated bandgap reference trim value
>> +
>> +  nvmem-cell-names:
>> +    items:
>> +      - const: bg_trim
>> +
>> +patternProperties:
>> +  "^(aldo|hpldo)$":
> 
> '^(a|hp)ldo$'
> 
>> +    type: object
>> +    $ref: regulator.yaml#
> 
>        unevaluatedProperties: false
> 
>> +
>> +required:
>> +  - compatible
>> +  - nvmem-cells
>> +  - nvmem-cell-names
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    audio-codec@2030000 {
>> +        compatible = "simple-mfd", "syscon";
> 
> Needs a specific compatible. Obviously there's some other functionality 
> here if this is an 'audio-codec'.

I am not yet ready to submit the binding or driver for the audio codec, as I
still need to work out the details for jack detection (and some other issues).
If I added the specific audio codec compatible now, without the properties
required for the sound driver, that would create backward compatibility issues,
right?

My intention is to add the specific compatible along with the ASoC support.

> 'simple-mfd' also means the child nodes have zero dependence on the 
> parent node and its resources.

That is correct. The regulators have zero dependencies on the audio codec
resources (clocks/resets/etc.). The _only_ relationship is the overlapping
address of the MMIO register.

>> +        reg = <0x2030000 0x1000>;
>> +
>> +        regulators {
>> +            compatible = "allwinner,sun20i-d1-analog-ldos";
> 
> Is there a defined set of registers for these regulators? If so, put 
> them into 'reg'.

What do you suggest for 'ranges' in the parent device? I ask because the
SRAM/system controller binding requires an empty 'ranges' property.

With empty 'ranges', the child has to compute the relative address for use with
the parent's regmap, but maybe that is okay?

>> +            nvmem-cells = <&bg_trim>;
>> +            nvmem-cell-names = "bg_trim";
>> +
>> +            reg_aldo: aldo {
>> +                regulator-min-microvolt = <1800000>;
>> +                regulator-max-microvolt = <1800000>;
>> +            };
>> +
>> +            reg_hpldo: hpldo {
>> +                regulator-min-microvolt = <1800000>;
>> +                regulator-max-microvolt = <1800000>;
>> +            };
>> +        };
>> +    };
>> +
>> +...
>> 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..c95030a827d6
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.yaml
>> @@ -0,0 +1,57 @@
>> +# 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
>> +
>> +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.
>> +
>> +maintainers:
>> +  - Samuel Holland <samuel@sholland.org>
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - allwinner,sun20i-d1-system-ldos
>> +
>> +patternProperties:
>> +  "^(ldoa|ldob)$":
> 
> '^ldo[ab]$'
> 
> Any reason the naming is not consistent with 'ldo' as the prefix or 
> suffix?

All four names match the pin names from the SoC datasheet.

>> +    type: object
>> +    $ref: regulator.yaml#
> 
>        unevaluatedProperties: false

I will fix these for v3.

Regards,
Samuel

>> +
>> +required:
>> +  - compatible
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    syscon@3000000 {
>> +        compatible = "allwinner,sun20i-d1-system-control";
>> +        reg = <0x3000000 0x1000>;
>> +        ranges;
>> +        #address-cells = <1>;
>> +        #size-cells = <1>;
>> +
>> +        regulators {
>> +            compatible = "allwinner,sun20i-d1-system-ldos";
>> +
>> +            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.35.1
>>
>>


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

Re: [PATCH v2 3/4] regulator: dt-bindings: Add Allwinner D1 LDOs

[Jernej Škrabec]

Dne četrtek, 04. avgust 2022 ob 05:03:07 CEST je Samuel Holland napisal(a):
> Hi Rob,
> 
> Thanks again for your review.
> 
> On 8/2/22 10:04 AM, Rob Herring wrote:
> > On Tue, Aug 02, 2022 at 12:32:12AM -0500, Samuel Holland wrote:
> >> The Allwinner D1 SoC contains two pairs of in-package LDOs. One pair is
> >> for general purpose use. LDOA generally powers the board's 1.8 V rail.
> >> LDOB generally powers the in-package DRAM, where applicable.
> >> 
> >> The other pair of LDOs powers the analog power domains inside the SoC,
> >> including the audio codec, thermal sensor, and ADCs. These LDOs require
> >> a 0.9 V bandgap voltage reference. The calibration value for the voltage
> >> reference is stored in an eFuse, accessed via an NVMEM cell.
> >> 
> >> Neither LDO control register is in its own MMIO range; instead, each
> >> regulator device relies on a regmap/syscon exported by its parent.
> >> 
> >> Signed-off-by: Samuel Holland <samuel@sholland.org>
> >> ---
> >> 
> >> Changes in v2:
> >>  - Remove syscon property from bindings
> >>  - Update binding examples to fix warnings and provide context
> >>  
> >>  .../allwinner,sun20i-d1-analog-ldos.yaml      | 65 +++++++++++++++++++
> >>  .../allwinner,sun20i-d1-system-ldos.yaml      | 57 ++++++++++++++++
> >>  2 files changed, 122 insertions(+)
> >>  create mode 100644
> >>  Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog-
> >>  ldos.yaml create mode 100644
> >>  Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-
> >>  ldos.yaml>> 
> >> diff --git
> >> a/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog
> >> -ldos.yaml
> >> b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog
> >> -ldos.yaml new file mode 100644
> >> index 000000000000..19c984ef4e53
> >> --- /dev/null
> >> +++
> >> b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog
> >> -ldos.yaml @@ -0,0 +1,65 @@
> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id:
> >> http://devicetree.org/schemas/regulator/allwinner,sun20i-d1-analog-ldos.
> >> yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Allwinner D1 Analog LDOs
> >> +
> >> +description:
> >> +  Allwinner D1 contains a set of LDOs which are designed to supply
> >> analog power +  inside and outside the SoC. They are controlled by a
> >> register within the audio +  codec MMIO space, but which is not part of
> >> the audio codec clock/reset domain. +
> >> +maintainers:
> >> +  - Samuel Holland <samuel@sholland.org>
> >> +
> >> +properties:
> >> +  compatible:
> >> +    enum:
> >> +      - allwinner,sun20i-d1-analog-ldos
> >> +
> >> +  nvmem-cells:
> >> +    items:
> >> +      - description: NVMEM cell for the calibrated bandgap reference
> >> trim value +
> >> +  nvmem-cell-names:
> >> +    items:
> >> +      - const: bg_trim
> >> +
> >> +patternProperties:
> > 
> >> +  "^(aldo|hpldo)$":
> > '^(a|hp)ldo$'
> > 
> >> +    type: object
> >> +    $ref: regulator.yaml#
> >> 
> >        unevaluatedProperties: false
> >> 
> >> +
> >> +required:
> >> +  - compatible
> >> +  - nvmem-cells
> >> +  - nvmem-cell-names
> >> +
> >> +unevaluatedProperties: false
> >> +
> >> +examples:
> >> +  - |
> >> +    audio-codec@2030000 {
> >> +        compatible = "simple-mfd", "syscon";
> > 
> > Needs a specific compatible. Obviously there's some other functionality
> > here if this is an 'audio-codec'.
> 
> I am not yet ready to submit the binding or driver for the audio codec, as I
> still need to work out the details for jack detection (and some other
> issues). If I added the specific audio codec compatible now, without the
> properties required for the sound driver, that would create backward
> compatibility issues, right?
> 
> My intention is to add the specific compatible along with the ASoC support.
> 
> > 'simple-mfd' also means the child nodes have zero dependence on the
> > parent node and its resources.
> 
> That is correct. The regulators have zero dependencies on the audio codec
> resources (clocks/resets/etc.). The _only_ relationship is the overlapping
> address of the MMIO register.

LDO registers are at the end of analogue codec MMIO space, right? We can 
easily split that to separate device, like it's been done for A20 timer & 
watchdog.

Best regards,
Jernej

> 
> >> +        reg = <0x2030000 0x1000>;
> >> +
> >> +        regulators {
> >> +            compatible = "allwinner,sun20i-d1-analog-ldos";
> > 
> > Is there a defined set of registers for these regulators? If so, put
> > them into 'reg'.
> 
> What do you suggest for 'ranges' in the parent device? I ask because the
> SRAM/system controller binding requires an empty 'ranges' property.
> 
> With empty 'ranges', the child has to compute the relative address for use
> with the parent's regmap, but maybe that is okay?
> 
> >> +            nvmem-cells = <&bg_trim>;
> >> +            nvmem-cell-names = "bg_trim";
> >> +
> >> +            reg_aldo: aldo {
> >> +                regulator-min-microvolt = <1800000>;
> >> +                regulator-max-microvolt = <1800000>;
> >> +            };
> >> +
> >> +            reg_hpldo: hpldo {
> >> +                regulator-min-microvolt = <1800000>;
> >> +                regulator-max-microvolt = <1800000>;
> >> +            };
> >> +        };
> >> +    };
> >> +
> >> +...
> >> 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..c95030a827d6
> >> --- /dev/null
> >> +++
> >> b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system
> >> -ldos.yaml @@ -0,0 +1,57 @@
> >> +# 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
> >> +
> >> +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.
> >> +
> >> +maintainers:
> >> +  - Samuel Holland <samuel@sholland.org>
> >> +
> >> +properties:
> >> +  compatible:
> >> +    enum:
> >> +      - allwinner,sun20i-d1-system-ldos
> >> +
> >> +patternProperties:
> > 
> >> +  "^(ldoa|ldob)$":
> > '^ldo[ab]$'
> > 
> > Any reason the naming is not consistent with 'ldo' as the prefix or
> > suffix?
> 
> All four names match the pin names from the SoC datasheet.
> 
> >> +    type: object
> >> +    $ref: regulator.yaml#
> >> 
> >        unevaluatedProperties: false
> 
> I will fix these for v3.
> 
> Regards,
> Samuel
> 
> >> +
> >> +required:
> >> +  - compatible
> >> +
> >> +unevaluatedProperties: false
> >> +
> >> +examples:
> >> +  - |
> >> +    syscon@3000000 {
> >> +        compatible = "allwinner,sun20i-d1-system-control";
> >> +        reg = <0x3000000 0x1000>;
> >> +        ranges;
> >> +        #address-cells = <1>;
> >> +        #size-cells = <1>;
> >> +
> >> +        regulators {
> >> +            compatible = "allwinner,sun20i-d1-system-ldos";
> >> +
> >> +            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>;
> >> +            };
> >> +        };
> >> +    };
> >> +
> >> +...





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

Re: [PATCH v2 3/4] regulator: dt-bindings: Add Allwinner D1 LDOs

[Samuel Holland]

On August 4, 2022 5:11:39 AM UTC, "Jernej Škrabec" <jernej.skrabec@gmail.com> wrote:
>> > 'simple-mfd' also means the child nodes have zero dependence on the
>> > parent node and its resources.
>> 
>> That is correct. The regulators have zero dependencies on the audio codec
>> resources (clocks/resets/etc.). The _only_ relationship is the overlapping
>> address of the MMIO register.
>
>LDO registers are at the end of analogue codec MMIO space, right? We can 
>easily split that to separate device, like it's been done for A20 timer & 
>watchdog.

No, there are some audio-related registers after the LDO register ("POWER_REG" in the D1 and V853 manuals).
So it is not possible to cleanly split the MMIO region.

Regards,
Samuel

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

Re: [PATCH v2 3/4] regulator: dt-bindings: Add Allwinner D1 LDOs

[Rob Herring]

On Wed, Aug 03, 2022 at 10:03:07PM -0500, Samuel Holland wrote:
> Hi Rob,
> 
> Thanks again for your review.
> 
> On 8/2/22 10:04 AM, Rob Herring wrote:
> > On Tue, Aug 02, 2022 at 12:32:12AM -0500, Samuel Holland wrote:
> >> The Allwinner D1 SoC contains two pairs of in-package LDOs. One pair is
> >> for general purpose use. LDOA generally powers the board's 1.8 V rail.
> >> LDOB generally powers the in-package DRAM, where applicable.
> >>
> >> The other pair of LDOs powers the analog power domains inside the SoC,
> >> including the audio codec, thermal sensor, and ADCs. These LDOs require
> >> a 0.9 V bandgap voltage reference. The calibration value for the voltage
> >> reference is stored in an eFuse, accessed via an NVMEM cell.
> >>
> >> Neither LDO control register is in its own MMIO range; instead, each
> >> regulator device relies on a regmap/syscon exported by its parent.
> >>
> >> Signed-off-by: Samuel Holland <samuel@sholland.org>
> >> ---
> >>
> >> Changes in v2:
> >>  - Remove syscon property from bindings
> >>  - Update binding examples to fix warnings and provide context
> >>
> >>  .../allwinner,sun20i-d1-analog-ldos.yaml      | 65 +++++++++++++++++++
> >>  .../allwinner,sun20i-d1-system-ldos.yaml      | 57 ++++++++++++++++
> >>  2 files changed, 122 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog-ldos.yaml
> >>  create mode 100644 Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog-ldos.yaml b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog-ldos.yaml
> >> new file mode 100644
> >> index 000000000000..19c984ef4e53
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-analog-ldos.yaml
> >> @@ -0,0 +1,65 @@
> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/regulator/allwinner,sun20i-d1-analog-ldos.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Allwinner D1 Analog LDOs
> >> +
> >> +description:
> >> +  Allwinner D1 contains a set of LDOs which are designed to supply analog power
> >> +  inside and outside the SoC. They are controlled by a register within the audio
> >> +  codec MMIO space, but which is not part of the audio codec clock/reset domain.
> >> +
> >> +maintainers:
> >> +  - Samuel Holland <samuel@sholland.org>
> >> +
> >> +properties:
> >> +  compatible:
> >> +    enum:
> >> +      - allwinner,sun20i-d1-analog-ldos
> >> +
> >> +  nvmem-cells:
> >> +    items:
> >> +      - description: NVMEM cell for the calibrated bandgap reference trim value
> >> +
> >> +  nvmem-cell-names:
> >> +    items:
> >> +      - const: bg_trim
> >> +
> >> +patternProperties:
> >> +  "^(aldo|hpldo)$":
> > 
> > '^(a|hp)ldo$'
> > 
> >> +    type: object
> >> +    $ref: regulator.yaml#
> > 
> >        unevaluatedProperties: false
> > 
> >> +
> >> +required:
> >> +  - compatible
> >> +  - nvmem-cells
> >> +  - nvmem-cell-names
> >> +
> >> +unevaluatedProperties: false
> >> +
> >> +examples:
> >> +  - |
> >> +    audio-codec@2030000 {
> >> +        compatible = "simple-mfd", "syscon";
> > 
> > Needs a specific compatible. Obviously there's some other functionality 
> > here if this is an 'audio-codec'.
> 
> I am not yet ready to submit the binding or driver for the audio codec, as I
> still need to work out the details for jack detection (and some other issues).
> If I added the specific audio codec compatible now, without the properties
> required for the sound driver, that would create backward compatibility issues,
> right?

Only if something used it.

> My intention is to add the specific compatible along with the ASoC support.
> 
> > 'simple-mfd' also means the child nodes have zero dependence on the 
> > parent node and its resources.
> 
> That is correct. The regulators have zero dependencies on the audio codec
> resources (clocks/resets/etc.). The _only_ relationship is the overlapping
> address of the MMIO register.

Okay.

> 
> >> +        reg = <0x2030000 0x1000>;
> >> +
> >> +        regulators {
> >> +            compatible = "allwinner,sun20i-d1-analog-ldos";
> > 
> > Is there a defined set of registers for these regulators? If so, put 
> > them into 'reg'.
> 
> What do you suggest for 'ranges' in the parent device? I ask because the
> SRAM/system controller binding requires an empty 'ranges' property.

I think you are misreading 'ranges: true'. That just means the property 
can be present, not that it's type is boolean.


> With empty 'ranges', the child has to compute the relative address for use with
> the parent's regmap, but maybe that is okay?

We should probably have some sort of 'address to regmap offset' helper 
that works no matter what ranges has.

In most cases, while I require 'reg', unless the child nodes use a mixed 
up set of registers, Linux doesn't currently use 'reg'. You can also 
just hardcode the offsets in your driver.

> >> +            nvmem-cells = <&bg_trim>;
> >> +            nvmem-cell-names = "bg_trim";
> >> +
> >> +            reg_aldo: aldo {
> >> +                regulator-min-microvolt = <1800000>;
> >> +                regulator-max-microvolt = <1800000>;
> >> +            };
> >> +
> >> +            reg_hpldo: hpldo {
> >> +                regulator-min-microvolt = <1800000>;
> >> +                regulator-max-microvolt = <1800000>;
> >> +            };
> >> +        };
> >> +    };
> >> +
> >> +...
> >> 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..c95030a827d6
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/regulator/allwinner,sun20i-d1-system-ldos.yaml
> >> @@ -0,0 +1,57 @@
> >> +# 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
> >> +
> >> +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.
> >> +
> >> +maintainers:
> >> +  - Samuel Holland <samuel@sholland.org>
> >> +
> >> +properties:
> >> +  compatible:
> >> +    enum:
> >> +      - allwinner,sun20i-d1-system-ldos
> >> +
> >> +patternProperties:
> >> +  "^(ldoa|ldob)$":
> > 
> > '^ldo[ab]$'
> > 
> > Any reason the naming is not consistent with 'ldo' as the prefix or 
> > suffix?
> 
> All four names match the pin names from the SoC datasheet.

Good enough for me.

Rob

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