Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] usb: cdns3: Rids of duplicate error message
@ 2020-10-12  6:42 Pawel Laszczak
  2020-10-12  8:16 ` Roger Quadros
  0 siblings, 1 reply; 6+ messages in thread
From: Pawel Laszczak @ 2020-10-12  6:42 UTC (permalink / raw)
  To: balbi
  Cc: peter.chen, rogerq, gregkh, linux-usb, linux-kernel, kurahul,
	Pawel Laszczak

On failure, the platform_get_irq_byname prints an error message
so, patch removes error message related to this function from
core.c file.

A change was suggested during reviewing CDNSP driver by Chunfeng Yun.

Signed-off-by: Pawel Laszczak <pawell@cadence.com>
---
Changelog:
v2
- simplified code as sugested by Roger Quadros.

 drivers/usb/cdns3/core.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
index a0f73d4711ae..85ef3025b293 100644
--- a/drivers/usb/cdns3/core.c
+++ b/drivers/usb/cdns3/core.c
@@ -469,22 +469,14 @@ static int cdns3_probe(struct platform_device *pdev)
 	if (cdns->dev_irq == -EPROBE_DEFER)
 		return cdns->dev_irq;
 
-	if (cdns->dev_irq < 0)
-		dev_err(dev, "couldn't get peripheral irq\n");
-
 	regs = devm_platform_ioremap_resource_byname(pdev, "dev");
 	if (IS_ERR(regs))
 		return PTR_ERR(regs);
 	cdns->dev_regs	= regs;
 
 	cdns->otg_irq = platform_get_irq_byname(pdev, "otg");
-	if (cdns->otg_irq == -EPROBE_DEFER)
-		return cdns->otg_irq;
-
-	if (cdns->otg_irq < 0) {
-		dev_err(dev, "couldn't get otg irq\n");
+	if (cdns->otg_irq < 0)
 		return cdns->otg_irq;
-	}
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "otg");
 	if (!res) {
-- 
2.17.1


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

* Re: [PATCH v2] usb: cdns3: Rids of duplicate error message
  2020-10-12  6:42 [PATCH v2] usb: cdns3: Rids of duplicate error message Pawel Laszczak
@ 2020-10-12  8:16 ` Roger Quadros
  2020-10-12 11:12   ` Pawel Laszczak
  0 siblings, 1 reply; 6+ messages in thread
From: Roger Quadros @ 2020-10-12  8:16 UTC (permalink / raw)
  To: Pawel Laszczak, balbi
  Cc: peter.chen, gregkh, linux-usb, linux-kernel, kurahul

Hi Pawel,

On 12/10/2020 09:42, Pawel Laszczak wrote:
> On failure, the platform_get_irq_byname prints an error message
> so, patch removes error message related to this function from
> core.c file.
> 
> A change was suggested during reviewing CDNSP driver by Chunfeng Yun.
> 
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> ---
> Changelog:
> v2
> - simplified code as sugested by Roger Quadros.
> 
>   drivers/usb/cdns3/core.c | 10 +---------
>   1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> index a0f73d4711ae..85ef3025b293 100644
> --- a/drivers/usb/cdns3/core.c
> +++ b/drivers/usb/cdns3/core.c
> @@ -469,22 +469,14 @@ static int cdns3_probe(struct platform_device *pdev)
>   	if (cdns->dev_irq == -EPROBE_DEFER)

Shouldn't this be
	if (cdns->dev_irq < 0)
?

>   		return cdns->dev_irq;
>   
> -	if (cdns->dev_irq < 0)
> -		dev_err(dev, "couldn't get peripheral irq\n");
> -
>   	regs = devm_platform_ioremap_resource_byname(pdev, "dev");
>   	if (IS_ERR(regs))
>   		return PTR_ERR(regs);
>   	cdns->dev_regs	= regs;
>   
>   	cdns->otg_irq = platform_get_irq_byname(pdev, "otg");
> -	if (cdns->otg_irq == -EPROBE_DEFER)
> -		return cdns->otg_irq;
> -
> -	if (cdns->otg_irq < 0) {
> -		dev_err(dev, "couldn't get otg irq\n");
> +	if (cdns->otg_irq < 0)
>   		return cdns->otg_irq;
> -	}
>   
>   	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "otg");
>   	if (!res) {
> 

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

* RE: [PATCH v2] usb: cdns3: Rids of duplicate error message
  2020-10-12  8:16 ` Roger Quadros
@ 2020-10-12 11:12   ` Pawel Laszczak
  2020-10-12 11:38     ` Roger Quadros
  0 siblings, 1 reply; 6+ messages in thread
From: Pawel Laszczak @ 2020-10-12 11:12 UTC (permalink / raw)
  To: Roger Quadros, balbi
  Cc: peter.chen, gregkh, linux-usb, linux-kernel, Rahul Kumar

Hi Roger,

On 12/10/2020 09:42, Pawel Laszczak wrote:
> On failure, the platform_get_irq_byname prints an error message
> so, patch removes error message related to this function from
> core.c file.
> 
> A change was suggested during reviewing CDNSP driver by Chunfeng Yun.
> 
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> ---
> Changelog:
> v2
> - simplified code as sugested by Roger Quadros.
> 
>   drivers/usb/cdns3/core.c | 10 +---------
>   1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> index a0f73d4711ae..85ef3025b293 100644
> --- a/drivers/usb/cdns3/core.c
> +++ b/drivers/usb/cdns3/core.c
> @@ -469,22 +469,14 @@ static int cdns3_probe(struct platform_device *pdev)
>   	if (cdns->dev_irq == -EPROBE_DEFER)

Shouldn't this be
	if (cdns->dev_irq < 0)
?

No, such line will change the original behavior of driver.

Current patch allows to run driver when we support only host. 
In such case the dev_irq can be < 0 and we still can use host  side.

Regards,
Pawel Laszczak

>   		return cdns->dev_irq;
>   
> -	if (cdns->dev_irq < 0)
> -		dev_err(dev, "couldn't get peripheral irq\n");
> -
>   	regs = devm_platform_ioremap_resource_byname(pdev, "dev");
>   	if (IS_ERR(regs))
>   		return PTR_ERR(regs);
>   	cdns->dev_regs	= regs;
>   
>   	cdns->otg_irq = platform_get_irq_byname(pdev, "otg");
> -	if (cdns->otg_irq == -EPROBE_DEFER)
> -		return cdns->otg_irq;
> -
> -	if (cdns->otg_irq < 0) {
> -		dev_err(dev, "couldn't get otg irq\n");
> +	if (cdns->otg_irq < 0)
>   		return cdns->otg_irq;
> -	}
>   
>   	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "otg");
>   	if (!res) {
> 


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

* Re: [PATCH v2] usb: cdns3: Rids of duplicate error message
  2020-10-12 11:12   ` Pawel Laszczak
@ 2020-10-12 11:38     ` Roger Quadros
  2020-10-12 12:18       ` Peter Chen
  0 siblings, 1 reply; 6+ messages in thread
From: Roger Quadros @ 2020-10-12 11:38 UTC (permalink / raw)
  To: Pawel Laszczak, balbi
  Cc: peter.chen, gregkh, linux-usb, linux-kernel, Rahul Kumar

Pawel,

On 12/10/2020 14:12, Pawel Laszczak wrote:
> Hi Roger,
> 
> On 12/10/2020 09:42, Pawel Laszczak wrote:
>> On failure, the platform_get_irq_byname prints an error message
>> so, patch removes error message related to this function from
>> core.c file.
>>
>> A change was suggested during reviewing CDNSP driver by Chunfeng Yun.
>>
>> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
>> ---
>> Changelog:
>> v2
>> - simplified code as sugested by Roger Quadros.
>>
>>    drivers/usb/cdns3/core.c | 10 +---------
>>    1 file changed, 1 insertion(+), 9 deletions(-)
>>
>> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>> index a0f73d4711ae..85ef3025b293 100644
>> --- a/drivers/usb/cdns3/core.c
>> +++ b/drivers/usb/cdns3/core.c
>> @@ -469,22 +469,14 @@ static int cdns3_probe(struct platform_device *pdev)
>>    	if (cdns->dev_irq == -EPROBE_DEFER)
> 
> Shouldn't this be
> 	if (cdns->dev_irq < 0)
> ?
> 
> No, such line will change the original behavior of driver.
> 
> Current patch allows to run driver when we support only host.
> In such case the dev_irq can be < 0 and we still can use host  side.

In that case should we check if we need dev_irq and if so then error out.
i.e. if mode is "peripheral" or "otg"

Also DT binding document says all 3 IRQs are mandatory. Do we need to update that?

cheers,
-roger

> 
> Regards,
> Pawel Laszczak
> 
>>    		return cdns->dev_irq;
>>    
>> -	if (cdns->dev_irq < 0)
>> -		dev_err(dev, "couldn't get peripheral irq\n");
>> -
>>    	regs = devm_platform_ioremap_resource_byname(pdev, "dev");
>>    	if (IS_ERR(regs))
>>    		return PTR_ERR(regs);
>>    	cdns->dev_regs	= regs;
>>    
>>    	cdns->otg_irq = platform_get_irq_byname(pdev, "otg");
>> -	if (cdns->otg_irq == -EPROBE_DEFER)
>> -		return cdns->otg_irq;
>> -
>> -	if (cdns->otg_irq < 0) {
>> -		dev_err(dev, "couldn't get otg irq\n");
>> +	if (cdns->otg_irq < 0)
>>    		return cdns->otg_irq;
>> -	}
>>    
>>    	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "otg");
>>    	if (!res) {
>>
> 

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

* Re: [PATCH v2] usb: cdns3: Rids of duplicate error message
  2020-10-12 11:38     ` Roger Quadros
@ 2020-10-12 12:18       ` Peter Chen
  2020-10-12 12:26         ` Pawel Laszczak
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Chen @ 2020-10-12 12:18 UTC (permalink / raw)
  To: Roger Quadros
  Cc: Pawel Laszczak, balbi, gregkh, linux-usb, linux-kernel, Rahul Kumar

On 20-10-12 14:38:16, Roger Quadros wrote:
> Pawel,
> 
> On 12/10/2020 14:12, Pawel Laszczak wrote:
> > Hi Roger,
> > 
> > On 12/10/2020 09:42, Pawel Laszczak wrote:
> > > On failure, the platform_get_irq_byname prints an error message
> > > so, patch removes error message related to this function from
> > > core.c file.
> > > 
> > > A change was suggested during reviewing CDNSP driver by Chunfeng Yun.
> > > 
> > > Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> > > ---
> > > Changelog:
> > > v2
> > > - simplified code as sugested by Roger Quadros.
> > > 
> > >    drivers/usb/cdns3/core.c | 10 +---------
> > >    1 file changed, 1 insertion(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> > > index a0f73d4711ae..85ef3025b293 100644
> > > --- a/drivers/usb/cdns3/core.c
> > > +++ b/drivers/usb/cdns3/core.c
> > > @@ -469,22 +469,14 @@ static int cdns3_probe(struct platform_device *pdev)
> > >    	if (cdns->dev_irq == -EPROBE_DEFER)
> > 
> > Shouldn't this be
> > 	if (cdns->dev_irq < 0)
> > ?
> > 
> > No, such line will change the original behavior of driver.
> > 
> > Current patch allows to run driver when we support only host.
> > In such case the dev_irq can be < 0 and we still can use host  side.
> 
> In that case should we check if we need dev_irq and if so then error out.
> i.e. if mode is "peripheral" or "otg"
> 
> Also DT binding document says all 3 IRQs are mandatory. Do we need to update that?
> 

I agree with Roger, or you change the driver behaviour that return error
if the dev_irq is < 0.

Peter
> 
> > 
> > Regards,
> > Pawel Laszczak
> > 
> > >    		return cdns->dev_irq;
> > > -	if (cdns->dev_irq < 0)
> > > -		dev_err(dev, "couldn't get peripheral irq\n");
> > > -
> > >    	regs = devm_platform_ioremap_resource_byname(pdev, "dev");
> > >    	if (IS_ERR(regs))
> > >    		return PTR_ERR(regs);
> > >    	cdns->dev_regs	= regs;
> > >    	cdns->otg_irq = platform_get_irq_byname(pdev, "otg");
> > > -	if (cdns->otg_irq == -EPROBE_DEFER)
> > > -		return cdns->otg_irq;
> > > -
> > > -	if (cdns->otg_irq < 0) {
> > > -		dev_err(dev, "couldn't get otg irq\n");
> > > +	if (cdns->otg_irq < 0)
> > >    		return cdns->otg_irq;
> > > -	}
> > >    	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "otg");
> > >    	if (!res) {
> > > 
> > 
> 
> -- 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

-- 

Thanks,
Peter Chen

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

* RE: [PATCH v2] usb: cdns3: Rids of duplicate error message
  2020-10-12 12:18       ` Peter Chen
@ 2020-10-12 12:26         ` Pawel Laszczak
  0 siblings, 0 replies; 6+ messages in thread
From: Pawel Laszczak @ 2020-10-12 12:26 UTC (permalink / raw)
  To: Peter Chen, Roger Quadros
  Cc: balbi, gregkh, linux-usb, linux-kernel, Rahul Kumar


>
>On 20-10-12 14:38:16, Roger Quadros wrote:
>> Pawel,
>>
>> On 12/10/2020 14:12, Pawel Laszczak wrote:
>> > Hi Roger,
>> >
>> > On 12/10/2020 09:42, Pawel Laszczak wrote:
>> > > On failure, the platform_get_irq_byname prints an error message
>> > > so, patch removes error message related to this function from
>> > > core.c file.
>> > >
>> > > A change was suggested during reviewing CDNSP driver by Chunfeng Yun.
>> > >
>> > > Signed-off-by: Pawel Laszczak <pawell@cadence.com>
>> > > ---
>> > > Changelog:
>> > > v2
>> > > - simplified code as sugested by Roger Quadros.
>> > >
>> > >    drivers/usb/cdns3/core.c | 10 +---------
>> > >    1 file changed, 1 insertion(+), 9 deletions(-)
>> > >
>> > > diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>> > > index a0f73d4711ae..85ef3025b293 100644
>> > > --- a/drivers/usb/cdns3/core.c
>> > > +++ b/drivers/usb/cdns3/core.c
>> > > @@ -469,22 +469,14 @@ static int cdns3_probe(struct platform_device *pdev)
>> > >    	if (cdns->dev_irq == -EPROBE_DEFER)
>> >
>> > Shouldn't this be
>> > 	if (cdns->dev_irq < 0)
>> > ?
>> >
>> > No, such line will change the original behavior of driver.
>> >
>> > Current patch allows to run driver when we support only host.
>> > In such case the dev_irq can be < 0 and we still can use host  side.
>>
>> In that case should we check if we need dev_irq and if so then error out.
>> i.e. if mode is "peripheral" or "otg"
>>
>> Also DT binding document says all 3 IRQs are mandatory. Do we need to update that?
>>
>
>I agree with Roger, or you change the driver behaviour that return error
>if the dev_irq is < 0.

Ok, I will  change the  condition as suggested by Roger. 

Pawel
>
>Peter
>>
>> >
>> > Regards,
>> > Pawel Laszczak
>> >
>> > >    		return cdns->dev_irq;
>> > > -	if (cdns->dev_irq < 0)
>> > > -		dev_err(dev, "couldn't get peripheral irq\n");
>> > > -
>> > >    	regs = devm_platform_ioremap_resource_byname(pdev, "dev");
>> > >    	if (IS_ERR(regs))
>> > >    		return PTR_ERR(regs);
>> > >    	cdns->dev_regs	= regs;
>> > >    	cdns->otg_irq = platform_get_irq_byname(pdev, "otg");
>> > > -	if (cdns->otg_irq == -EPROBE_DEFER)
>> > > -		return cdns->otg_irq;
>> > > -
>> > > -	if (cdns->otg_irq < 0) {
>> > > -		dev_err(dev, "couldn't get otg irq\n");
>> > > +	if (cdns->otg_irq < 0)
>> > >    		return cdns->otg_irq;
>> > > -	}
>> > >    	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "otg");
>> > >    	if (!res) {
>> > >
>> >
>>
>> --
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
>--
>
>Thanks,
>Peter Chen

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

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-12  6:42 [PATCH v2] usb: cdns3: Rids of duplicate error message Pawel Laszczak
2020-10-12  8:16 ` Roger Quadros
2020-10-12 11:12   ` Pawel Laszczak
2020-10-12 11:38     ` Roger Quadros
2020-10-12 12:18       ` Peter Chen
2020-10-12 12:26         ` Pawel Laszczak

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git