Re: [PATCH v3 4/4] phy: qualcomm: qmp-combo: Add support for Hawi SoC

On Fri, May 08, 2026 at 11:26:19PM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Missing DP PHY function pointers in `hawi_usb3dpphy_cfg` lead to NULL pointer dereferences when the DP PHY is initialized.
> - [High] Valid register offset `0x00` causes the AON toggle hardware reset override to be silently skipped.

Please add DP programming. There should be no NULL dereferences.

> > @@ -3699,9 +3915,9 @@ static int qmp_combo_com_init(struct qmp_combo *qmp, bool force)
> >  			SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
> >  
> >  	/* override hardware control for reset of qmp phy */
> > -	if (pcs_aon && cfg->regs[QPHY_AON_TOGGLE_ENABLE]) {
> > -		qphy_clrbits(pcs_aon, cfg->regs[QPHY_AON_TOGGLE_ENABLE], 0x1);
> > -		qphy_clrbits(pcs_aon, cfg->regs[QPHY_DP_AON_TOGGLE_ENABLE], 0x1);
> > +	if (aon_toggle && cfg->regs[QPHY_AON_TOGGLE_ENABLE]) {
> > +		qphy_clrbits(aon_toggle, cfg->regs[QPHY_AON_TOGGLE_ENABLE], 0x1);
> > +		qphy_clrbits(aon_toggle, cfg->regs[QPHY_DP_AON_TOGGLE_ENABLE], 0x1);
> >  	}
> 
> Is it possible for the hardware reset override to be silently skipped here?
> 
> The V10 register layout maps QPHY_AON_TOGGLE_ENABLE to
> QPHY_V10_COM_AON_USB3_AON_TOGGLE_ENABLE, which is defined as 0x00 in
> phy-qcom-qmp-com-aon-v10.h.
> 
> Because 0x00 is a valid register offset but evaluates to false in C, the
> check for cfg->regs[QPHY_AON_TOGGLE_ENABLE] will fail. This would skip the
> qphy_clrbits() calls entirely, bypassing the necessary hardware reset override
> sequence and leaving the PHY in a broken state. 
> 
> Could we use a different way to check if this register is supported by the
> layout without relying on the truthiness of the offset value?

This deserves a comment.

-- 
With best wishes
Dmitry