[PATCH v3 0/2] Add support for SA8255P in the phy-qcom-snps-femto-v2 phy driver

[PATCH v3 0/2] Add support for SA8255P in the phy-qcom-snps-femto-v2 phy driver

[Mattijs Korpershoek]

The Qualcomm automotive SA8255p SoC relies on firmware to configure
platform resources, including clocks, interconnects and TLMM.
The driver requests resources operations over SCMI using power protocols.

The SCMI power protocol enables or disables resources like clocks,
interconnect paths, and TLMM (GPIOs) using runtime PM framework APIs,
such as resume/suspend, to control power on/off.

Power domain "core" controls the GDSC, resets, and TLMM (GPIOs)
whereas power domain "transfer" controls clocks and interconnects.

Patch 1 adds the binding for sa8255p. Similarly to what has been done
for I2C [1], this is a separate binding.

Patch 2 adds the driver change, which is based on a public vendor
patch from qualcomm [2].
I did the following changes on top of that:
* Use devm_add_action_or_reset() for dev_pm_domain_detach_list() instead
  of implementing a driver .remove()
* Changed power domain names to drop 'usb_' prefix
* Fixed various error handling paths
* Added suspend/resume support

This has been tested on next-20260728 with a Ride4 SA8775P board that
has SCMI firmware (which is compatible).

[1] https://lore.kernel.org/all/20260617-enable-i2c-on-sa8255p-v7-1-ad736dbeab57@oss.qualcomm.com/
[2] https://git.codelinaro.org/clo/le/meta-qti-auto-kernel/-/blob/LY.AU.0.1.0.r1-16800-gen4meta.0/recipes-kernel/linux/files/usb/0014-PENDING-phy-qcom-snps-femto-v2-Add-support-for-SA825.patch

Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
---
Changes in v3:
- Applied R-b from Krzysztof and Neil
- Link to v2: https://patch.msgid.link/20260831-phy-qcom-snps-femto-v2-8255-v2-0-dc6197ffde1b@kernel.org

Changes in v2:
- Move 'core' as first domain, before 'transfer' (Krzysztof)
- Reworded binding commit message to be more precise about the various
  domains ('core', 'transfer' and 'perf') (Krzysztof)
- Link to v1: https://patch.msgid.link/20260730-phy-qcom-snps-femto-v2-8255-v1-0-8b8bf5105a68@kernel.org

---
Mattijs Korpershoek (1):
      dt-bindings: phy: Add sa8255p high-speed USB PHY

Sriram Dash (1):
      phy: qcom-snps-femto-v2: Add support for SA8255P

 .../bindings/phy/qcom,sa8255p-usb-hs-phy.yaml      |  52 +++++++++
 drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c      | 127 +++++++++++++++++++--
 2 files changed, 170 insertions(+), 9 deletions(-)
---
base-commit: e30626823a406725ce29bc75cb8ec467d3e1e326
change-id: 20260729-phy-qcom-snps-femto-v2-8255-90ffe7674001

Best regards,
-- 
Mattijs Korpershoek <mkorpershoek@kernel.org>

[PATCH v3 1/2] dt-bindings: phy: Add sa8255p high-speed USB PHY

[Mattijs Korpershoek]

SA8255p platform abstracts resources such as resets, clocks and
regulators in firmware. The SCMI firmware exposes multiple protocols to
control such resources which are identified by protocol_id.

For this USB phy, 2 SCMI protocols are exposed by the firmware:
* power domain management protocol (protocol_id=0x11)
* performance domain management protocol (protocol_id=0x13)

The power domain has 2 indices: 'core' and 'transfer'.
* core controls the regulators, the clocks and the reset
* transfer only controls the clocks

For example, to power on/off the PHY, we should use the 'core' domain.
To disable data transfer and keeping power enabled, we should use the
'transfer' domain.

Add a new binding for the Qualcomm SA8255p Synopsys Femto High-Speed
USB PHY V2 found in SA8255P.

Note: the performance domain (0x13) exposes opaques values (0xa to 0x11)
for which we don't have any information. Since it is not mandatory
for the PHY to be functional, we have ommited it from the bindings.
We can document it (and make them part of the dt ABI) once we have more
information.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
---
 .../bindings/phy/qcom,sa8255p-usb-hs-phy.yaml      | 52 ++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/qcom,sa8255p-usb-hs-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sa8255p-usb-hs-phy.yaml
new file mode 100644
index 000000000000..fff474a8345d
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,sa8255p-usb-hs-phy.yaml
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/qcom,sa8255p-usb-hs-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SA8255p Synopsys Femto High-Speed USB PHY V2
+
+maintainers:
+  - Mattijs Korpershoek <mkorpershoek@kernel.org>
+
+properties:
+  compatible:
+    const: qcom,sa8255p-usb-hs-phy
+
+  reg:
+    maxItems: 1
+
+  "#phy-cells":
+    const: 0
+
+  power-domains:
+    maxItems: 2
+
+  power-domain-names:
+    items:
+      - const: core
+      - const: transfer
+
+required:
+  - compatible
+  - reg
+  - "#phy-cells"
+  - power-domains
+  - power-domain-names
+
+additionalProperties: false
+
+examples:
+  - |
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        phy@88e4000 {
+            compatible = "qcom,sa8255p-usb-hs-phy";
+            reg = <0x0 0x088e4000 0x0 0x120>;
+            #phy-cells = <0>;
+            power-domains = <&scmi12_pd 1>, <&scmi12_pd 0>;
+            power-domain-names = "core", "transfer";
+        };
+    };

-- 
2.55.0

[PATCH v3 2/2] phy: qcom-snps-femto-v2: Add support for SA8255P

[Mattijs Korpershoek]

From: Sriram Dash <quic_sriramd@quicinc.com>

The Qualcomm automotive SA8255p SoC relies on firmware to configure
platform resources, including clocks, interconnects and TLMM.
The driver requests resources operations over SCMI using power protocols.

The SCMI power protocol enables or disables resources like clocks,
interconnect paths, and TLMM (GPIOs) using runtime PM framework APIs,
such as resume/suspend, to control power on/off.

Power domain "core" controls the GDSC, resets, and TLMM (GPIOs)
whereas power domain "transfer" controls clocks and interconnects.

In this variant, two disjoint sets of resources are modeled against
the power domain suppliers which can be turned on/off to achieve modeled
device states. Two modeled low power states d1 and d3 are supported.

transition: d0 <--> d3  :: both core and transfer suppliers
transition: d0 <--> d1  :: only transfer supplier

Signed-off-by: Sriram Dash <quic_sriramd@quicinc.com>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
---
 drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c | 127 ++++++++++++++++++++++++--
 1 file changed, 118 insertions(+), 9 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c b/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
index 980ad1fb1e2e..9fdf7ae681b6 100644
--- a/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
+++ b/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
@@ -12,6 +12,7 @@
 #include <linux/of.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
+#include <linux/pm_domain.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/reset.h>
@@ -95,6 +96,7 @@ struct override_param_map {
 	u8 table_size;
 	u8 reg_offset;
 	u8 param_mask;
+	bool fw_managed;
 };
 
 struct phy_override_seq {
@@ -121,6 +123,7 @@ struct phy_override_seq {
  * @phy_initialized: if PHY has been initialized correctly
  * @mode: contains the current mode the PHY is in
  * @update_seq_cfg: tuning parameters for phy init
+ * @pd_list: list of power domains associated with the device
  */
 struct qcom_snps_hsphy {
 	struct device *dev;
@@ -136,8 +139,81 @@ struct qcom_snps_hsphy {
 	bool phy_initialized;
 	enum phy_mode mode;
 	struct phy_override_seq update_seq_cfg[NUM_HSPHY_TUNING_PARAMS];
+
+	struct dev_pm_domain_list *pd_list;
 };
 
+static void qcom_snps_domain_detach(void *data)
+{
+	struct qcom_snps_hsphy *hsphy = data;
+
+	dev_pm_domain_detach_list(hsphy->pd_list);
+}
+
+static int qcom_snps_domain_attach(struct qcom_snps_hsphy *hsphy)
+{
+	struct dev_pm_domain_attach_data pd_data = {
+		.pd_flags       = PD_FLAG_NO_DEV_LINK,
+		.pd_names       = (const char*[]) { "core", "transfer" },
+		.num_pd_names   = 2,
+	};
+	struct device *dev = hsphy->dev;
+	int ret;
+
+	ret = dev_pm_domain_attach_list(dev, &pd_data, &hsphy->pd_list);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "domain attach failed\n");
+
+	return 0;
+}
+
+/* d3_to_d0 transition by turning on all the suppliers */
+static int qcom_snps_d3_to_d0(struct qcom_snps_hsphy *hsphy)
+{
+	int ret;
+
+	if (!hsphy->pd_list)
+		return 0;
+
+	ret = pm_runtime_resume_and_get(hsphy->pd_list->pd_devs[0]);
+	if (ret)
+		return ret;
+
+	ret = pm_runtime_resume_and_get(hsphy->pd_list->pd_devs[1]);
+	if (ret)
+		pm_runtime_put_sync(hsphy->pd_list->pd_devs[0]);
+
+	return ret;
+}
+
+/* d0_to_d3 transition by turning off all the suppliers */
+static void qcom_snps_d0_to_d3(struct qcom_snps_hsphy *hsphy)
+{
+	if (!hsphy->pd_list)
+		return;
+
+	pm_runtime_put_sync(hsphy->pd_list->pd_devs[1]);
+	pm_runtime_put_sync(hsphy->pd_list->pd_devs[0]);
+}
+
+/* d1_to_d0 transition by turning on the 'transfer' supplier */
+static int qcom_snps_d1_to_d0(struct qcom_snps_hsphy *hsphy)
+{
+	if (!hsphy->pd_list)
+		return 0;
+
+	return pm_runtime_resume_and_get(hsphy->pd_list->pd_devs[1]);
+}
+
+/* d0_to_d1 transition by turning off the 'transfer' supplier */
+static void qcom_snps_d0_to_d1(struct qcom_snps_hsphy *hsphy)
+{
+	if (!hsphy->pd_list)
+		return;
+
+	pm_runtime_put_sync(hsphy->pd_list->pd_devs[1]);
+}
+
 static int qcom_snps_hsphy_clk_init(struct qcom_snps_hsphy *hsphy)
 {
 	struct device *dev = hsphy->dev;
@@ -196,6 +272,8 @@ static int qcom_snps_hsphy_suspend(struct qcom_snps_hsphy *hsphy)
 					   0, USB2_AUTO_RESUME);
 	}
 
+	qcom_snps_d0_to_d1(hsphy);
+
 	return 0;
 }
 
@@ -203,7 +281,7 @@ static int qcom_snps_hsphy_resume(struct qcom_snps_hsphy *hsphy)
 {
 	dev_dbg(&hsphy->phy->dev, "Resume QCOM SNPS PHY, mode\n");
 
-	return 0;
+	return qcom_snps_d1_to_d0(hsphy);
 }
 
 static int __maybe_unused qcom_snps_hsphy_runtime_suspend(struct device *dev)
@@ -316,6 +394,10 @@ static const struct override_param ls_fs_output_impedance_sc7280[] = {
 	{ 1310, 0 },
 };
 
+static const struct override_param_map sa8255p_snps_hs_phy = {
+	.fw_managed = true,
+};
+
 static const struct override_param_map sc7280_snps_7nm_phy[] = {
 	{
 		"qcom,hs-disconnect-bp",
@@ -390,9 +472,15 @@ static int qcom_snps_hsphy_init(struct phy *phy)
 
 	dev_vdbg(&phy->dev, "%s(): Initializing SNPS HS phy\n", __func__);
 
+	ret = qcom_snps_d3_to_d0(hsphy);
+	if (ret < 0) {
+		dev_err(hsphy->dev, "Failed to transition to d0 state\n");
+		return ret;
+	}
+
 	ret = regulator_bulk_enable(ARRAY_SIZE(hsphy->vregs), hsphy->vregs);
 	if (ret)
-		return ret;
+		goto transition_to_d3;
 
 	ret = clk_bulk_prepare_enable(hsphy->num_clks, hsphy->clks);
 	if (ret) {
@@ -472,6 +560,8 @@ static int qcom_snps_hsphy_init(struct phy *phy)
 	clk_bulk_disable_unprepare(hsphy->num_clks, hsphy->clks);
 poweroff_phy:
 	regulator_bulk_disable(ARRAY_SIZE(hsphy->vregs), hsphy->vregs);
+transition_to_d3:
+	qcom_snps_d0_to_d3(hsphy);
 
 	return ret;
 }
@@ -485,6 +575,8 @@ static int qcom_snps_hsphy_exit(struct phy *phy)
 	regulator_bulk_disable(ARRAY_SIZE(hsphy->vregs), hsphy->vregs);
 	hsphy->phy_initialized = false;
 
+	qcom_snps_d0_to_d3(hsphy);
+
 	return 0;
 }
 
@@ -496,6 +588,10 @@ static const struct phy_ops qcom_snps_hsphy_gen_ops = {
 };
 
 static const struct of_device_id qcom_snps_hsphy_of_match_table[] = {
+	{
+		.compatible	= "qcom,sa8255p-usb-hs-phy",
+		.data		= &sa8255p_snps_hs_phy,
+	},
 	{ .compatible	= "qcom,sm8150-usb-hs-phy", },
 	{ .compatible	= "qcom,usb-snps-hs-5nm-phy", },
 	{
@@ -565,6 +661,7 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct qcom_snps_hsphy *hsphy;
+	const struct override_param_map *cfg;
 	struct phy_provider *phy_provider;
 	struct phy *generic_phy;
 	int ret, i;
@@ -580,14 +677,26 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev)
 	if (IS_ERR(hsphy->base))
 		return PTR_ERR(hsphy->base);
 
-	ret = qcom_snps_hsphy_clk_init(hsphy);
-	if (ret)
-		return dev_err_probe(dev, ret, "failed to initialize clocks\n");
+	cfg = of_device_get_match_data(dev);
+
+	if (cfg && cfg->fw_managed) {
+		ret = qcom_snps_domain_attach(hsphy);
+		if (ret)
+			return ret;
+
+		ret = devm_add_action_or_reset(dev, qcom_snps_domain_detach, hsphy);
+		if (ret)
+			return ret;
+	} else {
+		ret = qcom_snps_hsphy_clk_init(hsphy);
+		if (ret)
+			return dev_err_probe(dev, ret, "failed to initialize clocks\n");
 
-	hsphy->phy_reset = devm_reset_control_get_exclusive(&pdev->dev, NULL);
-	if (IS_ERR(hsphy->phy_reset)) {
-		dev_err(dev, "failed to get phy core reset\n");
-		return PTR_ERR(hsphy->phy_reset);
+		hsphy->phy_reset = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+		if (IS_ERR(hsphy->phy_reset)) {
+			dev_err(dev, "failed to get phy core reset\n");
+			return PTR_ERR(hsphy->phy_reset);
+		}
 	}
 
 	num = ARRAY_SIZE(hsphy->vregs);

-- 
2.55.0

Re: [PATCH v3 0/2] Add support for SA8255P in the phy-qcom-snps-femto-v2 phy driver

[Mattijs Korpershoek]

Hi Mani,

On Fri, Oct 02, 2026 at 11:41, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:

> The Qualcomm automotive SA8255p SoC relies on firmware to configure
> platform resources, including clocks, interconnects and TLMM.
> The driver requests resources operations over SCMI using power protocols.
>
> The SCMI power protocol enables or disables resources like clocks,
> interconnect paths, and TLMM (GPIOs) using runtime PM framework APIs,
> such as resume/suspend, to control power on/off.
>
> Power domain "core" controls the GDSC, resets, and TLMM (GPIOs)
> whereas power domain "transfer" controls clocks and interconnects.
>
> Patch 1 adds the binding for sa8255p. Similarly to what has been done
> for I2C [1], this is a separate binding.
>
> Patch 2 adds the driver change, which is based on a public vendor
> patch from qualcomm [2].
> I did the following changes on top of that:
> * Use devm_add_action_or_reset() for dev_pm_domain_detach_list() instead
>   of implementing a driver .remove()
> * Changed power domain names to drop 'usb_' prefix
> * Fixed various error handling paths
> * Added suspend/resume support
>
> This has been tested on next-20260728 with a Ride4 SA8775P board that
> has SCMI firmware (which is compatible).
>
> [1] https://lore.kernel.org/all/20260617-enable-i2c-on-sa8255p-v7-1-ad736dbeab57@oss.qualcomm.com/
> [2] https://git.codelinaro.org/clo/le/meta-qti-auto-kernel/-/blob/LY.AU.0.1.0.r1-16800-gen4meta.0/recipes-kernel/linux/files/usb/0014-PENDING-phy-qcom-snps-femto-v2-Add-support-for-SA825.patch
>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>

I don't know what happened but somehow I did not add you in To/Cc using
"b4 prep -c"

I've talked to Vinod at plumbers and he told me that I should request
some review on these phy patches.

Could you have a look whenever you have some time?

Thanks a lot for your help.
Mattijs

> ---
> Changes in v3:
> - Applied R-b from Krzysztof and Neil
> - Link to v2: https://patch.msgid.link/20260831-phy-qcom-snps-femto-v2-8255-v2-0-dc6197ffde1b@kernel.org
>
> Changes in v2:
> - Move 'core' as first domain, before 'transfer' (Krzysztof)
> - Reworded binding commit message to be more precise about the various
>   domains ('core', 'transfer' and 'perf') (Krzysztof)
> - Link to v1: https://patch.msgid.link/20260730-phy-qcom-snps-femto-v2-8255-v1-0-8b8bf5105a68@kernel.org
>
> ---
> Mattijs Korpershoek (1):
>       dt-bindings: phy: Add sa8255p high-speed USB PHY
>
> Sriram Dash (1):
>       phy: qcom-snps-femto-v2: Add support for SA8255P
>
>  .../bindings/phy/qcom,sa8255p-usb-hs-phy.yaml      |  52 +++++++++
>  drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c      | 127 +++++++++++++++++++--
>  2 files changed, 170 insertions(+), 9 deletions(-)
> ---
> base-commit: e30626823a406725ce29bc75cb8ec467d3e1e326
> change-id: 20260729-phy-qcom-snps-femto-v2-8255-90ffe7674001
>
> Best regards,
> -- 
> Mattijs Korpershoek <mkorpershoek@kernel.org>

Re: [PATCH v3 0/2] Add support for SA8255P in the phy-qcom-snps-femto-v2 phy driver

[Manivannan Sadhasivam]

On Thu, Oct 15, 2026 at 04:24:03PM +0200, Mattijs Korpershoek wrote:
> Hi Mani,
> 
> On Fri, Oct 02, 2026 at 11:41, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
> 
> > The Qualcomm automotive SA8255p SoC relies on firmware to configure
> > platform resources, including clocks, interconnects and TLMM.
> > The driver requests resources operations over SCMI using power protocols.
> >
> > The SCMI power protocol enables or disables resources like clocks,
> > interconnect paths, and TLMM (GPIOs) using runtime PM framework APIs,
> > such as resume/suspend, to control power on/off.
> >
> > Power domain "core" controls the GDSC, resets, and TLMM (GPIOs)
> > whereas power domain "transfer" controls clocks and interconnects.
> >
> > Patch 1 adds the binding for sa8255p. Similarly to what has been done
> > for I2C [1], this is a separate binding.
> >
> > Patch 2 adds the driver change, which is based on a public vendor
> > patch from qualcomm [2].
> > I did the following changes on top of that:
> > * Use devm_add_action_or_reset() for dev_pm_domain_detach_list() instead
> >   of implementing a driver .remove()
> > * Changed power domain names to drop 'usb_' prefix
> > * Fixed various error handling paths
> > * Added suspend/resume support
> >
> > This has been tested on next-20260728 with a Ride4 SA8775P board that
> > has SCMI firmware (which is compatible).
> >
> > [1] https://lore.kernel.org/all/20260617-enable-i2c-on-sa8255p-v7-1-ad736dbeab57@oss.qualcomm.com/
> > [2] https://git.codelinaro.org/clo/le/meta-qti-auto-kernel/-/blob/LY.AU.0.1.0.r1-16800-gen4meta.0/recipes-kernel/linux/files/usb/0014-PENDING-phy-qcom-snps-femto-v2-Add-support-for-SA825.patch
> >
> > Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
> 
> I don't know what happened but somehow I did not add you in To/Cc using
> "b4 prep -c"
>

No worries.
 
> I've talked to Vinod at plumbers and he told me that I should request
> some review on these phy patches.
> 
> Could you have a look whenever you have some time?
> 

Sure thing. This series is on my queue. Will get to it very soon.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v3 2/2] phy: qcom-snps-femto-v2: Add support for SA8255P

[Manivannan Sadhasivam]

On 2026-10-02 11:41 +0200, Mattijs Korpershoek wrote:
> From: Sriram Dash <quic_sriramd@quicinc.com>
> 
> The Qualcomm automotive SA8255p SoC relies on firmware to configure
> platform resources, including clocks, interconnects and TLMM.
> The driver requests resources operations over SCMI using power protocols.
> 
> The SCMI power protocol enables or disables resources like clocks,
> interconnect paths, and TLMM (GPIOs) using runtime PM framework APIs,
> such as resume/suspend, to control power on/off.
> 
> Power domain "core" controls the GDSC, resets, and TLMM (GPIOs)
> whereas power domain "transfer" controls clocks and interconnects.
> 

This is not consistent with other SCMI based drivers. Most of them are using
'power' and 'perf' power domains, not 'core', 'transfer'.

Why are these discrete powers domains be required? In my point of view, the PHY
should have only one power domain for controlling power and be done with it. If
you start having these fine grain controls over the resources from SCMI clients,
then it defeats the purpose of the SCMI based firmware abstraction design.

> In this variant, two disjoint sets of resources are modeled against
> the power domain suppliers which can be turned on/off to achieve modeled
> device states. Two modeled low power states d1 and d3 are supported.
> 
> transition: d0 <--> d3  :: both core and transfer suppliers
> transition: d0 <--> d1  :: only transfer supplier
> 

What are these transition levels and who defines these? We cannot have different
SCMI drivers inventing these kind of independent low power modes.

- Mani

Re: [PATCH v3 2/2] phy: qcom-snps-femto-v2: Add support for SA8255P

[Mattijs Korpershoek]

Hi Mani,

On Fri, Oct 16, 2026 at 15:20, Manivannan Sadhasivam <mani@kernel.org> wrote:

> On 2026-10-02 11:41 +0200, Mattijs Korpershoek wrote:
>> From: Sriram Dash <quic_sriramd@quicinc.com>
>> 
>> The Qualcomm automotive SA8255p SoC relies on firmware to configure
>> platform resources, including clocks, interconnects and TLMM.
>> The driver requests resources operations over SCMI using power protocols.
>> 
>> The SCMI power protocol enables or disables resources like clocks,
>> interconnect paths, and TLMM (GPIOs) using runtime PM framework APIs,
>> such as resume/suspend, to control power on/off.
>> 
>> Power domain "core" controls the GDSC, resets, and TLMM (GPIOs)
>> whereas power domain "transfer" controls clocks and interconnects.
>> 
>
> This is not consistent with other SCMI based drivers. Most of them are using
> 'power' and 'perf' power domains, not 'core', 'transfer'.

That's correct. Krzysztof had the same concerns [1] in the binding
reviews. To sum up from that thread:

For usb PHY, 2 SCMI protocols are exposed by the firmware:
* power domain management protocol (protocol_id=0x11)
* performance domain management protocol (protocol_id=0x13)

The power domain has 2 indices: 'core' and 'transfer'.
* core controls the regulators, the clocks and the reset
* transfer only controls the clocks

The performance domain (0x13) exposes opaques values (0xa to 0x11)
for which we don't have any information.

>
> Why are these discrete powers domains be required? In my point of view, the PHY
> should have only one power domain for controlling power and be done with it. If
> you start having these fine grain controls over the resources from SCMI clients,
> then it defeats the purpose of the SCMI based firmware abstraction design.

From what I understand, thse fine-grain controls are useful for runtime
suspend/resume (on the Linux side).
For example:
1. Enable 'core' power domain to enable the PHY
2. Disable 'transfer' to turn of the clocks. PHY is still powered but is
"idle"

If we look at the patch, we can see that this is what's done in
qcom_snps_hsphy_suspend().

Does that clarify things?

This is also explained in the binding [2]
Should I rephrase the commit message here to include that?

>
>> In this variant, two disjoint sets of resources are modeled against
>> the power domain suppliers which can be turned on/off to achieve modeled
>> device states. Two modeled low power states d1 and d3 are supported.
>> 
>> transition: d0 <--> d3  :: both core and transfer suppliers
>> transition: d0 <--> d1  :: only transfer supplier
>> 
>
> What are these transition levels and who defines these? We cannot have different
> SCMI drivers inventing these kind of independent low power modes.
>

Maybe the naming is not as good as it should be.
I took this originally from a downstream qcom patch and did not change
it.

I interpret this as:
* d3: phy is powered off, before probe (no regulators, no clocks and in reset mode)
* d1: phy is in lower power mode (regulator on, no clocks, no reset)
* d0: phy is powered on an operational (after probe)

I see that drivers/tty/serial/qcom_geni_serial.c uses a similar
mechanism. Should I try to follow that naming convention?

If you have other suggestions, I'm happy to get them.

Note that I can't change the power domains ('core' and 'transfer' since
those are enforced by the SCMI firmware)

Thanks again for the review
Mattijs

[1] https://lore.kernel.org/all/877blx2bga.fsf@kernel.org/
[2] https://lore.kernel.org/all/20261002-phy-qcom-snps-femto-v2-8255-v3-1-342542bd9bc6@kernel.org/

> - Mani

Re: [PATCH v3 2/2] phy: qcom-snps-femto-v2: Add support for SA8255P

[Manivannan Sadhasivam]

On 2026-10-16 15:14 +0200, Mattijs Korpershoek wrote:
> Hi Mani,
> 
> On Fri, Oct 16, 2026 at 15:20, Manivannan Sadhasivam <mani@kernel.org> wrote:
> 
> > On 2026-10-02 11:41 +0200, Mattijs Korpershoek wrote:
> >> From: Sriram Dash <quic_sriramd@quicinc.com>
> >> 
> >> The Qualcomm automotive SA8255p SoC relies on firmware to configure
> >> platform resources, including clocks, interconnects and TLMM.
> >> The driver requests resources operations over SCMI using power protocols.
> >> 
> >> The SCMI power protocol enables or disables resources like clocks,
> >> interconnect paths, and TLMM (GPIOs) using runtime PM framework APIs,
> >> such as resume/suspend, to control power on/off.
> >> 
> >> Power domain "core" controls the GDSC, resets, and TLMM (GPIOs)
> >> whereas power domain "transfer" controls clocks and interconnects.
> >> 
> >
> > This is not consistent with other SCMI based drivers. Most of them are using
> > 'power' and 'perf' power domains, not 'core', 'transfer'.
> 
> That's correct. Krzysztof had the same concerns [1] in the binding
> reviews. To sum up from that thread:
> 
> For usb PHY, 2 SCMI protocols are exposed by the firmware:
> * power domain management protocol (protocol_id=0x11)
> * performance domain management protocol (protocol_id=0x13)
> 
> The power domain has 2 indices: 'core' and 'transfer'.
> * core controls the regulators, the clocks and the reset
> * transfer only controls the clocks
> 

But this contradicts with your commit message:

"Power domain "core" controls the GDSC, resets, and TLMM (GPIOs)
whereas power domain "transfer" controls clocks and interconnects."

> The performance domain (0x13) exposes opaques values (0xa to 0x11)
> for which we don't have any information.
> 

It is a shame that it is being defined as opaque values, but as long as you
don't define it in the binding and not use it in the driver, I'm OK with it.

> >
> > Why are these discrete powers domains be required? In my point of view, the PHY
> > should have only one power domain for controlling power and be done with it. If
> > you start having these fine grain controls over the resources from SCMI clients,
> > then it defeats the purpose of the SCMI based firmware abstraction design.
> 
> From what I understand, thse fine-grain controls are useful for runtime
> suspend/resume (on the Linux side).
> For example:
> 1. Enable 'core' power domain to enable the PHY
> 2. Disable 'transfer' to turn of the clocks. PHY is still powered but is
> "idle"
> 
> If we look at the patch, we can see that this is what's done in
> qcom_snps_hsphy_suspend().
> 
> Does that clarify things?
> 

Even though the SCMI spec allows defining multiple sub-domains inside a
protocol, I'd argue that there should only be one power domain 'power'
controlling all resources such as clock, icc, resets etc... But in this case,
that ship has already sailed.

> This is also explained in the binding [2]
> Should I rephrase the commit message here to include that?
> 

Yes, please. Driver commit message should be independent.

> >
> >> In this variant, two disjoint sets of resources are modeled against
> >> the power domain suppliers which can be turned on/off to achieve modeled
> >> device states. Two modeled low power states d1 and d3 are supported.
> >> 
> >> transition: d0 <--> d3  :: both core and transfer suppliers
> >> transition: d0 <--> d1  :: only transfer supplier
> >> 
> >
> > What are these transition levels and who defines these? We cannot have different
> > SCMI drivers inventing these kind of independent low power modes.
> >
> 
> Maybe the naming is not as good as it should be.
> I took this originally from a downstream qcom patch and did not change
> it.
> 
> I interpret this as:
> * d3: phy is powered off, before probe (no regulators, no clocks and in reset mode)
> * d1: phy is in lower power mode (regulator on, no clocks, no reset)
> * d0: phy is powered on an operational (after probe)
> 

These looks like ACPI D-states, which are neither defined in the SCMI spec, nor
used by any other Qcom SCMI drivers. So this driver cannot just invent these
states on its own. Just use plain wording for the function names based on the
indices.

> I see that drivers/tty/serial/qcom_geni_serial.c uses a similar
> mechanism. Should I try to follow that naming convention?
> 
> If you have other suggestions, I'm happy to get them.
> 
> Note that I can't change the power domains ('core' and 'transfer' since
> those are enforced by the SCMI firmware)
> 

SCMI firmware don't define the power domain names, isn't it? They just define
indices and it is up to the OS to name it however it wants.

But if it is defined in any of the internal doc, then it is fine.

- Mani

Re: [PATCH v3 2/2] phy: qcom-snps-femto-v2: Add support for SA8255P

[Manivannan Sadhasivam]

On 2026-10-02 11:41 +0200, Mattijs Korpershoek wrote:
> From: Sriram Dash <quic_sriramd@quicinc.com>
> 
> The Qualcomm automotive SA8255p SoC relies on firmware to configure
> platform resources, including clocks, interconnects and TLMM.
> The driver requests resources operations over SCMI using power protocols.
> 
> The SCMI power protocol enables or disables resources like clocks,
> interconnect paths, and TLMM (GPIOs) using runtime PM framework APIs,
> such as resume/suspend, to control power on/off.
> 
> Power domain "core" controls the GDSC, resets, and TLMM (GPIOs)
> whereas power domain "transfer" controls clocks and interconnects.
> 
> In this variant, two disjoint sets of resources are modeled against
> the power domain suppliers which can be turned on/off to achieve modeled
> device states. Two modeled low power states d1 and d3 are supported.
> 
> transition: d0 <--> d3  :: both core and transfer suppliers
> transition: d0 <--> d1  :: only transfer supplier
> 
> Signed-off-by: Sriram Dash <quic_sriramd@quicinc.com>
> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
> ---
>  drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c | 127 ++++++++++++++++++++++++--
>  1 file changed, 118 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c b/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
> index 980ad1fb1e2e..9fdf7ae681b6 100644
> --- a/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
> +++ b/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
> @@ -12,6 +12,7 @@
>  #include <linux/of.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/reset.h>
> @@ -95,6 +96,7 @@ struct override_param_map {
>  	u8 table_size;
>  	u8 reg_offset;
>  	u8 param_mask;
> +	bool fw_managed;
>  };
>  
>  struct phy_override_seq {
> @@ -121,6 +123,7 @@ struct phy_override_seq {
>   * @phy_initialized: if PHY has been initialized correctly
>   * @mode: contains the current mode the PHY is in
>   * @update_seq_cfg: tuning parameters for phy init
> + * @pd_list: list of power domains associated with the device
>   */
>  struct qcom_snps_hsphy {
>  	struct device *dev;
> @@ -136,8 +139,81 @@ struct qcom_snps_hsphy {
>  	bool phy_initialized;
>  	enum phy_mode mode;
>  	struct phy_override_seq update_seq_cfg[NUM_HSPHY_TUNING_PARAMS];
> +
> +	struct dev_pm_domain_list *pd_list;
>  };
>  
> +static void qcom_snps_domain_detach(void *data)
> +{
> +	struct qcom_snps_hsphy *hsphy = data;
> +
> +	dev_pm_domain_detach_list(hsphy->pd_list);
> +}
> +
> +static int qcom_snps_domain_attach(struct qcom_snps_hsphy *hsphy)
> +{
> +	struct dev_pm_domain_attach_data pd_data = {
> +		.pd_flags       = PD_FLAG_NO_DEV_LINK,
> +		.pd_names       = (const char*[]) { "core", "transfer" },
> +		.num_pd_names   = 2,
> +	};
> +	struct device *dev = hsphy->dev;
> +	int ret;
> +
> +	ret = dev_pm_domain_attach_list(dev, &pd_data, &hsphy->pd_list);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "domain attach failed\n");
> +
> +	return 0;
> +}
> +
> +/* d3_to_d0 transition by turning on all the suppliers */
> +static int qcom_snps_d3_to_d0(struct qcom_snps_hsphy *hsphy)
> +{
> +	int ret;
> +
> +	if (!hsphy->pd_list)
> +		return 0;
> +
> +	ret = pm_runtime_resume_and_get(hsphy->pd_list->pd_devs[0]);
> +	if (ret)
> +		return ret;
> +
> +	ret = pm_runtime_resume_and_get(hsphy->pd_list->pd_devs[1]);
> +	if (ret)
> +		pm_runtime_put_sync(hsphy->pd_list->pd_devs[0]);
> +
> +	return ret;
> +}
> +
> +/* d0_to_d3 transition by turning off all the suppliers */
> +static void qcom_snps_d0_to_d3(struct qcom_snps_hsphy *hsphy)
> +{
> +	if (!hsphy->pd_list)
> +		return;
> +
> +	pm_runtime_put_sync(hsphy->pd_list->pd_devs[1]);
> +	pm_runtime_put_sync(hsphy->pd_list->pd_devs[0]);
> +}
> +
> +/* d1_to_d0 transition by turning on the 'transfer' supplier */
> +static int qcom_snps_d1_to_d0(struct qcom_snps_hsphy *hsphy)
> +{
> +	if (!hsphy->pd_list)
> +		return 0;
> +
> +	return pm_runtime_resume_and_get(hsphy->pd_list->pd_devs[1]);
> +}
> +
> +/* d0_to_d1 transition by turning off the 'transfer' supplier */
> +static void qcom_snps_d0_to_d1(struct qcom_snps_hsphy *hsphy)
> +{
> +	if (!hsphy->pd_list)
> +		return;
> +
> +	pm_runtime_put_sync(hsphy->pd_list->pd_devs[1]);
> +}
> +
>  static int qcom_snps_hsphy_clk_init(struct qcom_snps_hsphy *hsphy)
>  {
>  	struct device *dev = hsphy->dev;
> @@ -196,6 +272,8 @@ static int qcom_snps_hsphy_suspend(struct qcom_snps_hsphy *hsphy)
>  					   0, USB2_AUTO_RESUME);
>  	}
>  
> +	qcom_snps_d0_to_d1(hsphy);
> +
>  	return 0;
>  }
>  
> @@ -203,7 +281,7 @@ static int qcom_snps_hsphy_resume(struct qcom_snps_hsphy *hsphy)
>  {
>  	dev_dbg(&hsphy->phy->dev, "Resume QCOM SNPS PHY, mode\n");
>  
> -	return 0;
> +	return qcom_snps_d1_to_d0(hsphy);
>  }
>  
>  static int __maybe_unused qcom_snps_hsphy_runtime_suspend(struct device *dev)
> @@ -316,6 +394,10 @@ static const struct override_param ls_fs_output_impedance_sc7280[] = {
>  	{ 1310, 0 },
>  };
>  
> +static const struct override_param_map sa8255p_snps_hs_phy = {
> +	.fw_managed = true,
> +};
> +
>  static const struct override_param_map sc7280_snps_7nm_phy[] = {
>  	{
>  		"qcom,hs-disconnect-bp",
> @@ -390,9 +472,15 @@ static int qcom_snps_hsphy_init(struct phy *phy)
>  
>  	dev_vdbg(&phy->dev, "%s(): Initializing SNPS HS phy\n", __func__);
>  
> +	ret = qcom_snps_d3_to_d0(hsphy);
> +	if (ret < 0) {
> +		dev_err(hsphy->dev, "Failed to transition to d0 state\n");
> +		return ret;
> +	}
> +
>  	ret = regulator_bulk_enable(ARRAY_SIZE(hsphy->vregs), hsphy->vregs);

Why do you still control regulator natively in the SCMI path?

- Mani