* [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).