On Sun, 2019-07-21 at 19:32 +0100, Pawel Laszczak wrote:
> Patch moves some decoding functions from driver/usb/dwc3/debug.h driver
> to driver/usb/common/debug.c file. These moved functions include:
[]
> diff --git a/drivers/usb/common/debug.c b/drivers/usb/common/debug.c
[]
> +static void usb_decode_set_clear_feature(__u8 bRequestType, __u8 bRequest,
> + __u16 wValue, __u16 wIndex,
> + char *str, size_t size)
It's probably not necessary to use Hungarian
when moving these functions into generic code.
> +{
> + switch (bRequestType & USB_RECIP_MASK) {
> + case USB_RECIP_DEVICE:
> + snprintf(str, size, "%s Device Feature(%s%s)",
> + bRequest == USB_REQ_CLEAR_FEATURE ? "Clear" : "Set",
> + ({char *s;
> + switch (wValue) {
> + case USB_DEVICE_SELF_POWERED:
> + s = "Self Powered";
> + break;
> + case USB_DEVICE_REMOTE_WAKEUP:
> + s = "Remote Wakeup";
> + break;
> + case USB_DEVICE_TEST_MODE:
> + s = "Test Mode";
> + break;
> + case USB_DEVICE_U1_ENABLE:
> + s = "U1 Enable";
> + break;
> + case USB_DEVICE_U2_ENABLE:
> + s = "U2 Enable";
> + break;
> + case USB_DEVICE_LTM_ENABLE:
> + s = "LTM Enable";
> + break;
> + default:
> + s = "UNKNOWN";
> + } s; }),
> + wValue == USB_DEVICE_TEST_MODE ?
> + ({ char *s;
> + switch (wIndex) {
> + case TEST_J:
> + s = ": TEST_J";
> + break;
> + case TEST_K:
> + s = ": TEST_K";
> + break;
> + case TEST_SE0_NAK:
> + s = ": TEST_SE0_NAK";
> + break;
> + case TEST_PACKET:
> + s = ": TEST_PACKET";
> + break;
> + case TEST_FORCE_EN:
> + s = ": TEST_FORCE_EN";
> + break;
> + default:
> + s = ": UNKNOWN";
> + } s; }) : "");
I always find this sort of embedded switch/case char *
statement expressions difficult to read and think it's
better to use separate lookup functions.
I would much prefer something like:
static const char *usb_device_mode_desc(u16 mode)
{
switch (mode) {
case USB_DEVICE_SELF_POWERED:
return "Self Powered";
case USB_DEVICE_REMOTE_WAKEUP:
return "Remote Wakeup";
case USB_DEVICE_TEST_MODE:
return "Test Mode";
case USB_DEVICE_U1_ENABLE:
return "U1 Enable";
case USB_DEVICE_U2_ENABLE:
return "U2 Enable";
case USB_DEVICE_LTM_ENABLE:
return "LTM Enable";
}
return "UNKNOWN";
}
snprintf(str, size, "%s Device Feature(%s%s)",
> bRequest == USB_REQ_CLEAR_FEATURE ? "Clear" : "Set",
usb_device_mode_desc(wValue),
etc...);
etc...
