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

[Marek Vasut]

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 at 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>