Use IS_ERR() and PTR_ERR() instead of PTR_ERR_OR_ZERO() to
simplify code, avoid redundant judgements.
Signed-off-by: Zhang Shengju <[email protected]>
Signed-off-by: Tang Bin <[email protected]>
---
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
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 <[email protected]>
> Signed-off-by: Tang Bin <[email protected]>
> ---
> 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
Tang Bin <[email protected]> writes:
> Use IS_ERR() and PTR_ERR() instead of PTR_ERR_OR_ZERO() to
> simplify code, avoid redundant judgements.
>
> Signed-off-by: Zhang Shengju <[email protected]>
> Signed-off-by: Tang Bin <[email protected]>
Applied for next merge window. Make sure to get this driver out of
drivers/usb/phy and moved into drivers/phy ASAP.
--
balbi
On Thu, Sep 24, 2020 at 10:26:15AM +0300, Felipe Balbi wrote:
> Tang Bin <[email protected]> writes:
>
> > Use IS_ERR() and PTR_ERR() instead of PTR_ERR_OR_ZERO() to
> > simplify code, avoid redundant judgements.
> >
> > Signed-off-by: Zhang Shengju <[email protected]>
> > Signed-off-by: Tang Bin <[email protected]>
>
> 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/[email protected]/#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
Thierry Reding <[email protected]> writes:
> On Thu, Sep 24, 2020 at 10:26:15AM +0300, Felipe Balbi wrote:
>> Tang Bin <[email protected]> writes:
>>
>> > Use IS_ERR() and PTR_ERR() instead of PTR_ERR_OR_ZERO() to
>> > simplify code, avoid redundant judgements.
>> >
>> > Signed-off-by: Zhang Shengju <[email protected]>
>> > Signed-off-by: Tang Bin <[email protected]>
>>
>> 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/[email protected]/#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
Hi all:
?? 2020/9/24 18:37, Felipe Balbi ะด??:
> Thierry Reding <[email protected]> writes:
>
>> On Thu, Sep 24, 2020 at 10:26:15AM +0300, Felipe Balbi wrote:
>>> Tang Bin <[email protected]> writes:
>>>
>>>> Use IS_ERR() and PTR_ERR() instead of PTR_ERR_OR_ZERO() to
>>>> simplify code, avoid redundant judgements.
>>>>
>>>> Signed-off-by: Zhang Shengju <[email protected]>
>>>> Signed-off-by: Tang Bin <[email protected]>
>>> 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/[email protected]/#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
>