[PATCH V9 0/6] Add Qualcomm Technologies, Inc. PM8008 regulator driver

[PATCH V9 0/6] Add Qualcomm Technologies, Inc. PM8008 regulator driver

[Satya Priya]

Satya Priya (6):
  dt-bindings: mfd: pm8008: Add reset-gpios
  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: Add base dts file
  arm64: dts: qcom: sc7280: Add pm8008 support for sc7280-idp

 .../bindings/mfd/qcom,pm8008-regulators.yaml       |  84 +++++++++
 .../devicetree/bindings/mfd/qcom,pm8008.yaml       |   7 +
 arch/arm64/boot/dts/qcom/pm8008.dtsi               |  47 +++++
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi           |  68 +++++++
 drivers/mfd/qcom-pm8008.c                          |  80 +++++++-
 drivers/regulator/Kconfig                          |   9 +
 drivers/regulator/Makefile                         |   1 +
 drivers/regulator/qcom-pm8008-regulator.c          | 205 +++++++++++++++++++++
 8 files changed, 499 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/qcom,pm8008-regulators.yaml
 create mode 100644 arch/arm64/boot/dts/qcom/pm8008.dtsi
 create mode 100644 drivers/regulator/qcom-pm8008-regulator.c

-- 
2.7.4

[PATCH V9 1/6] dt-bindings: mfd: pm8008: Add reset-gpios

[Satya Priya]

Add reset-gpios property for pm8008.

Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
---
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.

Changes in V9:
 - Undo the changes from V8 and only add reset-gpios. Leave interrupts
   as required properties and do not change compatible.
   
 Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
index ec3138c..3312784 100644
--- a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
+++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
@@ -44,6 +44,9 @@ properties:
   "#size-cells":
     const: 0
 
+  reset-gpios:
+    maxItems: 1
+
 patternProperties:
   "^gpio@[0-9a-f]+$":
     type: object
@@ -92,6 +95,7 @@ required:
   - "#address-cells"
   - "#size-cells"
   - "#interrupt-cells"
+  - reset-gpios
 
 additionalProperties: false
 
@@ -99,6 +103,7 @@ 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>;
@@ -113,6 +118,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 V9 1/6] dt-bindings: mfd: pm8008: Add reset-gpios

[Stephen Boyd]

Quoting Satya Priya (2022-04-05 06:50:28)
> Add reset-gpios property for pm8008.
>
> Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Re: [PATCH V9 1/6] dt-bindings: mfd: pm8008: Add reset-gpios

[Rob Herring]

On Tue, 05 Apr 2022 19:20:28 +0530, Satya Priya wrote:
> Add reset-gpios property for pm8008.
> 
> Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
> ---
> 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.
> 
> Changes in V9:
>  - Undo the changes from V8 and only add reset-gpios. Leave interrupts
>    as required properties and do not change compatible.
> 
>  Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH V9 2/6] 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.

Changes in V9:
 - Remove description for reg and drop unused phandle from example.

 .../bindings/mfd/qcom,pm8008-regulators.yaml       | 84 ++++++++++++++++++++++
 1 file changed, 84 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..9a82a8b
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008-regulators.yaml
@@ -0,0 +1,84 @@
+# 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:
+    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>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      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

Re: [PATCH V9 2/6] dt-bindings: mfd: pm8008: Add pm8008 regulator bindings

[Stephen Boyd]

Quoting Satya Priya (2022-04-05 06:50:29)
> Add bindings for pm8008 regulators device.
>
> Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Re: [PATCH V9 2/6] dt-bindings: mfd: pm8008: Add pm8008 regulator bindings

[Rob Herring]

On Tue, Apr 05, 2022 at 07:20:29PM +0530, Satya Priya wrote:
> 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.
> 
> Changes in V9:
>  - Remove description for reg and drop unused phandle from example.
> 
>  .../bindings/mfd/qcom,pm8008-regulators.yaml       | 84 ++++++++++++++++++++++
>  1 file changed, 84 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..9a82a8b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008-regulators.yaml
> @@ -0,0 +1,84 @@
> +# 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:
> +    maxItems: 1
> +

> +  "#address-cells":
> +    const: 0
> +
> +  "#size-cells":
> +    const: 0

Not needed, you don't have child nodes with 'reg'.

> +
> +  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#"

/schemas/regulator/...

> +    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>
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      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 V9 3/6] mfd: pm8008: Add mfd cell struct to register LDOs

[Satya Priya]

Add mfd cell struct to match with the "qcom,pm8008-regulator"
driver and a separate probe to add pm8008_regulator_devs.
This separate probe is required to ensure the regulators are
registered only with the mfd device which contains regulators.

Add the reset-gpio toggling in the pm8008_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 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

Changes in V9:
 - Fixed nits.

 drivers/mfd/qcom-pm8008.c | 80 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 78 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/qcom-pm8008.c b/drivers/mfd/qcom-pm8008.c
index c472d7f..c7781ab 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;
 };
@@ -239,13 +246,44 @@ 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 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_match[] = {
 	{ .compatible = "qcom,pm8008", },
-	{ },
+	{ }
 };
+MODULE_DEVICE_TABLE(of, pm8008_match);
 
 static struct i2c_driver pm8008_mfd_driver = {
 	.driver = {
@@ -254,7 +292,45 @@ static struct i2c_driver pm8008_mfd_driver = {
 	},
 	.probe_new = pm8008_probe,
 };
-module_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-regulators",
+		.of_match_table = pm8008_regulators_match,
+	},
+	.probe_new = pm8008_probe_regulators,
+};
+
+static int __init pm8008_i2c_init(void)
+{
+	int ret;
+
+	ret = i2c_add_driver(&pm8008_mfd_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_mfd_driver);
+	i2c_del_driver(&pm8008_regulators_driver);
+}
+module_exit(pm8008_i2c_exit);
 
 MODULE_LICENSE("GPL v2");
 MODULE_ALIAS("i2c:qcom-pm8008");
-- 
2.7.4

Re: [PATCH V9 3/6] mfd: pm8008: Add mfd cell struct to register LDOs

[Stephen Boyd]

Quoting Satya Priya (2022-04-05 06:50:30)
> Add mfd cell struct to match with the "qcom,pm8008-regulator"
> driver and a separate probe to add pm8008_regulator_devs.
> This separate probe is required to ensure the regulators are
> registered only with the mfd device which contains regulators.

I don't get it. Shouldn't the pm8008-regulators compatible string be a
different i2c driver in drivers/regulator/ that matches the compatible
string for qcom,pm8008-regulators?

>
> Add the reset-gpio toggling in the pm8008_probe() to bring
> pm8008 chip out of reset instead of doing it in DT node using
> "output-high" property.

Please split this part off to a different patch.

Re: [PATCH V9 3/6] mfd: pm8008: Add mfd cell struct to register LDOs

[Satya Priya Kakitapalli (Temp)]

On 4/6/2022 12:38 AM, Stephen Boyd wrote:
> Quoting Satya Priya (2022-04-05 06:50:30)
>> Add mfd cell struct to match with the "qcom,pm8008-regulator"
>> driver and a separate probe to add pm8008_regulator_devs.
>> This separate probe is required to ensure the regulators are
>> registered only with the mfd device which contains regulators.
> I don't get it. Shouldn't the pm8008-regulators compatible string be a
> different i2c driver in drivers/regulator/ that matches the compatible
> string for qcom,pm8008-regulators?


Initially we had it that way but as per Mark's suggestion here [1] I've 
made these changes to register regulators directly through mfd driver.

[1] 
https://patchwork.kernel.org/project/linux-arm-msm/patch/1637314953-4215-3-git-send-email-quic_c_skakit@quicinc.com/#24618011


>> Add the reset-gpio toggling in the pm8008_probe() to bring
>> pm8008 chip out of reset instead of doing it in DT node using
>> "output-high" property.
> Please split this part off to a different patch.


Okay.

[PATCH V9 4/6] 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 V7:
 - Removed unused Macros and headers.

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

Changes in V9:
 - Nothing has changed.

 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 5ef2306..06b0a19 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 1b64ad5..83eed71 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

Re: [PATCH V9 4/6] regulator: Add a regulator driver for the PM8008 PMIC

[Mark Brown]

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

On Tue, Apr 05, 2022 at 07:20:31PM +0530, Satya Priya wrote:

> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/machine.h>

Why does the driver need machine.h?  That's usually a bug, though I
didn't spot anywhere where it's used so it's probably just an extra
header.

> +	.set_voltage_sel	= pm8008_regulator_set_voltage,
> +	.get_voltage		= pm8008_regulator_get_voltage,

You shouldn't mix and match the selector and non-selector operations,
since the device just takes a voltage you may as well just use the
non-selector version for both.

Otherwise this all looks good, just those two minor points.

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

Re: [PATCH V9 4/6] regulator: Add a regulator driver for the PM8008 PMIC

[Satya Priya Kakitapalli (Temp)]

On 4/5/2022 7:47 PM, Mark Brown wrote:
> On Tue, Apr 05, 2022 at 07:20:31PM +0530, Satya Priya wrote:
>
>> +#include <linux/regulator/driver.h>
>> +#include <linux/regulator/machine.h>
> Why does the driver need machine.h?  That's usually a bug, though I
> didn't spot anywhere where it's used so it's probably just an extra
> header.


Yeah, I'll remove it. Thanks for spotting this.


>> +	.set_voltage_sel	= pm8008_regulator_set_voltage,
>> +	.get_voltage		= pm8008_regulator_get_voltage,
> You shouldn't mix and match the selector and non-selector operations,
> since the device just takes a voltage you may as well just use the
> non-selector version for both.


I was suggested to use set_voltage_sel on my previous posts. I think 
I'll use get_voltage_sel to avoid mixing selector and non-selector APIs.


> Otherwise this all looks good, just those two minor points.

Re: [PATCH V9 4/6] regulator: Add a regulator driver for the PM8008 PMIC

[Stephen Boyd]

Quoting Satya Priya (2022-04-05 06:50:31)
> 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 @@
[...]
> +
> +static struct platform_driver pm8008_regulator_driver = {

Why isn't this an i2c driver?

> +       .driver = {
> +               .name           = "qcom,pm8008-regulators",
> +       },
> +       .probe  = pm8008_regulator_probe,
> +};

Re: [PATCH V9 4/6] regulator: Add a regulator driver for the PM8008 PMIC

[Mark Brown]

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

On Tue, Apr 05, 2022 at 02:09:06PM -0500, Stephen Boyd wrote:
> Quoting Satya Priya (2022-04-05 06:50:31)

> > +static struct platform_driver pm8008_regulator_driver = {

> Why isn't this an i2c driver?

It's a MFD function driver isn't it?

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

Re: [PATCH V9 4/6] regulator: Add a regulator driver for the PM8008 PMIC

[Stephen Boyd]

Quoting Mark Brown (2022-04-06 00:31:45)
> On Tue, Apr 05, 2022 at 02:09:06PM -0500, Stephen Boyd wrote:
> > Quoting Satya Priya (2022-04-05 06:50:31)
>
> > > +static struct platform_driver pm8008_regulator_driver = {
>
> > Why isn't this an i2c driver?
>
> It's a MFD function driver isn't it?

The DT binding shows a single i2c node at i2c address 0x9. The
compatible for it is "qcom,pm8008-regulators". It looks like an i2c
device that is dedicated to providing regulators. I'd only expect to see
an MFD if the device responding at i2c address 0x9 supported more than
just regulators.

Re: [PATCH V9 4/6] regulator: Add a regulator driver for the PM8008 PMIC

[Mark Brown]

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

On Wed, Apr 06, 2022 at 08:23:11AM -0700, Stephen Boyd wrote:
> Quoting Mark Brown (2022-04-06 00:31:45)

> > It's a MFD function driver isn't it?

> The DT binding shows a single i2c node at i2c address 0x9. The
> compatible for it is "qcom,pm8008-regulators". It looks like an i2c
> device that is dedicated to providing regulators. I'd only expect to see
> an MFD if the device responding at i2c address 0x9 supported more than
> just regulators.

There's a MFD parent for it, and if it's for an I2C device for a pm8008
why would it have a -regulators in the name?

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

Re: [PATCH V9 4/6] regulator: Add a regulator driver for the PM8008 PMIC

[Stephen Boyd]

Quoting Mark Brown (2022-04-06 08:45:41)
> On Wed, Apr 06, 2022 at 08:23:11AM -0700, Stephen Boyd wrote:
> > Quoting Mark Brown (2022-04-06 00:31:45)
>
> > > It's a MFD function driver isn't it?
>
> > The DT binding shows a single i2c node at i2c address 0x9. The
> > compatible for it is "qcom,pm8008-regulators". It looks like an i2c
> > device that is dedicated to providing regulators. I'd only expect to see
> > an MFD if the device responding at i2c address 0x9 supported more than
> > just regulators.
>
> There's a MFD parent for it, and if it's for an I2C device for a pm8008
> why would it have a -regulators in the name?

There are two i2c devices. One is pm8008 at i2c address 0x8 and one is
pm8008-regulators at i2c address 0x9. Earlier revisions of this patch
series were making it very confusing by redoing the pm8008 binding and
adding the pm8008-regulator i2c address device to the same binding and
driver.

My guess is that this is one IC that responds to multiple i2c addresses.
The "main" qcom,pm8008 address is 0x8 and that supports things like
interrupts. Then there's an address for regulators at 0x9 which controls
the handful of LDOs on the PMIC.

Re: [PATCH V9 4/6] regulator: Add a regulator driver for the PM8008 PMIC

[Mark Brown]

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

On Wed, Apr 06, 2022 at 08:51:48AM -0700, Stephen Boyd wrote:
> Quoting Mark Brown (2022-04-06 08:45:41)

> > There's a MFD parent for it, and if it's for an I2C device for a pm8008
> > why would it have a -regulators in the name?

> There are two i2c devices. One is pm8008 at i2c address 0x8 and one is
> pm8008-regulators at i2c address 0x9. Earlier revisions of this patch
> series were making it very confusing by redoing the pm8008 binding and
> adding the pm8008-regulator i2c address device to the same binding and
> driver.

> My guess is that this is one IC that responds to multiple i2c addresses.
> The "main" qcom,pm8008 address is 0x8 and that supports things like
> interrupts. Then there's an address for regulators at 0x9 which controls
> the handful of LDOs on the PMIC.

So it's like the TI TWL4030 and Palmas - in which case it should
probably be handled similarly?  Note that the original sumbission was
*also* a MFD subfunction, but using a DT compatible to match the
platform device - this is the first I've heard of this being a separate
I2C function.

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

Re: [PATCH V9 4/6] regulator: Add a regulator driver for the PM8008 PMIC

[Stephen Boyd]

Quoting Mark Brown (2022-04-06 09:36:14)
> On Wed, Apr 06, 2022 at 08:51:48AM -0700, Stephen Boyd wrote:
> > Quoting Mark Brown (2022-04-06 08:45:41)
>
> > > There's a MFD parent for it, and if it's for an I2C device for a pm8008
> > > why would it have a -regulators in the name?
>
> > There are two i2c devices. One is pm8008 at i2c address 0x8 and one is
> > pm8008-regulators at i2c address 0x9. Earlier revisions of this patch
> > series were making it very confusing by redoing the pm8008 binding and
> > adding the pm8008-regulator i2c address device to the same binding and
> > driver.
>
> > My guess is that this is one IC that responds to multiple i2c addresses.
> > The "main" qcom,pm8008 address is 0x8 and that supports things like
> > interrupts. Then there's an address for regulators at 0x9 which controls
> > the handful of LDOs on the PMIC.
>
> So it's like the TI TWL4030 and Palmas - in which case it should
> probably be handled similarly?

How did those work out? I wasn't involved and I don't know what you
mean. Do they have multiple i2c addresses they respond to?

> Note that the original sumbission was
> *also* a MFD subfunction, but using a DT compatible to match the
> platform device - this is the first I've heard of this being a separate
> I2C function.

I'm mainly looking at the dts file now. It clearly has two i2c devices
at 0x8 and 0x9. Maybe the regulator driver followed the mfd design
because the first driver for this device is an mfd.

Re: [PATCH V9 4/6] regulator: Add a regulator driver for the PM8008 PMIC

[Mark Brown]

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

On Wed, Apr 06, 2022 at 10:21:01AM -0700, Stephen Boyd wrote:
> Quoting Mark Brown (2022-04-06 09:36:14)
> > On Wed, Apr 06, 2022 at 08:51:48AM -0700, Stephen Boyd wrote:

> > > My guess is that this is one IC that responds to multiple i2c addresses.
> > > The "main" qcom,pm8008 address is 0x8 and that supports things like
> > > interrupts. Then there's an address for regulators at 0x9 which controls
> > > the handful of LDOs on the PMIC.

> > So it's like the TI TWL4030 and Palmas - in which case it should
> > probably be handled similarly?

> How did those work out? I wasn't involved and I don't know what you
> mean. Do they have multiple i2c addresses they respond to?

Yes, exactly.  The main device uses i2c_new_dummy_device() to
instantiate the extras when it probes.  See twl-core.c

> 
> > Note that the original sumbission was
> > *also* a MFD subfunction, but using a DT compatible to match the
> > platform device - this is the first I've heard of this being a separate
> > I2C function.

> I'm mainly looking at the dts file now. It clearly has two i2c devices
> at 0x8 and 0x9. Maybe the regulator driver followed the mfd design
> because the first driver for this device is an mfd.

I'm guessing from the naming that they're also externally described as
the same device - presumably it's two dies shoved together in the same
package for some reason without being otherwise joined up.  Is the
second device geniunely regulators only or does it have anything else
bundled in there?

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

Re: [PATCH V9 4/6] regulator: Add a regulator driver for the PM8008 PMIC

[Stephen Boyd]

Quoting Mark Brown (2022-04-06 10:27:44)
> On Wed, Apr 06, 2022 at 10:21:01AM -0700, Stephen Boyd wrote:
> > Quoting Mark Brown (2022-04-06 09:36:14)
> > > On Wed, Apr 06, 2022 at 08:51:48AM -0700, Stephen Boyd wrote:
>
> > > > My guess is that this is one IC that responds to multiple i2c addresses.
> > > > The "main" qcom,pm8008 address is 0x8 and that supports things like
> > > > interrupts. Then there's an address for regulators at 0x9 which controls
> > > > the handful of LDOs on the PMIC.
>
> > > So it's like the TI TWL4030 and Palmas - in which case it should
> > > probably be handled similarly?
>
> > How did those work out? I wasn't involved and I don't know what you
> > mean. Do they have multiple i2c addresses they respond to?
>
> Yes, exactly.  The main device uses i2c_new_dummy_device() to
> instantiate the extras when it probes.  See twl-core.c

Cool. That approach sounds good to me. Then the regulators can be child
nodes of the qcom,pm8008 node at i2c address 0x8? It still feels like
making a struct driver for each regulator node is overkill and will
waste memory.

>
> >
> > > Note that the original sumbission was
> > > *also* a MFD subfunction, but using a DT compatible to match the
> > > platform device - this is the first I've heard of this being a separate
> > > I2C function.
>
> > I'm mainly looking at the dts file now. It clearly has two i2c devices
> > at 0x8 and 0x9. Maybe the regulator driver followed the mfd design
> > because the first driver for this device is an mfd.
>
> I'm guessing from the naming that they're also externally described as
> the same device - presumably it's two dies shoved together in the same
> package for some reason without being otherwise joined up.  Is the
> second device geniunely regulators only or does it have anything else
> bundled in there?

I think it's regulators only. Pretty sure I asked qcom this a round or
two ago on this patch series and they said that. Let's wait for Satya to
respond.

Re: [PATCH V9 4/6] regulator: Add a regulator driver for the PM8008 PMIC

[Satya Priya Kakitapalli (Temp)]

On 4/6/2022 11:46 PM, Stephen Boyd wrote:
> Quoting Mark Brown (2022-04-06 10:27:44)
>> On Wed, Apr 06, 2022 at 10:21:01AM -0700, Stephen Boyd wrote:
>>> Quoting Mark Brown (2022-04-06 09:36:14)
>>>> On Wed, Apr 06, 2022 at 08:51:48AM -0700, Stephen Boyd wrote:
>>>>> My guess is that this is one IC that responds to multiple i2c addresses.
>>>>> The "main" qcom,pm8008 address is 0x8 and that supports things like
>>>>> interrupts. Then there's an address for regulators at 0x9 which controls
>>>>> the handful of LDOs on the PMIC.
>>>> So it's like the TI TWL4030 and Palmas - in which case it should
>>>> probably be handled similarly?
>>> How did those work out? I wasn't involved and I don't know what you
>>> mean. Do they have multiple i2c addresses they respond to?
>> Yes, exactly.  The main device uses i2c_new_dummy_device() to
>> instantiate the extras when it probes.  See twl-core.c
> Cool. That approach sounds good to me. Then the regulators can be child
> nodes of the qcom,pm8008 node at i2c address 0x8? It still feels like
> making a struct driver for each regulator node is overkill and will
> waste memory.
>
>>>> Note that the original sumbission was
>>>> *also* a MFD subfunction, but using a DT compatible to match the
>>>> platform device - this is the first I've heard of this being a separate
>>>> I2C function.
>>> I'm mainly looking at the dts file now. It clearly has two i2c devices
>>> at 0x8 and 0x9. Maybe the regulator driver followed the mfd design
>>> because the first driver for this device is an mfd.
>> I'm guessing from the naming that they're also externally described as
>> the same device - presumably it's two dies shoved together in the same
>> package for some reason without being otherwise joined up.  Is the
>> second device geniunely regulators only or does it have anything else
>> bundled in there?
> I think it's regulators only. Pretty sure I asked qcom this a round or
> two ago on this patch series and they said that. Let's wait for Satya to
> respond.


Yes, it contains regulators only.

Re: [PATCH V9 4/6] regulator: Add a regulator driver for the PM8008 PMIC

[Satya Priya Kakitapalli (Temp)]

On 4/6/2022 12:39 AM, Stephen Boyd wrote:
> Quoting Satya Priya (2022-04-05 06:50:31)
>> 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 @@
> [...]
>> +
>> +static struct platform_driver pm8008_regulator_driver = {
> Why isn't this an i2c driver?


The parent mfd driver(qcom-pm8008.c) uses i2c to communicate, this is 
the child to that and hence we don't use i2c driver here.


>> +       .driver = {
>> +               .name           = "qcom,pm8008-regulators",
>> +       },
>> +       .probe  = pm8008_regulator_probe,
>> +};

Re: [PATCH V9 4/6] regulator: Add a regulator driver for the PM8008 PMIC

[Stephen Boyd]

Quoting Satya Priya (2022-04-05 06:50:31)
> +
> +static struct platform_driver pm8008_regulator_driver = {
> +       .driver = {
> +               .name           = "qcom,pm8008-regulators",

Also, the name should be something like pm8008_regulators

Re: [PATCH V9 4/6] regulator: Add a regulator driver for the PM8008 PMIC

[Satya Priya Kakitapalli (Temp)]

On 4/6/2022 12:39 AM, Stephen Boyd wrote:
> Quoting Satya Priya (2022-04-05 06:50:31)
>> +
>> +static struct platform_driver pm8008_regulator_driver = {
>> +       .driver = {
>> +               .name           = "qcom,pm8008-regulators",
> Also, the name should be something like pm8008_regulators


I've seen the other qcom regulator driver names, I think I shouldn't be 
using coma here. How about qcom-pm8008-regulator ? similar to the other 
regulator drivers

[PATCH V9 5/6] arm64: dts: qcom: pm8008: Add base dts file

[Satya Priya]

Add base DTS file for pm8008 with infra and regulator nodes.

Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
---
Changes in V9:
 - Add single dt file for pm8008 instead of adding files like in V8.

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

diff --git a/arch/arm64/boot/dts/qcom/pm8008.dtsi b/arch/arm64/boot/dts/qcom/pm8008.dtsi
new file mode 100644
index 0000000..24bd832
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/pm8008.dtsi
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: BSD-3-Clause
+// Copyright (c) 2022, The Linux Foundation. All rights reserved.
+
+&pm8008_bus {
+	pm8008: pm8008@8 {
+		compatible = "qcom,pm8008";
+		reg = <0x8>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		#interrupt-cells = <2>;
+	};
+
+	pm8008_regulators: pm8008@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

Re: [PATCH V9 5/6] arm64: dts: qcom: pm8008: Add base dts file

[Stephen Boyd]

Quoting Satya Priya (2022-04-05 06:50:32)
> diff --git a/arch/arm64/boot/dts/qcom/pm8008.dtsi b/arch/arm64/boot/dts/qcom/pm8008.dtsi
> new file mode 100644
> index 0000000..24bd832
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/pm8008.dtsi
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +// Copyright (c) 2022, The Linux Foundation. All rights reserved.
> +
> +&pm8008_bus {
> +       pm8008: pm8008@8 {

pmic@8

> +               compatible = "qcom,pm8008";
> +               reg = <0x8>;
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +               #interrupt-cells = <2>;
> +       };
> +
> +       pm8008_regulators: pm8008@9 {

pmic@9

> +               compatible = "qcom,pm8008-regulators";
> +               reg = <0x9>;
> +               #address-cells = <0>;
> +               #size-cells = <0>;
> +

Re: [PATCH V9 5/6] arm64: dts: qcom: pm8008: Add base dts file

[Satya Priya Kakitapalli (Temp)]

On 4/6/2022 12:41 AM, Stephen Boyd wrote:
> Quoting Satya Priya (2022-04-05 06:50:32)
>> diff --git a/arch/arm64/boot/dts/qcom/pm8008.dtsi b/arch/arm64/boot/dts/qcom/pm8008.dtsi
>> new file mode 100644
>> index 0000000..24bd832
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/pm8008.dtsi
>> @@ -0,0 +1,47 @@
>> +// SPDX-License-Identifier: BSD-3-Clause
>> +// Copyright (c) 2022, The Linux Foundation. All rights reserved.
>> +
>> +&pm8008_bus {
>> +       pm8008: pm8008@8 {
> pmic@8


Sorry I missed this change. I'll change it in next post.


>> +               compatible = "qcom,pm8008";
>> +               reg = <0x8>;
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +               #interrupt-cells = <2>;
>> +       };
>> +
>> +       pm8008_regulators: pm8008@9 {
> pmic@9
>
>> +               compatible = "qcom,pm8008-regulators";
>> +               reg = <0x9>;
>> +               #address-cells = <0>;
>> +               #size-cells = <0>;
>> +

[PATCH V9 6/6] arm64: dts: qcom: sc7280: Add pm8008 support for sc7280-idp

[Satya Priya]

Add pm8008 infra and 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.

Changes in V9:
 - Added interrupts properties.

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

diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index ecbf2b8..6f39c05 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -263,6 +263,65 @@
 	};
 };
 
+pm8008_bus: &i2c1 {
+	status = "okay";
+};
+
+#include "pm8008.dtsi"
+
+&pm8008 {
+	interrupt-parent = <&tlmm>;
+	interrupts = <24 IRQ_TYPE_EDGE_RISING>;
+
+	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 +434,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 V9 6/6] arm64: dts: qcom: sc7280: Add pm8008 support for sc7280-idp

[Stephen Boyd]

Quoting Satya Priya (2022-04-05 06:50:33)
> Add pm8008 infra and regulators support for sc7280 idp.
>
> Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>