* Re: [PATCH net-next] net: phy: mdio-i2c: defer RollBall bridge probe to PHY discovery
From: Maxime Chevallier @ 2026-06-04 21:15 UTC (permalink / raw)
To: Petr Wozniak, netdev
Cc: kuba, andrew, hkallweit1, linux, davem, edumazet, pabeni,
linux-phy, linux-kernel, bjorn, olek2
In-Reply-To: <20260604181207.17468-1-petr.wozniak@gmail.com>
Hi,
On 6/4/26 20:12, Petr Wozniak wrote:
> commit 8fe125892f40 ("net: phy: sfp: probe for RollBall I2C-to-MDIO
> bridge in mdio-i2c") introduced a regression: for SFP modules inserted
> before system boot, the RollBall I2C-to-MDIO bridge is not yet ready to
> respond to CMD_READ/CMD_DONE cycles when sfp_sm_add_mdio_bus() runs.
> The 200 ms probe times out, i2c_mii_probe_rollball() returns -ENODEV,
> and sfp_sm_add_mdio_bus() sets mdio_protocol = MDIO_I2C_NONE. By the
> time sfp_sm_probe_for_phy() runs (up to ~17 s later on affected
> hardware), the bridge is fully initialized — but PHY probing is skipped
> because the protocol has already been changed to NONE.
>
> Move the probe from i2c_mii_init_rollball() — called at bus-creation
> time — to sfp_sm_probe_for_phy() in sfp.c, where it runs after the SFP
> state machine's module initialization delays. Export the probe function
> as mdio_i2c_probe_rollball() so sfp.c can call it.
>
> For RTL8261BE-based modules: the probe correctly returns -ENODEV at PHY
> discovery time, causing sfp_sm_probe_for_phy() to destroy the MDIO bus
> and set MDIO_I2C_NONE — eliminating the 5+ minute PHY probe retry loop.
>
> For genuine RollBall modules inserted before boot (e.g. FLYPRO
> SFP-10GT-CS-30M with Aquantia AQR113C): the probe now runs after
> initialization is complete and correctly returns 0 — PHY detection
> proceeds normally.
I'm not sure I'm getting it, how come the init doesn't have time to complete
for modules that are present at boot, while it works for hotplugged modules ?
I'd expect modules to take even longer to init for hotplugged modules, as they
aren't sitting in the cage during the whole linux boot.
Maxime
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH net-next] net: phy: mdio-i2c: defer RollBall bridge probe to PHY discovery
From: Petr Wozniak @ 2026-06-04 18:12 UTC (permalink / raw)
To: netdev
Cc: kuba, andrew, hkallweit1, linux, davem, edumazet, pabeni,
linux-phy, linux-kernel, maxime.chevallier, bjorn, olek2,
Petr Wozniak
commit 8fe125892f40 ("net: phy: sfp: probe for RollBall I2C-to-MDIO
bridge in mdio-i2c") introduced a regression: for SFP modules inserted
before system boot, the RollBall I2C-to-MDIO bridge is not yet ready to
respond to CMD_READ/CMD_DONE cycles when sfp_sm_add_mdio_bus() runs.
The 200 ms probe times out, i2c_mii_probe_rollball() returns -ENODEV,
and sfp_sm_add_mdio_bus() sets mdio_protocol = MDIO_I2C_NONE. By the
time sfp_sm_probe_for_phy() runs (up to ~17 s later on affected
hardware), the bridge is fully initialized — but PHY probing is skipped
because the protocol has already been changed to NONE.
Move the probe from i2c_mii_init_rollball() — called at bus-creation
time — to sfp_sm_probe_for_phy() in sfp.c, where it runs after the SFP
state machine's module initialization delays. Export the probe function
as mdio_i2c_probe_rollball() so sfp.c can call it.
For RTL8261BE-based modules: the probe correctly returns -ENODEV at PHY
discovery time, causing sfp_sm_probe_for_phy() to destroy the MDIO bus
and set MDIO_I2C_NONE — eliminating the 5+ minute PHY probe retry loop.
For genuine RollBall modules inserted before boot (e.g. FLYPRO
SFP-10GT-CS-30M with Aquantia AQR113C): the probe now runs after
initialization is complete and correctly returns 0 — PHY detection
proceeds normally.
Reported-by: Aleksander Bajkowski <olek2@wp.pl>
Fixes: 8fe125892f40 ("net: phy: sfp: probe for RollBall I2C-to-MDIO bridge in mdio-i2c")
Signed-off-by: Petr Wozniak <petr.wozniak@gmail.com>
---
drivers/net/mdio/mdio-i2c.c | 18 ++++--------------
drivers/net/phy/sfp.c | 18 ++++++++++++------
include/linux/mdio/mdio-i2c.h | 1 +
3 files changed, 17 insertions(+), 20 deletions(-)
diff --git a/drivers/net/mdio/mdio-i2c.c b/drivers/net/mdio/mdio-i2c.c
--- a/drivers/net/mdio/mdio-i2c.c
+++ b/drivers/net/mdio/mdio-i2c.c
@@ -352,7 +352,7 @@
return 0;
}
-static int i2c_mii_probe_rollball(struct i2c_adapter *i2c)
+int mdio_i2c_probe_rollball(struct i2c_adapter *i2c)
{
u8 data_buf[] = { ROLLBALL_DATA_ADDR, 0x01, 0x00, 0x00 };
u8 cmd_buf[] = { ROLLBALL_CMD_ADDR, ROLLBALL_CMD_READ };
@@ -397,9 +397,11 @@
return -ENODEV;
}
+EXPORT_SYMBOL_GPL(mdio_i2c_probe_rollball);
static int i2c_mii_init_rollball(struct i2c_adapter *i2c)
{
+ /* Send the RollBall unlock password; bridge presence is verified
+ * later, in sfp_sm_probe_for_phy(), after module initialization. */
struct i2c_msg msg;
u8 pw[5];
int ret;
@@ -419,7 +421,7 @@
if (ret != 1)
return -EIO;
- return i2c_mii_probe_rollball(i2c);
+ return 0;
}
struct mii_bus *mdio_i2c_alloc(struct device *parent, struct i2c_adapter *i2c,
@@ -444,12 +446,8 @@
case MDIO_I2C_ROLLBALL:
ret = i2c_mii_init_rollball(i2c);
if (ret < 0) {
- if (ret != -ENODEV)
- dev_err(parent,
- "Cannot initialize RollBall MDIO I2C protocol: %d\n",
- ret);
- /* -ENODEV propagates to caller: no bridge present,
- * PHY probing should be skipped for this module. */
+ dev_err(parent,
+ "Cannot initialize RollBall MDIO I2C protocol: %d\n",
+ ret);
mdiobus_free(mii);
return ERR_PTR(ret);
}
diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -2028,17 +2028,8 @@ static int sfp_sm_add_mdio_bus(struct sfp *sfp)
dev_info(sfp->dev, "probing phy device through the [%s] protocol\n",
mdio_i2c_proto_type(sfp->mdio_protocol));
- int ret;
-
if (sfp->mdio_protocol == MDIO_I2C_NONE)
return 0;
- ret = sfp_i2c_mdiobus_create(sfp);
- if (ret == -ENODEV) {
- /* Probe confirmed no bridge present; skip PHY discovery. */
- sfp->mdio_protocol = MDIO_I2C_NONE;
- return 0;
- }
- return ret;
+ return sfp_i2c_mdiobus_create(sfp);
}
/* Probe a SFP for a PHY device if the module supports copper - the PHY
@@ -2058,8 +2049,23 @@ static int sfp_sm_probe_for_phy(struct sfp *sfp)
case MDIO_I2C_ROLLBALL:
- err = sfp_sm_probe_phy(sfp, SFP_PHY_ADDR_ROLLBALL, true);
+ /* Probe here, after module initialization delays, so that
+ * genuine RollBall bridges have had time to start up.
+ * Modules without a bridge (e.g. RTL8261BE) return -ENODEV. */
+ err = mdio_i2c_probe_rollball(sfp->i2c);
+ if (err == -ENODEV) {
+ sfp_i2c_mdiobus_destroy(sfp);
+ sfp->mdio_protocol = MDIO_I2C_NONE;
+ break;
+ }
+ if (!err)
+ err = sfp_sm_probe_phy(sfp, SFP_PHY_ADDR_ROLLBALL, true);
break;
}
diff --git a/include/linux/mdio/mdio-i2c.h b/include/linux/mdio/mdio-i2c.h
--- a/include/linux/mdio/mdio-i2c.h
+++ b/include/linux/mdio/mdio-i2c.h
@@ -33,5 +33,6 @@
struct mii_bus *mdio_i2c_alloc(struct device *parent, struct i2c_adapter *i2c,
enum mdio_i2c_proto protocol);
+int mdio_i2c_probe_rollball(struct i2c_adapter *i2c);
#endif
--
2.51.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 1/3] dt-bindings: phy: qcom,sc8280xp-qmp-ufs-phy: Add Hawi UFS PHY compatible
From: Krzysztof Kozlowski @ 2026-06-04 16:54 UTC (permalink / raw)
To: palash.kambar, vkoul, neil.armstrong, robh, krzk+dt, conor+dt,
mani, alim.akhtar, bvanassche, andersson, dmitry.baryshkov,
abel.vesa, luca.weiss
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-scsi,
nitin.rawat
In-Reply-To: <20260526090956.2340262-2-palash.kambar@oss.qualcomm.com>
On 26/05/2026 11:09, palash.kambar@oss.qualcomm.com wrote:
> From: Palash Kambar <palash.kambar@oss.qualcomm.com>
>
> Document QMP UFS PHY compatible for Hawi SoC.
>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH v2 1/1] phy: qcom: usb-hs: program MSM8x60 vendor ULPI registers on power-on
From: Herman van Hazendonk @ 2026-06-04 16:23 UTC (permalink / raw)
To: vkoul
Cc: neil.armstrong, andersson, lumag, konrad.dybcio, p.zabel,
linux-arm-msm, linux-phy, linux-kernel, Herman van Hazendonk
In-Reply-To: <20260604162352.569269-1-github.com@herrie.org>
The MSM8x60-class PHY needs three vendor-register tweaks for stable
USB operation, which the legacy msm_otg driver used to drive from
board platform data:
- reg 0x32 bits [5:4] = 11b: pre-emphasis level set to 20%
(Qualcomm reference setting, identical across every MSM8x60
reference board, HP TouchPad and the HTC MSM8660 ports).
- reg 0x36 bits 1, 2 set: CDR auto-reset and SE1 gating disabled
(also identical across every MSM8x60 board surveyed).
- reg 0x32 bits [3:0] = 5: HS driver slope. This is the only
board-specific value; the HP TouchPad uses 5 (matches the
.hsdrvslope = 0x05 from the legacy mach-msm board file), while
HTC MSM8660 ports use 1. Since the TouchPad is the only in-tree
consumer, the value is hardcoded here with a note; a
per-compatible override can be introduced when a second
MSM8x60 board lands.
The writes live behind a runtime flag that only matches
"qcom,usb-hs-phy-msm8660" so the existing MSM8226/8916/8960/8974
consumers are untouched. They are issued *after* reset_control_reset()
so the values survive the register restore the reset performs.
Note: HTC's MSM8660 vendor kernels additionally write 0x0C to reg
0x31. The HP TouchPad webOS kernel does not touch that register
and USB is stable without it, so those bits are omitted here until
documentation is available to explain what they control.
Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
---
drivers/phy/qualcomm/phy-qcom-usb-hs.c | 79 ++++++++++++++++++++++++++
1 file changed, 79 insertions(+)
diff --git a/drivers/phy/qualcomm/phy-qcom-usb-hs.c b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
index 98a18987f1be..80508885a5b0 100644
--- a/drivers/phy/qualcomm/phy-qcom-usb-hs.c
+++ b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
@@ -20,6 +20,28 @@
# define ULPI_MISC_A_VBUSVLDEXTSEL BIT(1)
# define ULPI_MISC_A_VBUSVLDEXT BIT(0)
+/*
+ * Raw ULPI vendor-register addresses programmed at probe time for the
+ * MSM8x60 / APQ8060 PHY variant. These are NOT under
+ * ULPI_EXT_VENDOR_SPECIFIC; they live in the standard ULPI vendor
+ * range (0x30-0x3f) and are addressed directly.
+ */
+#define ULPI_MSM_CONFIG_REG3 0x32
+# define ULPI_MSM_HSDRVSLOPE_MASK GENMASK(3, 0)
+# define ULPI_MSM_PRE_EMPHASIS_MASK GENMASK(5, 4)
+# define ULPI_MSM_PRE_EMPHASIS_20PCT (3 << 4)
+#define ULPI_MSM_DIGOUT_CTRL 0x36
+# define ULPI_MSM_CDR_AUTORESET BIT(1)
+# define ULPI_MSM_SE1_GATE BIT(2)
+
+/*
+ * Per-board HS driver slope. Currently only the HP TouchPad (the only
+ * MSM8x60-class consumer that depends on this PHY initialisation) is in
+ * tree; HTC MSM8660 ports historically used a value of 1 here, so when
+ * those boards are added a per-compatible override will be needed.
+ */
+#define ULPI_MSM_HSDRVSLOPE_TENDERLOIN 0x05
+
struct ulpi_seq {
u8 addr;
@@ -37,6 +59,7 @@ struct qcom_usb_hs_phy {
struct ulpi_seq *init_seq;
struct extcon_dev *vbus_edev;
struct notifier_block vbus_notify;
+ bool msm8x60_init;
};
static int qcom_usb_hs_phy_set_mode(struct phy *phy,
@@ -105,6 +128,54 @@ qcom_usb_hs_phy_vbus_notifier(struct notifier_block *nb, unsigned long event,
return ulpi_write(uphy->ulpi, addr, ULPI_MISC_A_VBUSVLDEXT);
}
+/*
+ * Apply the fixed MSM8x60-class vendor-register initialisation that the
+ * legacy msm_otg driver used to drive from board platform data. The PHY
+ * has just been reset by reset_control_reset() in qcom_usb_hs_phy_power_on(),
+ * so the registers are at their POR defaults and an RMW preserves any
+ * reserved bits the silicon expects.
+ *
+ * - reg 0x32 [5:4] pre-emphasis = 20% (Qualcomm reference setting,
+ * identical across MSM8x60 reference boards, HP TouchPad and HTC).
+ * - reg 0x32 [3:0] HS driver slope = 5 (HP TouchPad value; HTC's
+ * MSM8660 boards used 1. This is the only board-specific bit and
+ * is hardcoded for now since the TouchPad is the only in-tree
+ * consumer; a per-compatible override can be added when a second
+ * board lands.)
+ * - reg 0x36 [2:1] CDR auto-reset and SE1 gating disabled (matches
+ * every MSM8x60 reference board and HP/HTC vendor kernels).
+ *
+ * Note: HTC MSM8660 vendor kernels additionally write 0x0C to reg 0x31.
+ * The HP TouchPad webOS kernel does not touch that register and USB is
+ * stable without it, so we omit those bits until documentation is
+ * available to explain what they control.
+ */
+static int qcom_usb_hs_phy_msm8x60_init(struct qcom_usb_hs_phy *uphy)
+{
+ struct ulpi *ulpi = uphy->ulpi;
+ int reg32, reg36, ret;
+
+ reg32 = ulpi_read(ulpi, ULPI_MSM_CONFIG_REG3);
+ if (reg32 < 0)
+ return reg32;
+
+ reg32 &= ~(ULPI_MSM_PRE_EMPHASIS_MASK | ULPI_MSM_HSDRVSLOPE_MASK);
+ reg32 |= ULPI_MSM_PRE_EMPHASIS_20PCT;
+ reg32 |= ULPI_MSM_HSDRVSLOPE_TENDERLOIN & ULPI_MSM_HSDRVSLOPE_MASK;
+
+ ret = ulpi_write(ulpi, ULPI_MSM_CONFIG_REG3, reg32);
+ if (ret)
+ return ret;
+
+ reg36 = ulpi_read(ulpi, ULPI_MSM_DIGOUT_CTRL);
+ if (reg36 < 0)
+ return reg36;
+
+ reg36 |= ULPI_MSM_CDR_AUTORESET | ULPI_MSM_SE1_GATE;
+
+ return ulpi_write(ulpi, ULPI_MSM_DIGOUT_CTRL, reg36);
+}
+
static int qcom_usb_hs_phy_power_on(struct phy *phy)
{
struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
@@ -154,6 +225,12 @@ static int qcom_usb_hs_phy_power_on(struct phy *phy)
goto err_ulpi;
}
+ if (uphy->msm8x60_init) {
+ ret = qcom_usb_hs_phy_msm8x60_init(uphy);
+ if (ret)
+ goto err_ulpi;
+ }
+
if (uphy->vbus_edev) {
state = extcon_get_state(uphy->vbus_edev, EXTCON_USB);
/* setup initial state */
@@ -214,6 +291,8 @@ static int qcom_usb_hs_phy_probe(struct ulpi *ulpi)
return -ENOMEM;
ulpi_set_drvdata(ulpi, uphy);
uphy->ulpi = ulpi;
+ uphy->msm8x60_init = of_device_is_compatible(ulpi->dev.of_node,
+ "qcom,usb-hs-phy-msm8660");
size = of_property_count_u8_elems(ulpi->dev.of_node, "qcom,init-seq");
if (size < 0)
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v2 0/1] phy: qcom: usb-hs: program MSM8x60 vendor ULPI registers on power-on
From: Herman van Hazendonk @ 2026-06-04 16:23 UTC (permalink / raw)
To: vkoul
Cc: neil.armstrong, andersson, lumag, konrad.dybcio, p.zabel,
linux-arm-msm, linux-phy, linux-kernel, Herman van Hazendonk
v2:
- Drop the new "qcom,vendor-init-seq" DT property entirely, per
Dmitry's review. The values being passed in were
platform-specific (identical across every MSM8x60 SoC) plus one
board-specific value, not per-device. Move all of it into the
driver.
- Drop the corresponding dt-bindings patch (the binding gains
nothing in v2).
- Hardcode the values in the driver behind a runtime flag that
only matches "qcom,usb-hs-phy-msm8660":
* reg 0x32 [5:4] = 11b - pre-emphasis 20% (platform-wide
across every MSM8x60 reference board / HP TouchPad / HTC)
* reg 0x36 bits 1,2 set - CDR auto-reset + SE1 gating
disabled (also platform-wide)
* reg 0x32 [3:0] = 5 - HS driver slope. This is the only
board-specific value; HP TouchPad webOS kernel uses 5, HTC
MSM8660 ports historically used 1. Since the TouchPad is
the only in-tree MSM8x60 consumer, the value is hardcoded
with a comment noting that a per-compatible override will
be needed when a second board lands.
- Comment in the driver also calls out that HTC vendor kernels
additionally write 0x0C to reg 0x31. The HP TouchPad webOS
kernel never touched that register and USB is stable without
it, so those bits are omitted until documentation surfaces.
- The companion DTS patch flipping the TouchPad PHY compatible
from "qcom,usb-hs-phy-apq8064" (a different SoC) to
"qcom,usb-hs-phy-msm8660" will be sent separately to the ARM
DTS tree as that's where it belongs.
On-device validation (HP TouchPad / APQ8060):
- Booted with v2 + the DTS compatible fix. PHY driver bound,
msm_hsusb HS link came up at high-speed, BC 1.2 charger
detection passed, UDC configured cleanly.
- 200 MB scp device->host + 200 MB scp host->device, with a
concurrent ping flood running in parallel:
- both transfers 19.1-19.3 s (~10.5 MB/s, the CPU+stack
ceiling for USB-net on a 1.5 GHz Scorpion)
- md5 round-trip identical both ways (zero corruption in
~400 MB of HS traffic)
- ping flood: 60/60 received, 0% loss
- usb0 counters: 0 errors, 0 dropped, 0 carrier losses,
0 collisions across 151k packets / 220 MB
- dmesg line count unchanged (no link resets, no PHY/USB
warnings emitted during stress)
The on-device test doesn't measure the analog effect of the
pre-emphasis / driver slope settings (that needs an oscilloscope
on D+/D-), but it does confirm the writes don't regress an
already-stable HS link.
Herman van Hazendonk (1):
phy: qcom: usb-hs: program MSM8x60 vendor ULPI registers on power-on
drivers/phy/qualcomm/phy-qcom-usb-hs.c | 79 ++++++++++++++++++++++++++
1 file changed, 79 insertions(+)
base-commit: 944125b4c454b58d2fe6e35f1087a932b2050dff
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 1/3] dt-bindings: phy: qcom,sc8280xp-qmp-ufs-phy: Add Hawi UFS PHY compatible
From: Palash Kambar @ 2026-06-04 16:21 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, mani, alim.akhtar,
bvanassche, andersson, dmitry.baryshkov, abel.vesa, luca.weiss,
linux-arm-msm, linux-phy, devicetree, linux-kernel, linux-scsi,
nitin.rawat
In-Reply-To: <20260531-rigorous-gay-sturgeon-e8cfe2@quoll>
On 5/31/2026 6:03 PM, Krzysztof Kozlowski wrote:
> On Tue, May 26, 2026 at 02:39:54PM +0530, palash.kambar@oss.qualcomm.com wrote:
>> From: Palash Kambar <palash.kambar@oss.qualcomm.com>
>>
>> Document QMP UFS PHY compatible for Hawi SoC.
>
> Lack of compatibility is a mistake or intentional?
>
Hawi Phy is not compatible with any old SoC.
> Best regards,
> Krzysztof
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH] phy: freescale: imx8qm-lvds-phy: Fix missing pm_runtime_disable() on probe error path
From: Felix Gu @ 2026-06-04 14:39 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Liu Ying
Cc: linux-phy, imx, linux-arm-kernel, linux-kernel, Felix Gu
If mixel_lvds_phy_reset() fails in probe after pm_runtime_enable(),
the function returns directly without calling pm_runtime_disable(),
leaving runtime PM permanently enabled for the device.
Fix this by using devm_pm_runtime_enable() so that cleanup is
automatic on any probe failure or driver unbind. This also allows
removing the manual err label and the .remove callback.
Fixes: 06ff622d61d2 ("phy: freescale: Add i.MX8qm Mixel LVDS PHY support")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c | 24 +++++++-----------------
1 file changed, 7 insertions(+), 17 deletions(-)
diff --git a/drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c b/drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c
index ece357443521..c662f91e598c 100644
--- a/drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c
+++ b/drivers/phy/freescale/phy-fsl-imx8qm-lvds-phy.c
@@ -345,7 +345,9 @@ static int mixel_lvds_phy_probe(struct platform_device *pdev)
dev_set_drvdata(dev, priv);
- pm_runtime_enable(dev);
+ ret = devm_pm_runtime_enable(dev);
+ if (ret)
+ return ret;
ret = mixel_lvds_phy_reset(dev);
if (ret) {
@@ -355,17 +357,15 @@ static int mixel_lvds_phy_probe(struct platform_device *pdev)
for (i = 0; i < PHY_NUM; i++) {
lvds_phy = devm_kzalloc(dev, sizeof(*lvds_phy), GFP_KERNEL);
- if (!lvds_phy) {
- ret = -ENOMEM;
- goto err;
- }
+ if (!lvds_phy)
+ return -ENOMEM;
phy = devm_phy_create(dev, NULL, &mixel_lvds_phy_ops);
if (IS_ERR(phy)) {
ret = PTR_ERR(phy);
dev_err(dev, "failed to create PHY for channel%d: %d\n",
i, ret);
- goto err;
+ return ret;
}
lvds_phy->phy = phy;
@@ -379,19 +379,10 @@ static int mixel_lvds_phy_probe(struct platform_device *pdev)
if (IS_ERR(phy_provider)) {
ret = PTR_ERR(phy_provider);
dev_err(dev, "failed to register PHY provider: %d\n", ret);
- goto err;
+ return ret;
}
return 0;
-err:
- pm_runtime_disable(dev);
-
- return ret;
-}
-
-static void mixel_lvds_phy_remove(struct platform_device *pdev)
-{
- pm_runtime_disable(&pdev->dev);
}
static int __maybe_unused mixel_lvds_phy_runtime_suspend(struct device *dev)
@@ -432,7 +423,6 @@ MODULE_DEVICE_TABLE(of, mixel_lvds_phy_of_match);
static struct platform_driver mixel_lvds_phy_driver = {
.probe = mixel_lvds_phy_probe,
- .remove = mixel_lvds_phy_remove,
.driver = {
.pm = &mixel_lvds_phy_pm_ops,
.name = "mixel-lvds-phy",
---
base-commit: a225caacc36546a09586e3ece36c0313146e7da9
change-id: 20260604-lvds-d67cb619df17
Best regards,
--
Felix Gu <ustc.gu@gmail.com>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* Re: [PATCH V1 1/2] arm64: dts: qcom: Add SD Card support for Shikra SoC
From: Dmitry Baryshkov @ 2026-06-04 13:49 UTC (permalink / raw)
To: Monish Chunara
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Vinod Koul, Neil Armstrong, Wesley Cheng,
Ulf Hansson, Kernel Team, linux-arm-msm, devicetree, linux-kernel,
linux-phy, linux-mmc, Nitin Rawat, Pradeep Pragallapati,
Komal Bajaj, Konrad Dybcio
In-Reply-To: <20260604122045.494712-2-monish.chunara@oss.qualcomm.com>
On Thu, Jun 04, 2026 at 05:50:44PM +0530, 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>
> ---
> arch/arm64/boot/dts/qcom/shikra.dtsi | 93 ++++++++++++++++++++++++++++
> 1 file changed, 93 insertions(+)
>
> + clocks = <&gcc GCC_SDCC2_AHB_CLK>,
> + <&gcc GCC_SDCC2_APPS_CLK>,
> + <&rpmcc RPM_SMD_XO_CLK_SRC>;
Misaligned
> + clock-names = "iface", "core", "xo";
One perline
> +
> + 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>,
Misaligned, make sure that ampersands are at the same column.
> + <&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";
> +
--
With best wishes
Dmitry
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH V1 0/2] arm64: dts: qcom: Shikra SD Card support
From: Dmitry Baryshkov @ 2026-06-04 13:22 UTC (permalink / raw)
To: Monish Chunara
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Vinod Koul, Neil Armstrong, Wesley Cheng,
Ulf Hansson, Kernel Team, linux-arm-msm, devicetree, linux-kernel,
linux-phy, linux-mmc, Nitin Rawat, Pradeep Pragallapati,
Komal Bajaj, Konrad Dybcio
In-Reply-To: <20260604122045.494712-1-monish.chunara@oss.qualcomm.com>
On Thu, Jun 04, 2026 at 05:50:43PM +0530, Monish Chunara wrote:
> 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.
>
> Testing:
> - Validated on Shikra EVK variants.
>
> This series depends on:
> - https://lore.kernel.org/all/20260527-shikra-dt-v4-0-b5ca1fa0b392@oss.qualcomm.com/
> - https://lore.kernel.org/all/20260521-shikra-rproc-v3-0-2fca0bbe1ad7@oss.qualcomm.com/
> - https://lore.kernel.org/linux-devicetree/20260513-tsens_binding-v1-1-1780c6a6caf2@oss.qualcomm.com/
And how does SDCC depend on TSENS or remote proc?
> - https://lore.kernel.org/all/20260524-shikra_epss_l3-v1-0-b1528a436134@oss.qualcomm.com/
> - https://lore.kernel.org/all/20260522-shikra-cpufreq-scaling-v4-0-f042a25896c5@oss.qualcomm.com/
> - https://lore.kernel.org/all/20260530-shikra-dt-m1-v2-0-6bb581035d13@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 | 18 ++++
> arch/arm64/boot/dts/qcom/shikra-cqs-evk.dts | 18 ++++
> arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts | 18 ++++
> arch/arm64/boot/dts/qcom/shikra.dtsi | 93 +++++++++++++++++++++
> 4 files changed, 147 insertions(+)
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH V1 2/2] arm64: dts: qcom: Enable SD card for Shikra EVK
From: Monish Chunara @ 2026-06-04 12:20 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: <20260604122045.494712-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 | 18 ++++++++++++++++++
arch/arm64/boot/dts/qcom/shikra-cqs-evk.dts | 18 ++++++++++++++++++
arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts | 18 ++++++++++++++++++
3 files changed, 54 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts b/arch/arm64/boot/dts/qcom/shikra-cqm-evk.dts
index c2ed0396533a..53d8569a7c65 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,22 @@ &sdhc_1 {
status = "okay";
};
+&sdhc_2 {
+ vmmc-supply = <&pm4125_l21>;
+ vqmmc-supply = <&pm4125_l4>;
+
+ no-sdio;
+ no-mmc;
+
+ pinctrl-0 = <&sdc2_default &sdc2_card_det_n>;
+ pinctrl-1 = <&sdc2_sleep &sdc2_card_det_n>;
+ pinctrl-names = "default", "sleep";
+
+ cd-gpios = <&tlmm 89 GPIO_ACTIVE_LOW>;
+
+ 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 3bfd0050064f..a461c5a03d63 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,22 @@ &sdhc_1 {
status = "okay";
};
+&sdhc_2 {
+ vmmc-supply = <&pm4125_l21>;
+ vqmmc-supply = <&pm4125_l4>;
+
+ no-sdio;
+ no-mmc;
+
+ pinctrl-0 = <&sdc2_default &sdc2_card_det_n>;
+ pinctrl-1 = <&sdc2_sleep &sdc2_card_det_n>;
+ pinctrl-names = "default", "sleep";
+
+ cd-gpios = <&tlmm 89 GPIO_ACTIVE_LOW>;
+
+ status = "okay";
+};
+
&uart8 {
status = "okay";
diff --git a/arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts b/arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts
index 95bd797d009d..ede4dca3e7c0 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,22 @@ &sdhc_1 {
status = "okay";
};
+&sdhc_2 {
+ vmmc-supply = <&pm8150_l10>;
+ vqmmc-supply = <&pm8150_l2>;
+
+ no-sdio;
+ no-mmc;
+
+ pinctrl-0 = <&sdc2_default &sdc2_card_det_n>;
+ pinctrl-1 = <&sdc2_sleep &sdc2_card_det_n>;
+ pinctrl-names = "default", "sleep";
+
+ cd-gpios = <&tlmm 89 GPIO_ACTIVE_LOW>;
+
+ 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 V1 1/2] arm64: dts: qcom: Add SD Card support for Shikra SoC
From: Monish Chunara @ 2026-06-04 12:20 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: <20260604122045.494712-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 | 93 ++++++++++++++++++++++++++++
1 file changed, 93 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/shikra.dtsi b/arch/arm64/boot/dts/qcom/shikra.dtsi
index 6bac6ebac8da..6733f2efe60a 100644
--- a/arch/arm64/boot/dts/qcom/shikra.dtsi
+++ b/arch/arm64/boot/dts/qcom/shikra.dtsi
@@ -827,6 +827,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 {
@@ -1079,6 +1126,52 @@ opp-384000000 {
};
};
+ sdhc_2: mmc@4784000 {
+ compatible = "qcom,shikra-sdhci", "qcom,sdhci-msm-v5";
+ reg = <0x0 0x4784000 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 V1 0/2] arm64: dts: qcom: Shikra SD Card support
From: Monish Chunara @ 2026-06-04 12:20 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 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.
Testing:
- Validated on Shikra EVK variants.
This series depends on:
- https://lore.kernel.org/all/20260527-shikra-dt-v4-0-b5ca1fa0b392@oss.qualcomm.com/
- https://lore.kernel.org/all/20260521-shikra-rproc-v3-0-2fca0bbe1ad7@oss.qualcomm.com/
- https://lore.kernel.org/linux-devicetree/20260513-tsens_binding-v1-1-1780c6a6caf2@oss.qualcomm.com/
- https://lore.kernel.org/all/20260524-shikra_epss_l3-v1-0-b1528a436134@oss.qualcomm.com/
- https://lore.kernel.org/all/20260522-shikra-cpufreq-scaling-v4-0-f042a25896c5@oss.qualcomm.com/
- https://lore.kernel.org/all/20260530-shikra-dt-m1-v2-0-6bb581035d13@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 | 18 ++++
arch/arm64/boot/dts/qcom/shikra-cqs-evk.dts | 18 ++++
arch/arm64/boot/dts/qcom/shikra-iqs-evk.dts | 18 ++++
arch/arm64/boot/dts/qcom/shikra.dtsi | 93 +++++++++++++++++++++
4 files changed, 147 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 v8 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Bryan O'Donoghue @ 2026-06-04 11:04 UTC (permalink / raw)
To: Vladimir Zapolskiy, Bryan O'Donoghue, Vijay Kumar Tumati,
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: <9ab0d8f4-e1b6-415b-976b-721ab7a29194@linaro.org>
On 04/06/2026 10:20, Vladimir Zapolskiy wrote:
> On 6/4/26 12:06, Bryan O'Donoghue wrote:
>> On 04/06/2026 09:46, Vladimir Zapolskiy wrote:
>>> On 6/4/26 03:30, Bryan O'Donoghue wrote:
>>>> On 04/06/2026 01:07, Vladimir Zapolskiy wrote:
>>>>> On 6/4/26 00:18, Bryan O'Donoghue wrote:
>>>>>> On 03/06/2026 21:51, Vladimir Zapolskiy wrote:
>>>>>>>> Actually, one more thing, Why isn't TITAN TOP GDSC here?>>>> +
>>>>>>> If CSIPHYs are true subdevices under the umbrella CAMSS device and
>>>>>>> well
>>>>>>> described as subnodes, then likely none of power domains are needed
>>>>>>> to be
>>>>>>> repeatedly described in the children device nodes, since this
>>>>>>> information
>>>>>>> can be obtained from the parent device by the driver.
>>>>>>>
>>>>>>> Technically 'power-domains' property can be safely removed, I
>>>>>>> believe.
>>>>>>
>>>>>> The policy is to describe the power-domain dependency fully since DT
>>>>>> describes hardware not software architecture.
>>>>>
>>>>> It brings no contardiction to the statement I've given above, the
>>>>> needed
>>>>> power domans will be properly described in the parent device, and
>>>>> there
>>>>> is no
>>>>> sense to repeat the properties it again and again in every child
>>>>> subdevice.
>>>>>
>>>>>> Also for a very practical reason a sub-devices can probe/run
>>>>>> asynchronously of the parent device being active so in fact we do
>>>>>> need
>>>>>> to describe the PDs fully.
>>>>>
>>>>> In opposite to the above this one is precisely a software centric
>>>>> argument,
>>>>> which should be excluded from the consideration, as well it's not a
>>>>> big
>>>>> deal to make a proper async initialization, removing excessive dt
>>>>> properties
>>>>> is worth it.
>>>>>
>>>>
>>>> Right look forget about that.
>>>>
>>>> - DT requires you to describe your hardware. You're not entitled to
>>>> have
>>>> some other device vote for a clock or a PD you rely on.
>>>>
>>>
>>> Above are two uncorrelated between each other sentences.
>>>
>>> A device ("consumer") can ask another device ("provider") to behave in
>>> one or another way, this is the only possible and thus natually selected
>>> system design, and nothing behind it was asked. There is no
>>> justification
>>> for the proposed flood of multiply repeated data, it's avoidable.
>>
>> CAMSS or rather the components of CAMSS modelled in the current node,
>> is/are not the provider of the GDSCs or the power-domains, it/they are
>> consumers themselves from CAMCC.
>
> Well, this is the argument about software, and software can be changed.
>
>> The producer/consumer model is CAMCC to components within the Camera
>> block. Some components depend on say MXA, MXC, some do not. Nothing in
>> CAMSS itself is a power-domain provider.
>>>> That's exactly the type of downstream short cut we are trying
>>>> to zap.
>>>>
>>>> - In our case we also need to vote on PDs individually when the PHY
>>>> is active.
>>>>
>>>> In extremis say we are only running the TPG then we have no reason to
>>>> vote for CSIPHY specific rails or operating points in the parent
>>>> device.
>>>
>>> So, TPG shall communicate with CAMSS, there is no CSIPHY in the
>>> equation.
>>
>> Right but it would be inappropriate to enable all of the PDs for all of
>> the components in the CAMSS block when we can do so more granularly.
>
> Whenever it is actually necessary, it should be possible to split PDs into
> generic/parent and subdevice specific groups, it's a part of software
> implementation. In some cases there might be no need to define any child
> side PDs, likely CSIPHY falls into this category.
Completely not a discussion about software. DT needs to represent
hardware. Its a different mindset.
I don't think we are going to resolve this debating the same thing again.
My position is the CSIPHY should list the set of power-domain
dependencies it has. The PDs don't come from any other thing in the
CAMSS block so the producer/consumer model is CSIPHY to CAMCC and CSIPHY
to RPMPD respectively.
I'll just ask @Rob, @Krzysztof or @Conor to offer up their opinion as DT
maintainers and work from there.
---
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-06-04 10:54 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: <478df3ed-d4ef-43aa-bb84-e2075798542b@kernel.org>
On 6/3/26 01:51, Bryan O'Donoghue wrote:
> On 02/06/2026 22:59, Vladimir Zapolskiy wrote:
>> On 5/23/26 05:48, Bryan O'Donoghue wrote:
>>> Add a base schema initially compatible with x1e80100 to describe MIPI CSI2
>>> PHY devices.
>>>
>>> The hardware can support both CPHY, DPHY and a special split-mode DPHY.
>>>
>>> The schema here defines three ports:
>>>
>>> port@0:
>>> The first input port where a sensor is always required.
>>>
>>> port@1:
>>> A second optional input port which if present implies DPHY split-mode.
>>>
>>> port@2:
>>> A third always required output port which connects to the controller.
>>>
>>
>> This port numeration is imperfect, because port@0 and port@2 are required,
>> while middle port@1 is optional.
>>
>> Like it was stated before a number of times, it seems natural to operate
>> with two ports, where input port may have two endpoints rather than 3 ports,
>> also that approach solves the problem of a hole in the port numeration.
>
> Can you confirm this is what you are after ?
>
> port@0 {
> #address-cells = <1>;
> #size-cells = <0>;
>
> endpoint@0 { /* primary sensor */
> reg = <0>;
> data-lanes = <0 1 2 3>;
> remote-endpoint = <&sensor0_out>;
> };
>
> endpoint@1 { /* split-mode second sensor, optional */
> reg = <1>;
> data-lanes = <0>;
> remote-endpoint = <&sensor1_out>;
> };
> };
>
> port@1 { /* output to CAMSS, was port@2 */
> endpoint { remote-endpoint = <&controller_in>; };
> };
>
> This works for me BTW.
>
Something like this will be a much better description, below are additional
notes:
1. for endpoints to sensors data-lanes and clock-lanes should be specified,
2. as it was stated before the numeration should start from 1,
3. output port may be just removed, otherwise there are supposed to be 3 (!)
links between the umbrella CAMSS and children CSIPHY devices, namely the
links are a) parent-child layout, b) phy connection and c) media endpoints,
it sounds as an excessive overload, where a simplification is possible.
--
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 2/2] phy: nuvoton: Add MA35D1 USB2 OTG PHY driver
From: sashiko-bot @ 2026-06-04 10:23 UTC (permalink / raw)
To: Joey Lu
Cc: olteanv, robh, devicetree, neil.armstrong, conor+dt, linux-phy,
vkoul
In-Reply-To: <20260604101220.1092822-3-a0987203069@gmail.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The driver implements a dummy USB role switch that falsely reports success without modifying hardware state.
- [High] Race condition exposing the PHY provider before initialization completes leads to a Use-After-Free.
- [High] PHY initialization fails due to polling clock stability bits while the PHY clock is disabled.
- [Medium] Ignoring the return value of `regmap_read()` results in the use of uninitialized stack memory.
--
commit 9e57cc2572b36a8f75f7ca9bc25a14c367c18b1a
Author: Joey Lu <a0987203069@gmail.com>
Subject: phy: nuvoton: Add MA35D1 USB2 OTG PHY driver
Add a PHY driver for the USB 2.0 PHYs in the Nuvoton MA35D1 SoC. It
implements Power-On Reset sequence, optional resistor calibration trim,
and a USB role switch.
> diff --git a/drivers/phy/nuvoton/phy-ma35d1-otg.c b/drivers/phy/nuvoton/phy-ma35d1-otg.c
> new file mode 100644
> index 0000000000000..53bc6ddf755e6
> --- /dev/null
> +++ b/drivers/phy/nuvoton/phy-ma35d1-otg.c
[ ... ]
> +static int ma35_otg_phy_init(struct phy *phy)
> +{
> + struct ma35_otg_phy *p = phy_get_drvdata(phy);
> + unsigned int n = p->phy_idx;
> + u32 ready_mask = USBPMISCR_PHY_HOST_READY(n);
> + unsigned int val;
> + int ret;
> +
> + regmap_read(p->sysreg, MA35_SYS_USBPMISCR, &val);
> + if ((val & ready_mask) == ready_mask)
> + return 0;
[Severity: Medium]
Does ignoring the return value of regmap_read() risk evaluating uninitialized
stack memory here?
If the read fails, val remains uninitialized, and using it in the bitwise
operation could cause non-deterministic behavior, such as incorrectly skipping
the PHY power-on reset sequence.
> +
> + regmap_update_bits(p->sysreg, MA35_SYS_USBPMISCR,
> + USBPMISCR_PHY_CTL_MASK(n),
> + USBPMISCR_PHY_POR(n) | USBPMISCR_PHY_SUSPEND(n));
> + msleep(20);
> +
> + regmap_update_bits(p->sysreg, MA35_SYS_USBPMISCR,
> + USBPMISCR_PHY_CTL_MASK(n),
> + USBPMISCR_PHY_SUSPEND(n));
> +
> + ret = regmap_read_poll_timeout(p->sysreg, MA35_SYS_USBPMISCR, val,
> + (val & ready_mask) == ready_mask,
> + 10, 1000);
[Severity: High]
Is it possible for this polling to time out because the PHY clock isn't
enabled yet?
The polling expects the PHY clock to be stable, but the PHY clock (p->clk) is
only enabled later in ma35_otg_phy_power_on(). Because the Linux PHY framework
guarantees that phy_init() is called before phy_power_on(), the PHY reference
clock remains disabled during this stability polling.
> + if (ret) {
> + dev_err(p->dev, "USB PHY%u clock not stable (USBPMISCR=0x%08x)\n",
> + n, val);
> + return ret;
> + }
> +
> + return 0;
> +}
[ ... ]
> +static enum usb_role ma35_otg_read_id(struct ma35_otg_phy *p)
> +{
> + unsigned int val;
> +
> + regmap_read(p->sysreg, MA35_SYS_PWRONOTP, &val);
> + return (val & PWRONOTP_USBP0ID) ? USB_ROLE_HOST : USB_ROLE_DEVICE;
> +}
[Severity: Medium]
Can this return an arbitrary USB role if regmap_read() fails?
If the read fails, the uninitialized stack variable val is evaluated, leading
to a potentially incorrect role being reported.
> +
> +static int ma35_otg_role_sw_set(struct usb_role_switch *sw,
> + enum usb_role role)
> +{
> + struct ma35_otg_phy *p = usb_role_switch_get_drvdata(sw);
> +
> + p->cur_role = role;
> +
> + return 0;
> +}
[Severity: High]
Since the hardware mux is entirely automatic based on the physical USB ID pin,
should this .set callback return an error instead of falsely reporting success?
Writing to p->cur_role doesn't configure any hardware register, and the .get
callback correctly queries the hardware directly. Returning 0 here defeats the
subsystem's -EOPNOTSUPP rejection for read-only switches, as writes from
userspace will appear to succeed but will not change the actual hardware state.
> +
> +static enum usb_role ma35_otg_role_sw_get(struct usb_role_switch *sw)
> +{
> + struct ma35_otg_phy *p = usb_role_switch_get_drvdata(sw);
> +
> + return ma35_otg_read_id(p);
> +}
> +
> +static int ma35_otg_role_switch_init(struct platform_device *pdev,
> + struct ma35_otg_phy *p)
> +{
> + struct usb_role_switch_desc sw_desc = { };
> +
> + p->cur_role = ma35_otg_read_id(p);
> +
> + sw_desc.set = ma35_otg_role_sw_set;
> + sw_desc.get = ma35_otg_role_sw_get;
> + sw_desc.allow_userspace_control = true;
[Severity: High]
Is it appropriate to set allow_userspace_control to true given that the switch
strictly reflects an automatic hardware state?
> + sw_desc.driver_data = p;
> + sw_desc.fwnode = dev_fwnode(&pdev->dev);
[ ... ]
> +static int ma35_otg_phy_probe(struct platform_device *pdev)
> +{
[ ... ]
> + provider = devm_of_phy_provider_register(&pdev->dev,
> + of_phy_simple_xlate);
> + if (IS_ERR(provider))
> + return dev_err_probe(&pdev->dev, PTR_ERR(provider),
> + "Failed to register PHY provider\n");
> +
> + if (p->phy_idx == 0) {
> + ret = ma35_otg_role_switch_init(pdev, p);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
[Severity: High]
Could registering the PHY provider before role switch initialization risk a
use-after-free?
If devm_of_phy_provider_register() exposes the PHY globally, a consumer on
another CPU could look up the PHY and call phy_init(), acquiring a reference.
If ma35_otg_role_switch_init() subsequently fails, probe returns an error,
triggering the devm cleanup which frees the struct ma35_otg_phy *p memory.
Because the consumer already holds a valid reference, subsequent calls to
PHY APIs will dereference the freed pointer.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260604101220.1092822-1-a0987203069@gmail.com?part=2
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: phy: nuvoton: Add MA35D1 USB2 OTG PHY binding
From: sashiko-bot @ 2026-06-04 10:19 UTC (permalink / raw)
To: Joey Lu
Cc: vkoul, conor+dt, olteanv, devicetree, linux-phy, robh,
neil.armstrong
In-Reply-To: <20260604101220.1092822-2-a0987203069@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The binding incorrectly represents a multi-port PHY controller as multiple independent nodes instead of a single provider with `#phy-cells = <1>`.
--
commit edfd20765d416e895bc152da5eefd3bf59f431bd
Author: Joey Lu <a0987203069@gmail.com>
dt-bindings: phy: nuvoton: Add MA35D1 USB2 OTG PHY binding
Adds the device tree binding documentation for the Nuvoton MA35D1 USB 2.0
OTG PHY driver. The PHY supports two ports (OTG and host-only) and allows
board-specific resistor calibration and over-current detect polarity.
> diff --git a/Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy-otg.yaml b/Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy-otg.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy-otg.yaml
[ ... ]
> + nuvoton,sys:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + items:
> + - items:
> + - description: phandle to the system management syscon.
> + - description: PHY instance index.
> + enum:
> + - 0 # USB0, OTG port (shared with DWC2 gadget controller)
> + - 1 # USB1, host-only port
> + description:
> + A phandle to the syscon node covering the SYS register block, with
> + one argument selecting the PHY instance. Index 0 selects the OTG
> + port PHY (USB0) and index 1 selects the host-only PHY (USB1).
> +
> + "#phy-cells":
> + const: 0
[Severity: Medium]
Should this multi-port PHY controller be represented as a single provider
node with #phy-cells = <1> instead of embedding the index in nuvoton,sys?
Using independent nodes for each port with #phy-cells = <0> sidesteps the
standard consumer argument pattern and forces non-standard node names to
avoid collisions at the same tree level.
[ ... ]
> + nuvoton,oc-active-high:
> + type: boolean
> + description:
> + 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.
Since this property is explicitly described as shared by both ports, does
instantiating separate nodes per port risk conflicting states if the nodes
configure this property differently?
A single PHY provider node would allow shared properties to be defined once,
while consumers could specify their port index via standard phys arguments.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260604101220.1092822-1-a0987203069@gmail.com?part=1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH 2/2] phy: nuvoton: Add MA35D1 USB2 OTG PHY driver
From: Joey Lu @ 2026-06-04 10:12 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jacky Huang,
Shan-Chun Hung, linux-phy, devicetree, linux-arm-kernel,
linux-kernel, Joey Lu
In-Reply-To: <20260604101220.1092822-1-a0987203069@gmail.com>
Add a PHY driver for the USB 2.0 PHYs in the Nuvoton MA35D1 SoC,
intended for use with the EHCI and OHCI host controllers.
The MA35D1 SoC has two USB ports:
- USB0: an OTG port shared between a DWC2 gadget controller and
EHCI0/OHCI0 host controllers. A hardware mux automatically routes
the physical USB0 signals to the appropriate controller based on the
USB ID pin state. The DWC2 IP is device-only in hardware,
so host-mode operation on USB0 is handled entirely by EHCI0/OHCI0.
- USB1: a dedicated host-only port served by EHCI1/OHCI1.
The driver implements:
- Power-On Reset sequence with a guard that skips re-initialization if
the PHY is already operational. This protects PHY0 when the DWC2
gadget driver has already run its own init before EHCI0 probes.
- Optional resistor calibration trim via nuvoton,rcalcode.
- Optional over-current detect polarity via nuvoton,oc-active-high.
- For PHY0 only: a USB role switch that exposes the hardware ID pin
state (PWRONOTP[16]).
Signed-off-by: Joey Lu <a0987203069@gmail.com>
---
drivers/phy/nuvoton/Kconfig | 15 ++
drivers/phy/nuvoton/Makefile | 1 +
drivers/phy/nuvoton/phy-ma35d1-otg.c | 264 +++++++++++++++++++++++++++
3 files changed, 280 insertions(+)
create mode 100644 drivers/phy/nuvoton/phy-ma35d1-otg.c
diff --git a/drivers/phy/nuvoton/Kconfig b/drivers/phy/nuvoton/Kconfig
index d02cae2db315..5fdd13f841e7 100644
--- a/drivers/phy/nuvoton/Kconfig
+++ b/drivers/phy/nuvoton/Kconfig
@@ -10,3 +10,18 @@ config PHY_MA35_USB
help
Enable this to support the USB2.0 PHY on the Nuvoton MA35
series SoCs.
+
+config PHY_MA35_USB_OTG
+ tristate "Nuvoton MA35 USB2.0 OTG PHY driver"
+ depends on ARCH_MA35 || COMPILE_TEST
+ depends on OF
+ select GENERIC_PHY
+ select MFD_SYSCON
+ select USB_ROLE_SWITCH
+ help
+ Enable this to support the USB2.0 OTG PHY on the Nuvoton MA35
+ series SoCs. This driver handles PHY initialization for the
+ EHCI/OHCI host controllers, including per-PHY power-on reset,
+ resistor calibration trim, and over-current polarity
+ configuration. For the OTG port (PHY0), it also monitors the
+ USB ID pin and registers a USB role switch.
diff --git a/drivers/phy/nuvoton/Makefile b/drivers/phy/nuvoton/Makefile
index 2937e3921898..3ecd76f35d7c 100644
--- a/drivers/phy/nuvoton/Makefile
+++ b/drivers/phy/nuvoton/Makefile
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_PHY_MA35_USB) += phy-ma35d1-usb2.o
+obj-$(CONFIG_PHY_MA35_USB_OTG) += phy-ma35d1-otg.o
diff --git a/drivers/phy/nuvoton/phy-ma35d1-otg.c b/drivers/phy/nuvoton/phy-ma35d1-otg.c
new file mode 100644
index 000000000000..53bc6ddf755e
--- /dev/null
+++ b/drivers/phy/nuvoton/phy-ma35d1-otg.c
@@ -0,0 +1,264 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Nuvoton MA35D1 USB 2.0 OTG PHY driver
+ *
+ * PHY0 (USB0) is shared between DWC2 gadget and EHCI0/OHCI0 host
+ * controllers. The hardware mux switches automatically via the USB
+ * ID pin. PHY1 (USB1) is host-only.
+ *
+ * Copyright (C) 2026 Nuvoton Technology Corp.
+ */
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/usb/role.h>
+
+#define MA35_SYS_PWRONOTP 0x04
+#define PWRONOTP_USBP0ID BIT(16)
+
+#define MA35_SYS_USBPMISCR 0x60
+#define USBPMISCR_PHY_POR(n) BIT(0 + (n) * 16)
+#define USBPMISCR_PHY_SUSPEND(n) BIT(1 + (n) * 16)
+#define USBPMISCR_PHY_COMN(n) BIT(2 + (n) * 16)
+#define USBPMISCR_PHY_HSTCKSTB(n) BIT(8 + (n) * 16)
+#define USBPMISCR_PHY_CK12MSTB(n) BIT(9 + (n) * 16)
+/* Mask for control bits (POR, SUSPEND, COMN) of one PHY */
+#define USBPMISCR_PHY_CTL_MASK(n) (0x7 << ((n) * 16))
+/* Host-mode ready: SUSPEND + HSTCKSTB + CK12MSTB */
+#define USBPMISCR_PHY_HOST_READY(n) (USBPMISCR_PHY_SUSPEND(n) | \
+ USBPMISCR_PHY_HSTCKSTB(n) | \
+ USBPMISCR_PHY_CK12MSTB(n))
+/* RCALCODE: 4-bit resistor trim at bits [15:12] (PHY0) or [31:28] (PHY1) */
+#define USBPMISCR_RCAL_SHIFT(n) (12 + (n) * 16)
+#define USBPMISCR_RCAL_MASK(n) GENMASK(USBPMISCR_RCAL_SHIFT(n) + 3, \
+ USBPMISCR_RCAL_SHIFT(n))
+
+#define MA35_SYS_MISCFCR0 0x70
+/* MISCFCR0[12]: USB host over-current detect polarity (shared, both ports) */
+#define MISCFCR0_UHOVRCURH BIT(12)
+
+struct ma35_otg_phy {
+ struct clk *clk;
+ struct device *dev;
+ struct regmap *sysreg;
+ unsigned int phy_idx;
+ struct usb_role_switch *role_sw;
+ enum usb_role cur_role;
+};
+
+static int ma35_otg_phy_init(struct phy *phy)
+{
+ struct ma35_otg_phy *p = phy_get_drvdata(phy);
+ unsigned int n = p->phy_idx;
+ u32 ready_mask = USBPMISCR_PHY_HOST_READY(n);
+ unsigned int val;
+ int ret;
+
+ regmap_read(p->sysreg, MA35_SYS_USBPMISCR, &val);
+ if ((val & ready_mask) == ready_mask)
+ return 0;
+
+ regmap_update_bits(p->sysreg, MA35_SYS_USBPMISCR,
+ USBPMISCR_PHY_CTL_MASK(n),
+ USBPMISCR_PHY_POR(n) | USBPMISCR_PHY_SUSPEND(n));
+ msleep(20);
+
+ regmap_update_bits(p->sysreg, MA35_SYS_USBPMISCR,
+ USBPMISCR_PHY_CTL_MASK(n),
+ USBPMISCR_PHY_SUSPEND(n));
+
+ ret = regmap_read_poll_timeout(p->sysreg, MA35_SYS_USBPMISCR, val,
+ (val & ready_mask) == ready_mask,
+ 10, 1000);
+ if (ret) {
+ dev_err(p->dev, "USB PHY%u clock not stable (USBPMISCR=0x%08x)\n",
+ n, val);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int ma35_otg_phy_power_on(struct phy *phy)
+{
+ struct ma35_otg_phy *p = phy_get_drvdata(phy);
+
+ return clk_prepare_enable(p->clk);
+}
+
+static int ma35_otg_phy_power_off(struct phy *phy)
+{
+ struct ma35_otg_phy *p = phy_get_drvdata(phy);
+
+ clk_disable_unprepare(p->clk);
+ return 0;
+}
+
+static const struct phy_ops ma35_otg_phy_ops = {
+ .init = ma35_otg_phy_init,
+ .power_on = ma35_otg_phy_power_on,
+ .power_off = ma35_otg_phy_power_off,
+ .owner = THIS_MODULE,
+};
+
+static enum usb_role ma35_otg_read_id(struct ma35_otg_phy *p)
+{
+ unsigned int val;
+
+ regmap_read(p->sysreg, MA35_SYS_PWRONOTP, &val);
+ return (val & PWRONOTP_USBP0ID) ? USB_ROLE_HOST : USB_ROLE_DEVICE;
+}
+
+static int ma35_otg_role_sw_set(struct usb_role_switch *sw,
+ enum usb_role role)
+{
+ struct ma35_otg_phy *p = usb_role_switch_get_drvdata(sw);
+
+ p->cur_role = role;
+
+ return 0;
+}
+
+static enum usb_role ma35_otg_role_sw_get(struct usb_role_switch *sw)
+{
+ struct ma35_otg_phy *p = usb_role_switch_get_drvdata(sw);
+
+ return ma35_otg_read_id(p);
+}
+
+static int ma35_otg_role_switch_init(struct platform_device *pdev,
+ struct ma35_otg_phy *p)
+{
+ struct usb_role_switch_desc sw_desc = { };
+
+ p->cur_role = ma35_otg_read_id(p);
+
+ sw_desc.set = ma35_otg_role_sw_set;
+ sw_desc.get = ma35_otg_role_sw_get;
+ sw_desc.allow_userspace_control = true;
+ sw_desc.driver_data = p;
+ sw_desc.fwnode = dev_fwnode(&pdev->dev);
+
+ p->role_sw = usb_role_switch_register(&pdev->dev, &sw_desc);
+ if (IS_ERR(p->role_sw))
+ return dev_err_probe(&pdev->dev, PTR_ERR(p->role_sw),
+ "failed to register role switch\n");
+
+ return 0;
+}
+
+static void ma35_otg_role_switch_exit(struct ma35_otg_phy *p)
+{
+ if (!p->role_sw)
+ return;
+
+ usb_role_switch_unregister(p->role_sw);
+ p->role_sw = NULL;
+}
+
+static int ma35_otg_phy_probe(struct platform_device *pdev)
+{
+ struct phy_provider *provider;
+ struct ma35_otg_phy *p;
+ unsigned int sys_args[1];
+ struct phy *phy;
+ u32 rcalcode;
+ int ret;
+
+ p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL);
+ if (!p)
+ return -ENOMEM;
+
+ p->dev = &pdev->dev;
+ platform_set_drvdata(pdev, p);
+
+ p->sysreg = syscon_regmap_lookup_by_phandle_args(pdev->dev.of_node,
+ "nuvoton,sys",
+ 1, sys_args);
+ if (IS_ERR(p->sysreg))
+ return dev_err_probe(&pdev->dev, PTR_ERR(p->sysreg),
+ "Failed to get SYS regmap\n");
+
+ p->phy_idx = sys_args[0];
+
+ if (p->phy_idx > 1)
+ return dev_err_probe(&pdev->dev, -EINVAL,
+ "invalid PHY index %u (must be 0 or 1)\n",
+ p->phy_idx);
+
+ p->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(p->clk))
+ return dev_err_probe(&pdev->dev, PTR_ERR(p->clk),
+ "failed to get PHY clock\n");
+
+ if (!of_property_read_u32(pdev->dev.of_node, "nuvoton,rcalcode",
+ &rcalcode)) {
+ if (rcalcode > 15)
+ return dev_err_probe(&pdev->dev, -EINVAL,
+ "rcalcode %u out of range (0-15)\n",
+ rcalcode);
+ regmap_update_bits(p->sysreg, MA35_SYS_USBPMISCR,
+ USBPMISCR_RCAL_MASK(p->phy_idx),
+ rcalcode << USBPMISCR_RCAL_SHIFT(p->phy_idx));
+ }
+
+ if (of_property_read_bool(pdev->dev.of_node, "nuvoton,oc-active-high"))
+ regmap_update_bits(p->sysreg, MA35_SYS_MISCFCR0,
+ MISCFCR0_UHOVRCURH, MISCFCR0_UHOVRCURH);
+
+ phy = devm_phy_create(&pdev->dev, pdev->dev.of_node, &ma35_otg_phy_ops);
+ if (IS_ERR(phy))
+ return dev_err_probe(&pdev->dev, PTR_ERR(phy),
+ "Failed to create PHY\n");
+
+ phy_set_drvdata(phy, p);
+
+ provider = devm_of_phy_provider_register(&pdev->dev,
+ of_phy_simple_xlate);
+ if (IS_ERR(provider))
+ return dev_err_probe(&pdev->dev, PTR_ERR(provider),
+ "Failed to register PHY provider\n");
+
+ if (p->phy_idx == 0) {
+ ret = ma35_otg_role_switch_init(pdev, p);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static void ma35_otg_phy_remove(struct platform_device *pdev)
+{
+ struct ma35_otg_phy *p = platform_get_drvdata(pdev);
+
+ ma35_otg_role_switch_exit(p);
+}
+
+static const struct of_device_id ma35_otg_phy_of_match[] = {
+ { .compatible = "nuvoton,ma35d1-usb2-phy-otg" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ma35_otg_phy_of_match);
+
+static struct platform_driver ma35_otg_phy_driver = {
+ .probe = ma35_otg_phy_probe,
+ .remove = ma35_otg_phy_remove,
+ .driver = {
+ .name = "ma35d1-usb2-phy-otg",
+ .of_match_table = ma35_otg_phy_of_match,
+ },
+};
+module_platform_driver(ma35_otg_phy_driver);
+
+MODULE_DESCRIPTION("Nuvoton MA35D1 USB 2.0 OTG PHY driver");
+MODULE_AUTHOR("Joey Lu <a0987203069@gmail.com>");
+MODULE_LICENSE("GPL");
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH 1/2] dt-bindings: phy: nuvoton: Add MA35D1 USB2 OTG PHY binding
From: Joey Lu @ 2026-06-04 10:12 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jacky Huang,
Shan-Chun Hung, linux-phy, devicetree, linux-arm-kernel,
linux-kernel, Joey Lu
In-Reply-To: <20260604101220.1092822-1-a0987203069@gmail.com>
Add device tree binding documentation for the Nuvoton MA35D1 USB 2.0
OTG PHY driver (nuvoton,ma35d1-usb2-phy-otg).
PHY index 0 (USB0) is an OTG port whose signals are routed by a hardware
mux to either the DWC2 device controller or the EHCI0/OHCI0 host
controllers depending on the USB ID pin state. PHY index 1 (USB1) is a
dedicated host-only port.
Optional properties allow board-specific resistor calibration trim
(nuvoton,rcalcode) and over-current detect polarity configuration
(nuvoton,oc-active-high).
Signed-off-by: Joey Lu <a0987203069@gmail.com>
---
.../phy/nuvoton,ma35d1-usb2-phy-otg.yaml | 79 +++++++++++++++++++
1 file changed, 79 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy-otg.yaml
diff --git a/Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy-otg.yaml b/Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy-otg.yaml
new file mode 100644
index 000000000000..19f074565cc6
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy-otg.yaml
@@ -0,0 +1,79 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/nuvoton,ma35d1-usb2-phy-otg.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton MA35D1 USB 2.0 host PHY
+
+maintainers:
+ - Joey Lu <yclu4@nuvoton.com>
+
+description:
+ USB 2.0 PHY driver for the Nuvoton MA35D1 SoC, used by the EHCI and
+ OHCI host controllers.
+
+ USB0 (PHY index 0) is an OTG port whose physical signals are routed to
+ either the DWC2 device controller or the EHCI0/OHCI0 host controller by
+ a hardware mux that follows the USB ID pin.
+
+ USB1 (PHY index 1) is a dedicated host port with no OTG capability.
+
+properties:
+ compatible:
+ const: nuvoton,ma35d1-usb2-phy-otg
+
+ clocks:
+ maxItems: 1
+
+ nuvoton,sys:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ items:
+ - items:
+ - description: phandle to the system management syscon.
+ - description: PHY instance index.
+ enum:
+ - 0 # USB0, OTG port (shared with DWC2 gadget controller)
+ - 1 # USB1, host-only port
+ description:
+ A phandle to the syscon node covering the SYS register block, with
+ one argument selecting the PHY instance. Index 0 selects the OTG
+ port PHY (USB0) and index 1 selects the host-only PHY (USB1).
+
+ "#phy-cells":
+ const: 0
+
+ nuvoton,rcalcode:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 15
+ description:
+ Resistor calibration trim code written to the RCALCODE field in
+ USBPMISCR. The 4-bit value adjusts the PHY's internal termination
+ resistance. When absent the hardware reset default is used.
+
+ nuvoton,oc-active-high:
+ type: boolean
+ description:
+ 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
+ - clocks
+ - nuvoton,sys
+ - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
+
+ usb_hphy0: usb-host-phy {
+ compatible = "nuvoton,ma35d1-usb2-phy-otg";
+ clocks = <&clk HUSBH0_GATE>;
+ nuvoton,sys = <&sys 0>;
+ #phy-cells = <0>;
+ };
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH 0/2] phy: nuvoton: Add MA35D1 USB2 OTG PHY driver
From: Joey Lu @ 2026-06-04 10:12 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jacky Huang,
Shan-Chun Hung, linux-phy, devicetree, linux-arm-kernel,
linux-kernel, Joey Lu
The Nuvoton MA35D1 SoC has two USB 2.0 ports:
USB0 is an OTG-capable port. Its physical signals are routed by a
hardware mux to either a DWC2 gadget controller or the EHCI0/OHCI0
host controllers, depending on the USB ID pin state. The DWC2 IP is
device-only in hardware, so all host-mode operation on USB0 is
handled by EHCI0/OHCI0.
USB1 is a dedicated host-only port served by EHCI1/OHCI1.
About this driver:
- Runs the PHY Power-On Reset sequence, with a guard that skips
re-initialization if the PHY is already operational.
- Supports optional resistor calibration trim (nuvoton,rcalcode) and
over-current detect polarity configuration (nuvoton,oc-active-high).
- For PHY0 (USB0) only: registers a USB role switch that reads the
hardware ID pin state from PWRONOTP[16] on every query.
Joey Lu (2):
dt-bindings: phy: nuvoton: Add MA35D1 USB2 OTG PHY binding
phy: nuvoton: Add MA35D1 USB2 OTG PHY driver
.../phy/nuvoton,ma35d1-usb2-phy-otg.yaml | 79 ++++++
drivers/phy/nuvoton/Kconfig | 15 +
drivers/phy/nuvoton/Makefile | 1 +
drivers/phy/nuvoton/phy-ma35d1-otg.c | 264 ++++++++++++++++++
4 files changed, 359 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/nuvoton,ma35d1-usb2-phy-otg.yaml
create mode 100644 drivers/phy/nuvoton/phy-ma35d1-otg.c
--
2.43.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-06-04 9:20 UTC (permalink / raw)
To: Bryan O'Donoghue, Bryan O'Donoghue, Vijay Kumar Tumati,
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: <67b6f6ae-bfca-4afd-adfb-6ec1741105d8@linaro.org>
On 6/4/26 12:06, Bryan O'Donoghue wrote:
> On 04/06/2026 09:46, Vladimir Zapolskiy wrote:
>> On 6/4/26 03:30, Bryan O'Donoghue wrote:
>>> On 04/06/2026 01:07, Vladimir Zapolskiy wrote:
>>>> On 6/4/26 00:18, Bryan O'Donoghue wrote:
>>>>> On 03/06/2026 21:51, Vladimir Zapolskiy wrote:
>>>>>>> Actually, one more thing, Why isn't TITAN TOP GDSC here?>>>> +
>>>>>> If CSIPHYs are true subdevices under the umbrella CAMSS device and
>>>>>> well
>>>>>> described as subnodes, then likely none of power domains are needed
>>>>>> to be
>>>>>> repeatedly described in the children device nodes, since this
>>>>>> information
>>>>>> can be obtained from the parent device by the driver.
>>>>>>
>>>>>> Technically 'power-domains' property can be safely removed, I believe.
>>>>>
>>>>> The policy is to describe the power-domain dependency fully since DT
>>>>> describes hardware not software architecture.
>>>>
>>>> It brings no contardiction to the statement I've given above, the needed
>>>> power domans will be properly described in the parent device, and there
>>>> is no
>>>> sense to repeat the properties it again and again in every child
>>>> subdevice.
>>>>
>>>>> Also for a very practical reason a sub-devices can probe/run
>>>>> asynchronously of the parent device being active so in fact we do need
>>>>> to describe the PDs fully.
>>>>
>>>> In opposite to the above this one is precisely a software centric
>>>> argument,
>>>> which should be excluded from the consideration, as well it's not a big
>>>> deal to make a proper async initialization, removing excessive dt
>>>> properties
>>>> is worth it.
>>>>
>>>
>>> Right look forget about that.
>>>
>>> - DT requires you to describe your hardware. You're not entitled to have
>>> some other device vote for a clock or a PD you rely on.
>>>
>>
>> Above are two uncorrelated between each other sentences.
>>
>> A device ("consumer") can ask another device ("provider") to behave in
>> one or another way, this is the only possible and thus natually selected
>> system design, and nothing behind it was asked. There is no justification
>> for the proposed flood of multiply repeated data, it's avoidable.
>
> CAMSS or rather the components of CAMSS modelled in the current node,
> is/are not the provider of the GDSCs or the power-domains, it/they are
> consumers themselves from CAMCC.
Well, this is the argument about software, and software can be changed.
> The producer/consumer model is CAMCC to components within the Camera
> block. Some components depend on say MXA, MXC, some do not. Nothing in
> CAMSS itself is a power-domain provider.
>>> That's exactly the type of downstream short cut we are trying to zap.
>>>
>>> - In our case we also need to vote on PDs individually when the PHY
>>> is active.
>>>
>>> In extremis say we are only running the TPG then we have no reason to
>>> vote for CSIPHY specific rails or operating points in the parent device.
>>
>> So, TPG shall communicate with CAMSS, there is no CSIPHY in the equation.
>
> Right but it would be inappropriate to enable all of the PDs for all of
> the components in the CAMSS block when we can do so more granularly.
Whenever it is actually necessary, it should be possible to split PDs into
generic/parent and subdevice specific groups, it's a part of software
implementation. In some cases there might be no need to define any child
side PDs, likely CSIPHY falls into this category.
> If you drive the CSID with a TPG you shouldn't be voting for MXA or MXC
> since these are PDs related to the CSIPHY only and TPG and CSIPHY do the
> same thing from the CSID perspective - input data.
>
>>
>>> We could make the parent power-domain argument for CAMSS and CCI but we
>>> have TITAN_TOP_GDSC in CCI specifically because we have to model the
>>> hardware - including the PDs for that device.
>>
>> CCI is not described as a child of CAMSS, here the situation is different.
>
> CCI probably _should_ be a child of CAMSS given the design we are going
> for here.
>
> Leaving that aside it doesn't matter if a node appears as a child node
> or a peer node - the DT should describe the hardware setup.
If a node is child or a sibling is a hardware description, different
hardware descriptions bring different dt properties, this is normal.
> I can't imagine a patch that would remove a power-domain from a device
> being accepted simply because the node being moved is expressed as a
> child of another node.
I strongly believe every dt binding describing actual hardware will find
its way to be implemented in software in a proper way, it should be doable.
>>> If tomorrow we put CCI as a sub-device of top-level CAMSS, that won't
>>> negate the need to include that GDSC.
>>
>> Of course in this case a phandle to Titan GDSC will be marked as obsolete
>> or unused for CCI, no problem here.
>>
--
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 v8 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Bryan O'Donoghue @ 2026-06-04 9:06 UTC (permalink / raw)
To: Vladimir Zapolskiy, Bryan O'Donoghue, Vijay Kumar Tumati,
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: <1b107aca-a857-4e58-a763-39c82af67747@linaro.org>
On 04/06/2026 09:46, Vladimir Zapolskiy wrote:
> On 6/4/26 03:30, Bryan O'Donoghue wrote:
>> On 04/06/2026 01:07, Vladimir Zapolskiy wrote:
>>> On 6/4/26 00:18, Bryan O'Donoghue wrote:
>>>> On 03/06/2026 21:51, Vladimir Zapolskiy wrote:
>>>>>> Actually, one more thing, Why isn't TITAN TOP GDSC here?>>>> +
>>>>> If CSIPHYs are true subdevices under the umbrella CAMSS device and
>>>>> well
>>>>> described as subnodes, then likely none of power domains are needed
>>>>> to be
>>>>> repeatedly described in the children device nodes, since this
>>>>> information
>>>>> can be obtained from the parent device by the driver.
>>>>>
>>>>> Technically 'power-domains' property can be safely removed, I believe.
>>>>
>>>> The policy is to describe the power-domain dependency fully since DT
>>>> describes hardware not software architecture.
>>>
>>> It brings no contardiction to the statement I've given above, the needed
>>> power domans will be properly described in the parent device, and there
>>> is no
>>> sense to repeat the properties it again and again in every child
>>> subdevice.
>>>
>>>> Also for a very practical reason a sub-devices can probe/run
>>>> asynchronously of the parent device being active so in fact we do need
>>>> to describe the PDs fully.
>>>
>>> In opposite to the above this one is precisely a software centric
>>> argument,
>>> which should be excluded from the consideration, as well it's not a big
>>> deal to make a proper async initialization, removing excessive dt
>>> properties
>>> is worth it.
>>>
>>
>> Right look forget about that.
>>
>> - DT requires you to describe your hardware. You're not entitled to have
>> some other device vote for a clock or a PD you rely on.
>>
>
> Above are two uncorrelated between each other sentences.
>
> A device ("consumer") can ask another device ("provider") to behave in
> one or another way, this is the only possible and thus natually selected
> system design, and nothing behind it was asked. There is no justification
> for the proposed flood of multiply repeated data, it's avoidable.
CAMSS or rather the components of CAMSS modelled in the current node,
is/are not the provider of the GDSCs or the power-domains, it/they are
consumers themselves from CAMCC.
The producer/consumer model is CAMCC to components within the Camera
block. Some components depend on say MXA, MXC, some do not. Nothing in
CAMSS itself is a power-domain provider.
>> That's exactly the type of downstream short cut we are trying to zap.
>>
>> - In our case we also need to vote on PDs individually when the PHY
>> is active.
>>
>> In extremis say we are only running the TPG then we have no reason to
>> vote for CSIPHY specific rails or operating points in the parent device.
>
> So, TPG shall communicate with CAMSS, there is no CSIPHY in the equation.
Right but it would be inappropriate to enable all of the PDs for all of
the components in the CAMSS block when we can do so more granularly.
If you drive the CSID with a TPG you shouldn't be voting for MXA or MXC
since these are PDs related to the CSIPHY only and TPG and CSIPHY do the
same thing from the CSID perspective - input data.
>
>> We could make the parent power-domain argument for CAMSS and CCI but we
>> have TITAN_TOP_GDSC in CCI specifically because we have to model the
>> hardware - including the PDs for that device.
>
> CCI is not described as a child of CAMSS, here the situation is different.
CCI probably _should_ be a child of CAMSS given the design we are going
for here.
Leaving that aside it doesn't matter if a node appears as a child node
or a peer node - the DT should describe the hardware setup.
I can't imagine a patch that would remove a power-domain from a device
being accepted simply because the node being moved is expressed as a
child of another node.
>> If tomorrow we put CCI as a sub-device of top-level CAMSS, that won't
>> negate the need to include that GDSC.
>
> Of course in this case a phandle to Titan GDSC will be marked as obsolete
> or unused for CCI, no problem here.
>
--
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-06-04 8:46 UTC (permalink / raw)
To: Bryan O'Donoghue, Bryan O'Donoghue, Vijay Kumar Tumati,
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: <83c12dc5-fcb4-4089-9917-9f0fcc4f940d@linaro.org>
On 6/4/26 03:30, Bryan O'Donoghue wrote:
> On 04/06/2026 01:07, Vladimir Zapolskiy wrote:
>> On 6/4/26 00:18, Bryan O'Donoghue wrote:
>>> On 03/06/2026 21:51, Vladimir Zapolskiy wrote:
>>>>> Actually, one more thing, Why isn't TITAN TOP GDSC here?>>>> +
>>>> If CSIPHYs are true subdevices under the umbrella CAMSS device and well
>>>> described as subnodes, then likely none of power domains are needed
>>>> to be
>>>> repeatedly described in the children device nodes, since this
>>>> information
>>>> can be obtained from the parent device by the driver.
>>>>
>>>> Technically 'power-domains' property can be safely removed, I believe.
>>>
>>> The policy is to describe the power-domain dependency fully since DT
>>> describes hardware not software architecture.
>>
>> It brings no contardiction to the statement I've given above, the needed
>> power domans will be properly described in the parent device, and there
>> is no
>> sense to repeat the properties it again and again in every child subdevice.
>>
>>> Also for a very practical reason a sub-devices can probe/run
>>> asynchronously of the parent device being active so in fact we do need
>>> to describe the PDs fully.
>>
>> In opposite to the above this one is precisely a software centric argument,
>> which should be excluded from the consideration, as well it's not a big
>> deal to make a proper async initialization, removing excessive dt
>> properties
>> is worth it.
>>
>
> Right look forget about that.
>
> - DT requires you to describe your hardware. You're not entitled to have
> some other device vote for a clock or a PD you rely on.
>
Above are two uncorrelated between each other sentences.
A device ("consumer") can ask another device ("provider") to behave in
one or another way, this is the only possible and thus natually selected
system design, and nothing behind it was asked. There is no justification
for the proposed flood of multiply repeated data, it's avoidable.
> That's exactly the type of downstream short cut we are trying to zap.
>
> - In our case we also need to vote on PDs individually when the PHY
> is active.
>
> In extremis say we are only running the TPG then we have no reason to
> vote for CSIPHY specific rails or operating points in the parent device.
So, TPG shall communicate with CAMSS, there is no CSIPHY in the equation.
> We could make the parent power-domain argument for CAMSS and CCI but we
> have TITAN_TOP_GDSC in CCI specifically because we have to model the
> hardware - including the PDs for that device.
CCI is not described as a child of CAMSS, here the situation is different.
> If tomorrow we put CCI as a sub-device of top-level CAMSS, that won't
> negate the need to include that GDSC.
Of course in this case a phandle to Titan GDSC will be marked as obsolete
or unused for CCI, no problem here.
--
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 v3 4/5] phy: fsl-imx8mq-usb: add control register regmap
From: Xu Yang @ 2026-06-04 2:34 UTC (permalink / raw)
To: Bough Chen
Cc: Vinod Koul, Neil Armstrong, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Jun Li, linux-phy, imx,
linux-arm-kernel, linux-kernel, Xu Yang
In-Reply-To: <20260603072107.xhrzxrgbrb7q6fhe@nxp.com>
On Wed, Jun 03, 2026 at 03:21:07PM +0800, Bough Chen wrote:
> On Wed, Jun 03, 2026 at 01:37:17PM +0800, Xu Yang wrote:
> > From: Xu Yang <xu.yang_2@nxp.com>
> >
> > The CR port is a simple 16-bit data/address parallel port that is
> > provided for on-chip access to the control registers inside the
> > USB 3.0 femtoPHY. Add control register regmap and export these
> > registers by debugfs to help PHY's diagnostic.
> >
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >
> > ---
> > Changes in v3:
> > - drop Frank's tag because it includes other changes
> > - new patch
> > ---
> > drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 27 ++++++++++++++++++++++++++-
> > 1 file changed, 26 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > index b0092c34416e..cda88ea1f12d 100644
> > --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > @@ -1,5 +1,5 @@
> > // SPDX-License-Identifier: GPL-2.0+
> > -/* Copyright (c) 2017 NXP. */
> > +/* Copyright 2017-2026 NXP. */
>
> Should be : Copyright 2017, 2026 NXP.
The Copyright hasn't been updated in a long time. So I want to extend the
period here.
>
> >
> > #include <linux/bitfield.h>
> > #include <linux/clk.h>
> > @@ -11,6 +11,7 @@
> > #include <linux/platform_device.h>
> > #include <linux/pm_runtime.h>
> > #include <linux/regulator/consumer.h>
> > +#include <linux/regmap.h>
> > #include <linux/usb/typec_mux.h>
> >
> > #define PHY_CTRL0 0x0
> > @@ -56,6 +57,8 @@
> > #define PHY_CTRL6_ALT_CLK_EN BIT(1)
> > #define PHY_CTRL6_ALT_CLK_SEL BIT(0)
> >
> > +#define PHY_CRCTL 0x30
> > +
> > #define PHY_TUNE_DEFAULT 0xffffffff
> >
> > #define TCA_CLK_RST 0x00
> > @@ -119,6 +122,7 @@ struct imx8mq_usb_phy {
> > void __iomem *base;
> > struct regulator *vbus;
> > struct tca_blk *tca;
> > + struct regmap *cr_regmap;
> > u32 pcs_tx_swing_full;
> > u32 pcs_tx_deemph_3p5db;
> > u32 tx_vref_tune;
> > @@ -665,6 +669,14 @@ static const struct of_device_id imx8mq_usb_phy_of_match[] = {
> > };
> > MODULE_DEVICE_TABLE(of, imx8mq_usb_phy_of_match);
> >
> > +static const struct regmap_config imx_cr_regmap_config = {
> > + .name = "cr",
> > + .reg_bits = 32,
> > + .val_bits = 32,
> > + .reg_stride = 4,
> > + .max_register = 0x7,
> > +};
> > +
>
> Your commit message says: The CR port is a simple 16-bit data/address parallel port
> But here you use 32 bit mmio, which is a bit confuse to reader.
> Maybe you can add the following comment before the regmap config:
>
> /*
> * CR Port MMIO Register Map
> *
> * The CR (Control Register) port uses a 16-bit data/address protocol,
> * but is accessed through 32-bit MMIO registers:
> *
> * Offset 0x0: CR Control Register
> * [31:16] - Control/Status flags
> * [15:0] - 16-bit CR Address
> *
> * Offset 0x4: CR Data Register
> * [31:16] - Reserved/Status
> * [15:0] - 16-bit CR Data
> */
>
> Or change your commit log like this:
>
> The CR port is a 16-bit data/address parallel port protocol that is
> accessed through 32-bit MMIO registers for on-chip access to the
> control registers inside the USB 3.0 femtoPHY. Add control register
> regmap and export these registers by debugfs to help PHY's diagnostic.
OK. Thanks for the suggestion.
Thanks,
Xu Yang
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 3/5] phy: fsl-imx8mq-usb: add runtime PM support
From: Xu Yang @ 2026-06-04 2:31 UTC (permalink / raw)
To: Frank Li
Cc: Vinod Koul, Neil Armstrong, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Jun Li, linux-phy, imx, linux-arm-kernel,
linux-kernel, Xu Yang
In-Reply-To: <aiB0Ivrz-Y-Ilv_W@lizhi-Precision-Tower-5810>
On Wed, Jun 03, 2026 at 02:36:18PM -0400, Frank Li wrote:
> On Wed, Jun 03, 2026 at 01:37:16PM +0800, Xu Yang wrote:
> > From: Xu Yang <xu.yang_2@nxp.com>
> >
> > Add runtime PM to ensure the PHY is properly powered and clocked during
> > register access, preventing potential system hangs.
> >
> > It guards register access in the following scenarios:
> > - PHY operations: init() and power_on/off() callbacks are guarded by
> > phy core
> > - Type-C orientation switching when PHY/Controller are suspended which
> > needs explicitly care
> > - Future PHY control port register regmap debugfs access
> >
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >
> > ---
> > Changes in v3:
> > - new patch
> > ---
> > drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 60 ++++++++++++++++++++----------
> > 1 file changed, 41 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > index 591ddf346061..b0092c34416e 100644
> > --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > @@ -9,6 +9,7 @@
> > #include <linux/of.h>
> > #include <linux/phy/phy.h>
> > #include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > #include <linux/regulator/consumer.h>
> > #include <linux/usb/typec_mux.h>
> >
> > @@ -136,17 +137,13 @@ static int tca_blk_typec_switch_set(struct typec_switch_dev *sw,
> > {
> > struct imx8mq_usb_phy *imx_phy = typec_switch_get_drvdata(sw);
> > struct tca_blk *tca = imx_phy->tca;
> > - int ret;
> >
> > if (tca->orientation == orientation)
> > return 0;
> >
> > - ret = clk_prepare_enable(imx_phy->clk);
> > - if (ret)
> > - return ret;
> > + guard(pm_runtime_active)(&imx_phy->phy->dev);
>
> use PM_RUNTIME_ACQUIRE macro
OK
>
> >
> > tca_blk_orientation_set(tca, orientation);
> > - clk_disable_unprepare(imx_phy->clk);
> >
> > return 0;
> > }
> > @@ -620,16 +617,6 @@ static int imx8mq_phy_power_on(struct phy *phy)
> > if (ret)
> > return ret;
> >
> > - ret = clk_prepare_enable(imx_phy->clk);
> > - if (ret)
> > - return ret;
> > -
> > - ret = clk_prepare_enable(imx_phy->alt_clk);
> > - if (ret) {
> > - clk_disable_unprepare(imx_phy->clk);
> > - return ret;
> > - }
> > -
> > /* Disable rx term override */
> > value = readl(imx_phy->base + PHY_CTRL6);
> > value &= ~PHY_CTRL6_RXTERM_OVERRIDE_SEL;
> > @@ -648,8 +635,6 @@ static int imx8mq_phy_power_off(struct phy *phy)
> > value |= PHY_CTRL6_RXTERM_OVERRIDE_SEL;
> > writel(value, imx_phy->base + PHY_CTRL6);
> >
> > - clk_disable_unprepare(imx_phy->alt_clk);
> > - clk_disable_unprepare(imx_phy->clk);
> > regulator_disable(imx_phy->vbus);
> >
> > return 0;
> > @@ -686,6 +671,7 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
> > struct device *dev = &pdev->dev;
> > struct imx8mq_usb_phy *imx_phy;
> > const struct phy_ops *phy_ops;
> > + int ret;
> >
> > imx_phy = devm_kzalloc(dev, sizeof(*imx_phy), GFP_KERNEL);
> > if (!imx_phy)
> > @@ -693,13 +679,13 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
> >
> > platform_set_drvdata(pdev, imx_phy);
> >
> > - imx_phy->clk = devm_clk_get(dev, "phy");
> > + imx_phy->clk = devm_clk_get_enabled(dev, "phy");
> > if (IS_ERR(imx_phy->clk)) {
> > dev_err(dev, "failed to get imx8mq usb phy clock\n");
> > return PTR_ERR(imx_phy->clk);
> > }
> >
> > - imx_phy->alt_clk = devm_clk_get_optional(dev, "alt");
> > + imx_phy->alt_clk = devm_clk_get_optional_enabled(dev, "alt");
> > if (IS_ERR(imx_phy->alt_clk))
> > return dev_err_probe(dev, PTR_ERR(imx_phy->alt_clk),
> > "Failed to get alt clk\n");
> > @@ -708,6 +694,10 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
> > if (IS_ERR(imx_phy->base))
> > return PTR_ERR(imx_phy->base);
> >
> > + ret = devm_pm_runtime_set_active_enabled(dev);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to enable runtime PM\n");
> > +
>
> you enable runtime pm here, when runtimes suspend
devm_pm_runtime_set_active_enabled() will firstly change status as runtime active,
then enable runtime PM. So it's already not in suspend state.
Thanks,
Xu Yang
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 1/5] phy: fsl-imx8mq-usb: fix typec switch leak on probe error path
From: Xu Yang @ 2026-06-04 2:29 UTC (permalink / raw)
To: Frank Li
Cc: Vinod Koul, Neil Armstrong, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Jun Li, linux-phy, imx, linux-arm-kernel,
linux-kernel, Felix Gu, stable, Xu Yang
In-Reply-To: <aiBxqjm5lsPDXEtW@lizhi-Precision-Tower-5810>
On Wed, Jun 03, 2026 at 02:25:46PM -0400, Frank Li wrote:
> On Wed, Jun 03, 2026 at 01:37:14PM +0800, Xu Yang wrote:
> > From: Felix Gu <ustc.gu@gmail.com>
> >
> > If probe fails after imx95_usb_phy_get_tca() succeeds, the typec
> > switch leaks because the only cleanup path was in .remove, which
> > never runs on probe failure.
> >
> > Use devm_add_action_or_reset() so the switch is cleaned up on both
> > probe failure and driver removal. The .remove callback and
> > imx95_usb_phy_put_tca() are no longer needed.
> >
> > Fixes: b58f0f86fd61 ("phy: fsl-imx8mq-usb: add tca function driver for imx95")
> > Cc: stable@vger.kernel.org
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > Reviewed-by: Xu Yang <xu.yang_2@nxp.com>
> > Signed-off-by: Felix Gu <ustc.gu@gmail.com>
>
> Xu yang, if you send out patch, need your s-o-b tag
OK. Will add it in v2.
Thanks,
Xu Yang
--
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