[PATCH v5 1/3] usb: gadget: ether: Inline functions used once

[PATCH v5 1/3] usb: gadget: ether: Inline functions used once

[Marek Vasut]

These functions here are only ever called once since drop of non-DM
networking code. Inline them. No functional change.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Lukasz Majewski <lukma@denx.de>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Simon Glass <sjg@chromium.org>
---
V2: No change
V3: No change
V4: No change
V5: No change
---
 drivers/usb/gadget/ether.c | 48 +++++++-------------------------------
 1 file changed, 9 insertions(+), 39 deletions(-)

diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index 85c971e4c43..88c656c4dc0 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -2273,10 +2273,11 @@ fail:
 }
 
 /*-------------------------------------------------------------------------*/
-static void _usb_eth_halt(struct ether_priv *priv);
+static void usb_eth_stop(struct udevice *dev);
 
-static int _usb_eth_init(struct ether_priv *priv)
+static int usb_eth_start(struct udevice *udev)
 {
+	struct ether_priv *priv = dev_get_priv(udev);
 	struct eth_dev *dev = &priv->ethdev;
 	struct usb_gadget *gadget;
 	unsigned long ts;
@@ -2347,12 +2348,13 @@ static int _usb_eth_init(struct ether_priv *priv)
 	rx_submit(dev, dev->rx_req, 0);
 	return 0;
 fail:
-	_usb_eth_halt(priv);
+	usb_eth_stop(udev);
 	return -1;
 }
 
-static int _usb_eth_send(struct ether_priv *priv, void *packet, int length)
+static int usb_eth_send(struct udevice *udev, void *packet, int length)
 {
+	struct ether_priv *priv = dev_get_priv(udev);
 	int			retval;
 	void			*rndis_pkt = NULL;
 	struct eth_dev		*dev = &priv->ethdev;
@@ -2419,15 +2421,9 @@ drop:
 	return -ENOMEM;
 }
 
-static int _usb_eth_recv(struct ether_priv *priv)
-{
-	usb_gadget_handle_interrupts(0);
-
-	return 0;
-}
-
-static void _usb_eth_halt(struct ether_priv *priv)
+static void usb_eth_stop(struct udevice *udev)
 {
+	struct ether_priv *priv = dev_get_priv(udev);
 	struct eth_dev *dev = &priv->ethdev;
 
 	/* If the gadget not registered, simple return */
@@ -2459,31 +2455,12 @@ static void _usb_eth_halt(struct ether_priv *priv)
 	usb_gadget_release(0);
 }
 
-static int usb_eth_start(struct udevice *dev)
-{
-	struct ether_priv *priv = dev_get_priv(dev);
-
-	return _usb_eth_init(priv);
-}
-
-static int usb_eth_send(struct udevice *dev, void *packet, int length)
-{
-	struct ether_priv *priv = dev_get_priv(dev);
-
-	return _usb_eth_send(priv, packet, length);
-}
-
 static int usb_eth_recv(struct udevice *dev, int flags, uchar **packetp)
 {
 	struct ether_priv *priv = dev_get_priv(dev);
 	struct eth_dev *ethdev = &priv->ethdev;
-	int ret;
 
-	ret = _usb_eth_recv(priv);
-	if (ret) {
-		pr_err("error packet receive\n");
-		return ret;
-	}
+	usb_gadget_handle_interrupts(0);
 
 	if (packet_received) {
 		if (ethdev->rx_req) {
@@ -2509,13 +2486,6 @@ static int usb_eth_free_pkt(struct udevice *dev, uchar *packet,
 	return rx_submit(ethdev, ethdev->rx_req, 0);
 }
 
-static void usb_eth_stop(struct udevice *dev)
-{
-	struct ether_priv *priv = dev_get_priv(dev);
-
-	_usb_eth_halt(priv);
-}
-
 static int usb_eth_probe(struct udevice *dev)
 {
 	struct ether_priv *priv = dev_get_priv(dev);
-- 
2.40.1

[PATCH v5 2/3] usb: gadget: ether: Move probe function above driver structure

[Marek Vasut]

Move the driver probe function above the driver structure, so it
can be placed alongside other related functions, like upcoming
remove function. No functional change.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Lukasz Majewski <lukma@denx.de>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Simon Glass <sjg@chromium.org>
---
V2: No change
V3: No change
V4: No change
V5: No change
---
 drivers/usb/gadget/ether.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index 88c656c4dc0..2040b5b5081 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -2486,20 +2486,6 @@ static int usb_eth_free_pkt(struct udevice *dev, uchar *packet,
 	return rx_submit(ethdev, ethdev->rx_req, 0);
 }
 
-static int usb_eth_probe(struct udevice *dev)
-{
-	struct ether_priv *priv = dev_get_priv(dev);
-	struct eth_pdata *pdata = dev_get_plat(dev);
-
-	priv->netdev = dev;
-	l_priv = priv;
-
-	get_ether_addr(CONFIG_USBNET_DEV_ADDR, pdata->enetaddr);
-	eth_env_set_enetaddr("usbnet_devaddr", pdata->enetaddr);
-
-	return 0;
-}
-
 static const struct eth_ops usb_eth_ops = {
 	.start		= usb_eth_start,
 	.send		= usb_eth_send,
@@ -2528,6 +2514,20 @@ int usb_ether_init(void)
 	return 0;
 }
 
+static int usb_eth_probe(struct udevice *dev)
+{
+	struct ether_priv *priv = dev_get_priv(dev);
+	struct eth_pdata *pdata = dev_get_plat(dev);
+
+	priv->netdev = dev;
+	l_priv = priv;
+
+	get_ether_addr(CONFIG_USBNET_DEV_ADDR, pdata->enetaddr);
+	eth_env_set_enetaddr("usbnet_devaddr", pdata->enetaddr);
+
+	return 0;
+}
+
 U_BOOT_DRIVER(eth_usb) = {
 	.name	= "usb_ether",
 	.id	= UCLASS_ETH,
-- 
2.40.1

Re: [PATCH v5 2/3] usb: gadget: ether: Move probe function above driver structure

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 346 bytes --]

On Fri, Aug 04, 2023 at 05:41:10PM +0200, Marek Vasut wrote:

> Move the driver probe function above the driver structure, so it
> can be placed alongside other related functions, like upcoming
> remove function. No functional change.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>

Tested-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v5 2/3] usb: gadget: ether: Move probe function above driver structure

[Miquel Raynal]

Hi Marek,

marex@denx.de wrote on Fri,  4 Aug 2023 17:41:10 +0200:

> Move the driver probe function above the driver structure, so it
> can be placed alongside other related functions, like upcoming
> remove function. No functional change.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc: Lukasz Majewski <lukma@denx.de>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Simon Glass <sjg@chromium.org>
> ---

Tested-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

Re: [PATCH v5 2/3] usb: gadget: ether: Move probe function above driver structure

[Miquel Raynal]

Hi Marek,

marex@denx.de wrote on Fri,  4 Aug 2023 17:41:10 +0200:

> Move the driver probe function above the driver structure, so it
> can be placed alongside other related functions, like upcoming
> remove function. No functional change.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc: Lukasz Majewski <lukma@denx.de>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Simon Glass <sjg@chromium.org>
> ---

Tested-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

Re: [PATCH v5 2/3] usb: gadget: ether: Move probe function above driver structure

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 504 bytes --]

On Fri, Aug 04, 2023 at 05:41:10PM +0200, Marek Vasut wrote:

> Move the driver probe function above the driver structure, so it
> can be placed alongside other related functions, like upcoming
> remove function. No functional change.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Tested-by: Tom Rini <trini@konsulko.com>
> Tested-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Tested-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH v5 3/3] usb: gadget: ether: Handle gadget driver registration in probe and remove

[Marek Vasut]

Move the ethernet gadget driver registration and removal from ethernet
bind and unbind callbacks into driver DM probe and remove callbacks.
This way, when the driver is bound, which is triggered deliberately
using 'bind' command, the USB ethernet gadget driver is instantiated
and bound to the matching UDC. In reverse, when the driver is unbound,
which is again triggered deliberately using 'unbind' command, the USB
ethernet gadget driver instance is removed.

Effectively, this now behaves like running either 'ums' or 'dfu' or
any other commands utilizing USB gadget functionality.

This also drops use of usb_gadget_release() and moves the use of
usb_gadget_initialize() into usb_ether_init() used only by legacy
platforms that do not use 'bind' command properly yet. Those have
no place in drivers.

Signed-off-by: Marek Vasut <marex@denx.de>
---
NOTE: This must be thoroughly tested.
---
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Lukasz Majewski <lukma@denx.de>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Simon Glass <sjg@chromium.org>
---
V2: Fix up return value handling in probe
V3: Reinstate usb_gadget_initialize() in usb_ether_init() to retain
    this obscure workaround for legacy platforms that fail to use
    bind command
V4: Call usb_gadget_release() in unbind callback to further help
    legacy platforms
V5: No change
---
 drivers/usb/gadget/ether.c | 97 ++++++++++++++++++++------------------
 1 file changed, 52 insertions(+), 45 deletions(-)

diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index 2040b5b5081..5ff06d3814b 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -2281,49 +2281,8 @@ static int usb_eth_start(struct udevice *udev)
 	struct eth_dev *dev = &priv->ethdev;
 	struct usb_gadget *gadget;
 	unsigned long ts;
-	int ret;
 	unsigned long timeout = USB_CONNECT_TIMEOUT;
 
-	ret = usb_gadget_initialize(0);
-	if (ret)
-		return ret;
-
-	/* Configure default mac-addresses for the USB ethernet device */
-#ifdef CONFIG_USBNET_DEV_ADDR
-	strlcpy(dev_addr, CONFIG_USBNET_DEV_ADDR, sizeof(dev_addr));
-#endif
-#ifdef CONFIG_USBNET_HOST_ADDR
-	strlcpy(host_addr, CONFIG_USBNET_HOST_ADDR, sizeof(host_addr));
-#endif
-	/* Check if the user overruled the MAC addresses */
-	if (env_get("usbnet_devaddr"))
-		strlcpy(dev_addr, env_get("usbnet_devaddr"),
-			sizeof(dev_addr));
-
-	if (env_get("usbnet_hostaddr"))
-		strlcpy(host_addr, env_get("usbnet_hostaddr"),
-			sizeof(host_addr));
-
-	if (!is_eth_addr_valid(dev_addr)) {
-		pr_err("Need valid 'usbnet_devaddr' to be set");
-		goto fail;
-	}
-	if (!is_eth_addr_valid(host_addr)) {
-		pr_err("Need valid 'usbnet_hostaddr' to be set");
-		goto fail;
-	}
-
-	priv->eth_driver.speed		= DEVSPEED;
-	priv->eth_driver.bind		= eth_bind;
-	priv->eth_driver.unbind		= eth_unbind;
-	priv->eth_driver.setup		= eth_setup;
-	priv->eth_driver.reset		= eth_disconnect;
-	priv->eth_driver.disconnect	= eth_disconnect;
-	priv->eth_driver.suspend	= eth_suspend;
-	priv->eth_driver.resume		= eth_resume;
-	if (usb_gadget_register_driver(&priv->eth_driver) < 0)
-		goto fail;
-
 	dev->network_started = 0;
 
 	packet_received = 0;
@@ -2450,9 +2409,6 @@ static void usb_eth_stop(struct udevice *udev)
 		usb_gadget_handle_interrupts(0);
 		dev->network_started = 0;
 	}
-
-	usb_gadget_unregister_driver(&priv->eth_driver);
-	usb_gadget_release(0);
 }
 
 static int usb_eth_recv(struct udevice *dev, int flags, uchar **packetp)
@@ -2511,7 +2467,7 @@ int usb_ether_init(void)
 		return ret;
 	}
 
-	return 0;
+	return usb_gadget_initialize(0);
 }
 
 static int usb_eth_probe(struct udevice *dev)
@@ -2525,6 +2481,55 @@ static int usb_eth_probe(struct udevice *dev)
 	get_ether_addr(CONFIG_USBNET_DEV_ADDR, pdata->enetaddr);
 	eth_env_set_enetaddr("usbnet_devaddr", pdata->enetaddr);
 
+	/* Configure default mac-addresses for the USB ethernet device */
+#ifdef CONFIG_USBNET_DEV_ADDR
+	strlcpy(dev_addr, CONFIG_USBNET_DEV_ADDR, sizeof(dev_addr));
+#endif
+#ifdef CONFIG_USBNET_HOST_ADDR
+	strlcpy(host_addr, CONFIG_USBNET_HOST_ADDR, sizeof(host_addr));
+#endif
+	/* Check if the user overruled the MAC addresses */
+	if (env_get("usbnet_devaddr"))
+		strlcpy(dev_addr, env_get("usbnet_devaddr"),
+			sizeof(dev_addr));
+
+	if (env_get("usbnet_hostaddr"))
+		strlcpy(host_addr, env_get("usbnet_hostaddr"),
+			sizeof(host_addr));
+
+	if (!is_eth_addr_valid(dev_addr)) {
+		pr_err("Need valid 'usbnet_devaddr' to be set");
+		return -EINVAL;
+	}
+	if (!is_eth_addr_valid(host_addr)) {
+		pr_err("Need valid 'usbnet_hostaddr' to be set");
+		return -EINVAL;
+	}
+
+	priv->eth_driver.speed		= DEVSPEED;
+	priv->eth_driver.bind		= eth_bind;
+	priv->eth_driver.unbind		= eth_unbind;
+	priv->eth_driver.setup		= eth_setup;
+	priv->eth_driver.reset		= eth_disconnect;
+	priv->eth_driver.disconnect	= eth_disconnect;
+	priv->eth_driver.suspend	= eth_suspend;
+	priv->eth_driver.resume		= eth_resume;
+	return usb_gadget_register_driver(&priv->eth_driver);
+}
+
+static int usb_eth_remove(struct udevice *dev)
+{
+	struct ether_priv *priv = dev_get_priv(dev);
+
+	usb_gadget_unregister_driver(&priv->eth_driver);
+
+	return 0;
+}
+
+static int usb_eth_unbind(struct udevice *dev)
+{
+	usb_gadget_release(0);
+
 	return 0;
 }
 
@@ -2532,6 +2537,8 @@ U_BOOT_DRIVER(eth_usb) = {
 	.name	= "usb_ether",
 	.id	= UCLASS_ETH,
 	.probe	= usb_eth_probe,
+	.remove	= usb_eth_remove,
+	.unbind	= usb_eth_unbind,
 	.ops	= &usb_eth_ops,
 	.priv_auto	= sizeof(struct ether_priv),
 	.plat_auto	= sizeof(struct eth_pdata),
-- 
2.40.1

[PATCH] cmd: Enable BIND by default if we have USB_ETHER

[Tom Rini]

The nature of the network stack means that if we are going to use the
gadget mode USB network driver there's no easy path to implicitly
bind/unbind the driver. Enable the "bind" command by default here so
that we can bind/unbind this as needed.

Signed-off-by: Tom Rini <trini@konsulko.com>
---
Cc: Marek Vasut <marex@denx.de>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
---
 cmd/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 2d6e5f993f04..7f65e3187a66 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -986,6 +986,7 @@ config CMD_BCB
 config CMD_BIND
 	bool "bind/unbind - Bind or unbind a device to/from a driver"
 	depends on DM
+	default y if USB_ETHER
 	help
 	  Bind or unbind a device to/from a driver from the command line.
 	  This is useful in situations where a device may be handled by several
-- 
2.34.1

Re: [PATCH] cmd: Enable BIND by default if we have USB_ETHER

[Miquel Raynal]

Hi Tom,

trini@konsulko.com wrote on Fri,  4 Aug 2023 12:06:21 -0400:

> The nature of the network stack means that if we are going to use the
> gadget mode USB network driver there's no easy path to implicitly
> bind/unbind the driver. Enable the "bind" command by default here so
> that we can bind/unbind this as needed.
> 
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
> Cc: Marek Vasut <marex@denx.de>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

Re: [PATCH v5 3/3] usb: gadget: ether: Handle gadget driver registration in probe and remove

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1020 bytes --]

On Fri, Aug 04, 2023 at 05:41:11PM +0200, Marek Vasut wrote:

> Move the ethernet gadget driver registration and removal from ethernet
> bind and unbind callbacks into driver DM probe and remove callbacks.
> This way, when the driver is bound, which is triggered deliberately
> using 'bind' command, the USB ethernet gadget driver is instantiated
> and bound to the matching UDC. In reverse, when the driver is unbound,
> which is again triggered deliberately using 'unbind' command, the USB
> ethernet gadget driver instance is removed.
> 
> Effectively, this now behaves like running either 'ums' or 'dfu' or
> any other commands utilizing USB gadget functionality.
> 
> This also drops use of usb_gadget_release() and moves the use of
> usb_gadget_initialize() into usb_ether_init() used only by legacy
> platforms that do not use 'bind' command properly yet. Those have
> no place in drivers.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>

Tested-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v5 3/3] usb: gadget: ether: Handle gadget driver registration in probe and remove

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1066 bytes --]

On Fri, Aug 04, 2023 at 05:41:11PM +0200, Marek Vasut wrote:

> Move the ethernet gadget driver registration and removal from ethernet
> bind and unbind callbacks into driver DM probe and remove callbacks.
> This way, when the driver is bound, which is triggered deliberately
> using 'bind' command, the USB ethernet gadget driver is instantiated
> and bound to the matching UDC. In reverse, when the driver is unbound,
> which is again triggered deliberately using 'unbind' command, the USB
> ethernet gadget driver instance is removed.
> 
> Effectively, this now behaves like running either 'ums' or 'dfu' or
> any other commands utilizing USB gadget functionality.
> 
> This also drops use of usb_gadget_release() and moves the use of
> usb_gadget_initialize() into usb_ether_init() used only by legacy
> platforms that do not use 'bind' command properly yet. Those have
> no place in drivers.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Tested-by: Tom Rini <trini@konsulko.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v5 1/3] usb: gadget: ether: Inline functions used once

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 296 bytes --]

On Fri, Aug 04, 2023 at 05:41:09PM +0200, Marek Vasut wrote:

> These functions here are only ever called once since drop of non-DM
> networking code. Inline them. No functional change.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>

Tested-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v5 1/3] usb: gadget: ether: Inline functions used once

[Miquel Raynal]

Hi Marek,

marex@denx.de wrote on Fri,  4 Aug 2023 17:41:09 +0200:

> These functions here are only ever called once since drop of non-DM
> networking code. Inline them. No functional change.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc: Lukasz Majewski <lukma@denx.de>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Simon Glass <sjg@chromium.org>
> ---

Tested-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

Re: [PATCH v5 1/3] usb: gadget: ether: Inline functions used once

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 398 bytes --]

On Fri, Aug 04, 2023 at 05:41:09PM +0200, Marek Vasut wrote:

> These functions here are only ever called once since drop of non-DM
> networking code. Inline them. No functional change.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Tested-by: Tom Rini <trini@konsulko.com>
> Tested-by: Miquel Raynal <miquel.raynal@bootlin.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]