Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] Typo in macro name: LPFS instead of LFPS.
- [Low] RX registers mistakenly prefixed with _TX_.
- [Low] Missing _N4 prefix in macro definition.
--
--- Patch [4]: [PATCH v4 4/5] phy: qualcomm: qmp-combo: Add USB4/TBT3 configuration data for Hamoa ---
commit afe3f2e27c72fec44781edf41959e4fc3ad27e3e
Author: Konrad Dybcio <[email protected]>
phy: qualcomm: qmp-combo: Add USB4/TBT3 configuration data for Hamoa
Add 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 dfdeec3ee4f5f..c150a4ef03a78 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
[ ... ]
> @@ -2028,6 +2107,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_RX_SIGDET_CNTRL, 0x04),
[ ... ]
> + 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),
[Severity: Low]
This isn't a bug, but should these UCDR (Universal Clock and Data Recovery)
registers be prefixed with _RX_ instead of _TX_? They are functionally
receiver registers falling within the RX offset block, and are being
initialized here inside x1e80100_usb4_rx_tbl.
[ ... ]
> 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_G3_FOM_EQ_CONFIG5 0x218
> +#define QPHY_V6_PCS_USB4_LPFS_TX_ECSTART 0x220
[Severity: Low]
This isn't a bug, but is there a typo in this macro name? It uses LPFS
instead of 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_TRAN_DRVR_EMP_EN 0xac
> #define QSERDES_V6_N4_TX_TX_BAND 0xd8
> #define QSERDES_V6_N4_TX_INTERFACE_SELECT 0xe4
[ ... ]
> +#define QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH3_RATE210 0xf8
> +#define QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH3_RATE3 0xfc
> #define QSERDES_V6_N4_TX_VMODE_CTRL1 0xb0
> +#define QSERDES_V6_TX_LDO_TIMER_CTRL 0x120
[Severity: Low]
This isn't a bug, but is this macro missing the _N4 infix? All other
SoC-specific macros added in this header use the _N4 format (e.g.,
QSERDES_V6_N4_TX_LANE_MODE_1).
--
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
--
linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.