From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Roese Date: Fri, 17 Jul 2020 12:08:13 +0200 Subject: [PATCH v1 3/4] usb: usb-uclass.c: Drop le16_to_cpu() as values are already swapped In-Reply-To: References: <20200702084733.2032531-1-sr@denx.de> <20200702084733.2032531-4-sr@denx.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 17.07.20 07:33, Bin Meng wrote: > Hi Stefan, > > On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese wrote: >> >> These values are already swapped to CPU endianess, so swapping them > > Can you please add more details as to when these values are swapped? I > assume this is inside usb_select_config() which is called before this > function is called? Yes. Its swapped exactly here: int usb_select_config(struct usb_device *dev) { unsigned char *tmpbuf = NULL; int err; err = get_descriptor_len(dev, USB_DT_DEVICE_SIZE, USB_DT_DEVICE_SIZE); if (err) return err; /* correct le values */ le16_to_cpus(&dev->descriptor.bcdUSB); le16_to_cpus(&dev->descriptor.idVendor); le16_to_cpus(&dev->descriptor.idProduct); le16_to_cpus(&dev->descriptor.bcdDevice); > But I wonder how this code ever worked on ARM/x86? On ARM/x86, the little-endian swapping is a no-op. So it doesn't matter if you swap once or twice or... ;) >> again is a bug. Let's remove the swap here instead. >> >> Signed-off-by: Stefan Roese >> Cc: Bin Meng >> Cc: Marek Vasut >> --- >> >> drivers/usb/host/usb-uclass.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c >> index cb79dfbbd5..aa08d4ffc6 100644 >> --- a/drivers/usb/host/usb-uclass.c >> +++ b/drivers/usb/host/usb-uclass.c >> @@ -416,21 +416,21 @@ static int usb_match_device(const struct usb_device_descriptor *desc, >> const struct usb_device_id *id) >> { >> if ((id->match_flags & USB_DEVICE_ID_MATCH_VENDOR) && >> - id->idVendor != le16_to_cpu(desc->idVendor)) >> + id->idVendor != desc->idVendor) >> return 0; >> >> if ((id->match_flags & USB_DEVICE_ID_MATCH_PRODUCT) && >> - id->idProduct != le16_to_cpu(desc->idProduct)) >> + id->idProduct != desc->idProduct) >> return 0; >> >> /* No need to test id->bcdDevice_lo != 0, since 0 is never >> greater than any unsigned number. */ >> if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_LO) && >> - (id->bcdDevice_lo > le16_to_cpu(desc->bcdDevice))) >> + (id->bcdDevice_lo > desc->bcdDevice)) >> return 0; >> >> if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_HI) && >> - (id->bcdDevice_hi < le16_to_cpu(desc->bcdDevice))) >> + (id->bcdDevice_hi < desc->bcdDevice)) >> return 0; >> >> if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_CLASS) && > > Reviewed-by: Bin Meng > > Regards, > Bin Thanks, Stefan