Re: [PATCH 4/5] phy: qualcomm: qmp-combo: Add USB4/TBT3 configuration data for Hamoa - sashiko-bot

From: sashiko-bot@kernel.org
To: "Konrad Dybcio" <konradybcio@kernel.org>
Cc: olteanv@gmail.com, linux-phy@lists.infradead.org,
	devicetree@vger.kernel.org, conor+dt@kernel.org, robh@kernel.org,
	neil.armstrong@linaro.org, vkoul@kernel.org
Subject: Re: [PATCH 4/5] phy: qualcomm: qmp-combo: Add USB4/TBT3 configuration data for Hamoa
Date: Mon, 18 May 2026 11:49:29 +0000	[thread overview]
Message-ID: <20260518114931.B7A0DC2BCB7@smtp.kernel.org> (raw)
In-Reply-To: <20260518-topic-usb4phy-v1-4-71d827c49dca@oss.qualcomm.com>

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Low] Incorrect `TX_` prefix for RX UCDR fastlock registers.
- [Low] Typo in register macro name: `LPFS` instead of `LFPS`.
- [Low] Redundant `PCS_PCS` prefix in register macro name.
- [Low] Missing `_N4_` infix in the `QSERDES_V6_TX_LDO_TIMER_CTRL` macro.
--

commit 13a3731f3b7c12c298d2f498011a7ef852fd4a42
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

phy: qualcomm: qmp-combo: Add USB4/TBT3 configuration data for Hamoa

This commit adds the offsets and configuration tables to support USB4
and Thunderbolt 3 operation on the USB4-capable PHYs found on Hamoa chips.

> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 898f42de4a086..0916d9a4599ed 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c

[ ... ]

> @@ -1873,6 +1952,68 @@ static const struct qmp_phy_init_tbl x1e80100_usb43dp_tx_tbl[] = {
> [ ... ]
> +static const struct qmp_phy_init_tbl x1e80100_usb4_rx_tbl[] = {
> [ ... ]
> +	QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_LOW_RATE2, 0x30),
> +	QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_HIGH_RATE2, 0x14),
> +	QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_LOW_RATE3, 0xff),
> +	QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_HIGH_RATE3, 0x3f),

This isn't a bug, but should these use the RX_ prefix instead of TX_? 
These registers appear to map to the RX block address space and are used
here to configure the RX hardware.

[ ... ]

> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-usb-v6.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-usb-v6.h
> index df670143feb1e..30fdb437146c4 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-usb-v6.h
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-usb-v6.h
> @@ -14,4 +14,19 @@
>  #define QPHY_V6_PCS_USB3_RCVR_DTCT_DLY_U3_L		0x40
>  #define QPHY_V6_PCS_USB3_RCVR_DTCT_DLY_U3_H		0x44
>  
> +#define QPHY_V6_PCS_USB4_POWER_STATE_CONFIG3		0x98
> [ ... ]
> +#define QPHY_V6_PCS_USB4_PCS_PCS_TX_RX_CONFIG1		0x1a8

This isn't a bug, but is there a duplicate PCS_ prefix in this macro name?

> +#define QPHY_V6_PCS_USB4_EQ_CONFIG1			0x1b8
> [ ... ]
> +#define QPHY_V6_PCS_USB4_LPFS_TX_ECSTART		0x220

This isn't a bug, but is this a typo for LFPS (Low Frequency Periodic
Signaling)?

[ ... ]

> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-v6_n4.h b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-v6_n4.h
> index d37cc0d4fd365..e72ae2cb460ac 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-v6_n4.h
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-v6_n4.h
> @@ -23,27 +23,63 @@
> [ ... ]
>  #define QSERDES_V6_N4_TX_VMODE_CTRL1			0xb0
> +#define QSERDES_V6_TX_LDO_TIMER_CTRL			0x120

This isn't a bug, but should this macro include the _N4_ infix for
consistency with the rest of the V6 N4 block definitions?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518-topic-usb4phy-v1-0-71d827c49dca@oss.qualcomm.com?part=4