linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] USB: at91: fix the number of endpoint parameter
@ 2014-01-17  2:59 Bo Shen
  2014-01-17  2:59 ` [PATCH 2/2] USB: at91: using USBA_NR_DMAS for DMA channels Bo Shen
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Bo Shen @ 2014-01-17  2:59 UTC (permalink / raw)
  To: linux-arm-kernel

In sama5d3 SoC, there are 16 endpoints. As the USBA_NR_ENDPOINTS
is only 7. So, fix it for sama5d3 SoC using the udc->num_ep.

Signed-off-by: Bo Shen <voice.shen@atmel.com>
---

 drivers/usb/gadget/atmel_usba_udc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c
index 2cb52e0..7e67a81 100644
--- a/drivers/usb/gadget/atmel_usba_udc.c
+++ b/drivers/usb/gadget/atmel_usba_udc.c
@@ -1670,7 +1670,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
 	if (ep_status) {
 		int i;
 
-		for (i = 0; i < USBA_NR_ENDPOINTS; i++)
+		for (i = 0; i < udc->num_ep; i++)
 			if (ep_status & (1 << i)) {
 				if (ep_is_control(&udc->usba_ep[i]))
 					usba_control_irq(udc, &udc->usba_ep[i]);
-- 
1.8.5.2

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

* [PATCH 2/2] USB: at91: using USBA_NR_DMAS for DMA channels
  2014-01-17  2:59 [PATCH 1/2] USB: at91: fix the number of endpoint parameter Bo Shen
@ 2014-01-17  2:59 ` Bo Shen
  2014-01-17  9:29   ` Nicolas Ferre
  2014-02-18 16:19   ` Felipe Balbi
  2014-01-17  9:29 ` [PATCH 1/2] USB: at91: fix the number of endpoint parameter Nicolas Ferre
  2014-01-18  5:20 ` Jean-Christophe PLAGNIOL-VILLARD
  2 siblings, 2 replies; 14+ messages in thread
From: Bo Shen @ 2014-01-17  2:59 UTC (permalink / raw)
  To: linux-arm-kernel

When the SoC is earlier than sama5d3 SoC, which have the same number
endpoints and DMAs. However for sama5d3 SoC, it has different number
for endpoints and DMAs. So, define USBA_NR_DMAs for DMA channels

Signed-off-by: Bo Shen <voice.shen@atmel.com>
---

 drivers/usb/gadget/atmel_usba_udc.c | 2 +-
 drivers/usb/gadget/atmel_usba_udc.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c
index 7e67a81..5cded1c 100644
--- a/drivers/usb/gadget/atmel_usba_udc.c
+++ b/drivers/usb/gadget/atmel_usba_udc.c
@@ -1661,7 +1661,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
 	if (dma_status) {
 		int i;
 
-		for (i = 1; i < USBA_NR_ENDPOINTS; i++)
+		for (i = 1; i < USBA_NR_DMAS; i++)
 			if (dma_status & (1 << i))
 				usba_dma_irq(udc, &udc->usba_ep[i]);
 	}
diff --git a/drivers/usb/gadget/atmel_usba_udc.h b/drivers/usb/gadget/atmel_usba_udc.h
index 2922db5..a70706e 100644
--- a/drivers/usb/gadget/atmel_usba_udc.h
+++ b/drivers/usb/gadget/atmel_usba_udc.h
@@ -210,7 +210,7 @@
 #define USBA_FIFO_BASE(x)	((x) << 16)
 
 /* Synth parameters */
-#define USBA_NR_ENDPOINTS	7
+#define USBA_NR_DMAS		7
 
 #define EP0_FIFO_SIZE		64
 #define EP0_EPT_SIZE		USBA_EPT_SIZE_64
-- 
1.8.5.2

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

* [PATCH 1/2] USB: at91: fix the number of endpoint parameter
  2014-01-17  2:59 [PATCH 1/2] USB: at91: fix the number of endpoint parameter Bo Shen
  2014-01-17  2:59 ` [PATCH 2/2] USB: at91: using USBA_NR_DMAS for DMA channels Bo Shen
@ 2014-01-17  9:29 ` Nicolas Ferre
  2014-01-18  5:20 ` Jean-Christophe PLAGNIOL-VILLARD
  2 siblings, 0 replies; 14+ messages in thread
From: Nicolas Ferre @ 2014-01-17  9:29 UTC (permalink / raw)
  To: linux-arm-kernel

On 17/01/2014 03:59, Bo Shen :
> In sama5d3 SoC, there are 16 endpoints. As the USBA_NR_ENDPOINTS
> is only 7. So, fix it for sama5d3 SoC using the udc->num_ep.
> 
> Signed-off-by: Bo Shen <voice.shen@atmel.com>

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

> ---
> 
>  drivers/usb/gadget/atmel_usba_udc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c
> index 2cb52e0..7e67a81 100644
> --- a/drivers/usb/gadget/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/atmel_usba_udc.c
> @@ -1670,7 +1670,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
>  	if (ep_status) {
>  		int i;
>  
> -		for (i = 0; i < USBA_NR_ENDPOINTS; i++)
> +		for (i = 0; i < udc->num_ep; i++)
>  			if (ep_status & (1 << i)) {
>  				if (ep_is_control(&udc->usba_ep[i]))
>  					usba_control_irq(udc, &udc->usba_ep[i]);
> 


-- 
Nicolas Ferre

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

* [PATCH 2/2] USB: at91: using USBA_NR_DMAS for DMA channels
  2014-01-17  2:59 ` [PATCH 2/2] USB: at91: using USBA_NR_DMAS for DMA channels Bo Shen
@ 2014-01-17  9:29   ` Nicolas Ferre
  2014-02-18 16:19   ` Felipe Balbi
  1 sibling, 0 replies; 14+ messages in thread
From: Nicolas Ferre @ 2014-01-17  9:29 UTC (permalink / raw)
  To: linux-arm-kernel

On 17/01/2014 03:59, Bo Shen :
> When the SoC is earlier than sama5d3 SoC, which have the same number
> endpoints and DMAs. However for sama5d3 SoC, it has different number
> for endpoints and DMAs. So, define USBA_NR_DMAs for DMA channels
> 
> Signed-off-by: Bo Shen <voice.shen@atmel.com>

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

> ---
> 
>  drivers/usb/gadget/atmel_usba_udc.c | 2 +-
>  drivers/usb/gadget/atmel_usba_udc.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c
> index 7e67a81..5cded1c 100644
> --- a/drivers/usb/gadget/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/atmel_usba_udc.c
> @@ -1661,7 +1661,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
>  	if (dma_status) {
>  		int i;
>  
> -		for (i = 1; i < USBA_NR_ENDPOINTS; i++)
> +		for (i = 1; i < USBA_NR_DMAS; i++)
>  			if (dma_status & (1 << i))
>  				usba_dma_irq(udc, &udc->usba_ep[i]);
>  	}
> diff --git a/drivers/usb/gadget/atmel_usba_udc.h b/drivers/usb/gadget/atmel_usba_udc.h
> index 2922db5..a70706e 100644
> --- a/drivers/usb/gadget/atmel_usba_udc.h
> +++ b/drivers/usb/gadget/atmel_usba_udc.h
> @@ -210,7 +210,7 @@
>  #define USBA_FIFO_BASE(x)	((x) << 16)
>  
>  /* Synth parameters */
> -#define USBA_NR_ENDPOINTS	7
> +#define USBA_NR_DMAS		7
>  
>  #define EP0_FIFO_SIZE		64
>  #define EP0_EPT_SIZE		USBA_EPT_SIZE_64
> 


-- 
Nicolas Ferre

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

* [PATCH 1/2] USB: at91: fix the number of endpoint parameter
  2014-01-17  2:59 [PATCH 1/2] USB: at91: fix the number of endpoint parameter Bo Shen
  2014-01-17  2:59 ` [PATCH 2/2] USB: at91: using USBA_NR_DMAS for DMA channels Bo Shen
  2014-01-17  9:29 ` [PATCH 1/2] USB: at91: fix the number of endpoint parameter Nicolas Ferre
@ 2014-01-18  5:20 ` Jean-Christophe PLAGNIOL-VILLARD
  2014-01-20  3:39   ` Bo Shen
  2 siblings, 1 reply; 14+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2014-01-18  5:20 UTC (permalink / raw)
  To: linux-arm-kernel

On 10:59 Fri 17 Jan     , Bo Shen wrote:
> In sama5d3 SoC, there are 16 endpoints. As the USBA_NR_ENDPOINTS
> is only 7. So, fix it for sama5d3 SoC using the udc->num_ep.
> 
> Signed-off-by: Bo Shen <voice.shen@atmel.com>
> ---
> 
>  drivers/usb/gadget/atmel_usba_udc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c
> index 2cb52e0..7e67a81 100644
> --- a/drivers/usb/gadget/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/atmel_usba_udc.c
> @@ -1670,7 +1670,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
>  	if (ep_status) {
>  		int i;
>  
> -		for (i = 0; i < USBA_NR_ENDPOINTS; i++)
> +		for (i = 0; i < udc->num_ep; i++)

no the limit need to specified in the driver as a checkpoint by the compatible
or platform driver id

Best Regards,
J.
>  			if (ep_status & (1 << i)) {
>  				if (ep_is_control(&udc->usba_ep[i]))
>  					usba_control_irq(udc, &udc->usba_ep[i]);
> -- 
> 1.8.5.2
> 

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

* [PATCH 1/2] USB: at91: fix the number of endpoint parameter
  2014-01-18  5:20 ` Jean-Christophe PLAGNIOL-VILLARD
@ 2014-01-20  3:39   ` Bo Shen
  2014-01-21  5:49     ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 1 reply; 14+ messages in thread
From: Bo Shen @ 2014-01-20  3:39 UTC (permalink / raw)
  To: linux-arm-kernel

Hi J,

On 01/18/2014 01:20 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 10:59 Fri 17 Jan     , Bo Shen wrote:
>> In sama5d3 SoC, there are 16 endpoints. As the USBA_NR_ENDPOINTS
>> is only 7. So, fix it for sama5d3 SoC using the udc->num_ep.
>>
>> Signed-off-by: Bo Shen <voice.shen@atmel.com>
>> ---
>>
>>   drivers/usb/gadget/atmel_usba_udc.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c
>> index 2cb52e0..7e67a81 100644
>> --- a/drivers/usb/gadget/atmel_usba_udc.c
>> +++ b/drivers/usb/gadget/atmel_usba_udc.c
>> @@ -1670,7 +1670,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
>>   	if (ep_status) {
>>   		int i;
>>
>> -		for (i = 0; i < USBA_NR_ENDPOINTS; i++)
>> +		for (i = 0; i < udc->num_ep; i++)
>
> no the limit need to specified in the driver as a checkpoint by the compatible
> or platform driver id

You mean, we should not trust the data passed from dt node or platform 
data? Or do you think we should do double confirm?

> Best Regards,
> J.
>>   			if (ep_status & (1 << i)) {
>>   				if (ep_is_control(&udc->usba_ep[i]))
>>   					usba_control_irq(udc, &udc->usba_ep[i]);
>> --
>> 1.8.5.2
>>

Best Regards,
Bo Shen

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

* [PATCH 1/2] USB: at91: fix the number of endpoint parameter
  2014-01-20  3:39   ` Bo Shen
@ 2014-01-21  5:49     ` Jean-Christophe PLAGNIOL-VILLARD
  2014-01-21  8:12       ` Bo Shen
  0 siblings, 1 reply; 14+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2014-01-21  5:49 UTC (permalink / raw)
  To: linux-arm-kernel

On 11:39 Mon 20 Jan     , Bo Shen wrote:
> Hi J,
> 
> On 01/18/2014 01:20 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >On 10:59 Fri 17 Jan     , Bo Shen wrote:
> >>In sama5d3 SoC, there are 16 endpoints. As the USBA_NR_ENDPOINTS
> >>is only 7. So, fix it for sama5d3 SoC using the udc->num_ep.
> >>
> >>Signed-off-by: Bo Shen <voice.shen@atmel.com>
> >>---
> >>
> >>  drivers/usb/gadget/atmel_usba_udc.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >>diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c
> >>index 2cb52e0..7e67a81 100644
> >>--- a/drivers/usb/gadget/atmel_usba_udc.c
> >>+++ b/drivers/usb/gadget/atmel_usba_udc.c
> >>@@ -1670,7 +1670,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
> >>  	if (ep_status) {
> >>  		int i;
> >>
> >>-		for (i = 0; i < USBA_NR_ENDPOINTS; i++)
> >>+		for (i = 0; i < udc->num_ep; i++)
> >
> >no the limit need to specified in the driver as a checkpoint by the compatible
> >or platform driver id
> 
> You mean, we should not trust the data passed from dt node or
> platform data? Or do you think we should do double confirm?

no base on the driver name or the compatible you will known the MAX EP

not based on the dt ep description

as we do on pinctrl-at91

Best Regards,
J.
> >>  			if (ep_status & (1 << i)) {
> >>  				if (ep_is_control(&udc->usba_ep[i]))
> >>  					usba_control_irq(udc, &udc->usba_ep[i]);
> >>--
> >>1.8.5.2
> >>
> 
> Best Regards,
> Bo Shen

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

* [PATCH 1/2] USB: at91: fix the number of endpoint parameter
  2014-01-21  5:49     ` Jean-Christophe PLAGNIOL-VILLARD
@ 2014-01-21  8:12       ` Bo Shen
  2014-01-21  9:06         ` Nicolas Ferre
  0 siblings, 1 reply; 14+ messages in thread
From: Bo Shen @ 2014-01-21  8:12 UTC (permalink / raw)
  To: linux-arm-kernel

Hi J,

On 01/21/2014 01:49 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 11:39 Mon 20 Jan     , Bo Shen wrote:
>> Hi J,
>>
>> On 01/18/2014 01:20 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>> On 10:59 Fri 17 Jan     , Bo Shen wrote:
>>>> In sama5d3 SoC, there are 16 endpoints. As the USBA_NR_ENDPOINTS
>>>> is only 7. So, fix it for sama5d3 SoC using the udc->num_ep.
>>>>
>>>> Signed-off-by: Bo Shen <voice.shen@atmel.com>
>>>> ---
>>>>
>>>>   drivers/usb/gadget/atmel_usba_udc.c | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c
>>>> index 2cb52e0..7e67a81 100644
>>>> --- a/drivers/usb/gadget/atmel_usba_udc.c
>>>> +++ b/drivers/usb/gadget/atmel_usba_udc.c
>>>> @@ -1670,7 +1670,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
>>>>   	if (ep_status) {
>>>>   		int i;
>>>>
>>>> -		for (i = 0; i < USBA_NR_ENDPOINTS; i++)
>>>> +		for (i = 0; i < udc->num_ep; i++)
>>>
>>> no the limit need to specified in the driver as a checkpoint by the compatible
>>> or platform driver id
>>
>> You mean, we should not trust the data passed from dt node or
>> platform data? Or do you think we should do double confirm?
>
> no base on the driver name or the compatible you will known the MAX EP
>
> not based on the dt ep description
>
> as we do on pinctrl-at91

I am sorry, I am not fully get it after reading the code of 
pinctrl-at91.c, can you give the example code in pinctrl-at91.c?

Btw, the udc->num_ep is get from the following code.
for dt
--->8---
	while ((pp = of_get_next_child(np, pp)))
		udc->num_ep++;
---<8---

for non-dt
--->8---
	udc->num_ep = pdata->num_ep;
---8<---

> Best Regards,
> J.
>>>>   			if (ep_status & (1 << i)) {
>>>>   				if (ep_is_control(&udc->usba_ep[i]))
>>>>   					usba_control_irq(udc, &udc->usba_ep[i]);
>>>> --
>>>> 1.8.5.2
>>>>
>>
>> Best Regards,
>> Bo Shen

Best Regards,
Bo Shen

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

* [PATCH 1/2] USB: at91: fix the number of endpoint parameter
  2014-01-21  8:12       ` Bo Shen
@ 2014-01-21  9:06         ` Nicolas Ferre
  2014-02-07  8:37           ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 1 reply; 14+ messages in thread
From: Nicolas Ferre @ 2014-01-21  9:06 UTC (permalink / raw)
  To: linux-arm-kernel

On 21/01/2014 09:12, Bo Shen :
> Hi J,
> 
> On 01/21/2014 01:49 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>> On 11:39 Mon 20 Jan     , Bo Shen wrote:
>>> Hi J,
>>>
>>> On 01/18/2014 01:20 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>> On 10:59 Fri 17 Jan     , Bo Shen wrote:
>>>>> In sama5d3 SoC, there are 16 endpoints. As the USBA_NR_ENDPOINTS
>>>>> is only 7. So, fix it for sama5d3 SoC using the udc->num_ep.
>>>>>
>>>>> Signed-off-by: Bo Shen <voice.shen@atmel.com>
>>>>> ---
>>>>>
>>>>>   drivers/usb/gadget/atmel_usba_udc.c | 2 +-
>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c
>>>>> index 2cb52e0..7e67a81 100644
>>>>> --- a/drivers/usb/gadget/atmel_usba_udc.c
>>>>> +++ b/drivers/usb/gadget/atmel_usba_udc.c
>>>>> @@ -1670,7 +1670,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
>>>>>   	if (ep_status) {
>>>>>   		int i;
>>>>>
>>>>> -		for (i = 0; i < USBA_NR_ENDPOINTS; i++)
>>>>> +		for (i = 0; i < udc->num_ep; i++)
>>>>
>>>> no the limit need to specified in the driver as a checkpoint by the compatible
>>>> or platform driver id
>>>
>>> You mean, we should not trust the data passed from dt node or
>>> platform data? Or do you think we should do double confirm?
>>
>> no base on the driver name or the compatible you will known the MAX EP
>>
>> not based on the dt ep description
>>
>> as we do on pinctrl-at91
> 
> I am sorry, I am not fully get it after reading the code of 
> pinctrl-at91.c, can you give the example code in pinctrl-at91.c?
> 
> Btw, the udc->num_ep is get from the following code.
> for dt
> --->8---
> 	while ((pp = of_get_next_child(np, pp)))
> 		udc->num_ep++;
> ---<8---
> 
> for non-dt
> --->8---
> 	udc->num_ep = pdata->num_ep;
> ---8<---

It seems to me pretty valid to use num_ep in this driver and not have to
rely on another compatibility string just for this.
The information is here, it is retrieved pretty cleanly so I vote for a
simple use of it: if we introduce another information we will have to
double check the cross errors that would happen...

Bye,

>>>>>   			if (ep_status & (1 << i)) {
>>>>>   				if (ep_is_control(&udc->usba_ep[i]))
>>>>>   					usba_control_irq(udc, &udc->usba_ep[i]);
>>>>> --
>>>>> 1.8.5.2
>>>>>
>>>
>>> Best Regards,
>>> Bo Shen
> 
> Best Regards,
> Bo Shen
> 


-- 
Nicolas Ferre

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

* [PATCH 1/2] USB: at91: fix the number of endpoint parameter
  2014-01-21  9:06         ` Nicolas Ferre
@ 2014-02-07  8:37           ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 0 replies; 14+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2014-02-07  8:37 UTC (permalink / raw)
  To: linux-arm-kernel

On 10:06 Tue 21 Jan     , Nicolas Ferre wrote:
> On 21/01/2014 09:12, Bo Shen :
> > Hi J,
> > 
> > On 01/21/2014 01:49 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >> On 11:39 Mon 20 Jan     , Bo Shen wrote:
> >>> Hi J,
> >>>
> >>> On 01/18/2014 01:20 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >>>> On 10:59 Fri 17 Jan     , Bo Shen wrote:
> >>>>> In sama5d3 SoC, there are 16 endpoints. As the USBA_NR_ENDPOINTS
> >>>>> is only 7. So, fix it for sama5d3 SoC using the udc->num_ep.
> >>>>>
> >>>>> Signed-off-by: Bo Shen <voice.shen@atmel.com>
> >>>>> ---
> >>>>>
> >>>>>   drivers/usb/gadget/atmel_usba_udc.c | 2 +-
> >>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c
> >>>>> index 2cb52e0..7e67a81 100644
> >>>>> --- a/drivers/usb/gadget/atmel_usba_udc.c
> >>>>> +++ b/drivers/usb/gadget/atmel_usba_udc.c
> >>>>> @@ -1670,7 +1670,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
> >>>>>   	if (ep_status) {
> >>>>>   		int i;
> >>>>>
> >>>>> -		for (i = 0; i < USBA_NR_ENDPOINTS; i++)
> >>>>> +		for (i = 0; i < udc->num_ep; i++)
> >>>>
> >>>> no the limit need to specified in the driver as a checkpoint by the compatible
> >>>> or platform driver id
> >>>
> >>> You mean, we should not trust the data passed from dt node or
> >>> platform data? Or do you think we should do double confirm?
> >>
> >> no base on the driver name or the compatible you will known the MAX EP
> >>
> >> not based on the dt ep description
> >>
> >> as we do on pinctrl-at91
> > 
> > I am sorry, I am not fully get it after reading the code of 
> > pinctrl-at91.c, can you give the example code in pinctrl-at91.c?
> > 
> > Btw, the udc->num_ep is get from the following code.
> > for dt
> > --->8---
> > 	while ((pp = of_get_next_child(np, pp)))
> > 		udc->num_ep++;
> > ---<8---
> > 
> > for non-dt
> > --->8---
> > 	udc->num_ep = pdata->num_ep;
> > ---8<---
> 
> It seems to me pretty valid to use num_ep in this driver and not have to
> rely on another compatibility string just for this.
> The information is here, it is retrieved pretty cleanly so I vote for a
> simple use of it: if we introduce another information we will have to
> double check the cross errors that would happen...

here is the key point you describe the HW

so choose detect the IP version and then check we do not describe  too many EP
or you prove the correct compatible so we known this not the same IP
and we can check the DT information

Best Regards,
J.
> 
> Bye,
> 
> >>>>>   			if (ep_status & (1 << i)) {
> >>>>>   				if (ep_is_control(&udc->usba_ep[i]))
> >>>>>   					usba_control_irq(udc, &udc->usba_ep[i]);
> >>>>> --
> >>>>> 1.8.5.2
> >>>>>
> >>>
> >>> Best Regards,
> >>> Bo Shen
> > 
> > Best Regards,
> > Bo Shen
> > 
> 
> 
> -- 
> Nicolas Ferre

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

* [PATCH 2/2] USB: at91: using USBA_NR_DMAS for DMA channels
  2014-01-17  2:59 ` [PATCH 2/2] USB: at91: using USBA_NR_DMAS for DMA channels Bo Shen
  2014-01-17  9:29   ` Nicolas Ferre
@ 2014-02-18 16:19   ` Felipe Balbi
  2014-02-19  1:14     ` Bo Shen
  1 sibling, 1 reply; 14+ messages in thread
From: Felipe Balbi @ 2014-02-18 16:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jan 17, 2014 at 10:59:25AM +0800, Bo Shen wrote:
> When the SoC is earlier than sama5d3 SoC, which have the same number
> endpoints and DMAs. However for sama5d3 SoC, it has different number
> for endpoints and DMAs. So, define USBA_NR_DMAs for DMA channels
> 
> Signed-off-by: Bo Shen <voice.shen@atmel.com>
> ---
> 
>  drivers/usb/gadget/atmel_usba_udc.c | 2 +-
>  drivers/usb/gadget/atmel_usba_udc.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c
> index 7e67a81..5cded1c 100644
> --- a/drivers/usb/gadget/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/atmel_usba_udc.c
> @@ -1661,7 +1661,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
>  	if (dma_status) {
>  		int i;
>  
> -		for (i = 1; i < USBA_NR_ENDPOINTS; i++)
> +		for (i = 1; i < USBA_NR_DMAS; i++)
>  			if (dma_status & (1 << i))
>  				usba_dma_irq(udc, &udc->usba_ep[i]);
>  	}
> diff --git a/drivers/usb/gadget/atmel_usba_udc.h b/drivers/usb/gadget/atmel_usba_udc.h
> index 2922db5..a70706e 100644
> --- a/drivers/usb/gadget/atmel_usba_udc.h
> +++ b/drivers/usb/gadget/atmel_usba_udc.h
> @@ -210,7 +210,7 @@
>  #define USBA_FIFO_BASE(x)	((x) << 16)
>  
>  /* Synth parameters */
> -#define USBA_NR_ENDPOINTS	7
> +#define USBA_NR_DMAS		7

what's the difference ? You just renamed this macro. Also, please
clarify a bit your commit log.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140218/32376893/attachment.sig>

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

* [PATCH 2/2] USB: at91: using USBA_NR_DMAS for DMA channels
  2014-02-18 16:19   ` Felipe Balbi
@ 2014-02-19  1:14     ` Bo Shen
  2014-02-19  1:22       ` Felipe Balbi
  0 siblings, 1 reply; 14+ messages in thread
From: Bo Shen @ 2014-02-19  1:14 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Felipe Balbi,

On 02/19/2014 12:19 AM, Felipe Balbi wrote:
> On Fri, Jan 17, 2014 at 10:59:25AM +0800, Bo Shen wrote:
>> When the SoC is earlier than sama5d3 SoC, which have the same number
>> endpoints and DMAs. However for sama5d3 SoC, it has different number
>> for endpoints and DMAs. So, define USBA_NR_DMAs for DMA channels
>>
>> Signed-off-by: Bo Shen <voice.shen@atmel.com>
>> ---
>>
>>   drivers/usb/gadget/atmel_usba_udc.c | 2 +-
>>   drivers/usb/gadget/atmel_usba_udc.h | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c
>> index 7e67a81..5cded1c 100644
>> --- a/drivers/usb/gadget/atmel_usba_udc.c
>> +++ b/drivers/usb/gadget/atmel_usba_udc.c
>> @@ -1661,7 +1661,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
>>   	if (dma_status) {
>>   		int i;
>>
>> -		for (i = 1; i < USBA_NR_ENDPOINTS; i++)
>> +		for (i = 1; i < USBA_NR_DMAS; i++)
>>   			if (dma_status & (1 << i))
>>   				usba_dma_irq(udc, &udc->usba_ep[i]);
>>   	}
>> diff --git a/drivers/usb/gadget/atmel_usba_udc.h b/drivers/usb/gadget/atmel_usba_udc.h
>> index 2922db5..a70706e 100644
>> --- a/drivers/usb/gadget/atmel_usba_udc.h
>> +++ b/drivers/usb/gadget/atmel_usba_udc.h
>> @@ -210,7 +210,7 @@
>>   #define USBA_FIFO_BASE(x)	((x) << 16)
>>
>>   /* Synth parameters */
>> -#define USBA_NR_ENDPOINTS	7
>> +#define USBA_NR_DMAS		7
>
> what's the difference ? You just renamed this macro. Also, please
> clarify a bit your commit log.

As commit message said, the SoC before sama5d3, the endpoint number is 
the same as DMA channel number, so use endpoints definition for DMA 
channel number, however after sama5d3, the endpoints is not the same as 
DMA channel, so use DMA micro for DMA channels.

Best Regards,
Bo Shen

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

* [PATCH 2/2] USB: at91: using USBA_NR_DMAS for DMA channels
  2014-02-19  1:14     ` Bo Shen
@ 2014-02-19  1:22       ` Felipe Balbi
  2014-02-19  1:46         ` Bo Shen
  0 siblings, 1 reply; 14+ messages in thread
From: Felipe Balbi @ 2014-02-19  1:22 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Feb 19, 2014 at 09:14:58AM +0800, Bo Shen wrote:
> Hi Felipe Balbi,
> 
> On 02/19/2014 12:19 AM, Felipe Balbi wrote:
> >On Fri, Jan 17, 2014 at 10:59:25AM +0800, Bo Shen wrote:
> >>When the SoC is earlier than sama5d3 SoC, which have the same number
> >>endpoints and DMAs. However for sama5d3 SoC, it has different number
> >>for endpoints and DMAs. So, define USBA_NR_DMAs for DMA channels
> >>
> >>Signed-off-by: Bo Shen <voice.shen@atmel.com>
> >>---
> >>
> >>  drivers/usb/gadget/atmel_usba_udc.c | 2 +-
> >>  drivers/usb/gadget/atmel_usba_udc.h | 2 +-
> >>  2 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c
> >>index 7e67a81..5cded1c 100644
> >>--- a/drivers/usb/gadget/atmel_usba_udc.c
> >>+++ b/drivers/usb/gadget/atmel_usba_udc.c
> >>@@ -1661,7 +1661,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
> >>  	if (dma_status) {
> >>  		int i;
> >>
> >>-		for (i = 1; i < USBA_NR_ENDPOINTS; i++)
> >>+		for (i = 1; i < USBA_NR_DMAS; i++)
> >>  			if (dma_status & (1 << i))
> >>  				usba_dma_irq(udc, &udc->usba_ep[i]);
> >>  	}
> >>diff --git a/drivers/usb/gadget/atmel_usba_udc.h b/drivers/usb/gadget/atmel_usba_udc.h
> >>index 2922db5..a70706e 100644
> >>--- a/drivers/usb/gadget/atmel_usba_udc.h
> >>+++ b/drivers/usb/gadget/atmel_usba_udc.h
> >>@@ -210,7 +210,7 @@
> >>  #define USBA_FIFO_BASE(x)	((x) << 16)
> >>
> >>  /* Synth parameters */
> >>-#define USBA_NR_ENDPOINTS	7
> >>+#define USBA_NR_DMAS		7
> >
> >what's the difference ? You just renamed this macro. Also, please
> >clarify a bit your commit log.
> 
> As commit message said, the SoC before sama5d3, the endpoint number
> is the same as DMA channel number, so use endpoints definition for
> DMA channel number, however after sama5d3, the endpoints is not the
> same as DMA channel, so use DMA micro for DMA channels.

which means you're just renaming the macro for the sake of clarity.
That's fine, just needs to be clearer in commit message.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140218/995235a1/attachment-0001.sig>

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

* [PATCH 2/2] USB: at91: using USBA_NR_DMAS for DMA channels
  2014-02-19  1:22       ` Felipe Balbi
@ 2014-02-19  1:46         ` Bo Shen
  0 siblings, 0 replies; 14+ messages in thread
From: Bo Shen @ 2014-02-19  1:46 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/19/2014 09:22 AM, Felipe Balbi wrote:
> On Wed, Feb 19, 2014 at 09:14:58AM +0800, Bo Shen wrote:
>> Hi Felipe Balbi,
>>
>> On 02/19/2014 12:19 AM, Felipe Balbi wrote:
>>> On Fri, Jan 17, 2014 at 10:59:25AM +0800, Bo Shen wrote:
>>>> When the SoC is earlier than sama5d3 SoC, which have the same number
>>>> endpoints and DMAs. However for sama5d3 SoC, it has different number
>>>> for endpoints and DMAs. So, define USBA_NR_DMAs for DMA channels
>>>>
>>>> Signed-off-by: Bo Shen <voice.shen@atmel.com>
>>>> ---
>>>>
>>>>   drivers/usb/gadget/atmel_usba_udc.c | 2 +-
>>>>   drivers/usb/gadget/atmel_usba_udc.h | 2 +-
>>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c
>>>> index 7e67a81..5cded1c 100644
>>>> --- a/drivers/usb/gadget/atmel_usba_udc.c
>>>> +++ b/drivers/usb/gadget/atmel_usba_udc.c
>>>> @@ -1661,7 +1661,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
>>>>   	if (dma_status) {
>>>>   		int i;
>>>>
>>>> -		for (i = 1; i < USBA_NR_ENDPOINTS; i++)
>>>> +		for (i = 1; i < USBA_NR_DMAS; i++)
>>>>   			if (dma_status & (1 << i))
>>>>   				usba_dma_irq(udc, &udc->usba_ep[i]);
>>>>   	}
>>>> diff --git a/drivers/usb/gadget/atmel_usba_udc.h b/drivers/usb/gadget/atmel_usba_udc.h
>>>> index 2922db5..a70706e 100644
>>>> --- a/drivers/usb/gadget/atmel_usba_udc.h
>>>> +++ b/drivers/usb/gadget/atmel_usba_udc.h
>>>> @@ -210,7 +210,7 @@
>>>>   #define USBA_FIFO_BASE(x)	((x) << 16)
>>>>
>>>>   /* Synth parameters */
>>>> -#define USBA_NR_ENDPOINTS	7
>>>> +#define USBA_NR_DMAS		7
>>>
>>> what's the difference ? You just renamed this macro. Also, please
>>> clarify a bit your commit log.
>>
>> As commit message said, the SoC before sama5d3, the endpoint number
>> is the same as DMA channel number, so use endpoints definition for
>> DMA channel number, however after sama5d3, the endpoints is not the
>> same as DMA channel, so use DMA micro for DMA channels.
>
> which means you're just renaming the macro for the sake of clarity.
> That's fine, just needs to be clearer in commit message.

Thanks, I will send v2 to make the commit message more clearer.

Best Regards,
Bo Shen

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

end of thread, other threads:[~2014-02-19  1:46 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-17  2:59 [PATCH 1/2] USB: at91: fix the number of endpoint parameter Bo Shen
2014-01-17  2:59 ` [PATCH 2/2] USB: at91: using USBA_NR_DMAS for DMA channels Bo Shen
2014-01-17  9:29   ` Nicolas Ferre
2014-02-18 16:19   ` Felipe Balbi
2014-02-19  1:14     ` Bo Shen
2014-02-19  1:22       ` Felipe Balbi
2014-02-19  1:46         ` Bo Shen
2014-01-17  9:29 ` [PATCH 1/2] USB: at91: fix the number of endpoint parameter Nicolas Ferre
2014-01-18  5:20 ` Jean-Christophe PLAGNIOL-VILLARD
2014-01-20  3:39   ` Bo Shen
2014-01-21  5:49     ` Jean-Christophe PLAGNIOL-VILLARD
2014-01-21  8:12       ` Bo Shen
2014-01-21  9:06         ` Nicolas Ferre
2014-02-07  8:37           ` Jean-Christophe PLAGNIOL-VILLARD

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).