[PATCH v4] USB: pxa27x_udc: check return value of clk_enable
From: Zhaoyang Yu
Date: Thu Mar 12 2026 - 04:13:25 EST
clk_enable() may fail according to the API contract.
Previously, udc_enable() ignored its return value.
This patch checks the return value and logs an error
without continuing initialization if clk_enable fails.
Signed-off-by: Zhaoyang Yu <2426767509@xxxxxx>
---
Changes in v4:
- Added missing state rollbacks in pxa_udc_pullup(), pxa_udc_vbus_session(),
and pxa_udc_resume() when udc_enable() fails, per Greg KH's review.
Changes in v3:
- Changed udc_enable() return type from void to int.
- Propagated the error to all caller functions and added proper
error handling/rollback per Greg KH's review.
Changes in v2:
- Fixed a formatting issue by moving the 'int ret' declaration to
the beginning of the function block.
drivers/usb/gadget/udc/pxa27x_udc.c | 66 +++++++++++++++++++++++------
1 file changed, 52 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c b/drivers/usb/gadget/udc/pxa27x_udc.c
index 897f53601b5b..348af4d29c4d 100644
--- a/drivers/usb/gadget/udc/pxa27x_udc.c
+++ b/drivers/usb/gadget/udc/pxa27x_udc.c
@@ -1462,7 +1462,7 @@ static int pxa_udc_wakeup(struct usb_gadget *_gadget)
return 0;
}
-static void udc_enable(struct pxa_udc *udc);
+static int udc_enable(struct pxa_udc *udc);
static void udc_disable(struct pxa_udc *udc);
/**
@@ -1519,14 +1519,20 @@ static int should_disable_udc(struct pxa_udc *udc)
static int pxa_udc_pullup(struct usb_gadget *_gadget, int is_active)
{
struct pxa_udc *udc = to_gadget_udc(_gadget);
+ int ret;
if (!udc->gpiod && !udc->udc_command)
return -EOPNOTSUPP;
dplus_pullup(udc, is_active);
- if (should_enable_udc(udc))
- udc_enable(udc);
+ if (should_enable_udc(udc)) {
+ ret = udc_enable(udc);
+ if (ret) {
+ dplus_pullup(udc, !is_active);
+ return ret;
+ }
+ }
if (should_disable_udc(udc))
udc_disable(udc);
return 0;
@@ -1545,10 +1551,16 @@ static int pxa_udc_pullup(struct usb_gadget *_gadget, int is_active)
static int pxa_udc_vbus_session(struct usb_gadget *_gadget, int is_active)
{
struct pxa_udc *udc = to_gadget_udc(_gadget);
+ int ret;
udc->vbus_sensed = is_active;
- if (should_enable_udc(udc))
- udc_enable(udc);
+ if (should_enable_udc(udc)) {
+ ret = udc_enable(udc);
+ if (ret) {
+ udc->vbus_sensed = !is_active;
+ return ret;
+ }
+ }
if (should_disable_udc(udc))
udc_disable(udc);
@@ -1691,12 +1703,18 @@ static void udc_init_data(struct pxa_udc *dev)
* Enables the udc device : enables clocks, udc interrupts, control endpoint
* interrupts, sets usb as UDC client and setups endpoints.
*/
-static void udc_enable(struct pxa_udc *udc)
+static int udc_enable(struct pxa_udc *udc)
{
+ int ret;
+
if (udc->enabled)
- return;
+ return 0;
- clk_enable(udc->clk);
+ ret = clk_enable(udc->clk);
+ if (ret) {
+ dev_err(udc->dev, "clk_enable failed: %d\n", ret);
+ return ret;
+ }
udc_writel(udc, UDCICR0, 0);
udc_writel(udc, UDCICR1, 0);
udc_clear_mask_UDCCR(udc, UDCCR_UDE);
@@ -1726,6 +1744,8 @@ static void udc_enable(struct pxa_udc *udc)
pio_irq_enable(&udc->pxa_ep[0]);
udc->enabled = 1;
+
+ return 0;
}
/**
@@ -1761,10 +1781,16 @@ static int pxa27x_udc_start(struct usb_gadget *g,
}
}
- if (should_enable_udc(udc))
- udc_enable(udc);
+ if (should_enable_udc(udc)) {
+ retval = udc_enable(udc);
+ if (retval)
+ goto fail_enable;
+ }
return 0;
+fail_enable:
+ if (!IS_ERR_OR_NULL(udc->transceiver))
+ otg_set_peripheral(udc->transceiver->otg, NULL);
fail:
udc->driver = NULL;
return retval;
@@ -2430,10 +2456,16 @@ static int pxa_udc_probe(struct platform_device *pdev)
goto err_add_gadget;
pxa_init_debugfs(udc);
- if (should_enable_udc(udc))
- udc_enable(udc);
+ if (should_enable_udc(udc)) {
+ retval = udc_enable(udc);
+ if (retval)
+ goto err_enable;
+ }
return 0;
+err_enable:
+ usb_del_gadget_udc(&udc->gadget);
+ pxa_cleanup_debugfs(udc);
err_add_gadget:
if (!IS_ERR_OR_NULL(udc->transceiver))
usb_unregister_notifier(udc->transceiver, &pxa27x_udc_phy);
@@ -2509,13 +2541,19 @@ static int pxa_udc_resume(struct platform_device *_dev)
{
struct pxa_udc *udc = platform_get_drvdata(_dev);
struct pxa_ep *ep;
+ int ret;
ep = &udc->pxa_ep[0];
udc_ep_writel(ep, UDCCSR, udc->udccsr0 & (UDCCSR0_FST | UDCCSR0_DME));
dplus_pullup(udc, udc->pullup_resume);
- if (should_enable_udc(udc))
- udc_enable(udc);
+ if (should_enable_udc(udc)) {
+ ret = udc_enable(udc);
+ if (ret) {
+ dplus_pullup(udc, !udc->pullup_resume);
+ return ret;
+ }
+ }
/*
* We do not handle OTG yet.
*
--
2.34.1