RE: [PATCH] usb: cdns3: platform_get_irq_byname_optional instead platform_get_irq_byname

2020-10-06 Thread Pawel Laszczak
Felipe, 

Please  ignore this patch. I will create the new one. 
It's no sense to send the v2 because I have to change the patch name,
Description and contents.

Regards,
Pawel,

>Roger,
>
>>Pawel,
>>
>>On 05/10/2020 08:54, Pawel Laszczak wrote:
>>> Roger,

 Pawel,

 On 02/10/2020 12:08, Pawel Laszczak wrote:
> Roger,
>
>>
>> On 30/09/2020 09:57, Pawel Laszczak wrote:
>>> To avoid duplicate error information patch replaces 
>>> platform_get_irq_byname
>>> into platform_get_irq_byname_optional.
>>
>> What is duplicate error information?
>
> The function platform_get_irq_byname print:
> " dev_err(>dev, "IRQ %s not found\n", name);" if error occurred.
>
> In core.c we have the another error message below invoking this function.
> e.g
>   if (cdns->dev_irq < 0)
>   dev_err(dev, "couldn't get peripheral irq\n");
>
> So, it's looks like one dev_err is  redundant.

 If we want all 3 IRQs to be valid irrespective of dr_mode then we should
 use platform_get_irq_byname() and error out in probe if (ret < 0 && ret != 
 -EPROBE_DEFER).

 We can get rid of the irq check and duplicate error message in other 
 places.
>>>
>>> To be sure we understand each other correctly.
>>>
>>> Are You suggesting  to leave the  platform_get_irq_byname()
>>> and rid of from core.c the following lines :
>>>
>>> if (cdns->dev_irq < 0)
>>> dev_err(dev, "couldn't get peripheral irq\n");
>>>
>>> and
>>>
>>> dev_err(dev, "couldn't get otg irq\n");
>>> ?
>>
>>Yes.
>>
>>>
>>> A word of explanation why this patch has been sent.
>>> During reviewing the cdnsp driver Chunfeng Yun add such comment:
>>>
>>> "
 +  cdns->dev_irq = platform_get_irq_byname(pdev, "peripheral");
 +  if (cdns->dev_irq == -EPROBE_DEFER)
 +  return cdns->dev_irq;
 +
 +  if (cdns->dev_irq < 0)
 +  dev_err(dev, "couldn't get peripheral irq\n");
>>> Use platform_get_irq_byname_optional? otherwise no need print this log,
>>> platform_get_irq_byname() will print it.
>>> "
>>>
>>> In this patch I've chosen the platform_get_irq_byname_optional because both
>>> function do the same but the error message from core.c tell us little more 
>>> then
>>> generic message from platform_get_irq_byname.
>>
>>Using platform_get_irq_byname_optional() says driver expects it is optional 
>>but
>>only to fail later. It will be confusing to new reader that's all. I leave it 
>>to
>>you to decide what approach to take.
>
>Thanks for clarification.
>I will send  new patch with platform_get_irq_byname.
>
>Cheers,
>Pawel
>
>>

>
>>
>>>
>>> A change was suggested during reviewing CDNSP driver by Chunfeng Yun.
>>>
>>> Signed-off-by: Pawel Laszczak 
>>> ---
>>> drivers/usb/cdns3/core.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>>> index a0f73d4711ae..a3f6dc44cf3a 100644
>>> --- a/drivers/usb/cdns3/core.c
>>> +++ b/drivers/usb/cdns3/core.c
>>> @@ -465,7 +465,7 @@ static int cdns3_probe(struct platform_device *pdev)
>>>
>>> cdns->xhci_res[1] = *res;
>>>
>>> -   cdns->dev_irq = platform_get_irq_byname(pdev, "peripheral");
>>> +   cdns->dev_irq = platform_get_irq_byname_optional(pdev, 
>>> "peripheral");
>>
>> As per DT binding document, these are mandatory properties
>
> I think that name platform_get_irq_byname_optional is little confusing.
> Function descriptions show that both are almost identical:
> /**
>* platform_get_irq_byname_optional - get an optional IRQ for a device 
> by name
>* @dev: platform device
>* @name: IRQ name
>*
>* Get an optional IRQ by name like platform_get_irq_byname(). Except 
> that it
>* does not print an error message if an IRQ can not be obtained.
>*
>* Return: non-zero IRQ number on success, negative error number on 
> failure.
>*/
>
>>
>>- interrupts: Interrupts used by cdns3 controller:
>>   "host" - interrupt used by XHCI driver.
>>   "peripheral" - interrupt used by device driver
>>   "otg" - interrupt used by DRD/OTG  part of driver
>>
>> for dr_mode == "otg" -> all 3 are mandatory.
>> for dr_mode == "host" -> "otg" and "peripheral" IRQs are not required.
>> for dr_mode == "periphearal" -> "otg" and "host" IRQs are not required.
>>
>>> if (cdns->dev_irq == -EPROBE_DEFER)
>>> return cdns->dev_irq;
>>>
>>> @@ -477,7 +477,7 @@ static int cdns3_probe(struct platform_device *pdev)
>>> return PTR_ERR(regs);
>>> cdns->dev_regs  = regs;
>>>
>>> -   cdns->otg_irq = platform_get_irq_byname(pdev, "otg");
>>> +   cdns->otg_irq = 

RE: [PATCH] usb: cdns3: platform_get_irq_byname_optional instead platform_get_irq_byname

2020-10-05 Thread Pawel Laszczak
Roger,

>Pawel,
>
>On 05/10/2020 08:54, Pawel Laszczak wrote:
>> Roger,
>>>
>>> Pawel,
>>>
>>> On 02/10/2020 12:08, Pawel Laszczak wrote:
 Roger,

>
> On 30/09/2020 09:57, Pawel Laszczak wrote:
>> To avoid duplicate error information patch replaces 
>> platform_get_irq_byname
>> into platform_get_irq_byname_optional.
>
> What is duplicate error information?

 The function platform_get_irq_byname print:
 " dev_err(>dev, "IRQ %s not found\n", name);" if error occurred.

 In core.c we have the another error message below invoking this function.
 e.g
if (cdns->dev_irq < 0)
dev_err(dev, "couldn't get peripheral irq\n");

 So, it's looks like one dev_err is  redundant.
>>>
>>> If we want all 3 IRQs to be valid irrespective of dr_mode then we should
>>> use platform_get_irq_byname() and error out in probe if (ret < 0 && ret != 
>>> -EPROBE_DEFER).
>>>
>>> We can get rid of the irq check and duplicate error message in other places.
>>
>> To be sure we understand each other correctly.
>>
>> Are You suggesting  to leave the  platform_get_irq_byname()
>> and rid of from core.c the following lines :
>>
>> if (cdns->dev_irq < 0)
>>  dev_err(dev, "couldn't get peripheral irq\n");
>>
>> and
>>
>> dev_err(dev, "couldn't get otg irq\n");
>> ?
>
>Yes.
>
>>
>> A word of explanation why this patch has been sent.
>> During reviewing the cdnsp driver Chunfeng Yun add such comment:
>>
>> "
>>> +   cdns->dev_irq = platform_get_irq_byname(pdev, "peripheral");
>>> +   if (cdns->dev_irq == -EPROBE_DEFER)
>>> +   return cdns->dev_irq;
>>> +
>>> +   if (cdns->dev_irq < 0)
>>> +   dev_err(dev, "couldn't get peripheral irq\n");
>> Use platform_get_irq_byname_optional? otherwise no need print this log,
>> platform_get_irq_byname() will print it.
>> "
>>
>> In this patch I've chosen the platform_get_irq_byname_optional because both
>> function do the same but the error message from core.c tell us little more 
>> then
>> generic message from platform_get_irq_byname.
>
>Using platform_get_irq_byname_optional() says driver expects it is optional but
>only to fail later. It will be confusing to new reader that's all. I leave it 
>to
>you to decide what approach to take.

Thanks for clarification. 
I will send  new patch with platform_get_irq_byname.

Cheers,
Pawel

>
>>>

>
>>
>> A change was suggested during reviewing CDNSP driver by Chunfeng Yun.
>>
>> Signed-off-by: Pawel Laszczak 
>> ---
>> drivers/usb/cdns3/core.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>> index a0f73d4711ae..a3f6dc44cf3a 100644
>> --- a/drivers/usb/cdns3/core.c
>> +++ b/drivers/usb/cdns3/core.c
>> @@ -465,7 +465,7 @@ static int cdns3_probe(struct platform_device *pdev)
>>
>>  cdns->xhci_res[1] = *res;
>>
>> -cdns->dev_irq = platform_get_irq_byname(pdev, "peripheral");
>> +cdns->dev_irq = platform_get_irq_byname_optional(pdev, 
>> "peripheral");
>
> As per DT binding document, these are mandatory properties

 I think that name platform_get_irq_byname_optional is little confusing.
 Function descriptions show that both are almost identical:
 /**
* platform_get_irq_byname_optional - get an optional IRQ for a device 
 by name
* @dev: platform device
* @name: IRQ name
*
* Get an optional IRQ by name like platform_get_irq_byname(). Except 
 that it
* does not print an error message if an IRQ can not be obtained.
*
* Return: non-zero IRQ number on success, negative error number on 
 failure.
*/

>
>- interrupts: Interrupts used by cdns3 controller:
>   "host" - interrupt used by XHCI driver.
>   "peripheral" - interrupt used by device driver
>   "otg" - interrupt used by DRD/OTG  part of driver
>
> for dr_mode == "otg" -> all 3 are mandatory.
> for dr_mode == "host" -> "otg" and "peripheral" IRQs are not required.
> for dr_mode == "periphearal" -> "otg" and "host" IRQs are not required.
>
>>  if (cdns->dev_irq == -EPROBE_DEFER)
>>  return cdns->dev_irq;
>>
>> @@ -477,7 +477,7 @@ static int cdns3_probe(struct platform_device *pdev)
>>  return PTR_ERR(regs);
>>  cdns->dev_regs  = regs;
>>
>> -cdns->otg_irq = platform_get_irq_byname(pdev, "otg");
>> +cdns->otg_irq = platform_get_irq_byname_optional(pdev, "otg");
>>  if (cdns->otg_irq == -EPROBE_DEFER)
>>  return cdns->otg_irq;
>>
>>
>

 Regards,
 Pawel

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

Re: [PATCH] usb: cdns3: platform_get_irq_byname_optional instead platform_get_irq_byname

2020-10-05 Thread Roger Quadros

Pawel,

On 05/10/2020 08:54, Pawel Laszczak wrote:

Roger,


Pawel,

On 02/10/2020 12:08, Pawel Laszczak wrote:

Roger,



On 30/09/2020 09:57, Pawel Laszczak wrote:

To avoid duplicate error information patch replaces platform_get_irq_byname
into platform_get_irq_byname_optional.


What is duplicate error information?


The function platform_get_irq_byname print:
" dev_err(>dev, "IRQ %s not found\n", name);" if error occurred.

In core.c we have the another error message below invoking this function.
e.g
if (cdns->dev_irq < 0)
dev_err(dev, "couldn't get peripheral irq\n");

So, it's looks like one dev_err is  redundant.


If we want all 3 IRQs to be valid irrespective of dr_mode then we should
use platform_get_irq_byname() and error out in probe if (ret < 0 && ret != 
-EPROBE_DEFER).

We can get rid of the irq check and duplicate error message in other places.


To be sure we understand each other correctly.

Are You suggesting  to leave the  platform_get_irq_byname()
and rid of from core.c the following lines :

if (cdns->dev_irq < 0)
dev_err(dev, "couldn't get peripheral irq\n");

and

dev_err(dev, "couldn't get otg irq\n");
?


Yes.



A word of explanation why this patch has been sent.
During reviewing the cdnsp driver Chunfeng Yun add such comment:

"

+   cdns->dev_irq = platform_get_irq_byname(pdev, "peripheral");
+   if (cdns->dev_irq == -EPROBE_DEFER)
+   return cdns->dev_irq;
+
+   if (cdns->dev_irq < 0)
+   dev_err(dev, "couldn't get peripheral irq\n");

Use platform_get_irq_byname_optional? otherwise no need print this log,
platform_get_irq_byname() will print it.
"

In this patch I've chosen the platform_get_irq_byname_optional because both
function do the same but the error message from core.c tell us little more then
generic message from platform_get_irq_byname.


Using platform_get_irq_byname_optional() says driver expects it is optional but
only to fail later. It will be confusing to new reader that's all. I leave it to
you to decide what approach to take.

cheers,
-roger









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

Signed-off-by: Pawel Laszczak 
---
drivers/usb/cdns3/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
index a0f73d4711ae..a3f6dc44cf3a 100644
--- a/drivers/usb/cdns3/core.c
+++ b/drivers/usb/cdns3/core.c
@@ -465,7 +465,7 @@ static int cdns3_probe(struct platform_device *pdev)

cdns->xhci_res[1] = *res;

-   cdns->dev_irq = platform_get_irq_byname(pdev, "peripheral");
+   cdns->dev_irq = platform_get_irq_byname_optional(pdev, "peripheral");


As per DT binding document, these are mandatory properties


I think that name platform_get_irq_byname_optional is little confusing.
Function descriptions show that both are almost identical:
/**
   * platform_get_irq_byname_optional - get an optional IRQ for a device by name
   * @dev: platform device
   * @name: IRQ name
   *
   * Get an optional IRQ by name like platform_get_irq_byname(). Except that it
   * does not print an error message if an IRQ can not be obtained.
   *
   * Return: non-zero IRQ number on success, negative error number on failure.
   */



   - interrupts: Interrupts used by cdns3 controller:
  "host" - interrupt used by XHCI driver.
  "peripheral" - interrupt used by device driver
  "otg" - interrupt used by DRD/OTG  part of driver

for dr_mode == "otg" -> all 3 are mandatory.
for dr_mode == "host" -> "otg" and "peripheral" IRQs are not required.
for dr_mode == "periphearal" -> "otg" and "host" IRQs are not required.


if (cdns->dev_irq == -EPROBE_DEFER)
return cdns->dev_irq;

@@ -477,7 +477,7 @@ static int cdns3_probe(struct platform_device *pdev)
return PTR_ERR(regs);
cdns->dev_regs   = regs;

-   cdns->otg_irq = platform_get_irq_byname(pdev, "otg");
+   cdns->otg_irq = platform_get_irq_byname_optional(pdev, "otg");
if (cdns->otg_irq == -EPROBE_DEFER)
return cdns->otg_irq;






Regards,
Pawel



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


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


RE: [PATCH] usb: cdns3: platform_get_irq_byname_optional instead platform_get_irq_byname

2020-10-04 Thread Pawel Laszczak
Roger,
>
>Pawel,
>
>On 02/10/2020 12:08, Pawel Laszczak wrote:
>> Roger,
>>
>>>
>>> On 30/09/2020 09:57, Pawel Laszczak wrote:
 To avoid duplicate error information patch replaces platform_get_irq_byname
 into platform_get_irq_byname_optional.
>>>
>>> What is duplicate error information?
>>
>> The function platform_get_irq_byname print:
>> " dev_err(>dev, "IRQ %s not found\n", name);" if error occurred.
>>
>> In core.c we have the another error message below invoking this function.
>> e.g
>>  if (cdns->dev_irq < 0)
>>  dev_err(dev, "couldn't get peripheral irq\n");
>>
>> So, it's looks like one dev_err is  redundant.
>
>If we want all 3 IRQs to be valid irrespective of dr_mode then we should
>use platform_get_irq_byname() and error out in probe if (ret < 0 && ret != 
>-EPROBE_DEFER).
>
>We can get rid of the irq check and duplicate error message in other places.

To be sure we understand each other correctly.

Are You suggesting  to leave the  platform_get_irq_byname()
and rid of from core.c the following lines :

if (cdns->dev_irq < 0)
dev_err(dev, "couldn't get peripheral irq\n");

and

dev_err(dev, "couldn't get otg irq\n"); 
?

A word of explanation why this patch has been sent.
During reviewing the cdnsp driver Chunfeng Yun add such comment:

"
> + cdns->dev_irq = platform_get_irq_byname(pdev, "peripheral");
> + if (cdns->dev_irq == -EPROBE_DEFER)
> + return cdns->dev_irq;
> +
> + if (cdns->dev_irq < 0)
> + dev_err(dev, "couldn't get peripheral irq\n");
Use platform_get_irq_byname_optional? otherwise no need print this log,
platform_get_irq_byname() will print it. 
"

In this patch I've chosen the platform_get_irq_byname_optional because both
function do the same but the error message from core.c tell us little more then
generic message from platform_get_irq_byname.

Cheers
Pawel Laszczak

>
>cheers,
>-roger
>
>>
>>>

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

 Signed-off-by: Pawel Laszczak 
 ---
drivers/usb/cdns3/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
 index a0f73d4711ae..a3f6dc44cf3a 100644
 --- a/drivers/usb/cdns3/core.c
 +++ b/drivers/usb/cdns3/core.c
 @@ -465,7 +465,7 @@ static int cdns3_probe(struct platform_device *pdev)

cdns->xhci_res[1] = *res;

 -  cdns->dev_irq = platform_get_irq_byname(pdev, "peripheral");
 +  cdns->dev_irq = platform_get_irq_byname_optional(pdev, "peripheral");
>>>
>>> As per DT binding document, these are mandatory properties
>>
>> I think that name platform_get_irq_byname_optional is little confusing.
>> Function descriptions show that both are almost identical:
>> /**
>>   * platform_get_irq_byname_optional - get an optional IRQ for a device by 
>> name
>>   * @dev: platform device
>>   * @name: IRQ name
>>   *
>>   * Get an optional IRQ by name like platform_get_irq_byname(). Except that 
>> it
>>   * does not print an error message if an IRQ can not be obtained.
>>   *
>>   * Return: non-zero IRQ number on success, negative error number on failure.
>>   */
>>
>>>
>>>   - interrupts: Interrupts used by cdns3 controller:
>>>  "host" - interrupt used by XHCI driver.
>>>  "peripheral" - interrupt used by device driver
>>>  "otg" - interrupt used by DRD/OTG  part of driver
>>>
>>> for dr_mode == "otg" -> all 3 are mandatory.
>>> for dr_mode == "host" -> "otg" and "peripheral" IRQs are not required.
>>> for dr_mode == "periphearal" -> "otg" and "host" IRQs are not required.
>>>
if (cdns->dev_irq == -EPROBE_DEFER)
return cdns->dev_irq;

 @@ -477,7 +477,7 @@ static int cdns3_probe(struct platform_device *pdev)
return PTR_ERR(regs);
cdns->dev_regs  = regs;

 -  cdns->otg_irq = platform_get_irq_byname(pdev, "otg");
 +  cdns->otg_irq = platform_get_irq_byname_optional(pdev, "otg");
if (cdns->otg_irq == -EPROBE_DEFER)
return cdns->otg_irq;


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


Re: [PATCH] usb: cdns3: platform_get_irq_byname_optional instead platform_get_irq_byname

2020-10-02 Thread Roger Quadros

Pawel,

On 02/10/2020 12:08, Pawel Laszczak wrote:

Roger,



On 30/09/2020 09:57, Pawel Laszczak wrote:

To avoid duplicate error information patch replaces platform_get_irq_byname
into platform_get_irq_byname_optional.


What is duplicate error information?


The function platform_get_irq_byname print:
" dev_err(>dev, "IRQ %s not found\n", name);" if error occurred.

In core.c we have the another error message below invoking this function.
e.g
if (cdns->dev_irq < 0)
dev_err(dev, "couldn't get peripheral irq\n");

So, it's looks like one dev_err is  redundant.


If we want all 3 IRQs to be valid irrespective of dr_mode then we should
use platform_get_irq_byname() and error out in probe if (ret < 0 && ret != 
-EPROBE_DEFER).

We can get rid of the irq check and duplicate error message in other places.

cheers,
-roger







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

Signed-off-by: Pawel Laszczak 
---
   drivers/usb/cdns3/core.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
index a0f73d4711ae..a3f6dc44cf3a 100644
--- a/drivers/usb/cdns3/core.c
+++ b/drivers/usb/cdns3/core.c
@@ -465,7 +465,7 @@ static int cdns3_probe(struct platform_device *pdev)

cdns->xhci_res[1] = *res;

-   cdns->dev_irq = platform_get_irq_byname(pdev, "peripheral");
+   cdns->dev_irq = platform_get_irq_byname_optional(pdev, "peripheral");


As per DT binding document, these are mandatory properties


I think that name platform_get_irq_byname_optional is little confusing.
Function descriptions show that both are almost identical:
/**
  * platform_get_irq_byname_optional - get an optional IRQ for a device by name
  * @dev: platform device
  * @name: IRQ name
  *
  * Get an optional IRQ by name like platform_get_irq_byname(). Except that it
  * does not print an error message if an IRQ can not be obtained.
  *
  * Return: non-zero IRQ number on success, negative error number on failure.
  */



  - interrupts: Interrupts used by cdns3 controller:
 "host" - interrupt used by XHCI driver.
 "peripheral" - interrupt used by device driver
 "otg" - interrupt used by DRD/OTG  part of driver

for dr_mode == "otg" -> all 3 are mandatory.
for dr_mode == "host" -> "otg" and "peripheral" IRQs are not required.
for dr_mode == "periphearal" -> "otg" and "host" IRQs are not required.


if (cdns->dev_irq == -EPROBE_DEFER)
return cdns->dev_irq;

@@ -477,7 +477,7 @@ static int cdns3_probe(struct platform_device *pdev)
return PTR_ERR(regs);
cdns->dev_regs   = regs;

-   cdns->otg_irq = platform_get_irq_byname(pdev, "otg");
+   cdns->otg_irq = platform_get_irq_byname_optional(pdev, "otg");
if (cdns->otg_irq == -EPROBE_DEFER)
return cdns->otg_irq;






Regards,
Pawel



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


RE: [PATCH] usb: cdns3: platform_get_irq_byname_optional instead platform_get_irq_byname

2020-10-02 Thread Pawel Laszczak
Roger,

>
>On 30/09/2020 09:57, Pawel Laszczak wrote:
>> To avoid duplicate error information patch replaces platform_get_irq_byname
>> into platform_get_irq_byname_optional.
>
>What is duplicate error information?

The function platform_get_irq_byname print:
" dev_err(>dev, "IRQ %s not found\n", name);" if error occurred. 

In core.c we have the another error message below invoking this function.
e.g 
if (cdns->dev_irq < 0)
dev_err(dev, "couldn't get peripheral irq\n");

So, it's looks like one dev_err is  redundant.

>
>>
>> A change was suggested during reviewing CDNSP driver by Chunfeng Yun.
>>
>> Signed-off-by: Pawel Laszczak 
>> ---
>>   drivers/usb/cdns3/core.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>> index a0f73d4711ae..a3f6dc44cf3a 100644
>> --- a/drivers/usb/cdns3/core.c
>> +++ b/drivers/usb/cdns3/core.c
>> @@ -465,7 +465,7 @@ static int cdns3_probe(struct platform_device *pdev)
>>
>>  cdns->xhci_res[1] = *res;
>>
>> -cdns->dev_irq = platform_get_irq_byname(pdev, "peripheral");
>> +cdns->dev_irq = platform_get_irq_byname_optional(pdev, "peripheral");
>
>As per DT binding document, these are mandatory properties

I think that name platform_get_irq_byname_optional is little confusing. 
Function descriptions show that both are almost identical:
/**
 * platform_get_irq_byname_optional - get an optional IRQ for a device by name
 * @dev: platform device
 * @name: IRQ name
 *
 * Get an optional IRQ by name like platform_get_irq_byname(). Except that it
 * does not print an error message if an IRQ can not be obtained.
 *
 * Return: non-zero IRQ number on success, negative error number on failure.
 */

>
>  - interrupts: Interrupts used by cdns3 controller:
> "host" - interrupt used by XHCI driver.
> "peripheral" - interrupt used by device driver
> "otg" - interrupt used by DRD/OTG  part of driver
>
>for dr_mode == "otg" -> all 3 are mandatory.
>for dr_mode == "host" -> "otg" and "peripheral" IRQs are not required.
>for dr_mode == "periphearal" -> "otg" and "host" IRQs are not required.
>
>>  if (cdns->dev_irq == -EPROBE_DEFER)
>>  return cdns->dev_irq;
>>
>> @@ -477,7 +477,7 @@ static int cdns3_probe(struct platform_device *pdev)
>>  return PTR_ERR(regs);
>>  cdns->dev_regs  = regs;
>>
>> -cdns->otg_irq = platform_get_irq_byname(pdev, "otg");
>> +cdns->otg_irq = platform_get_irq_byname_optional(pdev, "otg");
>>  if (cdns->otg_irq == -EPROBE_DEFER)
>>  return cdns->otg_irq;
>>
>>
>

Regards,
Pawel


Re: [PATCH] usb: cdns3: platform_get_irq_byname_optional instead platform_get_irq_byname

2020-10-02 Thread Roger Quadros

Pawel,

On 30/09/2020 09:57, Pawel Laszczak wrote:

To avoid duplicate error information patch replaces platform_get_irq_byname
into platform_get_irq_byname_optional.


What is duplicate error information?



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

Signed-off-by: Pawel Laszczak 
---
  drivers/usb/cdns3/core.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
index a0f73d4711ae..a3f6dc44cf3a 100644
--- a/drivers/usb/cdns3/core.c
+++ b/drivers/usb/cdns3/core.c
@@ -465,7 +465,7 @@ static int cdns3_probe(struct platform_device *pdev)
  
  	cdns->xhci_res[1] = *res;
  
-	cdns->dev_irq = platform_get_irq_byname(pdev, "peripheral");

+   cdns->dev_irq = platform_get_irq_byname_optional(pdev, "peripheral");


As per DT binding document, these are mandatory properties

 - interrupts: Interrupts used by cdns3 controller:
"host" - interrupt used by XHCI driver.
"peripheral" - interrupt used by device driver
"otg" - interrupt used by DRD/OTG  part of driver

for dr_mode == "otg" -> all 3 are mandatory.
for dr_mode == "host" -> "otg" and "peripheral" IRQs are not required.
for dr_mode == "periphearal" -> "otg" and "host" IRQs are not required.


if (cdns->dev_irq == -EPROBE_DEFER)
return cdns->dev_irq;
  
@@ -477,7 +477,7 @@ static int cdns3_probe(struct platform_device *pdev)

return PTR_ERR(regs);
cdns->dev_regs   = regs;
  
-	cdns->otg_irq = platform_get_irq_byname(pdev, "otg");

+   cdns->otg_irq = platform_get_irq_byname_optional(pdev, "otg");
if (cdns->otg_irq == -EPROBE_DEFER)
return cdns->otg_irq;
  



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


RE: [PATCH] usb: cdns3: platform_get_irq_byname_optional instead platform_get_irq_byname

2020-09-30 Thread Pawel Laszczak
Hi,

My email server complains that delivery to peter.c...@nxp.org has failed. 

This address has generated by  get_maintainer.pl.  
I only updated email to correct:  peter.c...@nxp.com.

Regards,
Pawel

>To avoid duplicate error information patch replaces platform_get_irq_byname
>into platform_get_irq_byname_optional.
>
>A change was suggested during reviewing CDNSP driver by Chunfeng Yun.
>
>Signed-off-by: Pawel Laszczak 
>---
> drivers/usb/cdns3/core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>index a0f73d4711ae..a3f6dc44cf3a 100644
>--- a/drivers/usb/cdns3/core.c
>+++ b/drivers/usb/cdns3/core.c
>@@ -465,7 +465,7 @@ static int cdns3_probe(struct platform_device *pdev)
>
>   cdns->xhci_res[1] = *res;
>
>-  cdns->dev_irq = platform_get_irq_byname(pdev, "peripheral");
>+  cdns->dev_irq = platform_get_irq_byname_optional(pdev, "peripheral");
>   if (cdns->dev_irq == -EPROBE_DEFER)
>   return cdns->dev_irq;
>
>@@ -477,7 +477,7 @@ static int cdns3_probe(struct platform_device *pdev)
>   return PTR_ERR(regs);
>   cdns->dev_regs  = regs;
>
>-  cdns->otg_irq = platform_get_irq_byname(pdev, "otg");
>+  cdns->otg_irq = platform_get_irq_byname_optional(pdev, "otg");
>   if (cdns->otg_irq == -EPROBE_DEFER)
>   return cdns->otg_irq;
>
>--
>2.17.1