| Message ID | 1394712342-15778-373-Taiwan-albertk@realtek.com |
|---|---|
| State | Rejected |
| Delegated to: | Marek Vasut |
| Headers | show |
| Series | eth/r8152: minor corrections | expand |
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 ?
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
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.
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 --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; } }
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(-)