[PATCH V8 0/7] Add Qualcomm Technologies, Inc. PM8008 regulator driver

[PATCH V8 0/7] Add Qualcomm Technologies, Inc. PM8008 regulator driver

[Satya Priya]

Satya Priya (7):
  dt-bindings: mfd: pm8008: Modify the compatible as per driver
  dt-bindings: mfd: pm8008: Add pm8008 regulator bindings
  mfd: pm8008: Add mfd cell struct to register LDOs
  regulator: Add a regulator driver for the PM8008 PMIC
  arm64: dts: qcom: pm8008-infra: Add base dts file for pm8008 infra
  arm64: dts: qcom: pm8008-regulators: Add base dts file for pm8008
    regulators
  arm64: dts: qcom: sc7280: Add pm8008 support for sc7280-idp

 .../bindings/mfd/qcom,pm8008-regulators.yaml       |  87 +++++++++
 .../devicetree/bindings/mfd/qcom,pm8008.yaml       |  15 +-
 arch/arm64/boot/dts/qcom/pm8008-infra.dtsi         |   9 +
 arch/arm64/boot/dts/qcom/pm8008-regulators.dtsi    |  37 ++++
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi           |  70 +++++++
 drivers/mfd/qcom-pm8008.c                          |  94 +++++++++-
 drivers/regulator/Kconfig                          |   9 +
 drivers/regulator/Makefile                         |   1 +
 drivers/regulator/qcom-pm8008-regulator.c          | 205 +++++++++++++++++++++
 9 files changed, 513 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/qcom,pm8008-regulators.yaml
 create mode 100644 arch/arm64/boot/dts/qcom/pm8008-infra.dtsi
 create mode 100644 arch/arm64/boot/dts/qcom/pm8008-regulators.dtsi
 create mode 100644 drivers/regulator/qcom-pm8008-regulator.c

-- 
2.7.4

[PATCH V8 1/7] dt-bindings: mfd: pm8008: Modify the compatible as per driver

[Satya Priya]

Modify the compatible string as per the pm8008 mfd driver.
Add reset-gpios property and make interrupts and interrupt-cells
as optional properties, they are not strictly required and may
cause yaml compilation errors when not added in the DT files.

Also, change the node name in example to match with the
pm8008_infra DT node.

Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
---
Changes in V5:
 - Remove compatible for regulators node.
 - Move supply nodes of the regulators to chip level.

Changes in V6:
 - No changes.

Changes in V7:
 - Removed the intermediate regulators node and added ldos
   directly under mfd node.

Changes in V8:
 - Change the compatible as per driver, remove interrupts from required
   properties, add reset-gpios and move regulators to separate binding.

 Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
index ec3138c..12431ea 100644
--- a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
+++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
@@ -16,7 +16,7 @@ description: |
 
 properties:
   compatible:
-    const: qcom,pm8008
+    const: qcom,pm8008-infra
 
   reg:
     description:
@@ -44,6 +44,10 @@ properties:
   "#size-cells":
     const: 0
 
+  reset-gpios:
+    description: |
+      Specifies the GPIO to be toggled to bring pm8008 chip out of reset.
+
 patternProperties:
   "^gpio@[0-9a-f]+$":
     type: object
@@ -88,10 +92,8 @@ patternProperties:
 required:
   - compatible
   - reg
-  - interrupts
   - "#address-cells"
   - "#size-cells"
-  - "#interrupt-cells"
 
 additionalProperties: false
 
@@ -99,11 +101,12 @@ examples:
   - |
     #include <dt-bindings/mfd/qcom-pm8008.h>
     #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/gpio/gpio.h>
     qupv3_se13_i2c {
       #address-cells = <1>;
       #size-cells = <0>;
-      pm8008i@8 {
-        compatible = "qcom,pm8008";
+      pm8008_infra: pmic@8 {
+        compatible = "qcom,pm8008-infra";
         reg = <0x8>;
         #address-cells = <1>;
         #size-cells = <0>;
@@ -113,6 +116,8 @@ examples:
         interrupt-parent = <&tlmm>;
         interrupts = <32 IRQ_TYPE_EDGE_RISING>;
 
+        reset-gpios = <&pm8350c_gpios 4 GPIO_ACTIVE_HIGH>;
+
         pm8008_gpios: gpio@c000 {
           compatible = "qcom,pm8008-gpio", "qcom,spmi-gpio";
           reg = <0xc000>;
-- 
2.7.4

[PATCH V8 2/7] dt-bindings: mfd: pm8008: Add pm8008 regulator bindings

[Satya Priya]

Add bindings for pm8008 regulators device.

Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
---
Changes in V8:
 - This is split from pm8008.yaml binding.

 .../bindings/mfd/qcom,pm8008-regulators.yaml       | 87 ++++++++++++++++++++++
 1 file changed, 87 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/qcom,pm8008-regulators.yaml

diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008-regulators.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008-regulators.yaml
new file mode 100644
index 0000000..139fbf9
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008-regulators.yaml
@@ -0,0 +1,87 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/qcom,pm8008-regulators.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Technologies, Inc. PM8008 Regulator bindings
+
+maintainers:
+  - Satya Priya <quic_c_skakit@quicinc.com>
+
+description: |
+  Qualcomm Technologies, Inc. PM8008 is an I2C controlled PMIC
+  containing 7 LDO regulators. This binding specifies the PM8008
+  regulator peripherals of PM8008 device.
+
+properties:
+  compatible:
+    const: qcom,pm8008-regulators
+
+  reg:
+    description:
+      I2C slave address.
+
+    maxItems: 1
+
+  "#address-cells":
+    const: 0
+
+  "#size-cells":
+    const: 0
+
+  vdd_l1_l2-supply:
+    description: Input supply phandle of ldo1 and ldo2 regulators.
+
+  vdd_l3_l4-supply:
+    description: Input supply phandle of ldo3 and ldo4 regulators.
+
+  vdd_l5-supply:
+    description: Input supply phandle of ldo5 regulator.
+
+  vdd_l6-supply:
+    description: Input supply phandle of ldo6 regulator.
+
+  vdd_l7-supply:
+    description: Input supply phandle of ldo7 regulator.
+
+patternProperties:
+  "^ldo[1-7]$":
+    type: object
+    $ref: "../regulator/regulator.yaml#"
+    description: PM8008 regulator peripherals of PM8008 regulator device
+
+required:
+  - compatible
+  - "#address-cells"
+  - "#size-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/mfd/qcom-pm8008.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    qupv3_se13_i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      pm8008_regulators: pmic@9 {
+        compatible = "qcom,pm8008-regulators";
+        reg = <0x9>;
+        #address-cells = <0>;
+        #size-cells = <0>;
+
+        vdd_l1_l2-supply = <&vreg_s8b_1p2>;
+        vdd_l3_l4-supply = <&vreg_s1b_1p8>;
+        vdd_l5-supply = <&vreg_bob>;
+        vdd_l6-supply = <&vreg_bob>;
+        vdd_l7-supply = <&vreg_bob>;
+
+        pm8008_l1: ldo1 {
+          regulator-name = "pm8008_l1";
+          regulator-min-microvolt = <950000>;
+          regulator-max-microvolt = <1300000>;
+        };
+      };
+    };
+...
-- 
2.7.4

[PATCH V8 3/7] mfd: pm8008: Add mfd cell struct to register LDOs

[Satya Priya]

Add mfd cell struct with regulator driver name to match
with the pm8008 regulator driver and probe the LDOs.

Split the probe for infra and regulator devices in order
to avoid using multiple 'if' conditions with data identifiers
to distinguish between infra and regulator mfd devices.
This also ensures that the chip init related stuff is
not repeated by calling single probe twice.

Add the reset-gpio toggling in the infra driver probe to bring
pm8008 chip out of reset instead of doing it in DT node using
"output-high" property.

Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
---
Changes in V5:
 - Changes newly added from V5.

Changes in V6:
 - Changed the mfd_cell struct to have only name of the regulator driver.
 - Using device_get_match_data() instead of of_match_node() to match data.
 - Fixed few nits.

Changes in V7:
 - Fixed minor errors.

Changes in V8:
 - Split the probe for infra and regulator devices
 - Add the reset-gpio toggling in the infra driver probe

 drivers/mfd/qcom-pm8008.c | 94 ++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 85 insertions(+), 9 deletions(-)

diff --git a/drivers/mfd/qcom-pm8008.c b/drivers/mfd/qcom-pm8008.c
index c472d7f..a622f65 100644
--- a/drivers/mfd/qcom-pm8008.c
+++ b/drivers/mfd/qcom-pm8008.c
@@ -4,10 +4,12 @@
  */
 
 #include <linux/bitops.h>
+#include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
+#include <linux/mfd/core.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/of_platform.h>
@@ -27,6 +29,10 @@
 #define INT_EN_CLR_OFFSET		0x16
 #define INT_LATCHED_STS_OFFSET		0x18
 
+static const struct mfd_cell pm8008_regulator_devs[] = {
+	MFD_CELL_NAME("qcom,pm8008-regulators"),
+};
+
 enum {
 	PM8008_MISC,
 	PM8008_TEMP_ALARM,
@@ -57,6 +63,7 @@ enum {
 struct pm8008_data {
 	struct device *dev;
 	struct regmap *regmap;
+	struct gpio_desc *reset_gpio;
 	int irq;
 	struct regmap_irq_chip_data *irq_data;
 };
@@ -217,7 +224,7 @@ static int pm8008_probe_irq_peripherals(struct pm8008_data *chip,
 	return 0;
 }
 
-static int pm8008_probe(struct i2c_client *client)
+static int pm8008_probe_infra(struct i2c_client *client)
 {
 	int rc;
 	struct pm8008_data *chip;
@@ -239,22 +246,91 @@ static int pm8008_probe(struct i2c_client *client)
 			dev_err(chip->dev, "Failed to probe irq periphs: %d\n", rc);
 	}
 
+	chip->reset_gpio = devm_gpiod_get(chip->dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(chip->reset_gpio)) {
+		dev_err(chip->dev, "failed to acquire reset gpio\n");
+		return PTR_ERR(chip->reset_gpio);
+	}
+	gpiod_set_value(chip->reset_gpio, 1);
+
 	return devm_of_platform_populate(chip->dev);
 }
 
-static const struct of_device_id pm8008_match[] = {
-	{ .compatible = "qcom,pm8008", },
-	{ },
+static int pm8008_probe_regulators(struct i2c_client *client)
+{
+
+	int rc;
+	struct pm8008_data *chip;
+
+	chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->dev = &client->dev;
+	chip->regmap = devm_regmap_init_i2c(client, &qcom_mfd_regmap_cfg);
+	if (!chip->regmap)
+		return -ENODEV;
+
+	rc = devm_mfd_add_devices(chip->dev, 0, pm8008_regulator_devs,
+			ARRAY_SIZE(pm8008_regulator_devs), NULL, 0, NULL);
+	if (rc)
+		dev_err(chip->dev, "Failed to add children: %d\n", rc);
+
+	return rc;
+}
+
+static const struct of_device_id pm8008_infra_match[] = {
+	{ .compatible = "qcom,pm8008-infra", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pm8008_infra_match);
+
+static struct i2c_driver pm8008_infra_driver = {
+	.driver = {
+		.name = "pm8008-infra",
+		.of_match_table = pm8008_infra_match,
+	},
+	.probe_new = pm8008_probe_infra,
 };
 
-static struct i2c_driver pm8008_mfd_driver = {
+static const struct of_device_id pm8008_regulators_match[] = {
+	{ .compatible = "qcom,pm8008-regulators", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pm8008_regulators_match);
+
+static struct i2c_driver pm8008_regulators_driver = {
 	.driver = {
-		.name = "pm8008",
-		.of_match_table = pm8008_match,
+		.name = "pm8008-regulators",
+		.of_match_table = pm8008_regulators_match,
 	},
-	.probe_new = pm8008_probe,
+	.probe_new = pm8008_probe_regulators,
 };
-module_i2c_driver(pm8008_mfd_driver);
+
+static int __init pm8008_i2c_init(void)
+{
+	int ret;
+
+	ret = i2c_add_driver(&pm8008_infra_driver);
+	if (ret) {
+		pr_err("Failed to register driver for pm8008_infra: %d\n", ret);
+		return ret;
+	}
+
+	ret = i2c_add_driver(&pm8008_regulators_driver);
+	if (ret)
+		pr_err("Failed to register driver for pm8008_regulators: %d\n", ret);
+
+	return ret;
+}
+module_init(pm8008_i2c_init);
+
+static void __exit pm8008_i2c_exit(void)
+{
+	i2c_del_driver(&pm8008_infra_driver);
+	i2c_del_driver(&pm8008_regulators_driver);
+}
+module_exit(pm8008_i2c_exit);
 
 MODULE_LICENSE("GPL v2");
 MODULE_ALIAS("i2c:qcom-pm8008");
-- 
2.7.4

[PATCH V8 4/7] regulator: Add a regulator driver for the PM8008 PMIC

[Satya Priya]

Qualcomm Technologies, Inc. PM8008 is an I2C controlled PMIC
containing 7 LDO regulators.  Add a PM8008 regulator driver to
support PMIC regulator management via the regulator framework.

Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
---
Changes in V5:
 - Removed Compatible string.
 - Changed the probe function accordingly to probe LDOs using mfd driver.
 - Added max headrooms for LDOs and removed the part reading min-dropout from DT.
 - Added base reg values in the regulator_data struct instead of reading it from DT.

Changes in V6:
 - Using loop to probe the regulators instead of creating separate platform device for
   each regulator.
 - Removed the of_parse_cb API as we are not parsing any DT properties. Moved slewrate
   configuration to probe.
 - Fixed other nits.

Changes in V7:
 - Removed unused Macros and headers.

Changes in V8:
 - Changed the regulators_data struct name to pm8008_regulator_data

 drivers/regulator/Kconfig                 |   9 ++
 drivers/regulator/Makefile                |   1 +
 drivers/regulator/qcom-pm8008-regulator.c | 205 ++++++++++++++++++++++++++++++
 3 files changed, 215 insertions(+)
 create mode 100644 drivers/regulator/qcom-pm8008-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 5d0a360..e2f7af5 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -925,6 +925,15 @@ config REGULATOR_PWM
 	  This driver supports PWM controlled voltage regulators. PWM
 	  duty cycle can increase or decrease the voltage.
 
+config REGULATOR_QCOM_PM8008
+	tristate "Qualcomm Technologies, Inc. PM8008 PMIC regulators"
+	depends on MFD_QCOM_PM8008
+	help
+	  Select this option to get support for the voltage regulators
+	  of Qualcomm Technologies, Inc. PM8008 PMIC chip. PM8008 has 7 LDO
+	  regulators. This driver provides support for basic operations like
+	  set/get voltage and enable/disable.
+
 config REGULATOR_QCOM_RPM
 	tristate "Qualcomm RPM regulator driver"
 	depends on MFD_QCOM_RPM
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 4b8794a..6462fd4 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -101,6 +101,7 @@ obj-$(CONFIG_REGULATOR_MT6380)	+= mt6380-regulator.o
 obj-$(CONFIG_REGULATOR_MT6397)	+= mt6397-regulator.o
 obj-$(CONFIG_REGULATOR_MTK_DVFSRC) += mtk-dvfsrc-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_LABIBB) += qcom-labibb-regulator.o
+obj-$(CONFIG_REGULATOR_QCOM_PM8008) += qcom-pm8008-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_RPM) += qcom_rpm-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_RPMH) += qcom-rpmh-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_SMD_RPM) += qcom_smd-regulator.o
diff --git a/drivers/regulator/qcom-pm8008-regulator.c b/drivers/regulator/qcom-pm8008-regulator.c
new file mode 100644
index 0000000..0f6d5cb
--- /dev/null
+++ b/drivers/regulator/qcom-pm8008-regulator.c
@@ -0,0 +1,205 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2022, The Linux Foundation. All rights reserved. */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+
+#define VSET_STEP_MV			8
+#define VSET_STEP_UV			(VSET_STEP_MV * 1000)
+
+#define LDO_ENABLE_REG(base)		((base) + 0x46)
+#define ENABLE_BIT			BIT(7)
+
+#define LDO_VSET_LB_REG(base)		((base) + 0x40)
+
+#define LDO_STEPPER_CTL_REG(base)	((base) + 0x3b)
+#define DEFAULT_VOLTAGE_STEPPER_RATE	38400
+#define STEP_RATE_MASK			GENMASK(1, 0)
+
+struct pm8008_regulator_data {
+	const char			*name;
+	const char			*supply_name;
+	u16				base;
+	int				min_uv;
+	int				max_uv;
+	int				min_dropout_uv;
+	const struct linear_range	*voltage_range;
+};
+
+struct pm8008_regulator {
+	struct device		*dev;
+	struct regmap		*regmap;
+	struct regulator_desc	rdesc;
+	u16			base;
+	int			step_rate;
+};
+
+static const struct linear_range nldo_ranges[] = {
+	REGULATOR_LINEAR_RANGE(528000, 0, 122, 8000),
+};
+
+static const struct linear_range pldo_ranges[] = {
+	REGULATOR_LINEAR_RANGE(1504000, 0, 237, 8000),
+};
+
+static const struct pm8008_regulator_data reg_data[] = {
+	/* name  parent       base   min_uv  max_uv  headroom_uv voltage_range */
+	{ "ldo1", "vdd_l1_l2", 0x4000,  528000, 1504000, 225000, nldo_ranges, },
+	{ "ldo2", "vdd_l1_l2", 0x4100,  528000, 1504000, 225000, nldo_ranges, },
+	{ "ldo3", "vdd_l3_l4", 0x4200, 1504000, 3400000, 300000, pldo_ranges, },
+	{ "ldo4", "vdd_l3_l4", 0x4300, 1504000, 3400000, 300000, pldo_ranges, },
+	{ "ldo5", "vdd_l5",    0x4400, 1504000, 3400000, 200000, pldo_ranges, },
+	{ "ldo6", "vdd_l6",    0x4500, 1504000, 3400000, 200000, pldo_ranges, },
+	{ "ldo7", "vdd_l7",    0x4600, 1504000, 3400000, 200000, pldo_ranges, },
+};
+
+static int pm8008_regulator_get_voltage(struct regulator_dev *rdev)
+{
+	struct pm8008_regulator *pm8008_reg = rdev_get_drvdata(rdev);
+	__le16 mV;
+	int rc;
+
+	rc = regmap_bulk_read(pm8008_reg->regmap,
+			LDO_VSET_LB_REG(pm8008_reg->base), (void *)&mV, 2);
+	if (rc < 0) {
+		dev_err(&rdev->dev, "failed to read regulator voltage rc=%d\n", rc);
+		return rc;
+	}
+
+	return le16_to_cpu(mV) * 1000;
+}
+
+static inline int pm8008_write_voltage(struct pm8008_regulator *pm8008_reg,
+							int mV)
+{
+	__le16 vset_raw;
+
+	vset_raw = cpu_to_le16(mV);
+
+	return regmap_bulk_write(pm8008_reg->regmap,
+			LDO_VSET_LB_REG(pm8008_reg->base),
+			(const void *)&vset_raw, sizeof(vset_raw));
+}
+
+static int pm8008_regulator_set_voltage_time(struct regulator_dev *rdev,
+				int old_uV, int new_uv)
+{
+	struct pm8008_regulator *pm8008_reg = rdev_get_drvdata(rdev);
+
+	return DIV_ROUND_UP(abs(new_uv - old_uV), pm8008_reg->step_rate);
+}
+
+static int pm8008_regulator_set_voltage(struct regulator_dev *rdev,
+					unsigned int selector)
+{
+	struct pm8008_regulator *pm8008_reg = rdev_get_drvdata(rdev);
+	int rc, mV;
+
+	/* voltage control register is set with voltage in millivolts */
+	mV = DIV_ROUND_UP(regulator_list_voltage_linear_range(rdev, selector),
+						1000);
+	if (mV < 0)
+		return mV;
+
+	rc = pm8008_write_voltage(pm8008_reg, mV);
+	if (rc < 0)
+		return rc;
+
+	dev_dbg(&rdev->dev, "voltage set to %d\n", mV * 1000);
+	return 0;
+}
+
+static const struct regulator_ops pm8008_regulator_ops = {
+	.enable			= regulator_enable_regmap,
+	.disable		= regulator_disable_regmap,
+	.is_enabled		= regulator_is_enabled_regmap,
+	.set_voltage_sel	= pm8008_regulator_set_voltage,
+	.get_voltage		= pm8008_regulator_get_voltage,
+	.list_voltage		= regulator_list_voltage_linear,
+	.set_voltage_time	= pm8008_regulator_set_voltage_time,
+};
+
+static int pm8008_regulator_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct regulator_dev    *rdev;
+	struct pm8008_regulator *pm8008_reg;
+	struct regmap *regmap;
+	struct regulator_config reg_config = {};
+	int rc, i;
+	unsigned int reg;
+
+	regmap = dev_get_regmap(dev->parent, NULL);
+	if (!regmap) {
+		dev_err(dev, "parent regmap is missing\n");
+		return -EINVAL;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(reg_data); i++) {
+		pm8008_reg = devm_kzalloc(dev, sizeof(*pm8008_reg), GFP_KERNEL);
+		if (!pm8008_reg)
+			return -ENOMEM;
+
+		pm8008_reg->regmap = regmap;
+		pm8008_reg->dev = dev;
+		pm8008_reg->base = reg_data[i].base;
+
+		/* get slew rate */
+		rc = regmap_bulk_read(pm8008_reg->regmap,
+				LDO_STEPPER_CTL_REG(pm8008_reg->base), &reg, 1);
+		if (rc < 0) {
+			dev_err(dev, "failed to read step rate configuration rc=%d\n", rc);
+			return rc;
+		}
+		reg &= STEP_RATE_MASK;
+		pm8008_reg->step_rate = DEFAULT_VOLTAGE_STEPPER_RATE >> reg;
+
+		pm8008_reg->rdesc.type = REGULATOR_VOLTAGE;
+		pm8008_reg->rdesc.ops = &pm8008_regulator_ops;
+		pm8008_reg->rdesc.name = reg_data[i].name;
+		pm8008_reg->rdesc.supply_name = reg_data[i].supply_name;
+		pm8008_reg->rdesc.of_match = reg_data[i].name;
+		pm8008_reg->rdesc.uV_step = VSET_STEP_UV;
+		pm8008_reg->rdesc.min_uV = reg_data[i].min_uv;
+		pm8008_reg->rdesc.n_voltages
+			= ((reg_data[i].max_uv - reg_data[i].min_uv)
+				/ pm8008_reg->rdesc.uV_step) + 1;
+		pm8008_reg->rdesc.linear_ranges = reg_data[i].voltage_range;
+		pm8008_reg->rdesc.n_linear_ranges = 1;
+		pm8008_reg->rdesc.enable_reg = LDO_ENABLE_REG(pm8008_reg->base);
+		pm8008_reg->rdesc.enable_mask = ENABLE_BIT;
+		pm8008_reg->rdesc.min_dropout_uV = reg_data[i].min_dropout_uv;
+
+		reg_config.dev = dev->parent;
+		reg_config.driver_data = pm8008_reg;
+
+		rdev = devm_regulator_register(dev, &pm8008_reg->rdesc, &reg_config);
+		if (IS_ERR(rdev)) {
+			rc = PTR_ERR(rdev);
+			dev_err(dev, "%s: failed to register regulator rc=%d\n",
+					reg_data[i].name, rc);
+			return rc;
+		}
+	}
+
+	return 0;
+}
+
+static struct platform_driver pm8008_regulator_driver = {
+	.driver	= {
+		.name		= "qcom,pm8008-regulators",
+	},
+	.probe	= pm8008_regulator_probe,
+};
+
+module_platform_driver(pm8008_regulator_driver);
+
+MODULE_DESCRIPTION("Qualcomm PM8008 PMIC Regulator Driver");
+MODULE_LICENSE("GPL");
-- 
2.7.4

[PATCH V8 5/7] arm64: dts: qcom: pm8008-infra: Add base dts file for pm8008 infra

[Satya Priya]

Add base DTS file for pm8008 infra with parent mfd node.

Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
---
Changes in V8:
 - Previously added pm8008 dtsi file is split into 2 files, one for infra
   and other for regulators.

 arch/arm64/boot/dts/qcom/pm8008-infra.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/pm8008-infra.dtsi

diff --git a/arch/arm64/boot/dts/qcom/pm8008-infra.dtsi b/arch/arm64/boot/dts/qcom/pm8008-infra.dtsi
new file mode 100644
index 0000000..aeba07a
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/pm8008-infra.dtsi
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: BSD-3-Clause
+// Copyright (c) 2022, The Linux Foundation. All rights reserved.
+
+pm8008_infra: pmic@8 {
+	compatible = "qcom,pm8008-infra";
+	reg = <0x8>;
+	#address-cells = <1>;
+	#size-cells = <0>;
+};
-- 
2.7.4

[PATCH V8 6/7] arm64: dts: qcom: pm8008-regulators: Add base dts file for pm8008 regulators

[Satya Priya]

Add base DTS file for pm8008 regulators with 7 ldo nodes.

Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
---
Changes in V8:
 - Previously added pm8008 dtsi file is split into 2 files, one for infra
   and other for regulators.

 arch/arm64/boot/dts/qcom/pm8008-regulators.dtsi | 37 +++++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/pm8008-regulators.dtsi

diff --git a/arch/arm64/boot/dts/qcom/pm8008-regulators.dtsi b/arch/arm64/boot/dts/qcom/pm8008-regulators.dtsi
new file mode 100644
index 0000000..3bfdefd
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/pm8008-regulators.dtsi
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: BSD-3-Clause
+// Copyright (c) 2022, The Linux Foundation. All rights reserved.
+
+pm8008_regulators: pmic@9 {
+	compatible = "qcom,pm8008-regulators";
+	reg = <0x9>;
+	#address-cells = <0>;
+	#size-cells = <0>;
+
+	pm8008_l1: ldo1 {
+		regulator-name = "pm8008_l1";
+	};
+
+	pm8008_l2: ldo2 {
+		regulator-name = "pm8008_l2";
+	};
+
+	pm8008_l3: ldo3 {
+		regulator-name = "pm8008_l3";
+	};
+
+	pm8008_l4: ldo4 {
+		regulator-name = "pm8008_l4";
+	};
+
+	pm8008_l5: ldo5 {
+		regulator-name = "pm8008_l5";
+	};
+
+	pm8008_l6: ldo6 {
+		regulator-name = "pm8008_l6";
+	};
+
+	pm8008_l7: ldo7 {
+		regulator-name = "pm8008_l7";
+	};
+};
-- 
2.7.4

[PATCH V8 7/7] arm64: dts: qcom: sc7280: Add pm8008 support for sc7280-idp

[Satya Priya]

Add pm8008_infra and pm8008_regulators support for sc7280 idp.

Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
---
Changes in V6:
 - No changes.

Changes in V7:
 - No Changes.

Changes in V8:
 - Add an extra phandle "pm8008_bus" and then include pm8008 dtsi files inside it.
 - Remove output-high from pm8008_active node.

 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 70 ++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index ecbf2b8..0843e92 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -263,6 +263,67 @@
 	};
 };
 
+pm8008_bus: &i2c1 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	status = "okay";
+};
+
+&pm8008_bus {
+	#include "pm8008-infra.dtsi"
+	#include "pm8008-regulators.dtsi"
+};
+
+&pm8008_infra {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pm8008_active>;
+
+	reset-gpios = <&pm8350c_gpios 4 GPIO_ACTIVE_HIGH>;
+};
+
+&pm8008_regulators {
+	vdd_l1_l2-supply = <&vreg_s8b_1p2>;
+	vdd_l3_l4-supply = <&vreg_s1b_1p8>;
+	vdd_l5-supply = <&vreg_bob>;
+	vdd_l6-supply = <&vreg_bob>;
+	vdd_l7-supply = <&vreg_bob>;
+};
+
+&pm8008_l1 {
+	regulator-min-microvolt = <950000>;
+	regulator-max-microvolt = <1300000>;
+};
+
+&pm8008_l2 {
+	regulator-min-microvolt = <950000>;
+	regulator-max-microvolt = <1250000>;
+};
+
+&pm8008_l3 {
+	regulator-min-microvolt = <1650000>;
+	regulator-max-microvolt = <3000000>;
+};
+
+&pm8008_l4 {
+	regulator-min-microvolt = <1504000>;
+	regulator-max-microvolt = <1600000>;
+};
+
+&pm8008_l5 {
+	regulator-min-microvolt = <2600000>;
+	regulator-max-microvolt = <3000000>;
+};
+
+&pm8008_l6 {
+	regulator-min-microvolt = <2600000>;
+	regulator-max-microvolt = <3000000>;
+};
+
+&pm8008_l7 {
+	regulator-min-microvolt = <3000000>;
+	regulator-max-microvolt = <3544000>;
+};
+
 &qfprom {
 	vcc-supply = <&vreg_l1c_1p8>;
 };
@@ -375,6 +436,15 @@
 	drive-strength = <2>;
 };
 
+&pm8350c_gpios {
+	pm8008_active: pm8008-active {
+		pins = "gpio4";
+		function = "normal";
+		bias-disable;
+		power-source = <0>;
+	};
+};
+
 &qspi_cs0 {
 	bias-disable;
 };
-- 
2.7.4

Re: [PATCH V8 6/7] arm64: dts: qcom: pm8008-regulators: Add base dts file for pm8008 regulators

[Stephen Boyd]

Quoting Satya Priya (2022-03-25 04:58:10)
> Add base DTS file for pm8008 regulators with 7 ldo nodes.
>
> Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
> ---
> Changes in V8:
>  - Previously added pm8008 dtsi file is split into 2 files, one for infra
>    and other for regulators.

Does pm8008 come in one package that gets soldered down on the board? If
so, this should be one file instead of two, i.e. pm8008.dtsi. We don't
make a -regulators dtsi file for other pmics.

Re: [PATCH V8 7/7] arm64: dts: qcom: sc7280: Add pm8008 support for sc7280-idp

[Stephen Boyd]

Quoting Satya Priya (2022-03-25 04:58:11)
> Add pm8008_infra and pm8008_regulators support for sc7280 idp.
>
> Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
> ---
> Changes in V6:
>  - No changes.
>
> Changes in V7:
>  - No Changes.
>
> Changes in V8:
>  - Add an extra phandle "pm8008_bus" and then include pm8008 dtsi files inside it.
>  - Remove output-high from pm8008_active node.
>
>  arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 70 ++++++++++++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> index ecbf2b8..0843e92 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> @@ -263,6 +263,67 @@
>         };
>  };
>
> +pm8008_bus: &i2c1 {
> +       #address-cells = <1>;
> +       #size-cells = <0>;

These two properties should already be in the i2c1 node. Can you remove
them?

> +       status = "okay";
> +};
> +
> +&pm8008_bus {
> +       #include "pm8008-infra.dtsi"
> +       #include "pm8008-regulators.dtsi"
> +};

I was thinking more of like

	&pm8008_bus: &i2c1 {
		status = "okay";
	};

	#include "pm8008.dtsi"

and then the nodes below.

Re: [PATCH V8 1/7] dt-bindings: mfd: pm8008: Modify the compatible as per driver

[Stephen Boyd]

Quoting Satya Priya (2022-03-25 04:58:05)
> Modify the compatible string as per the pm8008 mfd driver.
> Add reset-gpios property and make interrupts and interrupt-cells
> as optional properties, they are not strictly required and may
> cause yaml compilation errors when not added in the DT files.

Does it have an interrupt controller inside? If so, the properties
should be present even if the driver isn't using them.

>
> Also, change the node name in example to match with the
> pm8008_infra DT node.
>
> Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
> ---
> Changes in V5:
>  - Remove compatible for regulators node.
>  - Move supply nodes of the regulators to chip level.
>
> Changes in V6:
>  - No changes.
>
> Changes in V7:
>  - Removed the intermediate regulators node and added ldos
>    directly under mfd node.
>
> Changes in V8:
>  - Change the compatible as per driver, remove interrupts from required
>    properties, add reset-gpios and move regulators to separate binding.
>
>  Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
> index ec3138c..12431ea 100644
> --- a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
> +++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
> @@ -16,7 +16,7 @@ description: |
>
>  properties:
>    compatible:
> -    const: qcom,pm8008
> +    const: qcom,pm8008-infra

Why is the compatible being replaced with -infra postfix?

>
>    reg:
>      description:
> @@ -44,6 +44,10 @@ properties:
>    "#size-cells":
>      const: 0
>
> +  reset-gpios:
> +    description: |
> +      Specifies the GPIO to be toggled to bring pm8008 chip out of reset.

Remove description. Add maxItems: 1

> +
>  patternProperties:
>    "^gpio@[0-9a-f]+$":
>      type: object
> @@ -88,10 +92,8 @@ patternProperties:
>  required:
>    - compatible
>    - reg
> -  - interrupts
>    - "#address-cells"
>    - "#size-cells"
> -  - "#interrupt-cells"

Should reset-gpios be required? I'd expect this patch to be adding
reset-gpios and making it required and that's about it. Given that
there isn't a DT using this compatible upstream so far it looks like we
don't need to do anything like worry about backwards compatibility.

>
>  additionalProperties: false
>
> @@ -99,11 +101,12 @@ examples:
>    - |
>      #include <dt-bindings/mfd/qcom-pm8008.h>
>      #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/gpio/gpio.h>
>      qupv3_se13_i2c {
>        #address-cells = <1>;
>        #size-cells = <0>;
> -      pm8008i@8 {
> -        compatible = "qcom,pm8008";
> +      pm8008_infra: pmic@8 {

Remove unused phandle.

> +        compatible = "qcom,pm8008-infra";
>          reg = <0x8>;
>          #address-cells = <1>;
>          #size-cells = <0>;
> @@ -113,6 +116,8 @@ examples:
>          interrupt-parent = <&tlmm>;
>          interrupts = <32 IRQ_TYPE_EDGE_RISING>;
>
> +        reset-gpios = <&pm8350c_gpios 4 GPIO_ACTIVE_HIGH>;
> +
>          pm8008_gpios: gpio@c000 {
>            compatible = "qcom,pm8008-gpio", "qcom,spmi-gpio";
>            reg = <0xc000>;
> --
> 2.7.4
>

Re: [PATCH V8 2/7] dt-bindings: mfd: pm8008: Add pm8008 regulator bindings

[Stephen Boyd]

Quoting Satya Priya (2022-03-25 04:58:06)
> Add bindings for pm8008 regulators device.
>
> Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
> ---
> Changes in V8:
>  - This is split from pm8008.yaml binding.
>
>  .../bindings/mfd/qcom,pm8008-regulators.yaml       | 87 ++++++++++++++++++++++
>  1 file changed, 87 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/qcom,pm8008-regulators.yaml
>
> diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008-regulators.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008-regulators.yaml
> new file mode 100644
> index 0000000..139fbf9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008-regulators.yaml
> @@ -0,0 +1,87 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/qcom,pm8008-regulators.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Technologies, Inc. PM8008 Regulator bindings
> +
> +maintainers:
> +  - Satya Priya <quic_c_skakit@quicinc.com>
> +
> +description: |
> +  Qualcomm Technologies, Inc. PM8008 is an I2C controlled PMIC
> +  containing 7 LDO regulators. This binding specifies the PM8008
> +  regulator peripherals of PM8008 device.
> +
> +properties:
> +  compatible:
> +    const: qcom,pm8008-regulators
> +
> +  reg:
> +    description:
> +      I2C slave address.

Drop description? We know what it is.

> +
> +    maxItems: 1
> +
> +  "#address-cells":
> +    const: 0
> +
> +  "#size-cells":
> +    const: 0
> +
> +  vdd_l1_l2-supply:
> +    description: Input supply phandle of ldo1 and ldo2 regulators.
> +
> +  vdd_l3_l4-supply:
> +    description: Input supply phandle of ldo3 and ldo4 regulators.
> +
> +  vdd_l5-supply:
> +    description: Input supply phandle of ldo5 regulator.
> +
> +  vdd_l6-supply:
> +    description: Input supply phandle of ldo6 regulator.
> +
> +  vdd_l7-supply:
> +    description: Input supply phandle of ldo7 regulator.
> +
> +patternProperties:
> +  "^ldo[1-7]$":
> +    type: object
> +    $ref: "../regulator/regulator.yaml#"
> +    description: PM8008 regulator peripherals of PM8008 regulator device
> +
> +required:
> +  - compatible
> +  - "#address-cells"
> +  - "#size-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/mfd/qcom-pm8008.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    qupv3_se13_i2c {

i2c

> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      pm8008_regulators: pmic@9 {

Drop phandle please.

> +        compatible = "qcom,pm8008-regulators";
> +        reg = <0x9>;
> +        #address-cells = <0>;
> +        #size-cells = <0>;
> +
> +        vdd_l1_l2-supply = <&vreg_s8b_1p2>;
> +        vdd_l3_l4-supply = <&vreg_s1b_1p8>;
> +        vdd_l5-supply = <&vreg_bob>;
> +        vdd_l6-supply = <&vreg_bob>;
> +        vdd_l7-supply = <&vreg_bob>;
> +
> +        pm8008_l1: ldo1 {
> +          regulator-name = "pm8008_l1";
> +          regulator-min-microvolt = <950000>;
> +          regulator-max-microvolt = <1300000>;
> +        };
> +      };
> +    };
> +...
> --
> 2.7.4
>

Re: [PATCH V8 6/7] arm64: dts: qcom: pm8008-regulators: Add base dts file for pm8008 regulators

[Satya Priya Kakitapalli (Temp)]

On 3/25/2022 11:13 PM, Stephen Boyd wrote:
> Quoting Satya Priya (2022-03-25 04:58:10)
>> Add base DTS file for pm8008 regulators with 7 ldo nodes.
>>
>> Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
>> ---
>> Changes in V8:
>>   - Previously added pm8008 dtsi file is split into 2 files, one for infra
>>     and other for regulators.
> Does pm8008 come in one package that gets soldered down on the board? If
> so, this should be one file instead of two, i.e. pm8008.dtsi. We don't
> make a -regulators dtsi file for other pmics.


Yes, pm8008 is a single chip.

You're right I should add only one pm8008.dtsi (including regulators)

Re: [PATCH V8 7/7] arm64: dts: qcom: sc7280: Add pm8008 support for sc7280-idp

[Satya Priya Kakitapalli (Temp)]

On 3/25/2022 11:18 PM, Stephen Boyd wrote:
> Quoting Satya Priya (2022-03-25 04:58:11)
>> Add pm8008_infra and pm8008_regulators support for sc7280 idp.
>>
>> Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
>> ---
>> Changes in V6:
>>   - No changes.
>>
>> Changes in V7:
>>   - No Changes.
>>
>> Changes in V8:
>>   - Add an extra phandle "pm8008_bus" and then include pm8008 dtsi files inside it.
>>   - Remove output-high from pm8008_active node.
>>
>>   arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 70 ++++++++++++++++++++++++++++++++
>>   1 file changed, 70 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> index ecbf2b8..0843e92 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> @@ -263,6 +263,67 @@
>>          };
>>   };
>>
>> +pm8008_bus: &i2c1 {
>> +       #address-cells = <1>;
>> +       #size-cells = <0>;
> These two properties should already be in the i2c1 node. Can you remove
> them?


Okay.


>> +       status = "okay";
>> +};
>> +
>> +&pm8008_bus {
>> +       #include "pm8008-infra.dtsi"
>> +       #include "pm8008-regulators.dtsi"
>> +};
> I was thinking more of like
>
> 	&pm8008_bus: &i2c1 {
> 		status = "okay";
> 	};
>
> 	#include "pm8008.dtsi"
>
> and then the nodes below.


You mean

pm8008_bus: &i2c1 {

         status = "okay";

};

&pm8008_bus {

     #include "pm8008.dtsi"

};

and remaining nodes, right?

Re: [PATCH V8 1/7] dt-bindings: mfd: pm8008: Modify the compatible as per driver

[Satya Priya Kakitapalli (Temp)]

On 3/25/2022 11:32 PM, Stephen Boyd wrote:
> Quoting Satya Priya (2022-03-25 04:58:05)
>> Modify the compatible string as per the pm8008 mfd driver.
>> Add reset-gpios property and make interrupts and interrupt-cells
>> as optional properties, they are not strictly required and may
>> cause yaml compilation errors when not added in the DT files.
> Does it have an interrupt controller inside? If so, the properties
> should be present even if the driver isn't using them.


Yes it has an interrupt controller but we are not configuring/using any 
interrupts currently in the DT node. So, if we add the properties under 
"required" list it throws a make dtbs error. Do you think we can somehow 
bypass this error?


>> Also, change the node name in example to match with the
>> pm8008_infra DT node.
>>
>> Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
>> ---
>> Changes in V5:
>>   - Remove compatible for regulators node.
>>   - Move supply nodes of the regulators to chip level.
>>
>> Changes in V6:
>>   - No changes.
>>
>> Changes in V7:
>>   - Removed the intermediate regulators node and added ldos
>>     directly under mfd node.
>>
>> Changes in V8:
>>   - Change the compatible as per driver, remove interrupts from required
>>     properties, add reset-gpios and move regulators to separate binding.
>>
>>   Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml | 15 ++++++++++-----
>>   1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
>> index ec3138c..12431ea 100644
>> --- a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
>> +++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
>> @@ -16,7 +16,7 @@ description: |
>>
>>   properties:
>>     compatible:
>> -    const: qcom,pm8008
>> +    const: qcom,pm8008-infra
> Why is the compatible being replaced with -infra postfix?


I've changed the compatible after splitting the probe into two parts 
pm8008-infra-probe and pm8008-regulators-probe. Only reason for change 
is to make it more readable/understandable.


>>     reg:
>>       description:
>> @@ -44,6 +44,10 @@ properties:
>>     "#size-cells":
>>       const: 0
>>
>> +  reset-gpios:
>> +    description: |
>> +      Specifies the GPIO to be toggled to bring pm8008 chip out of reset.
> Remove description. Add maxItems: 1


Okay.


>> +
>>   patternProperties:
>>     "^gpio@[0-9a-f]+$":
>>       type: object
>> @@ -88,10 +92,8 @@ patternProperties:
>>   required:
>>     - compatible
>>     - reg
>> -  - interrupts
>>     - "#address-cells"
>>     - "#size-cells"
>> -  - "#interrupt-cells"
> Should reset-gpios be required? I'd expect this patch to be adding
> reset-gpios and making it required and that's about it. Given that
> there isn't a DT using this compatible upstream so far it looks like we
> don't need to do anything like worry about backwards compatibility.


Okay.


>>   additionalProperties: false
>>
>> @@ -99,11 +101,12 @@ examples:
>>     - |
>>       #include <dt-bindings/mfd/qcom-pm8008.h>
>>       #include <dt-bindings/interrupt-controller/irq.h>
>> +    #include <dt-bindings/gpio/gpio.h>
>>       qupv3_se13_i2c {
>>         #address-cells = <1>;
>>         #size-cells = <0>;
>> -      pm8008i@8 {
>> -        compatible = "qcom,pm8008";
>> +      pm8008_infra: pmic@8 {
> Remove unused phandle.


Okay.


>> +        compatible = "qcom,pm8008-infra";
>>           reg = <0x8>;
>>           #address-cells = <1>;
>>           #size-cells = <0>;
>> @@ -113,6 +116,8 @@ examples:
>>           interrupt-parent = <&tlmm>;
>>           interrupts = <32 IRQ_TYPE_EDGE_RISING>;
>>
>> +        reset-gpios = <&pm8350c_gpios 4 GPIO_ACTIVE_HIGH>;
>> +
>>           pm8008_gpios: gpio@c000 {
>>             compatible = "qcom,pm8008-gpio", "qcom,spmi-gpio";
>>             reg = <0xc000>;
>> --
>> 2.7.4
>>

Re: [PATCH V8 2/7] dt-bindings: mfd: pm8008: Add pm8008 regulator bindings

[Satya Priya Kakitapalli (Temp)]

On 3/25/2022 11:38 PM, Stephen Boyd wrote:
> Quoting Satya Priya (2022-03-25 04:58:06)
>> Add bindings for pm8008 regulators device.
>>
>> Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
>> ---
>> Changes in V8:
>>   - This is split from pm8008.yaml binding.
>>
>>   .../bindings/mfd/qcom,pm8008-regulators.yaml       | 87 ++++++++++++++++++++++
>>   1 file changed, 87 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/mfd/qcom,pm8008-regulators.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008-regulators.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008-regulators.yaml
>> new file mode 100644
>> index 0000000..139fbf9
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008-regulators.yaml
>> @@ -0,0 +1,87 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/mfd/qcom,pm8008-regulators.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm Technologies, Inc. PM8008 Regulator bindings
>> +
>> +maintainers:
>> +  - Satya Priya <quic_c_skakit@quicinc.com>
>> +
>> +description: |
>> +  Qualcomm Technologies, Inc. PM8008 is an I2C controlled PMIC
>> +  containing 7 LDO regulators. This binding specifies the PM8008
>> +  regulator peripherals of PM8008 device.
>> +
>> +properties:
>> +  compatible:
>> +    const: qcom,pm8008-regulators
>> +
>> +  reg:
>> +    description:
>> +      I2C slave address.
> Drop description? We know what it is.


okay


>> +
>> +    maxItems: 1
>> +
>> +  "#address-cells":
>> +    const: 0
>> +
>> +  "#size-cells":
>> +    const: 0
>> +
>> +  vdd_l1_l2-supply:
>> +    description: Input supply phandle of ldo1 and ldo2 regulators.
>> +
>> +  vdd_l3_l4-supply:
>> +    description: Input supply phandle of ldo3 and ldo4 regulators.
>> +
>> +  vdd_l5-supply:
>> +    description: Input supply phandle of ldo5 regulator.
>> +
>> +  vdd_l6-supply:
>> +    description: Input supply phandle of ldo6 regulator.
>> +
>> +  vdd_l7-supply:
>> +    description: Input supply phandle of ldo7 regulator.
>> +
>> +patternProperties:
>> +  "^ldo[1-7]$":
>> +    type: object
>> +    $ref: "../regulator/regulator.yaml#"
>> +    description: PM8008 regulator peripherals of PM8008 regulator device
>> +
>> +required:
>> +  - compatible
>> +  - "#address-cells"
>> +  - "#size-cells"
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/mfd/qcom-pm8008.h>
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    qupv3_se13_i2c {
> i2c
>
>> +      #address-cells = <1>;
>> +      #size-cells = <0>;
>> +      pm8008_regulators: pmic@9 {
> Drop phandle please.


Okay.


>> +        compatible = "qcom,pm8008-regulators";
>> +        reg = <0x9>;
>> +        #address-cells = <0>;
>> +        #size-cells = <0>;
>> +
>> +        vdd_l1_l2-supply = <&vreg_s8b_1p2>;
>> +        vdd_l3_l4-supply = <&vreg_s1b_1p8>;
>> +        vdd_l5-supply = <&vreg_bob>;
>> +        vdd_l6-supply = <&vreg_bob>;
>> +        vdd_l7-supply = <&vreg_bob>;
>> +
>> +        pm8008_l1: ldo1 {
>> +          regulator-name = "pm8008_l1";
>> +          regulator-min-microvolt = <950000>;
>> +          regulator-max-microvolt = <1300000>;
>> +        };
>> +      };
>> +    };
>> +...
>> --
>> 2.7.4
>>

Re: [PATCH V8 1/7] dt-bindings: mfd: pm8008: Modify the compatible as per driver

[Stephen Boyd]

Quoting Satya Priya Kakitapalli (Temp) (2022-03-30 03:55:03)
>
> On 3/25/2022 11:32 PM, Stephen Boyd wrote:
> > Quoting Satya Priya (2022-03-25 04:58:05)
> >> Modify the compatible string as per the pm8008 mfd driver.
> >> Add reset-gpios property and make interrupts and interrupt-cells
> >> as optional properties, they are not strictly required and may
> >> cause yaml compilation errors when not added in the DT files.
> > Does it have an interrupt controller inside? If so, the properties
> > should be present even if the driver isn't using them.
>
>
> Yes it has an interrupt controller but we are not configuring/using any
> interrupts currently in the DT node. So, if we add the properties under
> "required" list it throws a make dtbs error. Do you think we can somehow
> bypass this error?
>

Can you fix the existing node to have those properties?

>
> >> Also, change the node name in example to match with the
> >> pm8008_infra DT node.
> >>
> >> Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
> >> ---
> >> Changes in V5:
> >>   - Remove compatible for regulators node.
> >>   - Move supply nodes of the regulators to chip level.
> >>
> >> Changes in V6:
> >>   - No changes.
> >>
> >> Changes in V7:
> >>   - Removed the intermediate regulators node and added ldos
> >>     directly under mfd node.
> >>
> >> Changes in V8:
> >>   - Change the compatible as per driver, remove interrupts from required
> >>     properties, add reset-gpios and move regulators to separate binding.
> >>
> >>   Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml | 15 ++++++++++-----
> >>   1 file changed, 10 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
> >> index ec3138c..12431ea 100644
> >> --- a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
> >> +++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
> >> @@ -16,7 +16,7 @@ description: |
> >>
> >>   properties:
> >>     compatible:
> >> -    const: qcom,pm8008
> >> +    const: qcom,pm8008-infra
> > Why is the compatible being replaced with -infra postfix?
>
>
> I've changed the compatible after splitting the probe into two parts
> pm8008-infra-probe and pm8008-regulators-probe. Only reason for change
> is to make it more readable/understandable.
>
>

Please leave the pm8008 compatible alone.

Re: [PATCH V8 7/7] arm64: dts: qcom: sc7280: Add pm8008 support for sc7280-idp

[Stephen Boyd]

Quoting Satya Priya Kakitapalli (Temp) (2022-03-30 03:44:01)
>
> On 3/25/2022 11:18 PM, Stephen Boyd wrote:
> >       #include "pm8008.dtsi"
> >
> > and then the nodes below.
>
>
> You mean
>
> pm8008_bus: &i2c1 {
>
>          status = "okay";
>
> };
>
> &pm8008_bus {
>
>      #include "pm8008.dtsi"
>
> };
>
> and remaining nodes, right?
>

No. I meant what I wrote.


       &pm8008_bus: &i2c1 {
               status = "okay";
       };

       #include "pm8008.dtsi"

and then the pm8008.dtsi file would have

	&pm8008_bus {
		pm8008: pmic@8 {
			compatible = "qcom,pm8008";
			...
		};

		pm8008_regulators: regulators@9 {
			compatible = "qcom,pm8008-regulators";
			...
		};
	};

and then this patch would modify those nodes with phandles.

Re: [PATCH V8 7/7] arm64: dts: qcom: sc7280: Add pm8008 support for sc7280-idp

[Satya Priya Kakitapalli (Temp)]

On 4/1/2022 2:48 AM, Stephen Boyd wrote:
> Quoting Satya Priya Kakitapalli (Temp) (2022-03-30 03:44:01)
>> On 3/25/2022 11:18 PM, Stephen Boyd wrote:
>>>        #include "pm8008.dtsi"
>>>
>>> and then the nodes below.
>>
>> You mean
>>
>> pm8008_bus: &i2c1 {
>>
>>           status = "okay";
>>
>> };
>>
>> &pm8008_bus {
>>
>>       #include "pm8008.dtsi"
>>
>> };
>>
>> and remaining nodes, right?
>>
> No. I meant what I wrote.
>

Okay thank you for elaborating,


>         &pm8008_bus: &i2c1 {


but this still needs to be "pm8008_bus: &i2c {" right.. no '&' before 
pm8008_bus.


>                 status = "okay";
>         };
>
>         #include "pm8008.dtsi"
>
> and then the pm8008.dtsi file would have
>
> 	&pm8008_bus {
> 		pm8008: pmic@8 {
> 			compatible = "qcom,pm8008";
> 			...
> 		};
>
> 		pm8008_regulators: regulators@9 {
> 			compatible = "qcom,pm8008-regulators";
> 			...
> 		};
> 	};
>
> and then this patch would modify those nodes with phandles.

Re: [PATCH V8 7/7] arm64: dts: qcom: sc7280: Add pm8008 support for sc7280-idp

[Stephen Boyd]

Quoting Satya Priya Kakitapalli (Temp) (2022-04-01 03:48:46)
>
> On 4/1/2022 2:48 AM, Stephen Boyd wrote:
>
>
> >         &pm8008_bus: &i2c1 {
>
>
> but this still needs to be "pm8008_bus: &i2c {" right.. no '&' before
> pm8008_bus.

Correct. Thanks for spotting my typo.

Re: [PATCH V8 1/7] dt-bindings: mfd: pm8008: Modify the compatible as per driver

[Satya Priya Kakitapalli (Temp)]

On 4/1/2022 2:45 AM, Stephen Boyd wrote:
> Quoting Satya Priya Kakitapalli (Temp) (2022-03-30 03:55:03)
>> On 3/25/2022 11:32 PM, Stephen Boyd wrote:
>>> Quoting Satya Priya (2022-03-25 04:58:05)
>>>> Modify the compatible string as per the pm8008 mfd driver.
>>>> Add reset-gpios property and make interrupts and interrupt-cells
>>>> as optional properties, they are not strictly required and may
>>>> cause yaml compilation errors when not added in the DT files.
>>> Does it have an interrupt controller inside? If so, the properties
>>> should be present even if the driver isn't using them.
>>
>> Yes it has an interrupt controller but we are not configuring/using any
>> interrupts currently in the DT node. So, if we add the properties under
>> "required" list it throws a make dtbs error. Do you think we can somehow
>> bypass this error?
>>
> Can you fix the existing node to have those properties?


Okay.


>>>> Also, change the node name in example to match with the
>>>> pm8008_infra DT node.
>>>>
>>>> Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
>>>> ---
>>>> Changes in V5:
>>>>    - Remove compatible for regulators node.
>>>>    - Move supply nodes of the regulators to chip level.
>>>>
>>>> Changes in V6:
>>>>    - No changes.
>>>>
>>>> Changes in V7:
>>>>    - Removed the intermediate regulators node and added ldos
>>>>      directly under mfd node.
>>>>
>>>> Changes in V8:
>>>>    - Change the compatible as per driver, remove interrupts from required
>>>>      properties, add reset-gpios and move regulators to separate binding.
>>>>
>>>>    Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml | 15 ++++++++++-----
>>>>    1 file changed, 10 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
>>>> index ec3138c..12431ea 100644
>>>> --- a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
>>>> +++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
>>>> @@ -16,7 +16,7 @@ description: |
>>>>
>>>>    properties:
>>>>      compatible:
>>>> -    const: qcom,pm8008
>>>> +    const: qcom,pm8008-infra
>>> Why is the compatible being replaced with -infra postfix?
>>
>> I've changed the compatible after splitting the probe into two parts
>> pm8008-infra-probe and pm8008-regulators-probe. Only reason for change
>> is to make it more readable/understandable.
>>
>>
> Please leave the pm8008 compatible alone.


Okay.