All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] PCI: tegra: Use PTR_ERR_OR_ZERO
@ 2020-11-16 16:54 ` Sudip Mukherjee
  0 siblings, 0 replies; 17+ messages in thread
From: Sudip Mukherjee @ 2020-11-16 16:54 UTC (permalink / raw)
  To: Thierry Reding, Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas,
	Jonathan Hunter
  Cc: linux-kernel, linux-safety, linux-tegra, linux-pci, Sudip Mukherjee

Coccinelle suggested using PTR_ERR_OR_ZERO() and looking at the code,
we can use PTR_ERR_OR_ZERO() instead of checking IS_ERR() and then
doing 'return 0'.

Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
---
 drivers/pci/controller/pci-tegra.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
index 8fcabed7c6a6..4c52b2d58645 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -1308,10 +1308,8 @@ static int tegra_pcie_resets_get(struct tegra_pcie *pcie)
 		return PTR_ERR(pcie->afi_rst);
 
 	pcie->pcie_xrst = devm_reset_control_get_exclusive(dev, "pcie_x");
-	if (IS_ERR(pcie->pcie_xrst))
-		return PTR_ERR(pcie->pcie_xrst);
 
-	return 0;
+	return PTR_ERR_OR_ZERO(pcie->pcie_xrst);
 }
 
 static int tegra_pcie_phys_get_legacy(struct tegra_pcie *pcie)
-- 
2.11.0


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

* [linux-safety] [PATCH] PCI: tegra: Use PTR_ERR_OR_ZERO
@ 2020-11-16 16:54 ` Sudip Mukherjee
  0 siblings, 0 replies; 17+ messages in thread
From: Sudip Mukherjee @ 2020-11-16 16:54 UTC (permalink / raw)
  To: Thierry Reding, Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas,
	Jonathan Hunter
  Cc: linux-kernel, linux-safety, linux-tegra, linux-pci, Sudip Mukherjee

Coccinelle suggested using PTR_ERR_OR_ZERO() and looking at the code,
we can use PTR_ERR_OR_ZERO() instead of checking IS_ERR() and then
doing 'return 0'.

Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
---
 drivers/pci/controller/pci-tegra.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
index 8fcabed7c6a6..4c52b2d58645 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -1308,10 +1308,8 @@ static int tegra_pcie_resets_get(struct tegra_pcie *pcie)
 		return PTR_ERR(pcie->afi_rst);
 
 	pcie->pcie_xrst = devm_reset_control_get_exclusive(dev, "pcie_x");
-	if (IS_ERR(pcie->pcie_xrst))
-		return PTR_ERR(pcie->pcie_xrst);
 
-	return 0;
+	return PTR_ERR_OR_ZERO(pcie->pcie_xrst);
 }
 
 static int tegra_pcie_phys_get_legacy(struct tegra_pcie *pcie)
-- 
2.11.0



-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#170): https://lists.elisa.tech/g/linux-safety/message/170
Mute This Topic: https://lists.elisa.tech/mt/78296400/5278000
Group Owner: linux-safety+owner@lists.elisa.tech
Unsubscribe: https://lists.elisa.tech/g/linux-safety/unsub [linux-safety@archiver.kernel.org]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [PATCH] PCI: tegra: Use PTR_ERR_OR_ZERO
  2020-11-16 16:54 ` [linux-safety] " Sudip Mukherjee
@ 2020-11-16 17:01   ` Thierry Reding
  -1 siblings, 0 replies; 17+ messages in thread
From: Thierry Reding @ 2020-11-16 17:01 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas, Jonathan Hunter,
	linux-kernel, linux-safety, linux-tegra, linux-pci

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

On Mon, Nov 16, 2020 at 04:54:07PM +0000, Sudip Mukherjee wrote:
> Coccinelle suggested using PTR_ERR_OR_ZERO() and looking at the code,
> we can use PTR_ERR_OR_ZERO() instead of checking IS_ERR() and then
> doing 'return 0'.
> 
> Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> ---
>  drivers/pci/controller/pci-tegra.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

This has been proposed multiple times in the past and Bjorn and I have
agreed every time that this is not an improvement, so sorry, but NAK.

Thierry

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

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

* Re: [linux-safety] [PATCH] PCI: tegra: Use PTR_ERR_OR_ZERO
@ 2020-11-16 17:01   ` Thierry Reding
  0 siblings, 0 replies; 17+ messages in thread
From: Thierry Reding @ 2020-11-16 17:01 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas, Jonathan Hunter,
	linux-kernel, linux-safety, linux-tegra, linux-pci

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

On Mon, Nov 16, 2020 at 04:54:07PM +0000, Sudip Mukherjee wrote:
> Coccinelle suggested using PTR_ERR_OR_ZERO() and looking at the code,
> we can use PTR_ERR_OR_ZERO() instead of checking IS_ERR() and then
> doing 'return 0'.
> 
> Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> ---
>  drivers/pci/controller/pci-tegra.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

This has been proposed multiple times in the past and Bjorn and I have
agreed every time that this is not an improvement, so sorry, but NAK.

Thierry


-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#172): https://lists.elisa.tech/g/linux-safety/message/172
Mute This Topic: https://lists.elisa.tech/mt/78296400/5278000
Group Owner: linux-safety+owner@lists.elisa.tech
Unsubscribe: https://lists.elisa.tech/g/linux-safety/unsub [linux-safety@archiver.kernel.org]
-=-=-=-=-=-=-=-=-=-=-=-



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

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

* Re: [linux-safety] [PATCH] PCI: tegra: Use PTR_ERR_OR_ZERO
  2020-11-16 17:01   ` [linux-safety] " Thierry Reding
@ 2020-11-23 14:51     ` Sudip Mukherjee
  -1 siblings, 0 replies; 17+ messages in thread
From: Sudip Mukherjee @ 2020-11-23 14:51 UTC (permalink / raw)
  To: Thierry Reding, Sudip Mukherjee, Neil Armstrong
  Cc: Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas, Jonathan Hunter,
	linux-kernel, linux-safety, linux-tegra, linux-pci


On 16/11/2020 17:01, Thierry Reding wrote:
> On Mon, Nov 16, 2020 at 04:54:07PM +0000, Sudip Mukherjee wrote:
>> Coccinelle suggested using PTR_ERR_OR_ZERO() and looking at the code,
>> we can use PTR_ERR_OR_ZERO() instead of checking IS_ERR() and then
>> doing 'return 0'.
>>
>> Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
>> ---
>>  drivers/pci/controller/pci-tegra.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> This has been proposed multiple times in the past and Bjorn and I have
> agreed every time that this is not an improvement, so sorry, but NAK.

Thanks Thierry and Neil. I have now added a blacklist script in our CI
so "PTR_ERR_OR_ZERO" will not be flagged for anything in drivers/pci/*
anymore in our testing.



-- 
Regards
Sudip

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

* Re: [linux-safety] [PATCH] PCI: tegra: Use PTR_ERR_OR_ZERO
@ 2020-11-23 14:51     ` Sudip Mukherjee
  0 siblings, 0 replies; 17+ messages in thread
From: Sudip Mukherjee @ 2020-11-23 14:51 UTC (permalink / raw)
  To: Thierry Reding, Sudip Mukherjee, Neil Armstrong
  Cc: Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas, Jonathan Hunter,
	linux-kernel, linux-safety, linux-tegra, linux-pci


On 16/11/2020 17:01, Thierry Reding wrote:
> On Mon, Nov 16, 2020 at 04:54:07PM +0000, Sudip Mukherjee wrote:
>> Coccinelle suggested using PTR_ERR_OR_ZERO() and looking at the code,
>> we can use PTR_ERR_OR_ZERO() instead of checking IS_ERR() and then
>> doing 'return 0'.
>>
>> Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
>> ---
>>  drivers/pci/controller/pci-tegra.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> This has been proposed multiple times in the past and Bjorn and I have
> agreed every time that this is not an improvement, so sorry, but NAK.

Thanks Thierry and Neil. I have now added a blacklist script in our CI
so "PTR_ERR_OR_ZERO" will not be flagged for anything in drivers/pci/*
anymore in our testing.



-- 
Regards
Sudip


-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#189): https://lists.elisa.tech/g/linux-safety/message/189
Mute This Topic: https://lists.elisa.tech/mt/78296400/5278000
Group Owner: linux-safety+owner@lists.elisa.tech
Unsubscribe: https://lists.elisa.tech/g/linux-safety/unsub [linux-safety@archiver.kernel.org]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [linux-safety] [PATCH] PCI: tegra: Use PTR_ERR_OR_ZERO
  2020-11-23 14:51     ` Sudip Mukherjee
@ 2020-11-23 15:01       ` Lukas Bulwahn
  -1 siblings, 0 replies; 17+ messages in thread
From: Lukas Bulwahn @ 2020-11-23 15:01 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: Thierry Reding, Sudip Mukherjee, Neil Armstrong,
	Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas, Jonathan Hunter,
	Linux Kernel Mailing List, linux-safety, linux-tegra, linux-pci

On Mon, Nov 23, 2020 at 3:51 PM Sudip Mukherjee
<sudip.mukherjee@codethink.co.uk> wrote:
>
>
> On 16/11/2020 17:01, Thierry Reding wrote:
> > On Mon, Nov 16, 2020 at 04:54:07PM +0000, Sudip Mukherjee wrote:
> >> Coccinelle suggested using PTR_ERR_OR_ZERO() and looking at the code,
> >> we can use PTR_ERR_OR_ZERO() instead of checking IS_ERR() and then
> >> doing 'return 0'.
> >>
> >> Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> >> ---
> >>  drivers/pci/controller/pci-tegra.c | 4 +---
> >>  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > This has been proposed multiple times in the past and Bjorn and I have
> > agreed every time that this is not an improvement, so sorry, but NAK.
>
> Thanks Thierry and Neil. I have now added a blacklist script in our CI
> so "PTR_ERR_OR_ZERO" will not be flagged for anything in drivers/pci/*
> anymore in our testing.
>
>

Thanks, Sudip. I think that is a good idea to minimize churn on
subsystem maintainers from janitors.

We can continue to discuss how to share these settings on
ignore-rule-X-on-subsystem-Y with other "analysis tool CI
maintainers".

Lukas

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

* Re: [linux-safety] [PATCH] PCI: tegra: Use PTR_ERR_OR_ZERO
@ 2020-11-23 15:01       ` Lukas Bulwahn
  0 siblings, 0 replies; 17+ messages in thread
From: Lukas Bulwahn @ 2020-11-23 15:01 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: Thierry Reding, Sudip Mukherjee, Neil Armstrong,
	Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas, Jonathan Hunter,
	Linux Kernel Mailing List, linux-safety, linux-tegra, linux-pci

On Mon, Nov 23, 2020 at 3:51 PM Sudip Mukherjee
<sudip.mukherjee@codethink.co.uk> wrote:
>
>
> On 16/11/2020 17:01, Thierry Reding wrote:
> > On Mon, Nov 16, 2020 at 04:54:07PM +0000, Sudip Mukherjee wrote:
> >> Coccinelle suggested using PTR_ERR_OR_ZERO() and looking at the code,
> >> we can use PTR_ERR_OR_ZERO() instead of checking IS_ERR() and then
> >> doing 'return 0'.
> >>
> >> Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> >> ---
> >>  drivers/pci/controller/pci-tegra.c | 4 +---
> >>  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > This has been proposed multiple times in the past and Bjorn and I have
> > agreed every time that this is not an improvement, so sorry, but NAK.
>
> Thanks Thierry and Neil. I have now added a blacklist script in our CI
> so "PTR_ERR_OR_ZERO" will not be flagged for anything in drivers/pci/*
> anymore in our testing.
>
>

Thanks, Sudip. I think that is a good idea to minimize churn on
subsystem maintainers from janitors.

We can continue to discuss how to share these settings on
ignore-rule-X-on-subsystem-Y with other "analysis tool CI
maintainers".

Lukas


-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#190): https://lists.elisa.tech/g/linux-safety/message/190
Mute This Topic: https://lists.elisa.tech/mt/78296400/5278000
Group Owner: linux-safety+owner@lists.elisa.tech
Unsubscribe: https://lists.elisa.tech/g/linux-safety/unsub [linux-safety@archiver.kernel.org]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [PATCH] PCI: tegra: Use PTR_ERR_OR_ZERO
  2017-08-30 14:25     ` Thierry Reding
@ 2017-08-30 16:26       ` Bjorn Helgaas
  0 siblings, 0 replies; 17+ messages in thread
From: Bjorn Helgaas @ 2017-08-30 16:26 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Himanshu Jha, bhelgaas, jonathanh, linux-tegra, linux-pci, linux-kernel

On Wed, Aug 30, 2017 at 04:25:47PM +0200, Thierry Reding wrote:
> On Wed, Aug 30, 2017 at 08:59:31AM -0500, Bjorn Helgaas wrote:
> > On Tue, Aug 29, 2017 at 03:55:17PM +0200, Thierry Reding wrote:
> > > On Tue, Aug 29, 2017 at 07:09:00PM +0530, Himanshu Jha wrote:
> > > > Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR
> > > > 
> > > > Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> > > > ---
> > > >  drivers/pci/host/pci-tegra.c | 5 +----
> > > >  1 file changed, 1 insertion(+), 4 deletions(-)
> > > > 
> > > > diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> > > > index 9c40da5..90cda5b 100644
> > > > --- a/drivers/pci/host/pci-tegra.c
> > > > +++ b/drivers/pci/host/pci-tegra.c
> > > > @@ -1156,10 +1156,7 @@ static int tegra_pcie_resets_get(struct tegra_pcie *pcie)
> > > >  		return PTR_ERR(pcie->afi_rst);
> > > >  
> > > >  	pcie->pcie_xrst = devm_reset_control_get_exclusive(dev, "pcie_x");
> > > > -	if (IS_ERR(pcie->pcie_xrst))
> > > > -		return PTR_ERR(pcie->pcie_xrst);
> > > > -
> > > > -	return 0;
> > > > +	return PTR_ERR_OR_ZERO(pcie->pcie_xrst);
> > > >  }
> > > 
> > > I'm not a big fan of this construct because it's a pain to undo this if
> > > ever we need to add code to this function. But since we do have scripts
> > > that will flag this, I guess this would pop up every now and again. The
> > > driver is unlikely to change in this part, too, so:
> > 
> > Thanks for pointing this out.  Do you know what the benefit of
> > PTR_ERR_OR_ZERO() is?  To me, it makes the following code harder
> > to read because the error tests are no longer parallel:
> > 
> >   ...
> >   res->ahb_reset = devm_reset_control_get(dev, "ahb");
> >   if (IS_ERR(res->ahb_reset))
> >     return PTR_ERR(res->ahb_reset);
> > 
> >   res->por_reset = devm_reset_control_get(dev, "por");
> >   if (IS_ERR(res->por_reset))
> >     return PTR_ERR(res->por_reset);
> > 
> >   res->phy_reset = devm_reset_control_get(dev, "phy");
> >   return PTR_ERR_OR_ZERO(res->phy_reset);
> > 
> > So I'd be inclined to avoid it unless there's some significant benefit.
> 
> Yeah, I don't like the optics much either. Aside from the fact that it
> reduces the line count, I'm not aware of any benefits that this inline
> function has. It doesn't have any side-effects or anything, just wraps
> the common pattern into a single line.
> 
> Looking at the history of the semantic patch that is the basis for the
> conversions (scripts/coccinelle/api/ptr_ret.cocci), or the static inline
> function itself, no rationale is given for why people prefer this. I've
> certainly seen such patches applied in some cases, but I've also seen
> other maintainers (including myself) reject them because of personal
> preference.

OK, I'm going to drop this then.

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

* Re: [PATCH] PCI: tegra: Use PTR_ERR_OR_ZERO
  2017-08-30 13:59   ` Bjorn Helgaas
@ 2017-08-30 14:25     ` Thierry Reding
  2017-08-30 16:26       ` Bjorn Helgaas
  0 siblings, 1 reply; 17+ messages in thread
From: Thierry Reding @ 2017-08-30 14:25 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Himanshu Jha, bhelgaas, jonathanh, linux-tegra, linux-pci, linux-kernel

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

On Wed, Aug 30, 2017 at 08:59:31AM -0500, Bjorn Helgaas wrote:
> On Tue, Aug 29, 2017 at 03:55:17PM +0200, Thierry Reding wrote:
> > On Tue, Aug 29, 2017 at 07:09:00PM +0530, Himanshu Jha wrote:
> > > Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR
> > > 
> > > Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> > > ---
> > >  drivers/pci/host/pci-tegra.c | 5 +----
> > >  1 file changed, 1 insertion(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> > > index 9c40da5..90cda5b 100644
> > > --- a/drivers/pci/host/pci-tegra.c
> > > +++ b/drivers/pci/host/pci-tegra.c
> > > @@ -1156,10 +1156,7 @@ static int tegra_pcie_resets_get(struct tegra_pcie *pcie)
> > >  		return PTR_ERR(pcie->afi_rst);
> > >  
> > >  	pcie->pcie_xrst = devm_reset_control_get_exclusive(dev, "pcie_x");
> > > -	if (IS_ERR(pcie->pcie_xrst))
> > > -		return PTR_ERR(pcie->pcie_xrst);
> > > -
> > > -	return 0;
> > > +	return PTR_ERR_OR_ZERO(pcie->pcie_xrst);
> > >  }
> > 
> > I'm not a big fan of this construct because it's a pain to undo this if
> > ever we need to add code to this function. But since we do have scripts
> > that will flag this, I guess this would pop up every now and again. The
> > driver is unlikely to change in this part, too, so:
> 
> Thanks for pointing this out.  Do you know what the benefit of
> PTR_ERR_OR_ZERO() is?  To me, it makes the following code harder
> to read because the error tests are no longer parallel:
> 
>   ...
>   res->ahb_reset = devm_reset_control_get(dev, "ahb");
>   if (IS_ERR(res->ahb_reset))
>     return PTR_ERR(res->ahb_reset);
> 
>   res->por_reset = devm_reset_control_get(dev, "por");
>   if (IS_ERR(res->por_reset))
>     return PTR_ERR(res->por_reset);
> 
>   res->phy_reset = devm_reset_control_get(dev, "phy");
>   return PTR_ERR_OR_ZERO(res->phy_reset);
> 
> So I'd be inclined to avoid it unless there's some significant benefit.

Yeah, I don't like the optics much either. Aside from the fact that it
reduces the line count, I'm not aware of any benefits that this inline
function has. It doesn't have any side-effects or anything, just wraps
the common pattern into a single line.

Looking at the history of the semantic patch that is the basis for the
conversions (scripts/coccinelle/api/ptr_ret.cocci), or the static inline
function itself, no rationale is given for why people prefer this. I've
certainly seen such patches applied in some cases, but I've also seen
other maintainers (including myself) reject them because of personal
preference.

Thierry

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

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

* Re: [PATCH] PCI: tegra: Use PTR_ERR_OR_ZERO
  2017-08-29 13:55 ` Thierry Reding
  2017-08-29 14:14   ` Himanshu Jha
@ 2017-08-30 13:59   ` Bjorn Helgaas
  2017-08-30 14:25     ` Thierry Reding
  1 sibling, 1 reply; 17+ messages in thread
From: Bjorn Helgaas @ 2017-08-30 13:59 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Himanshu Jha, bhelgaas, jonathanh, linux-tegra, linux-pci, linux-kernel

On Tue, Aug 29, 2017 at 03:55:17PM +0200, Thierry Reding wrote:
> On Tue, Aug 29, 2017 at 07:09:00PM +0530, Himanshu Jha wrote:
> > Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR
> > 
> > Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> > ---
> >  drivers/pci/host/pci-tegra.c | 5 +----
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> > 
> > diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> > index 9c40da5..90cda5b 100644
> > --- a/drivers/pci/host/pci-tegra.c
> > +++ b/drivers/pci/host/pci-tegra.c
> > @@ -1156,10 +1156,7 @@ static int tegra_pcie_resets_get(struct tegra_pcie *pcie)
> >  		return PTR_ERR(pcie->afi_rst);
> >  
> >  	pcie->pcie_xrst = devm_reset_control_get_exclusive(dev, "pcie_x");
> > -	if (IS_ERR(pcie->pcie_xrst))
> > -		return PTR_ERR(pcie->pcie_xrst);
> > -
> > -	return 0;
> > +	return PTR_ERR_OR_ZERO(pcie->pcie_xrst);
> >  }
> 
> I'm not a big fan of this construct because it's a pain to undo this if
> ever we need to add code to this function. But since we do have scripts
> that will flag this, I guess this would pop up every now and again. The
> driver is unlikely to change in this part, too, so:

Thanks for pointing this out.  Do you know what the benefit of
PTR_ERR_OR_ZERO() is?  To me, it makes the following code harder
to read because the error tests are no longer parallel:

  ...
  res->ahb_reset = devm_reset_control_get(dev, "ahb");
  if (IS_ERR(res->ahb_reset))
    return PTR_ERR(res->ahb_reset);

  res->por_reset = devm_reset_control_get(dev, "por");
  if (IS_ERR(res->por_reset))
    return PTR_ERR(res->por_reset);

  res->phy_reset = devm_reset_control_get(dev, "phy");
  return PTR_ERR_OR_ZERO(res->phy_reset);

So I'd be inclined to avoid it unless there's some significant benefit.

Bjorn

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

* Re: [PATCH] PCI: tegra: Use PTR_ERR_OR_ZERO
  2017-08-29 14:14   ` Himanshu Jha
@ 2017-08-29 15:09       ` Thierry Reding
  0 siblings, 0 replies; 17+ messages in thread
From: Thierry Reding @ 2017-08-29 15:09 UTC (permalink / raw)
  To: Himanshu Jha
  Cc: bhelgaas-hpIqsD4AKlfQT0dZR+AlfA,
	jonathanh-DDmLM1+adcrQT0dZR+AlfA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-pci-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

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

On Tue, Aug 29, 2017 at 07:44:01PM +0530, Himanshu Jha wrote:
> On Tue, Aug 29, 2017 at 03:55:17PM +0200, Thierry Reding wrote:
> > On Tue, Aug 29, 2017 at 07:09:00PM +0530, Himanshu Jha wrote:
> > > Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR
> > > 
> > > Signed-off-by: Himanshu Jha <himanshujha199640-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > ---
> > >  drivers/pci/host/pci-tegra.c | 5 +----
> > >  1 file changed, 1 insertion(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> > > index 9c40da5..90cda5b 100644
> > > --- a/drivers/pci/host/pci-tegra.c
> > > +++ b/drivers/pci/host/pci-tegra.c
> > > @@ -1156,10 +1156,7 @@ static int tegra_pcie_resets_get(struct tegra_pcie *pcie)
> > >  		return PTR_ERR(pcie->afi_rst);
> > >  
> > >  	pcie->pcie_xrst = devm_reset_control_get_exclusive(dev, "pcie_x");
> > > -	if (IS_ERR(pcie->pcie_xrst))
> > > -		return PTR_ERR(pcie->pcie_xrst);
> > > -
> > > -	return 0;
> > > +	return PTR_ERR_OR_ZERO(pcie->pcie_xrst);
> > >  }
> > 
> > I'm not a big fan of this construct because it's a pain to undo this if
> > ever we need to add code to this function. But since we do have scripts
> > that will flag this, I guess this would pop up every now and again. The
> > driver is unlikely to change in this part, too, so:
> 
> What do you suggest ? Shall I stop sending these patches ?

No, it's fine. I'm just saying that there are cases where this doesn't
make sense. In this case I think it's fine because the driver is fairly
mature and unlikely to change, so there is not a lot of potential for
churn later on. In other cases, use your best judgement.

Ultimately it is up to maintainers whether or not they apply this kind
of patch.

Thierry

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

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

* Re: [PATCH] PCI: tegra: Use PTR_ERR_OR_ZERO
@ 2017-08-29 15:09       ` Thierry Reding
  0 siblings, 0 replies; 17+ messages in thread
From: Thierry Reding @ 2017-08-29 15:09 UTC (permalink / raw)
  To: Himanshu Jha; +Cc: bhelgaas, jonathanh, linux-tegra, linux-pci, linux-kernel

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

On Tue, Aug 29, 2017 at 07:44:01PM +0530, Himanshu Jha wrote:
> On Tue, Aug 29, 2017 at 03:55:17PM +0200, Thierry Reding wrote:
> > On Tue, Aug 29, 2017 at 07:09:00PM +0530, Himanshu Jha wrote:
> > > Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR
> > > 
> > > Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> > > ---
> > >  drivers/pci/host/pci-tegra.c | 5 +----
> > >  1 file changed, 1 insertion(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> > > index 9c40da5..90cda5b 100644
> > > --- a/drivers/pci/host/pci-tegra.c
> > > +++ b/drivers/pci/host/pci-tegra.c
> > > @@ -1156,10 +1156,7 @@ static int tegra_pcie_resets_get(struct tegra_pcie *pcie)
> > >  		return PTR_ERR(pcie->afi_rst);
> > >  
> > >  	pcie->pcie_xrst = devm_reset_control_get_exclusive(dev, "pcie_x");
> > > -	if (IS_ERR(pcie->pcie_xrst))
> > > -		return PTR_ERR(pcie->pcie_xrst);
> > > -
> > > -	return 0;
> > > +	return PTR_ERR_OR_ZERO(pcie->pcie_xrst);
> > >  }
> > 
> > I'm not a big fan of this construct because it's a pain to undo this if
> > ever we need to add code to this function. But since we do have scripts
> > that will flag this, I guess this would pop up every now and again. The
> > driver is unlikely to change in this part, too, so:
> 
> What do you suggest ? Shall I stop sending these patches ?

No, it's fine. I'm just saying that there are cases where this doesn't
make sense. In this case I think it's fine because the driver is fairly
mature and unlikely to change, so there is not a lot of potential for
churn later on. In other cases, use your best judgement.

Ultimately it is up to maintainers whether or not they apply this kind
of patch.

Thierry

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

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

* Re: [PATCH] PCI: tegra: Use PTR_ERR_OR_ZERO
  2017-08-29 13:55 ` Thierry Reding
@ 2017-08-29 14:14   ` Himanshu Jha
  2017-08-29 15:09       ` Thierry Reding
  2017-08-30 13:59   ` Bjorn Helgaas
  1 sibling, 1 reply; 17+ messages in thread
From: Himanshu Jha @ 2017-08-29 14:14 UTC (permalink / raw)
  To: Thierry Reding; +Cc: bhelgaas, jonathanh, linux-tegra, linux-pci, linux-kernel

On Tue, Aug 29, 2017 at 03:55:17PM +0200, Thierry Reding wrote:
> On Tue, Aug 29, 2017 at 07:09:00PM +0530, Himanshu Jha wrote:
> > Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR
> > 
> > Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> > ---
> >  drivers/pci/host/pci-tegra.c | 5 +----
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> > 
> > diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> > index 9c40da5..90cda5b 100644
> > --- a/drivers/pci/host/pci-tegra.c
> > +++ b/drivers/pci/host/pci-tegra.c
> > @@ -1156,10 +1156,7 @@ static int tegra_pcie_resets_get(struct tegra_pcie *pcie)
> >  		return PTR_ERR(pcie->afi_rst);
> >  
> >  	pcie->pcie_xrst = devm_reset_control_get_exclusive(dev, "pcie_x");
> > -	if (IS_ERR(pcie->pcie_xrst))
> > -		return PTR_ERR(pcie->pcie_xrst);
> > -
> > -	return 0;
> > +	return PTR_ERR_OR_ZERO(pcie->pcie_xrst);
> >  }
> 
> I'm not a big fan of this construct because it's a pain to undo this if
> ever we need to add code to this function. But since we do have scripts
> that will flag this, I guess this would pop up every now and again. The
> driver is unlikely to change in this part, too, so:

What do you suggest ? Shall I stop sending these patches ?

Thanks
-Himanshu Jha

> Acked-by: Thierry Reding <treding@nvidia.com>

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

* Re: [PATCH] PCI: tegra: Use PTR_ERR_OR_ZERO
  2017-08-29 13:39 ` Himanshu Jha
  (?)
@ 2017-08-29 13:55 ` Thierry Reding
  2017-08-29 14:14   ` Himanshu Jha
  2017-08-30 13:59   ` Bjorn Helgaas
  -1 siblings, 2 replies; 17+ messages in thread
From: Thierry Reding @ 2017-08-29 13:55 UTC (permalink / raw)
  To: Himanshu Jha; +Cc: bhelgaas, jonathanh, linux-tegra, linux-pci, linux-kernel

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

On Tue, Aug 29, 2017 at 07:09:00PM +0530, Himanshu Jha wrote:
> Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR
> 
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> ---
>  drivers/pci/host/pci-tegra.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> index 9c40da5..90cda5b 100644
> --- a/drivers/pci/host/pci-tegra.c
> +++ b/drivers/pci/host/pci-tegra.c
> @@ -1156,10 +1156,7 @@ static int tegra_pcie_resets_get(struct tegra_pcie *pcie)
>  		return PTR_ERR(pcie->afi_rst);
>  
>  	pcie->pcie_xrst = devm_reset_control_get_exclusive(dev, "pcie_x");
> -	if (IS_ERR(pcie->pcie_xrst))
> -		return PTR_ERR(pcie->pcie_xrst);
> -
> -	return 0;
> +	return PTR_ERR_OR_ZERO(pcie->pcie_xrst);
>  }

I'm not a big fan of this construct because it's a pain to undo this if
ever we need to add code to this function. But since we do have scripts
that will flag this, I guess this would pop up every now and again. The
driver is unlikely to change in this part, too, so:

Acked-by: Thierry Reding <treding@nvidia.com>

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

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

* [PATCH] PCI: tegra: Use PTR_ERR_OR_ZERO
@ 2017-08-29 13:39 ` Himanshu Jha
  0 siblings, 0 replies; 17+ messages in thread
From: Himanshu Jha @ 2017-08-29 13:39 UTC (permalink / raw)
  To: thierry.reding-Re5JQEeQqe8AvxtiuMwx3w
  Cc: bhelgaas-hpIqsD4AKlfQT0dZR+AlfA,
	jonathanh-DDmLM1+adcrQT0dZR+AlfA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-pci-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Himanshu Jha

Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR

Signed-off-by: Himanshu Jha <himanshujha199640-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/pci/host/pci-tegra.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index 9c40da5..90cda5b 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -1156,10 +1156,7 @@ static int tegra_pcie_resets_get(struct tegra_pcie *pcie)
 		return PTR_ERR(pcie->afi_rst);
 
 	pcie->pcie_xrst = devm_reset_control_get_exclusive(dev, "pcie_x");
-	if (IS_ERR(pcie->pcie_xrst))
-		return PTR_ERR(pcie->pcie_xrst);
-
-	return 0;
+	return PTR_ERR_OR_ZERO(pcie->pcie_xrst);
 }
 
 static int tegra_pcie_phys_get_legacy(struct tegra_pcie *pcie)
-- 
2.7.4

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

* [PATCH] PCI: tegra: Use PTR_ERR_OR_ZERO
@ 2017-08-29 13:39 ` Himanshu Jha
  0 siblings, 0 replies; 17+ messages in thread
From: Himanshu Jha @ 2017-08-29 13:39 UTC (permalink / raw)
  To: thierry.reding
  Cc: bhelgaas, jonathanh, linux-tegra, linux-pci, linux-kernel, Himanshu Jha

Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR

Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
 drivers/pci/host/pci-tegra.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index 9c40da5..90cda5b 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -1156,10 +1156,7 @@ static int tegra_pcie_resets_get(struct tegra_pcie *pcie)
 		return PTR_ERR(pcie->afi_rst);
 
 	pcie->pcie_xrst = devm_reset_control_get_exclusive(dev, "pcie_x");
-	if (IS_ERR(pcie->pcie_xrst))
-		return PTR_ERR(pcie->pcie_xrst);
-
-	return 0;
+	return PTR_ERR_OR_ZERO(pcie->pcie_xrst);
 }
 
 static int tegra_pcie_phys_get_legacy(struct tegra_pcie *pcie)
-- 
2.7.4

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

end of thread, other threads:[~2020-11-23 15:02 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-16 16:54 [PATCH] PCI: tegra: Use PTR_ERR_OR_ZERO Sudip Mukherjee
2020-11-16 16:54 ` [linux-safety] " Sudip Mukherjee
2020-11-16 17:01 ` Thierry Reding
2020-11-16 17:01   ` [linux-safety] " Thierry Reding
2020-11-23 14:51   ` Sudip Mukherjee
2020-11-23 14:51     ` Sudip Mukherjee
2020-11-23 15:01     ` Lukas Bulwahn
2020-11-23 15:01       ` Lukas Bulwahn
  -- strict thread matches above, loose matches on Subject: below --
2017-08-29 13:39 Himanshu Jha
2017-08-29 13:39 ` Himanshu Jha
2017-08-29 13:55 ` Thierry Reding
2017-08-29 14:14   ` Himanshu Jha
2017-08-29 15:09     ` Thierry Reding
2017-08-29 15:09       ` Thierry Reding
2017-08-30 13:59   ` Bjorn Helgaas
2017-08-30 14:25     ` Thierry Reding
2017-08-30 16:26       ` Bjorn Helgaas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.