usb: common: debug: Hex dump non-standard control requests

usb_decode_ctrl() only decodes standard control requests. Don't attempt
to decode non-standard requests. Just dump the content of the requests.

Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
---
 drivers/usb/common/debug.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)


base-commit: 46d2c20b0b10cf07a2a24b047a09195ba96c84f7

On Fri, Mar 25, 2022 at 05:33:04PM -0700, Thinh Nguyen wrote:
> usb_decode_ctrl() only decodes standard control requests. Don't attempt
> to decode non-standard requests. Just dump the content of the requests.
> 
> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> ---
>  drivers/usb/common/debug.c | 31 +++++++++++++++++++++++--------
>  1 file changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/usb/common/debug.c b/drivers/usb/common/debug.c
> index 075f6b1b2a1a..cb38725f9276 100644
> --- a/drivers/usb/common/debug.c
> +++ b/drivers/usb/common/debug.c
> @@ -208,6 +208,20 @@ static void usb_decode_set_isoch_delay(__u8 wValue, char *str, size_t size)
>  	snprintf(str, size, "Set Isochronous Delay(Delay = %d ns)", wValue);
>  }
>  
> +static void usb_hex_dump_ctrl(char *str, size_t size, __u8 bRequestType,
> +			      __u8 bRequest, __u16 wValue, __u16 wIndex,
> +			      __u16 wLength)
> +{
> +	snprintf(str, size, "%02x %02x %02x %02x %02x %02x %02x %02x",
> +		 bRequestType, bRequest,
> +		 (u8)(cpu_to_le16(wValue) & 0xff),
> +		 (u8)(cpu_to_le16(wValue) >> 8),
> +		 (u8)(cpu_to_le16(wIndex) & 0xff),
> +		 (u8)(cpu_to_le16(wIndex) >> 8),
> +		 (u8)(cpu_to_le16(wLength) & 0xff),
> +		 (u8)(cpu_to_le16(wLength) >> 8));
> +}
> +
>  /**
>   * usb_decode_ctrl - Returns human readable representation of control request.
>   * @str: buffer to return a human-readable representation of control request.
> @@ -233,6 +247,12 @@ const char *usb_decode_ctrl(char *str, size_t size, __u8 bRequestType,
>  			    __u8 bRequest, __u16 wValue, __u16 wIndex,
>  			    __u16 wLength)
>  {
> +	if ((bRequestType & USB_TYPE_MASK) != USB_TYPE_STANDARD) {
> +		usb_hex_dump_ctrl(str, size, bRequestType, bRequest,
> +				  wValue, wIndex, wLength);
> +		return str;
> +	}

But why not try to decode the other types and say what they are?
Wouldn't that be more helpful?

thanks,

greg k-h

Hi Greg,

Greg Kroah-Hartman wrote:
> On Fri, Mar 25, 2022 at 05:33:04PM -0700, Thinh Nguyen wrote:
>> usb_decode_ctrl() only decodes standard control requests. Don't attempt
>> to decode non-standard requests. Just dump the content of the requests.
>>
>> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
>> ---
>>  drivers/usb/common/debug.c | 31 +++++++++++++++++++++++--------
>>  1 file changed, 23 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/usb/common/debug.c b/drivers/usb/common/debug.c
>> index 075f6b1b2a1a..cb38725f9276 100644
>> --- a/drivers/usb/common/debug.c
>> +++ b/drivers/usb/common/debug.c
>> @@ -208,6 +208,20 @@ static void usb_decode_set_isoch_delay(__u8 wValue, char *str, size_t size)
>>  	snprintf(str, size, "Set Isochronous Delay(Delay = %d ns)", wValue);
>>  }
>>  
>> +static void usb_hex_dump_ctrl(char *str, size_t size, __u8 bRequestType,
>> +			      __u8 bRequest, __u16 wValue, __u16 wIndex,
>> +			      __u16 wLength)
>> +{
>> +	snprintf(str, size, "%02x %02x %02x %02x %02x %02x %02x %02x",
>> +		 bRequestType, bRequest,
>> +		 (u8)(cpu_to_le16(wValue) & 0xff),
>> +		 (u8)(cpu_to_le16(wValue) >> 8),
>> +		 (u8)(cpu_to_le16(wIndex) & 0xff),
>> +		 (u8)(cpu_to_le16(wIndex) >> 8),
>> +		 (u8)(cpu_to_le16(wLength) & 0xff),
>> +		 (u8)(cpu_to_le16(wLength) >> 8));
>> +}
>> +
>>  /**
>>   * usb_decode_ctrl - Returns human readable representation of control request.
>>   * @str: buffer to return a human-readable representation of control request.
>> @@ -233,6 +247,12 @@ const char *usb_decode_ctrl(char *str, size_t size, __u8 bRequestType,
>>  			    __u8 bRequest, __u16 wValue, __u16 wIndex,
>>  			    __u16 wLength)
>>  {
>> +	if ((bRequestType & USB_TYPE_MASK) != USB_TYPE_STANDARD) {
>> +		usb_hex_dump_ctrl(str, size, bRequestType, bRequest,
>> +				  wValue, wIndex, wLength);
>> +		return str;
>> +	}
> 
> But why not try to decode the other types and say what they are?
> Wouldn't that be more helpful?
> 

I agree. It would be great if someone can enhance this to decode other
types also (probably class type only?). This is just a quick fix to make
sure it doesn't incorrectly decode the wrong type.

BR,
Thinh