[PATCH v2 2/3] usb: dwc3: xilinx: re-assert resets on ZynqMP init error paths
Pandey, Radhey Shyam
radheys at amd.com
Sat Sep 26 05:32:42 PDT 2026
On 9/23/2026 2:43 PM, Philipp Zabel wrote:
> On Di, 2026-09-22 at 23:51 +0530, Radhey Shyam Pandey wrote:
>> If reset deassert or PHY setup fails partway through
>> dwc3_xlnx_init_zynqmp(), re-assert any resets that were already
>> released before unwinding the PHY. Use fall-through error labels so
>> unwind matches how far init progressed, for both USB2 and USB3 paths.
>>
>> Save the ZynqMP reset handles in driver private data so later probe
>> teardown can re-assert released resets.
>>
>> Fixes: 84770f028fab ("usb: dwc3: Add driver for Xilinx platforms")
>> Cc: stable at vger.kernel.org
>> Assisted-by: Claude:claude-opus-5
>> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey at amd.com>
>> ---
>> Changes in v2:
>> - Split out of the combined five patch series; see patch 1.
>> - Reordered ahead of the platform-data cleanups.
>> - Added Cc: stable.
>> - No functional change to the patch itself.
>>
>> drivers/usb/dwc3/dwc3-xilinx.c | 49 +++++++++++++++++++++-------------
>> 1 file changed, 30 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c
>> index 8c63e02575f1..d18d3e364381 100644
>> --- a/drivers/usb/dwc3/dwc3-xilinx.c
>> +++ b/drivers/usb/dwc3/dwc3-xilinx.c
>> @@ -48,6 +48,10 @@ struct dwc3_xlnx {
>> void __iomem *regs;
>> int (*pltfm_init)(struct dwc3_xlnx *data);
>> struct phy *usb3_phy;
>> + struct reset_control *usb_crst;
>> + struct reset_control *usb_hibrst;
>> + struct reset_control *usb_apbrst;
>> + bool usb_resets_released;
>> };
>>
>> static void dwc3_xlnx_mask_phy_rst(struct dwc3_xlnx *priv_data, bool mask)
>> @@ -112,7 +116,6 @@ static int dwc3_xlnx_init_versal(struct dwc3_xlnx *priv_data)
>> static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
>> {
>> struct device *dev = priv_data->dev;
>> - struct reset_control *crst, *hibrst, *apbrst;
>> struct gpio_desc *reset_gpio;
>> int ret = 0;
>>
>> @@ -124,25 +127,25 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
>> goto err;
>> }
>>
>> - crst = devm_reset_control_get_exclusive(dev, "usb_crst");
>> - if (IS_ERR(crst)) {
>> - ret = PTR_ERR(crst);
>> + priv_data->usb_crst = devm_reset_control_get_exclusive(dev, "usb_crst");
>> + if (IS_ERR(priv_data->usb_crst)) {
>> + ret = PTR_ERR(priv_data->usb_crst);
>> dev_err_probe(dev, ret,
>> "failed to get core reset signal\n");
>> goto err;
>> }
>>
>> - hibrst = devm_reset_control_get_exclusive(dev, "usb_hibrst");
>> - if (IS_ERR(hibrst)) {
>> - ret = PTR_ERR(hibrst);
>> + priv_data->usb_hibrst = devm_reset_control_get_exclusive(dev, "usb_hibrst");
>> + if (IS_ERR(priv_data->usb_hibrst)) {
>> + ret = PTR_ERR(priv_data->usb_hibrst);
>> dev_err_probe(dev, ret,
>> "failed to get hibernation reset signal\n");
>> goto err;
>> }
>>
>> - apbrst = devm_reset_control_get_exclusive(dev, "usb_apbrst");
>> - if (IS_ERR(apbrst)) {
>> - ret = PTR_ERR(apbrst);
>> + priv_data->usb_apbrst = devm_reset_control_get_exclusive(dev, "usb_apbrst");
>> + if (IS_ERR(priv_data->usb_apbrst)) {
>> + ret = PTR_ERR(priv_data->usb_apbrst);
>> dev_err_probe(dev, ret,
>> "failed to get APB reset signal\n");
>> goto err;
>> @@ -156,19 +159,19 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
>> * absent.
>> */
>> if (priv_data->usb3_phy) {
>> - ret = reset_control_assert(crst);
>> + ret = reset_control_assert(priv_data->usb_crst);
>> if (ret < 0) {
>> dev_err(dev, "Failed to assert core reset\n");
>> goto err;
>> }
>>
>> - ret = reset_control_assert(hibrst);
>> + ret = reset_control_assert(priv_data->usb_hibrst);
>> if (ret < 0) {
>> dev_err(dev, "Failed to assert hibernation reset\n");
>> goto err;
>> }
>>
>> - ret = reset_control_assert(apbrst);
>> + ret = reset_control_assert(priv_data->usb_apbrst);
>> if (ret < 0) {
>> dev_err(dev, "Failed to assert APB reset\n");
>> goto err;
>> @@ -179,7 +182,7 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
>> if (ret < 0)
>> goto err;
>>
>> - ret = reset_control_deassert(apbrst);
>> + ret = reset_control_deassert(priv_data->usb_apbrst);
>> if (ret < 0) {
>> dev_err(dev, "Failed to release APB reset\n");
>> goto err_phy_exit;
>> @@ -195,21 +198,21 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
>> writel(PIPE_CLK_DESELECT, priv_data->regs + XLNX_USB_FPD_PIPE_CLK);
>> }
>>
>> - ret = reset_control_deassert(crst);
>> + ret = reset_control_deassert(priv_data->usb_crst);
>> if (ret < 0) {
>> dev_err(dev, "Failed to release core reset\n");
>> - goto err_phy_exit;
>> + goto err_apbrst_assert;
>> }
>>
>> - ret = reset_control_deassert(hibrst);
>> + ret = reset_control_deassert(priv_data->usb_hibrst);
>> if (ret < 0) {
>> dev_err(dev, "Failed to release hibernation reset\n");
>> - goto err_phy_exit;
>> + goto err_crst_assert;
>> }
>>
>> ret = phy_power_on(priv_data->usb3_phy);
>> if (ret < 0)
>> - goto err_phy_exit;
>> + goto err_hibrst_assert;
>>
>> /* ulpi reset via gpio-modepin or gpio-framework driver */
>> reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
>
> Are the resets asserted if this fails?
YEs, if reset_gpio fails resets are asserted.
>
> Same question about if probe fails after pltfm_init() succeeded.
Valid point. In this patch, only the init_zynqmp() error handling is
fixed; probe failure handling is addressed in the subsequent patch.
>
>> @@ -226,10 +229,18 @@ static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
>>
>> dwc3_xlnx_set_coherency(priv_data, XLNX_USB_TRAFFIC_ROUTE_CONFIG);
>>
>> + priv_data->usb_resets_released = true;
>> +
>> return 0;
>>
>> err_phy_power_off:
>> phy_power_off(priv_data->usb3_phy);
>> +err_hibrst_assert:
>> + reset_control_assert(priv_data->usb_hibrst);
>> +err_crst_assert:
>> + reset_control_assert(priv_data->usb_crst);
>> +err_apbrst_assert:
>> + reset_control_assert(priv_data->usb_apbrst);
>> err_phy_exit:
>> phy_exit(priv_data->usb3_phy);
>> err:
>
> Rather than storing usb_resets_released state, and then having
> conditional teardown in the next patch, this could be handled via
> devm_add_action_or_reset.
I agree on it, it simplifies the error handling and will spin v3.
Thanks,
Radhey
More information about the linux-arm-kernel
mailing list