[PATCH 1/6] usb: gadget: Inline usb_add_gadget_udc_release
The release parameter of usb_add_gadget_udc_release() is never used. The function is never called from anywhere except from a wrapper in udc-core.c . Inline the function into the wrapper.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org --- Cc: Linus Walleij linus.walleij@linaro.org Cc: Lukasz Majewski lukma@denx.de Cc: Mattijs Korpershoek mkorpershoek@baylibre.com Cc: Miquel Raynal miquel.raynal@bootlin.com Cc: Neil Armstrong neil.armstrong@linaro.org Cc: Nishanth Menon nm@ti.com Cc: Zixun LI admin@hifiphile.com --- drivers/usb/gadget/udc/udc-core.c | 20 ++------------------ include/linux/usb/gadget.h | 2 -- 2 files changed, 2 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c index 6bb419ae2ab..37c0ee43c52 100644 --- a/drivers/usb/gadget/udc/udc-core.c +++ b/drivers/usb/gadget/udc/udc-core.c @@ -166,16 +166,14 @@ static void usb_udc_release(struct device *dev) }
/** - * usb_add_gadget_udc_release - adds a new gadget to the udc class driver list + * usb_add_gadget_udc - adds a new gadget to the udc class driver list * @parent: the parent device to this udc. Usually the controller driver's * device. * @gadget: the gadget to be added to the list. - * @release: a gadget release function. * * Returns zero on success, negative errno otherwise. */ -int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget, - void (*release)(struct device *dev)) +int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget) { struct usb_udc *udc; int ret = -ENOMEM; @@ -205,20 +203,6 @@ int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget, err1: return ret; } -EXPORT_SYMBOL_GPL(usb_add_gadget_udc_release); - -/** - * usb_add_gadget_udc - adds a new gadget to the udc class driver list - * @parent: the parent device to this udc. Usually the controller - * driver's device. - * @gadget: the gadget to be added to the list - * - * Returns zero on success, negative errno otherwise. - */ -int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget) -{ - return usb_add_gadget_udc_release(parent, gadget, NULL); -} EXPORT_SYMBOL_GPL(usb_add_gadget_udc);
static void usb_gadget_remove_driver(struct usb_udc *udc) diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h index c7927df15aa..d62fba0ca02 100644 --- a/include/linux/usb/gadget.h +++ b/include/linux/usb/gadget.h @@ -890,8 +890,6 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver); */ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver);
-int usb_add_gadget_udc_release(struct device *parent, - struct usb_gadget *gadget, void (*release)(struct device *dev)); int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget); void usb_del_gadget_udc(struct usb_gadget *gadget); /*-------------------------------------------------------------------------*/
This callback is never called, drop it. Instead, call kfree(udc) in usb_del_gadget_udc() to free the struct usb_udc data.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org --- Cc: Linus Walleij linus.walleij@linaro.org Cc: Lukasz Majewski lukma@denx.de Cc: Mattijs Korpershoek mkorpershoek@baylibre.com Cc: Miquel Raynal miquel.raynal@bootlin.com Cc: Neil Armstrong neil.armstrong@linaro.org Cc: Nishanth Menon nm@ti.com Cc: Zixun LI admin@hifiphile.com --- drivers/usb/gadget/udc/udc-core.c | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-)
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c index 37c0ee43c52..275d6fe7be8 100644 --- a/drivers/usb/gadget/udc/udc-core.c +++ b/drivers/usb/gadget/udc/udc-core.c @@ -41,7 +41,6 @@ struct usb_udc { struct list_head list; };
-static struct class *udc_class; static LIST_HEAD(udc_list); DEFINE_MUTEX(udc_lock);
@@ -150,21 +149,6 @@ static inline void usb_gadget_udc_stop(struct usb_udc *udc) udc->gadget->ops->udc_stop(udc->gadget); }
-/** - * usb_udc_release - release the usb_udc struct - * @dev: the dev member within usb_udc - * - * This is called by driver's core in order to free memory once the last - * reference is released. - */ -static void usb_udc_release(struct device *dev) -{ - struct usb_udc *udc; - - udc = container_of(dev, struct usb_udc, dev); - kfree(udc); -} - /** * usb_add_gadget_udc - adds a new gadget to the udc class driver list * @parent: the parent device to this udc. Usually the controller driver's @@ -185,8 +169,6 @@ int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget) dev_set_name(&gadget->dev, "gadget"); gadget->dev.parent = parent;
- udc->dev.release = usb_udc_release; - udc->dev.class = udc_class; udc->dev.parent = parent;
udc->gadget = gadget; @@ -247,6 +229,7 @@ found:
if (udc->driver) usb_gadget_remove_driver(udc); + kfree(udc); } EXPORT_SYMBOL_GPL(usb_del_gadget_udc);
Hi Marek,
Thank you for the patch.
On lun., août 26, 2024 at 16:38, Marek Vasut marek.vasut+renesas@mailbox.org wrote:
This callback is never called, drop it. Instead, call kfree(udc) in usb_del_gadget_udc() to free the struct usb_udc data.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org
Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com
Cc: Linus Walleij linus.walleij@linaro.org Cc: Lukasz Majewski lukma@denx.de Cc: Mattijs Korpershoek mkorpershoek@baylibre.com Cc: Miquel Raynal miquel.raynal@bootlin.com Cc: Neil Armstrong neil.armstrong@linaro.org Cc: Nishanth Menon nm@ti.com Cc: Zixun LI admin@hifiphile.com
drivers/usb/gadget/udc/udc-core.c | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-)
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c index 37c0ee43c52..275d6fe7be8 100644 --- a/drivers/usb/gadget/udc/udc-core.c +++ b/drivers/usb/gadget/udc/udc-core.c @@ -41,7 +41,6 @@ struct usb_udc { struct list_head list; };
-static struct class *udc_class; static LIST_HEAD(udc_list); DEFINE_MUTEX(udc_lock);
@@ -150,21 +149,6 @@ static inline void usb_gadget_udc_stop(struct usb_udc *udc) udc->gadget->ops->udc_stop(udc->gadget); }
-/**
- usb_udc_release - release the usb_udc struct
- @dev: the dev member within usb_udc
- This is called by driver's core in order to free memory once the last
- reference is released.
- */
-static void usb_udc_release(struct device *dev) -{
- struct usb_udc *udc;
- udc = container_of(dev, struct usb_udc, dev);
- kfree(udc);
-}
/**
- usb_add_gadget_udc - adds a new gadget to the udc class driver list
- @parent: the parent device to this udc. Usually the controller driver's
@@ -185,8 +169,6 @@ int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget) dev_set_name(&gadget->dev, "gadget"); gadget->dev.parent = parent;
udc->dev.release = usb_udc_release;
udc->dev.class = udc_class; udc->dev.parent = parent;
udc->gadget = gadget;
@@ -247,6 +229,7 @@ found:
if (udc->driver) usb_gadget_remove_driver(udc);
- kfree(udc);
} EXPORT_SYMBOL_GPL(usb_del_gadget_udc);
-- 2.45.2
Move driver data directly into struct usb_gadget {} in preparation for conversion of struct device to struct udevice * in struct usb_gadget.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org --- Cc: Linus Walleij linus.walleij@linaro.org Cc: Lukasz Majewski lukma@denx.de Cc: Mattijs Korpershoek mkorpershoek@baylibre.com Cc: Miquel Raynal miquel.raynal@bootlin.com Cc: Neil Armstrong neil.armstrong@linaro.org Cc: Nishanth Menon nm@ti.com Cc: Zixun LI admin@hifiphile.com --- include/linux/usb/gadget.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h index d62fba0ca02..a2b8520bc99 100644 --- a/include/linux/usb/gadget.h +++ b/include/linux/usb/gadget.h @@ -544,17 +544,18 @@ struct usb_gadget { unsigned a_alt_hnp_support:1; const char *name; struct device dev; + void *driver_data; unsigned quirk_ep_out_aligned_size:1; };
static inline void set_gadget_data(struct usb_gadget *gadget, void *data) { - gadget->dev.driver_data = data; + gadget->driver_data = data; }
static inline void *get_gadget_data(struct usb_gadget *gadget) { - return gadget->dev.driver_data; + return gadget->driver_data; }
static inline struct usb_gadget *dev_to_usb_gadget(struct device *dev)
Hi Marek,
Thank you for the patch.
On lun., août 26, 2024 at 16:38, Marek Vasut marek.vasut+renesas@mailbox.org wrote:
Move driver data directly into struct usb_gadget {} in preparation for conversion of struct device to struct udevice * in struct usb_gadget.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org
Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com
Cc: Linus Walleij linus.walleij@linaro.org Cc: Lukasz Majewski lukma@denx.de Cc: Mattijs Korpershoek mkorpershoek@baylibre.com Cc: Miquel Raynal miquel.raynal@bootlin.com Cc: Neil Armstrong neil.armstrong@linaro.org Cc: Nishanth Menon nm@ti.com Cc: Zixun LI admin@hifiphile.com
include/linux/usb/gadget.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h index d62fba0ca02..a2b8520bc99 100644 --- a/include/linux/usb/gadget.h +++ b/include/linux/usb/gadget.h @@ -544,17 +544,18 @@ struct usb_gadget { unsigned a_alt_hnp_support:1; const char *name; struct device dev;
- void *driver_data; unsigned quirk_ep_out_aligned_size:1;
};
static inline void set_gadget_data(struct usb_gadget *gadget, void *data) {
- gadget->dev.driver_data = data;
- gadget->driver_data = data;
}
static inline void *get_gadget_data(struct usb_gadget *gadget) {
- return gadget->dev.driver_data;
- return gadget->driver_data;
}
static inline struct usb_gadget *dev_to_usb_gadget(struct device *dev)
2.45.2
This function is unused, remove it.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org --- Cc: Linus Walleij linus.walleij@linaro.org Cc: Lukasz Majewski lukma@denx.de Cc: Mattijs Korpershoek mkorpershoek@baylibre.com Cc: Miquel Raynal miquel.raynal@bootlin.com Cc: Neil Armstrong neil.armstrong@linaro.org Cc: Nishanth Menon nm@ti.com Cc: Zixun LI admin@hifiphile.com --- include/linux/usb/gadget.h | 5 ----- 1 file changed, 5 deletions(-)
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h index a2b8520bc99..194025ebd12 100644 --- a/include/linux/usb/gadget.h +++ b/include/linux/usb/gadget.h @@ -558,11 +558,6 @@ static inline void *get_gadget_data(struct usb_gadget *gadget) return gadget->driver_data; }
-static inline struct usb_gadget *dev_to_usb_gadget(struct device *dev) -{ - return container_of(dev, struct usb_gadget, dev); -} - /* iterates the non-control endpoints; 'tmp' is a struct usb_ep pointer */ #define gadget_for_each_ep(tmp, gadget) \ list_for_each_entry(tmp, &(gadget)->ep_list, ep_list)
Hi Marek,
Thank you for the patch.
On lun., août 26, 2024 at 16:38, Marek Vasut marek.vasut+renesas@mailbox.org wrote:
This function is unused, remove it.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org
Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com
Cc: Linus Walleij linus.walleij@linaro.org Cc: Lukasz Majewski lukma@denx.de Cc: Mattijs Korpershoek mkorpershoek@baylibre.com Cc: Miquel Raynal miquel.raynal@bootlin.com Cc: Neil Armstrong neil.armstrong@linaro.org Cc: Nishanth Menon nm@ti.com Cc: Zixun LI admin@hifiphile.com
include/linux/usb/gadget.h | 5 ----- 1 file changed, 5 deletions(-)
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h index a2b8520bc99..194025ebd12 100644 --- a/include/linux/usb/gadget.h +++ b/include/linux/usb/gadget.h @@ -558,11 +558,6 @@ static inline void *get_gadget_data(struct usb_gadget *gadget) return gadget->driver_data; }
-static inline struct usb_gadget *dev_to_usb_gadget(struct device *dev) -{
- return container_of(dev, struct usb_gadget, dev);
-}
/* iterates the non-control endpoints; 'tmp' is a struct usb_ep pointer */ #define gadget_for_each_ep(tmp, gadget) \ list_for_each_entry(tmp, &(gadget)->ep_list, ep_list) -- 2.45.2
Neither get_udc_gadget_private_data() nor set_udc_gadget_private_data() is ever called anywhere, remove both.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org --- Cc: Linus Walleij linus.walleij@linaro.org Cc: Lukasz Majewski lukma@denx.de Cc: Mattijs Korpershoek mkorpershoek@baylibre.com Cc: Miquel Raynal miquel.raynal@bootlin.com Cc: Neil Armstrong neil.armstrong@linaro.org Cc: Nishanth Menon nm@ti.com Cc: Zixun LI admin@hifiphile.com --- drivers/usb/gadget/dwc2_udc_otg.c | 13 ------------- 1 file changed, 13 deletions(-)
diff --git a/drivers/usb/gadget/dwc2_udc_otg.c b/drivers/usb/gadget/dwc2_udc_otg.c index 7e9dd6f4268..fbd6c9600fc 100644 --- a/drivers/usb/gadget/dwc2_udc_otg.c +++ b/drivers/usb/gadget/dwc2_udc_otg.c @@ -121,19 +121,6 @@ static void nuke(struct dwc2_ep *ep, int status); static int dwc2_udc_set_halt(struct usb_ep *_ep, int value); static void dwc2_udc_set_nak(struct dwc2_ep *ep);
-void set_udc_gadget_private_data(void *p) -{ - debug_cond(DEBUG_SETUP != 0, - "%s: the_controller: 0x%p, p: 0x%p\n", __func__, - the_controller, p); - the_controller->gadget.dev.device_data = p; -} - -void *get_udc_gadget_private_data(struct usb_gadget *gadget) -{ - return gadget->dev.device_data; -} - static struct usb_ep_ops dwc2_ep_ops = { .enable = dwc2_ep_enable, .disable = dwc2_ep_disable,
Hi Marek,
Thank you for the patch.
On lun., août 26, 2024 at 16:38, Marek Vasut marek.vasut+renesas@mailbox.org wrote:
Neither get_udc_gadget_private_data() nor set_udc_gadget_private_data() is ever called anywhere, remove both.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org
Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com
Cc: Linus Walleij linus.walleij@linaro.org Cc: Lukasz Majewski lukma@denx.de Cc: Mattijs Korpershoek mkorpershoek@baylibre.com Cc: Miquel Raynal miquel.raynal@bootlin.com Cc: Neil Armstrong neil.armstrong@linaro.org Cc: Nishanth Menon nm@ti.com Cc: Zixun LI admin@hifiphile.com
drivers/usb/gadget/dwc2_udc_otg.c | 13 ------------- 1 file changed, 13 deletions(-)
diff --git a/drivers/usb/gadget/dwc2_udc_otg.c b/drivers/usb/gadget/dwc2_udc_otg.c index 7e9dd6f4268..fbd6c9600fc 100644 --- a/drivers/usb/gadget/dwc2_udc_otg.c +++ b/drivers/usb/gadget/dwc2_udc_otg.c @@ -121,19 +121,6 @@ static void nuke(struct dwc2_ep *ep, int status); static int dwc2_udc_set_halt(struct usb_ep *_ep, int value); static void dwc2_udc_set_nak(struct dwc2_ep *ep);
-void set_udc_gadget_private_data(void *p) -{
- debug_cond(DEBUG_SETUP != 0,
"%s: the_controller: 0x%p, p: 0x%p\n", __func__,the_controller, p);- the_controller->gadget.dev.device_data = p;
-}
-void *get_udc_gadget_private_data(struct usb_gadget *gadget) -{
- return gadget->dev.device_data;
-}
static struct usb_ep_ops dwc2_ep_ops = { .enable = dwc2_ep_enable, .disable = dwc2_ep_disable, -- 2.45.2
Almost every UDC driver already passes casted pointer to struct udevice usb_add_gadget_udc(), even if the behavior is not exactly correct. That struct udevice is at risk of being corrupted in case something modified it in the UDC core, which does not happen right now. Switch UDC core to use struct udevice outright and drop the casts.
UX500 has to be updated slightly, as that is the last place which does correctly use private struct device instance.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org --- Cc: Linus Walleij linus.walleij@linaro.org Cc: Lukasz Majewski lukma@denx.de Cc: Mattijs Korpershoek mkorpershoek@baylibre.com Cc: Miquel Raynal miquel.raynal@bootlin.com Cc: Neil Armstrong neil.armstrong@linaro.org Cc: Nishanth Menon nm@ti.com Cc: Zixun LI admin@hifiphile.com --- drivers/usb/cdns3/gadget.c | 2 +- drivers/usb/dwc3/gadget.c | 2 +- drivers/usb/gadget/dwc2_udc_otg.c | 2 +- drivers/usb/gadget/max3420_udc.c | 2 +- drivers/usb/gadget/udc/udc-core.c | 19 +++++++++---------- drivers/usb/mtu3/mtu3_gadget.c | 2 +- drivers/usb/musb-new/omap2430.c | 2 +- drivers/usb/musb-new/ti-musb.c | 2 +- drivers/usb/musb-new/ux500.c | 2 +- include/linux/usb/gadget.h | 4 ++-- 10 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c index 32b2c412068..b3513cc9bdc 100644 --- a/drivers/usb/cdns3/gadget.c +++ b/drivers/usb/cdns3/gadget.c @@ -2660,7 +2660,7 @@ static int cdns3_gadget_start(struct cdns3 *cdns) }
/* add USB gadget device */ - ret = usb_add_gadget_udc((struct device *)priv_dev->dev, + ret = usb_add_gadget_udc(priv_dev->dev, &priv_dev->gadget); if (ret < 0) { dev_err(priv_dev->dev, diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index fe33e307d3e..a3c3437aa3e 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -2688,7 +2688,7 @@ int dwc3_gadget_init(struct dwc3 *dwc) if (ret) goto err4;
- ret = usb_add_gadget_udc((struct device *)dwc->dev, &dwc->gadget); + ret = usb_add_gadget_udc(dwc->dev, &dwc->gadget); if (ret) { dev_err(dwc->dev, "failed to register udc\n"); goto err4; diff --git a/drivers/usb/gadget/dwc2_udc_otg.c b/drivers/usb/gadget/dwc2_udc_otg.c index fbd6c9600fc..4b6327d86d1 100644 --- a/drivers/usb/gadget/dwc2_udc_otg.c +++ b/drivers/usb/gadget/dwc2_udc_otg.c @@ -1135,7 +1135,7 @@ static int dwc2_udc_otg_probe(struct udevice *dev)
the_controller->driver = 0;
- ret = usb_add_gadget_udc((struct device *)dev, &the_controller->gadget); + ret = usb_add_gadget_udc(dev, &the_controller->gadget);
return ret; } diff --git a/drivers/usb/gadget/max3420_udc.c b/drivers/usb/gadget/max3420_udc.c index 557a1f0644e..7df73cd3625 100644 --- a/drivers/usb/gadget/max3420_udc.c +++ b/drivers/usb/gadget/max3420_udc.c @@ -836,7 +836,7 @@ static int max3420_udc_probe(struct udevice *dev) max3420_setup_eps(udc); max3420_setup_spi(udc);
- usb_add_gadget_udc((struct device *)dev, &udc->gadget); + usb_add_gadget_udc(dev, &udc->gadget);
return 0; } diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c index 275d6fe7be8..102d74ec02a 100644 --- a/drivers/usb/gadget/udc/udc-core.c +++ b/drivers/usb/gadget/udc/udc-core.c @@ -37,7 +37,7 @@ struct usb_udc { struct usb_gadget_driver *driver; struct usb_gadget *gadget; - struct device dev; + struct udevice *dev; struct list_head list; };
@@ -157,7 +157,7 @@ static inline void usb_gadget_udc_stop(struct usb_udc *udc) * * Returns zero on success, negative errno otherwise. */ -int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget) +int usb_add_gadget_udc(struct udevice *parent, struct usb_gadget *gadget) { struct usb_udc *udc; int ret = -ENOMEM; @@ -166,10 +166,9 @@ int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget) if (!udc) goto err1;
- dev_set_name(&gadget->dev, "gadget"); - gadget->dev.parent = parent; + gadget->dev = parent;
- udc->dev.parent = parent; + udc->dev = parent;
udc->gadget = gadget;
@@ -189,7 +188,7 @@ EXPORT_SYMBOL_GPL(usb_add_gadget_udc);
static void usb_gadget_remove_driver(struct usb_udc *udc) { - dev_dbg(&udc->dev, "unregistering UDC driver [%s]\n", + dev_dbg(udc->dev, "unregistering UDC driver [%s]\n", udc->driver->function);
usb_gadget_disconnect(udc->gadget); @@ -216,13 +215,13 @@ void usb_del_gadget_udc(struct usb_gadget *gadget) if (udc->gadget == gadget) goto found;
- dev_err(gadget->dev.parent, "gadget not registered.\n"); + dev_err(gadget->dev, "gadget not registered.\n"); mutex_unlock(&udc_lock);
return;
found: - dev_vdbg(gadget->dev.parent, "unregistering gadget\n"); + dev_vdbg(gadget->dev, "unregistering gadget\n");
list_del(&udc->list); mutex_unlock(&udc_lock); @@ -260,7 +259,7 @@ static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *dri { int ret;
- dev_dbg(&udc->dev, "registering UDC driver [%s]\n", + dev_dbg(udc->dev, "registering UDC driver [%s]\n", driver->function);
udc->driver = driver; @@ -280,7 +279,7 @@ static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *dri return 0; err1: if (ret != -EISNAM) - dev_err(&udc->dev, "failed to start %s: %d\n", + dev_err(udc->dev, "failed to start %s: %d\n", udc->driver->function, ret); udc->driver = NULL; return ret; diff --git a/drivers/usb/mtu3/mtu3_gadget.c b/drivers/usb/mtu3/mtu3_gadget.c index 027b7e61113..949b22f41c7 100644 --- a/drivers/usb/mtu3/mtu3_gadget.c +++ b/drivers/usb/mtu3/mtu3_gadget.c @@ -631,7 +631,7 @@ int mtu3_gadget_setup(struct mtu3 *mtu)
mtu3_gadget_init_eps(mtu);
- return usb_add_gadget_udc((struct device *)mtu->dev, &mtu->g); + return usb_add_gadget_udc(mtu->dev, &mtu->g); }
void mtu3_gadget_cleanup(struct mtu3 *mtu) diff --git a/drivers/usb/musb-new/omap2430.c b/drivers/usb/musb-new/omap2430.c index ba600d01102..54dff01db71 100644 --- a/drivers/usb/musb-new/omap2430.c +++ b/drivers/usb/musb-new/omap2430.c @@ -231,7 +231,7 @@ static int omap2430_musb_probe(struct udevice *dev) if (!host->host) return -EIO;
- return usb_add_gadget_udc((struct device *)otg_board_data, &host->host->g); + return usb_add_gadget_udc(dev, &host->host->g); }
musbp = musb_register(&plat->plat, (struct device *)otg_board_data, diff --git a/drivers/usb/musb-new/ti-musb.c b/drivers/usb/musb-new/ti-musb.c index ec1baa9337d..9e3f793af06 100644 --- a/drivers/usb/musb-new/ti-musb.c +++ b/drivers/usb/musb-new/ti-musb.c @@ -247,7 +247,7 @@ static int ti_musb_peripheral_probe(struct udevice *dev)
ti_musb_set_phy_power(dev, 1); musb_gadget_setup(priv->periph); - return usb_add_gadget_udc((struct device *)dev, &priv->periph->g); + return usb_add_gadget_udc(dev, &priv->periph->g); }
static int ti_musb_peripheral_remove(struct udevice *dev) diff --git a/drivers/usb/musb-new/ux500.c b/drivers/usb/musb-new/ux500.c index be0085f403d..c994dc2f04d 100644 --- a/drivers/usb/musb-new/ux500.c +++ b/drivers/usb/musb-new/ux500.c @@ -130,7 +130,7 @@ static int ux500_musb_probe(struct udevice *dev) if (!host->host) return -EIO;
- return usb_add_gadget_udc(&glue->dev, &host->host->g); + return usb_add_gadget_udc(dev, &host->host->g); #endif }
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h index 194025ebd12..e631f71eab8 100644 --- a/include/linux/usb/gadget.h +++ b/include/linux/usb/gadget.h @@ -543,7 +543,7 @@ struct usb_gadget { unsigned a_hnp_support:1; unsigned a_alt_hnp_support:1; const char *name; - struct device dev; + struct udevice *dev; void *driver_data; unsigned quirk_ep_out_aligned_size:1; }; @@ -886,7 +886,7 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver); */ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver);
-int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget); +int usb_add_gadget_udc(struct udevice *parent, struct usb_gadget *gadget); void usb_del_gadget_udc(struct usb_gadget *gadget); /*-------------------------------------------------------------------------*/
Hi Marek,
Thank you for the patch.
On lun., août 26, 2024 at 16:38, Marek Vasut marek.vasut+renesas@mailbox.org wrote:
Almost every UDC driver already passes casted pointer to struct udevice usb_add_gadget_udc(), even if the behavior is not exactly correct. That struct udevice is at risk of being corrupted in case something modified it in the UDC core, which does not happen right now. Switch UDC core to use struct udevice outright and drop the casts.
UX500 has to be updated slightly, as that is the last place which does correctly use private struct device instance.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org
Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com
Cc: Linus Walleij linus.walleij@linaro.org Cc: Lukasz Majewski lukma@denx.de Cc: Mattijs Korpershoek mkorpershoek@baylibre.com Cc: Miquel Raynal miquel.raynal@bootlin.com Cc: Neil Armstrong neil.armstrong@linaro.org Cc: Nishanth Menon nm@ti.com Cc: Zixun LI admin@hifiphile.com
drivers/usb/cdns3/gadget.c | 2 +- drivers/usb/dwc3/gadget.c | 2 +- drivers/usb/gadget/dwc2_udc_otg.c | 2 +- drivers/usb/gadget/max3420_udc.c | 2 +- drivers/usb/gadget/udc/udc-core.c | 19 +++++++++---------- drivers/usb/mtu3/mtu3_gadget.c | 2 +- drivers/usb/musb-new/omap2430.c | 2 +- drivers/usb/musb-new/ti-musb.c | 2 +- drivers/usb/musb-new/ux500.c | 2 +- include/linux/usb/gadget.h | 4 ++-- 10 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c index 32b2c412068..b3513cc9bdc 100644 --- a/drivers/usb/cdns3/gadget.c +++ b/drivers/usb/cdns3/gadget.c @@ -2660,7 +2660,7 @@ static int cdns3_gadget_start(struct cdns3 *cdns) }
/* add USB gadget device */
- ret = usb_add_gadget_udc((struct device *)priv_dev->dev,
- ret = usb_add_gadget_udc(priv_dev->dev, &priv_dev->gadget); if (ret < 0) { dev_err(priv_dev->dev,
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index fe33e307d3e..a3c3437aa3e 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -2688,7 +2688,7 @@ int dwc3_gadget_init(struct dwc3 *dwc) if (ret) goto err4;
- ret = usb_add_gadget_udc((struct device *)dwc->dev, &dwc->gadget);
- ret = usb_add_gadget_udc(dwc->dev, &dwc->gadget); if (ret) { dev_err(dwc->dev, "failed to register udc\n"); goto err4;
diff --git a/drivers/usb/gadget/dwc2_udc_otg.c b/drivers/usb/gadget/dwc2_udc_otg.c index fbd6c9600fc..4b6327d86d1 100644 --- a/drivers/usb/gadget/dwc2_udc_otg.c +++ b/drivers/usb/gadget/dwc2_udc_otg.c @@ -1135,7 +1135,7 @@ static int dwc2_udc_otg_probe(struct udevice *dev)
the_controller->driver = 0;
- ret = usb_add_gadget_udc((struct device *)dev, &the_controller->gadget);
ret = usb_add_gadget_udc(dev, &the_controller->gadget);
return ret;
} diff --git a/drivers/usb/gadget/max3420_udc.c b/drivers/usb/gadget/max3420_udc.c index 557a1f0644e..7df73cd3625 100644 --- a/drivers/usb/gadget/max3420_udc.c +++ b/drivers/usb/gadget/max3420_udc.c @@ -836,7 +836,7 @@ static int max3420_udc_probe(struct udevice *dev) max3420_setup_eps(udc); max3420_setup_spi(udc);
- usb_add_gadget_udc((struct device *)dev, &udc->gadget);
usb_add_gadget_udc(dev, &udc->gadget);
return 0;
} diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c index 275d6fe7be8..102d74ec02a 100644 --- a/drivers/usb/gadget/udc/udc-core.c +++ b/drivers/usb/gadget/udc/udc-core.c @@ -37,7 +37,7 @@ struct usb_udc { struct usb_gadget_driver *driver; struct usb_gadget *gadget;
- struct device dev;
- struct udevice *dev; struct list_head list;
};
@@ -157,7 +157,7 @@ static inline void usb_gadget_udc_stop(struct usb_udc *udc)
- Returns zero on success, negative errno otherwise.
*/ -int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget) +int usb_add_gadget_udc(struct udevice *parent, struct usb_gadget *gadget) { struct usb_udc *udc; int ret = -ENOMEM; @@ -166,10 +166,9 @@ int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget) if (!udc) goto err1;
- dev_set_name(&gadget->dev, "gadget");
- gadget->dev.parent = parent;
- gadget->dev = parent;
- udc->dev.parent = parent;
udc->dev = parent;
udc->gadget = gadget;
@@ -189,7 +188,7 @@ EXPORT_SYMBOL_GPL(usb_add_gadget_udc);
static void usb_gadget_remove_driver(struct usb_udc *udc) {
- dev_dbg(&udc->dev, "unregistering UDC driver [%s]\n",
dev_dbg(udc->dev, "unregistering UDC driver [%s]\n", udc->driver->function);
usb_gadget_disconnect(udc->gadget);
@@ -216,13 +215,13 @@ void usb_del_gadget_udc(struct usb_gadget *gadget) if (udc->gadget == gadget) goto found;
- dev_err(gadget->dev.parent, "gadget not registered.\n");
dev_err(gadget->dev, "gadget not registered.\n"); mutex_unlock(&udc_lock);
return;
found:
- dev_vdbg(gadget->dev.parent, "unregistering gadget\n");
dev_vdbg(gadget->dev, "unregistering gadget\n");
list_del(&udc->list); mutex_unlock(&udc_lock);
@@ -260,7 +259,7 @@ static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *dri { int ret;
- dev_dbg(&udc->dev, "registering UDC driver [%s]\n",
dev_dbg(udc->dev, "registering UDC driver [%s]\n", driver->function);
udc->driver = driver;
@@ -280,7 +279,7 @@ static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *dri return 0; err1: if (ret != -EISNAM)
dev_err(&udc->dev, "failed to start %s: %d\n",
dev_err(udc->dev, "failed to start %s: %d\n", udc->driver->function, ret);diff --git a/drivers/usb/mtu3/mtu3_gadget.c b/drivers/usb/mtu3/mtu3_gadget.c index 027b7e61113..949b22f41c7 100644 --- a/drivers/usb/mtu3/mtu3_gadget.c +++ b/drivers/usb/mtu3/mtu3_gadget.c @@ -631,7 +631,7 @@ int mtu3_gadget_setup(struct mtu3 *mtu)
mtu3_gadget_init_eps(mtu);
- return usb_add_gadget_udc((struct device *)mtu->dev, &mtu->g);
- return usb_add_gadget_udc(mtu->dev, &mtu->g);
}
void mtu3_gadget_cleanup(struct mtu3 *mtu) diff --git a/drivers/usb/musb-new/omap2430.c b/drivers/usb/musb-new/omap2430.c index ba600d01102..54dff01db71 100644 --- a/drivers/usb/musb-new/omap2430.c +++ b/drivers/usb/musb-new/omap2430.c @@ -231,7 +231,7 @@ static int omap2430_musb_probe(struct udevice *dev) if (!host->host) return -EIO;
return usb_add_gadget_udc((struct device *)otg_board_data, &host->host->g);
return usb_add_gadget_udc(dev, &host->host->g);}
musbp = musb_register(&plat->plat, (struct device *)otg_board_data,
diff --git a/drivers/usb/musb-new/ti-musb.c b/drivers/usb/musb-new/ti-musb.c index ec1baa9337d..9e3f793af06 100644 --- a/drivers/usb/musb-new/ti-musb.c +++ b/drivers/usb/musb-new/ti-musb.c @@ -247,7 +247,7 @@ static int ti_musb_peripheral_probe(struct udevice *dev)
ti_musb_set_phy_power(dev, 1); musb_gadget_setup(priv->periph);
- return usb_add_gadget_udc((struct device *)dev, &priv->periph->g);
- return usb_add_gadget_udc(dev, &priv->periph->g);
}
static int ti_musb_peripheral_remove(struct udevice *dev) diff --git a/drivers/usb/musb-new/ux500.c b/drivers/usb/musb-new/ux500.c index be0085f403d..c994dc2f04d 100644 --- a/drivers/usb/musb-new/ux500.c +++ b/drivers/usb/musb-new/ux500.c @@ -130,7 +130,7 @@ static int ux500_musb_probe(struct udevice *dev) if (!host->host) return -EIO;
- return usb_add_gadget_udc(&glue->dev, &host->host->g);
- return usb_add_gadget_udc(dev, &host->host->g);
#endif }
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h index 194025ebd12..e631f71eab8 100644 --- a/include/linux/usb/gadget.h +++ b/include/linux/usb/gadget.h @@ -543,7 +543,7 @@ struct usb_gadget { unsigned a_hnp_support:1; unsigned a_alt_hnp_support:1; const char *name;
- struct device dev;
- struct udevice *dev; void *driver_data; unsigned quirk_ep_out_aligned_size:1;
}; @@ -886,7 +886,7 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver); */ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver);
-int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget); +int usb_add_gadget_udc(struct udevice *parent, struct usb_gadget *gadget); void usb_del_gadget_udc(struct usb_gadget *gadget); /*-------------------------------------------------------------------------*/
-- 2.45.2
On Mon, Aug 26, 2024 at 4:39 PM Marek Vasut marek.vasut+renesas@mailbox.org wrote:
Almost every UDC driver already passes casted pointer to struct udevice usb_add_gadget_udc(), even if the behavior is not exactly correct. That struct udevice is at risk of being corrupted in case something modified it in the UDC core, which does not happen right now. Switch UDC core to use struct udevice outright and drop the casts.
UX500 has to be updated slightly, as that is the last place which does correctly use private struct device instance.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org
Reviewed-by: Linus Walleij linus.walleij@linaro.org
Yours, Linus Walleij
Hi Marek,
marek.vasut+renesas@mailbox.org wrote on Mon, 26 Aug 2024 16:38:36 +0200:
The release parameter of usb_add_gadget_udc_release() is never used. The function is never called from anywhere except from a wrapper in udc-core.c . Inline the function into the wrapper.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org
Cc: Linus Walleij linus.walleij@linaro.org Cc: Lukasz Majewski lukma@denx.de Cc: Mattijs Korpershoek mkorpershoek@baylibre.com Cc: Miquel Raynal miquel.raynal@bootlin.com Cc: Neil Armstrong neil.armstrong@linaro.org Cc: Nishanth Menon nm@ti.com Cc: Zixun LI admin@hifiphile.com
LGTM, thanks for the cleanup! Just a minor comment, I would really appreciate a very succinct cover letter with your overall goal. Would also be easier for the maintainers to pick-up the tags, like this one.
For the series:
Reviewed-by: Miquel Raynal miquel.raynal@bootlin.com
Thanks! Miquèl
Hi Marek,
Thank you for the patch.
On lun., août 26, 2024 at 16:38, Marek Vasut marek.vasut+renesas@mailbox.org wrote:
The release parameter of usb_add_gadget_udc_release() is never used. The function is never called from anywhere except from a wrapper in udc-core.c . Inline the function into the wrapper.
Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org
Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com
Cc: Linus Walleij linus.walleij@linaro.org Cc: Lukasz Majewski lukma@denx.de Cc: Mattijs Korpershoek mkorpershoek@baylibre.com Cc: Miquel Raynal miquel.raynal@bootlin.com Cc: Neil Armstrong neil.armstrong@linaro.org Cc: Nishanth Menon nm@ti.com Cc: Zixun LI admin@hifiphile.com
drivers/usb/gadget/udc/udc-core.c | 20 ++------------------ include/linux/usb/gadget.h | 2 -- 2 files changed, 2 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c index 6bb419ae2ab..37c0ee43c52 100644 --- a/drivers/usb/gadget/udc/udc-core.c +++ b/drivers/usb/gadget/udc/udc-core.c @@ -166,16 +166,14 @@ static void usb_udc_release(struct device *dev) }
/**
- usb_add_gadget_udc_release - adds a new gadget to the udc class driver list
- usb_add_gadget_udc - adds a new gadget to the udc class driver list
- @parent: the parent device to this udc. Usually the controller driver's
- device.
- @gadget: the gadget to be added to the list.
*/
- @release: a gadget release function.
- Returns zero on success, negative errno otherwise.
-int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget,
void (*release)(struct device *dev))+int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget) { struct usb_udc *udc; int ret = -ENOMEM; @@ -205,20 +203,6 @@ int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget, err1: return ret; } -EXPORT_SYMBOL_GPL(usb_add_gadget_udc_release);
-/**
- usb_add_gadget_udc - adds a new gadget to the udc class driver list
- @parent: the parent device to this udc. Usually the controller
- driver's device.
- @gadget: the gadget to be added to the list
- Returns zero on success, negative errno otherwise.
- */
-int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget) -{
- return usb_add_gadget_udc_release(parent, gadget, NULL);
-} EXPORT_SYMBOL_GPL(usb_add_gadget_udc);
static void usb_gadget_remove_driver(struct usb_udc *udc) diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h index c7927df15aa..d62fba0ca02 100644 --- a/include/linux/usb/gadget.h +++ b/include/linux/usb/gadget.h @@ -890,8 +890,6 @@ int usb_gadget_register_driver(struct usb_gadget_driver *driver); */ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver);
-int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget, void (*release)(struct device *dev));int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget); void usb_del_gadget_udc(struct usb_gadget *gadget); /*-------------------------------------------------------------------------*/ -- 2.45.2
Hi,
On Mon, 26 Aug 2024 16:38:36 +0200, Marek Vasut wrote:
The release parameter of usb_add_gadget_udc_release() is never used. The function is never called from anywhere except from a wrapper in udc-core.c . Inline the function into the wrapper.
Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu-next)
[1/6] usb: gadget: Inline usb_add_gadget_udc_release https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/188b26ea92f86b5... [2/6] usb: gadget: Drop usb_udc_release() https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/1ef703d73dbb5cf... [3/6] usb: gadget: Track driver data as part of struct usb_gadget https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/534a3e8c1a067da... [4/6] usb: gadget: Drop dev_to_usb_gadget() https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/1d030ba01de4ed7... [5/6] usb: gadget: dwc2: Drop get/set_udc_gadget_private_data() https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/a765f5d6b498220... [6/6] usb: gadget: Pass struct udevice to usb_add_gadget_udc() https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/75c142c73242d91...
-- Mattijs
participants (4)
-
Linus Walleij -
Marek Vasut -
Mattijs Korpershoek -
Miquel Raynal