From: Mattijs Korpershoek <mkorpershoek@baylibre.com>
To: Marek Vasut <marek.vasut+renesas@mailbox.org>, u-boot@lists.denx.de
Cc: Marek Vasut <marek.vasut+renesas@mailbox.org>,
Linus Walleij <linus.walleij@linaro.org>,
Lukasz Majewski <lukma@denx.de>,
Miquel Raynal <miquel.raynal@bootlin.com>,
Neil Armstrong <neil.armstrong@linaro.org>,
Nishanth Menon <nm@ti.com>, Zixun LI <admin@hifiphile.com>
Subject: Re: [PATCH 6/6] usb: gadget: Pass struct udevice to usb_add_gadget_udc()
Date: Thu, 29 Aug 2024 10:44:08 +0200 [thread overview]
Message-ID: <87ttf3ra5j.fsf@baylibre.com> (raw)
In-Reply-To: <20240826143851.8020-6-marek.vasut+renesas@mailbox.org>
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);
> 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);
> /*-------------------------------------------------------------------------*/
>
> --
> 2.45.2
next prev parent reply other threads:[~2024-08-29 8:44 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-26 14:38 [PATCH 1/6] usb: gadget: Inline usb_add_gadget_udc_release Marek Vasut
2024-08-26 14:38 ` [PATCH 2/6] usb: gadget: Drop usb_udc_release() Marek Vasut
2024-08-29 7:41 ` Mattijs Korpershoek
2024-08-26 14:38 ` [PATCH 3/6] usb: gadget: Track driver data as part of struct usb_gadget Marek Vasut
2024-08-29 7:42 ` Mattijs Korpershoek
2024-08-26 14:38 ` [PATCH 4/6] usb: gadget: Drop dev_to_usb_gadget() Marek Vasut
2024-08-29 7:43 ` Mattijs Korpershoek
2024-08-26 14:38 ` [PATCH 5/6] usb: gadget: dwc2: Drop get/set_udc_gadget_private_data() Marek Vasut
2024-08-29 7:44 ` Mattijs Korpershoek
2024-08-26 14:38 ` [PATCH 6/6] usb: gadget: Pass struct udevice to usb_add_gadget_udc() Marek Vasut
2024-08-29 8:44 ` Mattijs Korpershoek [this message]
2024-08-30 22:44 ` Linus Walleij
2024-08-26 15:47 ` [PATCH 1/6] usb: gadget: Inline usb_add_gadget_udc_release Miquel Raynal
2024-08-29 7:08 ` Mattijs Korpershoek
2024-09-10 8:27 ` Mattijs Korpershoek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87ttf3ra5j.fsf@baylibre.com \
--to=mkorpershoek@baylibre.com \
--cc=admin@hifiphile.com \
--cc=linus.walleij@linaro.org \
--cc=lukma@denx.de \
--cc=marek.vasut+renesas@mailbox.org \
--cc=miquel.raynal@bootlin.com \
--cc=neil.armstrong@linaro.org \
--cc=nm@ti.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.