[RFC PATCH 0/1] myriad-ma24xx-vsc: Helper fw driver for AI inference accelerator
So this series is an effort to start a conversation in the community, with the intent to free the Intel(R) Myriad(TM) ma24xx stack, as found within: * USB Intel Neural Compute Stick (ma2450) * USB Intel Neural Compute Stick 2 (ma2485) What this patch series is: * RFC for a helper driver that loads a (currently) non-redistributable firmware binary blob for this hotswappable AI inference hardware. What this patch series is NOT: * Kernel driver that provides clean API access to this AI accelerator. * Any usermode support. Hardware: === The Myriad ma24xx in the USB Intel Neural Compute Stick and Intel Neural Compute Stick 2 provides an API to accelerate AI inference calculations on the dedicated SHAVE VLIW vector co-processors, which are orchestrated by one or more LEON SPARC v8 real time cores. This VPU chip is positioned as a specialised low-power processor chip for computer vision inference applications. It contains a dedicated hardware accelerator for neural network deep-learning inferences. It was originally designed by Movidius Ltd, who announced they were to be acquired by Intel in September 2016. Boot process: === The core chip is housed within a USB enclosure (there is a different variant embedded on a SoC over a local bus, but that's out of scope). When physically connected to a host system, the accelerator card first needs firmware to be loaded for the device to be useable. An uninitialised Myriad ma24xx presents with a distinctive USB ID. After firmware loading, the device detaches from the USB bus and reattaches with a new device ID. It can then be claimed by the usermode driver. Current software stack: === + * + * The firmware is non-free and must be extracted by the user. + */ Intel packages and distributes a downstream vendor Intel(R) Distribution of OpenVINO(TM) toolkit [0] (under a limited proprietary license) for these devices which, amongst other binary packages, contains the Deep Learning Deployment Toolkit [1] (Apache License Version 2.0). However, the non-redistributable firmware binary blob that must be loaded onto the Myriad ma24xx is not available under a suitable FOSS license. There have been periods of time during which support for non-x86 host platforms (e.g. Raspberry Pi ARMv8 64-bit) were missing or delayed in the Intel-distributed downstream vendor fork OpenVINO. Whilst Movidius was an independent company, a separate (also non-FOSS) "Myriad Development Kit" was made available for developers, although it is no longer actively supported nor new features developed for it. Let's try to open more of this stack to FOSS mainline, on any host! This patch series would provide mainline Linux kernel support to load the Myriad ma24xx firmware, were it to become freely licensed. As the linux-accelerators list is forming as somewhat of a home for Linux AI accelerators (training and inference) subsystem, I have copied that list as a heads-up to the larger effort to integrate this hardware into a fully open stack. May the RFC comments flood in! [0] https://software.intel.com/en-us/openvino-toolkit [1] https://github.com/opencv/dldt Note: Intel, the Intel logo, Movidius, Myriad, OpenVINO and the OpenVINO logo are trademarks of Intel Corporation or its subsidiaries in the U.S. and/or other countries. Rhys Kidd (1): USB: myriad-ma24xx-vsc: Firmware loader driver for USB Myriad ma24xx MAINTAINERS | 6 + drivers/usb/misc/Kconfig | 8 ++ drivers/usb/misc/Makefile| 1 + drivers/usb/misc/myriad-ma24xx-vsc.c | 171 +++ 4 files changed, 186 insertions(+) create mode 100644 drivers/usb/misc/myriad-ma24xx-vsc.c -- 2.20.1
[RFC PATCH 1/1] USB: myriad-ma24xx-vsc: Firmware loader driver for USB Myriad ma24xx
The Myriad ma24xx in USB Intel Neural Compute Stick and Intel Neural
Compute Stick 2 provides an API to accelerate AI inference calculations
on the dedicated SHAVE VLIW vector co-processors, which are orchestrated
by one or more LEON SPARC v8 real time cores.
However, they need firmware to be loaded beforehand. An uninitialised
Myriad ma24xx presents with a distinctive USB ID. After firmware
loading, the device detaches from the USB bus and reattaches with a new
device ID. It can then be claimed by the usermode driver.
Signed-off-by: Rhys Kidd
---
MAINTAINERS | 6 +
drivers/usb/misc/Kconfig | 8 ++
drivers/usb/misc/Makefile| 1 +
drivers/usb/misc/myriad-ma24xx-vsc.c | 171 +++
4 files changed, 186 insertions(+)
create mode 100644 drivers/usb/misc/myriad-ma24xx-vsc.c
diff --git a/MAINTAINERS b/MAINTAINERS
index a50e97a63bc8..2c3ab39ac26a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16682,6 +16682,12 @@ T: git
git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
S: Maintained
F: sound/usb/midi.*
+USB MYRIAD MA24XX FIRMWARE DRIVER
+M: Rhys Kidd
+L: linux-usb@vger.kernel.org
+S: Maintained
+F: drivers/usb/misc/myriad-ma24xx-vsc.c
+
USB NETWORKING DRIVERS
L: linux-usb@vger.kernel.org
S: Odd Fixes
diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig
index bdae62b2ffe0..5d600fb8ac50 100644
--- a/drivers/usb/misc/Kconfig
+++ b/drivers/usb/misc/Kconfig
@@ -275,3 +275,11 @@ config USB_CHAOSKEY
To compile this driver as a module, choose M here: the
module will be called chaoskey.
+
+config USB_MYRIAD_MA24XX_VSC
+ tristate "USB Intel Myriad MA24xx firmware loading support"
+ select FW_LOADER
+ help
+ This driver loads firmware for Myriad ma24xx AI inference
accelerators, as
+ found in the USB Intel Neural Compute Stick (ma2450) and Intel Neural
+ Compute Stick 2 (ma2485).
\ No newline at end of file
diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile
index 109f54f5b9aa..e19d1348c5b6 100644
--- a/drivers/usb/misc/Makefile
+++ b/drivers/usb/misc/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_USB_ISIGHTFW)+= isight_firmware.o
obj-$(CONFIG_USB_LCD) += usblcd.o
obj-$(CONFIG_USB_LD) += ldusb.o
obj-$(CONFIG_USB_LEGOTOWER)+= legousbtower.o
+obj-$(CONFIG_USB_MYRIAD_MA24XX_VSC)+= myriad-ma24xx-vsc.o
obj-$(CONFIG_USB_RIO500) += rio500.o
obj-$(CONFIG_USB_TEST) += usbtest.o
obj-$(CONFIG_USB_EHSET_TEST_FIXTURE)+= ehset.o
diff --git a/drivers/usb/misc/myriad-ma24xx-vsc.c
b/drivers/usb/misc/myriad-ma24xx-vsc.c
new file mode 100644
index ..f516c236a8f5
--- /dev/null
+++ b/drivers/usb/misc/myriad-ma24xx-vsc.c
@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for loading USB Myriad ma24xx firmware
+ *
+ * Copyright (C) 2018 Rhys Kidd
+ *
+ * The Myriad ma24xx in USB Intel Neural Compute Stick and Intel Neural
+ * Compute Stick 2 provides an API to accelerate AI inference calculations
+ * on the dedicated SHAVE VLIW vector co-processors, which are orchestrated
+ * by one or more LEON SPARC v8 real time cores.
+ *
+ * However, they need firmware to be loaded beforehand. An uninitialised
+ * Myriad ma24xx presents with a distinctive USB ID. After firmware
+ * loading, the device detaches from the USB bus and reattaches with a new
+ * device ID. It can then be claimed by the usermode driver.
+ *
+ * The firmware is non-free and must be extracted by the user.
+ */
+
+/* Standard include files */
+#include
+#include
+#include
+#include
+#include
+
+#define usb_dbg(usb_if, format, arg...) \
+ dev_dbg(&(usb_if)->dev, format, ## arg)
+
+#define usb_err(usb_if, format, arg...) \
+ dev_err(&(usb_if)->dev, format, ## arg)
+
+/* Version Information */
+#define DRIVER_AUTHOR "Rhys Kidd "
+#define DRIVER_DESC "Driver for loading USB Myriad ma24xx firmware"
+#define DRIVER_SHORT "myriad_ma24xx_vsc"
+
+MODULE_AUTHOR(DRIVER_AUTHOR);
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE("GPL");
+
+#define FW_DIR "myriad/"
+#define MA2450_FIRMWARE FW_DIR "mvncapi-2450.mvcmd"
+#define MA2480_FIRMWARE FW_DIR "mvncapi-2480.mvcmd"
+
+MODULE_FIRMWARE(MA2450_FIRMWARE);
+MODULE_FIRMWARE(MA2480_FIRMWARE);
+
+enum {
+ MA2450FW = 0,
+ MA2480FW
+};
+
+/* macros for struct usb_device_id */
+#define MYRIAD_CHIP_VERSION(x) \
+ ((x)->driver_info & 0xf)
+
+#define MYRIAD_VID 0x03e7 /* Movidius Ltd, an Intel company */
+#define MA2450_PID 0x2150 /* ma2450 VSC loopback device, without fw */
+#define MA2485_PID 0x2485 /* ma2485 VSC loopback device, without fw */
+
+#define MYRIAD_BUF_LEN 512 /* max size of USB_SPEED_HIGH packet */
+#define WRITE_TIMEOUT 2000 /* milliseconds */
+
+static const struct usb_device_id id_table[] = {
+ { USB_DEVICE(MYRIAD_VID, MA2450_PID)
[PATCH V2 0/3] ADD interconnect support for USB
This path series aims to add interconnect support in dwc3-qcom driver on SDM845 SoCs. Changes since V1: > Comments by Georgi Djakov on "[PATCH 2/3]" addressed > [PATCH 1/3] and [PATCH 3/3] remains unchanged Chandana Kishori Chiluveru (3): dt-bindings: Introduce interconnect bindings for usb usb: dwc3: qcom: Add interconnect support in dwc3 driver arm64: dts: sdm845: Add interconnect properties for USB .../devicetree/bindings/usb/qcom,dwc3.txt | 13 ++ arch/arm64/boot/dts/qcom/sdm845.dtsi | 12 ++ drivers/usb/dwc3/dwc3-qcom.c | 145 - 3 files changed, 168 insertions(+), 2 deletions(-) -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
[PATCH V2 2/3] usb: dwc3: qcom: Add interconnect support in dwc3 driver
Add interconnect support in dwc3-qcom driver to vote for bus
bandwidth.
This requires for two different paths - from USB master to
DDR slave. The other is from APPS master to USB slave.
Signed-off-by: Chandana Kishori Chiluveru
---
drivers/usb/dwc3/dwc3-qcom.c | 145 ++-
1 file changed, 143 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index 184df4d..4f6c9736 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -59,8 +60,13 @@ struct dwc3_qcom {
enum usb_dr_modemode;
boolis_suspended;
boolpm_suspended;
+ struct icc_path *usb_ddr_icc_path;
+ struct icc_path *apps_usb_icc_path;
};
+static int usb_interconnect_enable(struct dwc3_qcom *qcom);
+static int usb_interconnect_disable(struct dwc3_qcom *qcom);
+
static inline void dwc3_qcom_setbits(void __iomem *base, u32 offset, u32 val)
{
u32 reg;
@@ -222,7 +228,7 @@ static void dwc3_qcom_enable_interrupts(struct dwc3_qcom
*qcom)
static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
{
u32 val;
- int i;
+ int i, ret;
if (qcom->is_suspended)
return 0;
@@ -234,6 +240,11 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
for (i = qcom->num_clocks - 1; i >= 0; i--)
clk_disable_unprepare(qcom->clks[i]);
+ /* Remove bus voting */
+ ret = usb_interconnect_disable(qcom);
+ if (ret)
+ dev_err(qcom->dev, "bus bw voting failed %d\n", ret);
+
qcom->is_suspended = true;
dwc3_qcom_enable_interrupts(qcom);
@@ -259,6 +270,11 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom)
}
}
+ /* Add bus voting */
+ ret = usb_interconnect_enable(qcom);
+ if (ret)
+ dev_err(qcom->dev, "bus bw voting failed %d\n", ret);
+
/* Clear existing events from PHY related to L2 in/out */
dwc3_qcom_setbits(qcom->qscratch_base, PWR_EVNT_IRQ_STAT_REG,
PWR_EVNT_LPM_IN_L2_MASK | PWR_EVNT_LPM_OUT_L2_MASK);
@@ -268,6 +284,117 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom)
return 0;
}
+/* Interconnect path bandwidths in MBps */
+#define USB_MEMORY_AVG_HS_BW MBps_to_icc(240)
+#define USB_MEMORY_PEAK_HS_BW MBps_to_icc(700)
+#define USB_MEMORY_AVG_SS_BW MBps_to_icc(1000)
+#define USB_MEMORY_PEAK_SS_BW MBps_to_icc(2500)
+#define APPS_USB_AVG_BW 0
+#define APPS_USB_PEAK_BW MBps_to_icc(40)
+
+/**
+ * usb_interconnect_init() - Request to get interconnect path handle
+ * @qcom: Pointer to the concerned usb core.
+ *
+ */
+static int usb_interconnect_init(struct dwc3_qcom *qcom)
+{
+ struct device *dev = qcom->dev;
+
+ qcom->usb_ddr_icc_path = of_icc_get(dev, "usb-ddr");
+ if (IS_ERR(qcom->usb_ddr_icc_path)) {
+ dev_err(dev, "Error: (%ld) failed getting %s path\n",
+ PTR_ERR(qcom->usb_ddr_icc_path), "usb-ddr");
+ return PTR_ERR(qcom->usb_ddr_icc_path);
+ }
+
+ qcom->apps_usb_icc_path = of_icc_get(dev, "apps-usb");
+ if (IS_ERR(qcom->apps_usb_icc_path)) {
+ dev_err(dev, "Error: (%ld) failed getting %s path\n",
+ PTR_ERR(qcom->apps_usb_icc_path), "apps-usb");
+ return PTR_ERR(qcom->usb_ddr_icc_path);
+ }
+
+ return 0;
+}
+
+/**
+ * geni_interconnect_exit() - Request to release interconnect path handle
+ * @qcom: Pointer to the concerned usb core.
+ *
+ * This function is used to release interconnect path handle.
+ */
+static void usb_interconnect_exit(struct dwc3_qcom *qcom)
+{
+ icc_put(qcom->usb_ddr_icc_path);
+ icc_put(qcom->apps_usb_icc_path);
+}
+
+/* Currently we only use bandwidth level, so just "enable" interconnects */
+static int usb_interconnect_enable(struct dwc3_qcom *qcom)
+{
+ struct dwc3 *dwc;
+ int ret;
+
+ dwc = platform_get_drvdata(qcom->dwc3);
+ if (!dwc) {
+ dev_err(qcom->dev, "Failed to get dwc3 device\n");
+ return -EPROBE_DEFER;
+ }
+
+ if (dwc->maximum_speed == USB_SPEED_SUPER) {
+ ret = icc_set_bw(qcom->usb_ddr_icc_path,
+ USB_MEMORY_AVG_SS_BW, USB_MEMORY_PEAK_SS_BW);
+ if (ret)
+ return ret;
+ } else {
+ ret = icc_set_bw(qcom->usb_ddr_icc_path,
+ USB_MEMORY_AVG_HS_BW, USB_MEMORY_PEAK_HS_BW);
+ if (ret)
+ return ret;
+ }
+
+ ret = icc_set_bw(qcom->apps_usb_icc_path,
+ APPS_USB_AVG_BW, APPS_USB_PEAK_BW);
+ if (ret)
+ goto err_disable_mem_path;
+
+ return
[PATCH V2 1/3] dt-bindings: Introduce interconnect bindings for usb
Add documentation for the interconnects and interconnect-names bindings for USB as detailed by bindings/interconnect/interconnect.txt. Signed-off-by: Chandana Kishori Chiluveru --- Documentation/devicetree/bindings/usb/qcom,dwc3.txt | 13 + 1 file changed, 13 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt index cb695aa..7e9cb97 100644 --- a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt +++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt @@ -33,6 +33,16 @@ Optional clocks: Optional properties: - resets: Phandle to reset control that resets core and wrapper. +- interconnects: Pairs of phandles and interconnect provider specifier + to denote the edge source and destination ports of + the interconnect path. Please refer to + Documentation/devicetree/bindings/interconnect/ + for more details. +- interconnect-names: List of interconnect path name strings sorted in the same + order as the interconnects property. Consumers drivers will use + interconnect-names to match interconnect paths with interconnect + specifiers. Please refer to Documentation/devicetree/bindings/ + interconnect/ for more details. - interrupts: specifies interrupts from controller wrapper used to wakeup from low power/susepnd state. Must contain one or more entry for interrupt-names property @@ -74,6 +84,9 @@ Example device nodes: #size-cells = <1>; ranges; + interconnects = <&qnoc MASTER_USB3_0 &qnoc SLAVE_EBI1>, + <&qnoc MASTER_APPSS_PROC &qnoc SLAVE_USB3_0>; + interconnect-names = "usb-ddr", "apps-usb"; interrupts = <0 131 0>, <0 486 0>, <0 488 0>, <0 489 0>; interrupt-names = "hs_phy_irq", "ss_phy_irq", "dm_hs_phy_irq", "dp_hs_phy_irq"; -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
[PATCH V2 3/3] arm64: dts: sdm845: Add interconnect properties for USB
Populate USB DT node with interconnect properties.
Signed-off-by: Chandana Kishori Chiluveru
---
arch/arm64/boot/dts/qcom/sdm845.dtsi | 12
1 file changed, 12 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index fcb9330..1c41922 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -1837,6 +1837,12 @@
resets = <&gcc GCC_USB30_PRIM_BCR>;
+ interconnects = <&rsc_hlos MASTER_USB3_0
+ &rsc_hlos SLAVE_EBI1>,
+ <&rsc_hlos MASTER_APPSS_PROC
+ &rsc_hlos SLAVE_USB3_0>;
+ interconnect-names = "usb-ddr", "apps-usb";
+
usb_1_dwc3: dwc3@a60 {
compatible = "snps,dwc3";
reg = <0 0x0a60 0 0xcd00>;
@@ -1881,6 +1887,12 @@
resets = <&gcc GCC_USB30_SEC_BCR>;
+ interconnects = <&rsc_hlos MASTER_USB3_1
+ &rsc_hlos SLAVE_EBI1>,
+ <&rsc_hlos MASTER_APPSS_PROC
+ &rsc_hlos SLAVE_USB3_1>;
+ interconnect-names = "usb-ddr", "apps-usb";
+
usb_2_dwc3: dwc3@a80 {
compatible = "snps,dwc3";
reg = <0 0x0a80 0 0xcd00>;
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: No SuperSpeedPlus on ASM2142
On 16.9.2019 5.41, Loïc Yhuel wrote: Hello, I'm trying to get Gen 2 working on this controller. It drives 2 USB ports on the back panel of an ASUS Prime X399-A (latest BIOS). ASM2142 FW is 170308_70_02_00 (seen with ASM2142A_MPTool on Windows). On Windows 10 it uses the Microsoft xhci driver, and Gen 2 works. On 5.3, I get : [1.008270] xhci_hcd :08:00.0: Host supports USB 3.0 SuperSpeed ... [1.333145] usb 4-1: new SuperSpeed Gen 1 USB device number 2 using xhci_hcd lsusb shows 10 Gbps support in "SuperSpeedPlus USB Device Capability" for both the root hub and the device. For the root hub, commit ddd57980 broke the detection, since xhci->usb3_rhub.min_rev is 0x1 instead of expected 0x10 (SBRN is 0x30). So both places that indicate USB 3.1 support are not according to latest spec, SBRN (Serial Bus Release Number) is 30 instead of 31, and supported protocol capability minor revision is 0x1 instead of 0x10. Reverting it changes to "Host supports USB 3.1 Enhanced SuperSpeed", and the speed of the root hub is 1 in sysfs. However, I only got the device detected as "SuperSpeedPlus Gen 2 USB" once, and the performance didn't increase, so even if the "speed" in sysfs was 1, I think it didn't work. After a reboot, it reverted to being detected as Gen 1. Most reliable way of checking the current actual port speed is reading protocol speed id from the ports PORTSC register port-speed field. Use debugfs: (with your correct pci address and port number) cat /sys/kernel/debug/usb/xhci/\:00\:15.0/ports/port13/portsc Powered Connected Enabled Link:U0 PortSpeed:4 Change: Wake: PortSpeed:4 is default for SS 5Gbps, Gen1x1 PortSpeed:5 is defaulf for SSP 10Gbps, Gen2x1 The device (JMS580 USB Gen 2 to SATA bridge, with an SSD) seems to have a performance issue on Gen 1 (doesn't depend on the controller or the OS), with about 280MB/s read (almost the same without UAS). But Gen 2 on Windows gives 510MB/s read, so even the only time Linux reported 10 Gbps speed, if it was working "hdparm -t" should have improved. I recall similar numbers in Linux, ~500-550Mb/s with a USB 3.1 to SATA adapter and fast SSD. Reading with dd As a side note, the runtime power management doesn't seem to work either, but since it isn't the default configuration, unless this controller is used on laptops it probably doesn't matter. If the power/control of the PCIe device and its two root hubs are all set to "auto", it is suspended if there is no USB device, and doesn't wake up on plug. Is the xHCI controller id PCI D0 state even when runtime suspeded? Some ACPI tables end up preventing D3 for runtime suspend, keeping controller in D0 and possibly preventing PME# wake signaling -Mathias
Re: Event ring is full when do iozone test on UAS storage
On 16.9.2019 12.41, Peter Chen wrote: Hi Mathias, I met "event ring full error" like below, this error is met when I do iozone test on UAS storage at v4.19.35 kernel, but not meet this error at linux-next tree (08/24). The same host and test UAS storage device are used. This issue is due to xhci_handle_event does not return 0 long time, maybe the xHC speed is fast enough at that time. If I force the xhci_handle_event only run 100 times before update ERST dequene pointer, it will not occur this error. I did not see any changes for xhci_handle_event at the latest code, so in theory, it should have this issue too. Do you think if we need to improve xhci_handle_event to avoid event ring? Possibly. We need to check the details of what types of events the ring is filled with, and why handling them takes so long. does irqsoff tracing show anything blocking interrupts for long? It's also possible that we don't get interrupts early enough. Either if interupts are moderated, or event ring is filled with events that don't generate interrupts (BEI flag set). -Mathias
Re: No SuperSpeedPlus on ASM2142
Le lun. 16 sept. 2019 à 14:57, Mathias Nyman a écrit : > So both places that indicate USB 3.1 support are not according to latest spec, > SBRN (Serial Bus Release Number) is 30 instead of 31, and supported protocol > capability minor revision is 0x1 instead of 0x10. Yes. I searched for firmwares, but I only saw a much older version available on Internet. > Most reliable way of checking the current actual port speed is reading > protocol speed id > from the ports PORTSC register port-speed field. > Use debugfs: (with your correct pci address and port number) Currently I have "PortSpeed:4" which matches with the "Gen 1" trace. If I even get a "Gen 2" trace again, I will check. > Is the xHCI controller id PCI D0 state even when runtime suspeded? > Some ACPI tables end up preventing D3 for runtime suspend, keeping controller > in D0 > and possibly preventing PME# wake signaling It seems you are right, lspci still shows D0 : Flags: PMEClk- DSI- D1- D2- AuxCurrent=55mA PME(D0+,D1-,D2-,D3hot-,D3cold-) Status: D0 NoSoftRst+ PME-Enable+ DSel=0 DScale=0 PME- The other USB controllers (AMD 3.0 and 3.1) have "PME(D0+,D1-,D2-,D3hot+,D3cold+)", are in D3 state when runtime suspended, and wake up correctly. Looking at this, I realized the front panel USB3.0 connectors are on the AMD 3.1 controller, I wonder how they are limited to SuperSpeed (configured by the BIOS ?).
Re: [RFC PATCH 1/1] USB: myriad-ma24xx-vsc: Firmware loader driver for USB Myriad ma24xx
On Mon, 16 Sep 2019, Rhys Kidd wrote: > The Myriad ma24xx in USB Intel Neural Compute Stick and Intel Neural > Compute Stick 2 provides an API to accelerate AI inference calculations > on the dedicated SHAVE VLIW vector co-processors, which are orchestrated > by one or more LEON SPARC v8 real time cores. > > However, they need firmware to be loaded beforehand. An uninitialised > Myriad ma24xx presents with a distinctive USB ID. After firmware > loading, the device detaches from the USB bus and reattaches with a new > device ID. It can then be claimed by the usermode driver. Is there any particular reason you need to have a kernel driver load the firmware? Why can't a user program take care of it? The usb_modeswitch program does that sort of thing all the time. Alan Stern
Re: No SuperSpeedPlus on ASM2142
Le lun. 16 sept. 2019 à 17:19, Loïc Yhuel a écrit : > > > Most reliable way of checking the current actual port speed is reading > > protocol speed id > > from the ports PORTSC register port-speed field. > > Use debugfs: (with your correct pci address and port number) > Currently I have "PortSpeed:4" which matches with the "Gen 1" trace. > If I even get a "Gen 2" trace again, I will check. > Even after several reboots into Linux, I was still getting "Gen 1" / "PortSpeed 4". So I tried booting into Windows, HWinfo64 showed "Gen 2", then rebooting into Linux. Then I got "Gen 2" / "PortSpeed: 5" on a 5.3 with the revert, or 5.0 kernel. With the Fedora 5.2.14, it was "Gen 1" / "PortSpeed: 5". But performance was still the same as Gen 1. Reboot of poweroff didn't change anything. So I tried to play with the BIOS "Fast boot" option, even if it always lists the USB drives. I rebooted into Windows, HWInfo64 showed Gen 2, I did a performance test which returned 510MB/s. Then I rebooted into Linux, which then showed "Gen 1" / "PortSpeed: 4", but the performance was now good ! Even after several reboots, or toggling "Fast Boot" on/off, it didn't change : PortSpeed 4, but performance showing 10 Gbps was working. Then I tried powering off, unplugging AC and the USB cable, then booting without it. On plug, it was detected as "Gen 2", PortSpeed was 5, and performance was good. After a reboot (so device plugged on boot), still the same. After a poweroff, still the same. So I really don't know how to make sense of this. Perhaps a register of the xhci controller had a bad value kept even after a poweroff due to the standby +5V, and was reset either by the BIOS or Windows at some point. But that doesn't explain why the PORTSC value was many time incoherent with the performance, in both ways. Btw, I found another problem on resuming the system after a suspend : [ 137.029272] pcieport :00:01.1: PME: Spurious native interrupt! ... [ 137.129618] xhci_hcd :08:00.0: WARN: xHC restore state timeout [ 137.129624] xhci_hcd :08:00.0: PCI post-resume error -110! [ 137.129625] xhci_hcd :08:00.0: HC died; cleaning up [ 137.129633] PM: dpm_run_callback(): pci_pm_resume+0x0/0x90 returns -110 [ 137.129636] PM: Device :08:00.0 failed to resume async: error -110 Then a "echo 1 > remove, then "echo 1 > ../rescan" on sysfs got it back. This is a completely different issue, but at least, I can reproduce this one reliably.
Re: [PATCH 1/3] dt-bindings: Introduce interconnect bindings for usb
On Wed, Sep 11, 2019 at 10:24:33AM +0530, Chandana Kishori Chiluveru wrote: > Add documentation for the interconnects and interconnect-names > bindings for USB as detailed by bindings/interconnect/interconnect.txt. This isn't a generic binding for USB, but for the qcom,dwc3, the commit message (including subject) should reflect this. nit: you might want to replace 'bindings' with 'properties'. > Signed-off-by: Chandana Kishori Chiluveru > --- > Documentation/devicetree/bindings/usb/qcom,dwc3.txt | 13 + > 1 file changed, 13 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt > b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt > index cb695aa..7e9cb97 100644 > --- a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt > +++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt > @@ -33,6 +33,16 @@ Optional clocks: > > Optional properties: > - resets:Phandle to reset control that resets core and wrapper. > +- interconnects: Pairs of phandles and interconnect provider specifier consistency nit: should be either 'phandle' & 'specifier' or 'phandles' & 'specifiers'.
Re: [PATCH V2 2/3] usb: dwc3: qcom: Add interconnect support in dwc3 driver
Hi Chandana,
On Mon, Sep 16, 2019 at 05:11:00PM +0530, Chandana Kishori Chiluveru wrote:
> Add interconnect support in dwc3-qcom driver to vote for bus
> bandwidth.
>
> This requires for two different paths - from USB master to
> DDR slave. The other is from APPS master to USB slave.
>
> Signed-off-by: Chandana Kishori Chiluveru
> ---
> drivers/usb/dwc3/dwc3-qcom.c | 145
> ++-
> 1 file changed, 143 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index 184df4d..4f6c9736 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -14,6 +14,7 @@
> #include
> #include
> #include
> +#include
please use alphabetical order, i.e. this should be after 'linux/extcon.h'.
> #include
> #include
> #include
> @@ -59,8 +60,13 @@ struct dwc3_qcom {
> enum usb_dr_modemode;
> boolis_suspended;
> boolpm_suspended;
> + struct icc_path *usb_ddr_icc_path;
> + struct icc_path *apps_usb_icc_path;
> };
>
> +static int usb_interconnect_enable(struct dwc3_qcom *qcom);
> +static int usb_interconnect_disable(struct dwc3_qcom *qcom);
> +
> static inline void dwc3_qcom_setbits(void __iomem *base, u32 offset, u32 val)
> {
> u32 reg;
> @@ -222,7 +228,7 @@ static void dwc3_qcom_enable_interrupts(struct dwc3_qcom
> *qcom)
> static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
> {
> u32 val;
> - int i;
> + int i, ret;
>
> if (qcom->is_suspended)
> return 0;
> @@ -234,6 +240,11 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
> for (i = qcom->num_clocks - 1; i >= 0; i--)
> clk_disable_unprepare(qcom->clks[i]);
>
> + /* Remove bus voting */
IMO the comment doesn't add much, especially since there is a log in
case of failure.
> + ret = usb_interconnect_disable(qcom);
> + if (ret)
> + dev_err(qcom->dev, "bus bw voting failed %d\n", ret);
This should probably be a warning, since the function continues with
normal execution.
nit: the function is called usb_interconnect_disable(), but the
message says "bus bw voting failed". The function name encapsulates
what the function does, but the message talks about implementation
details. To be consistent either the function should have something
about 'voting' in it's name, or the message should be "failed to
disable interconnect" or similar.
> +
> qcom->is_suspended = true;
> dwc3_qcom_enable_interrupts(qcom);
>
> @@ -259,6 +270,11 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom)
> }
> }
>
> + /* Add bus voting */
> + ret = usb_interconnect_enable(qcom);
> + if (ret)
> + dev_err(qcom->dev, "bus bw voting failed %d\n", ret);
> +
same comments as for suspend()
> /* Clear existing events from PHY related to L2 in/out */
> dwc3_qcom_setbits(qcom->qscratch_base, PWR_EVNT_IRQ_STAT_REG,
> PWR_EVNT_LPM_IN_L2_MASK | PWR_EVNT_LPM_OUT_L2_MASK);
> @@ -268,6 +284,117 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom)
> return 0;
> }
>
> +/* Interconnect path bandwidths in MBps */
> +#define USB_MEMORY_AVG_HS_BW MBps_to_icc(240)
> +#define USB_MEMORY_PEAK_HS_BW MBps_to_icc(700)
> +#define USB_MEMORY_AVG_SS_BW MBps_to_icc(1000)
> +#define USB_MEMORY_PEAK_SS_BW MBps_to_icc(2500)
> +#define APPS_USB_AVG_BW 0
> +#define APPS_USB_PEAK_BW MBps_to_icc(40)
> +
> +/**
> + * usb_interconnect_init() -
A function named 'usb_*' is typically associated to be a USB core
function. I would suggest to use the 'dwc3_qcom_' prefix like all
other functions in this file. Applicable to all new functions.
> Request to get interconnect path handle
nit: handles
Get rid of "Request to"?
> + * @qcom:Pointer to the concerned usb core.
> + *
> + */
> +static int usb_interconnect_init(struct dwc3_qcom *qcom)
> +{
> + struct device *dev = qcom->dev;
> +
> + qcom->usb_ddr_icc_path = of_icc_get(dev, "usb-ddr");
> + if (IS_ERR(qcom->usb_ddr_icc_path)) {
> + dev_err(dev, "Error: (%ld) failed getting %s path\n",
> + PTR_ERR(qcom->usb_ddr_icc_path), "usb-ddr");
Why not put 'usb-ddr' in the format string, instead of using '%s'?
> + return PTR_ERR(qcom->usb_ddr_icc_path);
> + }
> +
> + qcom->apps_usb_icc_path = of_icc_get(dev, "apps-usb");
> + if (IS_ERR(qcom->apps_usb_icc_path)) {
> + dev_err(dev, "Error: (%ld) failed getting %s path\n",
> + PTR_ERR(qcom->apps_usb_icc_path), "apps-usb");
same comment as above.
icc_put(qcom->usb_ddr_icc_path);
> + return PTR_ERR(qcom->usb_ddr_icc_path);
should be 'qcom->apps_usb_icc_path'
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * geni_interconnect_exit() -
wrong prefix
> Request to release
Re: [PATCH V2 3/3] arm64: dts: sdm845: Add interconnect properties for USB
Hi Chandana.
On Mon, Sep 16, 2019 at 05:11:01PM +0530, Chandana Kishori Chiluveru wrote:
> Populate USB DT node with interconnect properties.
nit: nodes
> Signed-off-by: Chandana Kishori Chiluveru
> ---
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index fcb9330..1c41922 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -1837,6 +1837,12 @@
>
> resets = <&gcc GCC_USB30_PRIM_BCR>;
>
> + interconnects = <&rsc_hlos MASTER_USB3_0
> + &rsc_hlos SLAVE_EBI1>,
nit: align second line after '<' for better readability (like
'interrupts' or 'clocks' properties):
interconnects = <&rsc_hlos MASTER_USB3_0
&rsc_hlos SLAVE_EBI1>,
same for the other entries.
> + <&rsc_hlos MASTER_APPSS_PROC
> + &rsc_hlos SLAVE_USB3_0>;
> + interconnect-names = "usb-ddr", "apps-usb";
> +
> usb_1_dwc3: dwc3@a60 {
> compatible = "snps,dwc3";
> reg = <0 0x0a60 0 0xcd00>;
> @@ -1881,6 +1887,12 @@
>
> resets = <&gcc GCC_USB30_SEC_BCR>;
>
> + interconnects = <&rsc_hlos MASTER_USB3_1
> + &rsc_hlos SLAVE_EBI1>,
> + <&rsc_hlos MASTER_APPSS_PROC
> + &rsc_hlos SLAVE_USB3_1>;
> + interconnect-names = "usb-ddr", "apps-usb";
> +
> usb_2_dwc3: dwc3@a80 {
> compatible = "snps,dwc3";
> reg = <0 0x0a80 0 0xcd00>;
Besides the nits:
Reviewed-by: Matthias Kaehlcke
Re: patch "usbip: tools: fix GCC8 warning for strncpy" added to usb-next
Hi Greg, Do we have plan to merge this patch into 5.3 Release? I hit the build problem(warning turns to be error under -Werror) when building 5.3 version. B.R. Changcheng On 09:02 Fri 26 Jul, gre...@linuxfoundation.org wrote: > > This is a note to let you know that I've just added the patch titled > > usbip: tools: fix GCC8 warning for strncpy > > to my usb git tree which can be found at > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git > in the usb-next branch. > > The patch will show up in the next release of the linux-next tree > (usually sometime within the next 24 hours during the week.) > > The patch will also be merged in the next major kernel release > during the merge window. > > If you have any questions about this process, please let me know. > > > From 6389a62ff798e781567645c0b0ca3dd7b8a4289d Mon Sep 17 00:00:00 2001 > From: "Liu, Changcheng" > Date: Thu, 25 Jul 2019 21:22:09 +0800 > Subject: usbip: tools: fix GCC8 warning for strncpy > > GCC8 started emitting warning about using strncpy with number of bytes > exactly equal destination size which could lead to non-zero terminated > string being copied. Use "SYSFS_PATH_MAX - 1" & "SYSFS_BUS_ID_SIZE - 1" > as number of bytes to ensure name is always zero-terminated. > > Signed-off-by: Changcheng Liu > Acked-by: Shuah Khan > Link: https://lore.kernel.org/r/20190725132209.GA27590@jerryopenix > Signed-off-by: Greg Kroah-Hartman > --- > tools/usb/usbip/libsrc/usbip_common.c| 6 -- > tools/usb/usbip/libsrc/usbip_device_driver.c | 6 -- > 2 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/tools/usb/usbip/libsrc/usbip_common.c > b/tools/usb/usbip/libsrc/usbip_common.c > index bb424638d75b..b8d7d480595a 100644 > --- a/tools/usb/usbip/libsrc/usbip_common.c > +++ b/tools/usb/usbip/libsrc/usbip_common.c > @@ -226,8 +226,10 @@ int read_usb_device(struct udev_device *sdev, struct > usbip_usb_device *udev) > path = udev_device_get_syspath(sdev); > name = udev_device_get_sysname(sdev); > > - strncpy(udev->path, path, SYSFS_PATH_MAX); > - strncpy(udev->busid, name, SYSFS_BUS_ID_SIZE); > + strncpy(udev->path, path, SYSFS_PATH_MAX - 1); > + udev->path[SYSFS_PATH_MAX - 1] = '\0'; > + strncpy(udev->busid, name, SYSFS_BUS_ID_SIZE - 1); > + udev->busid[SYSFS_BUS_ID_SIZE - 1] = '\0'; > > sscanf(name, "%u-%u", &busnum, &devnum); > udev->busnum = busnum; > diff --git a/tools/usb/usbip/libsrc/usbip_device_driver.c > b/tools/usb/usbip/libsrc/usbip_device_driver.c > index 5a3726eb44ab..051d7d3f443b 100644 > --- a/tools/usb/usbip/libsrc/usbip_device_driver.c > +++ b/tools/usb/usbip/libsrc/usbip_device_driver.c > @@ -91,7 +91,8 @@ int read_usb_vudc_device(struct udev_device *sdev, struct > usbip_usb_device *dev) > copy_descr_attr16(dev, &descr, idProduct); > copy_descr_attr16(dev, &descr, bcdDevice); > > - strncpy(dev->path, path, SYSFS_PATH_MAX); > + strncpy(dev->path, path, SYSFS_PATH_MAX - 1); > + dev->path[SYSFS_PATH_MAX - 1] = '\0'; > > dev->speed = USB_SPEED_UNKNOWN; > speed = udev_device_get_sysattr_value(sdev, "current_speed"); > @@ -110,7 +111,8 @@ int read_usb_vudc_device(struct udev_device *sdev, struct > usbip_usb_device *dev) > dev->busnum = 0; > > name = udev_device_get_sysname(plat); > - strncpy(dev->busid, name, SYSFS_BUS_ID_SIZE); > + strncpy(dev->busid, name, SYSFS_BUS_ID_SIZE - 1); > + dev->busid[SYSFS_BUS_ID_SIZE - 1] = '\0'; > return 0; > err: > fclose(fd); > -- > 2.22.0 > >
Re: [PATCH V2 0/3] ADD interconnect support for USB
On 9/16/2019 5:10 PM, Chandana Kishori Chiluveru wrote: > This path series aims to add interconnect support in > dwc3-qcom driver on SDM845 SoCs. As Matthias commented on other patch, can you update the subject to mention that series is for dwc3-qcom only? And update same dt-bindings patch commit subject as well > > Changes since V1: > > Comments by Georgi Djakov on "[PATCH 2/3]" addressed > > [PATCH 1/3] and [PATCH 3/3] remains unchanged > > Chandana Kishori Chiluveru (3): > dt-bindings: Introduce interconnect bindings for usb > usb: dwc3: qcom: Add interconnect support in dwc3 driver > arm64: dts: sdm845: Add interconnect properties for USB > > .../devicetree/bindings/usb/qcom,dwc3.txt | 13 ++ > arch/arm64/boot/dts/qcom/sdm845.dtsi | 12 ++ > drivers/usb/dwc3/dwc3-qcom.c | 145 > - > 3 files changed, 168 insertions(+), 2 deletions(-) > -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: patch "usbip: tools: fix GCC8 warning for strncpy" added to usb-next
A: http://en.wikipedia.org/wiki/Top_post Q: Were do I find info about this thing called top-posting? A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? A: No. Q: Should I include quotations after my reply? http://daringfireball.net/2007/07/on_top On Tue, Sep 17, 2019 at 09:13:15AM +0800, Liu, Changcheng wrote: > Hi Greg, >Do we have plan to merge this patch into 5.3 Release? >I hit the build problem(warning turns to be error under -Werror) when >building 5.3 version. This will be submitted for 5.4-rc1. If this is needed in 5.3, please ask sta...@vger.kernel.org to backport it once it is in Linus's tree. But we do not build the kernel with -Werror, so this should not be a "real" problem for you. thanks, greg k-h
