[PATCH v1 3/4] usb: usb-uclass.c: Drop le16_to_cpu() as values are already swapped

Bin Meng bmeng.cn at gmail.com
Fri Jul 17 12:11:22 CEST 2020


Hi Stefan,

On Fri, Jul 17, 2020 at 6:08 PM Stefan Roese <sr at denx.de> wrote:
>
> On 17.07.20 07:33, Bin Meng wrote:
> > Hi Stefan,
> >
> > On Thu, Jul 2, 2020 at 4:47 PM Stefan Roese <sr at denx.de> 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... ;)
>

Ah, yes! I misread this as an unconditional swap ...

> >> again is a bug. Let's remove the swap here instead.
> >>
> >> Signed-off-by: Stefan Roese <sr at denx.de>
> >> Cc: Bin Meng <bmeng.cn at gmail.com>
> >> Cc: Marek Vasut <marex at denx.de>
> >> ---
> >>
> >>   drivers/usb/host/usb-uclass.c | 8 ++++----
> >>   1 file changed, 4 insertions(+), 4 deletions(-)
> >>

Regards,
Bin


More information about the U-Boot mailing list