2019-06-05 10:40:23

by Pawel Laszczak

[permalink] [raw]
Subject: [PATCH v7 0/6] Introduced new Cadence USBSS DRD Driver.

This patch introduces new Cadence USBSS DRD driver to Linux kernel.

The Cadence USBSS DRD Driver is a highly configurable IP Core which
can be instantiated as Dual-Role Device (DRD), Peripheral Only and
Host Only (XHCI)configurations.

The current driver has been validated with FPGA burned. We have support
for PCIe bus, which is used on FPGA prototyping.

The host side of USBSS-DRD controller is compliance with XHCI
specification, so it works with standard XHCI Linux driver.

Change since v6:
- Fixed issue with L1 support. Controller has issue with hardware
resuming from L1 state. It was fixed in software.
- Fixed issues related with Transfer Ring Size equal 2.
- Fixed issue with removing cdns3.ko module. Issue appeared on the latest
version of kernel.
- Added separate interrupt resources for host, device and otg. It was
added mainly for compatibility with TI J721e platforms.
- Added enabling ISO OUT just before arming endpoint. It's recommended by
controller specification.
- Added support for 0x0002450d controller version. This version allows to set
DMULT mode per endpoint. It also fixes WA1 issue.
- Added support for separate interrupt line for Device and OTG/DRD.
- Removed drd_update_mode from drd_init, 'desired_dr_mode' is not yet correctly
set based on enabled drivers and dr_mode in DT.
- Added phy power on/off.
- Added setting dma and coherent mask to 32-bits, because controller can do
only 32-bit access.
- Added Idle state for Type-C for platform TI J721e as suggested by Roger.
- Improved the flow according with Figure 24 from Software OTG Control user
guide as sugested by Roger.

Change since v5:
- Fixed controller issue with handling SETUP that has occurred on 0x0002450C
controller version. In some case EP_STS_SETUP is reported but SETUP
packet has not been copied yet to system memory. This bug caused that
driver started handling the previous SETUP packet.
- Added handling ZLP for EP0.
- Removed unused cdns3_gadget_ep0_giveback function.
- Fixed issue with disabling endpoint. Added waiting for clearing EP_STS_DBUSY
bit between disabling endpoint and calling EP_CMD_EPRST command.
EP_CMD_EPRST command can be called only when DMA is stopped.
- Fixed issue: EP_CFG_TDL_CHK is currently supported only for OUT direction,
It was enabled for both IN/OUT direction.
- Improved resetting of interrupt in cdns3_device_irq_handler.
- Fixed issue with ISOC IN transfer in cdns3_ep_run_transfer function. In some
cases driver set incorrect Cycle Bit in TRBs.
- Fixed issue in function cdns3_ep_onchip_buffer_reserve. Driver assigned
incorrect value to priv_dev->out_mem_is_allocated field.

Change since v5:
- fixed compilation error.

Changes since v4:
- fixed issue: with choosing incorrect dr_mode in cdns3_core_init_role.
- speed up DRD timings by adding an appropriate entry to OTGSIMULATE
register in cdns3_drd_init function.
- added detecting transition to DEV_IDLE/H_IDLE state instead using
usleep_range in cdns3_drd_switch_gadget and cdns3_drd_switch_host functions.
- fixed issue with setting incorrect burst and mult field during endpoint
configuration.
- fixed issue in WA1 algorithm. The previous one could not work correct with
slow CPU or in case the access to AXI bus would be blocked for some time.
- fixed issue with compilation driver occurred when driver was configured
as build in. This fix required to move cdns3_handshake function from
gadget.c to core.c file.
- added missing pci_disable_device in cdns3-pci-wrap.c file.
- fixed issue with pm_runtime_get_sync in cdns3_role_switch function.
- fixed incorrect condition in cdns3_decode_usb_irq function.
- removed cdns3_data_flush function - is no longer used.
- fixed issue in cdns3_descmissing_packet function - fixed incorrect condition
- added missed callback informing upper layer about reset event.
- added resetting endpoint in cdns3_gadget_ep_disable function.
- fixed issue: added statement removing request from descmiss_req_list in
cdns3_gadget_ep_disable function.
- fixed issue in cdns3_ep_onchip_buffer_reserve.
- fixed issue with incorrect calculation the number of required on-chip buffer
for OUT endpoints cdns3_ep_onchip_buffer_reserve.
- fixed issue in __cdns3_gadget_init function: pm_runtime_get_sync was in
incorrect place in.
- removed some typos and improved comments as suggested by reviewers.
- made some other minor changes as suggested by revivers.

Changes since v3:
- updated dt-binding as suggested by Rob Herring
- updated patch 002, 003 and 004 according with patch:
https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git/commit/
?h=next&id=7790b3556fccc555ae422f1576e97bf34c8ab8b6 posted by Felipe Balbi.
- fixed issues related to isochronous transfers.
- improved algorithm calculating number of on-chip buffers required
by endpoints.
- fixed incorrect macro EP_CFG_MULT in gadget.h file.
- fixed potential issue with incorrect order of instruction - added wmb().
- made some minor changes suggested by reviewers.

*Changes since v2:
- made some text correction in Kconfig file as suggested by Randy Dunlap.
- simplified Makefile as suggested by Peter Chan.
- removed phy-names from dt-binding as suggested Rob Herring.
- changed cdns3-usb.txt to cdns3-usb3.txt as suggested by Rob Herring.
- added checking error code in some function in drd.c file
as suggested by Peter Chan.
- added reg-names to dt-binding documentation as suggested by Chunfeng Yun.
- replaced platform_get_resource with platform_get_resource_byname.
- made other changes suggested by Chunfeng Yun.
- fixed bug in cdns3_get_id. Now function return id instead 1.
- added trace_cdns3_log trace event.
- simplify cdns3_disable_write function.
- create separate patch for work around related with blocking endpoint
issue.
- Fixed issue related with stale data address in TRB.
Issue: At some situations, the controller may get stale data address
in TRB at below sequences:
1. Controller read TRB includes data address.
2. Software updates TRBs includes data address and Cycle bit.
3. Controller read TRB which includes Cycle bit.
4. DMA run with stale data address.
- Fixed issue without transfer. In some cases not all interrupts
disabled in Hard IRQ was enabled in Soft Irq.
- Modified LFPS minimal U1 Exit time for controller revision 0x00024505.
- Fixed issue - in some case selected endpoint was unexpectedly changed.
- Fixed issue - after clearing halted endpoint transfer was not started.
- Fixed issue - in some case driver send ACK instead STALL in status phase.
- Fixed issues related to dequeue request.
- Fixed incorrect operator in cdns3_ep_run_transfer function.

Changes since v1:
- Removed not implemented Suspend/Resume functions.
- Fixed some issues in debugging related functions.
- Added trace_cdns3_request_handled marker.
- Added support for Isochronous transfer.
- Added some additional descriptions.
- Fixed compilation error in cdns3_gadget_ep_disable.
- Added detection of device controller version at runtime.
- Upgraded dt-binding documentation.
- Deleted ENOSYS from phy initialization section. It should be also removed
from generic PHY driver.
- added ep0_stage flag used during enumeration process.
- Fixed issue with TEST MODE.
- Added one common function for finish control transfer.
- Separated some decoding function from dwc3 driver to common library file,
and removed equivalents function from debug.h file as suggested by Felipe.
- replaced function name cdns3_gadget_unconfig with cdns3_hw_reset_eps_config.
- Improved algorithm fixing hardware issue related to blocking endpoints.
This issue is related to on-chip shared FIFO buffers for OUT packets.
Problem was reported by Peter Chan.
- Changed organization of endpoint array in cdns3_device object.
- added ep0 to common eps array
- removed cdns3_free_trb_pool and cdns3_ep_addr_to_bit_pos macros.
- removed ep0_trb_dma, ep0_trb fields from cdns3_device.
- Removed ep0_request and ep_nums fields from cdns3_device.
- Other minor changes according with Felipe suggestion.

---

Pawel Laszczak (6):
dt-bindings: add binding for USBSS-DRD controller.
usb:common Separated decoding functions from dwc3 driver.
usb:common Patch simplify usb_decode_set_clear_feature function.
usb:common Simplify usb_decode_get_set_descriptor function.
usb:cdns3 Add Cadence USB3 DRD Driver
usb:cdns3 Fix for stuck packets in on-chip OUT buffer.

.../devicetree/bindings/usb/cdns-usb3.txt | 30 +
drivers/usb/Kconfig | 2 +
drivers/usb/Makefile | 2 +
drivers/usb/cdns3/Kconfig | 44 +
drivers/usb/cdns3/Makefile | 14 +
drivers/usb/cdns3/cdns3-pci-wrap.c | 157 +
drivers/usb/cdns3/core.c | 529 ++++
drivers/usb/cdns3/core.h | 121 +
drivers/usb/cdns3/debug.h | 173 ++
drivers/usb/cdns3/debugfs.c | 173 ++
drivers/usb/cdns3/drd.c | 379 +++
drivers/usb/cdns3/drd.h | 166 ++
drivers/usb/cdns3/ep0.c | 915 ++++++
drivers/usb/cdns3/gadget-export.h | 28 +
drivers/usb/cdns3/gadget.c | 2616 +++++++++++++++++
drivers/usb/cdns3/gadget.h | 1326 +++++++++
drivers/usb/cdns3/host-export.h | 28 +
drivers/usb/cdns3/host.c | 76 +
drivers/usb/cdns3/trace.c | 23 +
drivers/usb/cdns3/trace.h | 447 +++
drivers/usb/common/Makefile | 2 +-
drivers/usb/common/debug.c | 273 ++
drivers/usb/dwc3/debug.h | 252 --
drivers/usb/dwc3/trace.h | 2 +-
include/linux/usb/ch9.h | 25 +
25 files changed, 7549 insertions(+), 254 deletions(-)
create mode 100644 Documentation/devicetree/bindings/usb/cdns-usb3.txt
create mode 100644 drivers/usb/cdns3/Kconfig
create mode 100644 drivers/usb/cdns3/Makefile
create mode 100644 drivers/usb/cdns3/cdns3-pci-wrap.c
create mode 100644 drivers/usb/cdns3/core.c
create mode 100644 drivers/usb/cdns3/core.h
create mode 100644 drivers/usb/cdns3/debug.h
create mode 100644 drivers/usb/cdns3/debugfs.c
create mode 100644 drivers/usb/cdns3/drd.c
create mode 100644 drivers/usb/cdns3/drd.h
create mode 100644 drivers/usb/cdns3/ep0.c
create mode 100644 drivers/usb/cdns3/gadget-export.h
create mode 100644 drivers/usb/cdns3/gadget.c
create mode 100644 drivers/usb/cdns3/gadget.h
create mode 100644 drivers/usb/cdns3/host-export.h
create mode 100644 drivers/usb/cdns3/host.c
create mode 100644 drivers/usb/cdns3/trace.c
create mode 100644 drivers/usb/cdns3/trace.h
create mode 100644 drivers/usb/common/debug.c

--
2.17.1


2019-06-05 11:08:15

by Pawel Laszczak

[permalink] [raw]
Subject: [PATCH v7 2/6] usb:common Separated decoding functions from dwc3 driver.

Patch moves some decoding functions from driver/usb/dwc3/debug.h driver
to driver/usb/common/debug.c file. These moved functions include:
dwc3_decode_get_status
dwc3_decode_set_clear_feature
dwc3_decode_set_address
dwc3_decode_get_set_descriptor
dwc3_decode_get_configuration
dwc3_decode_set_configuration
dwc3_decode_get_intf
dwc3_decode_set_intf
dwc3_decode_synch_frame
dwc3_decode_set_sel
dwc3_decode_set_isoch_delay
dwc3_decode_ctrl

These functions are used also in inroduced cdns3 driver.

All functions prefixes were changed from dwc3 to usb.
Also, function's parameters has been extended according to the name
of fields in standard SETUP packet.
Additionally, patch adds usb_decode_ctrl function to
include/linux/usb/ch9.h file.i

Signed-off-by: Pawel Laszczak <[email protected]>
---
drivers/usb/common/Makefile | 2 +-
drivers/usb/common/debug.c | 273 ++++++++++++++++++++++++++++++++++++
drivers/usb/dwc3/debug.h | 252 ---------------------------------
drivers/usb/dwc3/trace.h | 2 +-
include/linux/usb/ch9.h | 25 ++++
5 files changed, 300 insertions(+), 254 deletions(-)
create mode 100644 drivers/usb/common/debug.c

diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 0a7c45e85481..02eb01666289 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -4,7 +4,7 @@
#

obj-$(CONFIG_USB_COMMON) += usb-common.o
-usb-common-y += common.o
+usb-common-y += common.o debug.o
usb-common-$(CONFIG_USB_LED_TRIG) += led.o

obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
diff --git a/drivers/usb/common/debug.c b/drivers/usb/common/debug.c
new file mode 100644
index 000000000000..f7218d794aa6
--- /dev/null
+++ b/drivers/usb/common/debug.c
@@ -0,0 +1,273 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * Common USB debugging functions
+ *
+ * Copyright (C) 2010-2011 Texas Instruments Incorporated - http://www.ti.com
+ *
+ * Authors: Felipe Balbi <[email protected]>,
+ * Sebastian Andrzej Siewior <[email protected]>
+ */
+
+#ifndef __LINUX_USB_COMMON_DEBUG
+#define __LINUX_USB_COMMON_DEBUG
+
+#include <linux/usb/ch9.h>
+
+static void usb_decode_get_status(__u8 bRequestType, __u16 wIndex,
+ __u16 wLength, char *str, size_t size)
+{
+ switch (bRequestType & USB_RECIP_MASK) {
+ case USB_RECIP_DEVICE:
+ snprintf(str, size, "Get Device Status(Length = %d)", wLength);
+ break;
+ case USB_RECIP_INTERFACE:
+ snprintf(str, size,
+ "Get Interface Status(Intf = %d, Length = %d)",
+ wIndex, wLength);
+ break;
+ case USB_RECIP_ENDPOINT:
+ snprintf(str, size, "Get Endpoint Status(ep%d%s)",
+ wIndex & ~USB_DIR_IN,
+ wIndex & USB_DIR_IN ? "in" : "out");
+ break;
+ }
+}
+
+static void usb_decode_set_clear_feature(__u8 bRequestType, __u8 bRequest,
+ __u16 wValue, __u16 wIndex,
+ char *str, size_t size)
+{
+ 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; }) : "");
+ break;
+ case USB_RECIP_INTERFACE:
+ snprintf(str, size, "%s Interface Feature(%s)",
+ bRequest == USB_REQ_CLEAR_FEATURE ? "Clear" : "Set",
+ wValue == USB_INTRF_FUNC_SUSPEND ?
+ "Function Suspend" : "UNKNOWN");
+ break;
+ case USB_RECIP_ENDPOINT:
+ snprintf(str, size, "%s Endpoint Feature(%s ep%d%s)",
+ bRequest == USB_REQ_CLEAR_FEATURE ? "Clear" : "Set",
+ wValue == USB_ENDPOINT_HALT ? "Halt" : "UNKNOWN",
+ wIndex & ~USB_DIR_IN,
+ wIndex & USB_DIR_IN ? "in" : "out");
+ break;
+ }
+}
+
+static void usb_decode_set_address(__u16 wValue, char *str, size_t size)
+{
+ snprintf(str, size, "Set Address(Addr = %02x)", wValue);
+}
+
+static void usb_decode_get_set_descriptor(__u8 bRequestType, __u8 bRequest,
+ __u16 wValue, __u16 wIndex,
+ __u16 wLength, char *str, size_t size)
+{
+ snprintf(str, size, "%s %s Descriptor(Index = %d, Length = %d)",
+ bRequest == USB_REQ_GET_DESCRIPTOR ? "Get" : "Set",
+ ({ char *s;
+ switch (wValue >> 8) {
+ case USB_DT_DEVICE:
+ s = "Device";
+ break;
+ case USB_DT_CONFIG:
+ s = "Configuration";
+ break;
+ case USB_DT_STRING:
+ s = "String";
+ break;
+ case USB_DT_INTERFACE:
+ s = "Interface";
+ break;
+ case USB_DT_ENDPOINT:
+ s = "Endpoint";
+ break;
+ case USB_DT_DEVICE_QUALIFIER:
+ s = "Device Qualifier";
+ break;
+ case USB_DT_OTHER_SPEED_CONFIG:
+ s = "Other Speed Config";
+ break;
+ case USB_DT_INTERFACE_POWER:
+ s = "Interface Power";
+ break;
+ case USB_DT_OTG:
+ s = "OTG";
+ break;
+ case USB_DT_DEBUG:
+ s = "Debug";
+ break;
+ case USB_DT_INTERFACE_ASSOCIATION:
+ s = "Interface Association";
+ break;
+ case USB_DT_BOS:
+ s = "BOS";
+ break;
+ case USB_DT_DEVICE_CAPABILITY:
+ s = "Device Capability";
+ break;
+ case USB_DT_PIPE_USAGE:
+ s = "Pipe Usage";
+ break;
+ case USB_DT_SS_ENDPOINT_COMP:
+ s = "SS Endpoint Companion";
+ break;
+ case USB_DT_SSP_ISOC_ENDPOINT_COMP:
+ s = "SSP Isochronous Endpoint Companion";
+ break;
+ default:
+ s = "UNKNOWN";
+ break;
+ } s; }), wValue & 0xff, wLength);
+}
+
+static void usb_decode_get_configuration(__u16 wLength, char *str, size_t size)
+{
+ snprintf(str, size, "Get Configuration(Length = %d)", wLength);
+}
+
+static void usb_decode_set_configuration(__u8 wValue, char *str, size_t size)
+{
+ snprintf(str, size, "Set Configuration(Config = %d)", wValue);
+}
+
+static void usb_decode_get_intf(__u16 wIndex, __u16 wLength, char *str,
+ size_t size)
+{
+ snprintf(str, size, "Get Interface(Intf = %d, Length = %d)",
+ wIndex, wLength);
+}
+
+static void usb_decode_set_intf(__u8 wValue, __u16 wIndex, char *str,
+ size_t size)
+{
+ snprintf(str, size, "Set Interface(Intf = %d, Alt.Setting = %d)",
+ wIndex, wValue);
+}
+
+static void usb_decode_synch_frame(__u16 wIndex, __u16 wLength,
+ char *str, size_t size)
+{
+ snprintf(str, size, "Synch Frame(Endpoint = %d, Length = %d)",
+ wIndex, wLength);
+}
+
+static void usb_decode_set_sel(__u16 wLength, char *str, size_t size)
+{
+ snprintf(str, size, "Set SEL(Length = %d)", wLength);
+}
+
+static void usb_decode_set_isoch_delay(__u8 wValue, char *str, size_t size)
+{
+ snprintf(str, size, "Set Isochronous Delay(Delay = %d ns)", wValue);
+}
+
+/**
+ * usb_decode_ctrl - returns a string representation of ctrl request
+ */
+const char *usb_decode_ctrl(char *str, size_t size, __u8 bRequestType,
+ __u8 bRequest, __u16 wValue, __u16 wIndex,
+ __u16 wLength)
+{
+ switch (bRequest) {
+ case USB_REQ_GET_STATUS:
+ usb_decode_get_status(bRequestType, wIndex, wLength, str, size);
+ break;
+ case USB_REQ_CLEAR_FEATURE:
+ case USB_REQ_SET_FEATURE:
+ usb_decode_set_clear_feature(bRequestType, bRequest, wValue,
+ wIndex, str, size);
+ break;
+ case USB_REQ_SET_ADDRESS:
+ usb_decode_set_address(wValue, str, size);
+ break;
+ case USB_REQ_GET_DESCRIPTOR:
+ case USB_REQ_SET_DESCRIPTOR:
+ usb_decode_get_set_descriptor(bRequestType, bRequest, wValue,
+ wIndex, wLength, str, size);
+ break;
+ case USB_REQ_GET_CONFIGURATION:
+ usb_decode_get_configuration(wLength, str, size);
+ break;
+ case USB_REQ_SET_CONFIGURATION:
+ usb_decode_set_configuration(wValue, str, size);
+ break;
+ case USB_REQ_GET_INTERFACE:
+ usb_decode_get_intf(wIndex, wLength, str, size);
+ break;
+ case USB_REQ_SET_INTERFACE:
+ usb_decode_set_intf(wValue, wIndex, str, size);
+ break;
+ case USB_REQ_SYNCH_FRAME:
+ usb_decode_synch_frame(wIndex, wLength, str, size);
+ break;
+ case USB_REQ_SET_SEL:
+ usb_decode_set_sel(wLength, str, size);
+ break;
+ case USB_REQ_SET_ISOCH_DELAY:
+ usb_decode_set_isoch_delay(wValue, str, size);
+ break;
+ default:
+ 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));
+ }
+
+ return str;
+}
+EXPORT_SYMBOL_GPL(usb_decode_ctrl);
+
+#endif /* __LINUX_USB_COMMON_DEBUG */
diff --git a/drivers/usb/dwc3/debug.h b/drivers/usb/dwc3/debug.h
index 068259fdfb0c..9baabed87d61 100644
--- a/drivers/usb/dwc3/debug.h
+++ b/drivers/usb/dwc3/debug.h
@@ -246,258 +246,6 @@ static inline const char *dwc3_gadget_event_string(char *str, size_t size,
return str;
}

-static inline void dwc3_decode_get_status(__u8 t, __u16 i, __u16 l, char *str,
- size_t size)
-{
- switch (t & USB_RECIP_MASK) {
- case USB_RECIP_DEVICE:
- snprintf(str, size, "Get Device Status(Length = %d)", l);
- break;
- case USB_RECIP_INTERFACE:
- snprintf(str, size, "Get Interface Status(Intf = %d, Length = %d)",
- i, l);
- break;
- case USB_RECIP_ENDPOINT:
- snprintf(str, size, "Get Endpoint Status(ep%d%s)",
- i & ~USB_DIR_IN,
- i & USB_DIR_IN ? "in" : "out");
- break;
- }
-}
-
-static inline void dwc3_decode_set_clear_feature(__u8 t, __u8 b, __u16 v,
- __u16 i, char *str, size_t size)
-{
- switch (t & USB_RECIP_MASK) {
- case USB_RECIP_DEVICE:
- snprintf(str, size, "%s Device Feature(%s%s)",
- b == USB_REQ_CLEAR_FEATURE ? "Clear" : "Set",
- ({char *s;
- switch (v) {
- 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; }),
- v == USB_DEVICE_TEST_MODE ?
- ({ char *s;
- switch (i) {
- 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; }) : "");
- break;
- case USB_RECIP_INTERFACE:
- snprintf(str, size, "%s Interface Feature(%s)",
- b == USB_REQ_CLEAR_FEATURE ? "Clear" : "Set",
- v == USB_INTRF_FUNC_SUSPEND ?
- "Function Suspend" : "UNKNOWN");
- break;
- case USB_RECIP_ENDPOINT:
- snprintf(str, size, "%s Endpoint Feature(%s ep%d%s)",
- b == USB_REQ_CLEAR_FEATURE ? "Clear" : "Set",
- v == USB_ENDPOINT_HALT ? "Halt" : "UNKNOWN",
- i & ~USB_DIR_IN,
- i & USB_DIR_IN ? "in" : "out");
- break;
- }
-}
-
-static inline void dwc3_decode_set_address(__u16 v, char *str, size_t size)
-{
- snprintf(str, size, "Set Address(Addr = %02x)", v);
-}
-
-static inline void dwc3_decode_get_set_descriptor(__u8 t, __u8 b, __u16 v,
- __u16 i, __u16 l, char *str, size_t size)
-{
- snprintf(str, size, "%s %s Descriptor(Index = %d, Length = %d)",
- b == USB_REQ_GET_DESCRIPTOR ? "Get" : "Set",
- ({ char *s;
- switch (v >> 8) {
- case USB_DT_DEVICE:
- s = "Device";
- break;
- case USB_DT_CONFIG:
- s = "Configuration";
- break;
- case USB_DT_STRING:
- s = "String";
- break;
- case USB_DT_INTERFACE:
- s = "Interface";
- break;
- case USB_DT_ENDPOINT:
- s = "Endpoint";
- break;
- case USB_DT_DEVICE_QUALIFIER:
- s = "Device Qualifier";
- break;
- case USB_DT_OTHER_SPEED_CONFIG:
- s = "Other Speed Config";
- break;
- case USB_DT_INTERFACE_POWER:
- s = "Interface Power";
- break;
- case USB_DT_OTG:
- s = "OTG";
- break;
- case USB_DT_DEBUG:
- s = "Debug";
- break;
- case USB_DT_INTERFACE_ASSOCIATION:
- s = "Interface Association";
- break;
- case USB_DT_BOS:
- s = "BOS";
- break;
- case USB_DT_DEVICE_CAPABILITY:
- s = "Device Capability";
- break;
- case USB_DT_PIPE_USAGE:
- s = "Pipe Usage";
- break;
- case USB_DT_SS_ENDPOINT_COMP:
- s = "SS Endpoint Companion";
- break;
- case USB_DT_SSP_ISOC_ENDPOINT_COMP:
- s = "SSP Isochronous Endpoint Companion";
- break;
- default:
- s = "UNKNOWN";
- break;
- } s; }), v & 0xff, l);
-}
-
-
-static inline void dwc3_decode_get_configuration(__u16 l, char *str,
- size_t size)
-{
- snprintf(str, size, "Get Configuration(Length = %d)", l);
-}
-
-static inline void dwc3_decode_set_configuration(__u8 v, char *str, size_t size)
-{
- snprintf(str, size, "Set Configuration(Config = %d)", v);
-}
-
-static inline void dwc3_decode_get_intf(__u16 i, __u16 l, char *str,
- size_t size)
-{
- snprintf(str, size, "Get Interface(Intf = %d, Length = %d)", i, l);
-}
-
-static inline void dwc3_decode_set_intf(__u8 v, __u16 i, char *str, size_t size)
-{
- snprintf(str, size, "Set Interface(Intf = %d, Alt.Setting = %d)", i, v);
-}
-
-static inline void dwc3_decode_synch_frame(__u16 i, __u16 l, char *str,
- size_t size)
-{
- snprintf(str, size, "Synch Frame(Endpoint = %d, Length = %d)", i, l);
-}
-
-static inline void dwc3_decode_set_sel(__u16 l, char *str, size_t size)
-{
- snprintf(str, size, "Set SEL(Length = %d)", l);
-}
-
-static inline void dwc3_decode_set_isoch_delay(__u8 v, char *str, size_t size)
-{
- snprintf(str, size, "Set Isochronous Delay(Delay = %d ns)", v);
-}
-
-/**
- * dwc3_decode_ctrl - returns a string represetion of ctrl request
- */
-static inline const char *dwc3_decode_ctrl(char *str, size_t size,
- __u8 bRequestType, __u8 bRequest, __u16 wValue, __u16 wIndex,
- __u16 wLength)
-{
- switch (bRequest) {
- case USB_REQ_GET_STATUS:
- dwc3_decode_get_status(bRequestType, wIndex, wLength, str,
- size);
- break;
- case USB_REQ_CLEAR_FEATURE:
- case USB_REQ_SET_FEATURE:
- dwc3_decode_set_clear_feature(bRequestType, bRequest, wValue,
- wIndex, str, size);
- break;
- case USB_REQ_SET_ADDRESS:
- dwc3_decode_set_address(wValue, str, size);
- break;
- case USB_REQ_GET_DESCRIPTOR:
- case USB_REQ_SET_DESCRIPTOR:
- dwc3_decode_get_set_descriptor(bRequestType, bRequest, wValue,
- wIndex, wLength, str, size);
- break;
- case USB_REQ_GET_CONFIGURATION:
- dwc3_decode_get_configuration(wLength, str, size);
- break;
- case USB_REQ_SET_CONFIGURATION:
- dwc3_decode_set_configuration(wValue, str, size);
- break;
- case USB_REQ_GET_INTERFACE:
- dwc3_decode_get_intf(wIndex, wLength, str, size);
- break;
- case USB_REQ_SET_INTERFACE:
- dwc3_decode_set_intf(wValue, wIndex, str, size);
- break;
- case USB_REQ_SYNCH_FRAME:
- dwc3_decode_synch_frame(wIndex, wLength, str, size);
- break;
- case USB_REQ_SET_SEL:
- dwc3_decode_set_sel(wLength, str, size);
- break;
- case USB_REQ_SET_ISOCH_DELAY:
- dwc3_decode_set_isoch_delay(wValue, str, size);
- break;
- default:
- snprintf(str, size, "%02x %02x %02x %02x %02x %02x %02x %02x",
- bRequestType, bRequest,
- cpu_to_le16(wValue) & 0xff,
- cpu_to_le16(wValue) >> 8,
- cpu_to_le16(wIndex) & 0xff,
- cpu_to_le16(wIndex) >> 8,
- cpu_to_le16(wLength) & 0xff,
- cpu_to_le16(wLength) >> 8);
- }
-
- return str;
-}
-
/**
* dwc3_ep_event_string - returns event name
* @event: then event code
diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h
index 818a63da1a44..9edff17111f7 100644
--- a/drivers/usb/dwc3/trace.h
+++ b/drivers/usb/dwc3/trace.h
@@ -86,7 +86,7 @@ DECLARE_EVENT_CLASS(dwc3_log_ctrl,
__entry->wIndex = le16_to_cpu(ctrl->wIndex);
__entry->wLength = le16_to_cpu(ctrl->wLength);
),
- TP_printk("%s", dwc3_decode_ctrl(__get_str(str), DWC3_MSG_MAX,
+ TP_printk("%s", usb_decode_ctrl(__get_str(str), DWC3_MSG_MAX,
__entry->bRequestType,
__entry->bRequest, __entry->wValue,
__entry->wIndex, __entry->wLength)
diff --git a/include/linux/usb/ch9.h b/include/linux/usb/ch9.h
index da82606be605..a601810ad0f9 100644
--- a/include/linux/usb/ch9.h
+++ b/include/linux/usb/ch9.h
@@ -70,4 +70,29 @@ extern enum usb_device_speed usb_get_maximum_speed(struct device *dev);
*/
extern const char *usb_state_string(enum usb_device_state state);

+/**
+ * usb_decode_ctrl - Returns human readable representation of control request.
+ * @str: buffer to return a human-readable representation of control request.
+ * This buffer should have about 200 bytes.
+ * @size: size of str buffer.
+ * @bRequestType: matches the USB bmRequestType field
+ * @bRequest: matches the USB bRequest field
+ * @wValue: matches the USB wValue field (CPU byte order)
+ * @wIndex: matches the USB wIndex field (CPU byte order)
+ * @wLength: matches the USB wLength field (CPU byte order)
+ *
+ * Function returns decoded, formatted and human-readable description of
+ * control request packet.
+ *
+ * The usage scenario for this is for tracepoints, so function as a return
+ * use the same value as in parameters. This approach allows to use this
+ * function in TP_printk
+ *
+ * Important: wValue, wIndex, wLength parameters before invoking this function
+ * should be processed by le16_to_cpu macro.
+ */
+const char *usb_decode_ctrl(char *str, size_t size, __u8 bRequestType,
+ __u8 bRequest, __u16 wValue, __u16 wIndex,
+ __u16 wLength);
+
#endif /* __LINUX_USB_CH9_H */
--
2.17.1

2019-06-06 01:14:03

by Lars Melin

[permalink] [raw]
Subject: Re: [PATCH v7 0/6] Introduced new Cadence USBSS DRD Driver.

On 6/5/2019 17:03, Pawel Laszczak wrote:
> This patch introduces new Cadence USBSS DRD driver to Linux kernel.
>
> The Cadence USBSS DRD Driver is a highly configurable IP Core which
> can be instantiated as Dual-Role Device (DRD), Peripheral Only and
> Host Only (XHCI)configurations.
>
The driver is not an IP Core, the hardware device is.


/Lars


2019-06-06 05:15:40

by Pawel Laszczak

[permalink] [raw]
Subject: RE: [PATCH v7 0/6] Introduced new Cadence USBSS DRD Driver.

>On 6/5/2019 17:03, Pawel Laszczak wrote:
>> This patch introduces new Cadence USBSS DRD driver to Linux kernel.
>>
>> The Cadence USBSS DRD Driver is a highly configurable IP Core which
>> can be instantiated as Dual-Role Device (DRD), Peripheral Only and
>> Host Only (XHCI)configurations.
>>
>The driver is not an IP Core, the hardware device is.
>
I remember that I had such comment in the paste. I don't know why there is "IP Core".

I will change it.
thanks ,
Pawel
>
>/Lars
>

2019-06-08 13:42:03

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v7 2/6] usb:common Separated decoding functions from dwc3 driver.

On Wed, Jun 05, 2019 at 11:03:46AM +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:
> dwc3_decode_get_status
> dwc3_decode_set_clear_feature
> dwc3_decode_set_address
> dwc3_decode_get_set_descriptor
> dwc3_decode_get_configuration
> dwc3_decode_set_configuration
> dwc3_decode_get_intf
> dwc3_decode_set_intf
> dwc3_decode_synch_frame
> dwc3_decode_set_sel
> dwc3_decode_set_isoch_delay
> dwc3_decode_ctrl
>
> These functions are used also in inroduced cdns3 driver.
>
> All functions prefixes were changed from dwc3 to usb.
> Also, function's parameters has been extended according to the name
> of fields in standard SETUP packet.
> Additionally, patch adds usb_decode_ctrl function to
> include/linux/usb/ch9.h file.i
>
> Signed-off-by: Pawel Laszczak <[email protected]>
> ---
> drivers/usb/common/Makefile | 2 +-
> drivers/usb/common/debug.c | 273 ++++++++++++++++++++++++++++++++++++
> drivers/usb/dwc3/debug.h | 252 ---------------------------------
> drivers/usb/dwc3/trace.h | 2 +-
> include/linux/usb/ch9.h | 25 ++++
> 5 files changed, 300 insertions(+), 254 deletions(-)
> create mode 100644 drivers/usb/common/debug.c
>
> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
> index 0a7c45e85481..02eb01666289 100644
> --- a/drivers/usb/common/Makefile
> +++ b/drivers/usb/common/Makefile
> @@ -4,7 +4,7 @@
> #
>
> obj-$(CONFIG_USB_COMMON) += usb-common.o
> -usb-common-y += common.o
> +usb-common-y += common.o debug.o
> usb-common-$(CONFIG_USB_LED_TRIG) += led.o
>
> obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
> diff --git a/drivers/usb/common/debug.c b/drivers/usb/common/debug.c
> new file mode 100644
> index 000000000000..f7218d794aa6
> --- /dev/null
> +++ b/drivers/usb/common/debug.c
> @@ -0,0 +1,273 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/**
> + * Common USB debugging functions
> + *
> + * Copyright (C) 2010-2011 Texas Instruments Incorporated - http://www.ti.com
> + *
> + * Authors: Felipe Balbi <[email protected]>,
> + * Sebastian Andrzej Siewior <[email protected]>
> + */
> +
> +#ifndef __LINUX_USB_COMMON_DEBUG
> +#define __LINUX_USB_COMMON_DEBUG

Why are you doing thsi in a .c file?

thanks,

greg k-h

2019-06-10 06:30:43

by Pawel Laszczak

[permalink] [raw]
Subject: RE: [PATCH v7 2/6] usb:common Separated decoding functions from dwc3 driver.


>On Wed, Jun 05, 2019 at 11:03:46AM +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:
>
>> dwc3_decode_get_status
>
>> dwc3_decode_set_clear_feature
>
>> dwc3_decode_set_address
>
>> dwc3_decode_get_set_descriptor
>
>> dwc3_decode_get_configuration
>
>> dwc3_decode_set_configuration
>
>> dwc3_decode_get_intf
>
>> dwc3_decode_set_intf
>
>> dwc3_decode_synch_frame
>
>> dwc3_decode_set_sel
>
>> dwc3_decode_set_isoch_delay
>
>> dwc3_decode_ctrl
>
>>
>
>> These functions are used also in inroduced cdns3 driver.
>
>>
>
>> All functions prefixes were changed from dwc3 to usb.
>
>> Also, function's parameters has been extended according to the name
>
>> of fields in standard SETUP packet.
>
>> Additionally, patch adds usb_decode_ctrl function to
>
>> include/linux/usb/ch9.h file.i
>
>>
>
>> Signed-off-by: Pawel Laszczak <[email protected]>
>
>> ---
>
>> drivers/usb/common/Makefile | 2 +-
>
>> drivers/usb/common/debug.c | 273 ++++++++++++++++++++++++++++++++++++
>
>> drivers/usb/dwc3/debug.h | 252 ---------------------------------
>
>> drivers/usb/dwc3/trace.h | 2 +-
>
>> include/linux/usb/ch9.h | 25 ++++
>
>> 5 files changed, 300 insertions(+), 254 deletions(-)
>
>> create mode 100644 drivers/usb/common/debug.c
>
>>
>
>> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
>
>> index 0a7c45e85481..02eb01666289 100644
>
>> --- a/drivers/usb/common/Makefile
>
>> +++ b/drivers/usb/common/Makefile
>
>> @@ -4,7 +4,7 @@
>
>> #
>
>>
>
>> obj-$(CONFIG_USB_COMMON) += usb-common.o
>
>> -usb-common-y += common.o
>
>> +usb-common-y += common.o debug.o
>
>> usb-common-$(CONFIG_USB_LED_TRIG) += led.o
>
>>
>
>> obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
>
>> diff --git a/drivers/usb/common/debug.c b/drivers/usb/common/debug.c
>
>> new file mode 100644
>
>> index 000000000000..f7218d794aa6
>
>> --- /dev/null
>
>> +++ b/drivers/usb/common/debug.c
>
>> @@ -0,0 +1,273 @@
>
>> +// SPDX-License-Identifier: GPL-2.0
>
>> +/**
>
>> + * Common USB debugging functions
>
>> + *
>
>> + * Copyright (C) 2010-2011 Texas Instruments Incorporated - https://urldefense.proofpoint.com/v2/url?u=http-
>3A__http://www.ti.com&d=DwIBAg&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_haXqY&r=e1OgxfvkL0qo9XO6fX1gscva-
>w03uSYC1nIyxl89-rI&m=hCAftKt20FnC6KiCwNbVrg7WoY-WnCtUjVuast3iJSw&s=Q4qhFXl4s1P2u0s65WgkulIRgV4KZtGphj7Xjr4t6yA&e=
>
>> + *
>
>> + * Authors: Felipe Balbi <[email protected]>,
>
>> + * Sebastian Andrzej Siewior <[email protected]>
>
>> + */
>
>> +
>
>> +#ifndef __LINUX_USB_COMMON_DEBUG
>
>> +#define __LINUX_USB_COMMON_DEBUG
>
>
>
>Why are you doing thsi in a .c file?
>
Good question, I don't know :).
Was removed

Also I remember that you complained about placing it in
drivers/usb/common.
Currently this file is used only by two drivers dwc3 and cdns3.
Both are USB controller drivers. Maybe it could be better to move it to:
drivers/gadget/udc/debug.c
drivers/gadget/debug.c
drivers/gadget/common/debug.c
or
drivers/gadget/debug/debug.c

What do you think Roger?

Felipe it's also question for you.


Thanks,
Pawel

>
>
>thanks,
>
>
>
>greg k-h

2019-06-10 12:19:08

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH v7 2/6] usb:common Separated decoding functions from dwc3 driver.



On 10/06/2019 09:29, Pawel Laszczak wrote:
>
>> On Wed, Jun 05, 2019 at 11:03:46AM +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:
>>
>>> dwc3_decode_get_status
>>
>>> dwc3_decode_set_clear_feature
>>
>>> dwc3_decode_set_address
>>
>>> dwc3_decode_get_set_descriptor
>>
>>> dwc3_decode_get_configuration
>>
>>> dwc3_decode_set_configuration
>>
>>> dwc3_decode_get_intf
>>
>>> dwc3_decode_set_intf
>>
>>> dwc3_decode_synch_frame
>>
>>> dwc3_decode_set_sel
>>
>>> dwc3_decode_set_isoch_delay
>>
>>> dwc3_decode_ctrl
>>
>>>
>>
>>> These functions are used also in inroduced cdns3 driver.
>>
>>>
>>
>>> All functions prefixes were changed from dwc3 to usb.
>>
>>> Also, function's parameters has been extended according to the name
>>
>>> of fields in standard SETUP packet.
>>
>>> Additionally, patch adds usb_decode_ctrl function to
>>
>>> include/linux/usb/ch9.h file.i
>>
>>>
>>
>>> Signed-off-by: Pawel Laszczak <[email protected]>
>>
>>> ---
>>
>>> drivers/usb/common/Makefile | 2 +-
>>
>>> drivers/usb/common/debug.c | 273 ++++++++++++++++++++++++++++++++++++
>>
>>> drivers/usb/dwc3/debug.h | 252 ---------------------------------
>>
>>> drivers/usb/dwc3/trace.h | 2 +-
>>
>>> include/linux/usb/ch9.h | 25 ++++
>>
>>> 5 files changed, 300 insertions(+), 254 deletions(-)
>>
>>> create mode 100644 drivers/usb/common/debug.c
>>
>>>
>>
>>> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
>>
>>> index 0a7c45e85481..02eb01666289 100644
>>
>>> --- a/drivers/usb/common/Makefile
>>
>>> +++ b/drivers/usb/common/Makefile
>>
>>> @@ -4,7 +4,7 @@
>>
>>> #
>>
>>>
>>
>>> obj-$(CONFIG_USB_COMMON) += usb-common.o
>>
>>> -usb-common-y += common.o
>>
>>> +usb-common-y += common.o debug.o
>>
>>> usb-common-$(CONFIG_USB_LED_TRIG) += led.o
>>
>>>
>>
>>> obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
>>
>>> diff --git a/drivers/usb/common/debug.c b/drivers/usb/common/debug.c
>>
>>> new file mode 100644
>>
>>> index 000000000000..f7218d794aa6
>>
>>> --- /dev/null
>>
>>> +++ b/drivers/usb/common/debug.c
>>
>>> @@ -0,0 +1,273 @@
>>
>>> +// SPDX-License-Identifier: GPL-2.0
>>
>>> +/**
>>
>>> + * Common USB debugging functions
>>
>>> + *
>>
>>> + * Copyright (C) 2010-2011 Texas Instruments Incorporated - https://urldefense.proofpoint.com/v2/url?u=http-
>> 3A__http://www.ti.com&d=DwIBAg&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_haXqY&r=e1OgxfvkL0qo9XO6fX1gscva-
>> w03uSYC1nIyxl89-rI&m=hCAftKt20FnC6KiCwNbVrg7WoY-WnCtUjVuast3iJSw&s=Q4qhFXl4s1P2u0s65WgkulIRgV4KZtGphj7Xjr4t6yA&e=
>>
>>> + *
>>
>>> + * Authors: Felipe Balbi <[email protected]>,
>>
>>> + * Sebastian Andrzej Siewior <[email protected]>
>>
>>> + */
>>
>>> +
>>
>>> +#ifndef __LINUX_USB_COMMON_DEBUG
>>
>>> +#define __LINUX_USB_COMMON_DEBUG
>>
>>
>>
>> Why are you doing thsi in a .c file?
>>
> Good question, I don't know :).
> Was removed
>
> Also I remember that you complained about placing it in
> drivers/usb/common.
> Currently this file is used only by two drivers dwc3 and cdns3.
> Both are USB controller drivers. Maybe it could be better to move it to:
> drivers/gadget/udc/debug.c
> drivers/gadget/debug.c
> drivers/gadget/common/debug.c
> or
> drivers/gadget/debug/debug.c
>
> What do you think Roger?

Since they are only used by gadget code, I would opt to have it in drivers/usb/gadget/

>
> Felipe it's also question for you.
>
>
> Thanks,
> Pawel
>
>>
>>
>> thanks,
>>
>>
>>
>> greg k-h
>

--
cheers,
-roger
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki