On Mon, Aug 20, 2012 at 06:49:36PM -0700, Kevin Cernekee wrote:
> Driver for the "USB20D" / "USBD" block on BCM6328, BCM6368, BCM6816,
> BCM6362, BCM3383, and others.
looks good btw.
> diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
> index 51ab5fd..01efd9d 100644
> --- a/drivers/usb/gadget/Kconfig
> +++ b/drivers/usb/gadget/Kconfig
> @@ -160,6 +160,19 @@ config USB_ATMEL_USBA
> USBA is the integrated high-speed USB Device controller on
> the AT32AP700x, some AT91SAM9 and AT91CAP9 processors from Atmel.
>
> +config USB_BCM63XX_UDC
> + tristate "Broadcom BCM63xx Peripheral Controller"
> + select USB_GADGET_DUALSPEED
> + depends on BCM63XX
> + help
> + Many Broadcom BCM63xx chipsets (such as the BCM6328) have a
> + high speed USB Device Port with support for four fixed endpoints
> + (plus endpoint zero).
> +
> + Say "y" to link the driver statically, or "m" to build a
> + dynamically linked module called "bcm63xx_udc" and force
> + all gadget drivers to also be dynamically linked.
please drop the "force all gadget drivers to also be dynamically linked" part.
We may want to change this one day :)
> +
> config USB_FSL_USB2
> tristate "Freescale Highspeed USB DR Peripheral Controller"
> depends on FSL_SOC || ARCH_MXC
> diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
> index 3fd8cd0..d84f923 100644
> diff --git a/drivers/usb/gadget/bcm63xx_udc.c b/drivers/usb/gadget/bcm63xx_udc.c
> new file mode 100644
> index 0000000..a44352b
> --- /dev/null
> +++ b/drivers/usb/gadget/bcm63xx_udc.c
...
> +static bool use_fullspeed;
> +module_param(use_fullspeed, bool, S_IRUGO);
> +MODULE_PARM_DESC(use_fullspeed, "true for fullspeed only");
How important is this option? Maybe this should become a generic option?
> +static int bcm63xx_udc_start(struct usb_gadget *gadget,
> + struct usb_gadget_driver *driver)
> +{
> + struct bcm63xx_udc *udc = gadget_to_udc(gadget);
> + unsigned long flags;
> +
> + if (!driver || driver->max_speed < USB_SPEED_HIGH ||
Hmm. But if you set use_fullspeed isn't this kinda legal?
> +static int __devinit bcm63xx_udc_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct bcm63xx_usbd_platform_data *pd = dev->platform_data;
> + struct bcm63xx_udc *udc;
> + struct resource *res;
> + int rc = -ENOMEM, i, irq;
> +
> + udc = devm_kzalloc(dev, sizeof(*udc), GFP_KERNEL);
> + if (!udc) {
> + dev_err(dev, "cannot allocate memory\n");
> + return -ENOMEM;
> + }
> +
> + platform_set_drvdata(pdev, udc);
> + udc->dev = dev;
> + udc->pd = pd;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(dev, "error finding USBD resource\n");
> + return -ENXIO;
> + }
> + udc->usbd_regs = devm_request_and_ioremap(dev, res);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + if (!res) {
> + dev_err(dev, "error finding IUDMA resource\n");
> + return -ENXIO;
> + }
> + udc->iudma_regs = devm_request_and_ioremap(dev, res);
> +
> + if (!udc->usbd_regs || !udc->iudma_regs) {
> + dev_err(dev, "error requesting resources\n");
> + return -ENXIO;
> + }
> +
> + spin_lock_init(&udc->lock);
> + dev_set_name(&udc->gadget.dev, "gadget");
> +
> + udc->gadget.ops = &bcm63xx_udc_ops;
> + udc->gadget.name = dev_name(dev);
> + udc->gadget.dev.parent = dev;
> + udc->gadget.dev.release = bcm63xx_udc_gadget_release;
> + udc->gadget.dev.dma_mask = dev->dma_mask;
> +
> + if (!pd->use_fullspeed && !use_fullspeed)
so it is a good advice to have pd not set to NULL :)
Sebastian
