[PATCH] usb: dwc3 dwc3_exynos_probe() change goto labels to meaningful names

[PATCH] usb: dwc3 dwc3_exynos_probe() change goto labels to meaningful names

[Shuah Khan]

Change goto labels to meaningful names from a series of errNs.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 drivers/usb/dwc3/dwc3-exynos.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
index f7421c2..ea50f74 100644
--- a/drivers/usb/dwc3/dwc3-exynos.c
+++ b/drivers/usb/dwc3/dwc3-exynos.c
@@ -147,53 +147,53 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
 	exynos->vdd33 = devm_regulator_get(dev, "vdd33");
 	if (IS_ERR(exynos->vdd33)) {
 		ret = PTR_ERR(exynos->vdd33);
-		goto err2;
+		goto vdd33_err;
 	}
 	ret = regulator_enable(exynos->vdd33);
 	if (ret) {
 		dev_err(dev, "Failed to enable VDD33 supply\n");
-		goto err2;
+		goto vdd33_err;
 	}
 
 	exynos->vdd10 = devm_regulator_get(dev, "vdd10");
 	if (IS_ERR(exynos->vdd10)) {
 		ret = PTR_ERR(exynos->vdd10);
-		goto err3;
+		goto vdd10_err;
 	}
 	ret = regulator_enable(exynos->vdd10);
 	if (ret) {
 		dev_err(dev, "Failed to enable VDD10 supply\n");
-		goto err3;
+		goto vdd10_err;
 	}
 
 	ret = dwc3_exynos_register_phys(exynos);
 	if (ret) {
 		dev_err(dev, "couldn't register PHYs\n");
-		goto err4;
+		goto phys_err;
 	}
 
 	if (node) {
 		ret = of_platform_populate(node, NULL, NULL, dev);
 		if (ret) {
 			dev_err(dev, "failed to add dwc3 core\n");
-			goto err5;
+			goto populate_err;
 		}
 	} else {
 		dev_err(dev, "no device node, failed to add dwc3 core\n");
 		ret = -ENODEV;
-		goto err5;
+		goto populate_err;
 	}
 
 	return 0;
 
-err5:
+populate_err:
 	platform_device_unregister(exynos->usb2_phy);
 	platform_device_unregister(exynos->usb3_phy);
-err4:
+phys_err:
 	regulator_disable(exynos->vdd10);
-err3:
+vdd10_err:
 	regulator_disable(exynos->vdd33);
-err2:
+vdd33_err:
 	clk_disable_unprepare(exynos->axius_clk);
 axius_clk_err:
 	clk_disable_unprepare(exynos->susp_clk);
-- 
2.7.4

Re: [PATCH] usb: dwc3 dwc3_exynos_probe() change goto labels to meaningful names

[Javier Martinez Canillas]

Hello Shuah,

On 01/11/2017 01:45 PM, Shuah Khan wrote:
> Change goto labels to meaningful names from a series of errNs.
> 
> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
> ---
>  drivers/usb/dwc3/dwc3-exynos.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> index f7421c2..ea50f74 100644
> --- a/drivers/usb/dwc3/dwc3-exynos.c
> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> @@ -147,53 +147,53 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
>  	exynos->vdd33 = devm_regulator_get(dev, "vdd33");
>  	if (IS_ERR(exynos->vdd33)) {
>  		ret = PTR_ERR(exynos->vdd33);
> -		goto err2;
> +		goto vdd33_err;
>  	}

I think it's better to use as labels what will be done in the error path
rather than what failed. IOW, "disable_axius_clk" or "disable_clks" are
more clear than vdd33_err for me, since can give you an idea of what will
happen in the error path.

But I don't have a strong opinion and your naming is an improvement over
the current one, so:

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH] usb: dwc3 dwc3_exynos_probe() change goto labels to meaningful names

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 242 bytes --]


Hi,

Shuah Khan <shuahkh@osg.samsung.com> writes:
> Change goto labels to meaningful names from a series of errNs.
>
> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>

doesn't apply to testing/next, please rebase.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] usb: dwc3 dwc3_exynos_probe() change goto labels to meaningful names

[Shuah Khan]

On 01/16/2017 03:33 AM, Felipe Balbi wrote:
> 
> Hi,
> 
> Shuah Khan <shuahkh@osg.samsung.com> writes:
>> Change goto labels to meaningful names from a series of errNs.
>>
>> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
> 
> doesn't apply to testing/next, please rebase.
> 

Hi Felipe,

This patch is dependent on the

usb: dwc3: exynos fix axius clock error path to do cleanup
https://lkml.org/lkml/2017/1/10/1081

The above made it into usb-linus. It will apply to usb-next
or usb-testing after it gets merged. I can resend it after
the dependent patch makes it into the next rc release.

thanks,
-- Shuah

Re: [PATCH] usb: dwc3 dwc3_exynos_probe() change goto labels to meaningful names

[Shuah Khan]

On 03/17/2017 12:24 AM, Vivek Gautam wrote:
> On Tue, Jan 31, 2017 at 12:55 AM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>> Change goto labels to meaningful names from a series of errNs.
>>
>> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
>> ---
>>
>> Rebased to usb-next
>>
>>  drivers/usb/dwc3/dwc3-exynos.c | 22 +++++++++++-----------
>>  1 file changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
>> index 1515d45..98f74ff 100644
>> --- a/drivers/usb/dwc3/dwc3-exynos.c
>> +++ b/drivers/usb/dwc3/dwc3-exynos.c
>> @@ -147,53 +147,53 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
>>         exynos->vdd33 = devm_regulator_get(dev, "vdd33");
>>         if (IS_ERR(exynos->vdd33)) {
>>                 ret = PTR_ERR(exynos->vdd33);
>> -               goto err2;
>> +               goto vdd33_err;
> 
> While you are changing this in probe, care to change in
> dwc3_exynos_register_phys() as well ?

Yes I can do that.

> 
> [snip]
> 
> Regards
> Vivek

thanks,
-- Shuah

Re: [PATCH] usb: dwc3 dwc3_exynos_probe() change goto labels to meaningful names

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 544 bytes --]

Shuah Khan <shuahkh@osg.samsung.com> writes:

> On 01/30/2017 12:25 PM, Shuah Khan wrote:
>> Change goto labels to meaningful names from a series of errNs.
>> 
>> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
>> ---
>> 
>> Rebased to usb-next
>
> Hi Felipe,
>
> Are you planning to get this in or is there something you are
> waiting in for me to do?

already in testing/next

https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git/commit/?h=testing/next&id=e529082c33a0deaccfc8b28ef1436494ed95365f

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] usb: dwc3 dwc3_exynos_probe() change goto labels to meaningful names

[Vivek Gautam]

On Tue, Jan 31, 2017 at 12:55 AM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> Change goto labels to meaningful names from a series of errNs.
>
> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
> ---
>
> Rebased to usb-next
>
>  drivers/usb/dwc3/dwc3-exynos.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> index 1515d45..98f74ff 100644
> --- a/drivers/usb/dwc3/dwc3-exynos.c
> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> @@ -147,53 +147,53 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
>         exynos->vdd33 = devm_regulator_get(dev, "vdd33");
>         if (IS_ERR(exynos->vdd33)) {
>                 ret = PTR_ERR(exynos->vdd33);
> -               goto err2;
> +               goto vdd33_err;

While you are changing this in probe, care to change in
dwc3_exynos_register_phys() as well ?

[snip]

Regards
Vivek

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] usb: dwc3 dwc3_exynos_probe() change goto labels to meaningful names

[Shuah Khan]

On 01/30/2017 12:25 PM, Shuah Khan wrote:
> Change goto labels to meaningful names from a series of errNs.
> 
> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
> ---
> 
> Rebased to usb-next

Hi Felipe,

Are you planning to get this in or is there something you are
waiting in for me to do?

thanks,
-- Shuah


> 
>  drivers/usb/dwc3/dwc3-exynos.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> index 1515d45..98f74ff 100644
> --- a/drivers/usb/dwc3/dwc3-exynos.c
> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> @@ -147,53 +147,53 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
>  	exynos->vdd33 = devm_regulator_get(dev, "vdd33");
>  	if (IS_ERR(exynos->vdd33)) {
>  		ret = PTR_ERR(exynos->vdd33);
> -		goto err2;
> +		goto vdd33_err;
>  	}
>  	ret = regulator_enable(exynos->vdd33);
>  	if (ret) {
>  		dev_err(dev, "Failed to enable VDD33 supply\n");
> -		goto err2;
> +		goto vdd33_err;
>  	}
>  
>  	exynos->vdd10 = devm_regulator_get(dev, "vdd10");
>  	if (IS_ERR(exynos->vdd10)) {
>  		ret = PTR_ERR(exynos->vdd10);
> -		goto err3;
> +		goto vdd10_err;
>  	}
>  	ret = regulator_enable(exynos->vdd10);
>  	if (ret) {
>  		dev_err(dev, "Failed to enable VDD10 supply\n");
> -		goto err3;
> +		goto vdd10_err;
>  	}
>  
>  	ret = dwc3_exynos_register_phys(exynos);
>  	if (ret) {
>  		dev_err(dev, "couldn't register PHYs\n");
> -		goto err4;
> +		goto phys_err;
>  	}
>  
>  	if (node) {
>  		ret = of_platform_populate(node, NULL, NULL, dev);
>  		if (ret) {
>  			dev_err(dev, "failed to add dwc3 core\n");
> -			goto err5;
> +			goto populate_err;
>  		}
>  	} else {
>  		dev_err(dev, "no device node, failed to add dwc3 core\n");
>  		ret = -ENODEV;
> -		goto err5;
> +		goto populate_err;
>  	}
>  
>  	return 0;
>  
> -err5:
> +populate_err:
>  	platform_device_unregister(exynos->usb2_phy);
>  	platform_device_unregister(exynos->usb3_phy);
> -err4:
> +phys_err:
>  	regulator_disable(exynos->vdd10);
> -err3:
> +vdd10_err:
>  	regulator_disable(exynos->vdd33);
> -err2:
> +vdd33_err:
>  	clk_disable_unprepare(exynos->axius_clk);
>  axius_clk_err:
>  	clk_disable_unprepare(exynos->susp_clk);
> 

[PATCH] usb: dwc3 dwc3_exynos_probe() change goto labels to meaningful names

[Shuah Khan]

Change goto labels to meaningful names from a series of errNs.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---

Rebased to usb-next

 drivers/usb/dwc3/dwc3-exynos.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
index 1515d45..98f74ff 100644
--- a/drivers/usb/dwc3/dwc3-exynos.c
+++ b/drivers/usb/dwc3/dwc3-exynos.c
@@ -147,53 +147,53 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
 	exynos->vdd33 = devm_regulator_get(dev, "vdd33");
 	if (IS_ERR(exynos->vdd33)) {
 		ret = PTR_ERR(exynos->vdd33);
-		goto err2;
+		goto vdd33_err;
 	}
 	ret = regulator_enable(exynos->vdd33);
 	if (ret) {
 		dev_err(dev, "Failed to enable VDD33 supply\n");
-		goto err2;
+		goto vdd33_err;
 	}
 
 	exynos->vdd10 = devm_regulator_get(dev, "vdd10");
 	if (IS_ERR(exynos->vdd10)) {
 		ret = PTR_ERR(exynos->vdd10);
-		goto err3;
+		goto vdd10_err;
 	}
 	ret = regulator_enable(exynos->vdd10);
 	if (ret) {
 		dev_err(dev, "Failed to enable VDD10 supply\n");
-		goto err3;
+		goto vdd10_err;
 	}
 
 	ret = dwc3_exynos_register_phys(exynos);
 	if (ret) {
 		dev_err(dev, "couldn't register PHYs\n");
-		goto err4;
+		goto phys_err;
 	}
 
 	if (node) {
 		ret = of_platform_populate(node, NULL, NULL, dev);
 		if (ret) {
 			dev_err(dev, "failed to add dwc3 core\n");
-			goto err5;
+			goto populate_err;
 		}
 	} else {
 		dev_err(dev, "no device node, failed to add dwc3 core\n");
 		ret = -ENODEV;
-		goto err5;
+		goto populate_err;
 	}
 
 	return 0;
 
-err5:
+populate_err:
 	platform_device_unregister(exynos->usb2_phy);
 	platform_device_unregister(exynos->usb3_phy);
-err4:
+phys_err:
 	regulator_disable(exynos->vdd10);
-err3:
+vdd10_err:
 	regulator_disable(exynos->vdd33);
-err2:
+vdd33_err:
 	clk_disable_unprepare(exynos->axius_clk);
 axius_clk_err:
 	clk_disable_unprepare(exynos->susp_clk);
-- 
2.7.4