Re: [patch-2.6.34-rc2+ 11/11] musb_core: fix musb_init_controller() error cleanup path

[Sergei Shtylyov <sshtylyov@mvista.com>]

Hello.

Felipe Balbi wrote:

> From: Sergei Shtylyov <sshtylyov@ru.mvista.com>
>
> This function forgets to call usb_remove_hcd() or musb_gadget_cleanup() iff
> sysfs_create_group() fails.
>
> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
>
> [ felipe.balbi@nokia.com : review the entire error path
> 	not only when we fail hcd or gadget ]
>
> Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
> ---
>  drivers/usb/musb/musb_core.c |   50 +++++++++++++++++++++++++++--------------
>  1 files changed, 33 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 90ac17b..13f36bf 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
[...]


> @@ -2047,7 +2052,7 @@ bad_config:
>  
>  	}
>  	if (status < 0)
> -		goto fail2;
> +		goto fail4;
>   

   No, the label should be 'fail3' here. In this case usb_add_hcd() or 
musb_gadget_setup() have failed, so we shouldn't undo them...

>  	status = musb_init_debugfs(musb);
>  	if (status < 0)
>   

   Is there also a 'goto' that needs to be fixed up?

> @@ -2056,7 +2061,7 @@ bad_config:
>  #ifdef CONFIG_SYSFS
>  	status = sysfs_create_group(&musb->controller->kobj, &musb_attr_group);
>  	if (status)
> -		goto fail2;
> +		goto fail4;
>  #endif
>  
>  	dev_info(dev, "USB %s mode controller at %p using %s, IRQ %d\n",
> @@ -2073,17 +2078,28 @@ bad_config:
>  
>  	return 0;
>  
> -fail2:
> +fail4:
> +	if (!is_otg_enabled(musb) && is_host_enabled(musb))
> +		usb_remove_hcd(musb_to_hcd(musb));
> +	else
> +		musb_gadget_cleanup(musb);
> +	device_init_wakeup(dev, 0);
> +
> +fail3:
>  	musb_platform_exit(musb);
> -fail:
> -	dev_err(musb->controller,
> -		"musb_init_controller failed with status %d\n", status);
>  
> +fail2:
>  	if (musb->clock)
>  		clk_put(musb->clock);
> -	device_init_wakeup(dev, 0);
> +
> +fail1:
> +	dev_err(musb->controller,
> +		"musb_init_controller failed with status %d\n", status);
> +
>  	musb_free(musb);
>  
> +fail0:
> +
>  	return status;
>  
>  }
>   

WBR, Sergei