This patchset adds some OF related improvements suggested by Mark
Rutland.
This patchset also adds ep.maxpacket_limit to the debugfs file and adds
a check if gr_ep_enable is called with a maxpacket value greater than
ep.maxpacket_limit.
Andreas Larsson (6):
usb: gadget: gr_udc: Make struct platform_device variable name
clearer
usb: gadget: gr_udc: Expand devicetree documentation
usb: gadget: gr_udc: Use platform_get_irq instead of
irq_of_parse_and_map
usb: gadget: gr_udc: Use of_property_read_u32_index to access arrays
usb: gadget: gr_udc: Add ep.maxpacket_limit to debugfs information
usb: gadget: gr_udc: Return error code when trying to set
ep.maxpacket > ep.maxpacket_limit
Documentation/devicetree/bindings/usb/gr-udc.txt | 22 ++++++----
drivers/usb/gadget/gr_udc.c | 49 ++++++++++++----------
2 files changed, 40 insertions(+), 31 deletions(-)
--
1.7.10.4
Provide more information on the two different interrupt cases and more
information of endpoint buffer sizes. Suggested by Mark Rutland.
Signed-off-by: Andreas Larsson <[email protected]>
---
Documentation/devicetree/bindings/usb/gr-udc.txt | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/gr-udc.txt b/Documentation/devicetree/bindings/usb/gr-udc.txt
index 0c5118f..e944522 100644
--- a/Documentation/devicetree/bindings/usb/gr-udc.txt
+++ b/Documentation/devicetree/bindings/usb/gr-udc.txt
@@ -12,17 +12,23 @@ Required properties:
- reg : Address and length of the register set for the device
-- interrupts : Interrupt numbers for this device
+- interrupts : Interrupt numbers for this device. Either one interrupt number
+ for all interrupts, or one for status related interrupts, one for IN
+ endpoint related interrupts and one for OUT endpoint related interrupts.
Optional properties:
-- epobufsizes : An array of buffer sizes for OUT endpoints. If the property is
- not present, or for endpoints outside of the array, 1024 is assumed by
- the driver.
-
-- epibufsizes : An array of buffer sizes for IN endpoints. If the property is
- not present, or for endpoints outside of the array, 1024 is assumed by
- the driver.
+- epobufsizes : Array of buffer sizes for OUT endpoints when they differ
+ from the default size of 1024. The array is indexed by the OUT endpoint
+ number. If the property is present it typically contains one entry for
+ each OUT endpoint of the core. Fewer entries overrides the default sizes
+ only for as many endpoints as the array contains.
+
+- epibufsizes : Array of buffer sizes for IN endpoints when they differ
+ from the default size of 1024. The array is indexed by the IN endpoint
+ number. If the property is present it typically contains one entry for
+ each IN endpoint of the core. Fewer entries overrides the default sizes
+ only for as many endpoints as the array contains.
For further information look in the documentation for the GLIB IP core library:
http://www.gaisler.com/products/grlib/grip.pdf
--
1.7.10.4
Use platform_get_irq as no mapping needs to be done. No functional difference
for SPARC which is the typical environment for the driver though. Suggested by
Mark Rutland.
Signed-off-by: Andreas Larsson <[email protected]>
---
drivers/usb/gadget/gr_udc.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/gadget/gr_udc.c b/drivers/usb/gadget/gr_udc.c
index e66dcf0..e471db3 100644
--- a/drivers/usb/gadget/gr_udc.c
+++ b/drivers/usb/gadget/gr_udc.c
@@ -2114,20 +2114,22 @@ static int gr_probe(struct platform_device *pdev)
if (IS_ERR(regs))
return PTR_ERR(regs);
- dev->irq = irq_of_parse_and_map(dev->dev->of_node, 0);
- if (!dev->irq) {
+ dev->irq = platform_get_irq(pdev, 0);
+ if (dev->irq <= 0) {
dev_err(dev->dev, "No irq found\n");
return -ENODEV;
}
/* Some core configurations has separate irqs for IN and OUT events */
- dev->irqi = irq_of_parse_and_map(dev->dev->of_node, 1);
- if (dev->irqi) {
- dev->irqo = irq_of_parse_and_map(dev->dev->of_node, 2);
- if (!dev->irqo) {
+ dev->irqi = platform_get_irq(pdev, 1);
+ if (dev->irqi > 0) {
+ dev->irqo = platform_get_irq(pdev, 2);
+ if (dev->irqo <= 0) {
dev_err(dev->dev, "Found irqi but not irqo\n");
return -ENODEV;
}
+ } else {
+ dev->irqi = 0;
}
dev->gadget.name = driver_name;
--
1.7.10.4
Use an appropriate accessor function for property arrays to make the code nicer
and make the code correct if it would ever run on little endian architectures.
Suggested by Mark Rutland.
Signed-off-by: Andreas Larsson <[email protected]>
---
drivers/usb/gadget/gr_udc.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/gadget/gr_udc.c b/drivers/usb/gadget/gr_udc.c
index e471db3..8df35fc 100644
--- a/drivers/usb/gadget/gr_udc.c
+++ b/drivers/usb/gadget/gr_udc.c
@@ -2026,9 +2026,7 @@ static int gr_udc_init(struct gr_udc *dev)
u32 dmactrl_val;
int i;
int ret = 0;
- u32 *bufsizes;
u32 bufsize;
- int len;
gr_set_address(dev, 0);
@@ -2039,19 +2037,17 @@ static int gr_udc_init(struct gr_udc *dev)
INIT_LIST_HEAD(&dev->ep_list);
gr_set_ep0state(dev, GR_EP0_DISCONNECT);
- bufsizes = (u32 *)of_get_property(np, "epobufsizes", &len);
- len /= sizeof(u32);
for (i = 0; i < dev->nepo; i++) {
- bufsize = (bufsizes && i < len) ? bufsizes[i] : 1024;
+ if (of_property_read_u32_index(np, "epobufsizes", i, &bufsize))
+ bufsize = 1024;
ret = gr_ep_init(dev, i, 0, bufsize);
if (ret)
return ret;
}
- bufsizes = (u32 *)of_get_property(np, "epibufsizes", &len);
- len /= sizeof(u32);
for (i = 0; i < dev->nepi; i++) {
- bufsize = (bufsizes && i < len) ? bufsizes[i] : 1024;
+ if (of_property_read_u32_index(np, "epibufsizes", i, &bufsize))
+ bufsize = 1024;
ret = gr_ep_init(dev, i, 1, bufsize);
if (ret)
return ret;
--
1.7.10.4
Add information on ep.maxpacket_limit for each endpoint in the debugfs
information.
Signed-off-by: Andreas Larsson <[email protected]>
---
drivers/usb/gadget/gr_udc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/gadget/gr_udc.c b/drivers/usb/gadget/gr_udc.c
index 8df35fc..55757fc 100644
--- a/drivers/usb/gadget/gr_udc.c
+++ b/drivers/usb/gadget/gr_udc.c
@@ -143,6 +143,7 @@ static void gr_seq_ep_show(struct seq_file *seq, struct gr_ep *ep)
seq_printf(seq, " wedged = %d\n", ep->wedged);
seq_printf(seq, " callback = %d\n", ep->callback);
seq_printf(seq, " maxpacket = %d\n", ep->ep.maxpacket);
+ seq_printf(seq, " maxpacket_limit = %d\n", ep->ep.maxpacket_limit);
seq_printf(seq, " bytes_per_buffer = %d\n", ep->bytes_per_buffer);
if (mode == 1 || mode == 3)
seq_printf(seq, " nt = %d\n",
--
1.7.10.4
Rename struct platform_device pointers from ofdev to pdev for clarity.
Suggested by Mark Rutland.
Signed-off-by: Andreas Larsson <[email protected]>
---
drivers/usb/gadget/gr_udc.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/gadget/gr_udc.c b/drivers/usb/gadget/gr_udc.c
index 914cbd8..e66dcf0 100644
--- a/drivers/usb/gadget/gr_udc.c
+++ b/drivers/usb/gadget/gr_udc.c
@@ -2071,9 +2071,9 @@ static int gr_udc_init(struct gr_udc *dev)
return 0;
}
-static int gr_remove(struct platform_device *ofdev)
+static int gr_remove(struct platform_device *pdev)
{
- struct gr_udc *dev = dev_get_drvdata(&ofdev->dev);
+ struct gr_udc *dev = dev_get_drvdata(&pdev->dev);
if (dev->added)
usb_del_gadget_udc(&dev->gadget); /* Shuts everything down */
@@ -2083,7 +2083,7 @@ static int gr_remove(struct platform_device *ofdev)
gr_dfs_delete(dev);
if (dev->desc_pool)
dma_pool_destroy(dev->desc_pool);
- dev_set_drvdata(&ofdev->dev, NULL);
+ dev_set_drvdata(&pdev->dev, NULL);
gr_free_request(&dev->epi[0].ep, &dev->ep0reqi->req);
gr_free_request(&dev->epo[0].ep, &dev->ep0reqo->req);
@@ -2096,7 +2096,7 @@ static int gr_request_irq(struct gr_udc *dev, int irq)
IRQF_SHARED, driver_name, dev);
}
-static int gr_probe(struct platform_device *ofdev)
+static int gr_probe(struct platform_device *pdev)
{
struct gr_udc *dev;
struct resource *res;
@@ -2104,12 +2104,12 @@ static int gr_probe(struct platform_device *ofdev)
int retval;
u32 status;
- dev = devm_kzalloc(&ofdev->dev, sizeof(*dev), GFP_KERNEL);
+ dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
if (!dev)
return -ENOMEM;
- dev->dev = &ofdev->dev;
+ dev->dev = &pdev->dev;
- res = platform_get_resource(ofdev, IORESOURCE_MEM, 0);
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
regs = devm_ioremap_resource(dev->dev, res);
if (IS_ERR(regs))
return PTR_ERR(regs);
@@ -2138,7 +2138,7 @@ static int gr_probe(struct platform_device *ofdev)
spin_lock_init(&dev->lock);
dev->regs = regs;
- dev_set_drvdata(&ofdev->dev, dev);
+ dev_set_drvdata(&pdev->dev, dev);
/* Determine number of endpoints and data interface mode */
status = gr_read32(&dev->regs->status);
@@ -2210,7 +2210,7 @@ out:
spin_unlock(&dev->lock);
if (retval)
- gr_remove(ofdev);
+ gr_remove(pdev);
return retval;
}
--
1.7.10.4
Make gr_ep_enable fail properly when a call requests a larger ep.maxpacket than
ep.maxpacket_limit.
Signed-off-by: Andreas Larsson <[email protected]>
---
drivers/usb/gadget/gr_udc.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/usb/gadget/gr_udc.c b/drivers/usb/gadget/gr_udc.c
index 55757fc..5423391 100644
--- a/drivers/usb/gadget/gr_udc.c
+++ b/drivers/usb/gadget/gr_udc.c
@@ -1548,6 +1548,10 @@ static int gr_ep_enable(struct usb_ep *_ep,
} else if (max == 0) {
dev_err(dev->dev, "Max payload cannot be set to 0\n");
return -EINVAL;
+ } else if (max > ep->ep.maxpacket_limit) {
+ dev_err(dev->dev, "Requested max payload %d > limit %d\n",
+ max, ep->ep.maxpacket_limit);
+ return -EINVAL;
}
spin_lock(&ep->dev->lock);
--
1.7.10.4
On Thu, Jan 09, 2014 at 11:54:13AM +0100, Andreas Larsson wrote:
> Rename struct platform_device pointers from ofdev to pdev for clarity.
> Suggested by Mark Rutland.
>
> Signed-off-by: Andreas Larsson <[email protected]>
> ---
> drivers/usb/gadget/gr_udc.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/gadget/gr_udc.c b/drivers/usb/gadget/gr_udc.c
> index 914cbd8..e66dcf0 100644
> --- a/drivers/usb/gadget/gr_udc.c
> +++ b/drivers/usb/gadget/gr_udc.c
> @@ -2071,9 +2071,9 @@ static int gr_udc_init(struct gr_udc *dev)
> return 0;
> }
>
> -static int gr_remove(struct platform_device *ofdev)
> +static int gr_remove(struct platform_device *pdev)
> {
> - struct gr_udc *dev = dev_get_drvdata(&ofdev->dev);
> + struct gr_udc *dev = dev_get_drvdata(&pdev->dev);
you can use platform_get_drvdata()
> @@ -2083,7 +2083,7 @@ static int gr_remove(struct platform_device *ofdev)
> gr_dfs_delete(dev);
> if (dev->desc_pool)
> dma_pool_destroy(dev->desc_pool);
> - dev_set_drvdata(&ofdev->dev, NULL);
> + dev_set_drvdata(&pdev->dev, NULL);
and platform_set_drvdata()
--
balbi
On 2014-02-18 16:52, Felipe Balbi wrote:
> On Thu, Jan 09, 2014 at 11:54:13AM +0100, Andreas Larsson wrote:
>> Rename struct platform_device pointers from ofdev to pdev for clarity.
>> Suggested by Mark Rutland.
>>
>> Signed-off-by: Andreas Larsson <[email protected]>
>> ---
>> drivers/usb/gadget/gr_udc.c | 18 +++++++++---------
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/gr_udc.c b/drivers/usb/gadget/gr_udc.c
>> index 914cbd8..e66dcf0 100644
>> --- a/drivers/usb/gadget/gr_udc.c
>> +++ b/drivers/usb/gadget/gr_udc.c
>> @@ -2071,9 +2071,9 @@ static int gr_udc_init(struct gr_udc *dev)
>> return 0;
>> }
>>
>> -static int gr_remove(struct platform_device *ofdev)
>> +static int gr_remove(struct platform_device *pdev)
>> {
>> - struct gr_udc *dev = dev_get_drvdata(&ofdev->dev);
>> + struct gr_udc *dev = dev_get_drvdata(&pdev->dev);
>
> you can use platform_get_drvdata()
>
>> @@ -2083,7 +2083,7 @@ static int gr_remove(struct platform_device *ofdev)
>> gr_dfs_delete(dev);
>> if (dev->desc_pool)
>> dma_pool_destroy(dev->desc_pool);
>> - dev_set_drvdata(&ofdev->dev, NULL);
>> + dev_set_drvdata(&pdev->dev, NULL);
>
> and platform_set_drvdata()
>
Yes, but wouldn't that be better handled in a separate patch?
Best regards,
Andreas Larsson
On Mon, Feb 24, 2014 at 08:40:17AM +0100, Andreas Larsson wrote:
> On 2014-02-18 16:52, Felipe Balbi wrote:
> >On Thu, Jan 09, 2014 at 11:54:13AM +0100, Andreas Larsson wrote:
> >>Rename struct platform_device pointers from ofdev to pdev for clarity.
> >>Suggested by Mark Rutland.
> >>
> >>Signed-off-by: Andreas Larsson <[email protected]>
> >>---
> >> drivers/usb/gadget/gr_udc.c | 18 +++++++++---------
> >> 1 file changed, 9 insertions(+), 9 deletions(-)
> >>
> >>diff --git a/drivers/usb/gadget/gr_udc.c b/drivers/usb/gadget/gr_udc.c
> >>index 914cbd8..e66dcf0 100644
> >>--- a/drivers/usb/gadget/gr_udc.c
> >>+++ b/drivers/usb/gadget/gr_udc.c
> >>@@ -2071,9 +2071,9 @@ static int gr_udc_init(struct gr_udc *dev)
> >> return 0;
> >> }
> >>
> >>-static int gr_remove(struct platform_device *ofdev)
> >>+static int gr_remove(struct platform_device *pdev)
> >> {
> >>- struct gr_udc *dev = dev_get_drvdata(&ofdev->dev);
> >>+ struct gr_udc *dev = dev_get_drvdata(&pdev->dev);
> >
> >you can use platform_get_drvdata()
> >
> >>@@ -2083,7 +2083,7 @@ static int gr_remove(struct platform_device *ofdev)
> >> gr_dfs_delete(dev);
> >> if (dev->desc_pool)
> >> dma_pool_destroy(dev->desc_pool);
> >>- dev_set_drvdata(&ofdev->dev, NULL);
> >>+ dev_set_drvdata(&pdev->dev, NULL);
> >
> >and platform_set_drvdata()
> >
>
> Yes, but wouldn't that be better handled in a separate patch?
not necessary, you're adding the platform_device argument to this patch
anyway.
--
balbi