From: sashiko-bot@kernel.org
To: "George Moussalem" <george.moussalem@outlook.com>
Cc: linux-phy@lists.infradead.org, robh@kernel.org,
conor+dt@kernel.org, devicetree@vger.kernel.org,
vkoul@kernel.org, neil.armstrong@linaro.org, olteanv@gmail.com
Subject: Re: [PATCH v2 02/14] phy: qcom: Introduce Qualcomm IPQ5332 Super-Speed USB UNIPHY driver
Date: Wed, 12 Aug 2026 09:48:28 +0000 [thread overview]
Message-ID: <20260812094829.0A0381F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260812-ipq5018-usb3-v2-2-8f355d7ff0ef@outlook.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing `platform_set_drvdata()` in the probe function causes a NULL pointer dereference in the remove callback.
- [Medium] Probe failure paths fail to restore the TCSR multiplexer to its default hardware state.
- [Medium] The driver fails to `select MFD_SYSCON` in Kconfig, risking a silent probe failure.
--
--- Patch [2]: [PATCH v2 02/14] phy: qcom: Introduce Qualcomm IPQ5332 Super-Speed USB UNIPHY driver ---
commit 2ff7b4ffaf9a300ab45c516bb3b192d9f0a5c88e
Author: Praveenkumar I <quic_ipkumar@quicinc.com>
phy: qcom: Introduce Qualcomm IPQ5332 Super-Speed USB UNIPHY driver
Adds Qualcomm 22ull Super-Speed USB UNIPHY driver support which is
present in Qualcomm IPQ5018 & IPQ5332 SoCs.
This PHY is interfaced with SNPS DWC3 USB and SNPS DWC PCIe. Either one
of the interface can use the it and selection is done via mux present in
TCSR register. By default, the PHY is configured for PCIe interface and
needs this driver to configure the mux for USB.
> diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
> index 60a0ead127fa9..1ceb04afe1e3a 100644
> --- a/drivers/phy/qualcomm/Kconfig
> +++ b/drivers/phy/qualcomm/Kconfig
> @@ -158,6 +158,16 @@ config PHY_QCOM_UNIPHY_PCIE_28LP
> handles PHY initialization, clock management required after
> resetting the hardware and power management.
>
> +config PHY_QCOM_UNIPHY_USB_22ULL
> + tristate "Qualcomm IPQ5332 UNIPHY USB Super-Speed 22ull driver"
> + depends on USB && (ARCH_QCOM || COMPILE_TEST)
> + select GENERIC_PHY
[Severity: Medium]
Since this driver uses syscon_regmap_lookup_by_phandle_args(), should it
select MFD_SYSCON to ensure the required syscon utilities are compiled in
and avoid silent probe failures?
> + help
> + Enable this to support the Qualcomm USB Super-Speed UNIPHY transceiver
[ ... ]
> diff --git a/drivers/phy/qualcomm/phy-qcom-uniphy-usb-ss-22ull.c b/drivers/phy/qualcomm/phy-qcom-uniphy-usb-ss-22ull.c
> new file mode 100644
> index 0000000000000..089de7cebef03
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-uniphy-usb-ss-22ull.c
[ ... ]
> +static int qcom_uniphy_usb_probe(struct platform_device *pdev)
> +{
[ ... ]
> + /*
> + * Select the USB position of the phy mux before anything comes out
> + * of reset. Selecting it later leaves the SuperSpeed pads muxed away
> + * while the controller resets, and the SS link doesn't train and is
> + * stuck in Rx.Detect (USB2 is unaffected as it isn't muxed).
> + */
> + ret = qcom_uniphy_usb_mux_select(uniphy, true);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to select USB mux\n");
> +
> + uniphy->phy = devm_phy_create(dev, NULL, &uniphy_usb_ops);
> + if (IS_ERR(uniphy->phy)) {
> + ret = PTR_ERR(uniphy->phy);
> + return dev_err_probe(dev, ret, "failed to create PHY\n");
> + }
[Severity: Medium]
If devm_phy_create() or phy_pipe_clk_register() fail after the mux has been
selected, the function returns directly without restoring the hardware state.
Should there be a cleanup action (perhaps via devm_add_action_or_reset())
to restore the multiplexer to its default PCIe state on probe failure?
> +
> + ret = phy_pipe_clk_register(uniphy, uniphy->phy->id);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to register pipe clk\n");
> +
> + phy_set_drvdata(uniphy->phy, uniphy);
[Severity: High]
Does this function also need to call platform_set_drvdata(pdev, uniphy)?
Currently qcom_uniphy_usb_remove() relies on platform_get_drvdata(pdev),
which would return NULL if not set here, and cause a NULL pointer dereference
when passed to qcom_uniphy_usb_mux_select().
> +
> + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +
> + ret = PTR_ERR_OR_ZERO(phy_provider);
> +
> + return ret;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260812-ipq5018-usb3-v2-0-8f355d7ff0ef@outlook.com?part=2
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
next prev parent reply other threads:[~2026-08-12 9:48 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-12 9:34 [PATCH v2 00/14] Enable USB3 for Qualcomm IPQ5018 and IPQ5332 George Moussalem via B4 Relay
2026-08-12 9:34 ` [PATCH v2 01/14] dt-bindings: phy: Document the Qualcomm IPQ5332 UNIPHY USB Super-speed PHY George Moussalem via B4 Relay
2026-08-12 9:41 ` sashiko-bot
2026-08-12 9:34 ` [PATCH v2 02/14] phy: qcom: Introduce Qualcomm IPQ5332 Super-Speed USB UNIPHY driver George Moussalem via B4 Relay
2026-08-12 9:48 ` sashiko-bot [this message]
2026-08-12 9:34 ` [PATCH v2 03/14] arm64: dts: qcom: ipq5018: Add USB Super-Speed PHY node George Moussalem via B4 Relay
2026-08-12 9:51 ` sashiko-bot
2026-08-12 9:34 ` [PATCH v2 04/14] arm64: dts: qcom: ipq5332: Flatten usb controller node George Moussalem via B4 Relay
2026-08-12 10:25 ` sashiko-bot
2026-08-12 9:34 ` [PATCH v2 05/14] arm64: dts: qcom: ipq5018: " George Moussalem via B4 Relay
2026-08-12 9:56 ` sashiko-bot
2026-08-12 9:34 ` [PATCH v2 06/14] dt-bindings: usb: qcom,snps-dwc3: Add clocks for USB3 on Qualcomm IPQ5332 George Moussalem via B4 Relay
2026-08-12 9:46 ` sashiko-bot
2026-08-12 9:34 ` [PATCH v2 07/14] dt-bindings: usb: qcom,snps-dwc3: Add clocks for USB3 on Qualcomm IPQ5018 George Moussalem via B4 Relay
2026-08-12 9:51 ` sashiko-bot
2026-08-12 9:34 ` [PATCH v2 08/14] arm64: dts: qcom: ipq5332: Add clocks required for USB3 support George Moussalem via B4 Relay
2026-08-12 9:34 ` [PATCH v2 09/14] arm64: dts: qcom: ipq5018: " George Moussalem via B4 Relay
2026-08-12 9:48 ` sashiko-bot
2026-08-12 9:34 ` [PATCH v2 10/14] arm64: dts: qcom: ipq5332: Add Super-Speed UNIPHY to USB node George Moussalem via B4 Relay
2026-08-12 9:55 ` sashiko-bot
2026-08-12 10:56 ` Sergey Shtylyov
2026-08-12 9:34 ` [PATCH v2 11/14] arm64: dts: qcom: ipq5018: " George Moussalem via B4 Relay
2026-08-12 9:57 ` sashiko-bot
2026-08-12 10:57 ` Sergey Shtylyov
2026-08-12 9:34 ` [PATCH v2 12/14] arm64: dts: qcom: ipq5332: Enable USB Super-Speed PHY George Moussalem via B4 Relay
2026-08-12 9:54 ` sashiko-bot
2026-08-12 9:34 ` [PATCH v2 13/14] arm64: dts: qcom: ipq5018: " George Moussalem via B4 Relay
2026-08-12 9:57 ` sashiko-bot
2026-08-12 9:34 ` [PATCH v2 14/14] arm64: defconfig: Enable qcom USB UNIPHY driver George Moussalem via B4 Relay
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260812094829.0A0381F000E9@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=george.moussalem@outlook.com \
--cc=linux-phy@lists.infradead.org \
--cc=neil.armstrong@linaro.org \
--cc=olteanv@gmail.com \
--cc=robh@kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox