* Re: [PATCH v3 4/4] phy: qualcomm: qmp-combo: Add support for Hawi SoC
From: Dmitry Baryshkov @ 2026-07-20 22:24 UTC (permalink / raw)
To: sashiko
Cc: Ronak Raheja, robh, krzk+dt, olteanv, devicetree, vkoul, conor+dt,
neil.armstrong, linux-phy, linux-arm-msm
In-Reply-To: <20260508232620.2FD04C2BCB0@smtp.kernel.org>
On Fri, May 08, 2026 at 11:26:19PM +0000, sashiko-bot@kernel.org 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
^ permalink raw reply
page: | 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