Devicetree
 help / color / mirror / Atom feed
* [PATCH v2 0/3] regulator: qcom_usb_vbus: add pm4125 VBUS regulator support
@ 2026-07-01 10:28 Rakesh Kota
  2026-07-01 10:28 ` [PATCH v2 1/3] dt-bindings: regulator: qcom,usb-vbus-regulator: add qcom,pm4125-vbus-reg Rakesh Kota
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Rakesh Kota @ 2026-07-01 10:28 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Wesley Cheng, Bjorn Andersson, Konrad Dybcio
  Cc: linux-arm-msm, linux-kernel, devicetree, jishnu.prakash,
	kamal.wadhwa, Rakesh Kota

Add support for PM4125 USB VBUS regulator. Unlike PM8150B which uses
a current-limit selector, PM4125 uses a 2-bit VBOOST voltage selector
supporting 4.25 V, 4.5 V, 4.75 V and 5.0 V output.

Signed-off-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
---
Changes in v2:
- Fix the registers name and unit-address as per Konrad's suggestions.
- Use microvolt instead of microamp for qrb2210.
- Link to v1: https://lore.kernel.org/r/20260520-add_pm4125-vbus-reg-v1-0-f6d15d7dbbe8@oss.qualcomm.com

---
Rakesh Kota (3):
      dt-bindings: regulator: qcom,usb-vbus-regulator: add qcom,pm4125-vbus-reg
      regulator: qcom_usb_vbus: add support for qcom,pm4125-vbus-reg
      arm64: dts: qcom: Fix pm4125 vbus regulator compatible and constraints

 .../regulator/qcom,usb-vbus-regulator.yaml         |  50 +++++++++-
 arch/arm64/boot/dts/qcom/pm4125.dtsi               |   2 +-
 arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts |   4 +-
 arch/arm64/boot/dts/qcom/qrb2210-rb1.dts           |   4 +-
 drivers/regulator/qcom_usb_vbus-regulator.c        | 102 ++++++++++++++++++---
 5 files changed, 138 insertions(+), 24 deletions(-)
---
base-commit: be5c93fa674f0fc3c8f359c2143abce6bbb422e6
change-id: 20260520-add_pm4125-vbus-reg-95c858c6ae43

Best regards,
-- 
Rakesh Kota <rakesh.kota@oss.qualcomm.com>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 1/3] dt-bindings: regulator: qcom,usb-vbus-regulator: add qcom,pm4125-vbus-reg
  2026-07-01 10:28 [PATCH v2 0/3] regulator: qcom_usb_vbus: add pm4125 VBUS regulator support Rakesh Kota
@ 2026-07-01 10:28 ` Rakesh Kota
  2026-07-01 10:28 ` [PATCH v2 2/3] regulator: qcom_usb_vbus: add support for qcom,pm4125-vbus-reg Rakesh Kota
  2026-07-01 10:28 ` [PATCH v2 3/3] arm64: dts: qcom: Fix pm4125 vbus regulator compatible and constraints Rakesh Kota
  2 siblings, 0 replies; 20+ messages in thread
From: Rakesh Kota @ 2026-07-01 10:28 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Wesley Cheng, Bjorn Andersson, Konrad Dybcio
  Cc: linux-arm-msm, linux-kernel, devicetree, jishnu.prakash,
	kamal.wadhwa, Rakesh Kota

The pm4125 PMIC uses a different USB VBUS register layout than pm8150b.
It uses a 2-bit VBOOST voltage selector supporting output voltages of
4.25 V, 4.5 V, 4.75 V and 5.0 V, instead of a current-limit selector.

Move qcom,pm4125-vbus-reg from the pm8150b fallback items list into the
standalone enum since the driver handles it with its own match-data and
register layout.

Make regulator-min/max-microamp conditional so they are only required
for current-limit variants (pm8150b, pm6150, pm7250b, pmi632). Add an
if/then condition for qcom,pm4125-vbus-reg requiring regulator-min/
max-microvolt instead, and update the pm4125 example accordingly.

Signed-off-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
---
 .../regulator/qcom,usb-vbus-regulator.yaml         | 50 +++++++++++++++++++---
 1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/regulator/qcom,usb-vbus-regulator.yaml b/Documentation/devicetree/bindings/regulator/qcom,usb-vbus-regulator.yaml
index fcefc722ee2a495837fedba026669de97350c566..024b34d0eb1a694b9e0ff530a35f787582caa425 100644
--- a/Documentation/devicetree/bindings/regulator/qcom,usb-vbus-regulator.yaml
+++ b/Documentation/devicetree/bindings/regulator/qcom,usb-vbus-regulator.yaml
@@ -14,17 +14,21 @@ description: |
   regulator will be enabled in situations where the device is required to
   provide power to the connected peripheral.
 
-allOf:
-  - $ref: regulator.yaml#
+  The pm8150b variant uses an OTG current-limit selector, supporting limits
+  of 500 mA, 1000 mA, 1500 mA, 2000 mA, 2500 mA and 3000 mA.
+
+  The pm4125 variant uses a different register layout with a 2-bit VBOOST
+  voltage selector supporting output voltages of 4.25 V, 4.5 V, 4.75 V
+  and 5.0 V.
 
 properties:
   compatible:
     oneOf:
       - enum:
           - qcom,pm8150b-vbus-reg
+          - qcom,pm4125-vbus-reg
       - items:
           - enum:
-              - qcom,pm4125-vbus-reg
               - qcom,pm6150-vbus-reg
               - qcom,pm7250b-vbus-reg
               - qcom,pmi632-vbus-reg
@@ -34,11 +38,35 @@ properties:
     maxItems: 1
     description: VBUS output base address
 
+allOf:
+  - $ref: regulator.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,pm8150b-vbus-reg
+              - qcom,pm6150-vbus-reg
+              - qcom,pm7250b-vbus-reg
+              - qcom,pmi632-vbus-reg
+    then:
+      required:
+        - regulator-min-microamp
+        - regulator-max-microamp
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: qcom,pm4125-vbus-reg
+    then:
+      required:
+        - regulator-min-microvolt
+        - regulator-max-microvolt
+
 required:
   - compatible
   - reg
-  - regulator-min-microamp
-  - regulator-max-microamp
 
 unevaluatedProperties: false
 
@@ -55,4 +83,16 @@ examples:
             regulator-max-microamp = <3000000>;
         };
      };
+  - |
+     pmic {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        usb-vbus-regulator@1100 {
+            compatible = "qcom,pm4125-vbus-reg";
+            reg = <0x1100>;
+            regulator-min-microvolt = <4250000>;
+            regulator-max-microvolt = <5000000>;
+        };
+     };
 ...

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 2/3] regulator: qcom_usb_vbus: add support for qcom,pm4125-vbus-reg
  2026-07-01 10:28 [PATCH v2 0/3] regulator: qcom_usb_vbus: add pm4125 VBUS regulator support Rakesh Kota
  2026-07-01 10:28 ` [PATCH v2 1/3] dt-bindings: regulator: qcom,usb-vbus-regulator: add qcom,pm4125-vbus-reg Rakesh Kota
@ 2026-07-01 10:28 ` Rakesh Kota
  2026-07-01 10:34   ` sashiko-bot
                     ` (2 more replies)
  2026-07-01 10:28 ` [PATCH v2 3/3] arm64: dts: qcom: Fix pm4125 vbus regulator compatible and constraints Rakesh Kota
  2 siblings, 3 replies; 20+ messages in thread
From: Rakesh Kota @ 2026-07-01 10:28 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Wesley Cheng, Bjorn Andersson, Konrad Dybcio
  Cc: linux-arm-msm, linux-kernel, devicetree, jishnu.prakash,
	kamal.wadhwa, Rakesh Kota

The PM4125 PMIC uses a different register layout for USB VBUS control
compared to PM8150B. On PM4125, CMD_OTG is at offset 0x50, OTG_CFG is
at 0x56, and offset 0x52 is a 2-bit VBOOST voltage selector rather than
a current-limit selector.

Introduce per-compatible regulator descriptor data to accommodate these
differences. This keeps the existing PM8150B current-limit logic intact
while adding a dedicated voltage-selector path for PM4125.

Signed-off-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
---
 drivers/regulator/qcom_usb_vbus-regulator.c | 102 ++++++++++++++++++++++++----
 1 file changed, 88 insertions(+), 14 deletions(-)

diff --git a/drivers/regulator/qcom_usb_vbus-regulator.c b/drivers/regulator/qcom_usb_vbus-regulator.c
index cd94ed67621fee9f6d7a0327054db0ebab6cc7ee..40e7e5589d5b499b2e84a00b2cfa754fba7e2f0f 100644
--- a/drivers/regulator/qcom_usb_vbus-regulator.c
+++ b/drivers/regulator/qcom_usb_vbus-regulator.c
@@ -20,10 +20,35 @@
 #define OTG_CFG				0x53
 #define OTG_EN_SRC_CFG			BIT(1)
 
+#define PM4125_VBOOST_EN		0x50
+#define PM4125_VBOOST_CFG		0x52
+#define PM4125_VBOOST_CFG_MASK		GENMASK(1, 0)
+#define PM4125_VBOOST_CFG		0x56
+#define PM4125_OTG_EN_SRC_CFG		BIT(0)
+
+struct qcom_usb_vbus_reg_data {
+	u16 cmd_otg;
+	u16 otg_cfg;
+	u8  otg_en_src_cfg;
+	u16 csel_reg;
+	u8 csel_mask;
+	const unsigned int *curr_table;
+	unsigned int n_current_limits;
+	u16 vsel_reg;
+	u8 vsel_mask;
+	const unsigned int *volt_table;
+	unsigned int n_voltages;
+	const struct regulator_ops *ops;
+};
+
 static const unsigned int curr_table[] = {
 	500000, 1000000, 1500000, 2000000, 2500000, 3000000,
 };
 
+static const unsigned int pm4125_vboost_table[] = {
+	4250000, 4500000, 4750000, 5000000,
+};
+
 static const struct regulator_ops qcom_usb_vbus_reg_ops = {
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
@@ -32,19 +57,43 @@ static const struct regulator_ops qcom_usb_vbus_reg_ops = {
 	.set_current_limit = regulator_set_current_limit_regmap,
 };
 
-static struct regulator_desc qcom_usb_vbus_rdesc = {
-	.name = "usb_vbus",
-	.ops = &qcom_usb_vbus_reg_ops,
-	.owner = THIS_MODULE,
-	.type = REGULATOR_VOLTAGE,
+static const struct regulator_ops qcom_usb_vbus_pm4125_reg_ops = {
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.list_voltage = regulator_list_voltage_table,
+};
+
+static const struct qcom_usb_vbus_reg_data pm8150b_data = {
+	.cmd_otg = CMD_OTG,
+	.otg_cfg = OTG_CFG,
+	.otg_en_src_cfg = OTG_EN_SRC_CFG,
+	.csel_reg = OTG_CURRENT_LIMIT_CFG,
+	.csel_mask = OTG_CURRENT_LIMIT_MASK,
 	.curr_table = curr_table,
 	.n_current_limits = ARRAY_SIZE(curr_table),
+	.ops = &qcom_usb_vbus_reg_ops,
+};
+
+static const struct qcom_usb_vbus_reg_data pm4125_data = {
+	.cmd_otg = PM4125_VBOOST_EN,
+	.otg_cfg = PM4125_VBOOST_CFG,
+	.otg_en_src_cfg = PM4125_OTG_EN_SRC_CFG,
+	.vsel_reg = PM4125_VBOOST_CFG,
+	.vsel_mask = PM4125_VBOOST_CFG_MASK,
+	.volt_table = pm4125_vboost_table,
+	.n_voltages = ARRAY_SIZE(pm4125_vboost_table),
+	.ops = &qcom_usb_vbus_pm4125_reg_ops,
 };
 
 static int qcom_usb_vbus_regulator_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	const struct qcom_usb_vbus_reg_data *data;
 	struct regulator_dev *rdev;
+	struct regulator_desc *rdesc;
 	struct regmap *regmap;
 	struct regulator_config config = { };
 	struct regulator_init_data *init_data;
@@ -57,27 +106,51 @@ static int qcom_usb_vbus_regulator_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	data = of_device_get_match_data(dev);
+	if (!data)
+		return -EINVAL;
+
 	regmap = dev_get_regmap(dev->parent, NULL);
 	if (!regmap) {
 		dev_err(dev, "Failed to get regmap\n");
 		return -ENOENT;
 	}
 
-	init_data = of_get_regulator_init_data(dev, dev->of_node,
-					       &qcom_usb_vbus_rdesc);
+	rdesc = devm_kzalloc(dev, sizeof(*rdesc), GFP_KERNEL);
+	if (!rdesc)
+		return -ENOMEM;
+
+	rdesc->name = "usb_vbus";
+	rdesc->ops = data->ops;
+	rdesc->owner = THIS_MODULE;
+	rdesc->type = REGULATOR_VOLTAGE;
+	rdesc->enable_reg = base + data->cmd_otg;
+	rdesc->enable_mask = OTG_EN;
+
+	if (data->curr_table) {
+		rdesc->curr_table = data->curr_table;
+		rdesc->n_current_limits = data->n_current_limits;
+		rdesc->csel_reg = base + data->csel_reg;
+		rdesc->csel_mask = data->csel_mask;
+	}
+
+	if (data->volt_table) {
+		rdesc->volt_table = data->volt_table;
+		rdesc->n_voltages = data->n_voltages;
+		rdesc->vsel_reg = base + data->vsel_reg;
+		rdesc->vsel_mask = data->vsel_mask;
+	}
+
+	init_data = of_get_regulator_init_data(dev, dev->of_node, rdesc);
 	if (!init_data)
 		return -ENOMEM;
 
-	qcom_usb_vbus_rdesc.enable_reg = base + CMD_OTG;
-	qcom_usb_vbus_rdesc.enable_mask = OTG_EN;
-	qcom_usb_vbus_rdesc.csel_reg = base + OTG_CURRENT_LIMIT_CFG;
-	qcom_usb_vbus_rdesc.csel_mask = OTG_CURRENT_LIMIT_MASK;
 	config.dev = dev;
 	config.init_data = init_data;
 	config.of_node = dev->of_node;
 	config.regmap = regmap;
 
-	rdev = devm_regulator_register(dev, &qcom_usb_vbus_rdesc, &config);
+	rdev = devm_regulator_register(dev, rdesc, &config);
 	if (IS_ERR(rdev)) {
 		ret = PTR_ERR(rdev);
 		dev_err(dev, "not able to register vbus reg %d\n", ret);
@@ -85,13 +158,14 @@ static int qcom_usb_vbus_regulator_probe(struct platform_device *pdev)
 	}
 
 	/* Disable HW logic for VBUS enable */
-	regmap_update_bits(regmap, base + OTG_CFG, OTG_EN_SRC_CFG, 0);
+	regmap_update_bits(regmap, base + data->otg_cfg, data->otg_en_src_cfg, 0);
 
 	return 0;
 }
 
 static const struct of_device_id qcom_usb_vbus_regulator_match[] = {
-	{ .compatible = "qcom,pm8150b-vbus-reg" },
+	{ .compatible = "qcom,pm8150b-vbus-reg", .data = &pm8150b_data },
+	{ .compatible = "qcom,pm4125-vbus-reg", .data = &pm4125_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, qcom_usb_vbus_regulator_match);

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 3/3] arm64: dts: qcom: Fix pm4125 vbus regulator compatible and constraints
  2026-07-01 10:28 [PATCH v2 0/3] regulator: qcom_usb_vbus: add pm4125 VBUS regulator support Rakesh Kota
  2026-07-01 10:28 ` [PATCH v2 1/3] dt-bindings: regulator: qcom,usb-vbus-regulator: add qcom,pm4125-vbus-reg Rakesh Kota
  2026-07-01 10:28 ` [PATCH v2 2/3] regulator: qcom_usb_vbus: add support for qcom,pm4125-vbus-reg Rakesh Kota
@ 2026-07-01 10:28 ` Rakesh Kota
  2026-07-01 10:34   ` sashiko-bot
                     ` (2 more replies)
  2 siblings, 3 replies; 20+ messages in thread
From: Rakesh Kota @ 2026-07-01 10:28 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Wesley Cheng, Bjorn Andersson, Konrad Dybcio
  Cc: linux-arm-msm, linux-kernel, devicetree, jishnu.prakash,
	kamal.wadhwa, Rakesh Kota

Remove pm8150b fallback compatible from pm4125_vbus and fix regulator
constraints in qrb2210 DTS files to use microvolt instead of
microamp.

Signed-off-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/pm4125.dtsi               | 2 +-
 arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts | 4 ++--
 arch/arm64/boot/dts/qcom/qrb2210-rb1.dts           | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/pm4125.dtsi b/arch/arm64/boot/dts/qcom/pm4125.dtsi
index 542e8fe030da40dfefc7b744dcc30133fdb35e74..3dc8d667d091ceb4267081d32b7aae983fc25071 100644
--- a/arch/arm64/boot/dts/qcom/pm4125.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm4125.dtsi
@@ -37,7 +37,7 @@ pm4125_resin: resin {
 		};
 
 		pm4125_vbus: usb-vbus-regulator@1100 {
-			compatible = "qcom,pm4125-vbus-reg", "qcom,pm8150b-vbus-reg";
+			compatible = "qcom,pm4125-vbus-reg";
 			reg = <0x1100>;
 			status = "disabled";
 		};
diff --git a/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts b/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
index bf088fa9807f040f0c8f405f9111b01790b09377..2b50f3f577577d30a87fcb6466c7d7690270aaa3 100644
--- a/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
+++ b/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
@@ -235,8 +235,8 @@ &mdss_dsi0_phy {
 };
 
 &pm4125_vbus {
-	regulator-min-microamp = <500000>;
-	regulator-max-microamp = <500000>;
+	regulator-min-microvolt = <4250000>;
+	regulator-max-microvolt = <5000000>;
 
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts b/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
index 4ace2d6c06ceee74ced7c8cf9b341089b689f703..3cb8c3c090fff2524cd3655affc012d714629138 100644
--- a/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
+++ b/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
@@ -433,8 +433,8 @@ pm4125_ss_in: endpoint {
 };
 
 &pm4125_vbus {
-	regulator-min-microamp = <500000>;
-	regulator-max-microamp = <500000>;
+	regulator-min-microvolt = <4250000>;
+	regulator-max-microvolt = <5000000>;
 	status = "okay";
 };
 

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/3] regulator: qcom_usb_vbus: add support for qcom,pm4125-vbus-reg
  2026-07-01 10:28 ` [PATCH v2 2/3] regulator: qcom_usb_vbus: add support for qcom,pm4125-vbus-reg Rakesh Kota
@ 2026-07-01 10:34   ` sashiko-bot
  2026-07-02  7:54     ` Rakesh Kota
  2026-07-01 11:25   ` Konrad Dybcio
  2026-07-01 11:28   ` Bryan O'Donoghue
  2 siblings, 1 reply; 20+ messages in thread
From: sashiko-bot @ 2026-07-01 10:34 UTC (permalink / raw)
  To: Rakesh Kota; +Cc: conor+dt, devicetree, robh

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The macro `PM4125_VBOOST_CFG` is defined twice with conflicting values (`0x52` and `0x56`), causing the voltage selector to write to the wrong register and corrupt the OTG configuration.
--

commit 3778477cc88a05791ed34dc272e50cbdb4893643
Author: Rakesh Kota <rakesh.kota@oss.qualcomm.com>

regulator: qcom_usb_vbus: add support for qcom,pm4125-vbus-reg

The PM4125 PMIC uses a different register layout for USB VBUS control
compared to PM8150B. On PM4125, CMD_OTG is at offset 0x50, OTG_CFG is
at 0x56, and offset 0x52 is a 2-bit VBOOST voltage selector rather than
a current-limit selector. Introduce per-compatible regulator descriptor
data to accommodate these differences.

> diff --git a/drivers/regulator/qcom_usb_vbus-regulator.c b/drivers/regulator/qcom_usb_vbus-regulator.c
> index cd94ed67621fe..40e7e5589d5b4 100644
> --- a/drivers/regulator/qcom_usb_vbus-regulator.c
> +++ b/drivers/regulator/qcom_usb_vbus-regulator.c
> @@ -20,10 +20,35 @@
>  #define OTG_CFG				0x53
>  #define OTG_EN_SRC_CFG			BIT(1)
>  
> +#define PM4125_VBOOST_EN		0x50
> +#define PM4125_VBOOST_CFG		0x52
> +#define PM4125_VBOOST_CFG_MASK		GENMASK(1, 0)
> +#define PM4125_VBOOST_CFG		0x56

[Severity: High]
Will this redefine PM4125_VBOOST_CFG to 0x56 and silently override the
intended 0x52 offset?

When pm4125_data.vsel_reg is assigned PM4125_VBOOST_CFG, it appears the
regulator core will write to 0x56 instead of the 0x52 VBOOST selector
register.

Should the 0x56 definition be named PM4125_OTG_CFG instead?

> +#define PM4125_OTG_EN_SRC_CFG		BIT(0)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701-add_pm4125-vbus-reg-v2-0-6bac2bac7131@oss.qualcomm.com?part=2

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: qcom: Fix pm4125 vbus regulator compatible and constraints
  2026-07-01 10:28 ` [PATCH v2 3/3] arm64: dts: qcom: Fix pm4125 vbus regulator compatible and constraints Rakesh Kota
@ 2026-07-01 10:34   ` sashiko-bot
  2026-07-02  7:25     ` Rakesh Kota
  2026-07-01 11:26   ` Konrad Dybcio
  2026-07-01 11:29   ` Bryan O'Donoghue
  2 siblings, 1 reply; 20+ messages in thread
From: sashiko-bot @ 2026-07-01 10:34 UTC (permalink / raw)
  To: Rakesh Kota; +Cc: conor+dt, robh, devicetree

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The `PM4125_VBOOST_CFG` macro is defined twice in the driver with different values, causing the voltage selector address to be shadowed by the OTG configuration address. This leads to register corruption when applying the new device tree constraints.
--

commit ee5aeefd579f49a0fd9de6178216fb53f958a898
Author: Rakesh Kota <rakesh.kota@oss.qualcomm.com>

arm64: dts: qcom: Fix pm4125 vbus regulator compatible and constraints
    
Remove pm8150b fallback compatible from pm4125_vbus and fix regulator
constraints in qrb2210 DTS files to use microvolt instead of
microamp.

> diff --git a/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts b/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
> --- a/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
> +++ b/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
[ ... ]
> @@ -235,8 +235,8 @@ &mdss_dsi0_phy {
>  };
>  
>  &pm4125_vbus {
> -	regulator-min-microamp = <500000>;
> -	regulator-max-microamp = <500000>;
> +	regulator-min-microvolt = <4250000>;
> +	regulator-max-microvolt = <5000000>;

[Severity: High]
This is a pre-existing issue, but does adding these voltage constraints now
trigger a hardware register corruption in the regulator driver?

Looking at the driver in drivers/regulator/qcom_usb_vbus-regulator.c,
PM4125_VBOOST_CFG is defined twice with different values:

drivers/regulator/qcom_usb_vbus-regulator.c:
    #define PM4125_VBOOST_CFG		0x52
    #define PM4125_VBOOST_CFG_MASK		GENMASK(1, 0)
    #define PM4125_VBOOST_CFG		0x56

Because of this shadowing, the pm4125_data struct initializes both .otg_cfg
and .vsel_reg to 0x56:

drivers/regulator/qcom_usb_vbus-regulator.c:
    .otg_cfg = PM4125_VBOOST_CFG,
    .otg_en_src_cfg = PM4125_OTG_EN_SRC_CFG,
    .vsel_reg = PM4125_VBOOST_CFG,

With the new regulator-min-microvolt constraints added by this device tree
change, the regulator core will now call .set_voltage_sel on this regulator.

Will this cause the driver to incorrectly write the voltage selector mask
to the OTG config register at 0x56 instead of the intended voltage selector
register at 0x52, thereby overwriting OTG configuration bits?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701-add_pm4125-vbus-reg-v2-0-6bac2bac7131@oss.qualcomm.com?part=3

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/3] regulator: qcom_usb_vbus: add support for qcom,pm4125-vbus-reg
  2026-07-01 10:28 ` [PATCH v2 2/3] regulator: qcom_usb_vbus: add support for qcom,pm4125-vbus-reg Rakesh Kota
  2026-07-01 10:34   ` sashiko-bot
@ 2026-07-01 11:25   ` Konrad Dybcio
  2026-07-01 11:28   ` Bryan O'Donoghue
  2 siblings, 0 replies; 20+ messages in thread
From: Konrad Dybcio @ 2026-07-01 11:25 UTC (permalink / raw)
  To: Rakesh Kota, Liam Girdwood, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Wesley Cheng, Bjorn Andersson,
	Konrad Dybcio
  Cc: linux-arm-msm, linux-kernel, devicetree, jishnu.prakash,
	kamal.wadhwa

On 7/1/26 12:28 PM, Rakesh Kota wrote:
> The PM4125 PMIC uses a different register layout for USB VBUS control
> compared to PM8150B. On PM4125, CMD_OTG is at offset 0x50, OTG_CFG is
> at 0x56, and offset 0x52 is a 2-bit VBOOST voltage selector rather than
> a current-limit selector.
> 
> Introduce per-compatible regulator descriptor data to accommodate these
> differences. This keeps the existing PM8150B current-limit logic intact
> while adding a dedicated voltage-selector path for PM4125.
> 
> Signed-off-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: qcom: Fix pm4125 vbus regulator compatible and constraints
  2026-07-01 10:28 ` [PATCH v2 3/3] arm64: dts: qcom: Fix pm4125 vbus regulator compatible and constraints Rakesh Kota
  2026-07-01 10:34   ` sashiko-bot
@ 2026-07-01 11:26   ` Konrad Dybcio
  2026-07-02  7:33     ` Rakesh Kota
  2026-07-01 11:29   ` Bryan O'Donoghue
  2 siblings, 1 reply; 20+ messages in thread
From: Konrad Dybcio @ 2026-07-01 11:26 UTC (permalink / raw)
  To: Rakesh Kota, Liam Girdwood, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Wesley Cheng, Bjorn Andersson,
	Konrad Dybcio
  Cc: linux-arm-msm, linux-kernel, devicetree, jishnu.prakash,
	kamal.wadhwa

On 7/1/26 12:28 PM, Rakesh Kota wrote:
> Remove pm8150b fallback compatible from pm4125_vbus and fix regulator
> constraints in qrb2210 DTS files to use microvolt instead of
> microamp.
> 
> Signed-off-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/pm4125.dtsi               | 2 +-
>  arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts | 4 ++--
>  arch/arm64/boot/dts/qcom/qrb2210-rb1.dts           | 4 ++--
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/pm4125.dtsi b/arch/arm64/boot/dts/qcom/pm4125.dtsi
> index 542e8fe030da40dfefc7b744dcc30133fdb35e74..3dc8d667d091ceb4267081d32b7aae983fc25071 100644
> --- a/arch/arm64/boot/dts/qcom/pm4125.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pm4125.dtsi
> @@ -37,7 +37,7 @@ pm4125_resin: resin {
>  		};
>  
>  		pm4125_vbus: usb-vbus-regulator@1100 {
> -			compatible = "qcom,pm4125-vbus-reg", "qcom,pm8150b-vbus-reg";
> +			compatible = "qcom,pm4125-vbus-reg";
>  			reg = <0x1100>;
>  			status = "disabled";
>  		};
> diff --git a/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts b/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
> index bf088fa9807f040f0c8f405f9111b01790b09377..2b50f3f577577d30a87fcb6466c7d7690270aaa3 100644
> --- a/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
> +++ b/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
> @@ -235,8 +235,8 @@ &mdss_dsi0_phy {
>  };
>  
>  &pm4125_vbus {
> -	regulator-min-microamp = <500000>;
> -	regulator-max-microamp = <500000>;
> +	regulator-min-microvolt = <4250000>;
> +	regulator-max-microvolt = <5000000>;

Perhaps that's a stupid question, but is there any good reason to
not just keep it at 5V if enabled?

Konrad

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/3] regulator: qcom_usb_vbus: add support for qcom,pm4125-vbus-reg
  2026-07-01 10:28 ` [PATCH v2 2/3] regulator: qcom_usb_vbus: add support for qcom,pm4125-vbus-reg Rakesh Kota
  2026-07-01 10:34   ` sashiko-bot
  2026-07-01 11:25   ` Konrad Dybcio
@ 2026-07-01 11:28   ` Bryan O'Donoghue
  2026-07-02  7:51     ` Rakesh Kota
  2 siblings, 1 reply; 20+ messages in thread
From: Bryan O'Donoghue @ 2026-07-01 11:28 UTC (permalink / raw)
  To: Rakesh Kota, Liam Girdwood, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Wesley Cheng, Bjorn Andersson,
	Konrad Dybcio
  Cc: linux-arm-msm, linux-kernel, devicetree, jishnu.prakash,
	kamal.wadhwa

On 01/07/2026 11:28, Rakesh Kota wrote:
> The PM4125 PMIC uses a different register layout for USB VBUS control
> compared to PM8150B. On PM4125, CMD_OTG is at offset 0x50, OTG_CFG is
> at 0x56, and offset 0x52 is a 2-bit VBOOST voltage selector rather than
> a current-limit selector.
> 
> Introduce per-compatible regulator descriptor data to accommodate these
> differences. This keeps the existing PM8150B current-limit logic intact
> while adding a dedicated voltage-selector path for PM4125.
> 
> Signed-off-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
> ---
>   drivers/regulator/qcom_usb_vbus-regulator.c | 102 ++++++++++++++++++++++++----
>   1 file changed, 88 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/regulator/qcom_usb_vbus-regulator.c b/drivers/regulator/qcom_usb_vbus-regulator.c
> index cd94ed67621fee9f6d7a0327054db0ebab6cc7ee..40e7e5589d5b499b2e84a00b2cfa754fba7e2f0f 100644
> --- a/drivers/regulator/qcom_usb_vbus-regulator.c
> +++ b/drivers/regulator/qcom_usb_vbus-regulator.c
> @@ -20,10 +20,35 @@
>   #define OTG_CFG				0x53
>   #define OTG_EN_SRC_CFG			BIT(1)
> 
> +#define PM4125_VBOOST_EN		0x50
> +#define PM4125_VBOOST_CFG		0x52
> +#define PM4125_VBOOST_CFG_MASK		GENMASK(1, 0)
> +#define PM4125_VBOOST_CFG		0x56
> +#define PM4125_OTG_EN_SRC_CFG		BIT(0)
> +
> +struct qcom_usb_vbus_reg_data {
> +	u16 cmd_otg;
> +	u16 otg_cfg;
> +	u8  otg_en_src_cfg;
> +	u16 csel_reg;
> +	u8 csel_mask;
> +	const unsigned int *curr_table;
> +	unsigned int n_current_limits;
> +	u16 vsel_reg;
> +	u8 vsel_mask;
> +	const unsigned int *volt_table;
> +	unsigned int n_voltages;
> +	const struct regulator_ops *ops;
> +};
> +
>   static const unsigned int curr_table[] = {
>   	500000, 1000000, 1500000, 2000000, 2500000, 3000000,
>   };
> 
> +static const unsigned int pm4125_vboost_table[] = {
> +	4250000, 4500000, 4750000, 5000000,
> +};

Should vboost_table be absent for pm8150b ?

> +
>   static const struct regulator_ops qcom_usb_vbus_reg_ops = {
>   	.enable = regulator_enable_regmap,
>   	.disable = regulator_disable_regmap,
> @@ -32,19 +57,43 @@ static const struct regulator_ops qcom_usb_vbus_reg_ops = {
>   	.set_current_limit = regulator_set_current_limit_regmap,
>   };
> 
> -static struct regulator_desc qcom_usb_vbus_rdesc = {
> -	.name = "usb_vbus",
> -	.ops = &qcom_usb_vbus_reg_ops,
> -	.owner = THIS_MODULE,
> -	.type = REGULATOR_VOLTAGE,
> +static const struct regulator_ops qcom_usb_vbus_pm4125_reg_ops = {
> +	.enable = regulator_enable_regmap,
> +	.disable = regulator_disable_regmap,
> +	.is_enabled = regulator_is_enabled_regmap,
> +	.get_voltage_sel = regulator_get_voltage_sel_regmap,
> +	.set_voltage_sel = regulator_set_voltage_sel_regmap,
> +	.list_voltage = regulator_list_voltage_table,
> +};
> +
> +static const struct qcom_usb_vbus_reg_data pm8150b_data = {
> +	.cmd_otg = CMD_OTG,
> +	.otg_cfg = OTG_CFG,
> +	.otg_en_src_cfg = OTG_EN_SRC_CFG,
> +	.csel_reg = OTG_CURRENT_LIMIT_CFG,
> +	.csel_mask = OTG_CURRENT_LIMIT_MASK,
>   	.curr_table = curr_table,
>   	.n_current_limits = ARRAY_SIZE(curr_table),
> +	.ops = &qcom_usb_vbus_reg_ops,
> +};

I think you should split this patch up

#1 introduces the abstraction for the existing device
#2 introduces the new PM4125

> +static const struct qcom_usb_vbus_reg_data pm4125_data = {
> +	.cmd_otg = PM4125_VBOOST_EN,
> +	.otg_cfg = PM4125_VBOOST_CFG,
> +	.otg_en_src_cfg = PM4125_OTG_EN_SRC_CFG,
> +	.vsel_reg = PM4125_VBOOST_CFG,
> +	.vsel_mask = PM4125_VBOOST_CFG_MASK,
> +	.volt_table = pm4125_vboost_table,
> +	.n_voltages = ARRAY_SIZE(pm4125_vboost_table),
> +	.ops = &qcom_usb_vbus_pm4125_reg_ops,
>   };
> 
>   static int qcom_usb_vbus_regulator_probe(struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;
> +	const struct qcom_usb_vbus_reg_data *data;
>   	struct regulator_dev *rdev;
> +	struct regulator_desc *rdesc;
>   	struct regmap *regmap;
>   	struct regulator_config config = { };
>   	struct regulator_init_data *init_data;
> @@ -57,27 +106,51 @@ static int qcom_usb_vbus_regulator_probe(struct platform_device *pdev)
>   		return ret;
>   	}
> 
> +	data = of_device_get_match_data(dev);
> +	if (!data)
> +		return -EINVAL;
> +
>   	regmap = dev_get_regmap(dev->parent, NULL);
>   	if (!regmap) {
>   		dev_err(dev, "Failed to get regmap\n");
>   		return -ENOENT;
>   	}
> 
> -	init_data = of_get_regulator_init_data(dev, dev->of_node,
> -					       &qcom_usb_vbus_rdesc);
> +	rdesc = devm_kzalloc(dev, sizeof(*rdesc), GFP_KERNEL);
> +	if (!rdesc)
> +		return -ENOMEM;
> +
> +	rdesc->name = "usb_vbus";
> +	rdesc->ops = data->ops;
> +	rdesc->owner = THIS_MODULE;
> +	rdesc->type = REGULATOR_VOLTAGE;
> +	rdesc->enable_reg = base + data->cmd_otg;
> +	rdesc->enable_mask = OTG_EN;
> +
> +	if (data->curr_table) {
> +		rdesc->curr_table = data->curr_table;
> +		rdesc->n_current_limits = data->n_current_limits;
> +		rdesc->csel_reg = base + data->csel_reg;
> +		rdesc->csel_mask = data->csel_mask;
> +	}
> +
> +	if (data->volt_table) {
> +		rdesc->volt_table = data->volt_table;
> +		rdesc->n_voltages = data->n_voltages;
> +		rdesc->vsel_reg = base + data->vsel_reg;
> +		rdesc->vsel_mask = data->vsel_mask;
> +	}
> +
> +	init_data = of_get_regulator_init_data(dev, dev->of_node, rdesc);
>   	if (!init_data)
>   		return -ENOMEM;
> 
> -	qcom_usb_vbus_rdesc.enable_reg = base + CMD_OTG;
> -	qcom_usb_vbus_rdesc.enable_mask = OTG_EN;
> -	qcom_usb_vbus_rdesc.csel_reg = base + OTG_CURRENT_LIMIT_CFG;
> -	qcom_usb_vbus_rdesc.csel_mask = OTG_CURRENT_LIMIT_MASK;
>   	config.dev = dev;
>   	config.init_data = init_data;
>   	config.of_node = dev->of_node;
>   	config.regmap = regmap;
> 
> -	rdev = devm_regulator_register(dev, &qcom_usb_vbus_rdesc, &config);
> +	rdev = devm_regulator_register(dev, rdesc, &config);
>   	if (IS_ERR(rdev)) {
>   		ret = PTR_ERR(rdev);
>   		dev_err(dev, "not able to register vbus reg %d\n", ret);
> @@ -85,13 +158,14 @@ static int qcom_usb_vbus_regulator_probe(struct platform_device *pdev)
>   	}
> 
>   	/* Disable HW logic for VBUS enable */
> -	regmap_update_bits(regmap, base + OTG_CFG, OTG_EN_SRC_CFG, 0);
> +	regmap_update_bits(regmap, base + data->otg_cfg, data->otg_en_src_cfg, 0);
> 
>   	return 0;
>   }
> 
>   static const struct of_device_id qcom_usb_vbus_regulator_match[] = {
> -	{ .compatible = "qcom,pm8150b-vbus-reg" },
> +	{ .compatible = "qcom,pm8150b-vbus-reg", .data = &pm8150b_data },
> +	{ .compatible = "qcom,pm4125-vbus-reg", .data = &pm4125_data },
>   	{ }
>   };
>   MODULE_DEVICE_TABLE(of, qcom_usb_vbus_regulator_match);
> 
> --
> 2.34.1
> 
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: qcom: Fix pm4125 vbus regulator compatible and constraints
  2026-07-01 10:28 ` [PATCH v2 3/3] arm64: dts: qcom: Fix pm4125 vbus regulator compatible and constraints Rakesh Kota
  2026-07-01 10:34   ` sashiko-bot
  2026-07-01 11:26   ` Konrad Dybcio
@ 2026-07-01 11:29   ` Bryan O'Donoghue
  2026-07-02  8:02     ` Rakesh Kota
  2 siblings, 1 reply; 20+ messages in thread
From: Bryan O'Donoghue @ 2026-07-01 11:29 UTC (permalink / raw)
  To: Rakesh Kota, Liam Girdwood, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Wesley Cheng, Bjorn Andersson,
	Konrad Dybcio
  Cc: linux-arm-msm, linux-kernel, devicetree, jishnu.prakash,
	kamal.wadhwa

On 01/07/2026 11:28, Rakesh Kota wrote:
> Remove pm8150b fallback compatible from pm4125_vbus and fix regulator
> constraints in qrb2210 DTS files to use microvolt instead of
> microamp.

Break this patch up at the and

- Remove compatible
- Fix the voltages

Separate things should be separate patches.

---
bod

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: qcom: Fix pm4125 vbus regulator compatible and constraints
  2026-07-01 10:34   ` sashiko-bot
@ 2026-07-02  7:25     ` Rakesh Kota
  0 siblings, 0 replies; 20+ messages in thread
From: Rakesh Kota @ 2026-07-02  7:25 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: conor+dt, robh, devicetree, jishnu.prakash, kamal.wadhwa

On Wed, Jul 01, 2026 at 10:34:56AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The `PM4125_VBOOST_CFG` macro is defined twice in the driver with different values, causing the voltage selector address to be shadowed by the OTG configuration address. This leads to register corruption when applying the new device tree constraints.
> --
> 
> commit ee5aeefd579f49a0fd9de6178216fb53f958a898
> Author: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
> 
> arm64: dts: qcom: Fix pm4125 vbus regulator compatible and constraints
>     
> Remove pm8150b fallback compatible from pm4125_vbus and fix regulator
> constraints in qrb2210 DTS files to use microvolt instead of
> microamp.
> 
> > diff --git a/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts b/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
> > --- a/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
> > +++ b/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
> [ ... ]
> > @@ -235,8 +235,8 @@ &mdss_dsi0_phy {
> >  };
> >  
> >  &pm4125_vbus {
> > -	regulator-min-microamp = <500000>;
> > -	regulator-max-microamp = <500000>;
> > +	regulator-min-microvolt = <4250000>;
> > +	regulator-max-microvolt = <5000000>;
> 
> [Severity: High]
> This is a pre-existing issue, but does adding these voltage constraints now
> trigger a hardware register corruption in the regulator driver?
> 
> Looking at the driver in drivers/regulator/qcom_usb_vbus-regulator.c,
> PM4125_VBOOST_CFG is defined twice with different values:
> 
> drivers/regulator/qcom_usb_vbus-regulator.c:
>     #define PM4125_VBOOST_CFG		0x52
>     #define PM4125_VBOOST_CFG_MASK		GENMASK(1, 0)
>     #define PM4125_VBOOST_CFG		0x56
> 
> Because of this shadowing, the pm4125_data struct initializes both .otg_cfg
> and .vsel_reg to 0x56:
>
> drivers/regulator/qcom_usb_vbus-regulator.c:
>     .otg_cfg = PM4125_VBOOST_CFG,
>     .otg_en_src_cfg = PM4125_OTG_EN_SRC_CFG,
>     .vsel_reg = PM4125_VBOOST_CFG,
> 
> With the new regulator-min-microvolt constraints added by this device tree
> change, the regulator core will now call .set_voltage_sel on this regulator.
> 
> Will this cause the driver to incorrectly write the voltage selector mask
> to the OTG config register at 0x56 instead of the intended voltage selector
> register at 0x52, thereby overwriting OTG configuration bits?
> 
Yes, you're correct, I will fix in the next patch by splitting the defines:

#define PM4125_VBOOST_OTG       0x50
#define PM4125_VBOOST_SEL       0x52
#define PM4125_VBOOST_CFG_MASK  GENMASK(1, 0)
#define PM4125_VBOOST_CFG       0x56

And updating pm4125_data to use PM4125_VBOOST_SEL for .vsel_reg so
.set_voltage_sel writes to the correct register.

regards
Rakesh Kota

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: qcom: Fix pm4125 vbus regulator compatible and constraints
  2026-07-01 11:26   ` Konrad Dybcio
@ 2026-07-02  7:33     ` Rakesh Kota
  2026-07-02 12:07       ` Konrad Dybcio
  0 siblings, 1 reply; 20+ messages in thread
From: Rakesh Kota @ 2026-07-02  7:33 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Wesley Cheng, Bjorn Andersson, Konrad Dybcio,
	linux-arm-msm, linux-kernel, devicetree, jishnu.prakash,
	kamal.wadhwa

On Wed, Jul 01, 2026 at 01:26:11PM +0200, Konrad Dybcio wrote:
> On 7/1/26 12:28 PM, Rakesh Kota wrote:
> > Remove pm8150b fallback compatible from pm4125_vbus and fix regulator
> > constraints in qrb2210 DTS files to use microvolt instead of
> > microamp.
> > 
> > Signed-off-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
> > ---
> >  arch/arm64/boot/dts/qcom/pm4125.dtsi               | 2 +-
> >  arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts | 4 ++--
> >  arch/arm64/boot/dts/qcom/qrb2210-rb1.dts           | 4 ++--
> >  3 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/pm4125.dtsi b/arch/arm64/boot/dts/qcom/pm4125.dtsi
> > index 542e8fe030da40dfefc7b744dcc30133fdb35e74..3dc8d667d091ceb4267081d32b7aae983fc25071 100644
> > --- a/arch/arm64/boot/dts/qcom/pm4125.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/pm4125.dtsi
> > @@ -37,7 +37,7 @@ pm4125_resin: resin {
> >  		};
> >  
> >  		pm4125_vbus: usb-vbus-regulator@1100 {
> > -			compatible = "qcom,pm4125-vbus-reg", "qcom,pm8150b-vbus-reg";
> > +			compatible = "qcom,pm4125-vbus-reg";
> >  			reg = <0x1100>;
> >  			status = "disabled";
> >  		};
> > diff --git a/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts b/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
> > index bf088fa9807f040f0c8f405f9111b01790b09377..2b50f3f577577d30a87fcb6466c7d7690270aaa3 100644
> > --- a/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
> > +++ b/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
> > @@ -235,8 +235,8 @@ &mdss_dsi0_phy {
> >  };
> >  
> >  &pm4125_vbus {
> > -	regulator-min-microamp = <500000>;
> > -	regulator-max-microamp = <500000>;
> > +	regulator-min-microvolt = <4250000>;
> > +	regulator-max-microvolt = <5000000>;
> 
> Perhaps that's a stupid question, but is there any good reason to
> not just keep it at 5V if enabled?
>
We could hardcode 5V, but keeping the range allows clients to select a
lower voltage if needed. The driver supports voltage selection per use
case, so hardcoding 5V would restrict that flexibility.

regard
Rakesh kota

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/3] regulator: qcom_usb_vbus: add support for qcom,pm4125-vbus-reg
  2026-07-01 11:28   ` Bryan O'Donoghue
@ 2026-07-02  7:51     ` Rakesh Kota
  0 siblings, 0 replies; 20+ messages in thread
From: Rakesh Kota @ 2026-07-02  7:51 UTC (permalink / raw)
  To: Bryan O'Donoghue
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Wesley Cheng, Bjorn Andersson, Konrad Dybcio,
	linux-arm-msm, linux-kernel, devicetree, jishnu.prakash,
	kamal.wadhwa

On Wed, Jul 01, 2026 at 12:28:00PM +0100, Bryan O'Donoghue wrote:
> On 01/07/2026 11:28, Rakesh Kota wrote:
> > The PM4125 PMIC uses a different register layout for USB VBUS control
> > compared to PM8150B. On PM4125, CMD_OTG is at offset 0x50, OTG_CFG is
> > at 0x56, and offset 0x52 is a 2-bit VBOOST voltage selector rather than
> > a current-limit selector.
> > 
> > Introduce per-compatible regulator descriptor data to accommodate these
> > differences. This keeps the existing PM8150B current-limit logic intact
> > while adding a dedicated voltage-selector path for PM4125.
> > 
> > Signed-off-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
> > ---
> >   drivers/regulator/qcom_usb_vbus-regulator.c | 102 ++++++++++++++++++++++++----
> >   1 file changed, 88 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/regulator/qcom_usb_vbus-regulator.c b/drivers/regulator/qcom_usb_vbus-regulator.c
> > index cd94ed67621fee9f6d7a0327054db0ebab6cc7ee..40e7e5589d5b499b2e84a00b2cfa754fba7e2f0f 100644
> > --- a/drivers/regulator/qcom_usb_vbus-regulator.c
> > +++ b/drivers/regulator/qcom_usb_vbus-regulator.c
> > @@ -20,10 +20,35 @@
> >   #define OTG_CFG				0x53
> >   #define OTG_EN_SRC_CFG			BIT(1)
> > 
> > +#define PM4125_VBOOST_EN		0x50
> > +#define PM4125_VBOOST_CFG		0x52
> > +#define PM4125_VBOOST_CFG_MASK		GENMASK(1, 0)
> > +#define PM4125_VBOOST_CFG		0x56
> > +#define PM4125_OTG_EN_SRC_CFG		BIT(0)
> > +
> > +struct qcom_usb_vbus_reg_data {
> > +	u16 cmd_otg;
> > +	u16 otg_cfg;
> > +	u8  otg_en_src_cfg;
> > +	u16 csel_reg;
> > +	u8 csel_mask;
> > +	const unsigned int *curr_table;
> > +	unsigned int n_current_limits;
> > +	u16 vsel_reg;
> > +	u8 vsel_mask;
> > +	const unsigned int *volt_table;
> > +	unsigned int n_voltages;
> > +	const struct regulator_ops *ops;
> > +};
> > +
> >   static const unsigned int curr_table[] = {
> >   	500000, 1000000, 1500000, 2000000, 2500000, 3000000,
> >   };
> > 
> > +static const unsigned int pm4125_vboost_table[] = {
> > +	4250000, 4500000, 4750000, 5000000,
> > +};
> 
> Should vboost_table be absent for pm8150b ?
>
Yes, vboost_table not supported for pm8150b.

> > +
> >   static const struct regulator_ops qcom_usb_vbus_reg_ops = {
> >   	.enable = regulator_enable_regmap,
> >   	.disable = regulator_disable_regmap,
> > @@ -32,19 +57,43 @@ static const struct regulator_ops qcom_usb_vbus_reg_ops = {
> >   	.set_current_limit = regulator_set_current_limit_regmap,
> >   };
> > 
> > -static struct regulator_desc qcom_usb_vbus_rdesc = {
> > -	.name = "usb_vbus",
> > -	.ops = &qcom_usb_vbus_reg_ops,
> > -	.owner = THIS_MODULE,
> > -	.type = REGULATOR_VOLTAGE,
> > +static const struct regulator_ops qcom_usb_vbus_pm4125_reg_ops = {
> > +	.enable = regulator_enable_regmap,
> > +	.disable = regulator_disable_regmap,
> > +	.is_enabled = regulator_is_enabled_regmap,
> > +	.get_voltage_sel = regulator_get_voltage_sel_regmap,
> > +	.set_voltage_sel = regulator_set_voltage_sel_regmap,
> > +	.list_voltage = regulator_list_voltage_table,
> > +};
> > +
> > +static const struct qcom_usb_vbus_reg_data pm8150b_data = {
> > +	.cmd_otg = CMD_OTG,
> > +	.otg_cfg = OTG_CFG,
> > +	.otg_en_src_cfg = OTG_EN_SRC_CFG,
> > +	.csel_reg = OTG_CURRENT_LIMIT_CFG,
> > +	.csel_mask = OTG_CURRENT_LIMIT_MASK,
> >   	.curr_table = curr_table,
> >   	.n_current_limits = ARRAY_SIZE(curr_table),
> > +	.ops = &qcom_usb_vbus_reg_ops,
> > +};
> 
> I think you should split this patch up
> 
> #1 introduces the abstraction for the existing device
> #2 introduces the new PM4125
>
Splitting sounds clean, but the issue is that PM4125 currently falls
back to PM8150B in existing DTS. If we split, patch #1 would break
existing PM4125 DTS users before patch #2 lands. We also can't split the
.yaml changes across two patches. So keeping it as one patch avoids any
intermediate breakage.

If you have a specific suggestion on how this could be split cleanly
while avoiding the above concerns, we would be happy to consider and
incorporate your feedback.

regards
Rakesh kota

> > +static const struct qcom_usb_vbus_reg_data pm4125_data = {
> > +	.cmd_otg = PM4125_VBOOST_EN,
> > +	.otg_cfg = PM4125_VBOOST_CFG,
> > +	.otg_en_src_cfg = PM4125_OTG_EN_SRC_CFG,
> > +	.vsel_reg = PM4125_VBOOST_CFG,
> > +	.vsel_mask = PM4125_VBOOST_CFG_MASK,
> > +	.volt_table = pm4125_vboost_table,
> > +	.n_voltages = ARRAY_SIZE(pm4125_vboost_table),
> > +	.ops = &qcom_usb_vbus_pm4125_reg_ops,
> >   };
> > 
> >   static int qcom_usb_vbus_regulator_probe(struct platform_device *pdev)
> >   {
> >   	struct device *dev = &pdev->dev;
> > +	const struct qcom_usb_vbus_reg_data *data;
> >   	struct regulator_dev *rdev;
> > +	struct regulator_desc *rdesc;
> >   	struct regmap *regmap;
> >   	struct regulator_config config = { };
> >   	struct regulator_init_data *init_data;
> > @@ -57,27 +106,51 @@ static int qcom_usb_vbus_regulator_probe(struct platform_device *pdev)
> >   		return ret;
> >   	}
> > 
> > +	data = of_device_get_match_data(dev);
> > +	if (!data)
> > +		return -EINVAL;
> > +
> >   	regmap = dev_get_regmap(dev->parent, NULL);
> >   	if (!regmap) {
> >   		dev_err(dev, "Failed to get regmap\n");
> >   		return -ENOENT;
> >   	}
> > 
> > -	init_data = of_get_regulator_init_data(dev, dev->of_node,
> > -					       &qcom_usb_vbus_rdesc);
> > +	rdesc = devm_kzalloc(dev, sizeof(*rdesc), GFP_KERNEL);
> > +	if (!rdesc)
> > +		return -ENOMEM;
> > +
> > +	rdesc->name = "usb_vbus";
> > +	rdesc->ops = data->ops;
> > +	rdesc->owner = THIS_MODULE;
> > +	rdesc->type = REGULATOR_VOLTAGE;
> > +	rdesc->enable_reg = base + data->cmd_otg;
> > +	rdesc->enable_mask = OTG_EN;
> > +
> > +	if (data->curr_table) {
> > +		rdesc->curr_table = data->curr_table;
> > +		rdesc->n_current_limits = data->n_current_limits;
> > +		rdesc->csel_reg = base + data->csel_reg;
> > +		rdesc->csel_mask = data->csel_mask;
> > +	}
> > +
> > +	if (data->volt_table) {
> > +		rdesc->volt_table = data->volt_table;
> > +		rdesc->n_voltages = data->n_voltages;
> > +		rdesc->vsel_reg = base + data->vsel_reg;
> > +		rdesc->vsel_mask = data->vsel_mask;
> > +	}
> > +
> > +	init_data = of_get_regulator_init_data(dev, dev->of_node, rdesc);
> >   	if (!init_data)
> >   		return -ENOMEM;
> > 
> > -	qcom_usb_vbus_rdesc.enable_reg = base + CMD_OTG;
> > -	qcom_usb_vbus_rdesc.enable_mask = OTG_EN;
> > -	qcom_usb_vbus_rdesc.csel_reg = base + OTG_CURRENT_LIMIT_CFG;
> > -	qcom_usb_vbus_rdesc.csel_mask = OTG_CURRENT_LIMIT_MASK;
> >   	config.dev = dev;
> >   	config.init_data = init_data;
> >   	config.of_node = dev->of_node;
> >   	config.regmap = regmap;
> > 
> > -	rdev = devm_regulator_register(dev, &qcom_usb_vbus_rdesc, &config);
> > +	rdev = devm_regulator_register(dev, rdesc, &config);
> >   	if (IS_ERR(rdev)) {
> >   		ret = PTR_ERR(rdev);
> >   		dev_err(dev, "not able to register vbus reg %d\n", ret);
> > @@ -85,13 +158,14 @@ static int qcom_usb_vbus_regulator_probe(struct platform_device *pdev)
> >   	}
> > 
> >   	/* Disable HW logic for VBUS enable */
> > -	regmap_update_bits(regmap, base + OTG_CFG, OTG_EN_SRC_CFG, 0);
> > +	regmap_update_bits(regmap, base + data->otg_cfg, data->otg_en_src_cfg, 0);
> > 
> >   	return 0;
> >   }
> > 
> >   static const struct of_device_id qcom_usb_vbus_regulator_match[] = {
> > -	{ .compatible = "qcom,pm8150b-vbus-reg" },
> > +	{ .compatible = "qcom,pm8150b-vbus-reg", .data = &pm8150b_data },
> > +	{ .compatible = "qcom,pm4125-vbus-reg", .data = &pm4125_data },
> >   	{ }
> >   };
> >   MODULE_DEVICE_TABLE(of, qcom_usb_vbus_regulator_match);
> > 
> > --
> > 2.34.1
> > 
> > 
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/3] regulator: qcom_usb_vbus: add support for qcom,pm4125-vbus-reg
  2026-07-01 10:34   ` sashiko-bot
@ 2026-07-02  7:54     ` Rakesh Kota
  2026-07-02 12:59       ` Dmitry Baryshkov
  0 siblings, 1 reply; 20+ messages in thread
From: Rakesh Kota @ 2026-07-02  7:54 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: conor+dt, devicetree, robh, jishnu.prakash, kamal.wadhwa

On Wed, Jul 01, 2026 at 10:34:35AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The macro `PM4125_VBOOST_CFG` is defined twice with conflicting values (`0x52` and `0x56`), causing the voltage selector to write to the wrong register and corrupt the OTG configuration.
> --
> 
> commit 3778477cc88a05791ed34dc272e50cbdb4893643
> Author: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
> 
> regulator: qcom_usb_vbus: add support for qcom,pm4125-vbus-reg
> 
> The PM4125 PMIC uses a different register layout for USB VBUS control
> compared to PM8150B. On PM4125, CMD_OTG is at offset 0x50, OTG_CFG is
> at 0x56, and offset 0x52 is a 2-bit VBOOST voltage selector rather than
> a current-limit selector. Introduce per-compatible regulator descriptor
> data to accommodate these differences.
> 
> > diff --git a/drivers/regulator/qcom_usb_vbus-regulator.c b/drivers/regulator/qcom_usb_vbus-regulator.c
> > index cd94ed67621fe..40e7e5589d5b4 100644
> > --- a/drivers/regulator/qcom_usb_vbus-regulator.c
> > +++ b/drivers/regulator/qcom_usb_vbus-regulator.c
> > @@ -20,10 +20,35 @@
> >  #define OTG_CFG				0x53
> >  #define OTG_EN_SRC_CFG			BIT(1)
> >  
> > +#define PM4125_VBOOST_EN		0x50
> > +#define PM4125_VBOOST_CFG		0x52
> > +#define PM4125_VBOOST_CFG_MASK		GENMASK(1, 0)
> > +#define PM4125_VBOOST_CFG		0x56
> 
> [Severity: High]
> Will this redefine PM4125_VBOOST_CFG to 0x56 and silently override the
> intended 0x52 offset?
> 
> When pm4125_data.vsel_reg is assigned PM4125_VBOOST_CFG, it appears the
> regulator core will write to 0x56 instead of the 0x52 VBOOST selector
> register.
> 
> Should the 0x56 definition be named PM4125_OTG_CFG instead?
>
Yes, you're correct, I will fix in the next patch by splitting the
defines:

#define PM4125_VBOOST_OTG_CFG   0x50
#define PM4125_VBOOST_SEL       0x52
#define PM4125_VBOOST_CFG_MASK  GENMASK(1, 0)
#define PM4125_VBOOST_CFG       0x56

regards
Rakesh Kota

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: qcom: Fix pm4125 vbus regulator compatible and constraints
  2026-07-01 11:29   ` Bryan O'Donoghue
@ 2026-07-02  8:02     ` Rakesh Kota
  2026-07-02 16:10       ` Bryan O'Donoghue
  0 siblings, 1 reply; 20+ messages in thread
From: Rakesh Kota @ 2026-07-02  8:02 UTC (permalink / raw)
  To: Bryan O'Donoghue
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Wesley Cheng, Bjorn Andersson, Konrad Dybcio,
	linux-arm-msm, linux-kernel, devicetree, jishnu.prakash,
	kamal.wadhwa

On Wed, Jul 01, 2026 at 12:29:35PM +0100, Bryan O'Donoghue wrote:
> On 01/07/2026 11:28, Rakesh Kota wrote:
> > Remove pm8150b fallback compatible from pm4125_vbus and fix regulator
> > constraints in qrb2210 DTS files to use microvolt instead of
> > microamp.
> 
> Break this patch up at the and
> 
> - Remove compatible
> - Fix the voltages
> 
> Separate things should be separate patches.
>
Thanks for the review.

We kept these together to avoid intermediate breakage since the .yaml
compatible removal and voltage fix are coupled. Happy to split if you
have a suggestion on how to do so cleanly.

regards
Rakesh Kota

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: qcom: Fix pm4125 vbus regulator compatible and constraints
  2026-07-02  7:33     ` Rakesh Kota
@ 2026-07-02 12:07       ` Konrad Dybcio
  2026-07-02 12:43         ` Rakesh Kota
  0 siblings, 1 reply; 20+ messages in thread
From: Konrad Dybcio @ 2026-07-02 12:07 UTC (permalink / raw)
  To: Rakesh Kota
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Wesley Cheng, Bjorn Andersson, Konrad Dybcio,
	linux-arm-msm, linux-kernel, devicetree, jishnu.prakash,
	kamal.wadhwa

On 7/2/26 9:33 AM, Rakesh Kota wrote:
> On Wed, Jul 01, 2026 at 01:26:11PM +0200, Konrad Dybcio wrote:
>> On 7/1/26 12:28 PM, Rakesh Kota wrote:
>>> Remove pm8150b fallback compatible from pm4125_vbus and fix regulator
>>> constraints in qrb2210 DTS files to use microvolt instead of
>>> microamp.
>>>
>>> Signed-off-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
>>> ---
>>>  arch/arm64/boot/dts/qcom/pm4125.dtsi               | 2 +-
>>>  arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts | 4 ++--
>>>  arch/arm64/boot/dts/qcom/qrb2210-rb1.dts           | 4 ++--
>>>  3 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/pm4125.dtsi b/arch/arm64/boot/dts/qcom/pm4125.dtsi
>>> index 542e8fe030da40dfefc7b744dcc30133fdb35e74..3dc8d667d091ceb4267081d32b7aae983fc25071 100644
>>> --- a/arch/arm64/boot/dts/qcom/pm4125.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/pm4125.dtsi
>>> @@ -37,7 +37,7 @@ pm4125_resin: resin {
>>>  		};
>>>  
>>>  		pm4125_vbus: usb-vbus-regulator@1100 {
>>> -			compatible = "qcom,pm4125-vbus-reg", "qcom,pm8150b-vbus-reg";
>>> +			compatible = "qcom,pm4125-vbus-reg";
>>>  			reg = <0x1100>;
>>>  			status = "disabled";
>>>  		};
>>> diff --git a/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts b/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
>>> index bf088fa9807f040f0c8f405f9111b01790b09377..2b50f3f577577d30a87fcb6466c7d7690270aaa3 100644
>>> --- a/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
>>> +++ b/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
>>> @@ -235,8 +235,8 @@ &mdss_dsi0_phy {
>>>  };
>>>  
>>>  &pm4125_vbus {
>>> -	regulator-min-microamp = <500000>;
>>> -	regulator-max-microamp = <500000>;
>>> +	regulator-min-microvolt = <4250000>;
>>> +	regulator-max-microvolt = <5000000>;
>>
>> Perhaps that's a stupid question, but is there any good reason to
>> not just keep it at 5V if enabled?
>>
> We could hardcode 5V, but keeping the range allows clients to select a
> lower voltage if needed. The driver supports voltage selection per use
> case, so hardcoding 5V would restrict that flexibility.

I'm trying to understand if this would ever happen in practice

Konrad

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: qcom: Fix pm4125 vbus regulator compatible and constraints
  2026-07-02 12:07       ` Konrad Dybcio
@ 2026-07-02 12:43         ` Rakesh Kota
  2026-07-02 13:01           ` Dmitry Baryshkov
  0 siblings, 1 reply; 20+ messages in thread
From: Rakesh Kota @ 2026-07-02 12:43 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Wesley Cheng, Bjorn Andersson, Konrad Dybcio,
	linux-arm-msm, linux-kernel, devicetree, jishnu.prakash,
	kamal.wadhwa

On Thu, Jul 02, 2026 at 02:07:12PM +0200, Konrad Dybcio wrote:
> On 7/2/26 9:33 AM, Rakesh Kota wrote:
> > On Wed, Jul 01, 2026 at 01:26:11PM +0200, Konrad Dybcio wrote:
> >> On 7/1/26 12:28 PM, Rakesh Kota wrote:
> >>> Remove pm8150b fallback compatible from pm4125_vbus and fix regulator
> >>> constraints in qrb2210 DTS files to use microvolt instead of
> >>> microamp.
> >>>
> >>> Signed-off-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
> >>> ---
> >>>  arch/arm64/boot/dts/qcom/pm4125.dtsi               | 2 +-
> >>>  arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts | 4 ++--
> >>>  arch/arm64/boot/dts/qcom/qrb2210-rb1.dts           | 4 ++--
> >>>  3 files changed, 5 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/qcom/pm4125.dtsi b/arch/arm64/boot/dts/qcom/pm4125.dtsi
> >>> index 542e8fe030da40dfefc7b744dcc30133fdb35e74..3dc8d667d091ceb4267081d32b7aae983fc25071 100644
> >>> --- a/arch/arm64/boot/dts/qcom/pm4125.dtsi
> >>> +++ b/arch/arm64/boot/dts/qcom/pm4125.dtsi
> >>> @@ -37,7 +37,7 @@ pm4125_resin: resin {
> >>>  		};
> >>>  
> >>>  		pm4125_vbus: usb-vbus-regulator@1100 {
> >>> -			compatible = "qcom,pm4125-vbus-reg", "qcom,pm8150b-vbus-reg";
> >>> +			compatible = "qcom,pm4125-vbus-reg";
> >>>  			reg = <0x1100>;
> >>>  			status = "disabled";
> >>>  		};
> >>> diff --git a/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts b/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
> >>> index bf088fa9807f040f0c8f405f9111b01790b09377..2b50f3f577577d30a87fcb6466c7d7690270aaa3 100644
> >>> --- a/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
> >>> +++ b/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
> >>> @@ -235,8 +235,8 @@ &mdss_dsi0_phy {
> >>>  };
> >>>  
> >>>  &pm4125_vbus {
> >>> -	regulator-min-microamp = <500000>;
> >>> -	regulator-max-microamp = <500000>;
> >>> +	regulator-min-microvolt = <4250000>;
> >>> +	regulator-max-microvolt = <5000000>;
> >>
> >> Perhaps that's a stupid question, but is there any good reason to
> >> not just keep it at 5V if enabled?
> >>
> > We could hardcode 5V, but keeping the range allows clients to select a
> > lower voltage if needed. The driver supports voltage selection per use
> > case, so hardcoding 5V would restrict that flexibility.
> 
> I'm trying to understand if this would ever happen in practice
>
Fair point.The dependent client here is the one connecting the USB
device to the Host. We don't have a concrete example today, but the
voltage range is kept flexible to avoid restricting future use cases.
That said, if 5V is the agreed practical default, we're open to
hardcoding it for simplicity.

regards
Rakesh Kota

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/3] regulator: qcom_usb_vbus: add support for qcom,pm4125-vbus-reg
  2026-07-02  7:54     ` Rakesh Kota
@ 2026-07-02 12:59       ` Dmitry Baryshkov
  0 siblings, 0 replies; 20+ messages in thread
From: Dmitry Baryshkov @ 2026-07-02 12:59 UTC (permalink / raw)
  To: Rakesh Kota
  Cc: sashiko-reviews, conor+dt, devicetree, robh, jishnu.prakash,
	kamal.wadhwa

On Thu, Jul 02, 2026 at 01:24:42PM +0530, Rakesh Kota wrote:
> On Wed, Jul 01, 2026 at 10:34:35AM +0000, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [High] The macro `PM4125_VBOOST_CFG` is defined twice with conflicting values (`0x52` and `0x56`), causing the voltage selector to write to the wrong register and corrupt the OTG configuration.
> > --
> > 
> > commit 3778477cc88a05791ed34dc272e50cbdb4893643
> > Author: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
> > 
> > regulator: qcom_usb_vbus: add support for qcom,pm4125-vbus-reg
> > 
> > The PM4125 PMIC uses a different register layout for USB VBUS control
> > compared to PM8150B. On PM4125, CMD_OTG is at offset 0x50, OTG_CFG is
> > at 0x56, and offset 0x52 is a 2-bit VBOOST voltage selector rather than
> > a current-limit selector. Introduce per-compatible regulator descriptor
> > data to accommodate these differences.
> > 
> > > diff --git a/drivers/regulator/qcom_usb_vbus-regulator.c b/drivers/regulator/qcom_usb_vbus-regulator.c
> > > index cd94ed67621fe..40e7e5589d5b4 100644
> > > --- a/drivers/regulator/qcom_usb_vbus-regulator.c
> > > +++ b/drivers/regulator/qcom_usb_vbus-regulator.c
> > > @@ -20,10 +20,35 @@
> > >  #define OTG_CFG				0x53
> > >  #define OTG_EN_SRC_CFG			BIT(1)
> > >  
> > > +#define PM4125_VBOOST_EN		0x50
> > > +#define PM4125_VBOOST_CFG		0x52
> > > +#define PM4125_VBOOST_CFG_MASK		GENMASK(1, 0)
> > > +#define PM4125_VBOOST_CFG		0x56
> > 
> > [Severity: High]
> > Will this redefine PM4125_VBOOST_CFG to 0x56 and silently override the
> > intended 0x52 offset?
> > 
> > When pm4125_data.vsel_reg is assigned PM4125_VBOOST_CFG, it appears the
> > regulator core will write to 0x56 instead of the 0x52 VBOOST selector
> > register.
> > 
> > Should the 0x56 definition be named PM4125_OTG_CFG instead?
> >
> Yes, you're correct, I will fix in the next patch by splitting the
> defines:
> 
> #define PM4125_VBOOST_OTG_CFG   0x50
> #define PM4125_VBOOST_SEL       0x52
> #define PM4125_VBOOST_CFG_MASK  GENMASK(1, 0)
> #define PM4125_VBOOST_CFG       0x56

With the defines being fixed:


Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>



-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: qcom: Fix pm4125 vbus regulator compatible and constraints
  2026-07-02 12:43         ` Rakesh Kota
@ 2026-07-02 13:01           ` Dmitry Baryshkov
  0 siblings, 0 replies; 20+ messages in thread
From: Dmitry Baryshkov @ 2026-07-02 13:01 UTC (permalink / raw)
  To: Rakesh Kota
  Cc: Konrad Dybcio, Liam Girdwood, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Wesley Cheng, Bjorn Andersson,
	Konrad Dybcio, linux-arm-msm, linux-kernel, devicetree,
	jishnu.prakash, kamal.wadhwa

On Thu, Jul 02, 2026 at 06:13:31PM +0530, Rakesh Kota wrote:
> On Thu, Jul 02, 2026 at 02:07:12PM +0200, Konrad Dybcio wrote:
> > On 7/2/26 9:33 AM, Rakesh Kota wrote:
> > > On Wed, Jul 01, 2026 at 01:26:11PM +0200, Konrad Dybcio wrote:
> > >> On 7/1/26 12:28 PM, Rakesh Kota wrote:
> > >>> Remove pm8150b fallback compatible from pm4125_vbus and fix regulator
> > >>> constraints in qrb2210 DTS files to use microvolt instead of
> > >>> microamp.
> > >>>
> > >>> Signed-off-by: Rakesh Kota <rakesh.kota@oss.qualcomm.com>
> > >>> ---
> > >>>  arch/arm64/boot/dts/qcom/pm4125.dtsi               | 2 +-
> > >>>  arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts | 4 ++--
> > >>>  arch/arm64/boot/dts/qcom/qrb2210-rb1.dts           | 4 ++--
> > >>>  3 files changed, 5 insertions(+), 5 deletions(-)
> > >>>
> > >>> diff --git a/arch/arm64/boot/dts/qcom/pm4125.dtsi b/arch/arm64/boot/dts/qcom/pm4125.dtsi
> > >>> index 542e8fe030da40dfefc7b744dcc30133fdb35e74..3dc8d667d091ceb4267081d32b7aae983fc25071 100644
> > >>> --- a/arch/arm64/boot/dts/qcom/pm4125.dtsi
> > >>> +++ b/arch/arm64/boot/dts/qcom/pm4125.dtsi
> > >>> @@ -37,7 +37,7 @@ pm4125_resin: resin {
> > >>>  		};
> > >>>  
> > >>>  		pm4125_vbus: usb-vbus-regulator@1100 {
> > >>> -			compatible = "qcom,pm4125-vbus-reg", "qcom,pm8150b-vbus-reg";
> > >>> +			compatible = "qcom,pm4125-vbus-reg";
> > >>>  			reg = <0x1100>;
> > >>>  			status = "disabled";
> > >>>  		};
> > >>> diff --git a/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts b/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
> > >>> index bf088fa9807f040f0c8f405f9111b01790b09377..2b50f3f577577d30a87fcb6466c7d7690270aaa3 100644
> > >>> --- a/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
> > >>> +++ b/arch/arm64/boot/dts/qcom/qrb2210-arduino-imola.dts
> > >>> @@ -235,8 +235,8 @@ &mdss_dsi0_phy {
> > >>>  };
> > >>>  
> > >>>  &pm4125_vbus {
> > >>> -	regulator-min-microamp = <500000>;
> > >>> -	regulator-max-microamp = <500000>;
> > >>> +	regulator-min-microvolt = <4250000>;
> > >>> +	regulator-max-microvolt = <5000000>;
> > >>
> > >> Perhaps that's a stupid question, but is there any good reason to
> > >> not just keep it at 5V if enabled?
> > >>
> > > We could hardcode 5V, but keeping the range allows clients to select a
> > > lower voltage if needed. The driver supports voltage selection per use
> > > case, so hardcoding 5V would restrict that flexibility.
> > 
> > I'm trying to understand if this would ever happen in practice
> >
> Fair point.The dependent client here is the one connecting the USB
> device to the Host. We don't have a concrete example today, but the
> voltage range is kept flexible to avoid restricting future use cases.
> That said, if 5V is the agreed practical default, we're open to
> hardcoding it for simplicity.

Moreover, I think current configuration can end up with the regulator
being set to 4.25V and thus undervolting connected devices.

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] arm64: dts: qcom: Fix pm4125 vbus regulator compatible and constraints
  2026-07-02  8:02     ` Rakesh Kota
@ 2026-07-02 16:10       ` Bryan O'Donoghue
  0 siblings, 0 replies; 20+ messages in thread
From: Bryan O'Donoghue @ 2026-07-02 16:10 UTC (permalink / raw)
  To: Rakesh Kota
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Wesley Cheng, Bjorn Andersson, Konrad Dybcio,
	linux-arm-msm, linux-kernel, devicetree, jishnu.prakash,
	kamal.wadhwa

On 02/07/2026 09:02, Rakesh Kota wrote:
>> Break this patch up at the and
>>
>> - Remove compatible
>> - Fix the voltages
>>
>> Separate things should be separate patches.
>>
> Thanks for the review.
> 
> We kept these together to avoid intermediate breakage since the .yaml
> compatible removal and voltage fix are coupled. Happy to split if you
> have a suggestion on how to do so cleanly.
> 
> regards
> Rakesh Kota

You can add the register abstraction logic, then add the new compatible no ?

---
bod

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2026-07-02 16:10 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-01 10:28 [PATCH v2 0/3] regulator: qcom_usb_vbus: add pm4125 VBUS regulator support Rakesh Kota
2026-07-01 10:28 ` [PATCH v2 1/3] dt-bindings: regulator: qcom,usb-vbus-regulator: add qcom,pm4125-vbus-reg Rakesh Kota
2026-07-01 10:28 ` [PATCH v2 2/3] regulator: qcom_usb_vbus: add support for qcom,pm4125-vbus-reg Rakesh Kota
2026-07-01 10:34   ` sashiko-bot
2026-07-02  7:54     ` Rakesh Kota
2026-07-02 12:59       ` Dmitry Baryshkov
2026-07-01 11:25   ` Konrad Dybcio
2026-07-01 11:28   ` Bryan O'Donoghue
2026-07-02  7:51     ` Rakesh Kota
2026-07-01 10:28 ` [PATCH v2 3/3] arm64: dts: qcom: Fix pm4125 vbus regulator compatible and constraints Rakesh Kota
2026-07-01 10:34   ` sashiko-bot
2026-07-02  7:25     ` Rakesh Kota
2026-07-01 11:26   ` Konrad Dybcio
2026-07-02  7:33     ` Rakesh Kota
2026-07-02 12:07       ` Konrad Dybcio
2026-07-02 12:43         ` Rakesh Kota
2026-07-02 13:01           ` Dmitry Baryshkov
2026-07-01 11:29   ` Bryan O'Donoghue
2026-07-02  8:02     ` Rakesh Kota
2026-07-02 16:10       ` Bryan O'Donoghue

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox