[U-Boot] [PATCH v3 2/2] usb: Add CONFIG to fetch string descriptor

Marek Vasut
2012-03-01T11:45:03Z

Hi!

> Hi Marek,
>
> On Thursday 01 March 2012 02:59 AM, Marek Vasut wrote:
> >> Add "CONFIG_USB_STRING_FETCH" to fetch first string descriptor length
> >> and then pass this length to fetch string descriptor.
> >>
> >> Signed-off-by: Puneet Saxena<puneets@nvidia.com>
> >> ---
> >>
> >> Changes for V2:
> >> - Change existing config by "CONFIG_USB_STRING_FETCH"
> >>
> >> Changes for V3:
> >> - Removed extra new line
> >> - Explained "CONFIG_USB_STRING_FETCH" in top level README
> >>
> >> README | 4 ++++
> >> common/usb.c | 4 ++++
> >> include/configs/tegra2-common.h | 2 ++
> >> 3 files changed, 10 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/README b/README
> >> index 7adf7c7..c045a37 100644
> >> --- a/README
> >> +++ b/README
> >>
> >> @@ -1138,6 +1138,10 @@ The following options need to be configured:
> >> CONFIG_USB_EHCI_TXFIFO_THRESH enables setting of the
> >> txfilltuning field in the EHCI controller on reset.
> >>
> >> + CONFIG_USB_STRING_FETCH
> >> + Enables settings to USB core to handle string issues which
> >> + few devices can not handle.
> >> +
> >>
> >> - USB Device:
> >> Define the below if you wish to use the USB console.
> >> Once firmware is rebuilt from a serial console issue the
> >>
> >> diff --git a/common/usb.c b/common/usb.c
> >> index 191bc5b..a73cb60 100644
> >> --- a/common/usb.c
> >> +++ b/common/usb.c
> >> @@ -658,9 +658,13 @@ static int usb_string_sub(struct usb_device *dev,
> >> unsigned int langid, {
> >>
> >> int rc;
> >>
> >> +#ifdef CONFIG_USB_STRING_FETCH
> >
> > Shouldn't this be something like ... CONFIG_USB_AVOID_STRING_FETCH then?
> >
> > Anyway, how come some devices can't handle it? What happens then? What
> > devices are those (exact type etc)?
> >
> > I believe the bug is deeper and adding extra config options can be
> > avoided, what do you think?
> >
> > Thanks!
> >
> > M
>
> It does not avoid string fetch.

Well it does certainly not call usb_get_string()

> I checked with few mass storage devices that they does not handle string
> descriptor request correctly and so we get
> start/stop Cache alignment error.

Cache alignment error ? Wow, how's that actually related to SUB string fetching?
Maybe we should manually realign the result then? I still don't really
understand what you're seeing, sorry ... can you please elaborate?

> One way is, blacklist those devices by
> vendor id/product id..
> etc. This is done in Linux kernel. Plz see Message.c (drivers\usb\core)
> Line: 722.
> Blacklisting the device requires a framework to detect the
> device...However this could be achieved
> simply with this implementation.

Sure, I see your intention, but I still don't get the problem, see above. Can
you also tell me if there's particular device that's broken and which one is it
(precise type etc)?

Thanks a lot for your cooperation on this matter!

M

>
> >> + rc = -1;
> >> +#else
> >>
> >> /* Try to read the string descriptor by asking for the maximum
> >>
> >> * possible number of bytes */
> >>
> >> rc = usb_get_string(dev, langid, index, buf, 255);
> >>
> >> +#endif
> >>
> >> /* If that failed try to read the descriptor length, then
> >>
> >> * ask for just that many bytes */
> >>
> >> diff --git a/include/configs/tegra2-common.h
> >> b/include/configs/tegra2-common.h index 266d0e5..d20b49c 100644
> >> --- a/include/configs/tegra2-common.h
> >> +++ b/include/configs/tegra2-common.h
> >> @@ -93,6 +93,8 @@
> >>
> >> #define CONFIG_USB_EHCI_TXFIFO_THRESH 10
> >> #define CONFIG_EHCI_IS_TDI
> >> #define CONFIG_EHCI_DCACHE
> >>
> >> +/* string descriptors must not be fetched using a 255-byte read */
> >> +#define CONFIG_USB_STRING_FETCH
> >>
> >> /* include default commands */
> >> #include<config_cmd_default.h>