[U-Boot] [PATCH v2 6/6] drivers: usb: gadget: ether/rndis: convert driver to adopt device driver model

Mugunthan V N mugunthanvnm at ti.com
Thu May 12 07:43:17 CEST 2016


On Tuesday 10 May 2016 05:57 PM, Marek Vasut wrote:
> On 05/10/2016 01:44 PM, Mugunthan V N wrote:
>> Adopt usb ether gadget and rndis driver to adopt driver model
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm at ti.com>
>> ---
>>  drivers/usb/gadget/ether.c | 153 ++++++++++++++++++++++++++++++++++++++++++---
>>  drivers/usb/gadget/rndis.c |  13 +++-
>>  drivers/usb/gadget/rndis.h |  19 ++++--
>>  include/net.h              |   7 +++
>>  4 files changed, 177 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
>> index 2f70ebf..c436f75 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
>>  	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
>>  	struct eth_device netdev;
>> +#else
>> +	struct udevice *netdev;
>> +#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
>>  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;
>> +#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);
>> -#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;
>> @@ -2592,3 +2616,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");
> 
> Is this useful information ?

It is just a carry forward from non DM code.
I do feel that without a req, packet_receviced will never be true. If
you are okay, i will remove this in error log in next version.

> 
>> +			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("de:ad:be:ef:00:01", pdata->enetaddr);
> 
> Can we avoid hard-coding this default MAC ?

Yeah, will fix in next version.

> 
>> +	eth_setenv_enetaddr("usbnet_devaddr", pdata->enetaddr);
>> +
>> +	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");
> 
> Can you keep the messages consistent in some way ? Some start with
> capital letter, some don't . I would much rather see something like
> "%s: unable to bind usb_ether device\n", __func__

Yeah, will fix in next version.

Regards
Mugunthan V 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 */
>> diff --git a/drivers/usb/gadget/rndis.c b/drivers/usb/gadget/rndis.c
>> index 48463db..e6029ec 100644
>> --- a/drivers/usb/gadget/rndis.c
>> +++ b/drivers/usb/gadget/rndis.c
>> @@ -1123,7 +1123,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;
>>  
>> @@ -1151,8 +1155,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 7a389a5..084af85 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 05800c4..7dcc6c8 100644
>> --- a/include/net.h
>> +++ b/include/net.h
>> @@ -254,6 +254,13 @@ int eth_setenv_enetaddr_by_index(const char *base_name, int index,
>>  				 uchar *enetaddr);
>>  
>>  
>> +/*
>> + * 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:
>>
> 
> 



More information about the U-Boot mailing list