On Wednesday 30 November 2016 05:24 AM, Joe Hershberger wrote:
On Thu, Nov 17, 2016 at 11:39 PM, Mugunthan V N mugunthanvnm@ti.com wrote:
Adopt usb ether gadget and rndis driver to adopt driver model
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com
drivers/usb/gadget/Kconfig | 4 ++ drivers/usb/gadget/ether.c | 153 ++++++++++++++++++++++++++++++++++++++++++--- drivers/usb/gadget/rndis.c | 13 +++- drivers/usb/gadget/rndis.h | 19 ++++-- include/net.h | 7 +++ 5 files changed, 181 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig index 40839d89e9..261ed128ac 100644 --- a/drivers/usb/gadget/Kconfig +++ b/drivers/usb/gadget/Kconfig @@ -112,6 +112,10 @@ config G_DNL_VENDOR_NUM config G_DNL_PRODUCT_NUM hex "Product ID of USB device"
+config USBNET_DEVADDR
string "USB Gadget Ethernet device mac address"default "de:ad:be:ef:00:01"Please don't do this. We don't have "default" MAC addresses. They are either from the env, from ROM, or randomly generated.
Okay will remove this and use either env MAC or random MAC.
endif # USB_GADGET_DOWNLOAD
endif # USB_GADGET diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c index 046ad8ca2b..8c3c3fd9ab 100644 --- a/drivers/usb/gadget/ether.c +++ b/drivers/usb/gadget/ether.c @@ -25,6 +25,7 @@ #include "rndis.h"
#include <dm.h> +#include <dm/lists.h> #include <dm/uclass-internal.h> #include <dm/device-internal.h>
@@ -116,7 +117,11 @@ struct eth_dev {
struct usb_request *tx_req, *rx_req;+#ifndef CONFIG_DM_ETH
Please use positive logic.
Okay, will fix in next version.
struct eth_device *net;+#else
struct udevice *net;+#endif struct net_device_stats stats; unsigned int tx_qlen;
@@ -143,7 +148,11 @@ struct eth_dev { /*-------------------------------------------------------------------------*/ struct ether_priv { struct eth_dev ethdev; +#ifndef CONFIG_DM_ETH
Please use positive logic
struct eth_device netdev;+#else
struct udevice *netdev;Did you really intend to have a pointer here when the other is an inline structure?
Yes, udevice is the device allocated by probe and passed in probe, but in non-dm case netdev has to be allocated by driver, in this case it declared as inline structure.
+#endif struct usb_gadget_driver eth_driver; };
@@ -1851,7 +1860,11 @@ static void rndis_control_ack_complete(struct usb_ep *ep,
static char rndis_resp_buf[8] __attribute__((aligned(sizeof(__le32))));
+#ifndef CONFIG_DM_ETH
Please use positive logic.
static int rndis_control_ack(struct eth_device *net) +#else +static int rndis_control_ack(struct udevice *net) +#endif { struct ether_priv *priv = (struct ether_priv *)net->priv; struct eth_dev *dev = &priv->ethdev; @@ -2001,6 +2014,9 @@ static int eth_bind(struct usb_gadget *gadget) int status = -ENOMEM; int gcnum; u8 tmp[7]; +#ifdef CONFIG_DM_ETH
struct eth_pdata *pdata = dev_get_platdata(l_priv->netdev);+#endif
/* these flags are only ever cleared; compiler take note */#ifndef CONFIG_USB_ETH_CDC @@ -2188,7 +2204,11 @@ autoconf_fail:
/* network device setup */+#ifndef CONFIG_DM_ETH dev->net = &l_priv->netdev;
You wouldn't need this difference if the priv also used a ptr in the non-dm case.
Also, if you are opposed to cleaning this up (preferably by adding a preceding patch to make it a pointer), at least use positive logic (#ifdef CONFIG_DM_ETH). Same applies elsewhere.
Okay, will add a patch to convert netdev to pointer
+#else
dev->net = l_priv->netdev;+#endif
dev->cdc = cdc; dev->zlp = zlp;@@ -2197,6 +2217,7 @@ autoconf_fail: dev->out_ep = out_ep; dev->status_ep = status_ep;
memset(tmp, 0, sizeof(tmp)); /* * Module params for these addresses should come from ID proms. * The host side address is used with CDC and RNDIS, and commonly@@ -2204,10 +2225,13 @@ autoconf_fail: * host side code for the SAFE thing cares -- its original BLAN * thing didn't, Sharp never assigned those addresses on Zaurii. */ +#ifndef CONFIG_DM_ETH get_ether_addr(dev_addr, dev->net->enetaddr);
memset(tmp, 0, sizeof(tmp)); memcpy(tmp, dev->net->enetaddr, sizeof(dev->net->enetaddr));+#else
get_ether_addr(dev_addr, pdata->enetaddr);memcpy(tmp, pdata->enetaddr, sizeof(pdata->enetaddr));+#endif
get_ether_addr(host_addr, dev->host_mac);@@ -2268,10 +2292,11 @@ autoconf_fail: status_ep ? " STATUS " : "", status_ep ? status_ep->name : "" );
printf("MAC %02x:%02x:%02x:%02x:%02x:%02x\n",dev->net->enetaddr[0], dev->net->enetaddr[1],dev->net->enetaddr[2], dev->net->enetaddr[3],dev->net->enetaddr[4], dev->net->enetaddr[5]);+#ifndef CONFIG_DM_ETH
printf("MAC %pM\n", dev->net->enetaddr);+#else
printf("MAC %pM\n", pdata->enetaddr);+#endif
if (cdc || rndis) printf("HOST MAC %02x:%02x:%02x:%02x:%02x:%02x\n",@@ -2520,13 +2545,12 @@ void _usb_eth_halt(struct ether_priv *priv) }
usb_gadget_unregister_driver(&priv->eth_driver);-#ifdef CONFIG_DM_USB
device_remove(dev->usb_udev);Why is this not needed anymore?
Oops, thought of moveing device remove to .stop, but missed. Now realizing that this change is not needed as it will break !DM_ETH and DM_USB configuration.
-#else +#ifndef CONFIG_DM_USB board_usb_cleanup(0, USB_INIT_DEVICE); #endif }
+#ifndef CONFIG_DM_ETH static int usb_eth_init(struct eth_device *netdev, bd_t *bd) { struct ether_priv *priv = (struct ether_priv *)netdev->priv; @@ -2593,3 +2617,114 @@ int usb_eth_initialize(bd_t *bi) eth_register(netdev); return 0; } +#else +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) {error("error packet receive\n");return ret;}if (packet_received) {if (ethdev->rx_req) {*packetp = (uchar *)net_rx_packets[0];return ethdev->rx_req->length;} else {error("dev->rx_req invalid");return -EFAULT;}}return -EAGAIN;+}
+static int usb_eth_free_pkt(struct udevice *dev, uchar *packet,
int length)+{
struct ether_priv *priv = dev_get_priv(dev);struct eth_dev *ethdev = &priv->ethdev;packet_received = 0;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);struct eth_pdata *pdata = dev_get_platdata(dev);priv->netdev = dev;l_priv = priv;get_ether_addr(CONFIG_USBNET_DEVADDR, pdata->enetaddr);This function it just a separately implemented copy of other functions the net stack already provides. Assuming you should be manipulating the MAC addresses from some string at all, at least it should be using eth_parse_enetaddr().
eth_setenv_enetaddr("usbnet_devaddr", pdata->enetaddr);This means that it will always clobber user's env. That's no good. Also, UCLASS_ETH already deals with mac address names for USB NICs as just another ethaddr, not a special usbnet name. Why not let that work?
Sure, will verify this and remove this section. Let network stack handle it.
return 0;+}
+static const struct eth_ops usb_eth_ops = {
.start = usb_eth_start,.send = usb_eth_send,.recv = usb_eth_recv,.free_pkt = usb_eth_free_pkt,.stop = usb_eth_stop,+};
+int usb_ether_init(void) +{
struct udevice *dev;struct udevice *usb_dev;int ret;ret = uclass_first_device(UCLASS_USB_DEV_GENERIC, &usb_dev);if (!usb_dev || ret) {error("No USB device found\n");return ret;}ret = device_bind_driver(usb_dev, "usb_ether", "usb_ether", &dev);if (!dev || ret) {error("usb - not able to bind usb_ether device\n");return ret;}return 0;+}
+U_BOOT_DRIVER(eth_usb) = {
.name = "usb_ether",.id = UCLASS_ETH,.probe = usb_eth_probe,.ops = &usb_eth_ops,.priv_auto_alloc_size = sizeof(struct ether_priv),.platdata_auto_alloc_size = sizeof(struct eth_pdata),.flags = DM_FLAG_ALLOC_PRIV_DMA,+}; +#endif /* CONFIG_DM_ETH */
It seems that this driver now supports 3 configurations.
DM_ETH + DM_USB !DM_ETH + DM_USB !DM_ETH + !DM_USB
Since it doesn't support DM_ETH + !DM_USB, you should probably explicitly error in that case by checking at the top of the file.
Will add an error at the top of the file.
Regards Mugunthan V N
diff --git a/drivers/usb/gadget/rndis.c b/drivers/usb/gadget/rndis.c index 844a0c7236..5ad481302b 100644 --- a/drivers/usb/gadget/rndis.c +++ b/drivers/usb/gadget/rndis.c @@ -1121,7 +1121,11 @@ int rndis_msg_parser(u8 configNr, u8 *buf) return -ENOTSUPP; }
+#ifndef CONFIG_DM_ETH int rndis_register(int (*rndis_control_ack)(struct eth_device *)) +#else +int rndis_register(int (*rndis_control_ack)(struct udevice *)) +#endif { u8 i;
@@ -1149,8 +1153,13 @@ void rndis_deregister(int configNr) return; }
-int rndis_set_param_dev(u8 configNr, struct eth_device *dev, int mtu,
struct net_device_stats *stats, u16 *cdc_filter)+#ifndef CONFIG_DM_ETH +int rndis_set_param_dev(u8 configNr, struct eth_device *dev, int mtu,
struct net_device_stats *stats, u16 *cdc_filter)+#else +int rndis_set_param_dev(u8 configNr, struct udevice *dev, int mtu,
struct net_device_stats *stats, u16 *cdc_filter)+#endif { debug("%s: configNr = %d\n", __func__, configNr); if (!dev || !stats) diff --git a/drivers/usb/gadget/rndis.h b/drivers/usb/gadget/rndis.h index 7a389a580a..084af8541c 100644 --- a/drivers/usb/gadget/rndis.h +++ b/drivers/usb/gadget/rndis.h @@ -222,23 +222,34 @@ typedef struct rndis_params {
const u8 *host_mac; u16 *filter;
struct eth_device *dev; struct net_device_stats *stats; int mtu; u32 vendorID; const char *vendorDescr;int (*ack)(struct eth_device *);+#ifndef CONFIG_DM_ETH
struct eth_device *dev;int (*ack)(struct eth_device *);+#else
struct udevice *dev;int (*ack)(struct udevice *);+#endif struct list_head resp_queue; } rndis_params;
/* RNDIS Message parser and other useless functions */ int rndis_msg_parser(u8 configNr, u8 *buf); enum rndis_state rndis_get_state(int configNr); -int rndis_register(int (*rndis_control_ack)(struct eth_device *)); void rndis_deregister(int configNr); +#ifndef CONFIG_DM_ETH +int rndis_register(int (*rndis_control_ack)(struct eth_device *)); int rndis_set_param_dev(u8 configNr, struct eth_device *dev, int mtu,
struct net_device_stats *stats, u16 *cdc_filter);
struct net_device_stats *stats, u16 *cdc_filter);+#else +int rndis_register(int (*rndis_control_ack)(struct udevice *)); +int rndis_set_param_dev(u8 configNr, struct udevice *dev, int mtu,
struct net_device_stats *stats, u16 *cdc_filter);+#endif int rndis_set_param_vendor(u8 configNr, u32 vendorID, const char *vendorDescr); int rndis_set_param_medium(u8 configNr, u32 medium, u32 speed); diff --git a/include/net.h b/include/net.h index 06320c6514..1f4d947350 100644 --- a/include/net.h +++ b/include/net.h @@ -255,6 +255,13 @@ int eth_setenv_enetaddr_by_index(const char *base_name, int index,
/*
- Initialize USB ethernet device with CONFIG_DM_ETH
- Returns:
0 is success, non-zero is error status.- */
+int usb_ether_init(void);
+/*
- Get the hardware address for an ethernet interface .
- Args:
base_name - base name for device (normally "eth")-- 2.11.0.rc2
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot