From eeef1223025c68bd546a1f9411bdcdf518f8cb97 Mon Sep 17 00:00:00 2001 From: Stephan Gerhold Date: Mon, 7 Apr 2025 16:59:35 +0200 Subject: usb: gadget: f_acm: Claim requested USB endpoints U-Boot has an older version of the Linux gadget API, where USB endpoints returned by usb_ep_autoconfig() are not automatically claimed. As written in the documentation comment: "To prevent the endpoint from being returned by a later autoconfig call, claim it by assigning ep->driver_data to some non-null value." Right now f_acm doesn't do that, which means that e.g. ep_in and ep_notify may end up being assigned the same endpoint. Surprisingly, the ACM console is still somehow working, but this is not the expected behavior. It will break with a later commit that disallows calling usb_ep_enable() multiple times. Fix this by assigning some data to ep->driver_data, similar to the other gadget drivers. Fixes: fc2b399ac03b ("usb: gadget: Add CDC ACM function") Signed-off-by: Stephan Gerhold Reviewed-by: Mattijs Korpershoek Link: https://lore.kernel.org/r/20250407-acm-fixes-v1-1-e3dcb592d6d6@linaro.org Signed-off-by: Mattijs Korpershoek --- drivers/usb/gadget/f_acm.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/gadget/f_acm.c b/drivers/usb/gadget/f_acm.c index f18c6a0a761..2665fa4168f 100644 --- a/drivers/usb/gadget/f_acm.c +++ b/drivers/usb/gadget/f_acm.c @@ -238,18 +238,21 @@ static int acm_bind(struct usb_configuration *c, struct usb_function *f) return -ENODEV; f_acm->ep_in = ep; + ep->driver_data = c->cdev; /* claim */ ep = usb_ep_autoconfig(gadget, &acm_fs_out_desc); if (!ep) return -ENODEV; f_acm->ep_out = ep; + ep->driver_data = c->cdev; /* claim */ ep = usb_ep_autoconfig(gadget, &acm_fs_notify_desc); if (!ep) return -ENODEV; f_acm->ep_notify = ep; + ep->driver_data = c->cdev; /* claim */ if (gadget_is_dualspeed(gadget)) { /* Assume endpoint addresses are the same for both speeds */ -- cgit v1.3.1 From 22b5aad20ae0f17bd9617fb9c10ddb38d2b91920 Mon Sep 17 00:00:00 2001 From: Stephan Gerhold Date: Mon, 7 Apr 2025 16:59:36 +0200 Subject: usb: gadget: f_acm: Allow restarting ACM console after stopping it When using IOMUX, the "usbacm" console can be added/removed dynamically from the stdout/stderr/stdin environment variables to allow temporarily starting other USB gadgets (e.g. Fastboot). However, right now acm_stdio_stop() does not completely undo acm_stdio_start(): The USB gadget is unregistered, but as long as dev->priv stays set acm_stdio_start() will never register the USB gadget again. Clear dev->priv after we detach to make sure a start operation after a stop operation registers the gadget again. Fixes: fc2b399ac03b ("usb: gadget: Add CDC ACM function") Signed-off-by: Stephan Gerhold Reviewed-by: Mattijs Korpershoek Link: https://lore.kernel.org/r/20250407-acm-fixes-v1-2-e3dcb592d6d6@linaro.org Signed-off-by: Mattijs Korpershoek --- drivers/usb/gadget/f_acm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/f_acm.c b/drivers/usb/gadget/f_acm.c index 2665fa4168f..8f7256069f5 100644 --- a/drivers/usb/gadget/f_acm.c +++ b/drivers/usb/gadget/f_acm.c @@ -663,6 +663,7 @@ static int acm_stdio_stop(struct stdio_dev *dev) { g_dnl_unregister(); g_dnl_clear_detach(); + dev->priv = NULL; return 0; } -- cgit v1.3.1 From 59310d1ecb9f56a1bac405a5edfa9774f2d90220 Mon Sep 17 00:00:00 2001 From: Stephan Gerhold Date: Mon, 7 Apr 2025 16:59:37 +0200 Subject: usb: gadget: introduce 'enabled' flag in struct usb_ep f_acm calls usb_ep_disable(f_acm->ep_notify) unconditionally in acm_start_ctrl(), even if the USB endpoint was never enabled before. This causes crashes for some UDC drivers (e.g. ci_udc), because they dereference data structures that are assigned only after having called usb_ep_enable(). The f_acm driver in U-Boot is similar to the Linux driver, where this issue does not occur because usb_ep_disable() and usb_ep_enable() internally track the enabled state. In Linux this change was made in commit b0bac2581c19 ("usb: gadget: introduce 'enabled' flag in struct usb_ep") by Robert Baldyga. Fix the crashes for f_acm by making the same change in U-Boot. This makes the API less bug-prone and avoids introducing crashes when adapting new gadget drivers from Linux. Signed-off-by: Stephan Gerhold Reviewed-by: Mattijs Korpershoek Link: https://lore.kernel.org/r/20250407-acm-fixes-v1-3-e3dcb592d6d6@linaro.org Signed-off-by: Mattijs Korpershoek --- include/linux/usb/gadget.h | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h index c7927df15aa..fe79bf64a0e 100644 --- a/include/linux/usb/gadget.h +++ b/include/linux/usb/gadget.h @@ -179,6 +179,7 @@ struct usb_ep { const struct usb_ep_ops *ops; struct list_head ep_list; struct usb_ep_caps caps; + bool enabled; unsigned maxpacket:16; unsigned maxpacket_limit:16; unsigned max_streams:16; @@ -230,7 +231,18 @@ static inline void usb_ep_set_maxpacket_limit(struct usb_ep *ep, static inline int usb_ep_enable(struct usb_ep *ep, const struct usb_endpoint_descriptor *desc) { - return ep->ops->enable(ep, desc); + int ret; + + if (ep->enabled) + return 0; + + ret = ep->ops->enable(ep, desc); + if (ret) + return ret; + + ep->enabled = true; + + return 0; } /** @@ -247,7 +259,18 @@ static inline int usb_ep_enable(struct usb_ep *ep, */ static inline int usb_ep_disable(struct usb_ep *ep) { - return ep->ops->disable(ep); + int ret; + + if (!ep->enabled) + return 0; + + ret = ep->ops->disable(ep); + if (ret) + return ret; + + ep->enabled = false; + + return 0; } /** -- cgit v1.3.1 From 39a719dce59607d6abcac17cc2683aa9f2162539 Mon Sep 17 00:00:00 2001 From: Vincent Stehlé Date: Mon, 7 Apr 2025 19:05:25 +0200 Subject: fwu: developerbox: fix dfu alt buffer clearing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The set_dfu_alt_info() function calls the ALLOC_CACHE_ALIGN_BUFFER() macro to declare a `buf' variable pointer into an array allocated on the stack. It then calls the memset() function to clear the useable portion of the array using the idiomatic expression `sizeof(buf)'. While this would indeed work fine for an array, in the present case we end up clearing only the size of a pointer. Fix this by specifying the explicit size `DFU_ALT_BUF_LEN' instead. Fixes: 6b403ca4dcf4 ("fwu: DeveloperBox: add support for FWU") Signed-off-by: Vincent Stehlé Cc: Masahisa Kojima Cc: Tom Rini Cc: Jassi Brar Reviewed-by: Mattijs Korpershoek Link: https://lore.kernel.org/r/20250407170529.893307-2-vincent.stehle@arm.com Signed-off-by: Mattijs Korpershoek --- board/socionext/developerbox/fwu_plat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/board/socionext/developerbox/fwu_plat.c b/board/socionext/developerbox/fwu_plat.c index 26031795b09..a8b111477ef 100644 --- a/board/socionext/developerbox/fwu_plat.c +++ b/board/socionext/developerbox/fwu_plat.c @@ -18,7 +18,7 @@ void set_dfu_alt_info(char *interface, char *devstr) struct mtd_info *mtd; int ret; - memset(buf, 0, sizeof(buf)); + memset(buf, 0, DFU_ALT_BUF_LEN); mtd_probe_devices(); -- cgit v1.3.1