Re: [PATCH] PCI: rcar: fix return value check in rcar_pci_probe()

Re: [PATCH] PCI: rcar: fix return value check in rcar_pci_probe()

[Jingoo Han]

On Sunday, December 08, 2013 7:50 AM, Bjorn Helgaas wrote:
> On Tue, Nov 19, 2013 at 11:40:28AM +0800, Wei Yongjun wrote:
> > From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> >
> > In case of error, the function devm_ioremap_resource() returns ERR_PTR()
> > and never returns NULL. The NULL test in the return value check should
> > be replaced with IS_ERR().
> >
> > Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> 
> Applied with Jingoo's reviewed-by to pci/host-rcar for v3.14, thanks!
> 
> For drivers/pci/host/*, I normally look for an ack from the responsible
> person,  but this patch is trivial enough that I'm fine taking it without
> that.  But for more significant changes, I don't have any notes about who
> should own pci-rcar-gen2.c.  Valentine could be a candidate since he added
> it in the first place?  Or Jingoo?

(+cc Simon Horman, Magnus Damm, Kuninori Morimoto, linux-sh mailing-list)

Hi Bjorn,

I think that Valentine could be a candidate, because he is an author.
However, pci-rcar-gen2.c is working on Renesas SoC, so, it may be
necessary to get ACK from Renesas people such as Simon Horman,
Magnus Damm, and Kuninori Morimoto.

Simon Horman, Magnus Damm, and Kuninori Morimoto,
Who is a proper person responsible for RCar Gen PCIe driver?
(drivers/pci/host/pci-rcar-gen2.c)

Best regards,
Jingoo Han

> 
> Bjorn
> 
> > ---
> >  drivers/pci/host/pci-rcar-gen2.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/host/pci-rcar-gen2.c b/drivers/pci/host/pci-rcar-gen2.c
> > index cbaa5c4..96d1182 100644
> > --- a/drivers/pci/host/pci-rcar-gen2.c
> > +++ b/drivers/pci/host/pci-rcar-gen2.c
> > @@ -276,8 +276,8 @@ static int __init rcar_pci_probe(struct platform_device *pdev)
> >
> >  	cfg_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >  	reg = devm_ioremap_resource(&pdev->dev, cfg_res);
> > -	if (!reg)
> > -		return -ENODEV;
> > +	if (IS_ERR(reg))
> > +		return PTR_ERR(reg);
> >
> >  	mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> >  	if (!mem_res || !mem_res->start)
> >

Re: [PATCH] PCI: rcar: fix return value check in rcar_pci_probe()

[Simon Horman]

On Mon, Dec 09, 2013 at 10:46:17AM +0900, Jingoo Han wrote:
> On Sunday, December 08, 2013 7:50 AM, Bjorn Helgaas wrote:
> > On Tue, Nov 19, 2013 at 11:40:28AM +0800, Wei Yongjun wrote:
> > > From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> > >
> > > In case of error, the function devm_ioremap_resource() returns ERR_PTR()
> > > and never returns NULL. The NULL test in the return value check should
> > > be replaced with IS_ERR().
> > >
> > > Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> > 
> > Applied with Jingoo's reviewed-by to pci/host-rcar for v3.14, thanks!
> > 
> > For drivers/pci/host/*, I normally look for an ack from the responsible
> > person,  but this patch is trivial enough that I'm fine taking it without
> > that.  But for more significant changes, I don't have any notes about who
> > should own pci-rcar-gen2.c.  Valentine could be a candidate since he added
> > it in the first place?  Or Jingoo?
> 
> (+cc Simon Horman, Magnus Damm, Kuninori Morimoto, linux-sh mailing-list)
> 
> Hi Bjorn,
> 
> I think that Valentine could be a candidate, because he is an author.
> However, pci-rcar-gen2.c is working on Renesas SoC, so, it may be
> necessary to get ACK from Renesas people such as Simon Horman,
> Magnus Damm, and Kuninori Morimoto.
> 
> Simon Horman, Magnus Damm, and Kuninori Morimoto,
> Who is a proper person responsible for RCar Gen PCIe driver?
> (drivers/pci/host/pci-rcar-gen2.c)

Good question.

My feeling is that as it relates to Renesas ARM SoCs that
responsibility at least in part defaults to the Renesas ARM SoC
maintainers, Magnus and myself.

So I think it would be best if the following were CCed on any patches
to this driver:

Simon Horman <horms@verge.net.au>
Magnus Damm <magnus.damm@gmail.com>
linux-sh@vger.kernel.org

I would not be opposed for there being a MAINTAINERS file entry to that effect.

I will let Valentine volunteer himself if he wants to.

> 
> Best regards,
> Jingoo Han
> 
> > 
> > Bjorn
> > 
> > > ---
> > >  drivers/pci/host/pci-rcar-gen2.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/pci/host/pci-rcar-gen2.c b/drivers/pci/host/pci-rcar-gen2.c
> > > index cbaa5c4..96d1182 100644
> > > --- a/drivers/pci/host/pci-rcar-gen2.c
> > > +++ b/drivers/pci/host/pci-rcar-gen2.c
> > > @@ -276,8 +276,8 @@ static int __init rcar_pci_probe(struct platform_device *pdev)
> > >
> > >  	cfg_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > >  	reg = devm_ioremap_resource(&pdev->dev, cfg_res);
> > > -	if (!reg)
> > > -		return -ENODEV;
> > > +	if (IS_ERR(reg))
> > > +		return PTR_ERR(reg);
> > >
> > >  	mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > >  	if (!mem_res || !mem_res->start)
> > >
> 

Re: [PATCH] PCI: rcar: fix return value check in rcar_pci_probe()

[Valentine]

On 12/11/2013 06:30 AM, Simon Horman wrote:
> On Mon, Dec 09, 2013 at 10:46:17AM +0900, Jingoo Han wrote:
>> On Sunday, December 08, 2013 7:50 AM, Bjorn Helgaas wrote:
>>> On Tue, Nov 19, 2013 at 11:40:28AM +0800, Wei Yongjun wrote:
>>>> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>>>>
>>>> In case of error, the function devm_ioremap_resource() returns ERR_PTR()
>>>> and never returns NULL. The NULL test in the return value check should
>>>> be replaced with IS_ERR().
>>>>
>>>> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>>>
>>> Applied with Jingoo's reviewed-by to pci/host-rcar for v3.14, thanks!
>>>
>>> For drivers/pci/host/*, I normally look for an ack from the responsible
>>> person,  but this patch is trivial enough that I'm fine taking it without
>>> that.  But for more significant changes, I don't have any notes about who
>>> should own pci-rcar-gen2.c.  Valentine could be a candidate since he added
>>> it in the first place?  Or Jingoo?

Sorry, I didn't ack since I thought it was not really needed for a trivial
change like this one that had already been reviewed by Jingoo.

>>
>> (+cc Simon Horman, Magnus Damm, Kuninori Morimoto, linux-sh mailing-list)
>>
>> Hi Bjorn,
>>
>> I think that Valentine could be a candidate, because he is an author.
>> However, pci-rcar-gen2.c is working on Renesas SoC, so, it may be
>> necessary to get ACK from Renesas people such as Simon Horman,
>> Magnus Damm, and Kuninori Morimoto.
>>
>> Simon Horman, Magnus Damm, and Kuninori Morimoto,
>> Who is a proper person responsible for RCar Gen PCIe driver?
>> (drivers/pci/host/pci-rcar-gen2.c)
>
> Good question.
>
> My feeling is that as it relates to Renesas ARM SoCs that
> responsibility at least in part defaults to the Renesas ARM SoC
> maintainers, Magnus and myself.
>
> So I think it would be best if the following were CCed on any patches
> to this driver:
>
> Simon Horman <horms@verge.net.au>
> Magnus Damm <magnus.damm@gmail.com>
> linux-sh@vger.kernel.org

I agree.

>
> I would not be opposed for there being a MAINTAINERS file entry to that effect.
>
> I will let Valentine volunteer himself if he wants to.

I'll try to track the e-mails related to this driver as well.

Not sure if I want myself in the MAINTAINERS though, since it would probably
mean that "try to" should be changed to "have to" in my statement above.

Thanks,
Val.

>
>>
>> Best regards,
>> Jingoo Han
>>
>>>
>>> Bjorn
>>>
>>>> ---
>>>>   drivers/pci/host/pci-rcar-gen2.c | 4 ++--
>>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/pci/host/pci-rcar-gen2.c b/drivers/pci/host/pci-rcar-gen2.c
>>>> index cbaa5c4..96d1182 100644
>>>> --- a/drivers/pci/host/pci-rcar-gen2.c
>>>> +++ b/drivers/pci/host/pci-rcar-gen2.c
>>>> @@ -276,8 +276,8 @@ static int __init rcar_pci_probe(struct platform_device *pdev)
>>>>
>>>>   	cfg_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>   	reg = devm_ioremap_resource(&pdev->dev, cfg_res);
>>>> -	if (!reg)
>>>> -		return -ENODEV;
>>>> +	if (IS_ERR(reg))
>>>> +		return PTR_ERR(reg);
>>>>
>>>>   	mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>>>>   	if (!mem_res || !mem_res->start)
>>>>
>>

Re: [PATCH] PCI: rcar: fix return value check in rcar_pci_probe()

[Bjorn Helgaas]

On Fri, Dec 13, 2013 at 5:06 AM, Valentine
<valentine.barshak@cogentembedded.com> wrote:
> On 12/11/2013 06:30 AM, Simon Horman wrote:
>>
>> On Mon, Dec 09, 2013 at 10:46:17AM +0900, Jingoo Han wrote:
>>>
>>> On Sunday, December 08, 2013 7:50 AM, Bjorn Helgaas wrote:

>>>> For drivers/pci/host/*, I normally look for an ack from the responsible
>>>> person,  but this patch is trivial enough that I'm fine taking it
>>>> without
>>>> that.  But for more significant changes, I don't have any notes about
>>>> who
>>>> should own pci-rcar-gen2.c.  Valentine could be a candidate since he
>>>> added
>>>> it in the first place?  Or Jingoo?
>
>
> Sorry, I didn't ack since I thought it was not really needed for a trivial
> change like this one that had already been reviewed by Jingoo.

To be honest, I'm just trying to save myself some work by looking for
an ack.  I want to have an indication that at least one person besides
the author approves, and if nobody else looks at it, I have to do it
myself :)

>> I will let Valentine volunteer himself if he wants to.
>
>
> I'll try to track the e-mails related to this driver as well.
>
> Not sure if I want myself in the MAINTAINERS though, since it would probably
> mean that "try to" should be changed to "have to" in my statement above.

Fair enough, thanks!  Let me know if anything changes here.

Bjorn