Re: [PATCH 04/10] phy: qcom-ipq806x-usb: add MSM8974 HS PHY support

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 8/10/26 1:42 PM, Dmitry Baryshkov wrote:
> The MSM8974 DWC3 wrapper embeds the same generation of QSCRATCH
> controlled Synopsys 28nm HS PHY as IPQ806x, but the driver only knows
> the IPQ806x variants, so on MSM8974 the controller runs on whatever
> PHY state the bootloader left behind.
> 
> Add the qcom,msm8974-usb-phy-hs compatible with its own init
> sequence: unlike the IPQ806x one, it must not overwrite the whole
> PHY_CTRL register, as the power-on values of the reference frequency
> selection straps have to be preserved. Only enable the UTMI clock and
> the DP/DM MPM clamps, signal VBUS valid towards the link, and disable
> retention.
> 
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
>  drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c | 35 +++++++++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
> index f22c0000479f..21b59d4ca8d2 100644
> --- a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
> +++ b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
> @@ -17,6 +17,7 @@
>  #define HSUSB_PHY_CTRL_REG		(0x10)
>  
>  /* PHY_CTRL_REG */
> +#define HSUSB_CLAMP_MPM_DPSE_DMSE_EN_N	BIT(26)
>  #define HSUSB_CTRL_DMSEHV_CLAMP		BIT(24)
>  #define HSUSB_CTRL_USB2_SUSPEND		BIT(23)
>  #define HSUSB_CTRL_UTMI_CLK_EN		BIT(21)
> @@ -27,9 +28,13 @@
>  #define HSUSB_CTRL_ID_HV_CLAMP		BIT(9)
>  #define HSUSB_CTRL_OTGSESSVLD_CLAMP	BIT(8)
>  #define HSUSB_CTRL_CLAMP_EN		BIT(7)
> +#define HSUSB_CTRL_CLAMP_MPM_DPSE_DMSE	BIT(26)

leftover?

>  #define HSUSB_CTRL_RETENABLEN		BIT(1)
>  #define HSUSB_CTRL_POR			BIT(0)
>  
> +/* PHY parameter override register (eye diagram tuning) */
> +#define HSUSB_PARAMETER_OVERRIDE_X_REG	(0x14)
> +
>  /* QSCRATCH_GENERAL_CFG */
>  #define HSUSB_GCFG_XHCI_REV		BIT(2)
>  
> @@ -115,6 +120,8 @@
>  #define LATCH_SLEEP				40
>  #define LATCH_TIMEOUT				100
>  
> +struct phy_drvdata;

Can you just move the declaration here?

> +
>  struct usb_phy {
>  	void __iomem		*base;
>  	struct device		*dev;
> @@ -123,11 +130,14 @@ struct usb_phy {
>  	u32			rx_eq;
>  	u32			tx_deamp_3_5db;
>  	u32			mpll;
> +	const struct phy_drvdata *drvdata;
>  };
>  
>  struct phy_drvdata {
>  	struct phy_ops	ops;
>  	u32		clk_rate;
> +	u32		hs_tune_val;
> +	u32		fsel;
>  };
>  
>  /**
> @@ -273,7 +283,8 @@ static int qcom_ipq806x_usb_hs_phy_init(struct phy *phy)
>  		HSUSB_CTRL_RETENABLEN  | HSUSB_CTRL_COMMONONN |
>  		HSUSB_CTRL_OTGSESSVLD_CLAMP | HSUSB_CTRL_ID_HV_CLAMP |
>  		HSUSB_CTRL_UTMI_OTG_VBUS_VALID | HSUSB_CTRL_UTMI_CLK_EN |
> -		HSUSB_CTRL_CLAMP_EN | 0x70;
> +		HSUSB_CTRL_CLAMP_EN | HSUSB_CLAMP_MPM_DPSE_DMSE_EN_N |

This will *disable* (_N suffix) the clamp on ipq806x too - is that intended?


> +		phy_dwc3->drvdata->fsel;
>  
>  	/* use core clock if external reference is not present */
>  	if (!phy_dwc3->xo_clk)
> @@ -282,9 +293,14 @@ static int qcom_ipq806x_usb_hs_phy_init(struct phy *phy)
>  	writel(val, phy_dwc3->base + HSUSB_PHY_CTRL_REG);
>  	usleep_range(2000, 2200);
>  
> -	/* Disable (bypass) VBUS and ID filters */
> +	/* Set XHCI_REV bit (2) to 1 - XHCI version 1.0 */
>  	writel(HSUSB_GCFG_XHCI_REV, phy_dwc3->base + QSCRATCH_GENERAL_CFG);
>  
> +	if (phy_dwc3->drvdata->hs_tune_val)
> +		usb_phy_write_readback(phy_dwc3, HSUSB_PARAMETER_OVERRIDE_X_REG,
> +				       0x03ffffff,

GENMASK(25, 0)

Konrad