[PATCH v4 0/7] usb: gadget: atmel: Code refactor and DM_USB_GADGET support
Changes in v4: - Release clocks if probe failed - Add missing endpoint data free - Addressed comments
Changes in v3: - Separate code refactor into individual commits - Extract the controller point from udevice private data in DM functions
Changes in v2: - Fix null pointer deference in driver unbinding - Separate code refactor into 2 parts - Remove dead code
Changes in v1: - Based on [PATCH RFC] usb: gadget: atmel: Add DM_USB_GADGET support: https://lists.denx.de/pipermail/u-boot/2024-July/559503.html - Addressed comments, moved the refactoring to a preparatory patch.
Zixun LI (7): usb: gadget: atmel: Sort includes usb: gadget: atmel: Replace printf() and pr_err() by log_err() usb: gadget: atmel: Fix typo in usb gadget driver register and unregister usb: gadget: atmel: Move usba_udc_pdata() with other static functions usb: gadget: atmel: Rename atmel_usba_start()/_stop() to usba_udc_enable()/_disable() usb: gadget: atmel: Add attach/detach support usb: gadget: atmel: Add DM_USB_GADGET support
drivers/usb/gadget/atmel_usba_udc.c | 256 +++++++++++++++++++++++----- drivers/usb/gadget/atmel_usba_udc.h | 3 + include/linux/usb/atmel_usba_udc.h | 2 + 3 files changed, 214 insertions(+), 47 deletions(-)
-- 2.45.2
Sort includes in alphabetical order.
Signed-off-by: Zixun LI admin@hifiphile.com --- drivers/usb/gadget/atmel_usba_udc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c index f99553df8d..5f78251fdb 100644 --- a/drivers/usb/gadget/atmel_usba_udc.c +++ b/drivers/usb/gadget/atmel_usba_udc.c @@ -7,16 +7,16 @@ * Bo Shen voice.shen@atmel.com */
-#include <linux/bitops.h> -#include <linux/errno.h> +#include <malloc.h> #include <asm/gpio.h> #include <asm/hardware.h> +#include <linux/bitops.h> +#include <linux/errno.h> #include <linux/list.h> #include <linux/printk.h> #include <linux/usb/ch9.h> #include <linux/usb/gadget.h> #include <linux/usb/atmel_usba_udc.h> -#include <malloc.h>
#include "atmel_usba_udc.h"
To have a uniform printing function, also drop linux/printk.h as no longer used.
Signed-off-by: Zixun LI admin@hifiphile.com --- drivers/usb/gadget/atmel_usba_udc.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c index 5f78251fdb..4641638412 100644 --- a/drivers/usb/gadget/atmel_usba_udc.c +++ b/drivers/usb/gadget/atmel_usba_udc.c @@ -7,13 +7,13 @@ * Bo Shen voice.shen@atmel.com */
+#include <log.h> #include <malloc.h> #include <asm/gpio.h> #include <asm/hardware.h> #include <linux/bitops.h> #include <linux/errno.h> #include <linux/list.h> -#include <linux/printk.h> #include <linux/usb/ch9.h> #include <linux/usb/gadget.h> #include <linux/usb/atmel_usba_udc.h> @@ -1204,12 +1204,12 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver) int ret;
if (!driver || !driver->bind || !driver->setup) { - printf("bad paramter\n"); + log_err("bad paramter\n"); return -EINVAL; }
if (udc->driver) { - printf("UDC already has a gadget driver\n"); + log_err("UDC already has a gadget driver\n"); return -EBUSY; }
@@ -1219,7 +1219,7 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver)
ret = driver->bind(&udc->gadget); if (ret) { - pr_err("driver->bind() returned %d\n", ret); + log_err("driver->bind() returned %d\n", ret); udc->driver = NULL; }
@@ -1231,7 +1231,7 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver) struct usba_udc *udc = &controller;
if (!driver || !driver->unbind || !driver->disconnect) { - pr_err("bad paramter\n"); + log_err("bad paramter\n"); return -EINVAL; }
@@ -1252,7 +1252,7 @@ static struct usba_ep *usba_udc_pdata(struct usba_platform_data *pdata,
eps = malloc(sizeof(struct usba_ep) * pdata->num_ep); if (!eps) { - pr_err("failed to alloc eps\n"); + log_err("failed to alloc eps\n"); return NULL; }
Replace "paramter" by "parameter".
Signed-off-by: Zixun LI admin@hifiphile.com --- drivers/usb/gadget/atmel_usba_udc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c index 4641638412..2e3d8f36a2 100644 --- a/drivers/usb/gadget/atmel_usba_udc.c +++ b/drivers/usb/gadget/atmel_usba_udc.c @@ -1204,7 +1204,7 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver) int ret;
if (!driver || !driver->bind || !driver->setup) { - log_err("bad paramter\n"); + log_err("bad parameter\n"); return -EINVAL; }
@@ -1231,7 +1231,7 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver) struct usba_udc *udc = &controller;
if (!driver || !driver->unbind || !driver->disconnect) { - log_err("bad paramter\n"); + log_err("bad parameter\n"); return -EINVAL; }
To make all static functions in the top, no functional change.
Signed-off-by: Zixun LI admin@hifiphile.com --- drivers/usb/gadget/atmel_usba_udc.c | 80 ++++++++++++++--------------- 1 file changed, 40 insertions(+), 40 deletions(-)
diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c index 2e3d8f36a2..a3d24501ba 100644 --- a/drivers/usb/gadget/atmel_usba_udc.c +++ b/drivers/usb/gadget/atmel_usba_udc.c @@ -1179,6 +1179,46 @@ static int atmel_usba_stop(struct usba_udc *udc) return 0; }
+static struct usba_ep *usba_udc_pdata(struct usba_platform_data *pdata, + struct usba_udc *udc) +{ + struct usba_ep *eps; + int i; + + eps = malloc(sizeof(struct usba_ep) * pdata->num_ep); + if (!eps) { + log_err("failed to alloc eps\n"); + return NULL; + } + + udc->gadget.ep0 = &eps[0].ep; + + INIT_LIST_HEAD(&udc->gadget.ep_list); + INIT_LIST_HEAD(&eps[0].ep.ep_list); + + for (i = 0; i < pdata->num_ep; i++) { + struct usba_ep *ep = &eps[i]; + + ep->ep_regs = udc->regs + USBA_EPT_BASE(i); + ep->dma_regs = udc->regs + USBA_DMA_BASE(i); + ep->fifo = udc->fifo + USBA_FIFO_BASE(i); + ep->ep.ops = &usba_ep_ops; + ep->ep.name = pdata->ep[i].name; + ep->ep.maxpacket = pdata->ep[i].fifo_size; + ep->fifo_size = ep->ep.maxpacket; + ep->udc = udc; + INIT_LIST_HEAD(&ep->queue); + ep->nr_banks = pdata->ep[i].nr_banks; + ep->index = pdata->ep[i].index; + ep->can_dma = pdata->ep[i].can_dma; + ep->can_isoc = pdata->ep[i].can_isoc; + if (i) + list_add_tail(&ep->ep.ep_list, &udc->gadget.ep_list); + }; + + return eps; +} + static struct usba_udc controller = { .regs = (unsigned *)ATMEL_BASE_UDPHS, .fifo = (unsigned *)ATMEL_BASE_UDPHS_FIFO, @@ -1244,46 +1284,6 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver) return 0; }
-static struct usba_ep *usba_udc_pdata(struct usba_platform_data *pdata, - struct usba_udc *udc) -{ - struct usba_ep *eps; - int i; - - eps = malloc(sizeof(struct usba_ep) * pdata->num_ep); - if (!eps) { - log_err("failed to alloc eps\n"); - return NULL; - } - - udc->gadget.ep0 = &eps[0].ep; - - INIT_LIST_HEAD(&udc->gadget.ep_list); - INIT_LIST_HEAD(&eps[0].ep.ep_list); - - for (i = 0; i < pdata->num_ep; i++) { - struct usba_ep *ep = &eps[i]; - - ep->ep_regs = udc->regs + USBA_EPT_BASE(i); - ep->dma_regs = udc->regs + USBA_DMA_BASE(i); - ep->fifo = udc->fifo + USBA_FIFO_BASE(i); - ep->ep.ops = &usba_ep_ops; - ep->ep.name = pdata->ep[i].name; - ep->ep.maxpacket = pdata->ep[i].fifo_size; - ep->fifo_size = ep->ep.maxpacket; - ep->udc = udc; - INIT_LIST_HEAD(&ep->queue); - ep->nr_banks = pdata->ep[i].nr_banks; - ep->index = pdata->ep[i].index; - ep->can_dma = pdata->ep[i].can_dma; - ep->can_isoc = pdata->ep[i].can_isoc; - if (i) - list_add_tail(&ep->ep.ep_list, &udc->gadget.ep_list); - }; - - return eps; -} - int usba_udc_probe(struct usba_platform_data *pdata) { struct usba_udc *udc;
Rename atmel_usba_start() / atmel_usba_stop() to usba_udc_enable() / usba_udc_disable(), remove atmel_ prefix to be inline with other functions. Also avoid confusion with DM start() / stop() functions.
Signed-off-by: Zixun LI admin@hifiphile.com --- drivers/usb/gadget/atmel_usba_udc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c index a3d24501ba..ea9ad7585e 100644 --- a/drivers/usb/gadget/atmel_usba_udc.c +++ b/drivers/usb/gadget/atmel_usba_udc.c @@ -1153,7 +1153,7 @@ static int usba_udc_irq(struct usba_udc *udc) return 0; }
-static int atmel_usba_start(struct usba_udc *udc) +static int usba_udc_enable(struct usba_udc *udc) { udc->devstatus = 1 << USB_DEVICE_SELF_POWERED;
@@ -1168,7 +1168,7 @@ static int atmel_usba_start(struct usba_udc *udc) return 0; }
-static int atmel_usba_stop(struct usba_udc *udc) +static int usba_udc_disable(struct usba_udc *udc) { udc->gadget.speed = USB_SPEED_UNKNOWN; reset_all_endpoints(udc); @@ -1253,7 +1253,7 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver) return -EBUSY; }
- atmel_usba_start(udc); + usba_udc_enable(udc);
udc->driver = driver;
@@ -1279,7 +1279,7 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver) driver->unbind(&udc->gadget); udc->driver = NULL;
- atmel_usba_stop(udc); + usba_udc_disable(udc);
return 0; }
Add controller attach/detach support by using usb_gadget_ops.pullup() function.
Signed-off-by: Zixun LI admin@hifiphile.com --- drivers/usb/gadget/atmel_usba_udc.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c index ea9ad7585e..a7b96449f8 100644 --- a/drivers/usb/gadget/atmel_usba_udc.c +++ b/drivers/usb/gadget/atmel_usba_udc.c @@ -506,10 +506,28 @@ usba_udc_set_selfpowered(struct usb_gadget *gadget, int is_selfpowered) return 0; }
+static int usba_udc_pullup(struct usb_gadget *gadget, int is_on) +{ + struct usba_udc *udc = to_usba_udc(gadget); + u32 ctrl; + + ctrl = usba_readl(udc, CTRL); + + if (is_on) + ctrl &= ~USBA_DETACH; + else + ctrl |= USBA_DETACH; + + usba_writel(udc, CTRL, ctrl); + + return 0; +} + static const struct usb_gadget_ops usba_udc_ops = { .get_frame = usba_udc_get_frame, .wakeup = usba_udc_wakeup, .set_selfpowered = usba_udc_set_selfpowered, + .pullup = usba_udc_pullup, };
static struct usb_endpoint_descriptor usba_ep0_desc = {
Add driver model support by using the uclass UCLASS_USB_GADGET_GENERIC.
Disable local usb_gadget_register_driver()/usb_gadget_unregister_driver() implementation which is implemented in udc-core.c when DM_USB_GADGET is enabled.
Replace dm_usb_gadget_handle_interrupts() with handle_interrupts ops when DM_USB_GADGET is enabled.
Disable legacy struct usba_udc controller as controller point is extracted from udevice private data with DM.
Disable legacy usba_udc_probe() to avoid conflict with DM when it's enabled.
Compared to Linux driver only supported devices' DT bindings are included (sorted as Linux driver)
Signed-off-by: Zixun LI admin@hifiphile.com --- drivers/usb/gadget/atmel_usba_udc.c | 144 ++++++++++++++++++++++++++++ drivers/usb/gadget/atmel_usba_udc.h | 3 + include/linux/usb/atmel_usba_udc.h | 2 + 3 files changed, 149 insertions(+)
diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c index a7b96449f8..a77037a709 100644 --- a/drivers/usb/gadget/atmel_usba_udc.c +++ b/drivers/usb/gadget/atmel_usba_udc.c @@ -7,10 +7,14 @@ * Bo Shen voice.shen@atmel.com */
+#include <clk.h> +#include <dm.h> #include <log.h> #include <malloc.h> #include <asm/gpio.h> #include <asm/hardware.h> +#include <dm/device_compat.h> +#include <dm/devres.h> #include <linux/bitops.h> #include <linux/errno.h> #include <linux/list.h> @@ -18,6 +22,14 @@ #include <linux/usb/gadget.h> #include <linux/usb/atmel_usba_udc.h>
+#if CONFIG_IS_ENABLED(DM_USB_GADGET) +#include <mach/atmel_usba_udc.h> + +static int usba_udc_start(struct usb_gadget *gadget, + struct usb_gadget_driver *driver); +static int usba_udc_stop(struct usb_gadget *gadget); +#endif /* CONFIG_IS_ENABLED(DM_USB_GADGET) */ + #include "atmel_usba_udc.h"
static int vbus_is_present(struct usba_udc *udc) @@ -528,6 +540,10 @@ static const struct usb_gadget_ops usba_udc_ops = { .wakeup = usba_udc_wakeup, .set_selfpowered = usba_udc_set_selfpowered, .pullup = usba_udc_pullup, +#if CONFIG_IS_ENABLED(DM_USB_GADGET) + .udc_start = usba_udc_start, + .udc_stop = usba_udc_stop, +#endif };
static struct usb_endpoint_descriptor usba_ep0_desc = { @@ -1237,6 +1253,7 @@ static struct usba_ep *usba_udc_pdata(struct usba_platform_data *pdata, return eps; }
+#if !CONFIG_IS_ENABLED(DM_USB_GADGET) static struct usba_udc controller = { .regs = (unsigned *)ATMEL_BASE_UDPHS, .fifo = (unsigned *)ATMEL_BASE_UDPHS_FIFO, @@ -1312,3 +1329,130 @@ int usba_udc_probe(struct usba_platform_data *pdata)
return 0; } + +#else /* !CONFIG_IS_ENABLED(DM_USB_GADGET) */ +struct usba_priv_data { + struct clk_bulk clks; + struct usba_udc udc; +}; + +static int usba_udc_start(struct usb_gadget *gadget, + struct usb_gadget_driver *driver) +{ + struct usba_udc *udc = to_usba_udc(gadget); + + usba_udc_enable(udc); + + udc->driver = driver; + return 0; +} + +static int usba_udc_stop(struct usb_gadget *gadget) +{ + struct usba_udc *udc = to_usba_udc(gadget); + + udc->driver = NULL; + + usba_udc_disable(udc); + return 0; +} + +static int usba_udc_clk_init(struct udevice *dev, struct clk_bulk *clks) +{ + int ret; + + ret = clk_get_bulk(dev, clks); + if (ret == -ENOSYS) + return 0; + + if (ret) + return ret; + + ret = clk_enable_bulk(clks); + if (ret) { + clk_release_bulk(clks); + return ret; + } + + return 0; +} + +static int usba_udc_probe(struct udevice *dev) +{ + struct usba_priv_data *priv = dev_get_priv(dev); + struct usba_udc *udc = &priv->udc; + int ret; + + udc->fifo = (void __iomem *)dev_remap_addr_index(dev, FIFO_IOMEM_ID); + if (!udc->fifo) + return -EINVAL; + + udc->regs = (void __iomem *)dev_remap_addr_index(dev, CTRL_IOMEM_ID); + if (!udc->regs) + return -EINVAL; + + ret = usba_udc_clk_init(dev, &priv->clks); + if (ret) + return ret; + + udc->usba_ep = usba_udc_pdata(&pdata, udc); + + udc->gadget.ops = &usba_udc_ops; + udc->gadget.speed = USB_SPEED_HIGH, + udc->gadget.is_dualspeed = 1, + udc->gadget.name = "atmel_usba_udc", + + ret = usb_add_gadget_udc((struct device *)dev, &udc->gadget); + if (ret) + goto err; + + return 0; +err: + free(udc->usba_ep); + + clk_release_bulk(&priv->clks); + + return ret; +} + +static int usba_udc_remove(struct udevice *dev) +{ + struct usba_priv_data *priv = dev_get_priv(dev); + + usb_del_gadget_udc(&priv->udc.gadget); + + free(priv->udc.usba_ep); + + clk_release_bulk(&priv->clks); + + return dm_scan_fdt_dev(dev); +} + +static int usba_udc_handle_interrupts(struct udevice *dev) +{ + struct usba_priv_data *priv = dev_get_priv(dev); + + return usba_udc_irq(&priv->udc); +} + +static const struct usb_gadget_generic_ops usba_udc_gadget_ops = { + .handle_interrupts = usba_udc_handle_interrupts, +}; + +static const struct udevice_id usba_udc_ids[] = { + { .compatible = "atmel,at91sam9rl-udc" }, + { .compatible = "atmel,at91sam9g45-udc" }, + { .compatible = "atmel,sama5d3-udc" }, + {} +}; + +U_BOOT_DRIVER(atmel_usba_udc) = { + .name = "atmel_usba_udc", + .id = UCLASS_USB_GADGET_GENERIC, + .of_match = usba_udc_ids, + .ops = &usba_udc_gadget_ops, + .probe = usba_udc_probe, + .remove = usba_udc_remove, + .priv_auto = sizeof(struct usba_priv_data), +}; +#endif /* !CONFIG_IS_ENABLED(DM_USB_GADGET) */ diff --git a/drivers/usb/gadget/atmel_usba_udc.h b/drivers/usb/gadget/atmel_usba_udc.h index f6cb48c1cf..7f5e98f6c4 100644 --- a/drivers/usb/gadget/atmel_usba_udc.h +++ b/drivers/usb/gadget/atmel_usba_udc.h @@ -211,6 +211,9 @@ #define EP0_EPT_SIZE USBA_EPT_SIZE_64 #define EP0_NR_BANKS 1
+#define FIFO_IOMEM_ID 0 +#define CTRL_IOMEM_ID 1 + #define DBG_ERR 0x0001 /* report all error returns */ #define DBG_HW 0x0002 /* debug hardware initialization */ #define DBG_GADGET 0x0004 /* calls to/from gadget driver */ diff --git a/include/linux/usb/atmel_usba_udc.h b/include/linux/usb/atmel_usba_udc.h index c1c810759c..37c4f21849 100644 --- a/include/linux/usb/atmel_usba_udc.h +++ b/include/linux/usb/atmel_usba_udc.h @@ -20,6 +20,8 @@ struct usba_platform_data { struct usba_ep_data *ep; };
+#if !CONFIG_IS_ENABLED(DM_USB_GADGET) extern int usba_udc_probe(struct usba_platform_data *pdata); +#endif
#endif /* __LINUX_USB_USBA_H */
Hi Zixun,
Thank you for the patch.
On jeu., juil. 25, 2024 at 17:32, Zixun LI admin@hifiphile.com wrote:
Add driver model support by using the uclass UCLASS_USB_GADGET_GENERIC.
Disable local usb_gadget_register_driver()/usb_gadget_unregister_driver() implementation which is implemented in udc-core.c when DM_USB_GADGET is enabled.
Replace dm_usb_gadget_handle_interrupts() with handle_interrupts ops when DM_USB_GADGET is enabled.
Disable legacy struct usba_udc controller as controller point is extracted from udevice private data with DM.
Disable legacy usba_udc_probe() to avoid conflict with DM when it's enabled.
Compared to Linux driver only supported devices' DT bindings are included (sorted as Linux driver)
Signed-off-by: Zixun LI admin@hifiphile.com
Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com
drivers/usb/gadget/atmel_usba_udc.c | 144 ++++++++++++++++++++++++++++ drivers/usb/gadget/atmel_usba_udc.h | 3 + include/linux/usb/atmel_usba_udc.h | 2 + 3 files changed, 149 insertions(+)
diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c index a7b96449f8..a77037a709 100644 --- a/drivers/usb/gadget/atmel_usba_udc.c +++ b/drivers/usb/gadget/atmel_usba_udc.c @@ -7,10 +7,14 @@
Bo Shen <voice.shen@atmel.com>*/
+#include <clk.h> +#include <dm.h> #include <log.h> #include <malloc.h> #include <asm/gpio.h> #include <asm/hardware.h> +#include <dm/device_compat.h> +#include <dm/devres.h> #include <linux/bitops.h> #include <linux/errno.h> #include <linux/list.h> @@ -18,6 +22,14 @@ #include <linux/usb/gadget.h> #include <linux/usb/atmel_usba_udc.h>
+#if CONFIG_IS_ENABLED(DM_USB_GADGET) +#include <mach/atmel_usba_udc.h>
+static int usba_udc_start(struct usb_gadget *gadget,
struct usb_gadget_driver *driver);+static int usba_udc_stop(struct usb_gadget *gadget); +#endif /* CONFIG_IS_ENABLED(DM_USB_GADGET) */
#include "atmel_usba_udc.h"
static int vbus_is_present(struct usba_udc *udc) @@ -528,6 +540,10 @@ static const struct usb_gadget_ops usba_udc_ops = { .wakeup = usba_udc_wakeup, .set_selfpowered = usba_udc_set_selfpowered, .pullup = usba_udc_pullup, +#if CONFIG_IS_ENABLED(DM_USB_GADGET)
- .udc_start = usba_udc_start,
- .udc_stop = usba_udc_stop,
+#endif };
static struct usb_endpoint_descriptor usba_ep0_desc = { @@ -1237,6 +1253,7 @@ static struct usba_ep *usba_udc_pdata(struct usba_platform_data *pdata, return eps; }
+#if !CONFIG_IS_ENABLED(DM_USB_GADGET) static struct usba_udc controller = { .regs = (unsigned *)ATMEL_BASE_UDPHS, .fifo = (unsigned *)ATMEL_BASE_UDPHS_FIFO, @@ -1312,3 +1329,130 @@ int usba_udc_probe(struct usba_platform_data *pdata)
return 0; }
+#else /* !CONFIG_IS_ENABLED(DM_USB_GADGET) */ +struct usba_priv_data {
- struct clk_bulk clks;
- struct usba_udc udc;
+};
+static int usba_udc_start(struct usb_gadget *gadget,
struct usb_gadget_driver *driver)+{
- struct usba_udc *udc = to_usba_udc(gadget);
- usba_udc_enable(udc);
- udc->driver = driver;
- return 0;
+}
+static int usba_udc_stop(struct usb_gadget *gadget) +{
- struct usba_udc *udc = to_usba_udc(gadget);
- udc->driver = NULL;
- usba_udc_disable(udc);
- return 0;
+}
+static int usba_udc_clk_init(struct udevice *dev, struct clk_bulk *clks) +{
- int ret;
- ret = clk_get_bulk(dev, clks);
- if (ret == -ENOSYS)
return 0;- if (ret)
return ret;- ret = clk_enable_bulk(clks);
- if (ret) {
clk_release_bulk(clks);return ret;- }
- return 0;
+}
+static int usba_udc_probe(struct udevice *dev) +{
- struct usba_priv_data *priv = dev_get_priv(dev);
- struct usba_udc *udc = &priv->udc;
- int ret;
- udc->fifo = (void __iomem *)dev_remap_addr_index(dev, FIFO_IOMEM_ID);
- if (!udc->fifo)
return -EINVAL;- udc->regs = (void __iomem *)dev_remap_addr_index(dev, CTRL_IOMEM_ID);
- if (!udc->regs)
return -EINVAL;- ret = usba_udc_clk_init(dev, &priv->clks);
- if (ret)
return ret;- udc->usba_ep = usba_udc_pdata(&pdata, udc);
- udc->gadget.ops = &usba_udc_ops;
- udc->gadget.speed = USB_SPEED_HIGH,
- udc->gadget.is_dualspeed = 1,
- udc->gadget.name = "atmel_usba_udc",
- ret = usb_add_gadget_udc((struct device *)dev, &udc->gadget);
- if (ret)
goto err;- return 0;
+err:
- free(udc->usba_ep);
- clk_release_bulk(&priv->clks);
- return ret;
+}
+static int usba_udc_remove(struct udevice *dev) +{
- struct usba_priv_data *priv = dev_get_priv(dev);
- usb_del_gadget_udc(&priv->udc.gadget);
- free(priv->udc.usba_ep);
- clk_release_bulk(&priv->clks);
- return dm_scan_fdt_dev(dev);
+}
+static int usba_udc_handle_interrupts(struct udevice *dev) +{
- struct usba_priv_data *priv = dev_get_priv(dev);
- return usba_udc_irq(&priv->udc);
+}
+static const struct usb_gadget_generic_ops usba_udc_gadget_ops = {
- .handle_interrupts = usba_udc_handle_interrupts,
+};
+static const struct udevice_id usba_udc_ids[] = {
- { .compatible = "atmel,at91sam9rl-udc" },
- { .compatible = "atmel,at91sam9g45-udc" },
- { .compatible = "atmel,sama5d3-udc" },
- {}
+};
+U_BOOT_DRIVER(atmel_usba_udc) = {
- .name = "atmel_usba_udc",
- .id = UCLASS_USB_GADGET_GENERIC,
- .of_match = usba_udc_ids,
- .ops = &usba_udc_gadget_ops,
- .probe = usba_udc_probe,
- .remove = usba_udc_remove,
- .priv_auto = sizeof(struct usba_priv_data),
+}; +#endif /* !CONFIG_IS_ENABLED(DM_USB_GADGET) */ diff --git a/drivers/usb/gadget/atmel_usba_udc.h b/drivers/usb/gadget/atmel_usba_udc.h index f6cb48c1cf..7f5e98f6c4 100644 --- a/drivers/usb/gadget/atmel_usba_udc.h +++ b/drivers/usb/gadget/atmel_usba_udc.h @@ -211,6 +211,9 @@ #define EP0_EPT_SIZE USBA_EPT_SIZE_64 #define EP0_NR_BANKS 1
+#define FIFO_IOMEM_ID 0 +#define CTRL_IOMEM_ID 1
#define DBG_ERR 0x0001 /* report all error returns */ #define DBG_HW 0x0002 /* debug hardware initialization */ #define DBG_GADGET 0x0004 /* calls to/from gadget driver */ diff --git a/include/linux/usb/atmel_usba_udc.h b/include/linux/usb/atmel_usba_udc.h index c1c810759c..37c4f21849 100644 --- a/include/linux/usb/atmel_usba_udc.h +++ b/include/linux/usb/atmel_usba_udc.h @@ -20,6 +20,8 @@ struct usba_platform_data { struct usba_ep_data *ep; };
+#if !CONFIG_IS_ENABLED(DM_USB_GADGET) extern int usba_udc_probe(struct usba_platform_data *pdata); +#endif
#endif /* __LINUX_USB_USBA_H */
2.45.2
On Thu, Jul 25, 2024 at 8:24 PM Marek Vasut marex@denx.de wrote:
On 7/25/24 5:31 PM, Zixun LI wrote:
Changes in v4:
- Release clocks if probe failed
- Add missing endpoint data free
- Addressed comments
Please collect any AB/RB/TB tags that were provided in v3 too.
Sorry didn't know that, should I spin a v5 ?
On 7/25/24 8:48 PM, Zixun LI wrote:
On Thu, Jul 25, 2024 at 8:24 PM Marek Vasut marex@denx.de wrote:
On 7/25/24 5:31 PM, Zixun LI wrote:
Changes in v4:
- Release clocks if probe failed
- Add missing endpoint data free
- Addressed comments
Please collect any AB/RB/TB tags that were provided in v3 too.
Sorry didn't know that, should I spin a v5 ?
No worries, wait a few days for more feedback (if any), and then collect them for V5 and send V5.
Hi,
On jeu., juil. 25, 2024 at 20:57, Marek Vasut marex@denx.de wrote:
On 7/25/24 8:48 PM, Zixun LI wrote:
On Thu, Jul 25, 2024 at 8:24 PM Marek Vasut marex@denx.de wrote:
On 7/25/24 5:31 PM, Zixun LI wrote:
Changes in v4:
- Release clocks if probe failed
- Add missing endpoint data free
- Addressed comments
Please collect any AB/RB/TB tags that were provided in v3 too.
Sorry didn't know that, should I spin a v5 ?
No worries, wait a few days for more feedback (if any), and then collect them for V5 and send V5.
Indeed, it's important to collect the tags to make sure that the reviewers get credit for their time reviewing.
Zixun, make sure to collect them for all future contributions.
Fortunately, b4 (https://b4.docs.kernel.org/en/latest/index.html) can collect the tags:
$ b4 shazam -s -l --check 20240725153204.358925-1-admin@hifiphile.com
[...]
✓ [PATCH v4 1/7] usb: gadget: atmel: Sort includes + Reviewed-by: Marek Vasut marex@denx.de (✓ DKIM/denx.de) + Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com (✓ DKIM/baylibre-com.20230601.gappssmtp.com) + Link: https://lore.kernel.org/r/20240725153204.358925-2-admin@hifiphile.com + Signed-off-by: Mattijs Korpershoek mkorpershoek@baylibre.com ● checkpatch.pl: passed all checks
So, for this series, 2 cases can happen:
1. No other review comments are found. A v5 won't be required. I will collect the tags with b4 when applying to the u-boot-dfu tree
2. New review comments require the series to be modified. A v5 is required. In that case, make sure to collect all tags yourself.
Please wait a few days for others to get time to chime in this patch series.
Thank you for your contribution !
Mattijs
Hi,
On Thu, 25 Jul 2024 17:31:54 +0200, Zixun LI wrote:
Changes in v4:
- Release clocks if probe failed
- Add missing endpoint data free
- Addressed comments
Changes in v3:
- Separate code refactor into individual commits
- Extract the controller point from udevice private data in DM functions
[...]
Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu-next)
[1/7] usb: gadget: atmel: Sort includes https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/9cfee49022e2f22... [2/7] usb: gadget: atmel: Replace printf() and pr_err() by log_err() https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/9ec3b70b21d9f82... [3/7] usb: gadget: atmel: Fix typo in usb gadget driver register and unregister https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/dbbacf19d6310b5... [4/7] usb: gadget: atmel: Move usba_udc_pdata() with other static functions https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/9b97a354bdc0afc... [5/7] usb: gadget: atmel: Rename atmel_usba_start()/_stop() to usba_udc_enable()/_disable() https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/7a448c8f384022a... [6/7] usb: gadget: atmel: Add attach/detach support https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/ef5e1d1f97623f8... [7/7] usb: gadget: atmel: Add DM_USB_GADGET support https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/d6376f7ed823569...
-- Mattijs
participants (3)
-
Marek Vasut -
Mattijs Korpershoek -
Zixun LI