Hi,
Apologies for the late reply, I wasn't able to access my mail much over
the Christmas break.
On Mon, Dec 23, 2013 at 08:25:49PM +0000, Andreas Larsson wrote:This adds an UDC driver for GRUSBDC USB Device Controller cores available in the
GRLIB VHDL IP core library. The driver only supports DMA mode.
Signed-off-by: Andreas Larsson <andreas@xxxxxxxxxxx>
---
Differences since v1:
- Use hexprint for debug printoutes instead of homemade equivalent
- Use the dev_* printk variants over the board
- Get rid of unnecessary casts and includes
- Use USB_STATE_NOTATTACHED instead of USB_STATE_ATTACHED for clarity
- Get rid of commented out VERBOSE_DEBUG definition
- Do not devm-allocate any requests
- Get rid of unnecessary resqueduling of the workqueue handler
- Make sure that gadget setup function is called with interrupts disabled
Differences since v2:
- Fixed an error printout
- Got rid of the work queue in favor of threaded interrupts
Differences since v3:
- Disable interrupts when locking spinlocks instead of using
local_irq_save deeper within critical section
Differences since v4:
- Set quirk_ep_out_aligned_size
- Use usb_ep_set_maxpacket_limit
- Add devicetree documentation
Differences since v5:
- Declare unexpected spin_unlock and spin_lock for sparse
- Fix a bad comment
- Use ACCESS_ONCE instead of gr_read32 when checking for update of dma descriptor
Documentation/devicetree/bindings/usb/gr-udc.txt | 28 +
drivers/usb/gadget/Kconfig | 7 +
drivers/usb/gadget/Makefile | 1 +
drivers/usb/gadget/gr_udc.c | 2242 ++++++++++++++++++++++
drivers/usb/gadget/gr_udc.h | 220 +++
5 files changed, 2498 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/gr-udc.txt
create mode 100644 drivers/usb/gadget/gr_udc.c
create mode 100644 drivers/usb/gadget/gr_udc.h
diff --git a/Documentation/devicetree/bindings/usb/gr-udc.txt b/Documentation/devicetree/bindings/usb/gr-udc.txt
new file mode 100644
index 0000000..0c5118f
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/gr-udc.txt
@@ -0,0 +1,28 @@
+USB Peripheral Controller driver for Aeroflex Gaisler GRUSBDC.
+
+The GRUSBDC USB Device Controller core is available in the GRLIB VHDL
+IP core library.
+
+Note: In the ordinary environment for the core, a Leon SPARC system,
+these properties are built from information in the AMBA plug&play.
+
+Required properties:
+
+- name : Should be "GAISLER_USBDC" or "01_021"
What's this for?
Why is this not matched using a compatible string?
What does "01_021" mean?
+
+- reg : Address and length of the register set for the device
+
+- interrupts : Interrupt numbers for this device
How many, and what do they correspond to?
+
+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.
These names are rather opaque. Something like {input,output}-buf-lens
would be far clearer.
How many entries are expected?
A specific driver should have no relevance to the binding. Just say "if
not 1024".
[...]
+/* Must be called with dev->lock held */
+static int gr_udc_init(struct gr_udc *dev)
+{
+ struct device_node *np = dev->dev->of_node;
+ u32 epctrl_val;
+ u32 dmactrl_val;
+ int i;
+ int ret = 0;
+ u32 *bufsizes;
+ u32 bufsize;
+ int len;
+
+ gr_set_address(dev, 0);
+
+ INIT_LIST_HEAD(&dev->gadget.ep_list);
+ dev->gadget.speed = USB_SPEED_UNKNOWN;
+ dev->gadget.ep0 = &dev->epi[0].ep;
+
+ INIT_LIST_HEAD(&dev->ep_list);
+ gr_set_ep0state(dev, GR_EP0_DISCONNECT);
+
+ bufsizes = (u32 *)of_get_property(np, "epobufsizes", &len);
of_get_property gives you the raw property value bye string, which is
_not_ a u32 pointer -- it's not necessarily the same endianness as the
kernel. Please use an appropriate accessor.
+ len /= sizeof(u32);
+ for (i = 0; i < dev->nepo; i++) {
+ bufsize = (bufsizes && i < len) ? bufsizes[i] : 1024;
You can use of_property_read_u32_index within the loop for this:
if (of_property_read_u32_index(np, "epobufsizes", &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;
Likewise here.
[...]
+static int gr_probe(struct platform_device *ofdev)
+{
+ struct gr_udc *dev;
+ struct resource *res;
+ struct gr_regs __iomem *regs;
+ int retval;
+ u32 status;
+
+ dev = devm_kzalloc(&ofdev->dev, sizeof(*dev), GFP_KERNEL);
+ if (!dev)
+ return -ENOMEM;
+ dev->dev = &ofdev->dev;
+
+ res = platform_get_resource(ofdev, IORESOURCE_MEM, 0);
+ regs = devm_ioremap_resource(dev->dev, res);
+ if (IS_ERR(regs))
+ return PTR_ERR(regs);
+
+ dev->irq = irq_of_parse_and_map(dev->dev->of_node, 0);
+ if (!dev->irq) {
+ dev_err(dev->dev, "No irq found\n");
+ return -ENODEV;
+ }
The platform_device probing code will have parsed and mapped the irq
already. You can use platform_get_irq(ofdev->dev, 0) here.
Also, naming the platform_device ofdev is confusing. It has nothing to
do with OF, and the more common pdev name would be far clearer.
+
+ /* 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_err(dev->dev, "Found irqi but not irqo\n");
+ return -ENODEV;
+ }
+ }
Likewise here you can use platform_get_irq.
[...]
+static struct of_device_id gr_match[] = {
+ {.name = "GAISLER_USBDC"},
+ {.name = "01_021"},
+ {},
This seems extremely odd to me. I think a compatible string would be far
preferable.
Thanks,
Mark.