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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox