+CC: Felipe Balbi
On 11/24/2014 04:48 AM, Dan Carpenter wrote:
> On Mon, Nov 24, 2014 at 01:46:56PM +0300, Dan Carpenter wrote:
>> Hello Dinh Nguyen,
>>
>> The patch 8d736d8a9c44: "usb: dwc2: gadget: Do not fail probe if
>> there isn't a clock node" from Nov 11, 2014, leads to the following
>> static checker warning:
>>
>> drivers/usb/dwc2/gadget.c:3436 dwc2_gadget_init()
>> warn: passing zero to 'PTR_ERR'
>>
>> drivers/usb/dwc2/gadget.c
>> 3432 hsotg->clk = devm_clk_get(dev, "otg");
>> 3433 if (IS_ERR(hsotg->clk)) {
>> 3434 hsotg->clk = NULL;
>>
>> You need to preserve the error code. NULL means zero means success.
>>
>
> Oh, wait. You are returning success deliberately? Just "return 0;"
> in that case instead of obfuscating it this way. But shouldn't we
> continue with the rest of the function anyway? This patch is confusing
> to me.
Yes, I believe that we can remove the return, and that was Felipe's comment.
>
>> 3435 dev_err(dev, "cannot get otg clock\n");
>
> Do we need to print this error if it's a success path?
>
A perhaps a follow up patch to something like this?
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3451,8 +3451,7 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int
irq)
hsotg->clk = devm_clk_get(dev, "otg");
if (IS_ERR(hsotg->clk)) {
hsotg->clk = NULL;
- dev_err(dev, "cannot get otg clock\n");
- return PTR_ERR(hsotg->clk);
+ dev_dbg(dev, "cannot get otg clock\n");
}
hsotg->gadget.max_speed = USB_SPEED_HIGH;
@@ -3461,7 +3460,12 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg,
int irq)
/* reset the system */
- clk_prepare_enable(hsotg->clk);
+ ret = clk_prepare_enable(hsotg->clk);
+ if (ret) {
+ dev_err(dev, "failed to enable otg clk\n");
+ goto err_clk;
+ }
+
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
