linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] usb: phy: tegra: Use IS_ERR() to check and simplify code
@ 2020-09-10 11:56 Tang Bin
  2020-09-11  8:29 ` Sergei Shtylyov
  2020-09-24  7:26 ` Felipe Balbi
  0 siblings, 2 replies; 6+ messages in thread
From: Tang Bin @ 2020-09-10 11:56 UTC (permalink / raw)
  To: balbi, gregkh, thierry.reding, jonathanh
  Cc: linux-usb, linux-tegra, linux-kernel, Tang Bin, Zhang Shengju

Use IS_ERR() and PTR_ERR() instead of PTR_ERR_OR_ZERO() to
simplify code, avoid redundant judgements.

Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
---
 drivers/usb/phy/phy-tegra-usb.c | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
index 6153cc35a..3b901429d 100644
--- a/drivers/usb/phy/phy-tegra-usb.c
+++ b/drivers/usb/phy/phy-tegra-usb.c
@@ -1121,10 +1121,9 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
 		return PTR_ERR(tegra_phy->vbus);
 
 	tegra_phy->pll_u = devm_clk_get(&pdev->dev, "pll_u");
-	err = PTR_ERR_OR_ZERO(tegra_phy->pll_u);
-	if (err) {
+	if (IS_ERR(tegra_phy->pll_u)) {
 		dev_err(&pdev->dev, "Failed to get pll_u clock: %d\n", err);
-		return err;
+		return PTR_ERR(tegra_phy->pll_u);
 	}
 
 	phy_type = of_usb_get_phy_mode(np);
@@ -1135,20 +1134,18 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
 			return err;
 
 		tegra_phy->pad_clk = devm_clk_get(&pdev->dev, "utmi-pads");
-		err = PTR_ERR_OR_ZERO(tegra_phy->pad_clk);
-		if (err) {
+		if (IS_ERR(tegra_phy->pad_clk)) {
 			dev_err(&pdev->dev,
 				"Failed to get UTMIP pad clock: %d\n", err);
-			return err;
+			return PTR_ERR(tegra_phy->pad_clk);
 		}
 
 		reset = devm_reset_control_get_optional_shared(&pdev->dev,
 							       "utmi-pads");
-		err = PTR_ERR_OR_ZERO(reset);
-		if (err) {
+		if (IS_ERR(reset)) {
 			dev_err(&pdev->dev,
 				"Failed to get UTMI-pads reset: %d\n", err);
-			return err;
+			return PTR_ERR(reset);
 		}
 		tegra_phy->pad_rst = reset;
 		break;
@@ -1157,22 +1154,20 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
 		tegra_phy->is_ulpi_phy = true;
 
 		tegra_phy->clk = devm_clk_get(&pdev->dev, "ulpi-link");
-		err = PTR_ERR_OR_ZERO(tegra_phy->clk);
-		if (err) {
+		if (IS_ERR(tegra_phy->clk)) {
 			dev_err(&pdev->dev,
 				"Failed to get ULPI clock: %d\n", err);
-			return err;
+			return PTR_ERR(tegra_phy->clk);
 		}
 
 		gpiod = devm_gpiod_get_from_of_node(&pdev->dev, np,
 						    "nvidia,phy-reset-gpio",
 						    0, GPIOD_OUT_HIGH,
 						    "ulpi_phy_reset_b");
-		err = PTR_ERR_OR_ZERO(gpiod);
-		if (err) {
+		if (IS_ERR(gpiod)) {
 			dev_err(&pdev->dev,
 				"Request failed for reset GPIO: %d\n", err);
-			return err;
+			return PTR_ERR(gpiod);
 		}
 		tegra_phy->reset_gpio = gpiod;
 
-- 
2.20.1.windows.1




^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb: phy: tegra: Use IS_ERR() to check and simplify code
  2020-09-10 11:56 [PATCH] usb: phy: tegra: Use IS_ERR() to check and simplify code Tang Bin
@ 2020-09-11  8:29 ` Sergei Shtylyov
  2020-09-24  7:26 ` Felipe Balbi
  1 sibling, 0 replies; 6+ messages in thread
From: Sergei Shtylyov @ 2020-09-11  8:29 UTC (permalink / raw)
  To: Tang Bin, balbi, gregkh, thierry.reding, jonathanh
  Cc: linux-usb, linux-tegra, linux-kernel, Zhang Shengju

Hello!

On 10.09.2020 14:56, Tang Bin wrote:

> Use IS_ERR() and PTR_ERR() instead of PTR_ERR_OR_ZERO() to
> simplify code, avoid redundant judgements.
> 
> Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
> Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
> ---
>   drivers/usb/phy/phy-tegra-usb.c | 25 ++++++++++---------------
>   1 file changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
> index 6153cc35a..3b901429d 100644
> --- a/drivers/usb/phy/phy-tegra-usb.c
> +++ b/drivers/usb/phy/phy-tegra-usb.c
> @@ -1121,10 +1121,9 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
>   		return PTR_ERR(tegra_phy->vbus);
>   
>   	tegra_phy->pll_u = devm_clk_get(&pdev->dev, "pll_u");
> -	err = PTR_ERR_OR_ZERO(tegra_phy->pll_u);
> -	if (err) {
> +	if (IS_ERR(tegra_phy->pll_u)) {
>   		dev_err(&pdev->dev, "Failed to get pll_u clock: %d\n", err);

    'err' should be changed here too...

> -		return err;
> +		return PTR_ERR(tegra_phy->pll_u);
>   	}
>   
>   	phy_type = of_usb_get_phy_mode(np);
> @@ -1135,20 +1134,18 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
>   			return err;
>   
>   		tegra_phy->pad_clk = devm_clk_get(&pdev->dev, "utmi-pads");
> -		err = PTR_ERR_OR_ZERO(tegra_phy->pad_clk);
> -		if (err) {
> +		if (IS_ERR(tegra_phy->pad_clk)) {
>   			dev_err(&pdev->dev,
>   				"Failed to get UTMIP pad clock: %d\n", err);

    Same here.

> -			return err;
> +			return PTR_ERR(tegra_phy->pad_clk);
>   		}
>   
>   		reset = devm_reset_control_get_optional_shared(&pdev->dev,
>   							       "utmi-pads");
> -		err = PTR_ERR_OR_ZERO(reset);
> -		if (err) {
> +		if (IS_ERR(reset)) {
>   			dev_err(&pdev->dev,
>   				"Failed to get UTMI-pads reset: %d\n", err);

    And here.

> -			return err;
> +			return PTR_ERR(reset);
>   		}
>   		tegra_phy->pad_rst = reset;
>   		break;
> @@ -1157,22 +1154,20 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
>   		tegra_phy->is_ulpi_phy = true;
>   
>   		tegra_phy->clk = devm_clk_get(&pdev->dev, "ulpi-link");
> -		err = PTR_ERR_OR_ZERO(tegra_phy->clk);
> -		if (err) {
> +		if (IS_ERR(tegra_phy->clk)) {
>   			dev_err(&pdev->dev,
>   				"Failed to get ULPI clock: %d\n", err);

    And here.

> -			return err;
> +			return PTR_ERR(tegra_phy->clk);
>   		}
>   
>   		gpiod = devm_gpiod_get_from_of_node(&pdev->dev, np,
>   						    "nvidia,phy-reset-gpio",
>   						    0, GPIOD_OUT_HIGH,
>   						    "ulpi_phy_reset_b");
> -		err = PTR_ERR_OR_ZERO(gpiod);
> -		if (err) {
> +		if (IS_ERR(gpiod)) {
>   			dev_err(&pdev->dev,
>   				"Request failed for reset GPIO: %d\n", err);

    And here.

> -			return err;
> +			return PTR_ERR(gpiod);
>   		}
>   		tegra_phy->reset_gpio = gpiod;
>   

    Overall, this patch is broken and not even worth redoing -- the current 
code seems good...

MBR, Sergei

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb: phy: tegra: Use IS_ERR() to check and simplify code
  2020-09-10 11:56 [PATCH] usb: phy: tegra: Use IS_ERR() to check and simplify code Tang Bin
  2020-09-11  8:29 ` Sergei Shtylyov
@ 2020-09-24  7:26 ` Felipe Balbi
  2020-09-24 10:21   ` Thierry Reding
  1 sibling, 1 reply; 6+ messages in thread
From: Felipe Balbi @ 2020-09-24  7:26 UTC (permalink / raw)
  To: Tang Bin, gregkh, thierry.reding, jonathanh
  Cc: linux-usb, linux-tegra, linux-kernel, Tang Bin, Zhang Shengju

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

Tang Bin <tangbin@cmss.chinamobile.com> writes:

> Use IS_ERR() and PTR_ERR() instead of PTR_ERR_OR_ZERO() to
> simplify code, avoid redundant judgements.
>
> Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
> Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>

Applied for next merge window. Make sure to get this driver out of
drivers/usb/phy and moved into drivers/phy ASAP.

-- 
balbi

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb: phy: tegra: Use IS_ERR() to check and simplify code
  2020-09-24  7:26 ` Felipe Balbi
@ 2020-09-24 10:21   ` Thierry Reding
  2020-09-24 10:37     ` Felipe Balbi
  0 siblings, 1 reply; 6+ messages in thread
From: Thierry Reding @ 2020-09-24 10:21 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Tang Bin, gregkh, jonathanh, linux-usb, linux-tegra,
	linux-kernel, Zhang Shengju

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

On Thu, Sep 24, 2020 at 10:26:15AM +0300, Felipe Balbi wrote:
> Tang Bin <tangbin@cmss.chinamobile.com> writes:
> 
> > Use IS_ERR() and PTR_ERR() instead of PTR_ERR_OR_ZERO() to
> > simplify code, avoid redundant judgements.
> >
> > Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
> > Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
> 
> Applied for next merge window. Make sure to get this driver out of
> drivers/usb/phy and moved into drivers/phy ASAP.

Sergei had commented on this patch with valid concerns, see here in case
you don't have his reply in your inbox:

    http://patchwork.ozlabs.org/project/linux-tegra/patch/20200910115607.11392-1-tangbin@cmss.chinamobile.com/#2526208

I agree with those concerns. This patch is broken because it will output
the wrong error code on failure. I don't fully agree with Sergei's point
that this patch isn't worth redoing. I do like the idiomatic error
handling better, but I think we shouldn't be breaking the error messages
like this.

Thierry

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb: phy: tegra: Use IS_ERR() to check and simplify code
  2020-09-24 10:21   ` Thierry Reding
@ 2020-09-24 10:37     ` Felipe Balbi
  2020-09-28  8:47       ` Tang Bin
  0 siblings, 1 reply; 6+ messages in thread
From: Felipe Balbi @ 2020-09-24 10:37 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Tang Bin, gregkh, jonathanh, linux-usb, linux-tegra,
	linux-kernel, Zhang Shengju

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

Thierry Reding <thierry.reding@gmail.com> writes:

> On Thu, Sep 24, 2020 at 10:26:15AM +0300, Felipe Balbi wrote:
>> Tang Bin <tangbin@cmss.chinamobile.com> writes:
>> 
>> > Use IS_ERR() and PTR_ERR() instead of PTR_ERR_OR_ZERO() to
>> > simplify code, avoid redundant judgements.
>> >
>> > Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
>> > Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
>> 
>> Applied for next merge window. Make sure to get this driver out of
>> drivers/usb/phy and moved into drivers/phy ASAP.
>
> Sergei had commented on this patch with valid concerns, see here in case
> you don't have his reply in your inbox:
>
>     http://patchwork.ozlabs.org/project/linux-tegra/patch/20200910115607.11392-1-tangbin@cmss.chinamobile.com/#2526208
>
> I agree with those concerns. This patch is broken because it will output
> the wrong error code on failure. I don't fully agree with Sergei's point
> that this patch isn't worth redoing. I do like the idiomatic error
> handling better, but I think we shouldn't be breaking the error messages
> like this.

Sure thing, dropped for now.

-- 
balbi

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb: phy: tegra: Use IS_ERR() to check and simplify code
  2020-09-24 10:37     ` Felipe Balbi
@ 2020-09-28  8:47       ` Tang Bin
  0 siblings, 0 replies; 6+ messages in thread
From: Tang Bin @ 2020-09-28  8:47 UTC (permalink / raw)
  To: Felipe Balbi, Thierry Reding
  Cc: gregkh, jonathanh, linux-usb, linux-tegra, linux-kernel

Hi all:

在 2020/9/24 18:37, Felipe Balbi 写道:
> Thierry Reding <thierry.reding@gmail.com> writes:
>
>> On Thu, Sep 24, 2020 at 10:26:15AM +0300, Felipe Balbi wrote:
>>> Tang Bin <tangbin@cmss.chinamobile.com> writes:
>>>
>>>> Use IS_ERR() and PTR_ERR() instead of PTR_ERR_OR_ZERO() to
>>>> simplify code, avoid redundant judgements.
>>>>
>>>> Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
>>>> Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
>>> Applied for next merge window. Make sure to get this driver out of
>>> drivers/usb/phy and moved into drivers/phy ASAP.
>> Sergei had commented on this patch with valid concerns, see here in case
>> you don't have his reply in your inbox:
>>
>>      http://patchwork.ozlabs.org/project/linux-tegra/patch/20200910115607.11392-1-tangbin@cmss.chinamobile.com/#2526208
>>
>> I agree with those concerns. This patch is broken because it will output
>> the wrong error code on failure. I don't fully agree with Sergei's point
>> that this patch isn't worth redoing. I do like the idiomatic error
>> handling better, but I think we shouldn't be breaking the error messages
>> like this.
> Sure thing, dropped for now.

Sorry, it's my fault.

Thanks

Tang Bin

>



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-09-28  8:47 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-10 11:56 [PATCH] usb: phy: tegra: Use IS_ERR() to check and simplify code Tang Bin
2020-09-11  8:29 ` Sergei Shtylyov
2020-09-24  7:26 ` Felipe Balbi
2020-09-24 10:21   ` Thierry Reding
2020-09-24 10:37     ` Felipe Balbi
2020-09-28  8:47       ` Tang Bin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).