lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Mon, 29 Dec 2008 16:24:02 -0800
From:	David Brownell <david-b@...bell.net>
To:	Julia Lawall <julia@...u.dk>
Cc:	gregkh@...e.de, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org
Subject: Re: [PATCH 10/13, revised] drivers/usb/gadget: use USB API functions rather than constants

On Monday 29 December 2008, Julia Lawall wrote:
> As compared to the previous version, the includes of usb.h have been dropped.
> This patch depends on the previously submitted patch "Move definitions from
> usb.h to usb/ch9.h".
> 
> 
> 
> From: Julia Lawall <julia@...u.dk>
> 
> This set of patches introduces calls to the following set of functions:
> 
> usb_endpoint_dir_in(epd)
> usb_endpoint_dir_out(epd)
> usb_endpoint_is_bulk_in(epd)
> usb_endpoint_is_bulk_out(epd)
> usb_endpoint_is_int_in(epd)
> usb_endpoint_is_int_out(epd)
> usb_endpoint_is_isoc_in(epd)
> usb_endpoint_is_isoc_out(epd)
> usb_endpoint_num(epd)
> usb_endpoint_type(epd)
> usb_endpoint_xfer_bulk(epd)
> usb_endpoint_xfer_control(epd)
> usb_endpoint_xfer_int(epd)
> usb_endpoint_xfer_isoc(epd)
> 
> In some cases, introducing one of these functions is not possible, and it
> just replaces an explicit integer value by one of the following constants:
> 
> USB_ENDPOINT_XFER_BULK
> USB_ENDPOINT_XFER_CONTROL
> USB_ENDPOINT_XFER_INT
> USB_ENDPOINT_XFER_ISOC
> 
> An extract of the semantic patch that makes these changes is as follows:
> (http://www.emn.fr/x-info/coccinelle/)
> 
> // <smpl>
> @r1@ struct usb_endpoint_descriptor *epd; @@
> 
> - ((epd->bmAttributes & \(USB_ENDPOINT_XFERTYPE_MASK\|3\)) ==
> - \(USB_ENDPOINT_XFER_CONTROL\|0\))
> + usb_endpoint_xfer_control(epd)
> 
> @r5@ struct usb_endpoint_descriptor *epd; @@
> 
> - ((epd->bEndpointAddress & \(USB_ENDPOINT_DIR_MASK\|0x80\)) ==
> -  \(USB_DIR_IN\|0x80\))
> + usb_endpoint_dir_in(epd)
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@...u.dk>

Acked-by: David Brownell <dbrownell@...rs.sourceforge.net>

Thanks for the cleanup!


> 
> ---
>  drivers/usb/gadget/at91_udc.c       |    2 +-
>  drivers/usb/gadget/atmel_usba_udc.c |    6 +++---
>  drivers/usb/gadget/dummy_hcd.c      |   10 +++++-----
>  drivers/usb/gadget/epautoconf.c     |   10 +++++-----
>  drivers/usb/gadget/fsl_qe_udc.c     |    8 ++++----
>  drivers/usb/gadget/fsl_usb2_udc.c   |   16 +++++++---------
>  drivers/usb/gadget/goku_udc.c       |    4 ++--
>  drivers/usb/gadget/m66592-udc.c     |   12 ++++++------
>  drivers/usb/gadget/net2280.c        |    9 ++++-----
>  drivers/usb/gadget/s3c2410_udc.c    |    4 ++--
>  10 files changed, 39 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c
> index 0b2bb8f..0e27960 100644
> --- a/drivers/usb/gadget/at91_udc.c
> +++ b/drivers/usb/gadget/at91_udc.c
> @@ -485,7 +486,7 @@ static int at91_ep_enable(struct usb_ep *_ep,
>  		return -ESHUTDOWN;
>  	}
>  
> -	tmp = desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK;
> +	tmp = usb_endpoint_type(desc);
>  	switch (tmp) {
>  	case USB_ENDPOINT_XFER_CONTROL:
>  		DBG("only one control endpoint\n");
> diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c
> index 65b03e3..ddf00a7 100644
> --- a/drivers/usb/gadget/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/atmel_usba_udc.c
> @@ -529,7 +530,7 @@ usba_ep_enable(struct usb_ep *_ep, const struct usb_endpoint_descriptor *desc)
>  
>  	maxpacket = le16_to_cpu(desc->wMaxPacketSize) & 0x7ff;
>  
> -	if (((desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK) != ep->index)
> +	if ((usb_endpoint_num(desc) != ep->index)
>  			|| ep->index == 0
>  			|| desc->bDescriptorType != USB_DT_ENDPOINT
>  			|| maxpacket == 0
> @@ -550,12 +551,12 @@ usba_ep_enable(struct usb_ep *_ep, const struct usb_endpoint_descriptor *desc)
>  	DBG(DBG_HW, "%s: EPT_SIZE = %lu (maxpacket = %lu)\n",
>  			ep->ep.name, ept_cfg, maxpacket);
>  
> -	if ((desc->bEndpointAddress & USB_ENDPOINT_DIR_MASK) == USB_DIR_IN) {
> +	if (usb_endpoint_dir_in(desc)) {
>  		ep->is_in = 1;
>  		ept_cfg |= USBA_EPT_DIR_IN;
>  	}
>  
> -	switch (desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) {
> +	switch (usb_endpoint_type(desc)) {
>  	case USB_ENDPOINT_XFER_CONTROL:
>  		ept_cfg |= USBA_BF(EPT_TYPE, USBA_EPT_TYPE_CONTROL);
>  		ept_cfg |= USBA_BF(BK_NUMBER, USBA_BK_NUMBER_ONE);
> diff --git a/drivers/usb/gadget/dummy_hcd.c b/drivers/usb/gadget/dummy_hcd.c
> index 9064696..f51ae30 100644
> --- a/drivers/usb/gadget/dummy_hcd.c
> +++ b/drivers/usb/gadget/dummy_hcd.c
> @@ -356,7 +356,7 @@ dummy_enable (struct usb_ep *_ep, const struct usb_endpoint_descriptor *desc)
>  	 * especially for "ep9out" style fixed function ones.)
>  	 */
>  	retval = -EINVAL;
> -	switch (desc->bmAttributes & 0x03) {
> +	switch (usb_endpoint_type(desc)) {
>  	case USB_ENDPOINT_XFER_BULK:
>  		if (strstr (ep->ep.name, "-iso")
>  				|| strstr (ep->ep.name, "-int")) {
> @@ -423,8 +423,8 @@ dummy_enable (struct usb_ep *_ep, const struct usb_endpoint_descriptor *desc)
>  
>  	dev_dbg (udc_dev(dum), "enabled %s (ep%d%s-%s) maxpacket %d\n",
>  		_ep->name,
> -		desc->bEndpointAddress & 0x0f,
> -		(desc->bEndpointAddress & USB_DIR_IN) ? "in" : "out",
> +		usb_endpoint_num(desc),
> +		usb_endpoint_dir_in(desc) ? "in" : "out",
>  		({ char *val;
>  		 switch (desc->bmAttributes & 0x03) {
>  		 case USB_ENDPOINT_XFER_BULK: val = "bulk"; break;
> @@ -533,7 +533,7 @@ dummy_queue (struct usb_ep *_ep, struct usb_request *_req,
>  	spin_lock_irqsave (&dum->lock, flags);
>  
>  	/* implement an emulated single-request FIFO */
> -	if (ep->desc && (ep->desc->bEndpointAddress & USB_DIR_IN) &&
> +	if (ep->desc && usb_endpoint_dir_in(ep->desc) &&
>  			list_empty (&dum->fifo_req.queue) &&
>  			list_empty (&ep->queue) &&
>  			_req->length <= FIFO_SIZE) {
> @@ -612,7 +612,7 @@ dummy_set_halt_and_wedge(struct usb_ep *_ep, int value, int wedged)
>  		return -ESHUTDOWN;
>  	if (!value)
>  		ep->halted = ep->wedged = 0;
> -	else if (ep->desc && (ep->desc->bEndpointAddress & USB_DIR_IN) &&
> +	else if (ep->desc && usb_endpoint_dir_in(ep->desc) &&
>  			!list_empty (&ep->queue))
>  		return -EAGAIN;
>  	else {
> diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c
> index a36b117..8b3f994 100644
> --- a/drivers/usb/gadget/epautoconf.c
> +++ b/drivers/usb/gadget/epautoconf.c
> @@ -75,7 +76,7 @@ ep_matches (
>  		return 0;
>  
>  	/* only support ep0 for portable CONTROL traffic */
> -	type = desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK;
> +	type = usb_endpoint_type(desc);
>  	if (USB_ENDPOINT_XFER_CONTROL == type)
>  		return 0;
>  
> @@ -118,7 +119,7 @@ ep_matches (
>  		/* direction-restriction:  "..in-..", "out-.." */
>  		tmp--;
>  		if (!isdigit (*tmp)) {
> -			if (desc->bEndpointAddress & USB_DIR_IN) {
> +			if (usb_endpoint_dir_in(desc)) {
>  				if ('n' != *tmp)
>  					return 0;
>  			} else {
> @@ -164,7 +165,7 @@ ep_matches (
>  		u8	num = simple_strtoul (&ep->name [2], NULL, 10);
>  		desc->bEndpointAddress |= num;
>  #ifdef	MANY_ENDPOINTS
> -	} else if (desc->bEndpointAddress & USB_DIR_IN) {
> +	} else if (usb_endpoint_dir_in(desc)) {
>  		if (++in_epnum > 15)
>  			return 0;
>  		desc->bEndpointAddress = USB_DIR_IN | in_epnum;
> @@ -237,7 +238,7 @@ struct usb_ep * __init usb_ep_autoconfig (
>  	struct usb_ep	*ep;
>  	u8		type;
>  
> -	type = desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK;
> +	type = usb_endpoint_type(desc);
>  
>  	/* First, apply chip-specific "best usage" knowledge.
>  	 * This might make a good usb_gadget_ops hook ...
> @@ -258,7 +259,7 @@ struct usb_ep * __init usb_ep_autoconfig (
>  			if (ep && ep_matches (gadget, ep, desc))
>  				return ep;
>  		} else if (USB_ENDPOINT_XFER_BULK == type
> -				&& (USB_DIR_IN & desc->bEndpointAddress)) {
> +				&& usb_endpoint_dir_in(desc)) {
>  			/* DMA may be available */
>  			ep = find_ep (gadget, "ep2-bulk");
>  			if (ep && ep_matches (gadget, ep, desc))
> diff --git a/drivers/usb/gadget/fsl_qe_udc.c b/drivers/usb/gadget/fsl_qe_udc.c
> index d6c5bcd..5ae4bdb 100644
> --- a/drivers/usb/gadget/fsl_qe_udc.c
> +++ b/drivers/usb/gadget/fsl_qe_udc.c
> @@ -488,10 +489,10 @@ static int qe_ep_register_init(struct qe_udc *udc, unsigned char pipe_num)
>  	epparam = udc->ep_param[pipe_num];
>  
>  	usep = 0;
> -	logepnum = (ep->desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK);
> +	logepnum = usb_endpoint_num(ep->desc);
>  	usep |= (logepnum << USB_EPNUM_SHIFT);
>  
> -	switch (ep->desc->bmAttributes & 0x03) {
> +	switch (usb_endpoint_type(ep->desc)) {
>  	case USB_ENDPOINT_XFER_BULK:
>  		usep |= USB_TRANS_BULK;
>  		break;
> @@ -545,7 +546,7 @@ static int qe_ep_init(struct qe_udc *udc,
>  	/* Refer to USB2.0 spec table 9-13,
>  	*/
>  	if (pipe_num != 0) {
> -		switch (desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) {
> +		switch (usb_endpoint_type(desc)) {
>  		case USB_ENDPOINT_XFER_BULK:
>  			if (strstr(ep->ep.name, "-iso")
>  					|| strstr(ep->ep.name, "-int"))
> @@ -642,7 +643,7 @@ static int qe_ep_init(struct qe_udc *udc,
>  
>  	/* initialize ep structure */
>  	ep->ep.maxpacket = max;
> -	ep->tm = (u8)(desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK);
> +	ep->tm = (u8)usb_endpoint_type(desc);
>  	ep->desc = desc;
>  	ep->stopped = 0;
>  	ep->init = 1;
> diff --git a/drivers/usb/gadget/fsl_usb2_udc.c b/drivers/usb/gadget/fsl_usb2_udc.c
> index f3c6703..7e06e51 100644
> --- a/drivers/usb/gadget/fsl_usb2_udc.c
> +++ b/drivers/usb/gadget/fsl_usb2_udc.c
> @@ -465,7 +466,7 @@ static int fsl_ep_enable(struct usb_ep *_ep,
>  	zlt = 1;
>  
>  	/* Assume the max packet size from gadget is always correct */
> -	switch (desc->bmAttributes & 0x03) {
> +	switch (usb_endpoint_type(desc)) {
>  	case USB_ENDPOINT_XFER_CONTROL:
>  	case USB_ENDPOINT_XFER_BULK:
>  	case USB_ENDPOINT_XFER_INT:
> @@ -496,25 +497,23 @@ static int fsl_ep_enable(struct usb_ep *_ep,
>  	/* Init EPx Queue Head (Ep Capabilites field in QH
>  	 * according to max, zlt, mult) */
>  	struct_ep_qh_setup(udc, (unsigned char) ep_index(ep),
> -			(unsigned char) ((desc->bEndpointAddress & USB_DIR_IN)
> +			(unsigned char) (usb_endpoint_dir_in(desc)
>  					?  USB_SEND : USB_RECV),
> -			(unsigned char) (desc->bmAttributes
> -					& USB_ENDPOINT_XFERTYPE_MASK),
> +			(unsigned char) usb_endpoint_type(desc),
>  			max, zlt, mult);
>  
>  	/* Init endpoint ctrl register */
>  	dr_ep_setup((unsigned char) ep_index(ep),
> -			(unsigned char) ((desc->bEndpointAddress & USB_DIR_IN)
> +			(unsigned char) (usb_endpoint_dir_in(desc)
>  					? USB_SEND : USB_RECV),
> -			(unsigned char) (desc->bmAttributes
> -					& USB_ENDPOINT_XFERTYPE_MASK));
> +			(unsigned char) usb_endpoint_type(desc));
>  
>  	spin_unlock_irqrestore(&udc->lock, flags);
>  	retval = 0;
>  
>  	VDBG("enabled %s (ep%d%s) maxpacket %d",ep->ep.name,
> -			ep->desc->bEndpointAddress & 0x0f,
> -			(desc->bEndpointAddress & USB_DIR_IN)
> +			usb_endpoint_num(ep->desc),
> +			usb_endpoint_dir_in(desc)
>  				? "in" : "out", max);
>  en_done:
>  	return retval;
> diff --git a/drivers/usb/gadget/goku_udc.c b/drivers/usb/gadget/goku_udc.c
> index 60aa048..70d38b8 100644
> --- a/drivers/usb/gadget/goku_udc.c
> +++ b/drivers/usb/gadget/goku_udc.c
> @@ -110,10 +111,10 @@ goku_ep_enable(struct usb_ep *_ep, const struct usb_endpoint_descriptor *desc)
>  		return -EINVAL;
>  	if (!dev->driver || dev->gadget.speed == USB_SPEED_UNKNOWN)
>  		return -ESHUTDOWN;
> -	if (ep->num != (desc->bEndpointAddress & 0x0f))
> +	if (ep->num != usb_endpoint_num(desc))
>  		return -EINVAL;
>  
> -	switch (desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) {
> +	switch (usb_endpoint_type(desc)) {
>  	case USB_ENDPOINT_XFER_BULK:
>  	case USB_ENDPOINT_XFER_INT:
>  		break;
> diff --git a/drivers/usb/gadget/m66592-udc.c b/drivers/usb/gadget/m66592-udc.c
> index 43dcf9e..ed29838 100644
> --- a/drivers/usb/gadget/m66592-udc.c
> +++ b/drivers/usb/gadget/m66592-udc.c
> @@ -395,7 +396,7 @@ static void m66592_ep_setting(struct m66592 *m66592, struct m66592_ep *ep,
>  	ep->pipenum = pipenum;
>  	ep->ep.maxpacket = le16_to_cpu(desc->wMaxPacketSize);
>  	m66592->pipenum2ep[pipenum] = ep;
> -	m66592->epaddr2ep[desc->bEndpointAddress&USB_ENDPOINT_NUMBER_MASK] = ep;
> +	m66592->epaddr2ep[usb_endpoint_num(desc)] = ep;
>  	INIT_LIST_HEAD(&ep->queue);
>  }
>  
> @@ -427,7 +428,7 @@ static int alloc_pipe_config(struct m66592_ep *ep,
>  
>  	BUG_ON(ep->pipenum);
>  
> -	switch (desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) {
> +	switch (usb_endpoint_type(desc)) {
>  	case USB_ENDPOINT_XFER_BULK:
>  		if (m66592->bulk >= M66592_MAX_NUM_BULK) {
>  			if (m66592->isochronous >= M66592_MAX_NUM_ISOC) {
> @@ -469,10 +470,10 @@ static int alloc_pipe_config(struct m66592_ep *ep,
>  	}
>  	ep->type = info.type;
>  
> -	info.epnum = desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK;
> +	info.epnum = usb_endpoint_num(desc);
>  	info.maxpacket = le16_to_cpu(desc->wMaxPacketSize);
>  	info.interval = desc->bInterval;
> -	if (desc->bEndpointAddress & USB_ENDPOINT_DIR_MASK)
> +	if (usb_endpoint_dir_in(desc))
>  		info.dir_in = 1;
>  	else
>  		info.dir_in = 0;
> @@ -591,7 +592,7 @@ static void start_packet_read(struct m66592_ep *ep, struct m66592_request *req)
>  
>  static void start_packet(struct m66592_ep *ep, struct m66592_request *req)
>  {
> -	if (ep->desc->bEndpointAddress & USB_DIR_IN)
> +	if (usb_endpoint_dir_in(ep->desc))
>  		start_packet_write(ep, req);
>  	else
>  		start_packet_read(ep, req);
> @@ -905,7 +906,7 @@ static void irq_pipe_ready(struct m66592 *m66592, u16 status, u16 enb)
>  				ep = m66592->pipenum2ep[pipenum];
>  				req = list_entry(ep->queue.next,
>  						 struct m66592_request, queue);
> -				if (ep->desc->bEndpointAddress & USB_DIR_IN)
> +				if (usb_endpoint_dir_in(ep->desc))
>  					irq_packet_write(ep, req);
>  				else
>  					irq_packet_read(ep, req);
> diff --git a/drivers/usb/gadget/net2280.c b/drivers/usb/gadget/net2280.c
> index 12c6d83..8a1e5a7 100644
> --- a/drivers/usb/gadget/net2280.c
> +++ b/drivers/usb/gadget/net2280.c
> @@ -164,7 +165,7 @@ net2280_enable (struct usb_ep *_ep, const struct usb_endpoint_descriptor *desc)
>  		return -ESHUTDOWN;
>  
>  	/* erratum 0119 workaround ties up an endpoint number */
> -	if ((desc->bEndpointAddress & 0x0f) == EP_DONTUSE)
> +	if (usb_endpoint_num(desc) == EP_DONTUSE)
>  		return -EDOM;
>  
>  	/* sanity check ep-e/ep-f since their fifos are small */
> @@ -195,12 +196,12 @@ net2280_enable (struct usb_ep *_ep, const struct usb_endpoint_descriptor *desc)
>  
>  	/* set type, direction, address; reset fifo counters */
>  	writel ((1 << FIFO_FLUSH), &ep->regs->ep_stat);
> -	tmp = (desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK);
> +	tmp = usb_endpoint_type(desc);
>  	if (tmp == USB_ENDPOINT_XFER_INT) {
>  		/* erratum 0105 workaround prevents hs NYET */
>  		if (dev->chiprev == 0100
>  				&& dev->gadget.speed == USB_SPEED_HIGH
> -				&& !(desc->bEndpointAddress & USB_DIR_IN))
> +				&& !usb_endpoint_dir_in(desc))
>  			writel ((1 << CLEAR_NAK_OUT_PACKETS_MODE),
>  				&ep->regs->ep_rsp);
>  	} else if (tmp == USB_ENDPOINT_XFER_BULK) {
> @@ -1230,8 +1231,7 @@ net2280_set_halt_and_wedge(struct usb_ep *_ep, int value, int wedged)
>  		return -EINVAL;
>  	if (!ep->dev->driver || ep->dev->gadget.speed == USB_SPEED_UNKNOWN)
>  		return -ESHUTDOWN;
> -	if (ep->desc /* not ep0 */ && (ep->desc->bmAttributes & 0x03)
> -						== USB_ENDPOINT_XFER_ISOC)
> +	if (ep->desc /* not ep0 */ && usb_endpoint_xfer_isoc(ep->desc))
>  		return -EINVAL;
>  
>  	spin_lock_irqsave (&ep->dev->lock, flags);
> diff --git a/drivers/usb/gadget/s3c2410_udc.c b/drivers/usb/gadget/s3c2410_udc.c
> index 9a2b892..31a7617 100644
> --- a/drivers/usb/gadget/s3c2410_udc.c
> +++ b/drivers/usb/gadget/s3c2410_udc.c
> @@ -1077,7 +1077,7 @@ static int s3c2410_udc_ep_enable(struct usb_ep *_ep,
>  	udc_write(max >> 3, S3C2410_UDC_MAXP_REG);
>  
>  	/* set type, direction, address; reset fifo counters */
> -	if (desc->bEndpointAddress & USB_DIR_IN) {
> +	if (usb_endpoint_dir_in(desc)) {
>  		csr1 = S3C2410_UDC_ICSR1_FFLUSH|S3C2410_UDC_ICSR1_CLRDT;
>  		csr2 = S3C2410_UDC_ICSR2_MODEIN|S3C2410_UDC_ICSR2_DMAIEN;
>  
> @@ -1112,7 +1112,7 @@ static int s3c2410_udc_ep_enable(struct usb_ep *_ep,
>  	tmp = desc->bEndpointAddress;
>  	dprintk (DEBUG_NORMAL, "enable %s(%d) ep%x%s-blk max %02x\n",
>  		 _ep->name,ep->num, tmp,
> -		 desc->bEndpointAddress & USB_DIR_IN ? "in" : "out", max);
> +		 usb_endpoint_dir_in(desc) ? "in" : "out", max);
>  
>  	local_irq_restore (flags);
>  	s3c2410_udc_set_halt(_ep, 0);
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists