* [PATCH v10 4/5] phy: move and rename Airoha PCIe PHY driver to dedicated directory
From: Christian Marangi @ 2026-07-02 9:38 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Christian Marangi, Vinod Koul,
Neil Armstrong, Lorenzo Bianconi, Felix Fietkau, linux-clk,
devicetree, linux-kernel, linux-arm-kernel, linux-phy
In-Reply-To: <20260702093812.15918-1-ansuelsmth@gmail.com>
To keep the generic PHY directory tidy, move the PCIe PHY driver for
Airoha AN7581 SoC to a dedicated directory.
Also rename the driver and add the relevant SoC name to the .c and .h
file in preparation for support of PCIe and USB PHY driver for Airoha
AN7583 SoC that use a completely different implementation and
calibration for PHYs and will have their own dedicated drivers.
The rename permits to better identify the specific usage of the driver
in the future once the airoha PHY directory will have multiple driver
for multiple SoC.
The config is changed from PHY_AIROHA_PCIE to PHY_AIROHA_AN7581_PCIE.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
MAINTAINERS | 4 ++--
drivers/phy/Kconfig | 11 +----------
drivers/phy/Makefile | 4 ++--
drivers/phy/airoha/Kconfig | 13 +++++++++++++
drivers/phy/airoha/Makefile | 3 +++
.../phy-an7581-pcie-regs.h} | 2 +-
.../{phy-airoha-pcie.c => airoha/phy-an7581-pcie.c} | 6 +++---
7 files changed, 25 insertions(+), 18 deletions(-)
create mode 100644 drivers/phy/airoha/Kconfig
create mode 100644 drivers/phy/airoha/Makefile
rename drivers/phy/{phy-airoha-pcie-regs.h => airoha/phy-an7581-pcie-regs.h} (99%)
rename drivers/phy/{phy-airoha-pcie.c => airoha/phy-an7581-pcie.c} (99%)
diff --git a/MAINTAINERS b/MAINTAINERS
index 05e90c3f7fe0..952653034c4c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -765,8 +765,8 @@ M: Lorenzo Bianconi <lorenzo@kernel.org>
L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
S: Maintained
F: Documentation/devicetree/bindings/phy/airoha,en7581-pcie-phy.yaml
-F: drivers/phy/phy-airoha-pcie-regs.h
-F: drivers/phy/phy-airoha-pcie.c
+F: drivers/phy/airoha/phy-an7581-pcie-regs.h
+F: drivers/phy/airoha/phy-an7581-pcie.c
AIROHA SPI SNFI DRIVER
M: Lorenzo Bianconi <lorenzo@kernel.org>
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 19f3b7d12b7d..e93951a4a898 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -46,16 +46,6 @@ config GENERIC_PHY_MIPI_DPHY
Provides a number of helpers a core functions for MIPI D-PHY
drivers to us.
-config PHY_AIROHA_PCIE
- tristate "Airoha PCIe-PHY Driver"
- depends on ARCH_AIROHA || COMPILE_TEST
- depends on OF
- select GENERIC_PHY
- help
- Say Y here to add support for Airoha PCIe PHY driver.
- This driver create the basic PHY instance and provides initialize
- callback for PCIe GEN3 port.
-
config PHY_CAN_TRANSCEIVER
tristate "CAN transceiver PHY"
select GENERIC_PHY
@@ -168,6 +158,7 @@ config PHY_XGENE
help
This option enables support for APM X-Gene SoC multi-purpose PHY.
+source "drivers/phy/airoha/Kconfig"
source "drivers/phy/allwinner/Kconfig"
source "drivers/phy/amlogic/Kconfig"
source "drivers/phy/apple/Kconfig"
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index d7aa516bcc49..4ca4a07963fc 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -7,7 +7,6 @@ obj-$(CONFIG_PHY_COMMON_PROPS) += phy-common-props.o
obj-$(CONFIG_PHY_COMMON_PROPS_TEST) += phy-common-props-test.o
obj-$(CONFIG_GENERIC_PHY) += phy-core.o
obj-$(CONFIG_GENERIC_PHY_MIPI_DPHY) += phy-core-mipi-dphy.o
-obj-$(CONFIG_PHY_AIROHA_PCIE) += phy-airoha-pcie.o
obj-$(CONFIG_PHY_CAN_TRANSCEIVER) += phy-can-transceiver.o
obj-$(CONFIG_PHY_ECONET_PCIE) += phy-econet-pcie.o
obj-$(CONFIG_PHY_EYEQ5_ETH) += phy-eyeq5-eth.o
@@ -20,7 +19,8 @@ obj-$(CONFIG_PHY_PISTACHIO_USB) += phy-pistachio-usb.o
obj-$(CONFIG_PHY_SNPS_EUSB2) += phy-snps-eusb2.o
obj-$(CONFIG_PHY_XGENE) += phy-xgene.o
-obj-$(CONFIG_GENERIC_PHY) += allwinner/ \
+obj-$(CONFIG_GENERIC_PHY) += airoha/ \
+ allwinner/ \
amlogic/ \
apple/ \
axiado/ \
diff --git a/drivers/phy/airoha/Kconfig b/drivers/phy/airoha/Kconfig
new file mode 100644
index 000000000000..9a1b625a7701
--- /dev/null
+++ b/drivers/phy/airoha/Kconfig
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Phy drivers for Airoha devices
+#
+config PHY_AIROHA_AN7581_PCIE
+ tristate "Airoha AN7581 PCIe-PHY Driver"
+ depends on ARCH_AIROHA || COMPILE_TEST
+ depends on OF
+ select GENERIC_PHY
+ help
+ Say Y here to add support for Airoha AN7581 PCIe PHY driver.
+ This driver create the basic PHY instance and provides initialize
+ callback for PCIe GEN3 port.
diff --git a/drivers/phy/airoha/Makefile b/drivers/phy/airoha/Makefile
new file mode 100644
index 000000000000..912f3e11a061
--- /dev/null
+++ b/drivers/phy/airoha/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_PHY_AIROHA_AN7581_PCIE) += phy-an7581-pcie.o
diff --git a/drivers/phy/phy-airoha-pcie-regs.h b/drivers/phy/airoha/phy-an7581-pcie-regs.h
similarity index 99%
rename from drivers/phy/phy-airoha-pcie-regs.h
rename to drivers/phy/airoha/phy-an7581-pcie-regs.h
index 58572c793722..b938a7b468fe 100644
--- a/drivers/phy/phy-airoha-pcie-regs.h
+++ b/drivers/phy/airoha/phy-an7581-pcie-regs.h
@@ -1,4 +1,4 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
+// SPDX-License-Identifier: GPL-2.0-only
/*
* Copyright (c) 2024 AIROHA Inc
* Author: Lorenzo Bianconi <lorenzo@kernel.org>
diff --git a/drivers/phy/phy-airoha-pcie.c b/drivers/phy/airoha/phy-an7581-pcie.c
similarity index 99%
rename from drivers/phy/phy-airoha-pcie.c
rename to drivers/phy/airoha/phy-an7581-pcie.c
index 56e9ade8a9fd..81ddf0e7638b 100644
--- a/drivers/phy/phy-airoha-pcie.c
+++ b/drivers/phy/airoha/phy-an7581-pcie.c
@@ -13,7 +13,7 @@
#include <linux/platform_device.h>
#include <linux/slab.h>
-#include "phy-airoha-pcie-regs.h"
+#include "phy-an7581-pcie-regs.h"
#define LEQ_LEN_CTRL_MAX_VAL 7
#define FREQ_LOCK_MAX_ATTEMPT 10
@@ -1279,12 +1279,12 @@ MODULE_DEVICE_TABLE(of, airoha_pcie_phy_of_match);
static struct platform_driver airoha_pcie_phy_driver = {
.probe = airoha_pcie_phy_probe,
.driver = {
- .name = "airoha-pcie-phy",
+ .name = "airoha-an7581-pcie-phy",
.of_match_table = airoha_pcie_phy_of_match,
},
};
module_platform_driver(airoha_pcie_phy_driver);
-MODULE_DESCRIPTION("Airoha PCIe PHY driver");
+MODULE_DESCRIPTION("Airoha AN7581 PCIe PHY driver");
MODULE_AUTHOR("Lorenzo Bianconi <lorenzo@kernel.org>");
MODULE_LICENSE("GPL");
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v10 3/5] clk: en7523: Add support for selecting the Serdes port in SCU
From: Christian Marangi @ 2026-07-02 9:38 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Christian Marangi, Vinod Koul,
Neil Armstrong, Lorenzo Bianconi, Felix Fietkau, linux-clk,
devicetree, linux-kernel, linux-arm-kernel, linux-phy
In-Reply-To: <20260702093812.15918-1-ansuelsmth@gmail.com>
In the SCU register for clock and reset, there are also some register to
select the Serdes port mode. The Airoha AN7581 SoC have 4 different Serdes
that can switch between PCIe, USB or Ethernet mode.
Add a simple PHY provider that expose the .set_mode OP to toggle the
requested mode for the Serdes port.
Reviewed-by: Brian Masney <bmasney@redhat.com>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
drivers/clk/Kconfig | 1 +
drivers/clk/clk-en7523.c | 218 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 216 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 1717ce75a907..4d58c35d88bc 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -221,6 +221,7 @@ config COMMON_CLK_EN7523
bool "Clock driver for Airoha/EcoNet SoC system clocks"
depends on OF
depends on ARCH_AIROHA || ECONET || COMPILE_TEST
+ select GENERIC_PHY
default ARCH_AIROHA
help
This driver provides the fixed clocks and gates present on Airoha
diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
index 1ab0e2eca5d3..183cf7fe4bda 100644
--- a/drivers/clk/clk-en7523.c
+++ b/drivers/clk/clk-en7523.c
@@ -6,14 +6,18 @@
#include <linux/io.h>
#include <linux/mfd/syscon.h>
#include <linux/platform_device.h>
+#include <linux/phy.h>
+#include <linux/phy/phy.h>
#include <linux/property.h>
#include <linux/regmap.h>
#include <linux/reset-controller.h>
+#include <linux/spinlock.h>
#include <dt-bindings/clock/en7523-clk.h>
#include <dt-bindings/reset/airoha,en7523-reset.h>
#include <dt-bindings/reset/airoha,en7581-reset.h>
#include <dt-bindings/clock/econet,en751221-scu.h>
#include <dt-bindings/reset/econet,en751221-scu.h>
+#include <dt-bindings/soc/airoha,scu-ssr.h>
#define RST_NR_PER_BANK 32
@@ -40,9 +44,22 @@
#define REG_HIR_MASK GENMASK(31, 16)
/* EN7581 */
#define REG_NP_SCU_PCIC 0x88
+#define REG_NP_SCU_SSR3 0x94
+#define REG_SSUSB_HSGMII_SEL_MASK BIT(29)
+#define REG_SSUSB_HSGMII_SEL_HSGMII FIELD_PREP_CONST(REG_SSUSB_HSGMII_SEL_MASK, 0x0)
+#define REG_SSUSB_HSGMII_SEL_USB FIELD_PREP_CONST(REG_SSUSB_HSGMII_SEL_MASK, 0x1)
#define REG_NP_SCU_SSTR 0x9c
#define REG_PCIE_XSI0_SEL_MASK GENMASK(14, 13)
+#define REG_PCIE_XSI0_SEL_PCIE FIELD_PREP_CONST(REG_PCIE_XSI0_SEL_MASK, 0x0)
+#define REG_PCIE_XSI0_SEL_XFI FIELD_PREP_CONST(REG_PCIE_XSI0_SEL_MASK, 0x1)
+#define REG_PCIE_XSI0_SEL_HSGMII FIELD_PREP_CONST(REG_PCIE_XSI0_SEL_MASK, 0x2)
#define REG_PCIE_XSI1_SEL_MASK GENMASK(12, 11)
+#define REG_PCIE_XSI1_SEL_PCIE FIELD_PREP_CONST(REG_PCIE_XSI1_SEL_MASK, 0x0)
+#define REG_PCIE_XSI1_SEL_XFI FIELD_PREP_CONST(REG_PCIE_XSI1_SEL_MASK, 0x1)
+#define REG_PCIE_XSI1_SEL_HSGMII FIELD_PREP_CONST(REG_PCIE_XSI1_SEL_MASK, 0x2)
+#define REG_USB_PCIE_SEL_MASK BIT(3)
+#define REG_USB_PCIE_SEL_PCIE FIELD_PREP_CONST(REG_USB_PCIE_SEL_MASK, 0x0)
+#define REG_USB_PCIE_SEL_USB FIELD_PREP_CONST(REG_USB_PCIE_SEL_MASK, 0x1)
#define REG_CRYPTO_CLKSRC2 0x20c
/* EN751221 */
#define EN751221_REG_SPI_DIV 0x0cc
@@ -81,6 +98,8 @@ enum en_hir {
HIR_MAX = 14,
};
+#define EN_SERDES_PHY_NUM 4
+
struct en_clk_desc {
int id;
const char *name;
@@ -113,6 +132,18 @@ struct en_rst_data {
struct reset_controller_dev rcdev;
};
+struct en_serdes_phy_instance {
+ struct phy *phy;
+ unsigned int serdes_port;
+};
+
+struct en_clk_priv {
+ void __iomem *base;
+ /* protect SCU register */
+ spinlock_t lock;
+ struct en_serdes_phy_instance *serdes_phys[EN_SERDES_PHY_NUM];
+};
+
struct en_clk_soc_data {
u32 num_clocks;
const struct clk_ops pcie_ops;
@@ -830,12 +861,179 @@ static int en7581_reset_register(struct device *dev, void __iomem *base,
return devm_reset_controller_register(dev, &rst_data->rcdev);
}
+static int en7581_serdes_phy_set_mode(struct phy *phy, enum phy_mode mode,
+ int submode)
+{
+ struct en_serdes_phy_instance *instance = phy_get_drvdata(phy);
+ struct en_clk_priv *priv = dev_get_drvdata(phy->dev.parent);
+ u32 reg, mask, sel, val;
+ unsigned long flags;
+
+ switch (instance->serdes_port) {
+ case AIROHA_SCU_SERDES_PCIE1:
+ reg = REG_NP_SCU_SSTR;
+ mask = REG_PCIE_XSI0_SEL_MASK;
+
+ if (mode != PHY_MODE_ETHERNET && mode != PHY_MODE_PCIE)
+ return -EINVAL;
+
+ if (mode == PHY_MODE_ETHERNET) {
+ switch (submode) {
+ case PHY_INTERFACE_MODE_USXGMII:
+ case PHY_INTERFACE_MODE_10GBASER:
+ sel = REG_PCIE_XSI0_SEL_XFI;
+ break;
+ case PHY_INTERFACE_MODE_SGMII:
+ case PHY_INTERFACE_MODE_1000BASEX:
+ case PHY_INTERFACE_MODE_2500BASEX:
+ sel = REG_PCIE_XSI0_SEL_HSGMII;
+ break;
+ default:
+ return -EINVAL;
+ }
+ } else {
+ sel = REG_PCIE_XSI0_SEL_PCIE;
+ }
+
+ break;
+ case AIROHA_SCU_SERDES_PCIE2:
+ reg = REG_NP_SCU_SSTR;
+ mask = REG_PCIE_XSI1_SEL_MASK;
+
+ if (mode != PHY_MODE_ETHERNET && mode != PHY_MODE_PCIE)
+ return -EINVAL;
+
+ if (mode == PHY_MODE_ETHERNET) {
+ switch (submode) {
+ case PHY_INTERFACE_MODE_USXGMII:
+ case PHY_INTERFACE_MODE_10GBASER:
+ sel = REG_PCIE_XSI1_SEL_XFI;
+ break;
+ case PHY_INTERFACE_MODE_SGMII:
+ case PHY_INTERFACE_MODE_1000BASEX:
+ case PHY_INTERFACE_MODE_2500BASEX:
+ sel = REG_PCIE_XSI1_SEL_HSGMII;
+ break;
+ default:
+ return -EINVAL;
+ }
+ } else {
+ sel = REG_PCIE_XSI1_SEL_PCIE;
+ }
+
+ break;
+ case AIROHA_SCU_SERDES_USB1:
+ reg = REG_NP_SCU_SSR3;
+ mask = REG_SSUSB_HSGMII_SEL_MASK;
+
+ if (mode != PHY_MODE_ETHERNET && mode != PHY_MODE_USB_DEVICE &&
+ mode != PHY_MODE_USB_DEVICE_SS)
+ return -EINVAL;
+
+ if (mode == PHY_MODE_ETHERNET)
+ sel = REG_SSUSB_HSGMII_SEL_HSGMII;
+ else
+ sel = REG_SSUSB_HSGMII_SEL_USB;
+
+ break;
+ case AIROHA_SCU_SERDES_USB2:
+ reg = REG_NP_SCU_SSTR;
+ mask = REG_USB_PCIE_SEL_MASK;
+
+ if (mode != PHY_MODE_PCIE && mode != PHY_MODE_USB_DEVICE &&
+ mode != PHY_MODE_USB_DEVICE_SS)
+ return -EINVAL;
+
+ if (mode == PHY_MODE_PCIE)
+ sel = REG_USB_PCIE_SEL_PCIE;
+ else
+ sel = REG_USB_PCIE_SEL_USB;
+
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ spin_lock_irqsave(&priv->lock, flags);
+ val = readl(priv->base + reg);
+ val &= ~mask;
+ val |= sel;
+ writel(val, priv->base + reg);
+ spin_unlock_irqrestore(&priv->lock, flags);
+
+ return 0;
+}
+
+static const struct phy_ops en7581_serdes_phy_ops = {
+ .set_mode = en7581_serdes_phy_set_mode,
+ .owner = THIS_MODULE,
+};
+
+static struct phy *en7581_serdes_phy_xlate(struct device *dev,
+ const struct of_phandle_args *args)
+{
+ struct en_clk_priv *priv = dev_get_drvdata(dev);
+ struct en_serdes_phy_instance *instance;
+ unsigned int serdes_port;
+
+ if (args->args_count != 1) {
+ dev_err(dev, "invalid number of cells in 'phy' property\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ serdes_port = args->args[0];
+ if (serdes_port >= EN_SERDES_PHY_NUM) {
+ dev_err(dev, "invalid serdes port: %d\n", serdes_port);
+ return ERR_PTR(-EINVAL);
+ }
+
+ instance = priv->serdes_phys[serdes_port];
+ if (!instance) {
+ dev_err(dev, "failed to find appropriate serdes phy\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ return instance->phy;
+}
+
+static int en7581_serdes_phy_register(struct device *dev)
+{
+ struct en_clk_priv *priv = dev_get_drvdata(dev);
+ struct phy_provider *phy_provider;
+ int i;
+
+ for (i = 0; i < EN_SERDES_PHY_NUM; i++) {
+ struct en_serdes_phy_instance *instance;
+
+ instance = devm_kzalloc(dev, sizeof(*instance),
+ GFP_KERNEL);
+ if (!instance)
+ return -ENOMEM;
+
+ instance->phy = devm_phy_create(dev, NULL,
+ &en7581_serdes_phy_ops);
+ if (IS_ERR(instance->phy))
+ return dev_err_probe(dev, PTR_ERR(instance->phy), "failed to create phy\n");
+
+ instance->serdes_port = i;
+ priv->serdes_phys[i] = instance;
+
+ phy_set_drvdata(instance->phy, instance);
+ }
+
+ phy_provider = devm_of_phy_provider_register(dev, en7581_serdes_phy_xlate);
+
+ return PTR_ERR_OR_ZERO(phy_provider);
+}
+
static int en7581_clk_hw_init(struct platform_device *pdev,
struct clk_hw_onecell_data *clk_data)
{
+ struct en_clk_priv *priv = platform_get_drvdata(pdev);
struct regmap *map;
void __iomem *base;
u32 val;
+ int ret;
map = syscon_regmap_lookup_by_compatible("airoha,en7581-chip-scu");
if (IS_ERR(map))
@@ -845,6 +1043,8 @@ static int en7581_clk_hw_init(struct platform_device *pdev,
if (IS_ERR(base))
return PTR_ERR(base);
+ priv->base = base;
+
en7581_register_clocks(&pdev->dev, clk_data, map, base);
val = readl(base + REG_NP_SCU_SSTR);
@@ -853,9 +1053,12 @@ static int en7581_clk_hw_init(struct platform_device *pdev,
val = readl(base + REG_NP_SCU_PCIC);
writel(val | 3, base + REG_NP_SCU_PCIC);
- return en7581_reset_register(&pdev->dev, base, en7581_rst_map,
- ARRAY_SIZE(en7581_rst_map),
- en7581_rst_ofs);
+ ret = en7581_reset_register(&pdev->dev, base, en7581_rst_map,
+ ARRAY_SIZE(en7581_rst_map), en7581_rst_ofs);
+ if (ret)
+ return ret;
+
+ return en7581_serdes_phy_register(&pdev->dev);
}
static enum en_hir get_hw_id(void __iomem *np_base)
@@ -962,16 +1165,25 @@ static int en7523_clk_probe(struct platform_device *pdev)
struct device_node *node = pdev->dev.of_node;
const struct en_clk_soc_data *soc_data;
struct clk_hw_onecell_data *clk_data;
+ struct en_clk_priv *priv;
int r;
soc_data = device_get_match_data(&pdev->dev);
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ spin_lock_init(&priv->lock);
+
clk_data = devm_kzalloc(&pdev->dev,
struct_size(clk_data, hws, soc_data->num_clocks),
GFP_KERNEL);
if (!clk_data)
return -ENOMEM;
+ platform_set_drvdata(pdev, priv);
+
clk_data->num = soc_data->num_clocks;
r = soc_data->hw_init(pdev, clk_data);
if (r)
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v10 2/5] dt-bindings: phy: Add documentation for Airoha AN7581 USB PHY
From: Christian Marangi @ 2026-07-02 9:38 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Christian Marangi, Vinod Koul,
Neil Armstrong, Lorenzo Bianconi, Felix Fietkau, linux-clk,
devicetree, linux-kernel, linux-arm-kernel, linux-phy
Cc: Krzysztof Kozlowski
In-Reply-To: <20260702093812.15918-1-ansuelsmth@gmail.com>
Add documentation for Airoha AN7581 USB PHY that describe the USB PHY
for the USB controller.
Airoha AN7581 SoC support a maximum of 2 USB port. The USB 2.0 mode is
always supported. The USB 3.0 mode is optional and depends on the Serdes
mode currently configured on the system for the relevant USB port.
To correctly calibrate, the USB 2.0 port require correct value in
"airoha,usb2-monitor-clk-sel" property. Both the 2 USB 2.0 port permit
selecting one of the 4 monitor clock for calibration (internal clock not
exposed to the system) but each port have only one of the 4 actually
connected in HW hence the correct value needs to be specified in DT
based on board and the physical port. Normally it's monitor clock 1 for
USB1 and monitor clock 2 for USB2.
To correctly setup the Serdes mode attached to the USB 3.0 mode, a phys
property is required with the phandle pointing to the correct Serdes port
provided by the SCU node. Providing the phys property is optional if USB
3.0 is not used.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
.../bindings/phy/airoha,an7581-usb-phy.yaml | 62 +++++++++++++++++++
MAINTAINERS | 6 ++
2 files changed, 68 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/airoha,an7581-usb-phy.yaml
diff --git a/Documentation/devicetree/bindings/phy/airoha,an7581-usb-phy.yaml b/Documentation/devicetree/bindings/phy/airoha,an7581-usb-phy.yaml
new file mode 100644
index 000000000000..f42e3d49a61f
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/airoha,an7581-usb-phy.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/airoha,an7581-usb-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Airoha AN7581 SoC USB PHY
+
+maintainers:
+ - Christian Marangi <ansuelsmth@gmail.com>
+
+description: >
+ The Airoha AN7581 SoC USB PHY describes the USB PHY for the USB controller.
+
+ Airoha AN7581 SoC support a maximum of 2 USB port. The USB 2.0 mode is
+ always supported. The USB 3.0 mode is optional and depends on the Serdes
+ mode currently configured on the system for the relevant USB port.
+
+properties:
+ compatible:
+ const: airoha,an7581-usb-phy
+
+ reg:
+ maxItems: 1
+
+ airoha,usb2-monitor-clk-sel:
+ description: Describe what oscillator across the available 4
+ should be selected for USB 2.0 Slew Rate calibration.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 1, 2, 3]
+
+ phys:
+ items:
+ - description: phandle to Serdes PHY. Optional if USB 3.0 is not used.
+
+ '#phy-cells':
+ description: The cell contains the mode, PHY_TYPE_USB2 or PHY_TYPE_USB3,
+ as defined in dt-bindings/phy/phy.h.
+ const: 1
+
+required:
+ - compatible
+ - reg
+ - airoha,usb2-monitor-clk-sel
+ - '#phy-cells'
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/soc/airoha,scu-ssr.h>
+
+ phy@1fac0000 {
+ compatible = "airoha,an7581-usb-phy";
+ reg = <0x1fac0000 0x10000>;
+
+ airoha,usb2-monitor-clk-sel = <1>;
+ phys = <&scu AIROHA_SCU_SERDES_USB1>;
+
+ #phy-cells = <1>;
+ };
+
diff --git a/MAINTAINERS b/MAINTAINERS
index 1705eb823dd0..05e90c3f7fe0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -777,6 +777,12 @@ S: Maintained
F: Documentation/devicetree/bindings/spi/airoha,en7581-snand.yaml
F: drivers/spi/spi-airoha-snfi.c
+AIROHA USB PHY DRIVER
+M: Christian Marangi <ansuelsmth@gmail.com>
+L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
+S: Maintained
+F: Documentation/devicetree/bindings/phy/airoha,an7581-usb-phy.yaml
+
AIRSPY MEDIA DRIVER
L: linux-media@vger.kernel.org
S: Orphan
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v10 1/5] dt-bindings: clock: airoha: Add PHY binding for Serdes port
From: Christian Marangi @ 2026-07-02 9:38 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Christian Marangi, Vinod Koul,
Neil Armstrong, Lorenzo Bianconi, Felix Fietkau, linux-clk,
devicetree, linux-kernel, linux-arm-kernel, linux-phy
Cc: Krzysztof Kozlowski
In-Reply-To: <20260702093812.15918-1-ansuelsmth@gmail.com>
Add PHY cell property for Serdes port selection. Currently supported only
for Airoha AN7581 SoC, that support up to 4 Serdes port.
The Serdes port can support both PCIe, USB3 or Ethernet mode.
- PCIe1 Serdes can support PCIe or Ethernet mode.
- PCIe2 Serdes can support PCIe or Ethernet mode.
- USB1 Serdes can support USB3 or HSGMII mode.
- USB2 Serdes can support USB3 or PCIe mode.
Add bindings to permit correct reference of the Serdes ports in DT.
Values are just symbolic and enumerates the Serdes port with a specific
number for precise reference.
The available Serdes port can be selected following the dt-binding header
in [2].
[2] <include/dt-bindings/soc/airoha,scu-ssr.h>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
.../devicetree/bindings/clock/airoha,en7523-scu.yaml | 9 +++++++++
include/dt-bindings/soc/airoha,scu-ssr.h | 11 +++++++++++
2 files changed, 20 insertions(+)
create mode 100644 include/dt-bindings/soc/airoha,scu-ssr.h
diff --git a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
index eb24a5687639..913ddc16182b 100644
--- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
+++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
@@ -23,6 +23,7 @@ description: |
All these identifiers can be found in:
[1]: <include/dt-bindings/clock/en7523-clk.h>.
+ [2]: <include/dt-bindings/soc/airoha,scu-ssr.h>.
The clocks are provided inside a system controller node.
@@ -50,6 +51,12 @@ properties:
description: ID of the controller reset line
const: 1
+ '#phy-cells':
+ description:
+ The first cell indicates the serdes phy number, see [2] for the
+ available serdes port.
+ const: 1
+
required:
- compatible
- reg
@@ -65,6 +72,8 @@ allOf:
reg:
minItems: 2
+ '#phy-cells': false
+
- if:
properties:
compatible:
diff --git a/include/dt-bindings/soc/airoha,scu-ssr.h b/include/dt-bindings/soc/airoha,scu-ssr.h
new file mode 100644
index 000000000000..33c64844ada3
--- /dev/null
+++ b/include/dt-bindings/soc/airoha,scu-ssr.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+
+#ifndef __DT_BINDINGS_AIROHA_SCU_SSR_H
+#define __DT_BINDINGS_AIROHA_SCU_SSR_H
+
+#define AIROHA_SCU_SERDES_PCIE1 0
+#define AIROHA_SCU_SERDES_PCIE2 1
+#define AIROHA_SCU_SERDES_USB1 2
+#define AIROHA_SCU_SERDES_USB2 3
+
+#endif /* __DT_BINDINGS_AIROHA_SCU_SSR_H */
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v10 0/5] airoha: an7581: USB support
From: Christian Marangi @ 2026-07-02 9:38 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Christian Marangi, Vinod Koul,
Neil Armstrong, Lorenzo Bianconi, Felix Fietkau, linux-clk,
devicetree, linux-kernel, linux-arm-kernel, linux-phy
This is a major rework of the old v2 series.
The SoC always support USB 2.0 but for USB 3.0 it needs additional
configuration for the Serdes port. Such port can be either configured
for USB usage or for PCIe lines or HSGMII and these are configured
in the SCU space.
The previous implementation of a dedicated SSR driver was too
complex and fragile for the simple task of configuring a register
hence it was dropped and the handling is entirely in the PHY driver.
Everything was reducted to the dt-bindings to describe the Serdes line.
Also the property for the PHY are renamed to a more suitable name and
everything is now mandatory to simplify the implementation.
(the PHY are always present and active on the SoC)
Also other unrelated patch are dropped from this series.
Changes v10:
- Rebase on top of linux-next
- Add review tag for DT patch
Changes v9:
- Add spinlock init
- Add ack tag
- Add review tag
- Add additional comments for phys on USB 3.0
- Add extra logic to make phys optional for USB 3.0
Changes v8:
- Squash header to clk Documentation patch
- Address comments from AI Bot
Link: https://lore.kernel.org/all/20260520150912.11614-1-ansuelsmth@gmail.com/
Changes v7:
- Rework to double PHY implementation
(suggested by Rob)
Now the clk driver expose a PHY for Serdes port
USB PHY driver selects it
- Rebase on top of linux-next
Link: https://lore.kernel.org/all/20260519220813.28468-1-ansuelsmth@gmail.com/
Changes v6:
- Fix kernel test robot (sparse warning)
Link: https://lore.kernel.org/all/20260306190156.22297-1-ansuelsmth@gmail.com/
Changes v5:
- Add Ack and Review tag from Connor
- Implement Ethernet support in the USB driver
(testing support for this Serdes on a special reference board)
- Use an7581 prefix for USB PHY driver
Link: https://lore.kernel.org/all/20251107160251.2307088-1-ansuelsmth@gmail.com/
Changes v4:
- Rename PCIe and USB PHY to AN7581
- Drop airoha,scu (handled directly in driver)
- Drop dt-bindings for monitor clock in favor of raw values
- Better describe the usage of airoha,usb3-serdes
- Simplify values of dt-bindings SSR SERDES
Link: https://lore.kernel.org/all/20251107160251.2307088-1-ansuelsmth@gmail.com/
Changes v3:
- Drop clk changes
- Drop SSR driver
- Rename property in Documentation
- Simplify PHY handling
- Move SSR handling inside the PHY driver
Link: https://lore.kernel.org/all/20251029173713.7670-1-ansuelsmth@gmail.com/
Changes v2:
- Drop changes for simple-mfd
- Rework PHY node structure to single node
- Drop port-id property in favor of serdes-port and
usb2-monitor-clock-sel
- Make the SSR driver probe from the clock driver
Christian Marangi (5):
dt-bindings: clock: airoha: Add PHY binding for Serdes port
dt-bindings: phy: Add documentation for Airoha AN7581 USB PHY
clk: en7523: Add support for selecting the Serdes port in SCU
phy: move and rename Airoha PCIe PHY driver to dedicated directory
phy: airoha: Add support for Airoha AN7581 USB PHY
.../bindings/clock/airoha,en7523-scu.yaml | 9 +
.../bindings/phy/airoha,an7581-usb-phy.yaml | 62 ++
MAINTAINERS | 11 +-
drivers/clk/Kconfig | 1 +
drivers/clk/clk-en7523.c | 218 ++++++-
drivers/phy/Kconfig | 11 +-
drivers/phy/Makefile | 4 +-
drivers/phy/airoha/Kconfig | 24 +
drivers/phy/airoha/Makefile | 4 +
.../phy-an7581-pcie-regs.h} | 2 +-
.../phy-an7581-pcie.c} | 6 +-
drivers/phy/airoha/phy-an7581-usb.c | 559 ++++++++++++++++++
include/dt-bindings/soc/airoha,scu-ssr.h | 11 +
13 files changed, 901 insertions(+), 21 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/airoha,an7581-usb-phy.yaml
create mode 100644 drivers/phy/airoha/Kconfig
create mode 100644 drivers/phy/airoha/Makefile
rename drivers/phy/{phy-airoha-pcie-regs.h => airoha/phy-an7581-pcie-regs.h} (99%)
rename drivers/phy/{phy-airoha-pcie.c => airoha/phy-an7581-pcie.c} (99%)
create mode 100644 drivers/phy/airoha/phy-an7581-usb.c
create mode 100644 include/dt-bindings/soc/airoha,scu-ssr.h
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Vladimir Zapolskiy @ 2026-07-02 9:28 UTC (permalink / raw)
To: Bryan O'Donoghue, Bryan O'Donoghue, Vinod Koul,
Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong
Cc: linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel
In-Reply-To: <3e2e6184-e020-49cc-9a4b-609c05aed846@linaro.org>
On 7/2/26 11:46, Bryan O'Donoghue wrote:
> On 02/07/2026 09:31, Vladimir Zapolskiy wrote:
>>>>> phy-cells = 1 with CPHY/DPHY specified in the consumer, as was Rob's
>>>>> suggestion will specify the mode.
>>>>
>>>> This cell is just not needed, and unneeded complexity should not be
>>>> added.
>>>
>>> I'm going to stick to Rob's initial guidance on this.
>>>
>>> https://lore.kernel.org/linux-media/20250710230846.GA44483-
>>> robh@kernel.org/
>>>
>>
>> Sure, the discussion will be continued abouve v9 of the series, it's
>> just a chance to do it now. Rob properly pointed out, that the invented
>> "phy-type" is not needed at all.
>
> I'll quote here
>
> >> + phy-type:
> >> + description: D-PHY or C-PHY mode
> >> + enum: [ 10, 11 ]
> >> + $ref: /schemas/types.yaml#/definitions/uint32
>
> > Perhaps putting this in phy cells would be better because the consumer
> > decides on the mode.
>
> A clear instruction which I'm implementing.
It has no contradiction with what I've said above, you have a chance to
discuss the matter now or at v9 time.
>> Ther fact is that the whole placement of the CSIPHY to phy subsystem
>> looks execessive, since you add a driver for a media device, there will
>> be nothing PHY driver specific left in the CSIPHY driver but name.
> No, a block with its own pins, external voltage rails, even its own
> specific votes to scale individual clocks, is a distinct PHY.
>
> There will be no further CSIPHY init sequences getting buried in CAMSS.
> It is well past time to dispense with the monolith.
>
Again, there is no contradiction with what I've said above, so far there
is no argument to support CSIPHY driver registration within phy framework.
--
Best wishes,
Vladimir
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v4 net-next 1/2] dt-bindings: phy: cadence-torrent: Update property values to support 3 clocks
From: Gokul Praveen @ 2026-07-02 8:56 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: conor+dt, devicetree, krzk+dt, linux-arm-kernel, linux-kernel,
linux-phy, neil.armstrong, nm, robh, sjakhade, kristo, vigneshr,
vkoul, yamonkar, Gokul Praveen
In-Reply-To: <20260702-vigilant-tody-of-inquire-ffbede@quoll>
Hi Krzysztof,
On 02/07/26 11:53, Krzysztof Kozlowski wrote:
> On Wed, Jul 01, 2026 at 07:54:56PM +0530, Gokul Praveen wrote:
>> Update maxItems value of "clocks" property to 3 as description of
>> this parameter already indicates 3 clocks(refclk,pll1_refclk(optional)
>> and phy_en_refclk(optional)).
> But what if description is wrong? You need to provide rationale why you
> are doing it and you cannot use existing code alone as that rationale,
> because as you pointed out - existing code is not fully correct.
>
>> Update the maxItems and items value of "clock-names" property with multiple
>> combination of clock-names possible since pll1_refclk and phy_en_refclk are
>> optional clocks.
> Why? You need to describe why you are doing this, not what you are
> doing.
>
Would the below commit description be good enough ? Please feel free to
give any suggestions on this:
''
dt-bindings: phy: cadence-torrent: Update property values to support 3
clocks
Increase the maxItems value of "clocks" property to 3 to support
2 input clocks(refclk,pll1_refclk) and 1 output clock(phy_en_refclk).
For multilink SERDES configurations where the links require 2
different input clock speeds,
2 different input reference clocks and 1 output clock is needed so
as to cater to this requirement.
For eg: Considering the USXGMII+SGMII multilink SERDES
configuration usecase ,
having only 1 input reference clock fails because USXGMII requires
an input clock speed of 156.25 Mhz and
SGMII protocol requires an input clock speed of 100 Mhz.
Since one input reference clock(refclk) alone cannot cater to the 2
different clock speed requirements
of these protocols, the second input reference clock(pll1_refclk)
has to be added.
Signed-off-by: Gokul Praveen <g-praveen@ti.com>
''
>> Signed-off-by: Gokul Praveen <g-praveen@ti.com>
>> ---
>> .../bindings/phy/phy-cadence-torrent.yaml | 16 ++++++++++++----
>> 1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
>> index 9af39b33646a..96c664d50629 100644
>> --- a/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
>> +++ b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
>> @@ -34,7 +34,7 @@ properties:
>>
>> clocks:
>> minItems: 1
>> - maxItems: 2
>> + maxItems: 3
>> description:
>> PHY input reference clocks - refclk (for PLL0) & pll1_refclk (for PLL1).
>> pll1_refclk is optional and used for multi-protocol configurations requiring
>> @@ -45,9 +45,17 @@ properties:
>>
>> clock-names:
>> minItems: 1
>> - items:
>> - - const: refclk
>> - - enum: [ pll1_refclk, phy_en_refclk ]
>> + maxItems: 3
> Drop
sure krzysztof.
>> + oneOf:
>> + - items:
>> + - const: refclk
>> + - items:
>> + - const: refclk
>> + - enum: [ pll1_refclk, phy_en_refclk ]
> Drop these, pointless. You were supposed to grow existing syntax.
>
Sure Krzysztof
>> + - items:
>> + - const: refclk
>> + - const: pll1_refclk
> So here is the enum.
>
>> + - const: phy_en_refclk
>>
>> And this stays.
SUre Krzysztof
>>
>> You make changes which do not make the binding better and are not
>> explained in commit msg. Focus on WHY you are doing things and also
>> explain WHY you did such complicated syntax (if you insist on rewriting
>> correct code into something odd we do not expect).
>>
>> Best regards,
>> Krzysztof
Best Regards
Gokul Praveen
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Bryan O'Donoghue @ 2026-07-02 8:46 UTC (permalink / raw)
To: Vladimir Zapolskiy, Bryan O'Donoghue, Vinod Koul,
Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong
Cc: linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel
In-Reply-To: <1c583e24-09db-4d90-8f32-d3d3961188a0@linaro.org>
On 02/07/2026 09:31, Vladimir Zapolskiy wrote:
>>>> phy-cells = 1 with CPHY/DPHY specified in the consumer, as was Rob's
>>>> suggestion will specify the mode.
>>>
>>> This cell is just not needed, and unneeded complexity should not be
>>> added.
>>
>> I'm going to stick to Rob's initial guidance on this.
>>
>> https://lore.kernel.org/linux-media/20250710230846.GA44483-
>> robh@kernel.org/
>>
>
> Sure, the discussion will be continued abouve v9 of the series, it's
> just a chance to do it now. Rob properly pointed out, that the invented
> "phy-type" is not needed at all.
I'll quote here
>> + phy-type:
>> + description: D-PHY or C-PHY mode
>> + enum: [ 10, 11 ]
>> + $ref: /schemas/types.yaml#/definitions/uint32
> Perhaps putting this in phy cells would be better because the consumer
> decides on the mode.
A clear instruction which I'm implementing.
> Ther fact is that the whole placement of the CSIPHY to phy subsystem
> looks execessive, since you add a driver for a media device, there will
> be nothing PHY driver specific left in the CSIPHY driver but name.
No, a block with its own pins, external voltage rails, even its own
specific votes to scale individual clocks, is a distinct PHY.
There will be no further CSIPHY init sequences getting buried in CAMSS.
It is well past time to dispense with the monolith.
---
bod
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Vladimir Zapolskiy @ 2026-07-02 8:31 UTC (permalink / raw)
To: Bryan O'Donoghue, Bryan O'Donoghue, Vinod Koul,
Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong
Cc: linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel
In-Reply-To: <6b6492a4-7610-4dce-a81c-8dc0387a4061@linaro.org>
On 7/2/26 03:08, Bryan O'Donoghue wrote:
> On 02/07/2026 00:37, Vladimir Zapolskiy wrote:
>>>
>>> Inserting bus-type into the PHY layer is borrowing from one domain
>>> linux-media and pushing the concept into PHYs.
>>
>> It's the other way around, "bus-type" is an endpoint property of media
>> devices, reference to video-interfaces.yaml to get more details about it.
>>
>>> phy-cells = 1 with CPHY/DPHY specified in the consumer, as was Rob's
>>> suggestion will specify the mode.
>>
>> This cell is just not needed, and unneeded complexity should not be added.
>
> I'm going to stick to Rob's initial guidance on this.
>
> https://lore.kernel.org/linux-media/20250710230846.GA44483-robh@kernel.org/
>
Sure, the discussion will be continued abouve v9 of the series, it's
just a chance to do it now. Rob properly pointed out, that the invented
"phy-type" is not needed at all.
Ther fact is that the whole placement of the CSIPHY to phy subsystem
looks execessive, since you add a driver for a media device, there will
be nothing PHY driver specific left in the CSIPHY driver but name.
--
Best wishes,
Vladimir
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v4 net-next 1/2] dt-bindings: phy: cadence-torrent: Update property values to support 3 clocks
From: Krzysztof Kozlowski @ 2026-07-02 7:38 UTC (permalink / raw)
To: Gokul Praveen
Cc: conor+dt, devicetree, krzk+dt, linux-arm-kernel, linux-kernel,
linux-phy, neil.armstrong, nm, robh, sjakhade, kristo, vigneshr,
vkoul, yamonkar
In-Reply-To: <f40839d9-445e-4e48-ada9-97feb8e40584@ti.com>
On 02/07/2026 09:35, Gokul Praveen wrote:
> Hi Krzystof,
>
> On 02/07/26 11:53, Krzysztof Kozlowski wrote:
>> On Wed, Jul 01, 2026 at 07:54:56PM +0530, Gokul Praveen wrote:
>>> Update maxItems value of "clocks" property to 3 as description of
>>> this parameter already indicates 3 clocks(refclk,pll1_refclk(optional)
>>> and phy_en_refclk(optional)).
>> But what if description is wrong? You need to provide rationale why you
>> are doing it and you cannot use existing code alone as that rationale,
>> because as you pointed out - existing code is not fully correct.
>
> The description is correct because not all device may have 2 input
I do not see how you proved it in the commit msg.
> reference clocks , hence keeping the requirement of the 2nd reference
> clock(pll1_refclk) optional.
>
> Just as a note: phy_en_refclk is an output clock.
output clocks do not go to input clocks property.
>
> In those cases the multilink serdes configurations requiring 2 different
> input reference clocks will not work due to the limitation of having
> only 1 clock.
>
> However, when it comes to devices where 2 different input reference
> clocks are supported and a multilink serdes configuration is
> needed(where the links require separate reference clocks for each
> protocol so as to cater to the different clocking speed requirements of
> these links).
>
> Hence, in this case ,2 different input clocks are needed so as to cater
> to 2 different clock speeds.
>
> For eg: In the USXGMII+SGMII multilink serdes configuration which I had
> tested, it failed because
>
> USXGMII requires an input clock speed of 156.25 Mhz and SGMII protocol
> requires an input clock speed of 100 Mhz.
>
> But, since there was only one input clock(refclk) mentioned in the
> clocks and clock-name parameter , this multilink serdes configuration
> failed.
>
> Hence, to make it work, the pll1_refclk had to be added which provided a
> clock speed of 156.25 Mhz for USXGMI and the refclk provided
>
> a clock speed of 100 Mhz for SGMII.
>
>>> Update the maxItems and items value of "clock-names" property with multiple
>>> combination of clock-names possible since pll1_refclk and phy_en_refclk are
>>> optional clocks.
>> Why? You need to describe why you are doing this, not what you are
>> doing.
> Sure , Krzysztof, I will be careful about that and prioritize that in
> the commit message.
>>> Signed-off-by: Gokul Praveen <g-praveen@ti.com>
>>> ---
>>> .../bindings/phy/phy-cadence-torrent.yaml | 16 ++++++++++++----
>>> 1 file changed, 12 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
>>> index 9af39b33646a..96c664d50629 100644
>>> --- a/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
>>> +++ b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
>>> @@ -34,7 +34,7 @@ properties:
>>>
>>> clocks:
>>> minItems: 1
>>> - maxItems: 2
>>> + maxItems: 3
>>> description:
>>> PHY input reference clocks - refclk (for PLL0) & pll1_refclk (for PLL1).
>>> pll1_refclk is optional and used for multi-protocol configurations requiring
>>> @@ -45,9 +45,17 @@ properties:
>>>
>>> clock-names:
>>> minItems: 1
>>> - items:
>>> - - const: refclk
>>> - - enum: [ pll1_refclk, phy_en_refclk ]
>>> + maxItems: 3
>> Drop
> Sure, i will do that Krzysztof.
>>> + oneOf:
>>> + - items:
>>> + - const: refclk
>>> + - items:
>>> + - const: refclk
>>> + - enum: [ pll1_refclk, phy_en_refclk ]
>> Drop these, pointless. You were supposed to grow existing syntax.
>>
>>> + - items:
>>> + - const: refclk
>>> + - const: pll1_refclk
>> So here is the enum.
>>
>>> + - const: phy_en_refclk
>> And this stays.
>>
>> You make changes which do not make the binding better and are not
>> explained in commit msg. Focus on WHY you are doing things and also
>> explain WHY you did such complicated syntax (if you insist on rewriting
>> correct code into something odd we do not expect).
>
> So, the reason I added the oneOf property is to support the following
> combinations because pll1_refclk and phy_en_refclk are optional clocks.
> With the earlier enum , only either of pll1_refclk or phy_en_refclk
>
> can be used and both cannot be used at the same time.
>
> Combination 1: refclk
>
> Combination 2 : refclk, pll1_refclk
>
> Combination 3: reclk, phy_en_refclk
>
> Combination 4: refclk, pll1_refclk, phy_en_refclk
>
>
> Please feel free to suggest any alternative solution to support these
> combinations .
I already did. Read the feedback carefully.
Best regards,
Krzysztof
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v4 net-next 1/2] dt-bindings: phy: cadence-torrent: Update property values to support 3 clocks
From: Gokul Praveen @ 2026-07-02 7:35 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: conor+dt, devicetree, krzk+dt, linux-arm-kernel, linux-kernel,
linux-phy, neil.armstrong, nm, robh, sjakhade, kristo, vigneshr,
vkoul, yamonkar, Gokul Praveen
In-Reply-To: <20260702-vigilant-tody-of-inquire-ffbede@quoll>
Hi Krzystof,
On 02/07/26 11:53, Krzysztof Kozlowski wrote:
> On Wed, Jul 01, 2026 at 07:54:56PM +0530, Gokul Praveen wrote:
>> Update maxItems value of "clocks" property to 3 as description of
>> this parameter already indicates 3 clocks(refclk,pll1_refclk(optional)
>> and phy_en_refclk(optional)).
> But what if description is wrong? You need to provide rationale why you
> are doing it and you cannot use existing code alone as that rationale,
> because as you pointed out - existing code is not fully correct.
The description is correct because not all device may have 2 input
reference clocks , hence keeping the requirement of the 2nd reference
clock(pll1_refclk) optional.
Just as a note: phy_en_refclk is an output clock.
In those cases the multilink serdes configurations requiring 2 different
input reference clocks will not work due to the limitation of having
only 1 clock.
However, when it comes to devices where 2 different input reference
clocks are supported and a multilink serdes configuration is
needed(where the links require separate reference clocks for each
protocol so as to cater to the different clocking speed requirements of
these links).
Hence, in this case ,2 different input clocks are needed so as to cater
to 2 different clock speeds.
For eg: In the USXGMII+SGMII multilink serdes configuration which I had
tested, it failed because
USXGMII requires an input clock speed of 156.25 Mhz and SGMII protocol
requires an input clock speed of 100 Mhz.
But, since there was only one input clock(refclk) mentioned in the
clocks and clock-name parameter , this multilink serdes configuration
failed.
Hence, to make it work, the pll1_refclk had to be added which provided a
clock speed of 156.25 Mhz for USXGMI and the refclk provided
a clock speed of 100 Mhz for SGMII.
>> Update the maxItems and items value of "clock-names" property with multiple
>> combination of clock-names possible since pll1_refclk and phy_en_refclk are
>> optional clocks.
> Why? You need to describe why you are doing this, not what you are
> doing.
Sure , Krzysztof, I will be careful about that and prioritize that in
the commit message.
>> Signed-off-by: Gokul Praveen <g-praveen@ti.com>
>> ---
>> .../bindings/phy/phy-cadence-torrent.yaml | 16 ++++++++++++----
>> 1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
>> index 9af39b33646a..96c664d50629 100644
>> --- a/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
>> +++ b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
>> @@ -34,7 +34,7 @@ properties:
>>
>> clocks:
>> minItems: 1
>> - maxItems: 2
>> + maxItems: 3
>> description:
>> PHY input reference clocks - refclk (for PLL0) & pll1_refclk (for PLL1).
>> pll1_refclk is optional and used for multi-protocol configurations requiring
>> @@ -45,9 +45,17 @@ properties:
>>
>> clock-names:
>> minItems: 1
>> - items:
>> - - const: refclk
>> - - enum: [ pll1_refclk, phy_en_refclk ]
>> + maxItems: 3
> Drop
Sure, i will do that Krzysztof.
>> + oneOf:
>> + - items:
>> + - const: refclk
>> + - items:
>> + - const: refclk
>> + - enum: [ pll1_refclk, phy_en_refclk ]
> Drop these, pointless. You were supposed to grow existing syntax.
>
>> + - items:
>> + - const: refclk
>> + - const: pll1_refclk
> So here is the enum.
>
>> + - const: phy_en_refclk
> And this stays.
>
> You make changes which do not make the binding better and are not
> explained in commit msg. Focus on WHY you are doing things and also
> explain WHY you did such complicated syntax (if you insist on rewriting
> correct code into something odd we do not expect).
So, the reason I added the oneOf property is to support the following
combinations because pll1_refclk and phy_en_refclk are optional clocks.
With the earlier enum , only either of pll1_refclk or phy_en_refclk
can be used and both cannot be used at the same time.
Combination 1: refclk
Combination 2 : refclk, pll1_refclk
Combination 3: reclk, phy_en_refclk
Combination 4: refclk, pll1_refclk, phy_en_refclk
Please feel free to suggest any alternative solution to support these
combinations .
>
> Best regards,
> Krzysztof
>
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH V3 2/2] arm64: dts: qcom: Enable SD card for Shikra EVK
From: Monish Chunara @ 2026-07-02 7:33 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Vinod Koul, Neil Armstrong, Wesley Cheng,
Ulf Hansson, Kernel Team
Cc: linux-arm-msm, devicetree, linux-kernel, linux-phy, linux-mmc,
Nitin Rawat, Pradeep Pragallapati, Komal Bajaj, Monish Chunara,
Konrad Dybcio
In-Reply-To: <20260702073354.3641966-1-monish.chunara@oss.qualcomm.com>
Enable SD card for Shikra CQS, CQM and IQS EVK variants. Configure the
vmmc/vqmmc regulators and gpio-based card detection for each board
variant.
Signed-off-by: Monish Chunara <monish.chunara@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts | 9 +++++++++
arch/arm64/boot/dts/qcom/shikra-cqs-evk.dts | 9 +++++++++
arch/arm64/boot/dts/qcom/shikra-evk.dtsi | 11 +++++++++++
arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts | 9 +++++++++
4 files changed, 38 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts b/arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts
index 683b5245923b..d4a7d31fb111 100644
--- a/arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts
+++ b/arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts
@@ -7,6 +7,7 @@
#include "shikra-cqm-som.dtsi"
#include "shikra-evk.dtsi"
+#include <dt-bindings/gpio/gpio.h>
/ {
model = "Qualcomm Technologies, Inc. Shikra CQM EVK";
@@ -15,6 +16,7 @@ / {
aliases {
mmc0 = &sdhc_1;
+ mmc1 = &sdhc_2; /* SDC2 SD card slot */
serial0 = &uart0;
serial1 = &uart8;
};
@@ -95,6 +97,13 @@ &sdhc_1 {
status = "okay";
};
+&sdhc_2 {
+ vmmc-supply = <&pm4125_l21>;
+ vqmmc-supply = <&pm4125_l4>;
+
+ status = "okay";
+};
+
&uart8 {
status = "okay";
diff --git a/arch/arm64/boot/dts/qcom/shikra-cqs-evk.dts b/arch/arm64/boot/dts/qcom/shikra-cqs-evk.dts
index 26ff8007a819..a83e9fa98797 100644
--- a/arch/arm64/boot/dts/qcom/shikra-cqs-evk.dts
+++ b/arch/arm64/boot/dts/qcom/shikra-cqs-evk.dts
@@ -7,6 +7,7 @@
#include "shikra-cqm-som.dtsi"
#include "shikra-evk.dtsi"
+#include <dt-bindings/gpio/gpio.h>
/ {
model = "Qualcomm Technologies, Inc. Shikra CQS EVK";
@@ -15,6 +16,7 @@ / {
aliases {
mmc0 = &sdhc_1;
+ mmc1 = &sdhc_2; /* SDC2 SD card slot */
serial0 = &uart0;
serial1 = &uart8;
};
@@ -95,6 +97,13 @@ &sdhc_1 {
status = "okay";
};
+&sdhc_2 {
+ vmmc-supply = <&pm4125_l21>;
+ vqmmc-supply = <&pm4125_l4>;
+
+ status = "okay";
+};
+
&uart8 {
status = "okay";
diff --git a/arch/arm64/boot/dts/qcom/shikra-evk.dtsi b/arch/arm64/boot/dts/qcom/shikra-evk.dtsi
index 5411f22426b7..a9a5a6c99313 100644
--- a/arch/arm64/boot/dts/qcom/shikra-evk.dtsi
+++ b/arch/arm64/boot/dts/qcom/shikra-evk.dtsi
@@ -9,6 +9,17 @@ &qupv3_0 {
status = "okay";
};
+&sdhc_2 {
+ pinctrl-0 = <&sdc2_default &sdc2_card_det_n>;
+ pinctrl-1 = <&sdc2_sleep &sdc2_card_det_n>;
+ pinctrl-names = "default", "sleep";
+
+ no-sdio;
+ no-mmc;
+
+ cd-gpios = <&tlmm 89 GPIO_ACTIVE_LOW>;
+};
+
&tlmm {
sw_ctrl_default: sw-ctrl-default-state {
pins = "gpio88";
diff --git a/arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts b/arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts
index fd691d53a0fa..0fca5d0ac66e 100644
--- a/arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts
+++ b/arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts
@@ -7,6 +7,7 @@
#include "shikra-iqs-som.dtsi"
#include "shikra-evk.dtsi"
+#include <dt-bindings/gpio/gpio.h>
/ {
model = "Qualcomm Technologies, Inc. Shikra IQS EVK";
@@ -15,6 +16,7 @@ / {
aliases {
mmc0 = &sdhc_1;
+ mmc1 = &sdhc_2; /* SDC2 SD card slot */
serial0 = &uart0;
serial1 = &uart8;
};
@@ -103,6 +105,13 @@ &sdhc_1 {
status = "okay";
};
+&sdhc_2 {
+ vmmc-supply = <&pm8150_l10>;
+ vqmmc-supply = <&pm8150_l2>;
+
+ status = "okay";
+};
+
&uart8 {
status = "okay";
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH V3 1/2] arm64: dts: qcom: Add SD Card support for Shikra SoC
From: Monish Chunara @ 2026-07-02 7:33 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Vinod Koul, Neil Armstrong, Wesley Cheng,
Ulf Hansson, Kernel Team
Cc: linux-arm-msm, devicetree, linux-kernel, linux-phy, linux-mmc,
Nitin Rawat, Pradeep Pragallapati, Komal Bajaj, Monish Chunara,
Konrad Dybcio
In-Reply-To: <20260702073354.3641966-1-monish.chunara@oss.qualcomm.com>
Add support for SD card on Shikra SoC and enable the required pinctrl
configurations.
Signed-off-by: Monish Chunara <monish.chunara@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/shikra.dtsi | 97 ++++++++++++++++++++++++++++
1 file changed, 97 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/shikra.dtsi b/arch/arm64/boot/dts/qcom/shikra.dtsi
index e67fe047a683..02997d603099 100644
--- a/arch/arm64/boot/dts/qcom/shikra.dtsi
+++ b/arch/arm64/boot/dts/qcom/shikra.dtsi
@@ -829,6 +829,53 @@ rclk-pins {
bias-bus-hold;
};
};
+
+ sdc2_default: sdc2-default-state {
+ clk-pins {
+ pins = "sdc2_clk";
+ drive-strength = <14>;
+ bias-disable;
+ };
+
+ cmd-pins {
+ pins = "sdc2_cmd";
+ drive-strength = <14>;
+ bias-pull-up;
+ };
+
+ data-pins {
+ pins = "sdc2_data";
+ drive-strength = <14>;
+ bias-pull-up;
+ };
+ };
+
+ sdc2_sleep: sdc2-sleep-state {
+ clk-pins {
+ pins = "sdc2_clk";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ cmd-pins {
+ pins = "sdc2_cmd";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+
+ data-pins {
+ pins = "sdc2_data";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+ };
+
+ sdc2_card_det_n: sd-card-det-n-state {
+ pins = "gpio89";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
};
pmu@c91000 {
@@ -1081,6 +1128,56 @@ opp-384000000 {
};
};
+ sdhc_2: mmc@4784000 {
+ compatible = "qcom,shikra-sdhci", "qcom,sdhci-msm-v5";
+ reg = <0x0 0x04784000 0x0 0x1000>;
+
+ interrupts = <GIC_SPI 350 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 353 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "hc_irq",
+ "pwr_irq";
+
+ bus-width = <4>;
+
+ clocks = <&gcc GCC_SDCC2_AHB_CLK>,
+ <&gcc GCC_SDCC2_APPS_CLK>,
+ <&rpmcc RPM_SMD_XO_CLK_SRC>;
+ clock-names = "iface",
+ "core",
+ "xo";
+
+ qcom,dll-config = <0x0007442c>;
+ qcom,ddr-config = <0x80040868>;
+
+ iommus = <&apps_smmu 0x0a0 0x0>;
+
+ interconnects = <&system_noc MASTER_SDCC_2 RPM_ALWAYS_TAG
+ &mc_virt SLAVE_EBI_CH0 RPM_ALWAYS_TAG>,
+ <&mem_noc MASTER_AMPSS_M0 RPM_ACTIVE_TAG
+ &config_noc SLAVE_SDCC_2 RPM_ACTIVE_TAG>;
+ interconnect-names = "sdhc-ddr",
+ "cpu-sdhc";
+
+ power-domains = <&rpmpd RPMPD_VDDCX>;
+ operating-points-v2 = <&sdhc2_opp_table>;
+
+ status = "disabled";
+
+ sdhc2_opp_table: opp-table-2 {
+ compatible = "operating-points-v2";
+
+ opp-100000000 {
+ opp-hz = /bits/ 64 <100000000>;
+ required-opps = <&rpmpd_opp_low_svs>;
+ };
+
+ opp-202000000 {
+ opp-hz = /bits/ 64 <202000000>;
+ required-opps = <&rpmpd_opp_svs_plus>;
+ };
+ };
+ };
+
gpi_dma0: dma-controller@4a00000 {
compatible = "qcom,shikra-gpi-dma", "qcom,sm6350-gpi-dma";
reg = <0x0 0x04a00000 0x0 0x60000>;
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH V3 0/2] arm64: dts: qcom: Shikra SD Card support
From: Monish Chunara @ 2026-07-02 7:33 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Vinod Koul, Neil Armstrong, Wesley Cheng,
Ulf Hansson, Kernel Team
Cc: linux-arm-msm, devicetree, linux-kernel, linux-phy, linux-mmc,
Nitin Rawat, Pradeep Pragallapati, Komal Bajaj, Monish Chunara,
Konrad Dybcio
This is v3 of the Shikra SD Card support series.
This series adds SD card support for the Shikra platform.
The first patch adds the SDHC2 controller node and the necessary pinctrl
configurations to the base Shikra SoC dtsi. The second patch enables
this support on the Shikra EVK (CQS, CQM, and IQS variants) by defining
the regulator supplies and the card detection GPIO.
Signed-off-by: Monish Chunara <monish.chunara@oss.qualcomm.com>
Changes in v3:
- Padded the address part of the reg property in the sdhc_2 node to
8 hex digits with leading zeroes for consistency with other nodes
in the file
- Moved the common sdhc_2 properties to shikra-evk DT file
- Link to v2: https://lore.kernel.org/all/20260630165700.1886608-1-monish.chunara@oss.qualcomm.com
Changes in v2:
- Rectify the alignment for clocks and interconnect properties for SD
card
- Link to v1: https://lore.kernel.org/all/20260604122045.494712-1-monish.chunara@oss.qualcomm.com
Testing:
- Validated on Shikra EVK variants.
This series depends on:
- https://lore.kernel.org/all/20260612-shikra-dt-v6-2-6b6cb58db477@oss.qualcomm.com
Monish Chunara (2):
arm64: dts: qcom: Add SD Card support for Shikra SoC
arm64: dts: qcom: Enable SD card for Shikra EVK
arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts | 9 ++
arch/arm64/boot/dts/qcom/shikra-cqs-evk.dts | 9 ++
arch/arm64/boot/dts/qcom/shikra-evk.dtsi | 11 +++
arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts | 9 ++
arch/arm64/boot/dts/qcom/shikra.dtsi | 97 +++++++++++++++++++++
5 files changed, 135 insertions(+)
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH V2 2/2] arm64: dts: qcom: Enable SD card for Shikra EVK
From: Monish Chunara @ 2026-07-02 6:54 UTC (permalink / raw)
To: Konrad Dybcio, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Neil Armstrong,
Wesley Cheng, Ulf Hansson, Kernel Team
Cc: linux-arm-msm, devicetree, linux-kernel, linux-phy, linux-mmc,
Nitin Rawat, Pradeep Pragallapati, Komal Bajaj
In-Reply-To: <f5b75410-cf75-4bfb-872b-81b6180e386e@oss.qualcomm.com>
On 7/1/2026 5:08 PM, Konrad Dybcio wrote:
> On 6/30/26 6:57 PM, Monish Chunara wrote:
>> Enable SD card for Shikra CQS, CQM and IQS EVK variants. Configure the
>> vmmc/vqmmc regulators and gpio-based card detection for each board
>> variant.
>>
>> Signed-off-by: Monish Chunara <monish.chunara@oss.qualcomm.com>
>> ---
>> arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts | 18 ++++++++++++++++++
>> arch/arm64/boot/dts/qcom/shikra-cqs-evk.dts | 18 ++++++++++++++++++
>> arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts | 18 ++++++++++++++++++
>
> Move the commonalities to evk.dtsi and only override regulator
> in the board files, please
>
> Konrad
ACK, will re-arrange.
Thanks,
Monish
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH V2 1/2] arm64: dts: qcom: Add SD Card support for Shikra SoC
From: Monish Chunara @ 2026-07-02 6:52 UTC (permalink / raw)
To: Konrad Dybcio, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Neil Armstrong,
Wesley Cheng, Ulf Hansson, Kernel Team
Cc: linux-arm-msm, devicetree, linux-kernel, linux-phy, linux-mmc,
Nitin Rawat, Pradeep Pragallapati, Komal Bajaj
In-Reply-To: <b43691aa-6d1e-4df0-9d1a-cd5c4b3e3dff@oss.qualcomm.com>
On 7/1/2026 5:07 PM, Konrad Dybcio wrote:
> On 6/30/26 6:56 PM, Monish Chunara wrote:
>> Add support for SD card on Shikra SoC and enable the required pinctrl
>> configurations.
>>
>> Signed-off-by: Monish Chunara <monish.chunara@oss.qualcomm.com>
>> ---
>
> [...]
>
>> + sdhc_2: mmc@4784000 {
>> + compatible = "qcom,shikra-sdhci", "qcom,sdhci-msm-v5";
>> + reg = <0x0 0x4784000 0x0 0x1000>;
>
> Please pad the address part of reg to 8 hex digits with leading
> zeroes, like all the other nodes
>
> otherwise
>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> Konrad
>
ACK, Thanks. Will rectify in v3.
Regards,
Monish
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v4 net-next 1/2] dt-bindings: phy: cadence-torrent: Update property values to support 3 clocks
From: Krzysztof Kozlowski @ 2026-07-02 6:23 UTC (permalink / raw)
To: Gokul Praveen
Cc: conor+dt, devicetree, krzk+dt, linux-arm-kernel, linux-kernel,
linux-phy, neil.armstrong, nm, robh, sjakhade, kristo, vigneshr,
vkoul, yamonkar
In-Reply-To: <20260701142457.81874-2-g-praveen@ti.com>
On Wed, Jul 01, 2026 at 07:54:56PM +0530, Gokul Praveen wrote:
> Update maxItems value of "clocks" property to 3 as description of
> this parameter already indicates 3 clocks(refclk,pll1_refclk(optional)
> and phy_en_refclk(optional)).
But what if description is wrong? You need to provide rationale why you
are doing it and you cannot use existing code alone as that rationale,
because as you pointed out - existing code is not fully correct.
>
> Update the maxItems and items value of "clock-names" property with multiple
> combination of clock-names possible since pll1_refclk and phy_en_refclk are
> optional clocks.
Why? You need to describe why you are doing this, not what you are
doing.
>
> Signed-off-by: Gokul Praveen <g-praveen@ti.com>
> ---
> .../bindings/phy/phy-cadence-torrent.yaml | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
> index 9af39b33646a..96c664d50629 100644
> --- a/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
> +++ b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
> @@ -34,7 +34,7 @@ properties:
>
> clocks:
> minItems: 1
> - maxItems: 2
> + maxItems: 3
> description:
> PHY input reference clocks - refclk (for PLL0) & pll1_refclk (for PLL1).
> pll1_refclk is optional and used for multi-protocol configurations requiring
> @@ -45,9 +45,17 @@ properties:
>
> clock-names:
> minItems: 1
> - items:
> - - const: refclk
> - - enum: [ pll1_refclk, phy_en_refclk ]
> + maxItems: 3
Drop
> + oneOf:
> + - items:
> + - const: refclk
> + - items:
> + - const: refclk
> + - enum: [ pll1_refclk, phy_en_refclk ]
Drop these, pointless. You were supposed to grow existing syntax.
> + - items:
> + - const: refclk
> + - const: pll1_refclk
So here is the enum.
> + - const: phy_en_refclk
And this stays.
You make changes which do not make the binding better and are not
explained in commit msg. Focus on WHY you are doing things and also
explain WHY you did such complicated syntax (if you insist on rewriting
correct code into something odd we do not expect).
Best regards,
Krzysztof
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v2 2/4] dt-bindings: phy: nuvoton,ma35d1-usb2-phy: extend for dual-port OTG support
From: Joey Lu @ 2026-07-02 1:49 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Arnd Bergmann, Catalin Marinas, Jacky Huang,
Shan-Chun Hung, Hui-Ping Chen, Joey Lu, linux-phy, devicetree,
linux-arm-kernel, linux-kernel
In-Reply-To: <0f5a408b-5c55-4cd2-831a-49316c9912c9@kernel.org>
On 6/30/2026 2:16 PM, Krzysztof Kozlowski wrote:
> On 29/06/2026 12:40, Joey Lu wrote:
>> On 6/25/2026 3:58 PM, Krzysztof Kozlowski wrote:
>>> On Thu, Jun 25, 2026 at 10:39:56AM +0800, Joey Lu wrote:
>>>> properties:
>>>> compatible:
>>>> enum:
>>>> - nuvoton,ma35d1-usb2-phy
>>>>
>>>> + reg:
>>>> + maxItems: 1
>>>> +
>>>> "#phy-cells":
>>>> - const: 0
>>>> + const: 1
>>>> + description:
>>>> + The single cell selects the PHY port. 0 selects the OTG port (USB0,
>>>> + shared with DWC2 gadget controller) and 1 selects the host-only port
>>>> + (USB1).
>>>>
>>>> - clocks:
>>>> - maxItems: 1
>>> This is odd, considering that parent does not have clocks. So explain me
>>> this:
>>> 1. USB PHY needed clocks.
>>> 2. You extend USB PHY to cover second part.
>>> 3. That extension for second part means that clocks are not needed.
>>> Really, how? How is it possible in hardware?
>> The hardware has two independent clock domains:
>>
>> - The PHY analog block takes the 24 MHz HXT as its reference, wired
>> directly to the PHY's internal PLL, which derives the required
>> operating
>> frequencies internally. This reference path is entirely outside the SoC
>> software clock tree; no software-gatable clock gate needs to be enabled
>> for the PHY to power up and lock its PLL. The only software control the
>> PHY driver exercises is toggling each PHY's Power-On Reset (POR) bit,
>> which resides in the SYS register block. The driver accesses this via
>> the parent regmap
>>
>> - `HUSBH0_GATE` / `HUSBH1_GATE` / `USBD_GATE` are AHB/APB bus interface
>> clocks for the host and gadget (EHCI, OHCI, DWC2). They gate
>> the register-access path between the CPU and each controller, not
>> the PHY
>> analog circuitry itself.
>>
>> The original single-port driver enabled `HUSBH0_GATE` as if it belonged
>> to the
>> PHY, but that gate is actually owned by EHCI0/OHCI0 and is already
>> managed by
>> those controller drivers through their own `clocks` DTS bindings. The PHY
>> driver was redundantly enabling the same gate.
>>
>> When extending the driver to cover PHY1, the same pattern held: EHCI1/OHCI1
>> manage `HUSBH1_GATE` themselves. There is no clock that belongs
>> exclusively to
>> the PHY, so `clocks` will be dropped from the PHY binding entirely.
> What driver has to do with it?
>
> You did not answer the question. How adding missing OTG to existing
> device causes that hardware to lose a clock? How is it possible?
>
To answer the hardware question directly: adding OTG support does not
cause the hardware to lose a clock. The hardware clock topology is
identical in both the single-port and dual-port cases.
The PHY analog block derives its reference from the 24 MHz HXT crystal
through an always-on path via the PHY's internal PLL. No software-gated
clock in the kernel clock tree sits in this path. The PHY's power-on and
reset state is managed through USBPMISCR in the SYS register block, which
the driver accesses via the nuvoton,sys regmap outside the clock tree.
`HUSBH0_GATE` gates the AHB bus interface to the EHCI0/OHCI0 host
controllers that share PHY0. This relationship is the same in both the
single-port and dual-port configurations.
>>>> + nuvoton,rcalcode:
>>>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>>>> + minItems: 1
>>>> + maxItems: 2
>>> You should require two values. I understand that any PHY is optional,
>>> thus you skip the entry, so how would you provide value for PHY1 only?
>> `nuvoton,rcalcode` will be changed to require exactly two values
>> (`minItems: 2, maxItems: 2`), one for PHY0 and one for PHY1 respectively.
>> The property will remain optional overall; when absent, each port
>> retains its
>> power-on default value loaded at hardware initialisation. When present, both
>> entries must be supplied.
> So are you going to implement it or not?
>
Yes, I will implement this change in the next version of the patchset.
>>>> + items:
>>>> + minimum: 0
>>>> + maximum: 15
>>>> + description:
>>>> + Resistor calibration trim codes for PHY0 and PHY1 respectively.
>>>> + Each 4-bit value is written to the RCALCODE field in USBPMISCR and
>>>> + adjusts the PHY's internal termination resistance. Both entries are
>>>> + optional; when absent the hardware reset default is used.
>>>>
>>>> - nuvoton,sys:
>>>> - $ref: /schemas/types.yaml#/definitions/phandle
>>>> + nuvoton,oc-active-high:
>>>> + type: boolean
>>>> description:
>>>> - phandle to syscon for checking the PHY clock status.
>>>> + When present, the over-current detect input from the VBUS power switch
>>>> + is treated as active-high. The default (property absent) is active-low.
>>>> + This setting is shared by both USB host ports.
>>>>
>>>> required:
>>>> - compatible
>>>> + - reg
>>> That's ABI break which was not explained in the commit msg - neither
>>> specifying impact nor actually providing reasons why you break ABI.
>>>
>>> And honestly, you have no resources here except the address, so now it
>>> is clear that this should be folded into parent. See DTS101 talk slides.
>> The commit message will be updated to explicitly acknowledge the ABI break:
>> existing DTS files that contain a standalone `usb-phy` node without a `reg`
>> property will fail dt-schema validation after this change. The impact is
>> limited to the MA35D1 SoC; no upstream DTS for this SoC existed before this
>> patch series, so no in-tree board files are broken. The break is intentional
> But all of out of tree users are broken.
On reflection, the approach is being revised to eliminate the ABI break
entirely.
The next revision will keep the existing `nuvoton,ma35d1-usb2-phy` binding
intact with all its current required properties (`compatible`, `clocks`,
`nuvoton,sys`, `#phy-cells`). New OTG and dual-port capability will be
expressed as strictly optional backward-compatible additions:
- `#phy-cells` updated from `const: 0` to `enum: [0, 1]`. Existing DTS
with `#phy-cells = <0>` continues to validate and behave as before.
New DTS opting into dual-port uses `#phy-cells = <1>`.
- `nuvoton,rcalcode`: optional, exactly two values when present.
- `nuvoton,oc-active-high`: optional boolean.
>
> Best regards,
> Krzysztof
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v9 35/38] phy: rockchip: usbdp: Simplify power state handling
From: sashiko-bot @ 2026-07-02 0:12 UTC (permalink / raw)
To: Sebastian Reichel
Cc: robh, linux-phy, conor+dt, devicetree, neil.armstrong, vkoul,
olteanv
In-Reply-To: <20260702-rockchip-usbdp-cleanup-v9-35-e31efbb62d2e@collabora.com>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [Critical] Dynamic PHY re-initialization destroys active DP lane state, silently breaking the active display link.
- [High] State leak on initialization failure leaves the driver desynchronized from the PHY subsystem.
- [High] High-speed only configurations fully power on unused SuperSpeed PHY hardware, wasting power and risking timeouts.
- [High] The PHY fails to re-initialize when the Type-C mux changes between 2-lane DP and 4-lane DP while only DP is actively powered on.
- [High] Initialization failure during a mode change leaves the software state permanently desynchronized from the hardware.
- [Medium] PHY_NOTIFY_POST_RESET is sent to the DWC3 driver unconditionally, even when powering down the PHY.
--
commit e22432b71115cadf57aac2173cacace2579279d3
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
Subject: phy: rockchip: usbdp: Simplify power state handling
This commit introduces a sw_mode field to track software-requested PHY modes
independently from hardware capabilities. It centralizes power state
management and re-initialization into a single update function.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index a19b048fb39ca..51aac07ef9fe9 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -1023,66 +1015,61 @@ static int rk_udphy_parse_dt(struct rk_udphy *udphy)
> return rk_udphy_reset_init(udphy, dev);
> }
>
> -static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> +static int rk_udphy_update_power_state(struct rk_udphy *udphy)
> {
> + u8 target_mode = udphy->hw_mode & udphy->sw_mode;
> int ret;
>
> - if (!(udphy->hw_mode & mode)) {
> - dev_info(udphy->dev, "mode 0x%02x is not support\n", mode);
> + if (!udphy->phy_needs_reinit && udphy->status == target_mode)
> return 0;
[Severity: High]
Does this early return prevent the PHY from re-initializing when the Type-C
mux changes between 2-lane and 4-lane DP?
If the Type-C driver changes the mux configuration so hw_mode changes from
UDPHY_MODE_DP_USB to UDPHY_MODE_DP, but only DP is currently requested
(sw_mode == UDPHY_MODE_DP), target_mode will evaluate to UDPHY_MODE_DP in
both cases.
Because target_mode acts as a bitmask, the lane count change in hw_mode is
masked out, causing this check to succeed and entirely skip the required
re-initialization.
[ ... ]
> + if (udphy->status == UDPHY_MODE_NONE) {
> + /* Power up (incl. clocks) */
> ret = rk_udphy_setup(udphy);
> - if (ret)
> + if (ret) {
> + phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_POST_RESET);
> return ret;
> -
> - if (!udphy->hs && udphy->hw_mode & UDPHY_MODE_USB)
> - rk_udphy_u3_port_disable(udphy, false);
> - udphy->phy_needs_reinit = false;
> -
> - phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_POST_RESET);
> - } else if (udphy->phy_needs_reinit) {
> - phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_PRE_RESET);
> -
> - rk_udphy_u3_port_disable(udphy, true);
> - udelay(10);
> -
> + }
> + } else if (target_mode == UDPHY_MODE_NONE) {
> + /* Power down (incl. clocks) */
> + rk_udphy_disable(udphy);
> + } else {
> + /* Mode change => re-init */
> ret = rk_udphy_init(udphy);
[Severity: Critical]
Does dynamically re-initializing the PHY here destroy the active DP lane
state and silently break the display link?
When a concurrent mode change occurs, such as the DWC3 controller waking
from runtime suspend and requesting UDPHY_MODE_USB, rk_udphy_init is called
to re-initialize the PHY.
During initialization, rk_udphy_init unconditionally asserts resets and
writes 0 to CMN_DP_LANE_EN_ALL. However, rk_udphy_dp_lane_enable is
exclusively called during rk_udphy_dp_phy_power_on.
Since the DP lanes are never restored after this re-initialization, will
any USB runtime PM transition drop the display link without notifying the
DRM stack?
> if (ret) {
> phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_POST_RESET);
> return ret;
> }
[Severity: High]
If rk_udphy_init fails, does this leave the software state permanently
desynchronized from the hardware?
If a failure occurs during a mode change, such as a PLL lock timeout,
rk_udphy_init jumps to its assert_resets error label, leaving all PHY
hardware resets asserted.
Since this error path returns without clearing udphy->status, the software
state will believe the previous mode is actively running while the hardware
is dead. If the subsystem retries or reverts to the old mode,
udphy->status == target_mode will erroneously succeed, trapping the PHY
in a broken state.
[ ... ]
> + /* Ensure USB3 support is enabled when supported and requested */
> + if (!udphy->hs && target_mode & UDPHY_MODE_USB)
> + rk_udphy_u3_port_disable(udphy, false);
>
> - udphy->status |= mode;
> + /*
> + * Inform DWC3, that we are done with the reset, so that it can deassert
> + * its PIPE reset line.
> + */
> + phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_POST_RESET);
[Severity: Medium]
Is it safe to unconditionally send PHY_NOTIFY_POST_RESET to the DWC3 driver
here, even when target_mode == UDPHY_MODE_NONE?
Since this notification is now sent after the power-down branch calls
rk_udphy_disable, the DWC3 driver may deassert its internal reset line
and resume PIPE communication. Doing this when the PHY is completely powered
off and its clocks are disabled could lead to internal controller state
corruption or SErrors.
[ ... ]
> @@ -1091,9 +1078,11 @@ static int rk_udphy_dp_phy_power_on(struct phy *phy)
> int ret;
>
> scoped_guard(mutex, &udphy->mutex) {
> + udphy->sw_mode |= UDPHY_MODE_DP;
> +
> phy_set_bus_width(phy, udphy->dp_lanes);
>
> - ret = rk_udphy_power_on(udphy, UDPHY_MODE_DP);
> + ret = rk_udphy_update_power_state(udphy);
> if (ret)
> return ret;
[Severity: High]
If rk_udphy_update_power_state fails, does this leak the UDPHY_MODE_DP bit
in sw_mode?
The bit is added before the fallible update call and is never reverted on
error. The PHY framework considers the initialization failed and will never
call phy_power_off to clear the state. Future operations will then
incorrectly include this leaked mode in target_mode.
[ ... ]
> @@ -1329,13 +1320,9 @@ static int rk_udphy_usb3_phy_init(struct phy *phy)
>
> guard(mutex)(&udphy->mutex);
>
> - /* DP only or high-speed, disable U3 port */
> - if (!(udphy->hw_mode & UDPHY_MODE_USB) || udphy->hs) {
> - rk_udphy_u3_port_disable(udphy, true);
> - return 0;
> - }
> + udphy->sw_mode |= UDPHY_MODE_USB;
>
> - return rk_udphy_power_on(udphy, UDPHY_MODE_USB);
> + return rk_udphy_update_power_state(udphy);
> }
[Severity: High]
By removing the early return for high-speed only configurations (udphy->hs),
does this force full SuperSpeed initialization unnecessarily?
Without the early return, UDPHY_MODE_USB is unconditionally set in sw_mode.
This forces rk_udphy_update_power_state to power up the PMA/PCS and wait
for the SuperSpeed LCPLL to lock, which wastes power on unconnected or
unsupported hardware and may cause LCPLL lock timeouts.
[Severity: High]
Similar to the DP power on routine, if rk_udphy_update_power_state fails
here, does this leak the UDPHY_MODE_USB bit in sw_mode?
The bit is set before the update call and is never reverted on error,
desynchronizing the driver from the PHY subsystem.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-rockchip-usbdp-cleanup-v9-0-e31efbb62d2e@collabora.com?part=35
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v9 37/38] phy: rockchip: usbdp: Re-init PHY on mux change
From: sashiko-bot @ 2026-07-02 0:12 UTC (permalink / raw)
To: Sebastian Reichel
Cc: devicetree, vkoul, robh, conor+dt, olteanv, linux-phy,
neil.armstrong
In-Reply-To: <20260702-rockchip-usbdp-cleanup-v9-37-e31efbb62d2e@collabora.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The return value of `rk_udphy_update_power_state()` is ignored in `rk_udphy_typec_mux_set()`, silently masking hardware initialization failures from the Type-C framework.
- [High] A logic flaw in `rk_udphy_update_power_state()` causes a permanent clock reference leak and PHY state desynchronization if a mux change occurs when no PHY consumers are active.
--
commit 53a5cecf0aaba3a0de5eba24ab8177288b8ff3ea
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Re-init PHY on mux change
Ensure that the right part of the PHY are powered up when the
mode changes. This ensures the PHY is re-initialized when cable
orientation changes or when switching from DP-only to a mode with
USB support.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 735ce1bfac7af..d056da53cb1c8 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1372,6 +1372,8 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
>
> rk_udphy_set_typec_state(udphy, state->mode);
>
> + rk_udphy_update_power_state(udphy);
> +
[Severity: High]
Is it safe to ignore the return value of rk_udphy_update_power_state() here?
If hardware setup steps like PLL lock timeouts or clock enablement fail, the
error is discarded and the Type-C framework is left unaware. Could this lead
to asynchronous SErrors or kernel panics if the DWC3 controller is accessed
while the PHY is broken or disabled?
[Severity: High]
Does calling this function during a mux change when no PHY consumers
are active lead to a permanent clock reference leak?
If a Type-C mux or orientation change occurs while no PHY consumers are active
(for example, the DP driver isn't bound or USB is in runtime suspend),
udphy->sw_mode is 0, making target_mode 0.
Looking at rk_udphy_update_power_state(), it bypasses the early exit because
udphy->orientation_changed is true. Since udphy->status is 0, it calls
rk_udphy_setup() which increments clock refcounts:
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_update_power_state() {
...
if (udphy->status == UDPHY_MODE_NONE) {
/* Power up (incl. clocks) */
ret = rk_udphy_setup(udphy, target_mode);
...
}
It then assigns udphy->status = target_mode (which is 0). Because the status
remains 0 despite clocks being enabled, the next time a consumer activates
the PHY, rk_udphy_setup() is called again, double-incrementing the clock
refcounts.
During teardown, rk_udphy_disable() would only be called once, preventing the
SoC from entering deeper low-power states. Can this logic be adjusted to
properly track when clocks are enabled or avoid enabling them if target_mode
is 0?
> return 0;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-rockchip-usbdp-cleanup-v9-0-e31efbb62d2e@collabora.com?part=37
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Bryan O'Donoghue @ 2026-07-02 0:10 UTC (permalink / raw)
To: Bryan O'Donoghue, Vladimir Zapolskiy, Vinod Koul,
Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong
Cc: linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel
In-Reply-To: <6b6492a4-7610-4dce-a81c-8dc0387a4061@linaro.org>
On 02/07/2026 01:08, Bryan O'Donoghue wrote:
> Happy to revisit if the DT the Gods so command.
lol "the DT Gods"
I'm an Athiest anyway ;)
---
bod
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Bryan O'Donoghue @ 2026-07-02 0:08 UTC (permalink / raw)
To: Vladimir Zapolskiy, Bryan O'Donoghue, Vinod Koul,
Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong
Cc: linux-arm-msm, linux-phy, linux-media, devicetree, linux-kernel
In-Reply-To: <ae3a381b-5697-43ee-bdfd-aaf2d22ceedf@linaro.org>
On 02/07/2026 00:37, Vladimir Zapolskiy wrote:
>>
>> Inserting bus-type into the PHY layer is borrowing from one domain
>> linux-media and pushing the concept into PHYs.
>
> It's the other way around, "bus-type" is an endpoint property of media
> devices, reference to video-interfaces.yaml to get more details about it.
>
>> phy-cells = 1 with CPHY/DPHY specified in the consumer, as was Rob's
>> suggestion will specify the mode.
>
> This cell is just not needed, and unneeded complexity should not be added.
I'm going to stick to Rob's initial guidance on this.
https://lore.kernel.org/linux-media/20250710230846.GA44483-robh@kernel.org/
Happy to revisit if the DT the Gods so command.
---
bod
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v9 36/38] phy: rockchip: usbdp: Rename phy_needs_reinit to orientation_changed
From: sashiko-bot @ 2026-07-02 0:08 UTC (permalink / raw)
To: Sebastian Reichel
Cc: robh, conor+dt, vkoul, linux-phy, devicetree, neil.armstrong,
olteanv
In-Reply-To: <20260702-rockchip-usbdp-cleanup-v9-36-e31efbb62d2e@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] rk_udphy_update_power_state() will unexpectedly power up the PHY and leak clock references if called when status and target_mode are both UDPHY_MODE_NONE but orientation_changed is true. This problem wasn't introduced by this patch, but was introduced by a preceding patch in this series.
--
commit b963e04ecd7343df15170db1d1feeff729b0f62c
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Rename phy_needs_reinit to orientation_changed
This commit renames phy_needs_reinit to orientation_changed to better
reflect its purpose, as mode changes are now tracked separately.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -1020,7 +1020,7 @@ static int rk_udphy_update_power_state(struct rk_udphy *udphy)
> u8 target_mode = udphy->hw_mode & udphy->sw_mode;
> int ret;
>
> - if (!udphy->phy_needs_reinit && udphy->status == target_mode)
> + if (!udphy->orientation_changed && udphy->status == target_mode)
> return 0;
>
> /*
[ ... ]
> if (udphy->status == UDPHY_MODE_NONE) {
> /* Power up (incl. clocks) */
> ret = rk_udphy_setup(udphy, target_mode);
[Severity: High]
This is a pre-existing issue, but does this logic leak clock references if
called when both udphy->status and target_mode are UDPHY_MODE_NONE (0)?
If a Type-C cable is attached or the orientation is toggled while the DP
PHY client is powered off and the port negotiates a DP-only connection,
udphy->orientation_changed becomes true.
When rk_udphy_update_power_state() is subsequently called, the early return
at the top of the function is bypassed because udphy->orientation_changed
is true. Since udphy->status is UDPHY_MODE_NONE, the code takes this branch
and calls rk_udphy_setup(udphy, 0), which unconditionally prepares and
enables the clocks.
Because target_mode is 0, udphy->status remains UDPHY_MODE_NONE, meaning
subsequent genuine power-offs will fail to decrement the clock counts via
rk_udphy_disable(). Can this eventually overflow the clock reference counters?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-rockchip-usbdp-cleanup-v9-0-e31efbb62d2e@collabora.com?part=36
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v9 32/38] usb: dwc3: core: support PHY reset notifications
From: sashiko-bot @ 2026-07-02 0:08 UTC (permalink / raw)
To: Sebastian Reichel
Cc: neil.armstrong, linux-phy, conor+dt, robh, devicetree, olteanv,
vkoul
In-Reply-To: <20260702-rockchip-usbdp-cleanup-v9-32-e31efbb62d2e@collabora.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The PHY reset notifier checks `pm_runtime_suspended()` without securing a PM reference, which can cause a kernel crash if the DWC3 controller suspends concurrently.
- [High] Unsynchronized modification of the `phy_reset_in_progress` bitfield causes a data race that can corrupt adjacent bitfields in `struct dwc3`, including PM state flags like `suspended`.
- [High] The PHY reset notifier incorrectly treats a per-port PHY reset as a global event, applying soft reset to all USB3 ports and using a single global state flag, breaking multi-port controllers.
- [High] The notifier performs read-modify-write operations on `DWC3_GUSB3PIPECTL` without acquiring the DWC3 spinlock, leading to register state corruption.
--
commit e7db10d74b0389657368c887d592e9b830ec1fac
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
usb: dwc3: core: support PHY reset notifications
This adds handling for the new PHY framework reset notifications to assert
PHYSOFTRST within the DWC3 controller before the external PHY resets. This
prevents the DWC3 from interfering with PHY reinitialization and avoids
LCPLL lock failures on Rockchip platforms.
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
[ ... ]
> @@ -1430,6 +1430,7 @@ struct dwc3 {
> unsigned wakeup_configured:1;
> unsigned suspended:1;
> unsigned susphy_state:1;
> + unsigned phy_reset_in_progress:1;
[Severity: High]
Can modifying this locklessly corrupt the adjacent bitfield flags?
Since phy_reset_in_progress shares a memory word with suspended and
susphy_state, a concurrent modification to those PM state flags could be
overwritten during the non-atomic read-modify-write of this bitfield.
>
> u16 imod_interval;
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
[ ... ]
> @@ -886,10 +886,86 @@ static int dwc3_phy_init(struct dwc3 *dwc)
> return ret;
> }
>
> +static int dwc3_usb3_phy_notify(struct notifier_block *nb,
> + unsigned long action, void *data)
> +{
> + struct dwc3 *dwc = container_of(nb, struct dwc3_phy_nb, nb)->dwc;
> + int i;
> +
> + switch (action) {
> + case PHY_NOTIFY_PRE_RESET:
> + /*
> + * If the controller is already suspended (e.g. runtime PM),
> + * there is no internal state to clean up.
> + */
> + if (pm_runtime_suspended(dwc->dev))
> + return NOTIFY_OK;
[Severity: High]
Does this check protect against the device suspending immediately after?
If pm_runtime_suspended() returns false, the controller could still
asynchronously suspend before PHY_NOTIFY_POST_RESET occurs.
Should this acquire a reference via pm_runtime_get_if_active() to prevent
the device clocks from being gated while the reset is in progress?
> +
> + dwc->phy_reset_in_progress = true;
> +
> + /*
> + * Assert USB3 PHY soft reset within DWC3 before the external
> + * PHY resets. This disconnects the PIPE interface, preventing
> + * the DWC3 from interfering with PHY reinitialization and
> + * avoiding LCPLL lock failures.
> + */
> + for (i = 0; i < dwc->num_usb3_ports; i++) {
[Severity: High]
Does this unintentionally disrupt all ports on a multi-port controller?
The notifier block identifies the specific dwc3_phy_nb (and implicitly the
port) that triggered the event, but this loops over all num_usb3_ports and
asserts soft reset on all of them.
Overlapping resets on different ports might also clobber the shared
dwc->phy_reset_in_progress flag, dropping connections on ports that did
not trigger the notification.
> + u32 reg = dwc3_readl(dwc, DWC3_GUSB3PIPECTL(i));
> +
> + reg |= DWC3_GUSB3PIPECTL_PHYSOFTRST;
> + dwc3_writel(dwc, DWC3_GUSB3PIPECTL(i), reg);
[Severity: High]
Can this lockless read-modify-write corrupt DWC3_GUSB3PIPECTL?
This modifies the register without holding dwc->lock.
If this races with PM transitions running dwc3_enable_susphy(), which also
modifies this register, could one of the register updates be lost?
> + }
> + break;
> +
> + case PHY_NOTIFY_POST_RESET:
> + if (!dwc->phy_reset_in_progress)
> + return NOTIFY_OK;
> +
> + dwc->phy_reset_in_progress = false;
> +
> + /*
> + * Deassert PHY soft reset and reconfigure the PIPE interface
> + * settings after PHY reinitialization.
> + */
> + for (i = 0; i < dwc->num_usb3_ports; i++) {
> + u32 reg = dwc3_readl(dwc, DWC3_GUSB3PIPECTL(i));
[Severity: High]
If the device suspended asynchronously between PRE_RESET and POST_RESET,
could accessing this hardware register cause a synchronous external abort?
Since phy_reset_in_progress is true, this path executes unconditionally
even if the controller's clocks are now disabled.
> +
> + reg &= ~DWC3_GUSB3PIPECTL_PHYSOFTRST;
> + dwc3_writel(dwc, DWC3_GUSB3PIPECTL(i), reg);
> + }
> + break;
> + }
> +
> + return NOTIFY_OK;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-rockchip-usbdp-cleanup-v9-0-e31efbb62d2e@collabora.com?part=32
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v9 31/38] phy: core: add notifier infrastructure
From: sashiko-bot @ 2026-07-02 0:06 UTC (permalink / raw)
To: Sebastian Reichel
Cc: linux-phy, conor+dt, neil.armstrong, devicetree, robh, olteanv,
vkoul
In-Reply-To: <20260702-rockchip-usbdp-cleanup-v9-31-e31efbb62d2e@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] `phy_notify_reset` returns the raw notifier chain status instead of a standard negative error code, violating its documented return values and breaking error checking for callers.
--
commit 6d8c66a69f0a84cdc34b5b056c42d3cc35cb771e
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: core: add notifier infrastructure
This commit adds a new PHY notifier API to allow consumer drivers to hook
into reset events coming from a PHY device. This helps gracefully handle
situations where PHY devices require a reset due to configuration or cable
orientation changes.
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 21aaf2f76e53e..533473d975d37 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -542,6 +542,62 @@ int phy_notify_state(struct phy *phy, union phy_notify state)
[ ... ]
> +/**
> + * phy_notify_reset() - notify consumers of a PHY reset event
> + * @phy: the phy that is being reset
> + * @event: the notification event (PRE_RESET or POST_RESET)
> + *
> + * Called by PHY providers to notify consumers that the PHY is about to
> + * be reset or has completed a reset. This allows consumers to quiesce
> + * hardware before the PHY becomes unavailable.
> + *
> + * Returns: %0 if successful or no notifiers registered, a negative error
> + * code if a notifier returns an error (for PRE_RESET only)
> + */
> +int phy_notify_reset(struct phy *phy, enum phy_notification event)
> +{
> + if (!phy)
> + return 0;
> +
> + return blocking_notifier_call_chain(&phy->notifier, event, phy);
[Severity: Medium]
Does this return the raw notifier chain status (like NOTIFY_OK or NOTIFY_BAD)
instead of the documented negative error code?
If callers check the return value with `if (ret < 0)`, they will fail to
detect errors since NOTIFY_BAD and notifier_from_errno() results are positive
integers. Also, checking `if (ret)` might falsely detect an error on success
because NOTIFY_OK is 1.
Should this use notifier_to_errno() to convert the return value before passing
it back to the caller?
> +}
> +EXPORT_SYMBOL_GPL(phy_notify_reset);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-rockchip-usbdp-cleanup-v9-0-e31efbb62d2e@collabora.com?part=31
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox