[PATCH linux-next v3 0/4] usb: gadget: udc: atmel: Endpoint allocation scheme fixes

[PATCH linux-next v3 0/4] usb: gadget: udc: atmel: Endpoint allocation scheme fixes

[cristian.birsan]

From: Cristian Birsan <cristian.birsan@microchip.com>

This patch series provides fixes, based on the feedback received on the mailing list, for
the following:
	- fifo table parameters validation against device tree values
	- coding style
	- message display for EP configuration error
	- Kconfig comments for fifo_mode=0

Changes since v1:
	- Removed static for usba_config_fifo_table() function from "Check fifo
	configuration values against device tree" patch

Changes since v2:
	- Use shorter warning message if the fifo table size is greather than device 
	tree value
	- replace dev_warn() with dev_err() for hardware fifo allocation error

Cristian Birsan (4):
  usb: gadget: udc: atmel: Check fifo configuration values against
    device tree
  usb: gadget: udc: atmel: Minor code cleanup
  usb: gadget: udc: atmel: Use dev_err() to display EP configuration
    error
  usb: gadget: udc: atmel: Update Kconfig help for fifo_mode = 0

 drivers/usb/gadget/udc/Kconfig          |  5 ++--
 drivers/usb/gadget/udc/atmel_usba_udc.c | 49 ++++++++++++++++++++++-----------
 2 files changed, 36 insertions(+), 18 deletions(-)

-- 
2.7.4

[PATCH linux-next v3 0/4] usb: gadget: udc: atmel: Endpoint allocation scheme fixes

[cristian.birsan at microchip.com]

From: Cristian Birsan <cristian.birsan@microchip.com>

This patch series provides fixes, based on the feedback received on the mailing list, for
the following:
	- fifo table parameters validation against device tree values
	- coding style
	- message display for EP configuration error
	- Kconfig comments for fifo_mode=0

Changes since v1:
	- Removed static for usba_config_fifo_table() function from "Check fifo
	configuration values against device tree" patch

Changes since v2:
	- Use shorter warning message if the fifo table size is greather than device 
	tree value
	- replace dev_warn() with dev_err() for hardware fifo allocation error

Cristian Birsan (4):
  usb: gadget: udc: atmel: Check fifo configuration values against
    device tree
  usb: gadget: udc: atmel: Minor code cleanup
  usb: gadget: udc: atmel: Use dev_err() to display EP configuration
    error
  usb: gadget: udc: atmel: Update Kconfig help for fifo_mode = 0

 drivers/usb/gadget/udc/Kconfig          |  5 ++--
 drivers/usb/gadget/udc/atmel_usba_udc.c | 49 ++++++++++++++++++++++-----------
 2 files changed, 36 insertions(+), 18 deletions(-)

-- 
2.7.4

[PATCH linux-next v3 1/4] usb: gadget: udc: atmel: Check fifo configuration values against device tree

[cristian.birsan]

From: Cristian Birsan <cristian.birsan@microchip.com>

Check fifo configuration values against device tree values for endpoint fifo
in auto configuration mode (fifo_mode=0).

Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
 drivers/usb/gadget/udc/atmel_usba_udc.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 2035906b..a98734a 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -2118,14 +2118,34 @@ static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
 			dev_err(&pdev->dev, "of_probe: fifo-size error(%d)\n", ret);
 			goto err;
 		}
-		ep->fifo_size = fifo_mode ? udc->fifo_cfg[i].fifo_size : val;
+		if (fifo_mode) {
+			if (val < udc->fifo_cfg[i].fifo_size) {
+				dev_warn(&pdev->dev,
+					 "Using max fifo-size value from DT\n");
+				ep->fifo_size = val;
+			} else {
+				ep->fifo_size = udc->fifo_cfg[i].fifo_size;
+			}
+		} else {
+			ep->fifo_size = val;
+		}
 
 		ret = of_property_read_u32(pp, "atmel,nb-banks", &val);
 		if (ret) {
 			dev_err(&pdev->dev, "of_probe: nb-banks error(%d)\n", ret);
 			goto err;
 		}
-		ep->nr_banks = fifo_mode ? udc->fifo_cfg[i].nr_banks : val;
+		if (fifo_mode) {
+			if (val < udc->fifo_cfg[i].nr_banks) {
+				dev_warn(&pdev->dev,
+					 "Using max nb-banks value from DT\n");
+				ep->nr_banks = val;
+			} else {
+				ep->nr_banks = udc->fifo_cfg[i].nr_banks;
+			}
+		} else {
+			ep->nr_banks = val;
+		}
 
 		ep->can_dma = of_property_read_bool(pp, "atmel,can-dma");
 		ep->can_isoc = of_property_read_bool(pp, "atmel,can-isoc");
-- 
2.7.4

[PATCH linux-next v3 1/4] usb: gadget: udc: atmel: Check fifo configuration values against device tree

[cristian.birsan at microchip.com]

From: Cristian Birsan <cristian.birsan@microchip.com>

Check fifo configuration values against device tree values for endpoint fifo
in auto configuration mode (fifo_mode=0).

Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
 drivers/usb/gadget/udc/atmel_usba_udc.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 2035906b..a98734a 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -2118,14 +2118,34 @@ static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
 			dev_err(&pdev->dev, "of_probe: fifo-size error(%d)\n", ret);
 			goto err;
 		}
-		ep->fifo_size = fifo_mode ? udc->fifo_cfg[i].fifo_size : val;
+		if (fifo_mode) {
+			if (val < udc->fifo_cfg[i].fifo_size) {
+				dev_warn(&pdev->dev,
+					 "Using max fifo-size value from DT\n");
+				ep->fifo_size = val;
+			} else {
+				ep->fifo_size = udc->fifo_cfg[i].fifo_size;
+			}
+		} else {
+			ep->fifo_size = val;
+		}
 
 		ret = of_property_read_u32(pp, "atmel,nb-banks", &val);
 		if (ret) {
 			dev_err(&pdev->dev, "of_probe: nb-banks error(%d)\n", ret);
 			goto err;
 		}
-		ep->nr_banks = fifo_mode ? udc->fifo_cfg[i].nr_banks : val;
+		if (fifo_mode) {
+			if (val < udc->fifo_cfg[i].nr_banks) {
+				dev_warn(&pdev->dev,
+					 "Using max nb-banks value from DT\n");
+				ep->nr_banks = val;
+			} else {
+				ep->nr_banks = udc->fifo_cfg[i].nr_banks;
+			}
+		} else {
+			ep->nr_banks = val;
+		}
 
 		ep->can_dma = of_property_read_bool(pp, "atmel,can-dma");
 		ep->can_isoc = of_property_read_bool(pp, "atmel,can-isoc");
-- 
2.7.4

[PATCH linux-next v3 2/4] usb: gadget: udc: atmel: Minor code cleanup

[cristian.birsan]

From: Cristian Birsan <cristian.birsan@microchip.com>

Minor code cleanup based on feedback received on mailinglist.

Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
 drivers/usb/gadget/udc/atmel_usba_udc.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index a98734a..8bc0b52a 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -321,7 +321,6 @@ static inline void usba_cleanup_debugfs(struct usba_udc *udc)
 
 static ushort fifo_mode;
 
-/* "modprobe ... fifo_mode=1" etc */
 module_param(fifo_mode, ushort, 0x0);
 MODULE_PARM_DESC(fifo_mode, "Endpoint configuration mode");
 
@@ -371,7 +370,7 @@ static struct usba_fifo_cfg mode_4_cfg[] = {
 };
 /* Add additional configurations here */
 
-int usba_config_fifo_table(struct usba_udc *udc)
+static int usba_config_fifo_table(struct usba_udc *udc)
 {
 	int n;
 
@@ -1076,11 +1075,9 @@ static int atmel_usba_start(struct usb_gadget *gadget,
 		struct usb_gadget_driver *driver);
 static int atmel_usba_stop(struct usb_gadget *gadget);
 
-static struct usb_ep *atmel_usba_match_ep(
-		struct usb_gadget		*gadget,
-		struct usb_endpoint_descriptor	*desc,
-		struct usb_ss_ep_comp_descriptor *ep_comp
-)
+static struct usb_ep *atmel_usba_match_ep(struct usb_gadget *gadget,
+				struct usb_endpoint_descriptor	*desc,
+				struct usb_ss_ep_comp_descriptor *ep_comp)
 {
 	struct usb_ep	*_ep;
 	struct usba_ep *ep;
@@ -1100,7 +1097,6 @@ static struct usb_ep *atmel_usba_match_ep(
 		ep = to_usba_ep(_ep);
 
 		switch (usb_endpoint_type(desc)) {
-
 		case USB_ENDPOINT_XFER_CONTROL:
 			break;
 
@@ -1141,7 +1137,7 @@ static struct usb_ep *atmel_usba_match_ep(
 		ep->udc->configured_ep++;
 	}
 
-return _ep;
+	return _ep;
 }
 
 static const struct usb_gadget_ops usba_udc_ops = {
@@ -2089,8 +2085,9 @@ static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
 		while ((pp = of_get_next_child(np, pp)))
 			udc->num_ep++;
 		udc->configured_ep = 1;
-	} else
+	} else {
 		udc->num_ep = usba_config_fifo_table(udc);
+	}
 
 	eps = devm_kzalloc(&pdev->dev, sizeof(struct usba_ep) * udc->num_ep,
 			   GFP_KERNEL);
-- 
2.7.4

[PATCH linux-next v3 2/4] usb: gadget: udc: atmel: Minor code cleanup

[cristian.birsan at microchip.com]

From: Cristian Birsan <cristian.birsan@microchip.com>

Minor code cleanup based on feedback received on mailinglist.

Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
 drivers/usb/gadget/udc/atmel_usba_udc.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index a98734a..8bc0b52a 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -321,7 +321,6 @@ static inline void usba_cleanup_debugfs(struct usba_udc *udc)
 
 static ushort fifo_mode;
 
-/* "modprobe ... fifo_mode=1" etc */
 module_param(fifo_mode, ushort, 0x0);
 MODULE_PARM_DESC(fifo_mode, "Endpoint configuration mode");
 
@@ -371,7 +370,7 @@ static struct usba_fifo_cfg mode_4_cfg[] = {
 };
 /* Add additional configurations here */
 
-int usba_config_fifo_table(struct usba_udc *udc)
+static int usba_config_fifo_table(struct usba_udc *udc)
 {
 	int n;
 
@@ -1076,11 +1075,9 @@ static int atmel_usba_start(struct usb_gadget *gadget,
 		struct usb_gadget_driver *driver);
 static int atmel_usba_stop(struct usb_gadget *gadget);
 
-static struct usb_ep *atmel_usba_match_ep(
-		struct usb_gadget		*gadget,
-		struct usb_endpoint_descriptor	*desc,
-		struct usb_ss_ep_comp_descriptor *ep_comp
-)
+static struct usb_ep *atmel_usba_match_ep(struct usb_gadget *gadget,
+				struct usb_endpoint_descriptor	*desc,
+				struct usb_ss_ep_comp_descriptor *ep_comp)
 {
 	struct usb_ep	*_ep;
 	struct usba_ep *ep;
@@ -1100,7 +1097,6 @@ static struct usb_ep *atmel_usba_match_ep(
 		ep = to_usba_ep(_ep);
 
 		switch (usb_endpoint_type(desc)) {
-
 		case USB_ENDPOINT_XFER_CONTROL:
 			break;
 
@@ -1141,7 +1137,7 @@ static struct usb_ep *atmel_usba_match_ep(
 		ep->udc->configured_ep++;
 	}
 
-return _ep;
+	return _ep;
 }
 
 static const struct usb_gadget_ops usba_udc_ops = {
@@ -2089,8 +2085,9 @@ static struct usba_ep * atmel_udc_of_init(struct platform_device *pdev,
 		while ((pp = of_get_next_child(np, pp)))
 			udc->num_ep++;
 		udc->configured_ep = 1;
-	} else
+	} else {
 		udc->num_ep = usba_config_fifo_table(udc);
+	}
 
 	eps = devm_kzalloc(&pdev->dev, sizeof(struct usba_ep) * udc->num_ep,
 			   GFP_KERNEL);
-- 
2.7.4

[PATCH linux-next v3 3/4] usb: gadget: udc: atmel: Use dev_err() to display EP configuration error

[cristian.birsan]

From: Cristian Birsan <cristian.birsan@microchip.com>

Use dev_err() to display EP configuration error to avoid silent failure.

Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
 drivers/usb/gadget/udc/atmel_usba_udc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 8bc0b52a..3ccc341 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -1851,8 +1851,8 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
 		 * but it's clearly harmless...
 		 */
 		if (!(usba_ep_readl(ep0, CFG) & USBA_EPT_MAPPED))
-			dev_dbg(&udc->pdev->dev,
-				 "ODD: EP0 configuration is invalid!\n");
+			dev_err(&udc->pdev->dev,
+				"ODD: EP0 configuration is invalid!\n");
 
 		/* Preallocate other endpoints */
 		n = fifo_mode ? udc->num_ep : udc->configured_ep;
@@ -1860,8 +1860,8 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
 			ep = &udc->usba_ep[i];
 			usba_ep_writel(ep, CFG, ep->ept_cfg);
 			if (!(usba_ep_readl(ep, CFG) & USBA_EPT_MAPPED))
-				dev_dbg(&udc->pdev->dev,
-				 "ODD: EP%d configuration is invalid!\n", i);
+				dev_err(&udc->pdev->dev,
+					"ODD: EP%d configuration is invalid!\n", i);
 		}
 	}
 
-- 
2.7.4

[PATCH linux-next v3 3/4] usb: gadget: udc: atmel: Use dev_err() to display EP configuration error

[cristian.birsan at microchip.com]

From: Cristian Birsan <cristian.birsan@microchip.com>

Use dev_err() to display EP configuration error to avoid silent failure.

Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
 drivers/usb/gadget/udc/atmel_usba_udc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 8bc0b52a..3ccc341 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -1851,8 +1851,8 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
 		 * but it's clearly harmless...
 		 */
 		if (!(usba_ep_readl(ep0, CFG) & USBA_EPT_MAPPED))
-			dev_dbg(&udc->pdev->dev,
-				 "ODD: EP0 configuration is invalid!\n");
+			dev_err(&udc->pdev->dev,
+				"ODD: EP0 configuration is invalid!\n");
 
 		/* Preallocate other endpoints */
 		n = fifo_mode ? udc->num_ep : udc->configured_ep;
@@ -1860,8 +1860,8 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
 			ep = &udc->usba_ep[i];
 			usba_ep_writel(ep, CFG, ep->ept_cfg);
 			if (!(usba_ep_readl(ep, CFG) & USBA_EPT_MAPPED))
-				dev_dbg(&udc->pdev->dev,
-				 "ODD: EP%d configuration is invalid!\n", i);
+				dev_err(&udc->pdev->dev,
+					"ODD: EP%d configuration is invalid!\n", i);
 		}
 	}
 
-- 
2.7.4

[PATCH linux-next v3 4/4] usb: gadget: udc: atmel: Update Kconfig help for fifo_mode = 0

[cristian.birsan]

From: Cristian Birsan <cristian.birsan@microchip.com>

Update Kconfig help for fifo_mode = 0 to explain the behavior better.

Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
 drivers/usb/gadget/udc/Kconfig | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
index c6cc9d3..1fd3fe9 100644
--- a/drivers/usb/gadget/udc/Kconfig
+++ b/drivers/usb/gadget/udc/Kconfig
@@ -62,8 +62,9 @@ config USB_ATMEL_USBA
 
 	  The fifo_mode parameter is used to select endpoint allocation mode.
 	  fifo_mode = 0 is used to let the driver autoconfigure the endpoints.
-	  In this case 2 banks are allocated for isochronous endpoints and
-	  only one bank is allocated for the rest of the endpoints.
+	  In this case, for ep1 2 banks are allocated if it works in isochronous
+	  mode and only 1 bank otherwise. For the rest of the endpoints
+	  only 1 bank is allocated.
 
 	  fifo_mode = 1 is a generic maximum fifo size (1024 bytes) configuration
 	  allowing the usage of ep1 - ep6
-- 
2.7.4

[PATCH linux-next v3 4/4] usb: gadget: udc: atmel: Update Kconfig help for fifo_mode = 0

[cristian.birsan at microchip.com]

From: Cristian Birsan <cristian.birsan@microchip.com>

Update Kconfig help for fifo_mode = 0 to explain the behavior better.

Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
 drivers/usb/gadget/udc/Kconfig | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
index c6cc9d3..1fd3fe9 100644
--- a/drivers/usb/gadget/udc/Kconfig
+++ b/drivers/usb/gadget/udc/Kconfig
@@ -62,8 +62,9 @@ config USB_ATMEL_USBA
 
 	  The fifo_mode parameter is used to select endpoint allocation mode.
 	  fifo_mode = 0 is used to let the driver autoconfigure the endpoints.
-	  In this case 2 banks are allocated for isochronous endpoints and
-	  only one bank is allocated for the rest of the endpoints.
+	  In this case, for ep1 2 banks are allocated if it works in isochronous
+	  mode and only 1 bank otherwise. For the rest of the endpoints
+	  only 1 bank is allocated.
 
 	  fifo_mode = 1 is a generic maximum fifo size (1024 bytes) configuration
 	  allowing the usage of ep1 - ep6
-- 
2.7.4

Re: [PATCH linux-next v3 2/4] usb: gadget: udc: atmel: Minor code cleanup

[Felipe Balbi]

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


Hi,

cristian.birsan@microchip.com writes:
> From: Cristian Birsan <cristian.birsan@microchip.com>
>
> Minor code cleanup based on feedback received on mailinglist.
>
> Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
> Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

patch 1 applied fine, patch 2 didn't. Please rebase on testing/next

-- 
balbi

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

[PATCH linux-next v3 2/4] usb: gadget: udc: atmel: Minor code cleanup

[Felipe Balbi]

Hi,

cristian.birsan at microchip.com writes:
> From: Cristian Birsan <cristian.birsan@microchip.com>
>
> Minor code cleanup based on feedback received on mailinglist.
>
> Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
> Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

patch 1 applied fine, patch 2 didn't. Please rebase on testing/next

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170405/23ce3984/attachment-0001.sig>

Re: [PATCH linux-next v3 2/4] usb: gadget: udc: atmel: Minor code cleanup

[m91496]

Hi,

The patch 2 does not apply because it's already on testing/next.

I'll send a new patch series version that will apply clean on top of testing branch.

Regards,
Cristi

On 05.04.2017 15:33, Felipe Balbi wrote:
> 
> Hi,
> 
> cristian.birsan@microchip.com writes:
>> From: Cristian Birsan <cristian.birsan@microchip.com>
>>
>> Minor code cleanup based on feedback received on mailinglist.
>>
>> Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
>> Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> 
> patch 1 applied fine, patch 2 didn't. Please rebase on testing/next
> 

[PATCH linux-next v3 2/4] usb: gadget: udc: atmel: Minor code cleanup

[m91496]

Hi,

The patch 2 does not apply because it's already on testing/next.

I'll send a new patch series version that will apply clean on top of testing branch.

Regards,
Cristi

On 05.04.2017 15:33, Felipe Balbi wrote:
> 
> Hi,
> 
> cristian.birsan at microchip.com writes:
>> From: Cristian Birsan <cristian.birsan@microchip.com>
>>
>> Minor code cleanup based on feedback received on mailinglist.
>>
>> Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
>> Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> 
> patch 1 applied fine, patch 2 didn't. Please rebase on testing/next
>