* Re: [PATCH v4 2/2] phy: qcom-qmp-ufs: Add UFS PHY support on Hawi
From: Palash Kambar @ 2026-07-10 11:58 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: vkoul, neil.armstrong, robh, krzk+dt, conor+dt, 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: <tyvt6by2k7wxzds5n67fxpwiw5rwmtwjyluyyntjba7fjo3ri4@no5ay6hxntod>
On 6/17/2026 11:30 AM, Manivannan Sadhasivam wrote:
> On Mon, Jun 15, 2026 at 02:42:42PM +0530, palash.kambar@oss.qualcomm.com wrote:
>> From: Palash Kambar <palash.kambar@oss.qualcomm.com>
>>
>> Add the init sequence tables and config for the UFS QMP phy found in
>> the Hawi SoC.
>>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>> Signed-off-by: Palash Kambar <palash.kambar@oss.qualcomm.com>
>
> Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
>
> - Mani
>
>> ---
>> .../phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h | 24 +++
>> .../qualcomm/phy-qcom-qmp-qserdes-com-v8.h | 13 +-
>> .../phy-qcom-qmp-qserdes-txrx-ufs-v8.h | 37 +++++
>> drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 139 ++++++++++++++++++
>> 4 files changed, 212 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h
>> create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v8.h
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h
>> new file mode 100644
>> index 000000000000..e80d3dd6a190
>> --- /dev/null
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v7.h
>> @@ -0,0 +1,24 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright (c) 2026, The Linux Foundation. All rights reserved.
>> + */
>> +
>> +#ifndef QCOM_PHY_QMP_PCS_UFS_V7_H_
>> +#define QCOM_PHY_QMP_PCS_UFS_V7_H_
>> +
>> +/* Only for QMP V7 PHY - UFS PCS registers */
>> +#define QPHY_V7_PCS_UFS_PHY_START 0x000
>> +#define QPHY_V7_PCS_UFS_POWER_DOWN_CONTROL 0x004
>> +#define QPHY_V7_PCS_UFS_SW_RESET 0x008
>> +#define QPHY_V7_PCS_UFS_PCS_CTRL1 0x01C
>> +#define QPHY_V7_PCS_UFS_PLL_CNTL 0x028
>> +#define QPHY_V7_PCS_UFS_TX_LARGE_AMP_DRV_LVL 0x02C
>> +#define QPHY_V7_PCS_UFS_TX_HSGEAR_CAPABILITY 0x060
>> +#define QPHY_V7_PCS_UFS_RX_HSGEAR_CAPABILITY 0x094
>> +#define QPHY_V7_PCS_UFS_LINECFG_DISABLE 0x140
>> +#define QPHY_V7_PCS_UFS_RX_SIGDET_CTRL2 0x150
>> +#define QPHY_V7_PCS_UFS_READY_STATUS 0x16c
>> +#define QPHY_V7_PCS_UFS_TX_MID_TERM_CTRL1 0x1b8
>> +#define QPHY_V7_PCS_UFS_MULTI_LANE_CTRL1 0x1c0
>> +
>> +#endif
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-com-v8.h b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-com-v8.h
>> index d8ac4c4a2c31..d416113bcb3c 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-com-v8.h
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-com-v8.h
>> @@ -1,6 +1,6 @@
>> /* SPDX-License-Identifier: GPL-2.0 */
>> /*
>> - * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.
>> + * Copyright (c) 2026, The Linux Foundation. All rights reserved.
>> */
>>
>> #ifndef QCOM_PHY_QMP_QSERDES_COM_V8_H_
>> @@ -71,5 +71,16 @@
>> #define QSERDES_V8_COM_ADDITIONAL_MISC 0x1b4
>> #define QSERDES_V8_COM_CMN_STATUS 0x2c8
>> #define QSERDES_V8_COM_C_READY_STATUS 0x2f0
>> +#define QSERDES_V8_COM_PLL_IVCO_MODE1 0xf8
>> +#define QSERDES_V8_COM_CMN_IETRIM 0xfc
>> +#define QSERDES_V8_COM_CMN_IPTRIM 0x100
>> +#define QSERDES_V8_COM_VCO_TUNE_CTRL 0x13c
>> +#define QSERDES_V8_COM_ADAPTIVE_ANALOG_CONFIG 0x268
>> +#define QSERDES_V8_COM_CP_CTRL_ADAPTIVE_MODE0 0x26c
>> +#define QSERDES_V8_COM_PLL_RCCTRL_ADAPTIVE_MODE0 0x270
>> +#define QSERDES_V8_COM_PLL_CCTRL_ADAPTIVE_MODE0 0x274
>> +#define QSERDES_V8_COM_CP_CTRL_ADAPTIVE_MODE1 0x278
>> +#define QSERDES_V8_COM_PLL_RCCTRL_ADAPTIVE_MODE1 0x27c
>> +#define QSERDES_V8_COM_PLL_CCTRL_ADAPTIVE_MODE1 0x280
>>
>> #endif
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v8.h b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v8.h
>> new file mode 100644
>> index 000000000000..5f923c3e64ec
>> --- /dev/null
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v8.h
>> @@ -0,0 +1,37 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright (c) 2026, The Linux Foundation. All rights reserved.
>> + */
>> +
>> +#ifndef QCOM_PHY_QMP_QSERDES_TXRX_UFS_V8_H_
>> +#define QCOM_PHY_QMP_QSERDES_TXRX_UFS_V8_H_
>> +
>> +#define QSERDES_UFS_V8_TX_RES_CODE_LANE_OFFSET_TX (0x34)
>> +#define QSERDES_UFS_V8_TX_RES_CODE_LANE_OFFSET_RX (0x38)
>> +#define QSERDES_UFS_V8_TX_LANE_MODE_1 (0x80)
>> +#define QSERDES_UFS_V8_RX_UCDR_FO_GAIN_RATE2 (0x1BC)
>> +#define QSERDES_UFS_V8_RX_UCDR_FO_GAIN_RATE4 (0x1C4)
>> +#define QSERDES_UFS_V8_RX_UCDR_SO_GAIN_RATE4 (0x1DC)
>> +#define QSERDES_UFS_V8_RX_EQ_OFFSET_ADAPTOR_CNTRL1 (0x2C8)
>> +#define QSERDES_UFS_V8_RX_UCDR_PI_CONTROLS (0x1E4)
>> +#define QSERDES_UFS_V8_RX_OFFSET_ADAPTOR_CNTRL3 (0x2D0)
>> +#define QSERDES_UFS_V8_RX_UCDR_FASTLOCK_COUNT_HIGH_RATE4 (0x120)
>> +#define QSERDES_UFS_V8_RX_UCDR_FASTLOCK_FO_GAIN_RATE4 (0xD4)
>> +#define QSERDES_UFS_V8_RX_UCDR_FASTLOCK_SO_GAIN_RATE4 (0xEC)
>> +#define QSERDES_UFS_V8_RX_VGA_CAL_MAN_VAL (0x288)
>> +#define QSERDES_UFS_V8_RX_EQU_ADAPTOR_CNTRL4 (0x2B0)
>> +#define QSERDES_UFS_V8_RX_MODE_RATE_0_1_B4 (0x324)
>> +#define QSERDES_UFS_V8_RX_MODE_RATE4_SA_B7 (0x3B4)
>> +#define QSERDES_UFS_V8_RX_MODE_RATE4_SA_B9 (0x3BC)
>> +#define QSERDES_UFS_V8_RX_MODE_RATE4_SB_B7 (0x3E0)
>> +#define QSERDES_UFS_V8_RX_MODE_RATE4_SB_B9 (0x3E8)
>> +#define QSERDES_UFS_V8_RX_MODE_RATE5_SA_B7 (0x40C)
>> +#define QSERDES_UFS_V8_RX_MODE_RATE5_SA_B9 (0x414)
>> +#define QSERDES_UFS_V8_RX_MODE_RATE5_SB_B7 (0x438)
>> +#define QSERDES_UFS_V8_RX_MODE_RATE5_SB_B9 (0x440)
>> +#define QSERDES_UFS_V8_RX_UCDR_SO_SATURATION (0xF4)
>> +#define QSERDES_UFS_V8_RX_TERM_BW_CTRL0 (0x1AC)
>> +#define QSERDES_UFS_V8_RX_DLL0_FTUNE_CTRL (0x498)
>> +#define QSERDES_UFS_V8_RX_SIGDET_CAL_TRIM (0x4d0)
>> +
>> +#endif
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
>> index 0f4ad24aa405..d4aca22c181e 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
>> @@ -29,9 +29,11 @@
>> #include "phy-qcom-qmp-pcs-ufs-v4.h"
>> #include "phy-qcom-qmp-pcs-ufs-v5.h"
>> #include "phy-qcom-qmp-pcs-ufs-v6.h"
>> +#include "phy-qcom-qmp-pcs-ufs-v7.h"
>>
>> #include "phy-qcom-qmp-qserdes-txrx-ufs-v6.h"
>> #include "phy-qcom-qmp-qserdes-txrx-ufs-v7.h"
>> +#include "phy-qcom-qmp-qserdes-txrx-ufs-v8.h"
>>
>> /* QPHY_PCS_READY_STATUS bit */
>> #define PCS_READY BIT(0)
>> @@ -84,6 +86,13 @@ static const unsigned int ufsphy_v6_regs_layout[QPHY_LAYOUT_SIZE] = {
>> [QPHY_PCS_POWER_DOWN_CONTROL] = QPHY_V6_PCS_UFS_POWER_DOWN_CONTROL,
>> };
>>
>> +static const unsigned int ufsphy_v7_regs_layout[QPHY_LAYOUT_SIZE] = {
>> + [QPHY_START_CTRL] = QPHY_V7_PCS_UFS_PHY_START,
>> + [QPHY_PCS_READY_STATUS] = QPHY_V7_PCS_UFS_READY_STATUS,
>> + [QPHY_SW_RESET] = QPHY_V7_PCS_UFS_SW_RESET,
>> + [QPHY_PCS_POWER_DOWN_CONTROL] = QPHY_V7_PCS_UFS_POWER_DOWN_CONTROL,
>> +};
>> +
>> static const struct qmp_phy_init_tbl milos_ufsphy_serdes[] = {
>> QMP_PHY_INIT_CFG(QSERDES_V6_COM_SYSCLK_EN_SEL, 0xd9),
>> QMP_PHY_INIT_CFG(QSERDES_V6_COM_CMN_CONFIG_1, 0x16),
>> @@ -1307,6 +1316,11 @@ static const struct regulator_bulk_data sm8750_ufsphy_vreg_l[] = {
>> { .supply = "vdda-pll", .init_load_uA = 18300 },
>> };
>>
>> +static const struct regulator_bulk_data hawi_ufsphy_vreg_l[] = {
>> + { .supply = "vdda-phy", .init_load_uA = 324000 },
>> + { .supply = "vdda-pll", .init_load_uA = 27000 },
>> +};
>> +
>> static const struct qmp_ufs_offsets qmp_ufs_offsets = {
>> .serdes = 0,
>> .pcs = 0xc00,
>> @@ -1325,6 +1339,15 @@ static const struct qmp_ufs_offsets qmp_ufs_offsets_v6 = {
>> .rx2 = 0x1a00,
>> };
>>
>> +static const struct qmp_ufs_offsets qmp_ufs_offsets_v7 = {
>> + .serdes = 0,
>> + .pcs = 0x0400,
>> + .tx = 0x2000,
>> + .rx = 0x2000,
>> + .tx2 = 0x3000,
>> + .rx2 = 0x3000,
>> +};
>> +
>> static const struct qmp_phy_cfg milos_ufsphy_cfg = {
>> .lanes = 2,
>>
>> @@ -1845,6 +1868,119 @@ static const struct qmp_phy_cfg sm8750_ufsphy_cfg = {
>>
>> };
>>
>> +static const struct qmp_phy_init_tbl hawi_ufsphy_serdes[] = {
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_SYSCLK_EN_SEL, 0xd9),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CMN_CONFIG_1, 0x16),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_HSCLK_SEL_1, 0x11),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_HSCLK_HS_SWITCH_SEL_1, 0x00),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP_EN, 0x01),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP_CFG, 0x60),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_IVCO, 0x1f),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_IVCO_MODE1, 0x1f),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CMN_IETRIM, 0x07),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CMN_IPTRIM, 0x20),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_VCO_TUNE_MAP, 0x04),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_VCO_TUNE_CTRL, 0x40),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_ADAPTIVE_ANALOG_CONFIG, 0x06),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_DEC_START_MODE0, 0x41),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CP_CTRL_MODE0, 0x06),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_RCTRL_MODE0, 0x18),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_CCTRL_MODE0, 0x14),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CP_CTRL_ADAPTIVE_MODE0, 0x06),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_RCCTRL_ADAPTIVE_MODE0, 0x18),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_CCTRL_ADAPTIVE_MODE0, 0x14),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP1_MODE0, 0x7f),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP2_MODE0, 0x06),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_BIN_VCOCAL_CMP_CODE1_MODE0, 0x92),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_BIN_VCOCAL_CMP_CODE2_MODE0, 0x1e),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_DEC_START_MODE1, 0x4c),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CP_CTRL_MODE1, 0x06),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_RCTRL_MODE1, 0x18),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_CCTRL_MODE1, 0x14),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_CP_CTRL_ADAPTIVE_MODE1, 0x06),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_RCCTRL_ADAPTIVE_MODE1, 0x18),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_PLL_CCTRL_ADAPTIVE_MODE1, 0x14),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP1_MODE1, 0x99),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_LOCK_CMP2_MODE1, 0x07),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_BIN_VCOCAL_CMP_CODE1_MODE1, 0xbe),
>> + QMP_PHY_INIT_CFG(QSERDES_V8_COM_BIN_VCOCAL_CMP_CODE2_MODE1, 0x23),
>> +};
>> +
>> +static const struct qmp_phy_init_tbl hawi_ufsphy_tx[] = {
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_TX_LANE_MODE_1, 0x0c),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_TX_RES_CODE_LANE_OFFSET_TX, 0x07),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_TX_RES_CODE_LANE_OFFSET_RX, 0x17),
>> +};
>> +
>> +static const struct qmp_phy_init_tbl hawi_ufsphy_rx[] = {
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FO_GAIN_RATE2, 0x0c),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FO_GAIN_RATE4, 0x0c),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_SO_GAIN_RATE4, 0x04),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_EQ_OFFSET_ADAPTOR_CNTRL1, 0x14),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_PI_CONTROLS, 0x07),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_OFFSET_ADAPTOR_CNTRL3, 0x0e),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FASTLOCK_COUNT_HIGH_RATE4, 0x02),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FASTLOCK_FO_GAIN_RATE4, 0x1c),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_FASTLOCK_SO_GAIN_RATE4, 0x06),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_VGA_CAL_MAN_VAL, 0x8e),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_EQU_ADAPTOR_CNTRL4, 0x0f),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE_0_1_B4, 0xb8),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE4_SA_B7, 0x66),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE4_SA_B9, 0x1f),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE4_SB_B7, 0x66),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE4_SB_B9, 0x1f),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE5_SA_B7, 0x66),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE5_SA_B9, 0x1f),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE5_SB_B7, 0x66),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_MODE_RATE5_SB_B9, 0x1f),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_UCDR_SO_SATURATION, 0x1f),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_TERM_BW_CTRL0, 0xfa),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_DLL0_FTUNE_CTRL, 0x30),
>> + QMP_PHY_INIT_CFG(QSERDES_UFS_V8_RX_SIGDET_CAL_TRIM, 0x77),
>> +};
>> +
>> +static const struct qmp_phy_init_tbl hawi_ufsphy_pcs[] = {
>> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_TX_MID_TERM_CTRL1, 0x43),
>> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_PCS_CTRL1, 0x42),
>> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_TX_LARGE_AMP_DRV_LVL, 0x0f),
>> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_RX_SIGDET_CTRL2, 0x68),
>> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_MULTI_LANE_CTRL1, 0x02),
>> +};
>> +
>> +static const struct qmp_phy_init_tbl hawi_ufsphy_g5_pcs[] = {
>> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_PLL_CNTL, 0x3b),
>> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_TX_HSGEAR_CAPABILITY, 0x05),
>> + QMP_PHY_INIT_CFG(QPHY_V7_PCS_UFS_RX_HSGEAR_CAPABILITY, 0x05),
>> +};
>> +
>> +static const struct qmp_phy_cfg hawi_ufsphy_cfg = {
>> + .lanes = 2,
>> +
>> + .offsets = &qmp_ufs_offsets_v7,
>> + .max_supported_gear = UFS_HS_G5,
>> +
>> + .tbls = {
>> + .serdes = hawi_ufsphy_serdes,
>> + .serdes_num = ARRAY_SIZE(hawi_ufsphy_serdes),
>> + .tx = hawi_ufsphy_tx,
>> + .tx_num = ARRAY_SIZE(hawi_ufsphy_tx),
>> + .rx = hawi_ufsphy_rx,
>> + .rx_num = ARRAY_SIZE(hawi_ufsphy_rx),
>> + .pcs = hawi_ufsphy_pcs,
>> + .pcs_num = ARRAY_SIZE(hawi_ufsphy_pcs),
>> + },
>> +
>> + .tbls_hs_overlay[0] = {
>> + .pcs = hawi_ufsphy_g5_pcs,
>> + .pcs_num = ARRAY_SIZE(hawi_ufsphy_g5_pcs),
>> + .max_gear = UFS_HS_G5,
>> + },
>> +
>> + .vreg_list = hawi_ufsphy_vreg_l,
>> + .num_vregs = ARRAY_SIZE(hawi_ufsphy_vreg_l),
>> + .regs = ufsphy_v7_regs_layout,
>> +};
>> +
>> static void qmp_ufs_serdes_init(struct qmp_ufs *qmp, const struct qmp_phy_cfg_tbls *tbls)
>> {
>> void __iomem *serdes = qmp->serdes;
>> @@ -2259,6 +2395,9 @@ static int qmp_ufs_probe(struct platform_device *pdev)
>>
>> static const struct of_device_id qmp_ufs_of_match_table[] = {
>> {
>> + .compatible = "qcom,hawi-qmp-ufs-phy",
>> + .data = &hawi_ufsphy_cfg,
>> + }, {
>> .compatible = "qcom,milos-qmp-ufs-phy",
>> .data = &milos_ufsphy_cfg,
>> }, {
>> --
>> 2.34.1
>>
>
Hi Vinod,
Gentle reminder to integrate this patch.
Thanks.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 3/4] arm64: dts: qcom: Add Shikra CQM SoM platform
From: Rakesh Kota @ 2026-07-10 10:35 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Kamal Wadhwa, linux-arm-msm, sashiko-reviews, Komal Bajaj, robh,
linux-phy, neil.armstrong, vkoul, olteanv, krzk+dt, conor+dt,
devicetree, Jishnu Prakash
In-Reply-To: <sfilvfwibse2vpi74vownkx32kz2dhkkvriphtti5jn2p32ffx@2q4p7nahzoxz>
On Fri, Jul 03, 2026 at 06:28:27PM +0300, Dmitry Baryshkov wrote:
> On Tue, Jun 30, 2026 at 06:12:20PM +0530, Rakesh Kota wrote:
> > On Sun, Jun 28, 2026 at 03:33:23PM +0300, Dmitry Baryshkov wrote:
> > > On Thu, Jun 25, 2026 at 09:11:19PM +0530, Kamal Wadhwa wrote:
> > > > On Wed, Jun 17, 2026 at 03:48:14PM +0300, Dmitry Baryshkov wrote:
> > > > > On Mon, 18 May 2026 at 14:49, Kamal Wadhwa
> > > > > <kamal.wadhwa@oss.qualcomm.com> wrote:
> > > > > >
> > > > > > On Sun, May 17, 2026 at 08:18:15PM +0300, Dmitry Baryshkov wrote:
> > > > > > > On Thu, May 14, 2026 at 04:09:18PM +0530, Kamal Wadhwa wrote:
> > > > > > > > On Wed, May 13, 2026 at 06:14:20PM +0300, Dmitry Baryshkov wrote:
> > > > > > > > > On 13/05/2026 17:29, Rakesh Kota wrote:
> > > > > > > > > > On Wed, May 13, 2026 at 03:01:47PM +0300, Dmitry Baryshkov wrote:
> > > > > > > > > > > On Wed, May 13, 2026 at 04:28:35AM +0000, sashiko-bot@kernel.org wrote:
> > > > > > > > > > > > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> > > > > > > > > > > > - [High] The PMIC regulator definitions omit their required input supply dependencies (e.g., `vdd_s2-supply`, `vdd_l3-supply`), breaking the power hierarchy.
> > > > > > > > > > > > - [Medium] The device tree inaccurately hardcodes the `compatible` string to a different PMIC model (`qcom,rpm-pm2250-regulators`) instead of explicitly identifying the actual hardware (PM4125).
> > > > > > > > > > > > --
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + pm4125_s2: s2 {
> > > > > > > > > > > > > + regulator-min-microvolt = <1000000>;
> > > > > > > > > > > > > + regulator-max-microvolt = <1200000>;
> > > > > > > > > > > > > + };
> > > > > > > > > > > >
> > > > > > > > > > > > Do these regulators need to explicitly define their input supply dependencies
> > > > > > > > > > > > such as vdd_s2-supply?
> > > > > > > > > > > >
> > > > > > > > > > > > Without these properties, the regulator framework might be unaware that the
> > > > > > > > > > > > PMIC regulators draw power from upstream supplies.
> > > > > > > > > > > >
> > > > > > > > > > > > If the kernel dynamically manages the upstream supply and its reference count
> > > > > > > > > > > > drops to zero, could it be disabled, causing an unexpected power loss for
> > > > > > > > > > > > downstream components?
> > > > > > > > > > >
> > > > > > > > > > > And this is a correct comment. Please provide missing supplies.
> > > > > > > > > > >
> > > > > > > > > > As per the Qualcomm system design, the parent-child supply relationship
> > > > > > > > > > is managed by the RPM firmware, not the Linux regulator framework. The
> > > > > > > > > > RPM ensures the parent supply is never disabled until all subsystem
> > > > > > > > > > votes are cleared.
> > > > > > > > >
> > > > > > > > > How is this different from other, previous platforms?
> > > > > > > >
> > > > > > > > This is not different. In the previous platforms too this is taken care from the
> > > > > > > > RPM/RPMH firmware side, the only case where we may need explicit vote to parent
> > > > > > > > is for non-rpmh/rpm regulator rails (like i2c based regulator pm8008), which
> > > > > > > > may have a RPM/RPMH regulator as a parent.
> > > > > > > >
> > > > > > > > Even on those previous targets the parent rail of all RPM/RPMH regulators are
> > > > > > > > internally voted by RPM/RPMH FW at proper voltage with required headroom
> > > > > > > > calculated based on the active child rails. This was done for all the
> > > > > > > > subsystems (including APPS) regulators.
> > > > > > > >
> > > > > > > > So no explicit handling from the APPS is required for parent supply.
> > > > > > >
> > > > > > > You are explaining the driver behaviour. But the question is about the
> > > > > > > hardware description. If there is no difference, please add necessary
> > > > > > > supplies back.
> > > > > >
> > > > > > I understand your concern about descibing the parent-child relation in the
> > > > > > devicetree, and given that we have been almost always followed this for all
> > > > > > the previous targets, it will expected of us to add them.
> > > > >
> > > > > Yes.
> > > > >
> > > > > >
> > > > > > However, we want to avoid the unnecessary access to the parent from APPS.
> > > > >
> > > > > Why? What is the reason? Do we want to do the same for all the
> > > > > platforms? Only for Shikra? Something else?
> > > > >
> > > > > > At the moment, I do not see a way to avoid that, if we add the parent
> > > > > > regulators.
> > > > >
> > > > > That depend on the answer to the previous question. In the end, we can
> > > > > make the driver ignore the parents by removing them from the regulator
> > > > > desc.
> > > >
> > > > Ok, this seems like a good suggestion, so you mean its ok if we define the
> > > > regulator desc's supply column with NULL? And only keep that in the DT?
> > > >
> > > > you mean like this?
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/regulator/qcom-rpmh-regulator.c?h=v7.1#n1453
> > > >
> > > > (please let me know if i got that right. thanks)
> > >
> > > Yes. Don't forget to explain in the commit message, why you are doing
> > > so.
> >
> > Currently, Agatti uses the same PMIC, so we cannot set the driver
> > supply name reference to NULL. Since it's an older target,
> > we'll need to run a regression before making any driver-level changes.
>
> Sure, just do it please.
>
Sure, I will test this on both Agatti and Shikra and post it as a new
patch series, since the current changes have already been picked up.
> > Additionally, the child-to-parent regulator ganging differs between
> > Shikra and Agatti:
> >
> > - On Agatti, l3 regulator is ganged with vdd_l13_l14_l15_l16
> > - On Shikra, l3 is ganged with vdd_l2_l3
>
> Well, somebody tried to be too smart when contributing Agatti. Now that
> needs to be fixed. Be sure to keep backwards compatibility with the
> existing DTs.
>
> From the schematics that I see, the pins are:
>
> - vin_l1
> - vin_l2_l3
> - vin_l5_l6_l7_l11_l12
> - vin_l8_l9
> - vin_l10
> - vin_l13_l14
> - vin_l15_l16
> - vin_l17_l22
> - vin_l18_l19
> - vin_l4_l20_l21
> - vin_xo_rf
>
> Please correct the bindings and adjust the driver.
>
Sure, I'll fix the Both DT and driver based on the schematic pin names,
keeping backwards compatibility, and post it as a new patch series.
> > Since vdd_l2_l3 is not present as a supply name in the driver, it will
> > be skipped by the driver and would only serve as a representational
> > reference in the DT.
> >
> > We have two options to consider:
> >
> > Option 1: Skip adding the child/parent relationship for Shikra for now,
> > since the DT bindings are not enforcing it. (Ref:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/regulator/qcom,smd-rpm-regulator.yaml?h=v7.1)
> >
> > Option 2: Go ahead and add the Shikra-specific ganging in DT now. Since
> > the supply name (vdd_l2_l3) does not match what the driver expects
> > (Agatti's mapping), it will be gracefully skipped by the driver — making
> > it safe to add for documentation/representation purposes without any
> > functional impact.
> >
> > So,Please share your thoughts on above options ?
>
> Option 3. The PMIC is not Shikra-specific. You've spotted an error.
> Correct the way it is described an used while adding support for Shikra.
>
Sure, I'll correct the driver and DT description for both Agatti and
Shikra and post it as a new patch series.
regards
Rakesh kota
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 3/4] arm64: dts: qcom: Add Shikra CQM SoM platform
From: Rakesh Kota @ 2026-07-10 10:12 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Konrad Dybcio, Kamal Wadhwa, linux-arm-msm, sashiko-reviews,
Komal Bajaj, robh, linux-phy, neil.armstrong, vkoul, olteanv,
krzk+dt, conor+dt, devicetree, Jishnu Prakash
In-Reply-To: <fku5jad6op5upobwvrphbqteg76zukbovsifzarizz4m3ofuka@gdpaxzzuraee>
On Fri, Jul 03, 2026 at 06:32:10PM +0300, Dmitry Baryshkov wrote:
> On Fri, Jul 03, 2026 at 03:00:35PM +0530, Rakesh Kota wrote:
> > On Tue, Jun 30, 2026 at 03:50:16PM +0200, Konrad Dybcio wrote:
> > > On 6/30/26 2:42 PM, Rakesh Kota wrote:
> > > > On Sun, Jun 28, 2026 at 03:33:23PM +0300, Dmitry Baryshkov wrote:
> > > >> On Thu, Jun 25, 2026 at 09:11:19PM +0530, Kamal Wadhwa wrote:
> > > >>> On Wed, Jun 17, 2026 at 03:48:14PM +0300, Dmitry Baryshkov wrote:
> > > >>>> On Mon, 18 May 2026 at 14:49, Kamal Wadhwa
> > > >>>> <kamal.wadhwa@oss.qualcomm.com> wrote:
> > > >>>>>
> > > >>>>> On Sun, May 17, 2026 at 08:18:15PM +0300, Dmitry Baryshkov wrote:
> > > >>>>>> On Thu, May 14, 2026 at 04:09:18PM +0530, Kamal Wadhwa wrote:
> > > >>>>>>> On Wed, May 13, 2026 at 06:14:20PM +0300, Dmitry Baryshkov wrote:
> > > >>>>>>>> On 13/05/2026 17:29, Rakesh Kota wrote:
> > > >>>>>>>>> On Wed, May 13, 2026 at 03:01:47PM +0300, Dmitry Baryshkov wrote:
> > > >>>>>>>>>> On Wed, May 13, 2026 at 04:28:35AM +0000, sashiko-bot@kernel.org wrote:
> > > >>>>>>>>>>> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> > > >>>>>>>>>>> - [High] The PMIC regulator definitions omit their required input supply dependencies (e.g., `vdd_s2-supply`, `vdd_l3-supply`), breaking the power hierarchy.
> > > >>>>>>>>>>> - [Medium] The device tree inaccurately hardcodes the `compatible` string to a different PMIC model (`qcom,rpm-pm2250-regulators`) instead of explicitly identifying the actual hardware (PM4125).
> > > >>>>>>>>>>> --
> > > >>>>>>>>>>>> +
> > > >>>>>>>>>>>> + pm4125_s2: s2 {
> > > >>>>>>>>>>>> + regulator-min-microvolt = <1000000>;
> > > >>>>>>>>>>>> + regulator-max-microvolt = <1200000>;
> > > >>>>>>>>>>>> + };
> > > >>>>>>>>>>>
> > > >>>>>>>>>>> Do these regulators need to explicitly define their input supply dependencies
> > > >>>>>>>>>>> such as vdd_s2-supply?
> > > >>>>>>>>>>>
> > > >>>>>>>>>>> Without these properties, the regulator framework might be unaware that the
> > > >>>>>>>>>>> PMIC regulators draw power from upstream supplies.
> > > >>>>>>>>>>>
> > > >>>>>>>>>>> If the kernel dynamically manages the upstream supply and its reference count
> > > >>>>>>>>>>> drops to zero, could it be disabled, causing an unexpected power loss for
> > > >>>>>>>>>>> downstream components?
> > > >>>>>>>>>>
> > > >>>>>>>>>> And this is a correct comment. Please provide missing supplies.
> > > >>>>>>>>>>
> > > >>>>>>>>> As per the Qualcomm system design, the parent-child supply relationship
> > > >>>>>>>>> is managed by the RPM firmware, not the Linux regulator framework. The
> > > >>>>>>>>> RPM ensures the parent supply is never disabled until all subsystem
> > > >>>>>>>>> votes are cleared.
> > > >>>>>>>>
> > > >>>>>>>> How is this different from other, previous platforms?
> > > >>>>>>>
> > > >>>>>>> This is not different. In the previous platforms too this is taken care from the
> > > >>>>>>> RPM/RPMH firmware side, the only case where we may need explicit vote to parent
> > > >>>>>>> is for non-rpmh/rpm regulator rails (like i2c based regulator pm8008), which
> > > >>>>>>> may have a RPM/RPMH regulator as a parent.
> > > >>>>>>>
> > > >>>>>>> Even on those previous targets the parent rail of all RPM/RPMH regulators are
> > > >>>>>>> internally voted by RPM/RPMH FW at proper voltage with required headroom
> > > >>>>>>> calculated based on the active child rails. This was done for all the
> > > >>>>>>> subsystems (including APPS) regulators.
> > > >>>>>>>
> > > >>>>>>> So no explicit handling from the APPS is required for parent supply.
> > > >>>>>>
> > > >>>>>> You are explaining the driver behaviour. But the question is about the
> > > >>>>>> hardware description. If there is no difference, please add necessary
> > > >>>>>> supplies back.
> > > >>>>>
> > > >>>>> I understand your concern about descibing the parent-child relation in the
> > > >>>>> devicetree, and given that we have been almost always followed this for all
> > > >>>>> the previous targets, it will expected of us to add them.
> > > >>>>
> > > >>>> Yes.
> > > >>>>
> > > >>>>>
> > > >>>>> However, we want to avoid the unnecessary access to the parent from APPS.
> > > >>>>
> > > >>>> Why? What is the reason? Do we want to do the same for all the
> > > >>>> platforms? Only for Shikra? Something else?
> > > >>>>
> > > >>>>> At the moment, I do not see a way to avoid that, if we add the parent
> > > >>>>> regulators.
> > > >>>>
> > > >>>> That depend on the answer to the previous question. In the end, we can
> > > >>>> make the driver ignore the parents by removing them from the regulator
> > > >>>> desc.
> > > >>>
> > > >>> Ok, this seems like a good suggestion, so you mean its ok if we define the
> > > >>> regulator desc's supply column with NULL? And only keep that in the DT?
> > > >>>
> > > >>> you mean like this?
> > > >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/regulator/qcom-rpmh-regulator.c?h=v7.1#n1453
> > > >>>
> > > >>> (please let me know if i got that right. thanks)
> > > >>
> > > >> Yes. Don't forget to explain in the commit message, why you are doing
> > > >> so.
> > > >
> > > > Currently, Agatti uses the same PMIC, so we cannot set the driver
> > > > supply name reference to NULL. Since it's an older target,
> > > > we'll need to run a regression before making any driver-level changes.
> > > >
> > > > Additionally, the child-to-parent regulator ganging differs between
> > > > Shikra and Agatti:
> > > >
> > > > - On Agatti, l3 regulator is ganged with vdd_l13_l14_l15_l16
> > > > - On Shikra, l3 is ganged with vdd_l2_l3
> > >
> > > Is it configurable on the PMIC level? I was under the impression the
> > > supply maps are fixed in hardware. Is there a chance the agatti
> > > description is just wrong?
> >
> > The supply ganging between child LDOs and parent supplies is not fixed
> > at the PMIC hardware level — it varies per platform based on system
> > design requirements. The same LDO can have a different parent supply on
> > different SoCs.
>
> It can have different power supplies, but the supply is provided through
> a fixed pin of the PMIC. Please correct me if I'm wrong.
>
You're right, I apologize for the confusion. The supply rail is fixed in
hardware via physical pin connections. The differing supply name in the
Agatti DTS is likely incorrect.
I'll address this in the next patch series — updating the driver
to set the parent supply to NULL, and correcting the Agatti and Shikra
DT parent supply names to match the actual PMIC pin connections,
since the base change has already been merged.
https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/tree/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi?h=for-next#n52
> > I have verified the Agatti parent-child supply mappings and they are
> > correct. The difference between Agatti and Shikra is a legitimate
> > platform-level design difference, not an error in the Agatti DT.
>
> There is an error in the DT. It uses fake PMIC supply names.
>
yes, i will correct.
regards
Rakesh Kota
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v5 net-next 1/2] dt-bindings: phy: cadence-torrent: Update property values to support multilink SERDES configuration
From: Gokul Praveen @ 2026-07-10 9:46 UTC (permalink / raw)
To: Conor Dooley
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: <20260709-unvalued-washtub-d024d21f5624@spud>
Hi Conor,
On 09/07/26 21:21, Conor Dooley wrote:
> On Thu, Jul 09, 2026 at 03:37:15PM +0530, Praveen, Gokul wrote:
>> Hi Conor,
>>
>> On 08-07-2026 22:09, Conor Dooley wrote:
>>> On Wed, Jul 08, 2026 at 02:07:24PM +0530, Gokul Praveen wrote:
>>>> Update the maxItems value of clocks parameter as 3 clocks
>>>> (refclk,pll1_refclk,phy_en_refclk) are supported.
>>>>
>>>> Update the clock-names parameter to support mutilink SERDES configuration
>>>> as the existing enum configuration of the clock-names parameter does not
>>>> allow both pll1_refclk and phy_en_refclk to be used at the same time,
>>>> hence preventing the support for the configuration (refclk,pll1_refclk,
>>>> phy_en_refclk), which is neeed for multilink SERDES usecases.
>>>>
>>>> For multilink SERDES configurations where the links require different
>>>> clock speeds, all 3 clocks(refclk, pll1_refclk and phy_en_refclk)
>>>> are needed.
>>>>
>>>> For example,considering the USXGMII+SGMII multilink SERDES configuration
>>>> usecase, having only 1 reference clock(refclk) fails because USXGMII
>>>> requires a clock speed of 156.25 Mhz and SGMII protocol requires an
>>>> clock speed of 100 Mhz.
>>>>
>>>> Since one reference clock(refclk) alone cannot cater to the 2
>>>> different clock speed requirements of these protocols, the second
>>>> input reference clock(pll1_refclk) along with phy_en_refclk
>>>> is also needed.
>>> This binding supports 2 devices and the generic compatible. Do all these
>>> devices have the new refclk?
>> Not all of these devices have the new refclk(pll1_refclk), Conor, which is
>> is why the enum was kept as it is and in these devices multilink serdes
>> configuration will not be possible due to the limitation of not having the
>> new refclk(pll1_refclk).
> In that case, please restrict 3 clocks to only the devices which have
> them.
>
> pw-bot: changes-requested
>
> Thanks,
> Conor.
>
Apologies for my earlier message.
I confirmed that all of these devices(ie:2 devices and the generic
compatible) support the 3 clocks.
It was a misinformation earlier from my side.Apologies for that.
Thanking you
Yours respectfully
Gokul Praveen
>> However, The intent of this patch is to add multilink serdes support for the
>> devices which have the new refclk because the
>>
>> earlier clock-names configuration could not support having all the 3
>> clocks(refclk, pll1_refclk, phy_en_refclk) in the clock-names, which is
>> needed for mutlilink serdes configuration configuration.
>>
>> Also, Please feel free to ask if you have any other queries, Conor and thank
>> you for this query .
>>
>> Thanks and Best Regards
>>
>> Gokul Praveen
>>
>>> Thanks,
>>> Conor.
>>>
>>>> Signed-off-by: Gokul Praveen <g-praveen@ti.com>
>>>> ---
>>>> Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml | 3 ++-
>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
>>>> index 9af39b33646a..ac0f625cd76d 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
>>>> @@ -48,6 +48,7 @@ properties:
>>>> items:
>>>> - const: refclk
>>>> - enum: [ pll1_refclk, phy_en_refclk ]
>>>> + - const: phy_en_refclk
>>>> reg:
>>>> minItems: 1
>>>> --
>>>> 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 v3 3/3] phy: rockchip: phy-rockchip-inno-csidphy: add clock lane phase tuning
From: Michael Riesch @ 2026-07-10 9:31 UTC (permalink / raw)
To: Gerald Loacker, Vinod Koul, Neil Armstrong, Heiko Stuebner,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel,
devicetree
In-Reply-To: <20260630-feature-mipi-csi-dphy-4k60-v3-3-176792ab71fa@wolfvision.net>
Hi Gerald,
Thanks for your patch.
On 6/30/26 09:48, Gerald Loacker wrote:
> At high data rates like 4K60 (2500 Mbps), such as when using an
> LT6911GXD bridge chip on an RK3588 board, fixed default timing parameters
> can cause signal integrity issues and clock-data recovery failures.
> The driver currently lacks a mechanism to adjust the clock lane sampling
> phase to compensate for board-specific trace variations.
>
> Resolve this by parsing and applying the optional 'rockchip,clk-lane-phase'
> device tree property. This enables board-specific tuning of the clock
> lane sampling phase in ~40 ps steps (range 0-7) to optimize link
> stability. If the property is absent, the driver falls back to the
> hardware default.
>
> Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
> ---
> drivers/phy/rockchip/phy-rockchip-inno-csidphy.c | 25 ++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> index 5281f8dea0ad3..3a15840e86cad 100644
> --- a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> @@ -69,6 +69,10 @@
> #define RK1808_CSIDPHY_CLK_CALIB_EN 0x168
> #define RK3568_CSIDPHY_CLK_CALIB_EN 0x168
>
> +#define CSIDPHY_LANE_CLK_3_PHASE 0x38
> +#define CSIDPHY_CLK_PHASE_MASK GENMASK(6, 4)
> +#define CSIDPHY_CLK_PHASE_DEFAULT 3
This default value definition is unused right now, but...
> +
> #define RESETS_MAX 2
>
> /*
> @@ -151,6 +155,7 @@ struct rockchip_inno_csidphy {
> const struct dphy_drv_data *drv_data;
> struct phy_configure_opts_mipi_dphy config;
> u8 hsfreq;
> + int clk_phase;
> };
>
> static inline void write_grf_reg(struct rockchip_inno_csidphy *priv,
> @@ -304,6 +309,13 @@ static int rockchip_inno_csidphy_power_on(struct phy *phy)
> rockchip_inno_csidphy_ths_settle(priv, priv->hsfreq,
> CSIDPHY_LANE_THS_SETTLE(i));
>
> + if (priv->clk_phase >= 0) {
...you can make sure that clk_phase has a valid value in any case (apply
default value defined above if DT does not define it or defines
something invalid) and write the register unconditionally.
> + val = readl(priv->phy_base + CSIDPHY_LANE_CLK_3_PHASE);
> + val &= ~CSIDPHY_CLK_PHASE_MASK;
> + val |= FIELD_PREP(CSIDPHY_CLK_PHASE_MASK, priv->clk_phase);
> + writel(val, priv->phy_base + CSIDPHY_LANE_CLK_3_PHASE);
> + }
> +
> write_grf_reg(priv, GRF_DPHY_CSIPHY_CLKLANE_EN, 0x1);
> write_grf_reg(priv, GRF_DPHY_CSIPHY_DATALANE_EN,
> GENMASK(priv->config.lanes - 1, 0));
> @@ -449,6 +461,7 @@ static int rockchip_inno_csidphy_probe(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> struct phy_provider *phy_provider;
> struct phy *phy;
> + u32 phase;
> int ret;
>
> priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> @@ -464,6 +477,18 @@ static int rockchip_inno_csidphy_probe(struct platform_device *pdev)
> return -ENODEV;
> }
>
> + priv->clk_phase = -1;
> + if (device_property_read_u32(dev, "rockchip,clk-lane-phase",
> + &phase) == 0) {
> + if (phase >= BIT(3)) {
if (phase > 7)
> + dev_err(dev,
> + "rockchip,clk-lane-phase %u out of range [0,7]\n",
> + phase);
> + return -EINVAL;
Seems a bit harsh. What would you think about printing a warning and
applying the default value?
> + }
> + priv->clk_phase = phase;
> + }
Maybe
ret = device_property_read_u32(dev, "rockchip,clk-lane-phase",
&priv->clk_phase);
if (ret < 0 || priv->clk_phase > 7) {
dev_info(dev,
"found %s value for rockchip,clk-lane-phase,"
"assuming default value",
ret < 0 ? "no" : "invalid");
priv->clk_phase = CSIDPHY_CLK_PHASE_DEFAULT;
}
would do the trick too?
Best regards,
Michael
> +
> priv->grf = syscon_regmap_lookup_by_phandle(dev->of_node,
> "rockchip,grf");
> if (IS_ERR(priv->grf)) {
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 2/3] dt-bindings: phy: rockchip-inno-csi-dphy: add rockchip,clk-lane-phase property
From: Michael Riesch @ 2026-07-10 9:30 UTC (permalink / raw)
To: Gerald Loacker, Vinod Koul, Neil Armstrong, Heiko Stuebner,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel,
devicetree
In-Reply-To: <20260630-feature-mipi-csi-dphy-4k60-v3-2-176792ab71fa@wolfvision.net>
Hi Gerald,
On 6/30/26 09:48, Gerald Loacker wrote:
> Add support for the optional rockchip,clk-lane-phase device tree property
> to allow board-specific tuning of the clock lane sampling phase for
> improved signal integrity across supported data rates.
>
> Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
Acked-by: Michael Riesch <michael.riesch@collabora.com>
Thanks and best regards,
Michael
> ---
> .../devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> index 03950b3cad08c..913aa688c0ae9 100644
> --- a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
> @@ -56,6 +56,16 @@ properties:
> description:
> Some additional phy settings are access through GRF regs.
>
> + rockchip,clk-lane-phase:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 7
> + default: 3
> + description:
> + Clock lane sampling phase selection (hardware tap index 0–7). Each step
> + corresponds to an approximately 40 ps delay as described in the hardware
> + specification.
> +
> required:
> - compatible
> - reg
>
--
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] phy: rockchip: phy-rockchip-inno-csidphy: fix rk1808 hsfreq table
From: Michael Riesch @ 2026-07-10 9:28 UTC (permalink / raw)
To: Gerald Loacker, Vinod Koul, Neil Armstrong, Heiko Stuebner,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel,
devicetree
In-Reply-To: <20260630-feature-mipi-csi-dphy-4k60-v3-1-176792ab71fa@wolfvision.net>
Hi Gerald,
Thanks for your work.
On 6/30/26 09:48, Gerald Loacker wrote:
> The rk1808 hsfreq table capped at 2499 Mbps, preventing a data rate of
> exactly 2500 Mbps. Extend the final entry to 2500 Mbps to support this
> rate.
>
> This is essential for RK3588 reusing this array and fully supporting
> rates up to 2500 Mbps.
Makes sense to me.
>
> Fixes: bd1f775d6027 ("phy/rockchip: add Innosilicon-based CSI dphy")
> Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
Reviewed-by: Michael Riesch <michael.riesch@collabora.com>
Best regards,
Michael
> ---
> drivers/phy/rockchip/phy-rockchip-inno-csidphy.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> index c79fb53d8ee5c..5281f8dea0ad3 100644
> --- a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> @@ -170,7 +170,7 @@ static const struct hsfreq_range rk1808_mipidphy_hsfreq_ranges[] = {
> { 299, 0x06}, { 399, 0x08}, { 499, 0x0b}, { 599, 0x0e},
> { 699, 0x10}, { 799, 0x12}, { 999, 0x16}, {1199, 0x1e},
> {1399, 0x23}, {1599, 0x2d}, {1799, 0x32}, {1999, 0x37},
> - {2199, 0x3c}, {2399, 0x41}, {2499, 0x46}
> + {2199, 0x3c}, {2399, 0x41}, {2500, 0x46}
> };
>
> static const struct hsfreq_range rk3326_mipidphy_hsfreq_ranges[] = {
>
--
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: nuvoton,ma35d1-usb2-phy: extend for dual-port and OTG
From: Krzysztof Kozlowski @ 2026-07-10 8:54 UTC (permalink / raw)
To: Joey Lu
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: <20260708103606.1462960-2-a0987203069@gmail.com>
On Wed, Jul 08, 2026 at 06:36:04PM +0800, Joey Lu wrote:
> + 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
> @@ -39,7 +78,7 @@ examples:
>
> usb_phy: usb-phy {
> compatible = "nuvoton,ma35d1-usb2-phy";
> - clocks = <&clk USBD_GATE>;
> + clocks = <&clk HUSBH0_GATE>;
This change is really redundant. Instead, add optional properties like
nuvoton,rcalcode and nuvoton,oc-active-high.
The rest looks good, so with above change:
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
* Re: [PATCH RFC v4 9/9] arm64: dts: qcom: glymur: Wire PCIe3a/3b to shared Gen5x8 PHY
From: Konrad Dybcio @ 2026-07-10 8:44 UTC (permalink / raw)
To: Qiang Yu
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Bjorn Andersson, Konrad Dybcio,
linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <alBWrK8X7fW+UC7L@hu-qianyu-lv.qualcomm.com>
On 7/10/26 4:19 AM, Qiang Yu wrote:
> On Mon, Jun 29, 2026 at 11:20:07AM +0200, Konrad Dybcio wrote:
>> On 6/29/26 7:05 AM, Qiang Yu wrote:
>>> On Wed, Jun 17, 2026 at 01:19:49PM +0200, Konrad Dybcio wrote:
>>>> On 5/19/26 7:47 AM, Qiang Yu wrote:
>>>>> Glymur PCIe3 uses a single shared Gen5x8 QMP PHY block. Model PCIe3a and
>>>>> PCIe3b as consumers of that shared PHY provider instead of separate PHY
>>>>> nodes.
>>>>>
>>>>> Update the DTS wiring to:
>>>>> - point GCC PCIe3A/3B pipe parents to the shared PHY clock outputs
>>>>> - add PCIe3a controller node and route PCIe3a/PCIe3b port phys to
>>>>> &pcie3_phy using two-cell PHY arguments
>>>>> - configure the shared PHY node with link-mode and dual pipe outputs
>>>>>
>>>>> Use QMP_PCIE_GLYMUR_MODE_* dt-binding macros for mode selection.
>>>>>
>>>>> Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
>>>>> ---
>>>>
>>>> [...]
>>>>
>>>>> + pcie3a: pci@1c10000 {
>>>>> + device_type = "pci";
>>>>> + compatible = "qcom,glymur-pcie", "qcom,pcie-x1e80100";
>>>>> + reg = <0x0 0x01c10000 0x0 0x3000>,
>>>>> + <0x0 0x70000000 0x0 0xf20>,
>>>>> + <0x0 0x70000f40 0x0 0xa8>,
>>>>> + <0x0 0x70001000 0x0 0x4000>,
>>>>> + <0x0 0x70100000 0x0 0x100000>,
>>>>> + <0x0 0x01c13000 0x0 0x1000>;
>>>>> + reg-names = "parf",
>>>>> + "dbi",
>>>>> + "elbi",
>>>>> + "atu",
>>>>> + "config",
>>>>> + "mhi";
>>>>> + #address-cells = <3>;
>>>>> + #size-cells = <2>;
>>>>> + ranges = <0x01000000 0x0 0x00000000 0x0 0x70200000 0x0 0x100000>,
>>>>> + <0x02000000 0x0 0x70000000 0x0 0x70300000 0x0 0x3d00000>,
>>>>> + <0x03000000 0x7 0x00000000 0x7 0x00000000 0x0 0x40000000>,
>>>>> + <0x43000000 0x70 0x00000000 0x70 0x00000000 0x10 0x00000000>;
>>>>> +
>>>>> + bus-range = <0 0xff>;
>>>>> +
>>>>> + dma-coherent;
>>>>> +
>>>>> + linux,pci-domain = <3>;
>>>>> + num-lanes = <8>;
>>>>
>>>> Is it fine to keep num-lanes 8 here even for configurations with
>>>> bifurcated PHY?
>>>>
>>>> I would assume so, given essentially this is a x8 host, whose 4
>>>> lanes may simply be effectively NC
>>>>
>>> Actually, on existing platforms, the PCIe3a and PCIe3b controllers are
>>> never enabled at the same time. When PCIe3a is exposed, it is always in an
>>> x8 slot. But if we have a x4+x4 platform in future, we can simply override
>>> num-lanes to 4 in the board.dts.
>>
>> My question is whether that will be necessary - if yes, sure, we
>> can do it, but if not, we can conclude on this early and not have
>> to fight over it in a couple months
>>
> I think we do need to override it in that case. If both PCIe3a and PCIe3b
> are enabled in x4+x4 mode but PCIe3a keeps num-lanes = <8>, userspace
> will see an 8-lane slot. If an x8-capable EP is connected to that slot,
> both ends will advertise x8 support, but the link is up at x4. That looks
> like a genuine bug from the user's point of view.
Do we know what's advertised on x86 PCs with bifurcated lanes?
Konrad
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH RFC v4 9/9] arm64: dts: qcom: glymur: Wire PCIe3a/3b to shared Gen5x8 PHY
From: Qiang Yu @ 2026-07-10 2:19 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Bjorn Andersson, Konrad Dybcio,
linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <ae2e1bdd-59b0-4ff7-bd6f-ddd57267c2d9@oss.qualcomm.com>
On Mon, Jun 29, 2026 at 11:20:07AM +0200, Konrad Dybcio wrote:
> On 6/29/26 7:05 AM, Qiang Yu wrote:
> > On Wed, Jun 17, 2026 at 01:19:49PM +0200, Konrad Dybcio wrote:
> >> On 5/19/26 7:47 AM, Qiang Yu wrote:
> >>> Glymur PCIe3 uses a single shared Gen5x8 QMP PHY block. Model PCIe3a and
> >>> PCIe3b as consumers of that shared PHY provider instead of separate PHY
> >>> nodes.
> >>>
> >>> Update the DTS wiring to:
> >>> - point GCC PCIe3A/3B pipe parents to the shared PHY clock outputs
> >>> - add PCIe3a controller node and route PCIe3a/PCIe3b port phys to
> >>> &pcie3_phy using two-cell PHY arguments
> >>> - configure the shared PHY node with link-mode and dual pipe outputs
> >>>
> >>> Use QMP_PCIE_GLYMUR_MODE_* dt-binding macros for mode selection.
> >>>
> >>> Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> >>> ---
> >>
> >> [...]
> >>
> >>> + pcie3a: pci@1c10000 {
> >>> + device_type = "pci";
> >>> + compatible = "qcom,glymur-pcie", "qcom,pcie-x1e80100";
> >>> + reg = <0x0 0x01c10000 0x0 0x3000>,
> >>> + <0x0 0x70000000 0x0 0xf20>,
> >>> + <0x0 0x70000f40 0x0 0xa8>,
> >>> + <0x0 0x70001000 0x0 0x4000>,
> >>> + <0x0 0x70100000 0x0 0x100000>,
> >>> + <0x0 0x01c13000 0x0 0x1000>;
> >>> + reg-names = "parf",
> >>> + "dbi",
> >>> + "elbi",
> >>> + "atu",
> >>> + "config",
> >>> + "mhi";
> >>> + #address-cells = <3>;
> >>> + #size-cells = <2>;
> >>> + ranges = <0x01000000 0x0 0x00000000 0x0 0x70200000 0x0 0x100000>,
> >>> + <0x02000000 0x0 0x70000000 0x0 0x70300000 0x0 0x3d00000>,
> >>> + <0x03000000 0x7 0x00000000 0x7 0x00000000 0x0 0x40000000>,
> >>> + <0x43000000 0x70 0x00000000 0x70 0x00000000 0x10 0x00000000>;
> >>> +
> >>> + bus-range = <0 0xff>;
> >>> +
> >>> + dma-coherent;
> >>> +
> >>> + linux,pci-domain = <3>;
> >>> + num-lanes = <8>;
> >>
> >> Is it fine to keep num-lanes 8 here even for configurations with
> >> bifurcated PHY?
> >>
> >> I would assume so, given essentially this is a x8 host, whose 4
> >> lanes may simply be effectively NC
> >>
> > Actually, on existing platforms, the PCIe3a and PCIe3b controllers are
> > never enabled at the same time. When PCIe3a is exposed, it is always in an
> > x8 slot. But if we have a x4+x4 platform in future, we can simply override
> > num-lanes to 4 in the board.dts.
>
> My question is whether that will be necessary - if yes, sure, we
> can do it, but if not, we can conclude on this early and not have
> to fight over it in a couple months
>
I think we do need to override it in that case. If both PCIe3a and PCIe3b
are enabled in x4+x4 mode but PCIe3a keeps num-lanes = <8>, userspace
will see an 8-lane slot. If an x8-capable EP is connected to that slot,
both ends will advertise x8 support, but the link is up at x4. That looks
like a genuine bug from the user's point of view.
- Qiang Yu
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v11 35/36] phy: rockchip: usbdp: Re-init PHY on mux change
From: sashiko-bot @ 2026-07-09 19:41 UTC (permalink / raw)
To: Sebastian Reichel
Cc: devicetree, neil.armstrong, conor+dt, olteanv, robh, vkoul,
linux-phy
In-Reply-To: <20260709-rockchip-usbdp-cleanup-v11-35-a149ac60f76c@collabora.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] Deadlock / Recursive Lock
Pre-existing issues:
- [High] Silent DP PHY State Loss
--
--- Patch [35]: [PATCH v11 35/36] phy: rockchip: usbdp: Re-init PHY on mux change ---
commit 63e6102eaff345b4ebf6faefafd6aae88e7bb98e
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 without
DP being involved, and when switching from DP-only into a mode with USB
support.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index c5ad300620a65..ecf0ed0139f69 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1383,7 +1383,7 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
>
> rk_udphy_set_typec_state(udphy, state->mode);
>
> - return 0;
> + return rk_udphy_update_power_state(udphy);
[Severity: High]
Could this lead to a recursive locking deadlock if the DWC3 controller is
runtime-suspended?
Since rk_udphy_typec_mux_set() holds udphy->mutex using a guard, the
following chain seems to re-acquire the same mutex:
rk_udphy_typec_mux_set()
guard(mutex)(&udphy->mutex);
rk_udphy_update_power_state()
phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_PRE_RESET);
If the DWC3 driver handles this notification in device role:
dwc3_usb3_phy_notify()
pm_runtime_resume_and_get()
...
dwc3_resume_common()
dwc3_core_init_for_resume()
dwc3_phy_init()
phy_init()
The PHY init then attempts to acquire the lock again:
rk_udphy_usb3_phy_init()
guard(mutex)(&udphy->mutex);
Will this cause a system freeze when hotplugging or switching modes?
[Severity: High]
This isn't a bug introduced by this patch, but does invoking
rk_udphy_update_power_state() during a mux change cause the DP link to be
permanently lost?
When the power state updates, the PHY is re-initialized:
rk_udphy_update_power_state()
rk_udphy_init()
regmap_write(..., FIELD_PREP(CMN_DP_LANE_EN_ALL, 0));
This reset sequence clears the CMN_DP_LANE_EN_ALL bits and disables DP
transmission. Since rk_udphy_dp_lane_enable() does not appear to be called
afterward to restore the active DP configuration, does the DRM subsystem
lose the link without a full power cycle?
> }
>
> static void rk_udphy_typec_mux_unregister(void *data)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@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 v11 28/36] phy: rockchip: usbdp: Only enable USB3 when not in high-speed mode
From: sashiko-bot @ 2026-07-09 19:35 UTC (permalink / raw)
To: Sebastian Reichel
Cc: linux-phy, vkoul, conor+dt, devicetree, neil.armstrong, olteanv,
robh
In-Reply-To: <20260709-rockchip-usbdp-cleanup-v11-28-a149ac60f76c@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The USB3 port is unconditionally disabled during PHY re-initialization but never re-enabled, permanently breaking USB3 functionality after a mode change.
--
--- Patch [28]: [PATCH v11 28/36] phy: rockchip: usbdp: Only enable USB3 when not in high-speed mode ---
commit f42d06c6d124fe98eab8964760884de8f170d304
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Only enable USB3 when not in high-speed mode
Ensure that USB3 mode is not accidently enabled during PHY re-init
for systems that are configured as high-speed only via DT.
Closes: https://sashiko.dev/#/message/20260626212424.C215E1F000E9%40smtp.kernel.org
> 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
> @@ -1009,7 +1009,7 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> if (ret)
> return ret;
>
> - if (udphy->mode & UDPHY_MODE_USB)
> + if (!udphy->hs && udphy->mode & UDPHY_MODE_USB)
> rk_udphy_u3_port_disable(udphy, false);
> udphy->phy_needs_reinit = false;
> } else if (udphy->phy_needs_reinit) {
> rk_udphy_u3_port_disable(udphy, true);
> udelay(10);
>
> ret = rk_udphy_init(udphy);
> if (ret)
> return ret;
> udphy->phy_needs_reinit = false;
> }
[Severity: High]
This is a pre-existing issue, but does this missing re-enablement permanently
break USB3 functionality after a mode change?
If the PHY is already active and a mode change occurs (such as plugging
in a DP monitor or a Type-C cable flip), udphy->phy_needs_reinit is set
to true.
When rk_udphy_power_on() is subsequently called, the U3 port is disabled
in this branch to avoid xHCI SErrors, but there is no corresponding call to
rk_udphy_u3_port_disable(udphy, false) to restore it after rk_udphy_init()
completes.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=28
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v11 33/36] phy: rockchip: usbdp: Rename mode to hw_mode
From: sashiko-bot @ 2026-07-09 19:35 UTC (permalink / raw)
To: Sebastian Reichel
Cc: conor+dt, vkoul, devicetree, linux-phy, neil.armstrong, olteanv,
robh
In-Reply-To: <20260709-rockchip-usbdp-cleanup-v11-33-a149ac60f76c@collabora.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] TYPEC_STATE_USB and TYPEC_STATE_SAFE mux events incorrectly configure the PHY for DisplayPort instead of USB-only.
- [High] rk_udphy_power_off and rk_udphy_usb3_phy_exit incorrectly use hw_mode to bypass power-off, permanently leaking the PHY state and preventing rk_udphy_disable() from being called.
--
--- Patch [33]: [PATCH v11 33/36] phy: rockchip: usbdp: Rename mode to hw_mode ---
commit 6f61047a0ab430913024fc15e4a26038f67189e4
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Rename mode to hw_mode
Rename mode field to hw_mode to make clear that this is the modes
currently supported by the hardware, but not necessarily requested
by software. It is only set by either the USB-C state machine
or device-tree if the PHY is used in a fixed routing setup.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 3fb6eea2979b9..e2cd72643a7d7 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -616,12 +616,12 @@ static void rk_udphy_set_typec_state(struct rk_udphy *udphy, unsigned long state
> udphy->lane_mux_sel[2] = PHY_LANE_MUX_DP;
> udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
> }
> - mode = UDPHY_MODE_DP_USB;
> + hw_mode = UDPHY_MODE_DP_USB;
[Severity: High]
This isn't a bug introduced by this patch, but since the switch statement lacks
explicit cases for TYPEC_STATE_USB and TYPEC_STATE_SAFE, do they fall through
to the default case here and incorrectly configure the PHY for DisplayPort
instead of USB-only?
If a standard USB device is connected or the port is in a safe/unattached state,
the Type-C subsystem fires a mux event with state->mode = TYPEC_STATE_USB (or
TYPEC_STATE_SAFE), and it seems they would erroneously be assigned
UDPHY_MODE_DP_USB with two DisplayPort lanes instead of just UDPHY_MODE_USB.
> udphy->dp_lanes = 2;
> break;
> }
[ ... ]
> @@ -1033,7 +1033,7 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
>
> static void rk_udphy_power_off(struct rk_udphy *udphy, u8 mode)
> {
> - if (!(udphy->mode & mode)) {
> + if (!(udphy->hw_mode & mode)) {
[Severity: High]
This is a pre-existing issue, but does using hw_mode here bypass the power-off
sequence and permanently leak the physical PHY state?
If a Type-C event changes the hardware mode to DP-only (hw_mode =
UDPHY_MODE_DP), the USB mode is dropped. If rk_udphy_power_off() is called
afterwards, this check will bypass physical shutdown since the requested mode
was already removed from hw_mode, preventing rk_udphy_disable() from being
called.
> dev_info(udphy->dev, "mode 0x%02x is not support\n", mode);
> return;
> }
[ ... ]
> @@ -1307,7 +1307,7 @@ static int rk_udphy_usb3_phy_exit(struct phy *phy)
> guard(mutex)(&udphy->mutex);
>
> /* DP only or high-speed */
> - if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs) {
> + if (!(udphy->hw_mode & UDPHY_MODE_USB) || udphy->hs) {
[Severity: High]
This is a pre-existing issue, but this appears to have the same flaw as
rk_udphy_power_off() above.
When the USB consumer subsequently calls phy_exit(), and hw_mode has already
dropped the USB mode, this returns early without delegating to
rk_udphy_power_off(). If the PHY status reached UDPHY_MODE_NONE,
rk_udphy_disable() is never called, and the PHY remains physically powered on
indefinitely.
> udphy->status &= ~UDPHY_MODE_USB;
> return 0;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=33
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v11 32/36] phy: rockchip: usbdp: Drop -EPROBE_DEFER hack
From: sashiko-bot @ 2026-07-09 19:34 UTC (permalink / raw)
To: Sebastian Reichel
Cc: devicetree, robh, neil.armstrong, conor+dt, vkoul, linux-phy,
olteanv
In-Reply-To: <20260709-rockchip-usbdp-cleanup-v11-32-a149ac60f76c@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing PHY_NOTIFY_POST_RESET notification on the rk_udphy_setup() error path permanently leaks the PM runtime usage counter and leaves the USB controller in soft reset.
--
--- Patch [32]: [PATCH v11 32/36] phy: rockchip: usbdp: Drop -EPROBE_DEFER hack ---
Note: The format of this review has been altered to a free-form summary due
to system recitation restrictions. Standard inline diff quoting has been
omitted.
commit 734b15714983185f791bb43091c9362d69abf8d8
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Drop -EPROBE_DEFER hack
This commit removes the -EPROBE_DEFER return from rk_udphy_status_check()
when the LCPLL lock times out. It notes that the driver now performs a reset
during probe and handles re-initialization later, making the deferral
unnecessary.
[Severity: High]
In drivers/phy/rockchip/phy-rockchip-usbdp.c, specifically within
rk_udphy_power_on():
When transitioning from UDPHY_MODE_NONE, the driver sends a
PHY_NOTIFY_PRE_RESET notification. Immediately following this, it calls
rk_udphy_setup(). If rk_udphy_setup() fails (for example, due to the LCPLL
lock timeout propagating up from rk_udphy_status_check or a clock enable
failure), the function returns the error directly.
Does this error path permanently leak the PM runtime usage counter and leave
the USB controller in a soft reset state?
Since PHY_NOTIFY_POST_RESET is never sent on this early return error path,
consumer drivers like DWC3 that asserted a soft reset and incremented their
PM runtime usage counter on PRE_RESET will never receive the matching
POST_RESET to recover.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@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 v11 31/36] phy: rockchip: usbdp: Add phy reset notification support
From: sashiko-bot @ 2026-07-09 19:32 UTC (permalink / raw)
To: Sebastian Reichel
Cc: robh, conor+dt, vkoul, olteanv, devicetree, neil.armstrong,
linux-phy
In-Reply-To: <20260709-rockchip-usbdp-cleanup-v11-31-a149ac60f76c@collabora.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing PHY_NOTIFY_POST_RESET notification on the rk_udphy_setup() error path.
- [High] The return value of phy_notify_reset(..., PHY_NOTIFY_PRE_RESET) is completely ignored.
- [High] The USB3 port is permanently left disabled after a PHY re-initialization.
--
--- Patch [31]: [PATCH v11 31/36] phy: rockchip: usbdp: Add phy reset notification support ---
commit 8e70526891d152fa8ddbd23ac5f83716e6e5bc55
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Add phy reset notification support
This commit adds support for reset notifications to gracefully handle the
PHY being disabled. This avoids corrupting the controller's internal state
and fixes lock timeout errors after device replugs.
I noticed a few potential regressions related to error handling and hardware
enablement in this patch.
> 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
> @@ -1005,6 +1005,11 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> }
>
> if (udphy->status == UDPHY_MODE_NONE) {
> + phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_PRE_RESET);
[Severity: High]
Should the return value of phy_notify_reset() be checked here? If the
consumer's pre-reset notifier returns an error code, proceeding with the
hardware reset could potentially trigger xHCI SErrors, and unconditionally
calling the post-reset notification later might unbalance the PM runtime
reference.
> +
> + rk_udphy_u3_port_disable(udphy, true);
> + udelay(10);
> +
> ret = rk_udphy_setup(udphy);
> if (ret)
> return ret;
[Severity: High]
Does this error path leak a PM runtime reference by leaving the DWC3
controller permanently in a pre-reset state? It appears the early return
skips balancing the prior PHY_NOTIFY_PRE_RESET notification with a
PHY_NOTIFY_POST_RESET call.
[ ... ]
> @@ -1012,13 +1017,22 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> if (!udphy->hs && udphy->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);
[Severity: High]
Similar to the check above, does this code need to handle errors returned
from phy_notify_reset() to avoid hardware issues or underflowing the
atomic reset count?
> +
> rk_udphy_u3_port_disable(udphy, true);
> udelay(10);
>
> ret = rk_udphy_init(udphy);
> - if (ret)
> + if (ret) {
> + phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_POST_RESET);
> return ret;
> + }
> +
> + phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_POST_RESET);
> +
> udphy->phy_needs_reinit = false;
> }
[Severity: High]
Is the USB3 port intentionally left disabled on this success path? Unlike
the cold power-on branch above it, there is no corresponding call to
rk_udphy_u3_port_disable(udphy, false) when rk_udphy_init() succeeds.
Could this permanently break USB3 connectivity after a PHY re-initialization
or mode change?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=31
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v11 29/36] phy: core: add notifier infrastructure
From: sashiko-bot @ 2026-07-09 19:31 UTC (permalink / raw)
To: Sebastian Reichel
Cc: linux-phy, devicetree, robh, vkoul, neil.armstrong, olteanv,
conor+dt
In-Reply-To: <20260709-rockchip-usbdp-cleanup-v11-29-a149ac60f76c@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Potential self-deadlock or AB-BA deadlock between `phy->mutex` and `phy->notifier.rwsem` when consumers handle reset notifications.
--
--- Patch [29]: [PATCH v11 29/36] phy: core: add notifier infrastructure ---
commit 205cc0767ab713005453a3401f261c9b0a0289ce
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: core: add notifier infrastructure
This commit introduces a new PHY notifier API to allow consumer drivers to
handle reset events coming from a PHY device. It adds functions to register
and unregister notifiers, and to trigger reset notifications.
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
[ ... ]
> +int phy_notify_reset(struct phy *phy, enum phy_notification event)
> +{
> + int ret;
> +
> + if (!phy)
> + return 0;
> +
> + ret = blocking_notifier_call_chain(&phy->notifier, event, phy);
[Severity: Medium]
Could this lead to a self-deadlock or an AB-BA deadlock between phy->mutex
and the notifier rwsem?
If a PHY provider calls phy_notify_reset() from within a PHY operation
callback that executes with phy->mutex held, the notifier chain takes a
read lock on phy->notifier.rwsem. If a consumer callback then attempts to
quiesce its hardware by calling a PHY framework API like phy_power_off(),
it will try to acquire phy->mutex again, resulting in a self-deadlock:
phy_set_mode()
mutex_lock(&phy->mutex);
provider_ops->set_mode()
phy_notify_reset()
blocking_notifier_call_chain() // takes rwsem read lock
consumer_callback()
phy_power_off()
mutex_lock(&phy->mutex); // self-deadlock
Conversely, if phy_notify_reset() is called outside of phy->mutex, but the
consumer calls back into the PHY framework, it establishes a lock dependency
from the notifier rwsem to phy->mutex. A concurrent PHY operation that holds
phy->mutex and registers a notifier could create a classic AB-BA deadlock if
a writer is pending on the rwsem.
Should the documentation warn consumers to defer PHY API calls to a
workqueue when handling these notifications, or should the lock hierarchy
be explicitly defined?
> + return notifier_to_errno(ret);
> +}
> +EXPORT_SYMBOL_GPL(phy_notify_reset);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=29
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v11 23/36] phy: rockchip: usbdp: Clear USB status on PHY exit
From: sashiko-bot @ 2026-07-09 19:28 UTC (permalink / raw)
To: Sebastian Reichel
Cc: olteanv, linux-phy, neil.armstrong, conor+dt, robh, devicetree,
vkoul
In-Reply-To: <20260709-rockchip-usbdp-cleanup-v11-23-a149ac60f76c@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Clearing the `UDPHY_MODE_USB` flag from `udphy->status` on the early exit path can leave the hardware powered on and leak clock references if the overall PHY status drops to `UDPHY_MODE_NONE`.
--
--- Patch [23]: [PATCH v11 23/36] phy: rockchip: usbdp: Clear USB status on PHY exit ---
commit 7906fa37f6a1255421fda4caa815efbe878ca40d
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Clear USB status on PHY exit
This commit ensures the USB status flag is cleared when the USB3 PHY is
exited while the system is in DP-only mode to handle unbound controllers.
Link: https://sashiko.dev/#/message/20260625-rockchip-usbdp-cleanup-v7-24-38eb3cf654fd%40collabora.com
> 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
> @@ -1273,8 +1273,10 @@ static int rk_udphy_usb3_phy_exit(struct phy *phy)
> guard(mutex)(&udphy->mutex);
>
> /* DP only or high-speed */
> - if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
> + if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs) {
> + udphy->status &= ~UDPHY_MODE_USB;
> return 0;
> + }
[Severity: High]
Will this early return leak clock references if the overall PHY status drops
to 0 (UDPHY_MODE_NONE)?
If the USB3 PHY was previously initialized (enabling clocks via
rk_udphy_setup) and the mode switches to DP-only, udphy->status gets
cleared here.
Returning immediately skips rk_udphy_power_off() and its internal check
that calls rk_udphy_disable() when the status reaches 0.
Could this leave the hardware powered on and leak the clock references
obtained during initialization?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=23
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v11 26/36] phy: rockchip: usbdp: Avoid xHCI SErrors
From: sashiko-bot @ 2026-07-09 19:28 UTC (permalink / raw)
To: Sebastian Reichel
Cc: olteanv, neil.armstrong, devicetree, vkoul, linux-phy, robh,
conor+dt
In-Reply-To: <20260709-rockchip-usbdp-cleanup-v11-26-a149ac60f76c@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The USB3 port is unconditionally disabled before PHY re-initialization to avoid xHCI SErrors, but it is never re-enabled afterwards, permanently breaking USB3 functionality.
--
--- Patch [26]: [PATCH v11 26/36] phy: rockchip: usbdp: Avoid xHCI SErrors ---
commit 1f5e9aad179e51fb32f2117a4d8ff35248e145cc
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Avoid xHCI SErrors
The USBDP PHY provides the PIPE clock to the USB3 controller, which means
the PHY must be fully running when anything tries to access the xHCI
registers. This patch avoids SErrors during re-initialization by disabling
the USB3 port beforehand, forcing the clock source to a safe fallback.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index edee27933d890..9e77513643a33 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1000,8 +1000,8 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> rk_udphy_u3_port_disable(udphy, false);
> udphy->phy_needs_reinit = false;
> } else if (udphy->phy_needs_reinit) {
> - if (udphy->mode == UDPHY_MODE_DP)
> - rk_udphy_u3_port_disable(udphy, true);
> + rk_udphy_u3_port_disable(udphy, true);
> + udelay(10);
>
> ret = rk_udphy_init(udphy);
> if (ret)
[Severity: High]
Does this code permanently break the USB3 functionality?
We now unconditionally disable the USB3 port here before re-initializing the
PHY, but it appears there is no corresponding call to
rk_udphy_u3_port_disable(udphy, false) to re-enable it after rk_udphy_init()
completes.
Will this leave the DWC3 controller stuck on the UTMI fallback clock with
phystatus forced to 0, making the interface unusable after a mode switch?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=26
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v11 21/36] phy: rockchip: usbdp: Properly handle TYPEC_STATE_SAFE and TYPEC_STATE_USB
From: sashiko-bot @ 2026-07-09 19:27 UTC (permalink / raw)
To: Sebastian Reichel
Cc: vkoul, olteanv, devicetree, linux-phy, robh, neil.armstrong,
conor+dt
In-Reply-To: <20260709-rockchip-usbdp-cleanup-v11-21-a149ac60f76c@collabora.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Exiting DP AltMode fails to physically restore USB lane muxing if the USB controller remains logically active.
- [High] `rk_udphy_usb3_phy_exit` leaks the USB PHY power state if the Type-C mux mode was changed to DP-only before the USB controller calls `phy_exit`.
--
--- Patch [21]: [PATCH v11 21/36] phy: rockchip: usbdp: Properly handle TYPEC_STATE_SAFE and TYPEC_STATE_USB ---
commit 8663a5e7459552707b3ef82190885d5f5b02aa6b
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Properly handle TYPEC_STATE_SAFE and TYPEC_STATE_USB
This commit adds support for TYPEC_STATE_SAFE and TYPEC_STATE_USB Type-C
state events to ensure muxing is properly updated when exiting DP AltMode.
It fixes an issue in the rockchip usbdp combo phy driver.
Fixes: 2f70bbddeb45 ("phy: rockchip: add usbdp combo phy driver")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/message/20260619155020.CC7361F000E9%40smtp.kernel.org
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 4e54020d17551..0399cbf96e195 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1293,17 +1293,26 @@ static const struct phy_ops rk_udphy_usb3_phy_ops = {
> .owner = THIS_MODULE,
> };
[Severity: High]
This is a pre-existing issue, but I am wondering if rk_udphy_usb3_phy_exit()
leaks the USB PHY power state when the Type-C mux mode is changed to DP-only
before the USB controller calls phy_exit()?
If the Type-C mux switches to 4-lane DP AltMode, udphy->mode becomes
UDPHY_MODE_DP. If the USB controller then suspends or is unbound, it calls
rk_udphy_usb3_phy_exit(), which returns early:
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_usb3_phy_exit() {
...
/* DP only or high-speed */
if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
goto unlock;
rk_udphy_power_off(udphy, UDPHY_MODE_USB);
...
}
This skips rk_udphy_power_off(), leaving UDPHY_MODE_USB set in udphy->status.
Since status never reaches UDPHY_MODE_NONE when DP later powers off,
rk_udphy_disable() is never called. Will the PHY clocks and resets remain
permanently active when they should be suspended?
>
> +static bool rk_udphy_is_supported_mode(struct typec_mux_state *state)
> +{
> + /* Handle Safe State and USB State */
> + if (state->mode < TYPEC_STATE_MODAL)
> + return true;
> +
> + /* Handle DP AltMode */
> + if (state->alt && state->alt->svid == USB_TYPEC_DP_SID)
> + return true;
> +
> + return false;
> +}
> +
> static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
> struct typec_mux_state *state)
> {
> struct rk_udphy *udphy = typec_mux_get_drvdata(mux);
>
> - /*
> - * Ignore mux events not involving DP AltMode, because
> - * the mode field is being reused, e.g. state->mode == 4
> - * could be either TYPEC_MODE_USB4 or TYPEC_DP_STATE_C.
> - */
> - if (!state->alt || state->alt->svid != USB_TYPEC_DP_SID)
> + /* Ignore mux events not involving USB or DP */
> + if (!rk_udphy_is_supported_mode(state))
> return 0;
>
> mutex_lock(&udphy->mutex);
[Severity: High]
This is a pre-existing issue, but can the PHY get permanently stuck in 4-lane
DP mode when exiting DP AltMode if the USB controller remains logically active?
When rk_udphy_typec_mux_set() handles the transition to TYPEC_STATE_SAFE
or TYPEC_STATE_USB, it updates the software mode and sets phy_needs_reinit
to true via rk_udphy_set_typec_state() later in this function:
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_typec_mux_set() {
...
rk_udphy_set_typec_state(udphy, state->mode);
...
}
However, the hardware muxing is only updated in rk_udphy_power_on():
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_power_on() {
...
} else if (udphy->phy_needs_reinit) {
if (udphy->mode == UDPHY_MODE_DP)
rk_udphy_u3_port_disable(udphy, true);
ret = rk_udphy_init(udphy);
...
}
If the USB controller (e.g. dwc3) is already active, it will not call
phy_power_on() again. This causes the hardware re-initialization to be missed,
leaving the PHY physically in 4-lane DP mode. Is it possible for USB 3.x
to become completely unresponsive in this state, even though the Type-C
subsystem correctly returned to USB mode?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=21
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v11 19/36] phy: rockchip: usbdp: Re-init the PHY on orientation change
From: sashiko-bot @ 2026-07-09 19:26 UTC (permalink / raw)
To: Sebastian Reichel
Cc: devicetree, olteanv, robh, neil.armstrong, conor+dt, linux-phy,
vkoul
In-Reply-To: <20260709-rockchip-usbdp-cleanup-v11-19-a149ac60f76c@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The USB 3.0 PHY hardware is not re-initialized during an orientation switch for USB-only devices, causing USB 3.0 to silently fail.
--
--- Patch [19]: [PATCH v11 19/36] phy: rockchip: usbdp: Re-init the PHY on orientation change ---
commit 68cb52c178c7db7d63bbe3d9ed1bf373d3c040be
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Re-init the PHY on orientation change
This commit updates the Type-C orientation switch callback to track cable
orientation changes. It sets a flag to trigger PHY re-initialization when
the cable is flipped to fix DisplayPort functionality.
> 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
> @@ -620,6 +620,7 @@ static int rk_udphy_orien_sw_set(struct typec_switch_dev *sw,
> enum typec_orientation orien)
> {
> struct rk_udphy *udphy = typec_switch_get_drvdata(sw);
> + bool flipped = orien == TYPEC_ORIENTATION_REVERSE;
>
> mutex_lock(&udphy->mutex);
>
> @@ -631,7 +632,10 @@ static int rk_udphy_orien_sw_set(struct typec_switch_dev *sw,
> goto unlock_ret;
> }
>
> - udphy->flip = orien == TYPEC_ORIENTATION_REVERSE;
> + if (udphy->flip != flipped)
> + udphy->phy_needs_reinit = true;
[Severity: High]
This is a pre-existing issue, but does this actually fix the orientation swap
for fast replugs on USB-only devices?
If a user rapidly unplugs and replugs a Type-C cable upside down, the DWC3
controller's 5-second runtime PM autosuspend delay means the PHY active state
is not dynamically cycled via phy_init or phy_power_on.
Because rk_udphy_orien_sw_set() only sets a deferred phy_needs_reinit flag,
the hardware lane muxing will remain stuck in the old orientation if the PHY
is already active. This causes USB 3.0 connections to silently fail and fall
back to USB 2.0 speeds.
Is there a mechanism to force the hardware re-initialization immediately if
the PHY is currently active during this swap?
> +
> + udphy->flip = flipped;
> rk_udphy_set_typec_default_mapping(udphy);
> rk_udphy_usb_bvalid_enable(udphy, true);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=19
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v11 17/36] phy: rockchip: usbdp: Drop DP HPD handling
From: sashiko-bot @ 2026-07-09 19:23 UTC (permalink / raw)
To: Sebastian Reichel
Cc: conor+dt, linux-phy, olteanv, vkoul, neil.armstrong, devicetree,
robh
In-Reply-To: <20260709-rockchip-usbdp-cleanup-v11-17-a149ac60f76c@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The typec_mux_set callback explicitly ignores base Type-C states (TYPEC_STATE_SAFE and TYPEC_STATE_USB), preventing the PHY from reverting to standard USB mode when exiting DP AltMode.
--
--- Patch [17]: [PATCH v11 17/36] phy: rockchip: usbdp: Drop DP HPD handling ---
commit 4dd3353e7fa8e127b70281da8c2e85645bfbef65
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Drop DP HPD handling
This commit removes the HPD (Hot Plug Detect) handling logic from the
USBDP PHY driver, delegating it to the displayport controller. It also
simplifies the typec_mux_set callback by dropping AltMode specific event
handling and adding an early return for non-DP events.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 8ac6a83b0b2a9..24108816e3b95 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -1309,6 +1267,14 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
> struct rk_udphy *udphy = typec_mux_get_drvdata(mux);
> u8 mode;
>
> + /*
> + * Ignore mux events not involving DP AltMode, because
> + * the mode field is being reused, e.g. state->mode == 4
> + * could be either TYPEC_MODE_USB4 or TYPEC_DP_STATE_C.
> + */
> + if (!state->alt || state->alt->svid != USB_TYPEC_DP_SID)
> + return 0;
[Severity: High]
This isn't a bug introduced by this patch, but does this early return
prevent the PHY from reverting to standard USB mode when exiting DP AltMode?
When a user dynamically disables DP AltMode or the DP sink disconnects,
the Type-C core invokes typec_mux_set with state->mode = TYPEC_STATE_SAFE
and state->alt = NULL.
If this returns 0 immediately because state->alt is NULL, udphy->mode
remains stuck at UDPHY_MODE_DP. Later, rk_udphy_usb3_phy_init() will see
that the USB mode is not set (!(udphy->mode & UDPHY_MODE_USB)) and
will actively disable the USB3 port.
Should this condition be adjusted to only filter modal states, perhaps by
checking if state->mode >= TYPEC_STATE_MODAL before filtering based on
the AltMode SVID?
> +
> mutex_lock(&udphy->mutex);
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=17
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v11 12/36] phy: rockchip: usbdp: Limit DP lane count to muxed lanes
From: sashiko-bot @ 2026-07-09 19:21 UTC (permalink / raw)
To: Sebastian Reichel
Cc: neil.armstrong, robh, conor+dt, devicetree, olteanv, linux-phy,
vkoul
In-Reply-To: <20260709-rockchip-usbdp-cleanup-v11-12-a149ac60f76c@collabora.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] The DP lane bounds check and PHY configuration execute without holding the driver's mutex, creating a TOCTOU race condition with concurrent Type-C events.
- [High] The newly added lane count check is bypassed during standalone DP voltage updates, allowing the same USB3 register corruption the patch intended to fix.
--
--- Patch [12]: [PATCH v11 12/36] phy: rockchip: usbdp: Limit DP lane count to muxed lanes ---
commit 42fd02565f1c138c8faee25d5fd929ceb0f2e7e3
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
Subject: phy: rockchip: usbdp: Limit DP lane count to muxed lanes
This patch adds a check to limit the DisplayPort lane count to the muxed
lanes, preventing the DP controller from configuring 4 lanes when the PHY
is restricted to 2 lanes. This aims to protect USB3 usage from being
corrupted by incorrect voltage configurations.
Link: https://lore.kernel.org/linux-phy/20260612165546.98E1F1F000E9@smtp.kernel.org/
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index c4d62b234ea80..b172ce6a722ee 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1099,6 +1099,9 @@ static int rk_udphy_dp_phy_verify_link_rate(struct rk_udphy *udphy,
> static int rk_udphy_dp_phy_verify_lanes(struct rk_udphy *udphy,
> struct phy_configure_opts_dp *dp)
> {
> + if (dp->lanes > udphy->dp_lanes)
> + return -EINVAL;
> +
[Severity: High]
This is a pre-existing issue, but does this bounds check need to be
protected by a lock?
If a Type-C event (like a cable flip reducing DP lanes from 4 to 2)
occurs concurrently with a DP link training operation, could the bounds
check pass while udphy->dp_lanes is 4?
If a context switch allows the Type-C handler to reduce the lane count
and change udphy->lane_mux_sel while holding udphy->mutex, when this
thread resumes and proceeds to rk_udphy_dp_phy_configure() (which is
called without holding udphy->mutex), could it overwrite hardware
registers of physical lanes now assigned to USB3:
rk_udphy_dp_phy_configure() {
...
lane = udphy->dp_lane_sel[i];
...
regmap_update_bits(udphy->pma_regmap,
TRSV_ANA_TX_CLK_OFFSET_N(lane),
LN_ANA_TX_SER_TXCLK_INV,
FIELD_PREP(LN_ANA_TX_SER_TXCLK_INV,
udphy->lane_mux_sel[lane]));
...
}
[Severity: High]
This is also a pre-existing issue, but will this check be bypassed
during standalone DP voltage updates?
If a Type-C event asynchronously reduces udphy->dp_lanes (e.g., from
4 down to 2), and the DP controller subsequently issues a standalone
voltage update (where dp->set_voltages=1 and dp->set_lanes=0) prior to
a full link retrain:
rk_udphy_dp_phy_configure() {
...
if (dp->set_lanes) {
ret = rk_udphy_dp_phy_verify_lanes(udphy, dp);
if (ret)
return ret;
}
...
}
Would rk_udphy_dp_phy_verify_lanes() be bypassed entirely? This would
allow the configuration loop to iterate up to the stale cached
udphy->lanes limit:
rk_udphy_dp_phy_configure() {
...
for (i = 0; i < udphy->lanes; i++) {
...
and potentially cause the out-of-bounds writes to the USB3 hardware
registers that this patch intends to prevent.
> switch (dp->lanes) {
> case 1:
> case 2:
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=12
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v11 02/36] phy: rockchip: usbdp: Update mode_change after error handling
From: sashiko-bot @ 2026-07-09 19:20 UTC (permalink / raw)
To: Sebastian Reichel
Cc: olteanv, linux-phy, devicetree, conor+dt, robh, neil.armstrong,
vkoul
In-Reply-To: <20260709-rockchip-usbdp-cleanup-v11-2-a149ac60f76c@collabora.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] When changing the PHY mode from DP-only to a state that includes USB (e.g., DP+USB), the U3 port remains permanently disabled, breaking USB 3.0 functionality.
- [High] The PHY is left permanently enabled and clocks leak due to flawed mode-checking in the power off path during Type-C role switches.
- [High] Re-initializing the PHY during a mode change while the USB controller is active disables PHY clocks without re-routing the DWC3 clock, leading to a fatal SError on register access.
--
--- Patch [2]: [PATCH v11 02/36] phy: rockchip: usbdp: Update mode_change after error handling ---
commit 1bc42e532da1064edae3f14df8f37ade40a0072d
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Update mode_change after error handling
If rk_udphy_init() or rk_udphy_setup() fails, the reinit will not be
tried again. Fix this by only updating the variable after all potential
errors have been handled.
Link: https://lore.kernel.org/linux-phy/20260612163835.8D5471F000E9@smtp.kernel.org/
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index fba35510d88ce..a5ffdf7a6e76b 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1000,15 +1000,14 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> }
>
> if (udphy->status == UDPHY_MODE_NONE) {
> - udphy->mode_change = false;
> ret = rk_udphy_setup(udphy);
> if (ret)
> return ret;
>
> if (udphy->mode & UDPHY_MODE_USB)
> rk_udphy_u3_port_disable(udphy, false);
> - } else if (udphy->mode_change) {
> udphy->mode_change = false;
> + } else if (udphy->mode_change) {
> udphy->status = UDPHY_MODE_NONE;
> if (udphy->mode == UDPHY_MODE_DP)
> rk_udphy_u3_port_disable(udphy, true);
[Severity: High]
This isn't a bug introduced by this patch, but does this lead to a fatal
SError when changing to a mode that includes USB?
If a mode change to DP+USB triggers rk_udphy_power_on(), the code skips calling
rk_udphy_u3_port_disable(udphy, true). It then unconditionally calls
rk_udphy_disable(udphy) below, which turns off the PHY clocks.
Because the USB3 port wasn't disabled to safely re-route the clock, and the
DWC3 driver isn't notified via phy_notify_reset(), the active DWC3 controller
could crash the system when it touches its registers.
>
> rk_udphy_disable(udphy);
> ret = rk_udphy_setup(udphy);
> if (ret)
> return ret;
> + udphy->mode_change = false;
> }
[Severity: High]
This isn't a bug introduced by this patch, but does this skip re-enabling the
U3 port when the new mode requires USB?
When changing the PHY mode from DP-only to a state that includes USB (e.g.,
DP+USB), the U3 port remains permanently disabled. The mode change block
fails to call rk_udphy_u3_port_disable(udphy, false) to re-enable it for the
newly added USB 3.0 mode, leaving USB functionality broken until a power cycle.
[Severity: High]
This isn't a bug introduced by this patch, but can the PHY be left
permanently enabled during Type-C role switches?
When the Type-C mux changes mode from DP+USB to DP-only, udphy->mode loses
the UDPHY_MODE_USB bit immediately.
Later, when the USB core calls rk_udphy_usb3_phy_exit():
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_usb3_phy_exit() {
...
/* DP only or high-speed */
if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
goto unlock;
rk_udphy_power_off(udphy, UDPHY_MODE_USB);
...
}
It sees the new mode and skips calling rk_udphy_power_off(). Even if it
were called, rk_udphy_power_off() also refuses to act if the requested
mode is absent from udphy->mode:
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_power_off() {
...
if (!(udphy->mode & mode)) {
dev_info(udphy->dev, "mode 0x%02x is not support\n", mode);
return;
}
...
}
Consequently, udphy->status incorrectly retains the USB bit, preventing
rk_udphy_disable() from ever powering down the PHY and leaking clocks.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.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 v11 06/36] phy: rockchip: usbdp: Drop seamless DP takeover
From: sashiko-bot @ 2026-07-09 19:19 UTC (permalink / raw)
To: Sebastian Reichel
Cc: conor+dt, vkoul, neil.armstrong, olteanv, linux-phy, robh,
devicetree
In-Reply-To: <20260709-rockchip-usbdp-cleanup-v11-6-a149ac60f76c@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] If PHY setup fails during a mode change, the error path leaves the state machine inconsistent, leading to an unbalanced clock disable when the PHY is later powered off.
--
--- Patch [6]: [PATCH v11 06/36] phy: rockchip: usbdp: Drop seamless DP takeover ---
commit 6e8a746d34fb8b93e5acd40e0d0766f740b5b995
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Drop seamless DP takeover
This commit removes the initial status checking code from the Rockchip USBDP
combo PHY driver, dropping support for seamless DP takeover. The feature is
currently unsupported by DRM drivers and bootloaders, and would require
complex TCPM state synchronization for USB-C alternate modes. Removing it
cleans up unused code until it can be properly implemented.
Closes: https://lore.kernel.org/linux-phy/20260612164107.C7DB21F000E9@smtp.kernel.org/
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 3fc8222fcaec2..6cb9f6b4dbf6d 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -1495,10 +1468,6 @@ static int rk_udphy_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> - ret = rk_udphy_get_initial_status(udphy);
> - if (ret)
> - return ret;
> -
> mutex_init(&udphy->mutex);
> platform_set_drvdata(pdev, udphy);
>
[Severity: High]
This is a pre-existing issue, but I noticed a potential state inconsistency
in the PHY mode change error path.
In drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_power_on():
} else if (udphy->mode_change) {
if (udphy->mode == UDPHY_MODE_DP)
rk_udphy_u3_port_disable(udphy, true);
rk_udphy_disable(udphy);
ret = rk_udphy_setup(udphy);
if (ret)
return ret;
If rk_udphy_setup() fails (e.g. due to a PLL lock timeout), the function
returns an error directly without updating udphy->status.
Since the hardware has already been disabled by the preceding call to
rk_udphy_disable(), but udphy->status still reflects the previous active
state, could this lead to a double-disable of the bulk clocks when the
consumer later tears down the initial mode and calls phy_power_off()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=6
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v11 11/36] phy: rockchip: usbdp: Support single-lane DP
From: sashiko-bot @ 2026-07-09 19:19 UTC (permalink / raw)
To: Sebastian Reichel
Cc: vkoul, neil.armstrong, conor+dt, linux-phy, robh, devicetree,
olteanv
In-Reply-To: <20260709-rockchip-usbdp-cleanup-v11-11-a149ac60f76c@collabora.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing bounds check for requested DP lanes allows out-of-bounds register corruption.
- [High] Data race and unprotected register access in `rk_udphy_dp_phy_configure()` due to missing mutex lock.
--
--- Patch [11]: [PATCH v11 11/36] phy: rockchip: usbdp: Support single-lane DP ---
commit 543954c74eaa703430fe27a5fd2cfce0594a6d78
Author: Zhang Yubing <yubing.zhang@rock-chips.com>
phy: rockchip: usbdp: Support single-lane DP
Implement support for using just a single DisplayPort line.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 7540c897dde13..c4d62b234ea80 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -193,6 +193,7 @@ struct rk_udphy {
> int id;
>
> bool dp_in_use;
> + int dp_lanes;
[Severity: High]
This isn't a bug introduced by this patch, but does the requested lane
count need to be bounded against the newly added udphy->dp_lanes?
If a DRM bridge calls phy_configure() with dp->lanes = 4 when
udphy->dp_lanes = 2, it seems rk_udphy_dp_phy_verify_lanes() will
accept it:
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_dp_phy_verify_lanes() {
switch (dp->lanes) {
case 1:
case 2:
case 4:
/* valid lane count. */
udphy->lanes = dp->lanes;
break;
...
}
Would this cause rk_udphy_dp_phy_configure() to iterate out-of-bounds of
the DP-assigned lanes and apply DP voltage/pre-emphasis settings to USB
PMA analog registers, potentially corrupting the USB3 PHY state?
[ ... ]
> @@ -1050,18 +1039,17 @@ static int rk_udphy_dp_phy_exit(struct phy *phy)
> static int rk_udphy_dp_phy_power_on(struct phy *phy)
> {
> struct rk_udphy *udphy = phy_get_drvdata(phy);
> - int ret, dp_lanes;
> + int ret;
>
> mutex_lock(&udphy->mutex);
[Severity: High]
This is a pre-existing issue, but while rk_udphy_dp_phy_power_on() acquires
udphy->mutex before modifying driver state, rk_udphy_dp_phy_configure()
appears to access shared state and hardware registers without this lock:
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_dp_phy_configure() {
struct rk_udphy *udphy = phy_get_drvdata(phy);
struct phy_configure_opts_dp *dp = &opts->dp;
...
/* No mutex_lock(&udphy->mutex) here */
...
}
Can a Type-C event (like a cable flip or alt mode change) concurrently
trigger rk_udphy_typec_mux_set() and race with rk_udphy_dp_phy_configure()?
Could this data race on udphy->lane_mux_sel and concurrent writes to the
pma_regmap lead to hardware corruption or link training failures?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=11
--
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