| Message ID | 1450556218-26067-2-git-send-email-stefan.bruens@rwth-aachen.de |
|---|---|
| State | Superseded |
| Headers | show |
Hi, On 19-12-15 21:16, Stefan Brüns wrote: > The ifdef'ed Linux kernel code uses the 1 based port number, whereas U-Boot > puts a 0 based port number into the register. The reason the 0 based port > number apparently works can probably be taken from the USB 2.0 spec: > > 8.4.2.2 Start-Split Transaction Token > ... The host must correctly set the port field for single and multiple TT > hub implementations. A single TT hub implementation *may ignore* the port > field. > > Actually, as far as I unterstand, a multi TT hub defaults to single TT > (bAlternateSetting: 0) until switched via SetInterface, so even "port 42" > would work. > > The change was verified by hardcoding the port number to a wrong value, > SPLIT transactions kept working (although using a DWC2 instead of MUSB). > Tested hubs are the RPi onboard SMC9514 and an external "05e3:0608 > Genesys Logic, Inc. USB-2.0 4-Port HUB". The former is a multi TT hub, > the latter single TT only. > > Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de> > --- > drivers/usb/musb-new/musb_host.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/musb-new/musb_host.c b/drivers/usb/musb-new/musb_host.c > index fc4ba62..5c028d4 100644 > --- a/drivers/usb/musb-new/musb_host.c > +++ b/drivers/usb/musb-new/musb_host.c > @@ -2096,7 +2096,7 @@ int musb_urb_enqueue( > uint8_t hubaddr = 0; > usb_find_usb2_hub_address_port(udev, &hubaddr, &portnr) > qh->h_addr_reg = hubaddr; > - qh->h_port_reg = portnr - 1; > + qh->h_port_reg = portnr; > } > #endif > } > This patch needs to be re-spun to apply on top of the compile fixed [patch 1/2] of this set. With that done this patch is: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Tested-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans
On Monday, December 21, 2015 at 08:33:31 PM, Hans de Goede wrote: > Hi, > > On 19-12-15 21:16, Stefan Brüns wrote: > > The ifdef'ed Linux kernel code uses the 1 based port number, whereas > > U-Boot puts a 0 based port number into the register. The reason the 0 > > based port number apparently works can probably be taken from the USB > > 2.0 spec: > > > > 8.4.2.2 Start-Split Transaction Token > > ... The host must correctly set the port field for single and multiple TT > > hub implementations. A single TT hub implementation *may ignore* the port > > field. > > > > Actually, as far as I unterstand, a multi TT hub defaults to single TT > > (bAlternateSetting: 0) until switched via SetInterface, so even "port 42" > > would work. > > > > The change was verified by hardcoding the port number to a wrong value, > > SPLIT transactions kept working (although using a DWC2 instead of MUSB). > > Tested hubs are the RPi onboard SMC9514 and an external "05e3:0608 > > Genesys Logic, Inc. USB-2.0 4-Port HUB". The former is a multi TT hub, > > the latter single TT only. > > > > Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de> > > --- > > > > drivers/usb/musb-new/musb_host.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/usb/musb-new/musb_host.c > > b/drivers/usb/musb-new/musb_host.c index fc4ba62..5c028d4 100644 > > --- a/drivers/usb/musb-new/musb_host.c > > +++ b/drivers/usb/musb-new/musb_host.c > > @@ -2096,7 +2096,7 @@ int musb_urb_enqueue( > > > > uint8_t hubaddr = 0; > > usb_find_usb2_hub_address_port(udev, &hubaddr, &portnr) > > qh->h_addr_reg = hubaddr; > > > > - qh->h_port_reg = portnr - 1; > > + qh->h_port_reg = portnr; > > > > } > > > > #endif > > > > } > > This patch needs to be re-spun to apply on top of the compile fixed [patch > 1/2] of this set. With that done this patch is: > > Reviewed-by: Hans de Goede <hdegoede@redhat.com> > Tested-by: Hans de Goede <hdegoede@redhat.com> OK, would you, Stefan, please resend the patchset with the necessary Reviewed-by etc so I can pick it ? This thread is a chaos and I am lost. Thanks! Best regards, Marek Vasut
On Monday 21 December 2015 21:27:01 Marek Vasut wrote: > On Monday, December 21, 2015 at 08:33:31 PM, Hans de Goede wrote: > > This patch needs to be re-spun to apply on top of the compile fixed [patch > > 1/2] of this set. With that done this patch is: > > > > Reviewed-by: Hans de Goede <hdegoede@redhat.com> > > Tested-by: Hans de Goede <hdegoede@redhat.com> > > OK, would you, Stefan, please resend the patchset with the necessary > Reviewed-by etc so I can pick it ? This thread is a chaos and I am lost. > Hi, is it okay if I send just the patches which are cleanup/bugfix work, as I am currently reworking the DWC split support? Currently this is: 1. Dynamic allocation of configuration descriptor (1 patch) 2. Factor out function for hub address/port, fix musb port number (2 patches) 3. Fix out-of-bound access in DWC (1 patch) These four patches are the first 4 patches from the old 7 patch series, so have been reviewed. Kind regards, Stefan
On Tuesday, December 22, 2015 at 12:02:44 AM, Stefan Bruens wrote: > On Monday 21 December 2015 21:27:01 Marek Vasut wrote: > > On Monday, December 21, 2015 at 08:33:31 PM, Hans de Goede wrote: > > > This patch needs to be re-spun to apply on top of the compile fixed > > > [patch 1/2] of this set. With that done this patch is: > > > > > > Reviewed-by: Hans de Goede <hdegoede@redhat.com> > > > Tested-by: Hans de Goede <hdegoede@redhat.com> > > > > OK, would you, Stefan, please resend the patchset with the necessary > > Reviewed-by etc so I can pick it ? This thread is a chaos and I am lost. > > Hi, > > is it okay if I send just the patches which are cleanup/bugfix work, as I > am currently reworking the DWC split support? Currently this is: > > 1. Dynamic allocation of configuration descriptor (1 patch) > 2. Factor out function for hub address/port, fix musb port number (2 > patches) 3. Fix out-of-bound access in DWC (1 patch) > > These four patches are the first 4 patches from the old 7 patch series, so > have been reviewed. That's fine, thanks! Best regards, Marek Vasut
diff --git a/drivers/usb/musb-new/musb_host.c b/drivers/usb/musb-new/musb_host.c index fc4ba62..5c028d4 100644 --- a/drivers/usb/musb-new/musb_host.c +++ b/drivers/usb/musb-new/musb_host.c @@ -2096,7 +2096,7 @@ int musb_urb_enqueue( uint8_t hubaddr = 0; usb_find_usb2_hub_address_port(udev, &hubaddr, &portnr) qh->h_addr_reg = hubaddr; - qh->h_port_reg = portnr - 1; + qh->h_port_reg = portnr; } #endif }
The ifdef'ed Linux kernel code uses the 1 based port number, whereas U-Boot puts a 0 based port number into the register. The reason the 0 based port number apparently works can probably be taken from the USB 2.0 spec: 8.4.2.2 Start-Split Transaction Token ... The host must correctly set the port field for single and multiple TT hub implementations. A single TT hub implementation *may ignore* the port field. Actually, as far as I unterstand, a multi TT hub defaults to single TT (bAlternateSetting: 0) until switched via SetInterface, so even "port 42" would work. The change was verified by hardcoding the port number to a wrong value, SPLIT transactions kept working (although using a DWC2 instead of MUSB). Tested hubs are the RPi onboard SMC9514 and an external "05e3:0608 Genesys Logic, Inc. USB-2.0 4-Port HUB". The former is a multi TT hub, the latter single TT only. Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de> --- drivers/usb/musb-new/musb_host.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)