[net-next,v12,2/7] phy: qcom: add the SGMII SerDes PHY driver for SCMI systems
Commit Message
Implement support for the firmware-managed SGMII/SerDes PHY present on
Qualcomm platforms. Do this as a separate driver from the HLOS-managed
variant as they don't share almost any code.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/phy/qualcomm/Kconfig | 11 ++
drivers/phy/qualcomm/Makefile | 1 +
drivers/phy/qualcomm/phy-qcom-sgmii-eth-scmi.c | 161 +++++++++++++++++++++++++
3 files changed, 173 insertions(+)
Comments
On Mon, Jul 06, 2026 at 03:20:17PM +0200, Bartosz Golaszewski wrote:
> +#include <linux/mod_devicetable.h>
Please only include those <linux/device-id/*.h> that you actually need.
I'm actively working on deprecating <linux/mod_devicetable.h>.
Thanks
Uwe
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `set_mode` callback fails to dynamically apply the new performance state to the PM domain when the PHY is already active.
- [High] Incorrect ordering of `dev_pm_genpd_set_performance_state()` around runtime PM functions causes hardware under-voltage.
--
commit d60c07674fdf361d41a55e9d09fbdf040bae5797
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
phy: qcom: add the SGMII SerDes PHY driver for SCMI systems
This commit implements support for the firmware-managed SGMII/SerDes PHY
present on Qualcomm platforms. It introduces a separate driver for the SCMI
system variant as it shares almost no code with the HLOS-managed version.
> diff --git a/drivers/phy/qualcomm/phy-qcom-sgmii-eth-scmi.c b/drivers/phy/qualcomm/phy-qcom-sgmii-eth-scmi.c
> new file mode 100644
> index 0000000000000..8ee62189556fe
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-sgmii-eth-scmi.c
[ ... ]
> +static int qcom_dwmac_sgmii_phy_scmi_power_on(struct phy *phy)
> +{
> + struct qcom_dwmac_sgmii_phy_scmi *priv = phy_get_drvdata(phy);
> + struct device *dev = phy->dev.parent;
> + int ret;
> +
> + ret = pm_runtime_resume_and_get(dev);
> + if (ret)
> + return ret;
> +
> + ret = dev_pm_genpd_set_performance_state(dev, priv->perf_state);
[Severity: High]
Should dev_pm_genpd_set_performance_state() be called before
pm_runtime_resume_and_get()?
If pm_runtime_resume_and_get() runs first, it appears the genpd runtime
resume handler will power on the domain at its previous performance state (0)
before the driver raises it. Could this starve the hardware during power-on?
> + if (ret) {
> + pm_runtime_put(dev);
> + return ret;
> + }
> +
> + usleep_range(5000, 10000);
> +
> + return 0;
> +}
> +
> +static int qcom_dwmac_sgmii_phy_scmi_power_off(struct phy *phy)
> +{
> + struct device *dev = phy->dev.parent;
> +
> + dev_pm_genpd_set_performance_state(dev, 0);
> + pm_runtime_put(dev);
[Severity: High]
Does dropping the performance state before putting the device cause a voltage
drop while the hardware is still active?
Since pm_runtime_put() is asynchronous, the hardware might still be actively
powered when dev_pm_genpd_set_performance_state(dev, 0) drops the state. Could
this ordering violate hardware voltage requirements?
> +
> + return 0;
> +}
[ ... ]
> +static int qcom_dwmac_sgmii_phy_scmi_set_mode(struct phy *phy, enum phy_mode mode,
> + int submode)
> +{
> + struct qcom_dwmac_sgmii_phy_scmi *priv = phy_get_drvdata(phy);
> + int ret;
> +
> + ret = qcom_dwmac_sgmii_phy_scmi_validate(phy, mode, submode, NULL);
> + if (ret)
> + return ret;
> +
> + priv->perf_state = (submode == PHY_INTERFACE_MODE_2500BASEX) ?
> + SPEED_2500 : SPEED_1000;
> +
> + return 0;
> +}
[Severity: High]
Does this code need to apply the new performance state to the active PM domain
if the PHY is already powered on?
When the MAC driver dynamically changes the link speed while the interface is
up, updating only priv->perf_state without calling
dev_pm_genpd_set_performance_state() might leave the hardware operating at an
incorrect voltage state.
@@ -232,3 +232,14 @@ config PHY_QCOM_SGMII_ETH
help
Enable this to support the internal SerDes/SGMII PHY on various
Qualcomm chipsets.
+
+config PHY_QCOM_SGMII_ETH_SCMI
+ tristate "Qualcomm DWMAC SGMII SerDes/PHY driver (firmware managed)"
+ depends on OF && (ARCH_QCOM || COMPILE_TEST)
+ select GENERIC_PHY
+ select PM
+ select PM_GENERIC_DOMAINS
+ help
+ Enable this to support the internal SerDes/SGMII PHY on Qualcomm
+ chipsets where the SerDes hardware (clocks and registers) is owned
+ by the firmware.
@@ -25,3 +25,4 @@ obj-$(CONFIG_PHY_QCOM_USB_SS) += phy-qcom-usb-ss.o
obj-$(CONFIG_PHY_QCOM_USB_SNPS_FEMTO_V2)+= phy-qcom-snps-femto-v2.o
obj-$(CONFIG_PHY_QCOM_IPQ806X_USB) += phy-qcom-ipq806x-usb.o
obj-$(CONFIG_PHY_QCOM_SGMII_ETH) += phy-qcom-sgmii-eth.o
+obj-$(CONFIG_PHY_QCOM_SGMII_ETH_SCMI) += phy-qcom-sgmii-eth-scmi.o
new file mode 100644
@@ -0,0 +1,161 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ *
+ * Firmware-managed variant of the Qualcomm DWMAC SGMII SerDes/PHY driver.
+ */
+
+#include <linux/delay.h>
+#include <linux/ethtool.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/phy.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
+
+struct qcom_dwmac_sgmii_phy_scmi {
+ unsigned int perf_state;
+};
+
+static int qcom_dwmac_sgmii_phy_scmi_power_on(struct phy *phy)
+{
+ struct qcom_dwmac_sgmii_phy_scmi *priv = phy_get_drvdata(phy);
+ struct device *dev = phy->dev.parent;
+ int ret;
+
+ ret = pm_runtime_resume_and_get(dev);
+ if (ret)
+ return ret;
+
+ ret = dev_pm_genpd_set_performance_state(dev, priv->perf_state);
+ if (ret) {
+ pm_runtime_put(dev);
+ return ret;
+ }
+
+ usleep_range(5000, 10000);
+
+ return 0;
+}
+
+static int qcom_dwmac_sgmii_phy_scmi_power_off(struct phy *phy)
+{
+ struct device *dev = phy->dev.parent;
+
+ dev_pm_genpd_set_performance_state(dev, 0);
+ pm_runtime_put(dev);
+
+ return 0;
+}
+
+static int qcom_dwmac_sgmii_phy_scmi_validate(struct phy *phy, enum phy_mode mode,
+ int submode,
+ union phy_configure_opts *opts)
+{
+ if (mode != PHY_MODE_ETHERNET)
+ return -EINVAL;
+
+ switch (submode) {
+ case PHY_INTERFACE_MODE_SGMII:
+ case PHY_INTERFACE_MODE_1000BASEX:
+ case PHY_INTERFACE_MODE_2500BASEX:
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int qcom_dwmac_sgmii_phy_scmi_set_mode(struct phy *phy, enum phy_mode mode,
+ int submode)
+{
+ struct qcom_dwmac_sgmii_phy_scmi *priv = phy_get_drvdata(phy);
+ int ret;
+
+ ret = qcom_dwmac_sgmii_phy_scmi_validate(phy, mode, submode, NULL);
+ if (ret)
+ return ret;
+
+ priv->perf_state = (submode == PHY_INTERFACE_MODE_2500BASEX) ?
+ SPEED_2500 : SPEED_1000;
+
+ return 0;
+}
+
+static const struct phy_ops qcom_dwmac_sgmii_phy_scmi_ops = {
+ .power_on = qcom_dwmac_sgmii_phy_scmi_power_on,
+ .power_off = qcom_dwmac_sgmii_phy_scmi_power_off,
+ .set_mode = qcom_dwmac_sgmii_phy_scmi_set_mode,
+ .validate = qcom_dwmac_sgmii_phy_scmi_validate,
+ .owner = THIS_MODULE,
+};
+
+static void qcom_dwmac_sgmii_phy_scmi_runtime_disable(void *data)
+{
+ struct device *dev = data;
+
+ pm_runtime_disable(dev);
+}
+
+static int qcom_dwmac_sgmii_phy_scmi_probe(struct platform_device *pdev)
+{
+ struct qcom_dwmac_sgmii_phy_scmi *priv;
+ struct device *dev = &pdev->dev;
+ struct phy_provider *provider;
+ struct phy *phy;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->perf_state = SPEED_1000;
+
+ /*
+ * Enable runtime PM on the provider before creating the PHY so that the
+ * PHY core enables runtime PM on the PHY device too. The single SCMI
+ * power domain has already been attached to this device by the driver
+ * core, so runtime PM votes propagate to firmware through the genpd
+ * device link. No register or clock access is done here - firmware owns
+ * the SerDes.
+ */
+ pm_runtime_enable(dev);
+
+ ret = devm_add_action_or_reset(dev, qcom_dwmac_sgmii_phy_scmi_runtime_disable, dev);
+ if (ret)
+ return ret;
+
+ phy = devm_phy_create(dev, NULL, &qcom_dwmac_sgmii_phy_scmi_ops);
+ if (IS_ERR(phy))
+ return dev_err_probe(dev, PTR_ERR(phy), "failed to create the phy\n");
+
+ phy_set_drvdata(phy, priv);
+
+ provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+ if (IS_ERR(provider))
+ return dev_err_probe(dev, PTR_ERR(provider),
+ "failed to register the PHY provider\n");
+
+ return 0;
+}
+
+static const struct of_device_id qcom_dwmac_sgmii_phy_scmi_of_match[] = {
+ { .compatible = "qcom,sa8255p-dwmac-sgmii-phy" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, qcom_dwmac_sgmii_phy_scmi_of_match);
+
+static struct platform_driver qcom_dwmac_sgmii_phy_scmi_driver = {
+ .probe = qcom_dwmac_sgmii_phy_scmi_probe,
+ .driver = {
+ .name = "qcom-dwmac-sgmii-phy-scmi",
+ .of_match_table = qcom_dwmac_sgmii_phy_scmi_of_match,
+ },
+};
+module_platform_driver(qcom_dwmac_sgmii_phy_scmi_driver);
+
+MODULE_DESCRIPTION("Qualcomm DWMAC SGMII PHY driver (firmware managed)");
+MODULE_AUTHOR("Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>");
+MODULE_LICENSE("GPL");