diff mbox series

[u-boot,v2,1/2] eth/r8152: fix assigning the wrong endpoint

Message ID 1394712342-15778-373-Taiwan-albertk@realtek.com
State New
Delegated to: Marek Vasut
Headers show
Series eth/r8152: minor corrections | expand

Commit Message

Hayes Wang May 25, 2020, 7:47 a.m. UTC
Although I think it never occurs, the code doesn't make sense, because
it may allow to assign an IN endpoint to ss->ep_out.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/usb/eth/r8152.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

Comments

Marek Vasut May 25, 2020, 12:03 p.m. UTC | #1
On 5/25/20 9:47 AM, Hayes Wang wrote:
> Although I think it never occurs, the code doesn't make sense, because
> it may allow to assign an IN endpoint to ss->ep_out.
> 
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
> ---
>  drivers/usb/eth/r8152.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/usb/eth/r8152.c b/drivers/usb/eth/r8152.c
> index 61b8683230..9f7bc7986d 100644
> --- a/drivers/usb/eth/r8152.c
> +++ b/drivers/usb/eth/r8152.c
> @@ -1354,9 +1354,8 @@ int r8152_eth_probe(struct usb_device *dev, unsigned int ifnum,
>  	struct usb_interface *iface;
>  	struct usb_interface_descriptor *iface_desc;
>  	int ep_in_found = 0, ep_out_found = 0;
> -	int i;
> -
>  	struct r8152 *tp;
> +	int i;
>  
>  	/* let's examine the device now */
>  	iface = &dev->config.if_desc[ifnum];
> @@ -1399,16 +1398,15 @@ int r8152_eth_probe(struct usb_device *dev, unsigned int ifnum,
>  		if ((iface->ep_desc[i].bmAttributes &
>  		     USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_BULK) {
>  			u8 ep_addr = iface->ep_desc[i].bEndpointAddress;
> +
>  			if ((ep_addr & USB_DIR_IN) && !ep_in_found) {
>  				ss->ep_in = ep_addr &
>  					USB_ENDPOINT_NUMBER_MASK;
>  				ep_in_found = 1;
> -			} else {
> -				if (!ep_out_found) {
> -					ss->ep_out = ep_addr &
> -						USB_ENDPOINT_NUMBER_MASK;
> -					ep_out_found = 1;
> -				}
> +			} else if ((ep_addr & USB_DIR_OUT) && !ep_out_found) {


Sorry, I was wrong in my previous suggestion, the USB_DIR_OUT macro is
expanded to 0, so this patch cannot work. 2/2 is already upstream. Do
you have a chance to test these patches before sending them ?
Hayes Wang May 25, 2020, 12:52 p.m. UTC | #2
Marek Vasut [mailto:marex@denx.de]
> Sent: Monday, May 25, 2020 8:03 PM
[...]
				ep_out_found = 1;
> > -				}
> > +			} else if ((ep_addr & USB_DIR_OUT) && !ep_out_found) {
> 
> 
> Sorry, I was wrong in my previous suggestion, the USB_DIR_OUT macro is
> expanded to 0, so this patch cannot work. 2/2 is already upstream. Do
> you have a chance to test these patches before sending them ?

Excuse me. I test v1 only.
Do I have to resend v1 for patch #1?

Best Regards,
Hayes
Marek Vasut May 25, 2020, 1 p.m. UTC | #3
On 5/25/20 2:52 PM, Hayes Wang wrote:
> Marek Vasut [mailto:marex@denx.de]
>> Sent: Monday, May 25, 2020 8:03 PM
> [...]
> 				ep_out_found = 1;
>>> -				}
>>> +			} else if ((ep_addr & USB_DIR_OUT) && !ep_out_found) {
>>
>>
>> Sorry, I was wrong in my previous suggestion, the USB_DIR_OUT macro is
>> expanded to 0, so this patch cannot work. 2/2 is already upstream. Do
>> you have a chance to test these patches before sending them ?
> 
> Excuse me. I test v1 only.
> Do I have to resend v1 for patch #1?

I'll pick V1, no worries.
Hayes Wang May 26, 2020, 1:59 a.m. UTC | #4
Marek Vasut [mailto:marex@denx.de]
> Sent: Monday, May 25, 2020 9:01 PM
[...]
> > Excuse me. I test v1 only.
> > Do I have to resend v1 for patch #1?
> 
> I'll pick V1, no worries.

Thanks.

Best Regards,
Hayes
diff mbox series

Patch

diff --git a/drivers/usb/eth/r8152.c b/drivers/usb/eth/r8152.c
index 61b8683230..9f7bc7986d 100644
--- a/drivers/usb/eth/r8152.c
+++ b/drivers/usb/eth/r8152.c
@@ -1354,9 +1354,8 @@  int r8152_eth_probe(struct usb_device *dev, unsigned int ifnum,
 	struct usb_interface *iface;
 	struct usb_interface_descriptor *iface_desc;
 	int ep_in_found = 0, ep_out_found = 0;
-	int i;
-
 	struct r8152 *tp;
+	int i;
 
 	/* let's examine the device now */
 	iface = &dev->config.if_desc[ifnum];
@@ -1399,16 +1398,15 @@  int r8152_eth_probe(struct usb_device *dev, unsigned int ifnum,
 		if ((iface->ep_desc[i].bmAttributes &
 		     USB_ENDPOINT_XFERTYPE_MASK) == USB_ENDPOINT_XFER_BULK) {
 			u8 ep_addr = iface->ep_desc[i].bEndpointAddress;
+
 			if ((ep_addr & USB_DIR_IN) && !ep_in_found) {
 				ss->ep_in = ep_addr &
 					USB_ENDPOINT_NUMBER_MASK;
 				ep_in_found = 1;
-			} else {
-				if (!ep_out_found) {
-					ss->ep_out = ep_addr &
-						USB_ENDPOINT_NUMBER_MASK;
-					ep_out_found = 1;
-				}
+			} else if ((ep_addr & USB_DIR_OUT) && !ep_out_found) {
+				ss->ep_out = ep_addr &
+					USB_ENDPOINT_NUMBER_MASK;
+				ep_out_found = 1;
 			}
 		}