Re: [PATCH] usb: gadget: serial: replace hardcoded ttyGS with PREFIX
Hello, thanks for your reply! On Fri, 27 Jun 2014 10:46:28 -0700 Greg Kroah-Hartman gre...@linuxfoundation.org wrote: On Fri, Jun 27, 2014 at 01:37:21PM +0200, Richard Leitner wrote: Replace all hardcoded ttyGS strings with the PREFIX macro. Why? Because IMHO if PREFIX is available it should be used everywhere possible. Furthermore if you change the PREFIX the debug output wouldn't be consistent any more. Therefore the PREFIX definition is moved to u_serial.h. Why? Otherwise f_acm.c, f_obex.c and f_serial.c wouldn't know it. Isn't u_serial.h the right place for it? Furthermore the modified files are checkpatch.pl compliant now. You are doing two different things here in the same patch. Please split it up into two different patches. Ok, I'll do that. thanks, greg k-h thanks regards, richard -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] usb: chipidea: udc: delete td from req's td list at ep_dequeue
Hello Peter,
W dniu 30.06.2014 06:15, Peter Chen pisze:
We need to delete un-finished td from current request's td list
at ep_dequeue API, otherwise, this non-user td will be remained
at td list before this request is freed. So if we do ep_queue-
ep_dequeue-ep_queue sequence, when the complete interrupt for
the second ep_queue comes, we search td list for this request,
the first td (added by the first ep_queue) will be handled, and
its status is still active, so we will consider the this transfer
still not be completed, but in fact, it has completed. It causes
the peripheral side considers it never receives current data for
this transfer.
snip
+ struct td_node *node, *tmpnode;
if (ep == NULL || req == NULL || hwreq-req.status != -EALREADY ||
hwep-ep.desc == NULL || list_empty(hwreq-queue) ||
@@ -1331,6 +1332,13 @@ static int ep_dequeue(struct usb_ep *ep, struct
usb_request *req)
hw_ep_flush(hwep-ci, hwep-num, hwep-dir);
+ list_for_each_entry_safe(node, tmpnode, hwreq-tds, td) {
+ dma_pool_free(hwep-td_pool, node-ptr, node-dma);
+ list_del_init(node-td);
+ node-ptr = NULL;
Why bother initializing node-td's prev and next members and assigning NULL
to ptrif...
+ kfree(node);
... in the very next line the whole structure is freed?
I would suggest
+ list_for_each_entry_safe(node, tmpnode, hwreq-tds, td) {
+ dma_pool_free(hwep-td_pool, node-ptr, node-dma);
+ list_del(node-td);
+ kfree(node);
+ }
AP
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: host: Kconfig: Select PHY drivers for Exynos EHCI/OHCI
On Fri, Jun 27, 2014 at 10:23 PM, Olof Johansson o...@lixom.net wrote: On Fri, Jun 27, 2014 at 9:32 AM, Doug Anderson diand...@chromium.org wrote: Felipe, On Fri, Jun 27, 2014 at 8:59 AM, Felipe Balbi ba...@ti.com wrote: I'll admit to not having been involved with the previous discussions, but this seems strange to me. Are we throwing in the towel and deciding that it's too hard to get the Kconfigs right and that we'll just rely on individual users to figure out the right answer for themselves? no. select prevents a driver from be built as a dynamically linked module and distro-kernels might want to enable everything as modules. Ah, that's what the problem was! I wasn't aware of this issue with SELECT. Sorry for the noob-ness. Select is also fragile, for example if a main subsystem isn't enabled, the specific option will still be enabled (or there'll be a warning/error about it). Overall it tends to cause headaches so many maintainers are starting to push back against it. Really we want the PHY to be =y if the USB driver is =y or =m if the USB driver is =m, I think. You could argue that one might want to build the main USB driver into the kernel but have the phy drivers as modules, so you could possibly also try to support that... If there's not a good way to specify that, I guess we'll just have to use default and rely on the user not to purposely choose the wrong thing. Like the following (untested): config PHY_SAMSUNG_USB2 depends on USB_OHCI_EXYNOS || USB_EHCI_EXYNOS default y if ARCH_EXYNOS=y (USB_OHCI_EXYNOS=y || USB_EHCI_EXYNOS=y) default m if ARCH_EXYNOS=m (USB_OHCI_EXYNOS=m || USB_EHCI_EXYNOS=m) ... I see some syntax like that elsewhere in Kconfig so I assume it's reasonable... I think you can take out the test for ARCH_EXYNOS here (first of all, it can never be modular). I'm not sure it makes sense to work so hard to set the default right either, as long as the dependencies are correct. I.e. it'll still mess up randconfig if the combination doesn't build, and distros can still downgrade to 'm' if they want to. That'll just leave: config PHY_SAMSUNG_USB2 tristate foo depends on USB_OHCI_EXYNOS || USB_EHCI_EXYNOS default y (no need to add an if since it's taken care of by the depends) With the above the user could purposely enable the OHCI or EHCI driver and disable the PHY driver which is not really sensible. ...but it wouldn't cause a compile failure or crash--USB just won't work. Just make the sub-drivers silent options with defaults. I.e: config PHY_EXYNOS5250_USB2 bool SOC_EXYNOS5250 depends on PHY_SAMSUNG_USB2 and so on. Note that it doesn't actually scale to make it depend on a specific SoC though, so it should probably just be cut down the line of EXYNOS4/5 and err on the side of including a bit too much code instead. Yes, that seems the right thing to do. However, for now I will retain the SoC based structure. Considering the fragility involved in using 'select', I will re-do this by playing around with the default option. Thanks everyone for your inputs. -- Regards, Sachin. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2] usb: gadget: serial: replace hardcoded ttyGS with PREFIX
Replaces all hardcoded ttyGS strings with the PREFIX macro.
Due to the fact the strings are spread over different source files the
PREFIX definition is moved to u_serial.h
Signed-off-by: Richard Leitner richard.leit...@skidata.com
---
v2: removed checkpatch.pl resovling (will be in a separate patch)
---
drivers/usb/gadget/f_acm.c| 36 ++--
drivers/usb/gadget/f_obex.c | 27 ++-
drivers/usb/gadget/f_serial.c | 10 +-
drivers/usb/gadget/u_serial.c | 30 +++---
drivers/usb/gadget/u_serial.h |1 +
5 files changed, 53 insertions(+), 51 deletions(-)
diff --git a/drivers/usb/gadget/f_acm.c b/drivers/usb/gadget/f_acm.c
index ab1065a..c6e6457 100644
--- a/drivers/usb/gadget/f_acm.c
+++ b/drivers/usb/gadget/f_acm.c
@@ -313,15 +313,15 @@ static void acm_complete_set_line_coding(struct usb_ep
*ep,
struct usb_composite_dev *cdev = acm-port.func.config-cdev;
if (req-status != 0) {
- DBG(cdev, acm ttyGS%d completion, err %d\n,
- acm-port_num, req-status);
+ DBG(cdev, acm %s%d completion, err %d\n,
+ PREFIX, acm-port_num, req-status);
return;
}
/* normal completion */
if (req-actual != sizeof(acm-port_line_coding)) {
- DBG(cdev, acm ttyGS%d short resp, len %d\n,
- acm-port_num, req-actual);
+ DBG(cdev, acm %s%d short resp, len %d\n,
+ PREFIX, acm-port_num, req-actual);
usb_ep_set_halt(ep);
} else {
struct usb_cdc_line_coding *value = req-buf;
@@ -404,15 +404,15 @@ invalid:
/* respond with data transfer or status phase? */
if (value = 0) {
- DBG(cdev, acm ttyGS%d req%02x.%02x v%04x i%04x l%d\n,
- acm-port_num, ctrl-bRequestType, ctrl-bRequest,
- w_value, w_index, w_length);
+ DBG(cdev, acm %s%d req%02x.%02x v%04x i%04x l%d\n,
+ PREFIX, acm-port_num, ctrl-bRequestType,
+ ctrl-bRequest, w_value, w_index, w_length);
req-zero = 0;
req-length = value;
value = usb_ep_queue(cdev-gadget-ep0, req, GFP_ATOMIC);
if (value 0)
- ERROR(cdev, acm response on ttyGS%d, err %d\n,
- acm-port_num, value);
+ ERROR(cdev, acm response on %s%d, err %d\n,
+ PREFIX, acm-port_num, value);
}
/* device either stalls (value 0) or reports success */
@@ -440,11 +440,11 @@ static int acm_set_alt(struct usb_function *f, unsigned
intf, unsigned alt)
} else if (intf == acm-data_id) {
if (acm-port.in-driver_data) {
- DBG(cdev, reset acm ttyGS%d\n, acm-port_num);
+ DBG(cdev, reset acm %s%d\n, PREFIX, acm-port_num);
gserial_disconnect(acm-port);
}
if (!acm-port.in-desc || !acm-port.out-desc) {
- DBG(cdev, activate acm ttyGS%d\n, acm-port_num);
+ DBG(cdev, activate acm %s%d\n, PREFIX, acm-port_num);
if (config_ep_by_speed(cdev-gadget, f,
acm-port.in) ||
config_ep_by_speed(cdev-gadget, f,
@@ -467,7 +467,7 @@ static void acm_disable(struct usb_function *f)
struct f_acm*acm = func_to_acm(f);
struct usb_composite_dev *cdev = f-config-cdev;
- DBG(cdev, acm ttyGS%d deactivated\n, acm-port_num);
+ DBG(cdev, acm %s%d deactivated\n, PREFIX, acm-port_num);
gserial_disconnect(acm-port);
usb_ep_disable(acm-notify);
acm-notify-driver_data = NULL;
@@ -522,8 +522,8 @@ static int acm_cdc_notify(struct f_acm *acm, u8 type, u16
value,
if (status 0) {
ERROR(acm-port.func.config-cdev,
- acm ttyGS%d can't notify serial state, %d\n,
- acm-port_num, status);
+ acm %s%d can't notify serial state, %d\n,
+ PREFIX, acm-port_num, status);
acm-notify_req = req;
}
@@ -537,8 +537,8 @@ static int acm_notify_serial_state(struct f_acm *acm)
spin_lock(acm-lock);
if (acm-notify_req) {
- DBG(cdev, acm ttyGS%d serial state %04x\n,
- acm-port_num, acm-serial_state);
+ DBG(cdev, acm %s%d serial state %04x\n,
+ PREFIX, acm-port_num, acm-serial_state);
status = acm_cdc_notify(acm, USB_CDC_NOTIFY_SERIAL_STATE,
0, acm-serial_state,
RE: [PATCH v2] USB: ehci-pci: USB host controller support for Intel Quark X1000
The EHCI packet buffer in/out threshold is programmable for Intel
Quark X1000 USB host controller, and the default value is 0x20 dwords.
The in/out threshold can be programmed to 0x80 dwords, but only when
isochronous/interrupt transactions are not initiated by the USB host
controller. This patch is to reconfigure the packet buffer in/out
threshold as maximal as possible, and it is 0x7F dwords since the USB host
controller initiates isochronous/interrupt transactions.
This is still incomplete. _Why_ do you want to increase the threshold?
Does it fix a problem? Does it improve performance?
Try to set threshold as maximal as possible to maximize the performance. I will
update the descriptions.
Also, the lines are too long. They should be wrapped before 80 columns.
Got you.
+#define PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC0x0939
+static inline bool usb_is_intel_quark_x1000(struct pci_dev *pdev) {
+ return pdev-vendor == PCI_VENDOR_ID_INTEL
+ pdev-device == PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC;
+
+}
The usb_ prefix in the name isn't needed, because this is a static routine.
OK, I will remove the 'usb_' prefix.
+static void usb_set_qrk_bulk_thresh(struct pci_dev *pdev) {
+ void __iomem *base, *op_reg_base;
+ u8 cap_length;
+ u32 val;
+ u16 cmd;
+
+ if (!pci_resource_start(pdev, 0))
+ return;
+
+ if (pci_read_config_word(pdev, PCI_COMMAND, cmd)
+ || !(cmd PCI_COMMAND_MEMORY))
+ return;
+
+ base = pci_ioremap_bar(pdev, 0);
+ if (base == NULL)
+ return;
+
+ cap_length = readb(base);
+ op_reg_base = base + cap_length;
+
+ val = EHCI_INSNREG01_THRESH;
+ writel(val, op_reg_base + EHCI_INSNREG01);
+
+ iounmap(base);
+}
This is much more complicted that it needs to be. When this routine runs, the
controller has already been memory-mapped. All you need to do is:
ehci_writel(ehci, EHCI_INSNREG01_THRESH,
ehci-regs-insnreg01_thresh);
Since it's only one statement, you don't even need to put this in a separate
function. It can go directly into ehci_pci_reinit().
OK, I will remove ' usb_set_qrk_bulk_thresh', and change the code as the
suggestions.
Also, in include/linux/usb/ehci_defs.h, you'll have to add:
#define insnreg01_thresh hostpc[0]
I will add it in ehci-pci.c instead of /linux/usb/ehci_defs.h, because it is
not a generic micro, but just used for Intel Quark X1000.
with an explanatory comment, near the definition of the HOSTPC register.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCHv2] usb: gadget: serial: replace hardcoded ttyGS with PREFIX
From: Of Richard Leitner Replaces all hardcoded ttyGS strings with the PREFIX macro. Due to the fact the strings are spread over different source files the PREFIX definition is moved to u_serial.h Lots of changes like: - DBG(cdev, acm ttyGS%d completion, err %d\n, - acm-port_num, req-status); + DBG(cdev, acm %s%d completion, err %d\n, + PREFIX, acm-port_num, req-status); I'm not sure this improves the code. Do you see a need to ever change PREFIX? Even if it is a reasonable change, the name PREFIX is hopeless. It would have to be something like ACM_TTYNAME_PREFIX. David -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] usb: gadget: pxa27x_udc device-tree documentation
On Sun, Jun 29, 2014 at 10:29:49AM +0100, Robert Jarzmik wrote: Mark Rutland mark.rutl...@arm.com writes: On Wed, Jun 25, 2014 at 08:54:01PM +0100, Robert Jarzmik wrote: The name of the clock input doesn't make sense. I don't understand. With [1] does it make any more sense ? If not you'll have to expand a bit more the doesn't make sense. My concern is that clock-names is supposed to describe the name of the input clock line from the view of the IP block. pxa27x-udc doesn't sound like the name of a clock input line from the view of the UDC block. I assume the clock input line you care about has a more specific name than pxa27x-udc? Not as far as I know. The technical reference manual call it udc clock, so it's even less specific ... Not from the point of view of the device. The clock-names are namespaced to the particular binding, so they're equally specific. Given the above I'd recommend naming the clock udc or udc_clk. That doesn't pretend to be overly specific, and matches the TRM. Marvell engineers have probably the internal schematics and the name of the clock, but outsiders like me only have udc ... Sure, not having the full specs is always a pain. Mark. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] usb: gadget: serial: replace hardcoded ttyGS with PREFIX
Hi, On Mon, 30 Jun 2014 08:41:18 + David Laight david.lai...@aculab.com wrote: From: Of Richard Leitner Replaces all hardcoded ttyGS strings with the PREFIX macro. Due to the fact the strings are spread over different source files the PREFIX definition is moved to u_serial.h Lots of changes like: - DBG(cdev, acm ttyGS%d completion, err %d\n, - acm-port_num, req-status); + DBG(cdev, acm %s%d completion, err %d\n, + PREFIX, acm-port_num, req-status); I'm not sure this improves the code. Maybe you're right, the readability of the code wouldn't be better afterwards, but as I already mentioned IMHO if there is such a macro it should be used everywhere or nowhere... Do you see a need to ever change PREFIX? To be honest: I don't know who would ever need this (me probably not). So the other idea is to carve it out completely? Even if it is a reasonable change, the name PREFIX is hopeless. It would have to be something like ACM_TTYNAME_PREFIX. good point. David regards, richard -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] usb: dwc3: Kconfig: Fix dependency for Exynos DWC3
Make the config depend on ARCH_EXYNOS5 instead of ARCH_EXYNOS as this IP is available only on Exynos5 platforms. Signed-off-by: Sachin Kamat sachin.ka...@samsung.com --- drivers/usb/dwc3/Kconfig |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig index 261c3b428220..24a4f8ed9fd1 100644 --- a/drivers/usb/dwc3/Kconfig +++ b/drivers/usb/dwc3/Kconfig @@ -55,7 +55,7 @@ config USB_DWC3_OMAP config USB_DWC3_EXYNOS tristate Samsung Exynos Platform - depends on ARCH_EXYNOS || COMPILE_TEST + depends on ARCH_EXYNOS5 || COMPILE_TEST default USB_DWC3 help Recent Exynos5 SoCs ship with one DesignWare Core USB3 IP inside, -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCHv2] usb: gadget: serial: replace hardcoded ttyGS with PREFIX
From: Richard Leitner On Mon, 30 Jun 2014 08:41:18 + David Laight david.lai...@aculab.com wrote: From: Of Richard Leitner Replaces all hardcoded ttyGS strings with the PREFIX macro. Due to the fact the strings are spread over different source files the PREFIX definition is moved to u_serial.h Lots of changes like: - DBG(cdev, acm ttyGS%d completion, err %d\n, - acm-port_num, req-status); + DBG(cdev, acm %s%d completion, err %d\n, + PREFIX, acm-port_num, req-status); I'm not sure this improves the code. Maybe you're right, the readability of the code wouldn't be better afterwards, ... Indeed it will make most attempts to grep for the error message fail. David -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] USB: ehci-pci: USB host controller support for Intel Quark X1000
From: Bryan O'Donoghue bryan.odonog...@intel.com
The EHCI packet buffer in/out threshold is programmable for Intel Quark X1000
USB host controller, and the default value is 0x20 dwords. The in/out threshold
can be programmed to 0x80 dwords (512 Bytes) to maximize the perfomrance,
but only when isochronous/interrupt transactions are not initiated by the USB
host controller. This patch is to reconfigure the packet buffer in/out
threshold as maximal as possible to maximize the performance, and 0x7F dwords
(508 Bytes) should be used because the USB host controller initiates
isochronous/interrupt transactions.
Signed-off-by: Bryan O'Donoghue bryan.odonog...@intel.com
Signed-off-by: Alvin (Weike) Chen alvin.c...@intel.com
---
changelog v3:
*Update the description to explain why set in/out threshold
as maximal as possible.
*Improve the threshold value micros more readable.
*Remove 'usb_set_qrk_bulk_thresh'.
*Set threshold value by 'ehci_writel' instead of 'usb_set_qrk_bulk_thresh'.
*Change the function name from 'usb_is_intel_quark_x1000'
to 'is_intel_quark_x1000'.
drivers/usb/host/ehci-pci.c | 40
1 file changed, 40 insertions(+)
diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
index 3e86bf4..78f1622 100644
--- a/drivers/usb/host/ehci-pci.c
+++ b/drivers/usb/host/ehci-pci.c
@@ -35,11 +35,35 @@ static const char hcd_name[] = ehci-pci;
#define PCI_DEVICE_ID_INTEL_CE4100_USB 0x2e70
/*-*/
+#define PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC0x0939
+static inline bool is_intel_quark_x1000(struct pci_dev *pdev)
+{
+ return pdev-vendor == PCI_VENDOR_ID_INTEL
+ pdev-device == PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC;
+}
+
+/*
+ * The offset of in/out threshold register is 0x84.
+ * And it is the register of 'hostpc'
+ * in memory-mapped EHCI controller.
+*/
+#defineintel_quark_x1000_insnreg01 hostpc
+
+/* The maximal ehci packet buffer size is 512 bytes */
+#define INTEL_QUARK_X1000_EHCI_MAX_PACKET_BUFFER_SIZE 512
+
+/* The threshold value set the register is in DWORD */
+#define INTEL_QUARK_X1000_EHCI_THRESHOLD(size) ((size)/4u)
+#define INTEL_QUARK_X1000_EHCI_THRESHOLD_OUT_SHIFT 16
+#define INTEL_QUARK_X1000_EHCI_THRESHOLD_IN_SHIFT 0
+
/* called after powerup, by probe or system-pm wakeup */
static int ehci_pci_reinit(struct ehci_hcd *ehci, struct pci_dev *pdev)
{
int retval;
+ u32 val;
+ u32 thr;
/* we expect static quirk code to handle the extended capabilities
* (currently just BIOS handoff) allowed starting with EHCI 0.96
@@ -50,6 +74,22 @@ static int ehci_pci_reinit(struct ehci_hcd *ehci, struct
pci_dev *pdev)
if (!retval)
ehci_dbg(ehci, MWI active\n);
+ /* Reset the threshold limit */
+ if (is_intel_quark_x1000(pdev)) {
+ /*
+ * In order to support the isochronous/interrupt
+ * transactions, 508 bytes should be used as
+ * max threshold values to maximize the
+ * performance
+ */
+ thr = INTEL_QUARK_X1000_EHCI_THRESHOLD(
+ INTEL_QUARK_X1000_EHCI_MAX_PACKET_BUFFER_SIZE - 4
+ );
+ val = thrINTEL_QUARK_X1000_EHCI_THRESHOLD_OUT_SHIFT |
+ thrINTEL_QUARK_X1000_EHCI_THRESHOLD_IN_SHIFT;
+ ehci_writel(ehci, val, ehci-regs-intel_quark_x1000_insnreg01);
+ }
+
return 0;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] USB: ehci-pci: Add support for Intel Quark X1000 USB
From: Alvin (Weike) Chen alvin.c...@intel.com Hi, Intel Quark X1000 consists of one USB host controller which can be PCI enumerated. And the exsiting EHCI-PCI framework supports it with the default packet buffer in/out threshold. We reconfigure the in/out threshold as maximal as possible to maximize the performance. Bryan O'Donoghue (1): USB: ehci-pci: USB host controller support for Intel Quark X1000 drivers/usb/host/ehci-pci.c | 40 +++ 1 file changed, 40 insertions(+) -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 3/3] usb: phy: msm: Do not do runtime pm if the phy is not idle
Hi Felipe, On 27/06/14 16:54, Felipe Balbi wrote: Hi, On Wed, Jun 18, 2014 at 06:01:08PM +0100, Srinivas Kandagatla wrote: Use case is when the phy is configured in host mode and a usb device is attached to board before bootup. On bootup, with the existing code and runtime pm enabled, the driver would decrement the pm usage count without checking the current state of the phy. This pm usage count decrement would trigger the runtime pm which than would abort the usb enumeration which was in progress. In my case a usb stick gets detected and then immediatly the driver goes to low power mode which is not correct. log: [1.631412] msm_hsusb_host 1252.usb: EHCI Host Controller [1.636556] msm_hsusb_host 1252.usb: new USB bus registered, assigned bus number 1 [1.642563] msm_hsusb_host 1252.usb: irq 220, io mem 0x1252 [1.658197] msm_hsusb_host 1252.usb: USB 2.0 started, EHCI 1.00 [1.659473] hub 1-0:1.0: USB hub found [1.663415] hub 1-0:1.0: 1 port detected ... [1.973352] usb 1-1: new high-speed USB device number 2 using msm_hsusb_host [2.107707] usb-storage 1-1:1.0: USB Mass Storage device detected [2.108993] scsi0 : usb-storage 1-1:1.0 [2.678341] msm_otg 1252.phy: USB in low power mode [3.168977] usb 1-1: USB disconnect, device number 2 This issue was detected on IFC6410 board. This patch fixes the intial runtime pm trigger by checking the phy state and decrementing the pm use count only when the phy state is IDLE. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org --- drivers/usb/phy/phy-msm-usb.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c index 3bb559d..78cc870 100644 --- a/drivers/usb/phy/phy-msm-usb.c +++ b/drivers/usb/phy/phy-msm-usb.c @@ -1229,7 +1229,9 @@ static void msm_otg_sm_work(struct work_struct *w) motg-chg_state = USB_CHG_STATE_UNDEFINED; motg-chg_type = USB_INVALID_CHARGER; } - pm_runtime_put_sync(otg-phy-dev); + + if (otg-phy-state == OTG_STATE_B_IDLE) + pm_runtime_put_sync(otg-phy-dev); instead, you might as well just use autosuspend. autosuspend is a good idea and will provide a delay before rumtime suspend, however the bug which is fixed here still needs to be fixed anyway. runtime PM in this driver is not that great, the driver just increments the pm use count if the phy is configured in host or deivce mode. This patch fixes a bug in its state-machine which decrements pm use-count without checking the state. As this is a PHY driver, am not sure moving to autosuspend means we would end up just adding some static inactivity delay? Is it really going to add any value to this PHY driver? --srini -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v3] HID: leds: move led_mode attribute to led-class devices in MSI GT683R driver
On Wed, Jun 25, 2014 at 09:13:09PM +0300, Janne Kanniainen wrote: Move led_mode attribute from HID device to led-class devices and rename it msi_mode. This will also fix race condition by using attribute-groups. Signed-off-by: Janne Kanniainen janne.kanniai...@gmail.com --- Changes in v3: - Style fixes - Rename sysfs-class-hid-driver-gt683r to sysfs-class-leds-driver-gt683r .../ABI/testing/sysfs-class-hid-driver-gt683r | 14 --- .../ABI/testing/sysfs-class-leds-driver-gt683r | 16 + drivers/hid/hid-gt683r.c | 28 -- 3 files changed, 32 insertions(+), 26 deletions(-) delete mode 100644 Documentation/ABI/testing/sysfs-class-hid-driver-gt683r create mode 100644 Documentation/ABI/testing/sysfs-class-leds-driver-gt683r diff --git a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r deleted file mode 100644 index 317e9d5..000 --- a/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r +++ /dev/null @@ -1,14 +0,0 @@ -What:/sys/class/hidraw/hidraw/device/leds_mode -Date:Jun 2014 -KernelVersion: 3.17 -Contact: Janne Kanniainen janne.kanniai...@gmail.com -Description: - Set the mode of LEDs - - 0 - normal - 1 - audio - 2 - breathing - - Normal: LEDs are fully on when enabled - Audio: LEDs brightness depends on sound level - Breathing: LEDs brightness varies at human breathing rate \ No newline at end of file diff --git a/Documentation/ABI/testing/sysfs-class-leds-driver-gt683r b/Documentation/ABI/testing/sysfs-class-leds-driver-gt683r new file mode 100644 index 000..29769fb --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-leds-driver-gt683r @@ -0,0 +1,16 @@ +What:/sys/class/leds/led/msi_mode The ABI-file name now sort of matches the attribute path and there are examples of attributes being documented in this particular way, but naming is far from consistent in Documentation/ABI. Perhaps we should use the name field of the attribute group and kill two birds with one stone by making the sysfs file name match the attribute path, while also making it even more obvious that the mode attribute is a driver specific attribute (and not a common led class one) by placing it in a subdirectory. That is, if you set the .name field to gt683r (and rename the attribute and ABI-file again) then the file name and attribute path could match: Documentation/ABI/testing/sysfs-class-leds-gt683r What: /sys/class/leds/led/gt683r/mode Both patches look good otherwise. Johan -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v2] HID: leds: Use attribute-groups in MSI GT683R driver
On Wed, Jun 25, 2014 at 03:55:10PM -0700, Bryan Wu wrote: On Wed, Jun 25, 2014 at 12:09 PM, Jiri Kosina jkos...@suse.cz wrote: On Wed, 25 Jun 2014, Johan Hovold wrote: Did you see the attribute-race series I posted? Not sure how best to handle the dependency, as those patches should probably go in through the LEDs tree, while the first patch in that series (adding the groups field) is a dependency for this patch. Jiri, how would this best be solved? I think the best course of action here is to gather Acks from the respective maintainers, and take the whole lot trough a single tree (probably the leds tree in this case) to avoid unnecessary intra-tree dependencies in a rather straighforward situation like this. I think the better place is HID/input tree, since this patch depends on the initial one which is not in my tree. I'm going to merge Johan's whole patchset and this patch probably depends Johan's work too. Dmitry has ACKed the input-patch and Bryan has applied that one and the leds-patches to his tree (of which the first one is a dependency of this patch). Jiri, are you saying that the gt683r-driver should go in through his tree as well, that is all three patches including the first that you have already applied? I just assumed your for-next branch was immutable, but perhaps I was mistaken. Thanks, Johan -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] phy: omap-usb2: Add PHY regulator to DT binding documentation
The PHY driver can manage a regulator that supplies power to the USB2 PHY. Add DT binding information for that. Signed-off-by: Roger Quadros rog...@ti.com --- Documentation/devicetree/bindings/phy/ti-phy.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt b/Documentation/devicetree/bindings/phy/ti-phy.txt index 9ce458f..a6acd99 100644 --- a/Documentation/devicetree/bindings/phy/ti-phy.txt +++ b/Documentation/devicetree/bindings/phy/ti-phy.txt @@ -41,6 +41,7 @@ Required properties: Optional properties: - ctrl-module : phandle of the control module used by PHY driver to power on the PHY. + - phy-supply: phandle of the regulator that supplies power to the PHY. This is usually a subnode of ocp2scp to which it is connected. -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] ARM: dts: dra7-evm: Add regulator information to USB2 PHYs
The ldousb_reg regulator provides power to the USB1 and USB2
High Speed PHYs.
Signed-off-by: Roger Quadros rog...@ti.com
---
arch/arm/boot/dts/dra7-evm.dts | 8
1 file changed, 8 insertions(+)
diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
index 4adc280..fd96ced 100644
--- a/arch/arm/boot/dts/dra7-evm.dts
+++ b/arch/arm/boot/dts/dra7-evm.dts
@@ -495,3 +495,11 @@
};
};
};
+
+usb2_phy1 {
+ phy-supply = ldousb_reg;
+};
+
+usb2_phy2 {
+ phy-supply = ldousb_reg;
+};
--
1.8.3.2
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] PHY: ti-pipe3: Manage 3.3V PHY regulator
Hi, On some SoCs e.g. J6 the 3.3V supply to the USB2 PHY can be powered down when the PHY is not in use. Add regulator management code to control this power line. cheers, -roger --- Roger Quadros (3): phy: omap-usb2: Manage PHY 3.3V supply regulator phy: omap-usb2: Add PHY regulator to DT binding documentation ARM: dts: dra7-evm: Add regulator information to USB2 PHYs Documentation/devicetree/bindings/phy/ti-phy.txt | 1 + arch/arm/boot/dts/dra7-evm.dts | 8 drivers/phy/phy-omap-usb2.c | 25 include/linux/phy/omap_usb.h | 1 + 4 files changed, 35 insertions(+) -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] phy: omap-usb2: Manage PHY 3.3V supply regulator
On some SoCs e.g. J6 the 3.3V supply to the USB2 PHY can be
powered down when the PHY is not in use. Add regulator
management code to control this power line.
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/phy/phy-omap-usb2.c | 25 +
include/linux/phy/omap_usb.h | 1 +
2 files changed, 26 insertions(+)
diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
index 7007c11..2afc79c 100644
--- a/drivers/phy/phy-omap-usb2.c
+++ b/drivers/phy/phy-omap-usb2.c
@@ -30,6 +30,7 @@
#include linux/phy/omap_control_phy.h
#include linux/phy/phy.h
#include linux/of_platform.h
+#include linux/regulator/consumer.h
#define USB2PHY_DISCON_BYP_LATCH (1 31)
#define USB2PHY_ANA_CONFIG1 0x4c
@@ -107,6 +108,14 @@ static int omap_usb_power_off(struct phy *x)
omap_control_phy_power(phy-control_dev, 0);
+ if (phy-pwr) {
+ int ret;
+
+ ret = regulator_disable(phy-pwr);
+ if (ret)
+ return ret;
+ }
+
return 0;
}
@@ -114,6 +123,14 @@ static int omap_usb_power_on(struct phy *x)
{
struct omap_usb *phy = phy_get_drvdata(x);
+ if (phy-pwr) {
+ int ret;
+
+ ret = regulator_enable(phy-pwr);
+ if (ret)
+ return ret;
+ }
+
omap_control_phy_power(phy-control_dev, 1);
return 0;
@@ -253,6 +270,14 @@ static int omap_usb2_probe(struct platform_device *pdev)
phy-control_dev = control_pdev-dev;
omap_control_phy_power(phy-control_dev, 0);
+ /* phy-supply */
+ phy-pwr = devm_regulator_get_optional(phy-dev, phy);
+ if (IS_ERR(phy-pwr)) {
+ if (PTR_ERR(phy-pwr) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ phy-pwr = NULL;
+ }
+
otg-set_host = omap_usb_set_host;
otg-set_peripheral = omap_usb_set_peripheral;
if (phy_data-flags OMAP_USB2_HAS_SET_VBUS)
diff --git a/include/linux/phy/omap_usb.h b/include/linux/phy/omap_usb.h
index dc2c541..e2c46df 100644
--- a/include/linux/phy/omap_usb.h
+++ b/include/linux/phy/omap_usb.h
@@ -40,6 +40,7 @@ struct omap_usb {
struct clk *wkupclk;
struct clk *optclk;
u8 flags;
+ struct regulator*pwr;
};
struct usb_phy_data {
--
1.8.3.2
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] BUGFIX: usb: musb: dsps: fix the base address for accessing the mode register
The first patch cleans up the coding style of the register accessor functions which made my eyes sore. The second patch makes the function added in commit 943c13971c08 usb: musb: dsps: implement -set_mode() actually functional by using the correct base address for the register accesses. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] usb: musb: dsps: fix the base address for accessing the mode register
commit 943c13971c08 usb: musb: dsps: implement -set_mode()
should have made it possible to use the driver with boards that have
the USBID pin unconnected. This doesn't actually work, since the
driver uses the wrong base address to access the mode register.
Furthermore it uses different base addresses in different places to
access the same register (phy_utmi).
Signed-off-by: Lothar Waßmann l...@karo-electronics.de
---
drivers/usb/musb/musb_dsps.c |9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 56c5d3f..1bb34af 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -502,10 +502,9 @@ static int dsps_musb_set_mode(struct musb *musb, u8 mode)
struct dsps_glue *glue = dev_get_drvdata(dev-parent);
const struct dsps_musb_wrapper *wrp = glue-wrp;
void __iomem *ctrl_base = musb-ctrl_base;
- void __iomem *base = musb-mregs;
u32 reg;
- reg = dsps_readl(base, wrp-mode);
+ reg = dsps_readl(ctrl_base, wrp-mode);
switch (mode) {
case MUSB_HOST:
@@ -518,7 +517,7 @@ static int dsps_musb_set_mode(struct musb *musb, u8 mode)
*/
reg |= (1 wrp-iddig_mux);
- dsps_writel(base, wrp-mode, reg);
+ dsps_writel(ctrl_base, wrp-mode, reg);
dsps_writel(ctrl_base, wrp-phy_utmi, 0x02);
break;
case MUSB_PERIPHERAL:
@@ -531,10 +530,10 @@ static int dsps_musb_set_mode(struct musb *musb, u8 mode)
*/
reg |= (1 wrp-iddig_mux);
- dsps_writel(base, wrp-mode, reg);
+ dsps_writel(ctrl_base, wrp-mode, reg);
break;
case MUSB_OTG:
- dsps_writel(base, wrp-phy_utmi, 0x02);
+ dsps_writel(ctrl_base, wrp-phy_utmi, 0x02);
break;
default:
dev_err(glue-dev, unsupported mode %d\n, mode);
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] usb: musb: dsps: coding style cleanup
There is no reason for the register accessor functions not to adhere
to the CodingStyle rules.
Signed-off-by: Lothar Waßmann l...@karo-electronics.de
---
drivers/usb/musb/musb_dsps.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 51beb13..56c5d3f 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -56,16 +56,24 @@ static const struct of_device_id musb_dsps_of_match[];
* dependent on musb core layer symbols.
*/
static inline u8 dsps_readb(const void __iomem *addr, unsigned offset)
- { return __raw_readb(addr + offset); }
+{
+ return __raw_readb(addr + offset);
+}
static inline u32 dsps_readl(const void __iomem *addr, unsigned offset)
- { return __raw_readl(addr + offset); }
+{
+ return __raw_readl(addr + offset);
+}
static inline void dsps_writeb(void __iomem *addr, unsigned offset, u8 data)
- { __raw_writeb(data, addr + offset); }
+{
+ __raw_writeb(data, addr + offset);
+}
static inline void dsps_writel(void __iomem *addr, unsigned offset, u32 data)
- { __raw_writel(data, addr + offset); }
+{
+ __raw_writel(data, addr + offset);
+}
/**
* DSPS musb wrapper register offset.
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] usb: gadget: serial: replace hardcoded ttyGS with PREFIX
Hi, On Mon, 30 Jun 2014 09:08:01 + David Laight david.lai...@aculab.com wrote: From: Of Richard Leitner Replaces all hardcoded ttyGS strings with the PREFIX macro. Due to the fact the strings are spread over different source files the PREFIX definition is moved to u_serial.h Lots of changes like: - DBG(cdev, acm ttyGS%d completion, err %d\n, - acm-port_num, req-status); + DBG(cdev, acm %s%d completion, err %d\n, + PREFIX, acm-port_num, req-status); I'm not sure this improves the code. Maybe you're right, the readability of the code wouldn't be better afterwards, but as I already mentioned IMHO if there is such a macro it should be used everywhere or nowhere... Indeed it will make most attempts to grep for the error message fail. So your thought is to carve it out completely? Are there any other opinions/ideas on that? regards, richard -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v2] HID: leds: Use attribute-groups in MSI GT683R driver
On Mon, 30 Jun 2014, Johan Hovold wrote: I think the better place is HID/input tree, since this patch depends on the initial one which is not in my tree. I'm going to merge Johan's whole patchset and this patch probably depends Johan's work too. Dmitry has ACKed the input-patch and Bryan has applied that one and the leds-patches to his tree (of which the first one is a dependency of this patch). Jiri, are you saying that the gt683r-driver should go in through his tree as well, that is all three patches including the first that you have already applied? I just assumed your for-next branch was immutable, but perhaps I was mistaken. Well, for-next branch is a collection of all the topic branches I am queuing for the following merge window. I am never really rebasing it, but I can definitely not include 'for-3.17/hid-gt683r' topic branch in the pile I will be sending to Linus (all the scheduled branches are getting merged into 'for-linus' only when merge window open). So the only potential conflict between hid.git and Bryan's tree would be in linux-next (and probably there will be none, git can handle duplicate patches nicely). So once Bryan confirms he's queued it (please preserve my Signoff from my tree), then I will just not include for-3.17/hid-gt683r branch in pull request to Linus and all is fine. Thanks, -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] usb: chipidea: udc: delete td from req's td list at ep_dequeue
On Mon, Jun 30, 2014 at 12:15:03PM +0800, Peter Chen wrote:
We need to delete un-finished td from current request's td list
at ep_dequeue API, otherwise, this non-user td will be remained
at td list before this request is freed. So if we do ep_queue-
ep_dequeue-ep_queue sequence, when the complete interrupt for
the second ep_queue comes, we search td list for this request,
the first td (added by the first ep_queue) will be handled, and
its status is still active, so we will consider the this transfer
still not be completed, but in fact, it has completed. It causes
the peripheral side considers it never receives current data for
this transfer.
We met this problem when do Error Recovery Test - Device Configured
test item for USBCV2 MSC test, the host has never received ACK for
the IN token for CSW due to peripheral considers it does not get this
CBW, the USBCV test log like belows:
--
INFO
Issuing BOT MSC Reset, reset should always succeed
INFO
Retrieving status on CBW endpoint
INFO
CBW endpoint status = 0x0
INFO
Retrieving status on CSW endpoint
INFO
CSW endpoint status = 0x0
INFO
Issuing required command (Test Unit Ready) to verify device has recovered
INFO
Issuing CBW (attempt #1):
INFO
|- CBW LUN = 0x0
INFO
|- CBW Flags= 0x0
INFO
|- CBW Data Transfer Length = 0x0
INFO
|- CBW CDB Length = 0x6
INFO
|- CBW CDB-00 = 0x0
INFO
|- CBW CDB-01 = 0x0
INFO
|- CBW CDB-02 = 0x0
INFO
|- CBW CDB-03 = 0x0
INFO
|- CBW CDB-04 = 0x0
INFO
|- CBW CDB-05 = 0x0
INFO
Issuing CSW : try 1
INFO
CSW Bulk Request timed out!
ERROR
Failed CSW phase : should have been success or stall
FAIL
(5.3.4) The CSW status value must be 0x00, 0x01, or 0x02.
ERROR
BOTCommonMSCRequest failed: error=80004000
Cc: sta...@vger.kernel.org
Signed-off-by: Peter Chen peter.c...@freescale.com
---
drivers/usb/chipidea/udc.c |8
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 69425b3..0522e59 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1321,6 +1321,7 @@ static int ep_dequeue(struct usb_ep *ep, struct
usb_request *req)
struct ci_hw_ep *hwep = container_of(ep, struct ci_hw_ep, ep);
struct ci_hw_req *hwreq = container_of(req, struct ci_hw_req, req);
unsigned long flags;
+ struct td_node *node, *tmpnode;
if (ep == NULL || req == NULL || hwreq-req.status != -EALREADY ||
hwep-ep.desc == NULL || list_empty(hwreq-queue) ||
@@ -1331,6 +1332,13 @@ static int ep_dequeue(struct usb_ep *ep, struct
usb_request *req)
hw_ep_flush(hwep-ci, hwep-num, hwep-dir);
+ list_for_each_entry_safe(node, tmpnode, hwreq-tds, td) {
+ dma_pool_free(hwep-td_pool, node-ptr, node-dma);
+ list_del_init(node-td);
+ node-ptr = NULL;
+ kfree(node);
+ }
+
Can't we reuse ep_free_request after the list_del_init(hwreq-queue); ?
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] usb: chipidea: udc: delete td from req's td list at ep_dequeue
On Mon, Jun 30, 2014 at 02:19:04PM +0200, Michael Grzeschik wrote:
On Mon, Jun 30, 2014 at 12:15:03PM +0800, Peter Chen wrote:
We need to delete un-finished td from current request's td list
at ep_dequeue API, otherwise, this non-user td will be remained
at td list before this request is freed. So if we do ep_queue-
ep_dequeue-ep_queue sequence, when the complete interrupt for
the second ep_queue comes, we search td list for this request,
the first td (added by the first ep_queue) will be handled, and
its status is still active, so we will consider the this transfer
still not be completed, but in fact, it has completed. It causes
the peripheral side considers it never receives current data for
this transfer.
We met this problem when do Error Recovery Test - Device Configured
test item for USBCV2 MSC test, the host has never received ACK for
the IN token for CSW due to peripheral considers it does not get this
CBW, the USBCV test log like belows:
--
INFO
Issuing BOT MSC Reset, reset should always succeed
INFO
Retrieving status on CBW endpoint
INFO
CBW endpoint status = 0x0
INFO
Retrieving status on CSW endpoint
INFO
CSW endpoint status = 0x0
INFO
Issuing required command (Test Unit Ready) to verify device has recovered
INFO
Issuing CBW (attempt #1):
INFO
|- CBW LUN = 0x0
INFO
|- CBW Flags= 0x0
INFO
|- CBW Data Transfer Length = 0x0
INFO
|- CBW CDB Length = 0x6
INFO
|- CBW CDB-00 = 0x0
INFO
|- CBW CDB-01 = 0x0
INFO
|- CBW CDB-02 = 0x0
INFO
|- CBW CDB-03 = 0x0
INFO
|- CBW CDB-04 = 0x0
INFO
|- CBW CDB-05 = 0x0
INFO
Issuing CSW : try 1
INFO
CSW Bulk Request timed out!
ERROR
Failed CSW phase : should have been success or stall
FAIL
(5.3.4) The CSW status value must be 0x00, 0x01, or 0x02.
ERROR
BOTCommonMSCRequest failed: error=80004000
Cc: sta...@vger.kernel.org
Signed-off-by: Peter Chen peter.c...@freescale.com
---
drivers/usb/chipidea/udc.c |8
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 69425b3..0522e59 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1321,6 +1321,7 @@ static int ep_dequeue(struct usb_ep *ep, struct
usb_request *req)
struct ci_hw_ep *hwep = container_of(ep, struct ci_hw_ep, ep);
struct ci_hw_req *hwreq = container_of(req, struct ci_hw_req, req);
unsigned long flags;
+ struct td_node *node, *tmpnode;
if (ep == NULL || req == NULL || hwreq-req.status != -EALREADY ||
hwep-ep.desc == NULL || list_empty(hwreq-queue) ||
@@ -1331,6 +1332,13 @@ static int ep_dequeue(struct usb_ep *ep, struct
usb_request *req)
hw_ep_flush(hwep-ci, hwep-num, hwep-dir);
+ list_for_each_entry_safe(node, tmpnode, hwreq-tds, td) {
+ dma_pool_free(hwep-td_pool, node-ptr, node-dma);
+ list_del_init(node-td);
+ node-ptr = NULL;
+ kfree(node);
+ }
+
Can't we reuse ep_free_request after the list_del_init(hwreq-queue); ?
No, we can't, we may still use this request later, we can't free it.
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] usb: chipidea: udc: delete td from req's td list at ep_dequeue
On Mon, Jun 30, 2014 at 09:11:11AM +0200, Andrzej Pietrasiewicz wrote:
Hello Peter,
W dniu 30.06.2014 06:15, Peter Chen pisze:
We need to delete un-finished td from current request's td list
at ep_dequeue API, otherwise, this non-user td will be remained
at td list before this request is freed. So if we do ep_queue-
ep_dequeue-ep_queue sequence, when the complete interrupt for
the second ep_queue comes, we search td list for this request,
the first td (added by the first ep_queue) will be handled, and
its status is still active, so we will consider the this transfer
still not be completed, but in fact, it has completed. It causes
the peripheral side considers it never receives current data for
this transfer.
snip
+struct td_node *node, *tmpnode;
if (ep == NULL || req == NULL || hwreq-req.status != -EALREADY ||
hwep-ep.desc == NULL || list_empty(hwreq-queue) ||
@@ -1331,6 +1332,13 @@ static int ep_dequeue(struct usb_ep *ep, struct
usb_request *req)
hw_ep_flush(hwep-ci, hwep-num, hwep-dir);
+list_for_each_entry_safe(node, tmpnode, hwreq-tds, td) {
+dma_pool_free(hwep-td_pool, node-ptr, node-dma);
+list_del_init(node-td);
+node-ptr = NULL;
Why bother initializing node-td's prev and next members and assigning NULL
to ptrif...
+kfree(node);
... in the very next line the whole structure is freed?
I would suggest
+ list_for_each_entry_safe(node, tmpnode, hwreq-tds, td) {
+ dma_pool_free(hwep-td_pool, node-ptr, node-dma);
+ list_del(node-td);
+ kfree(node);
+ }
Thanks, AP.
Yes, it seems it is enough, I just copy it from the ep_free_request.
I don't know why Michael did it before, Michael, can you recall it?
Besides, I see some code use list_del_init, why we need to re-init
it after we delete it from the list.
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] USB: ehci-pci: USB host controller support for Intel Quark X1000
Hello.
On 06/30/2014 09:06 PM, Chen, Alvin wrote:
From: Bryan O'Donoghue bryan.odonog...@intel.com
The EHCI packet buffer in/out threshold is programmable for Intel Quark X1000
USB host controller, and the default value is 0x20 dwords. The in/out threshold
can be programmed to 0x80 dwords (512 Bytes) to maximize the perfomrance,
but only when isochronous/interrupt transactions are not initiated by the USB
host controller. This patch is to reconfigure the packet buffer in/out
threshold as maximal as possible to maximize the performance, and 0x7F dwords
(508 Bytes) should be used because the USB host controller initiates
isochronous/interrupt transactions.
Signed-off-by: Bryan O'Donoghue bryan.odonog...@intel.com
Signed-off-by: Alvin (Weike) Chen alvin.c...@intel.com
---
changelog v3:
*Update the description to explain why set in/out threshold
as maximal as possible.
*Improve the threshold value micros more readable.
*Remove 'usb_set_qrk_bulk_thresh'.
*Set threshold value by 'ehci_writel' instead of 'usb_set_qrk_bulk_thresh'.
*Change the function name from 'usb_is_intel_quark_x1000'
to 'is_intel_quark_x1000'.
drivers/usb/host/ehci-pci.c | 40
1 file changed, 40 insertions(+)
diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
index 3e86bf4..78f1622 100644
--- a/drivers/usb/host/ehci-pci.c
+++ b/drivers/usb/host/ehci-pci.c
@@ -35,11 +35,35 @@ static const char hcd_name[] = ehci-pci;
#define PCI_DEVICE_ID_INTEL_CE4100_USB0x2e70
/*-*/
+#define PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC0x0939
+static inline bool is_intel_quark_x1000(struct pci_dev *pdev)
+{
+ return pdev-vendor == PCI_VENDOR_ID_INTEL
+ pdev-device == PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC;
+}
Why not just put this check inline into ehci_pci_reinit()?
+
+/*
+ * The offset of in/out threshold register is 0x84.
+ * And it is the register of 'hostpc'
+ * in memory-mapped EHCI controller.
+*/
The preferred multi-line kernel style is this:
/*
* bla
* bla
*/
+#defineintel_quark_x1000_insnreg01 hostpc
+
+/* The maximal ehci packet buffer size is 512 bytes */
s/ehci/EHCI/
+#define INTEL_QUARK_X1000_EHCI_MAX_PACKET_BUFFER_SIZE 512
+
+/* The threshold value set the register is in DWORD */
+#define INTEL_QUARK_X1000_EHCI_THRESHOLD(size) ((size)/4u)
+#define INTEL_QUARK_X1000_EHCI_THRESHOLD_OUT_SHIFT 16
+#define INTEL_QUARK_X1000_EHCI_THRESHOLD_IN_SHIFT 0
+
Too many empty lines...
/* called after powerup, by probe or system-pm wakeup */
static int ehci_pci_reinit(struct ehci_hcd *ehci, struct pci_dev *pdev)
{
int retval;
+ u32 val;
+ u32 thr;
Why not declare these where they are used?
/* we expect static quirk code to handle the extended capabilities
* (currently just BIOS handoff) allowed starting with EHCI 0.96
@@ -50,6 +74,22 @@ static int ehci_pci_reinit(struct ehci_hcd *ehci, struct
pci_dev *pdev)
if (!retval)
ehci_dbg(ehci, MWI active\n);
+ /* Reset the threshold limit */
+ if (is_intel_quark_x1000(pdev)) {
+ /*
+ * In order to support the isochronous/interrupt
+ * transactions, 508 bytes should be used as
+ * max threshold values to maximize the
+ * performance
+ */
Same comment about the comment style...
+ thr = INTEL_QUARK_X1000_EHCI_THRESHOLD(
+ INTEL_QUARK_X1000_EHCI_MAX_PACKET_BUFFER_SIZE - 4
+ );
+ val = thrINTEL_QUARK_X1000_EHCI_THRESHOLD_OUT_SHIFT |
+ thrINTEL_QUARK_X1000_EHCI_THRESHOLD_IN_SHIFT;
Please surround with spaces for consistency.
+ ehci_writel(ehci, val, ehci-regs-intel_quark_x1000_insnreg01);
+ }
+
return 0;
}
WBR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 07/12] usb: chipidea: add a generic driver
Peter,
On Fri, Jun 27, 2014 at 11:25:07AM +0800, Peter Chen wrote:
On Tue, Jun 24, 2014 at 12:35:16PM +0200, Antoine Ténart wrote:
ifneq ($(CONFIG_OF),)
obj-$(CONFIG_USB_CHIPIDEA) += usbmisc_imx.o ci_hdrc_imx.o
+ obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_generic.o
endif
As a generic driver, you may need to support both dt and non-dt
solution.
Since the dt is now the best practice and since there is no need (yet)
for a non-dt usage of this driver shouldn't we let anyone needing it
implement it when the time comes?
+static int ci_hdrc_generic_probe(struct platform_device *pdev)
+{
+ struct device *dev = pdev-dev;
+ struct ci_hdrc_generic_priv *priv;
+ struct ci_hdrc_platform_data ci_pdata = {
+ .name = ci_hdrc,
How about this using dev_name(pdev-dev) for name?
Yes, why not. I don't have a strong preference.
+
+clk_err:
+ clk_disable_unprepare(priv-clk);
You may need to add if (!IS_ERR(priv-clk))
Right! I'll update this.
+
+static const struct of_device_id ci_hdrc_generic_of_match[] = {
+ { .compatible = chipidea-usb },
+ { }
+};
Even as a generic driver, you can also use your own compatible string.
Well, there is nothing specific about the Berlin CI. Some subsystems
use the 'generic' keyword in these cases. Do you see a particular reason
I should use some Berlin related compatible here?
Thanks for the review!
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 07/12] usb: chipidea: add a generic driver
Hello, On Tue, Jun 24, 2014 at 07:51:01PM +0900, Jingoo Han wrote: On Tuesday, June 24, 2014 7:35 PM, Antoine Tenart wrote: Add a generic ChipIdea driver, with optional PHY and clock, to support ChipIdea controllers that doesn't need specific functions. s/doesn't/don't Needed for the Marvell Berlin SoCs SUB controllers. s/SUB/USB Right, I'll fix these. + +MODULE_DESCRIPTION(Generic ChipIdea HDRC USB binding); +MODULE_AUTHOR(Antoine Ténart antoine.ten...@free-electrons.com); +MODULE_LICENSE(GPL); How about GPL v2? Well, GPL stands for GNU Public License v2 or later as documented in: http://lxr.free-electrons.com/source/include/linux/module.h#L100 Is there a reason I should use GPLv2? Or is this a best practice? Thanks! Antoine -- Antoine Ténart, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [usb-storage] Re: External USB3 disk fails with Invalid field in cdb
On Fri, 27 Jun 2014 21:52:35 +0200 Michael Büsch m...@bues.ch wrote: On Fri, 27 Jun 2014 15:23:42 -0400 (EDT) Alan Stern st...@rowland.harvard.edu wrote: Does the patch below do what you and James want? Yes, this does work. Thanks! You can add my Tested-by: Michael Büsch m...@bues.ch Does somebody pick this up for inclusion? It'd also be good, if this hit stable. -- Michael signature.asc Description: PGP signature
Re: XHCI, brain-dead scanner, and microframe rounding
On 06/30/2014 12:02 AM, Mike Mammarella wrote: recently got a computer [1] with USB3 ports, and plugged my Fujitsu Scansnap S1500 scanner into one of them. It turns out this doesn't work, and after a lot of searching the web and tinkering with it I've discovered that it has something to do with XHCI. If I recompile the kernel without the xhci_hcd driver (merely not loading the module is insufficient, since the ehci_hcd driver knows to defer to xhci_hcd in that case), then the scanner works. It looks like this has been known for some time [2], but nobody's been able to work out exactly what the problem is. (I'm using Debian, but Ubuntu's similar; I see the same microframe rounding messages, for instance.) Following some of the debugging instructions on a thread on sane-devel [3], I found that when using xhci_hcd, SANE thinks my scanner is brain-dead. (The thread is about a slightly different Fujitsu scanner, but I see very similar symptoms.) So, can I convince you to take a look at this with me in more detail? I know very little about the USB(3) stack and the its implementation in Linux, but I am a proficient software engineer and I've written (out of tree) kernel modules before, so I hope that remote debugging this hardware driver issue through my hands won't be so terrib Hi Can you add xhci debugging by enabling CONFIG_DYNAMIC_DEBUG, and run `echo -n 'module xhci_hcd =p' /sys/kernel/debug/dynamic_debug/control` as root, and send me the output of dmesg. Without debugging info it's hard to guess what's going on. The microframe rounding look a bit suspicious: [12864.453456] usb 3-4: ep 0x81 - rounding interval to 128 microframes, ep desc says 255 microframes xhci specs says it needs the interval rounded to nearest 2^(X) value, which would be 256, not 128. I'll take a look at that. An other possibility is that it's related to how xhci handles halted endpoints. I got some untested code to fix this, It needs a lot of cleanup but can be tested. If you are able to test my ep_reset_halt_test branch (with xhci debugging) I'd be interested to know if it helps. Code is at: git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git ep_reset_halt_test -Mathias -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [usb-storage] Re: External USB3 disk fails with Invalid field in cdb
On Fri, 2014-06-27 at 15:23 -0400, Alan Stern wrote: On Fri, 27 Jun 2014, Michael Büsch wrote: On Fri, 27 Jun 2014 14:42:01 -0400 (EDT) Alan Stern st...@rowland.harvard.edu wrote: Michael, can you post the lsusb -v output for this device? I see it is made by JMicron; they are notorious for buggy USB-ATA bridges. Of course. Here you go: Bus 004 Device 009: ID 152d:0567 JMicron Technology Corp. / JMicron USA Technology Corp. Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 3.00 bDeviceClass0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 9 idVendor 0x152d JMicron Technology Corp. / JMicron USA Technology Corp. idProduct 0x0567 bcdDevice1.14 iManufacturer 1 JMicron iProduct2 USB to ATA/ATAPI Bridge iSerial 3 xxx bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 121 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 4 USB Mass Storage bmAttributes 0xc0 Self Powered MaxPower2mA ... MaxPower=2mA is a nice guess for a hard disk. ;) That refers to the amount of power the device draws from the USB bus. Since the disk drive is self-powered, it doesn't use much bus power. Does the patch below do what you and James want? Yes, that's the usual annoying additions to our blacklist. You can add my acked-by and could you cc stable? Thanks, James -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [usb-storage] Re: External USB3 disk fails with Invalid field in cdb
On Mon, 30 Jun 2014, James Bottomley wrote: Does the patch below do what you and James want? Yes, that's the usual annoying additions to our blacklist. You can add my acked-by and could you cc stable? Will do. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] USB: ehci-pci: USB host controller support for Intel Quark X1000
On Mon, 30 Jun 2014, Chen, Alvin wrote:
From: Bryan O'Donoghue bryan.odonog...@intel.com
The EHCI packet buffer in/out threshold is programmable for Intel Quark X1000
USB host controller, and the default value is 0x20 dwords. The in/out
threshold
can be programmed to 0x80 dwords (512 Bytes) to maximize the perfomrance,
but only when isochronous/interrupt transactions are not initiated by the USB
host controller. This patch is to reconfigure the packet buffer in/out
threshold as maximal as possible to maximize the performance, and 0x7F dwords
(508 Bytes) should be used because the USB host controller initiates
isochronous/interrupt transactions.
Signed-off-by: Bryan O'Donoghue bryan.odonog...@intel.com
Signed-off-by: Alvin (Weike) Chen alvin.c...@intel.com
This is getting a lot better.
diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
index 3e86bf4..78f1622 100644
--- a/drivers/usb/host/ehci-pci.c
+++ b/drivers/usb/host/ehci-pci.c
@@ -35,11 +35,35 @@ static const char hcd_name[] = ehci-pci;
#define PCI_DEVICE_ID_INTEL_CE4100_USB 0x2e70
/*-*/
+#define PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC 0x0939
+static inline bool is_intel_quark_x1000(struct pci_dev *pdev)
+{
+ return pdev-vendor == PCI_VENDOR_ID_INTEL
+ pdev-device == PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC;
+}
Whether to put this test directly into ehci_pci_reset() or leave it as
a separate subroutine is up to you. I don't care either way.
+
+/*
+ * The offset of in/out threshold register is 0x84.
+ * And it is the register of 'hostpc'
+ * in memory-mapped EHCI controller.
+*/
0x84 is the same as offset of the hostpc register in the Intel
Moorestown controller. hostpc is not present in general EHCI
controllers.
+#define intel_quark_x1000_insnreg01 hostpc
+
+/* The maximal ehci packet buffer size is 512 bytes */
+#define INTEL_QUARK_X1000_EHCI_MAX_PACKET_BUFFER_SIZE512
+
+/* The threshold value set the register is in DWORD */
+#define INTEL_QUARK_X1000_EHCI_THRESHOLD(size) ((size)/4u)
+#define INTEL_QUARK_X1000_EHCI_THRESHOLD_OUT_SHIFT 16
+#define INTEL_QUARK_X1000_EHCI_THRESHOLD_IN_SHIFT0
+
/* called after powerup, by probe or system-pm wakeup */
static int ehci_pci_reinit(struct ehci_hcd *ehci, struct pci_dev *pdev)
{
int retval;
+ u32 val;
+ u32 thr;
/* we expect static quirk code to handle the extended capabilities
* (currently just BIOS handoff) allowed starting with EHCI 0.96
@@ -50,6 +74,22 @@ static int ehci_pci_reinit(struct ehci_hcd *ehci, struct
pci_dev *pdev)
if (!retval)
ehci_dbg(ehci, MWI active\n);
+ /* Reset the threshold limit */
+ if (is_intel_quark_x1000(pdev)) {
+ /*
+ * In order to support the isochronous/interrupt
+ * transactions, 508 bytes should be used as
+ * max threshold values to maximize the
+ * performance
+ */
+ thr = INTEL_QUARK_X1000_EHCI_THRESHOLD(
+ INTEL_QUARK_X1000_EHCI_MAX_PACKET_BUFFER_SIZE - 4
+ );
+ val = thrINTEL_QUARK_X1000_EHCI_THRESHOLD_OUT_SHIFT |
+ thrINTEL_QUARK_X1000_EHCI_THRESHOLD_IN_SHIFT;
+ ehci_writel(ehci, val, ehci-regs-intel_quark_x1000_insnreg01);
I saw what other people told you about the original patch version, and
I disagree with them. It is not necessary to include a detailed
calculation like this, it only makes the code harder to read. It will
be better to have a single #define with a comment explaining it, like
this:
/* Maximum usable threshold value is 0x7f dwords for both IN and OUT */
#define INTEL_QUARK_X1000_EHCI_MAX_THRESHOLD0x007f007f
Then here, just use INTEL_QUARK_X1000_EHCI_MAX_THRESHOLD instead of
val. The comment can simply say:
/*
* For the Intel QUARK X1000, raise the I/O threshold to the
* maximum usable value in order to improve performance.
*/
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: XHCI, brain-dead scanner, and microframe rounding
On Mon, 30 Jun 2014, Mathias Nyman wrote: xhci specs says it needs the interval rounded to nearest 2^(X) value, which would be 256, not 128. I'll take a look at that. Doesn't the spec says it needs the interval rounded _down_ to a power of 2? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] usb: gadget: serial: replace hardcoded ttyGS with PREFIX
On Mon, Jun 30, 2014 at 01:23:27PM +0200, Richard Leitner wrote: Hi, On Mon, 30 Jun 2014 09:08:01 + David Laight david.lai...@aculab.com wrote: From: Of Richard Leitner Replaces all hardcoded ttyGS strings with the PREFIX macro. Due to the fact the strings are spread over different source files the PREFIX definition is moved to u_serial.h Lots of changes like: - DBG(cdev, acm ttyGS%d completion, err %d\n, - acm-port_num, req-status); + DBG(cdev, acm %s%d completion, err %d\n, + PREFIX, acm-port_num, req-status); I'm not sure this improves the code. Maybe you're right, the readability of the code wouldn't be better afterwards, but as I already mentioned IMHO if there is such a macro it should be used everywhere or nowhere... Indeed it will make most attempts to grep for the error message fail. So your thought is to carve it out completely? Are there any other opinions/ideas on that? Use the proper debug macros that the kernel provides you (i.e. dev_dbg() and family) and then you don't need to put the string in there at all, the kernel will do it automatically for you, in the correct format, so that all userspace tools can properly track what is going on. So please remove the horrid DBG() macro entirely. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 05/12] usb: phy: add the Berlin USB PHY driver
Felipe, On Fri, Jun 27, 2014 at 06:04:33PM -0500, Felipe Balbi wrote: On Fri, Jun 27, 2014 at 06:05:57PM +0200, Antoine Ténart wrote: Hi Felipe, On Fri, Jun 27, 2014 at 10:56:22AM -0500, Felipe Balbi wrote: On Tue, Jun 24, 2014 at 12:35:14PM +0200, Antoine Ténart wrote: Add the driver driving the Marvell Berlin USB PHY. This allows to initialize the PHY and to use it from the USB driver later. Signed-off-by: Antoine Ténart antoine.ten...@free-electrons.com since this is a brand new driver, it should go to drivers/phy instead. This PHY is used by a ChipIdea USB driver, which uses the provided common function for ChipIdea. These functions use the usb_phy framework. That's why this PHY driver is there. right, but you can add support for the new PHY layer which would help other users convert their PHY drivers to the new PHY framework. Adding the support for the new PHY layer in the common CI code is not complicated, but these functions also use some parts from usb hcd or usb otg which are only usb_phy compatible. Shouldn't these parts add the new PHY support first, to avoid ending up with a fairly big series, quite long to do and complicated to review? Antoine -- Antoine Ténart, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb-storage/SCSI: Add broken_fua blacklist flag
Some buggy JMicron USB-ATA bridges don't know how to translate the FUA
bit in READs or WRITEs. This patch adds an entry in unusual_devs.h
and a blacklist flag to tell the sd driver not to use FUA.
Signed-off-by: Alan Stern st...@rowland.harvard.edu
Reported-by: Michael Büsch m...@bues.ch
Tested-by: Michael Büsch m...@bues.ch
Acked-by: James Bottomley james.bottom...@hansenpartnership.com
CC: Matthew Dharm mdharm-...@one-eyed-alien.net
CC: sta...@vger.kernel.org
---
[as1751]
drivers/scsi/sd.c |5 -
drivers/usb/storage/scsiglue.c |4
drivers/usb/storage/unusual_devs.h |7 +++
include/linux/usb_usual.h |4 +++-
include/scsi/scsi_device.h |1 +
5 files changed, 19 insertions(+), 2 deletions(-)
Index: usb-3.16/include/linux/usb_usual.h
===
--- usb-3.16.orig/include/linux/usb_usual.h
+++ usb-3.16/include/linux/usb_usual.h
@@ -70,7 +70,9 @@
US_FLAG(NEEDS_CAP16,0x0040) \
/* cannot handle READ_CAPACITY_10 */\
US_FLAG(IGNORE_UAS, 0x0080) \
- /* Device advertises UAS but it is broken */
+ /* Device advertises UAS but it is broken */\
+ US_FLAG(BROKEN_FUA, 0x0100) \
+ /* Cannot handle FUA in WRITE or READ CDBs */ \
#define US_FLAG(name, value) US_FL_##name = value ,
enum { US_DO_ALL_FLAGS };
Index: usb-3.16/include/scsi/scsi_device.h
===
--- usb-3.16.orig/include/scsi/scsi_device.h
+++ usb-3.16/include/scsi/scsi_device.h
@@ -173,6 +173,7 @@ struct scsi_device {
unsigned is_visible:1; /* is the device visible in sysfs */
unsigned wce_default_on:1; /* Cache is ON by default */
unsigned no_dif:1; /* T10 PI (DIF) should be disabled */
+ unsigned broken_fua:1; /* Don't set FUA bit */
atomic_t disk_events_disable_depth; /* disable depth for disk events */
Index: usb-3.16/drivers/usb/storage/unusual_devs.h
===
--- usb-3.16.orig/drivers/usb/storage/unusual_devs.h
+++ usb-3.16/drivers/usb/storage/unusual_devs.h
@@ -1936,6 +1936,13 @@ UNUSUAL_DEV( 0x14cd, 0x6600, 0x0201, 0x
USB_SC_DEVICE, USB_PR_DEVICE, NULL,
US_FL_IGNORE_RESIDUE ),
+/* Reported by Michael Büsch m...@bues.ch */
+UNUSUAL_DEV( 0x152d, 0x0567, 0x0114, 0x0114,
+ JMicron,
+ USB to ATA/ATAPI Bridge,
+ USB_SC_DEVICE, USB_PR_DEVICE, NULL,
+ US_FL_BROKEN_FUA ),
+
/* Reported by Alexandre Oliva ol...@lsd.ic.unicamp.br
* JMicron responds to USN and several other SCSI ioctls with a
* residue that causes subsequent I/O requests to fail. */
Index: usb-3.16/drivers/usb/storage/scsiglue.c
===
--- usb-3.16.orig/drivers/usb/storage/scsiglue.c
+++ usb-3.16/drivers/usb/storage/scsiglue.c
@@ -256,6 +256,10 @@ static int slave_configure(struct scsi_d
if (us-fflags US_FL_WRITE_CACHE)
sdev-wce_default_on = 1;
+ /* A few buggy USB-ATA bridges don't understand FUA */
+ if (us-fflags US_FL_BROKEN_FUA)
+ sdev-broken_fua = 1;
+
} else {
/* Non-disk-type devices don't need to blacklist any pages
Index: usb-3.16/drivers/scsi/sd.c
===
--- usb-3.16.orig/drivers/scsi/sd.c
+++ usb-3.16/drivers/scsi/sd.c
@@ -2441,7 +2441,10 @@ sd_read_cache_type(struct scsi_disk *sdk
}
sdkp-DPOFUA = (data.device_specific 0x10) != 0;
- if (sdkp-DPOFUA !sdkp-device-use_10_for_rw) {
+ if (sdp-broken_fua) {
+ sd_first_printk(KERN_NOTICE, sdkp, Disabling FUA\n);
+ sdkp-DPOFUA = 0;
+ } else if (sdkp-DPOFUA !sdkp-device-use_10_for_rw) {
sd_first_printk(KERN_NOTICE, sdkp,
Uses READ/WRITE(6), disabling FUA\n);
sdkp-DPOFUA = 0;
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: host: ohci-tmio: Use managed interfaces for resource allocation
On Sun, 29 Jun 2014, Himangi Saraogi wrote:
This patch moves resources allocated using ioremap or
dma_declare_coherent_memory to the corresponding managed interface. The
function calls to free the allocated resources are removed in the probe
and remove functions as they are no longer required. Also, some labels
are done away with and a new label err added to make it less specific to
the context.
Signed-off-by: Himangi Saraogi himangi...@gmail.com
Acked-by: Julia Lawall julia.law...@lip6.fr
---
drivers/usb/host/ohci-tmio.c | 36
1 file changed, 12 insertions(+), 24 deletions(-)
diff --git a/drivers/usb/host/ohci-tmio.c b/drivers/usb/host/ohci-tmio.c
index bb40958..1a6034f 100644
--- a/drivers/usb/host/ohci-tmio.c
+++ b/drivers/usb/host/ohci-tmio.c
@@ -203,10 +203,8 @@ static int ohci_hcd_tmio_drv_probe(struct
platform_device *dev)
return -EINVAL;
hcd = usb_create_hcd(ohci_tmio_hc_driver, dev-dev,
dev_name(dev-dev));
- if (!hcd) {
- ret = -ENOMEM;
- goto err_usb_create_hcd;
- }
+ if (!hcd)
+ return -ENOMEM;
hcd-rsrc_start = regs-start;
hcd-rsrc_len = resource_size(regs);
@@ -215,30 +213,31 @@ static int ohci_hcd_tmio_drv_probe(struct
platform_device *dev)
spin_lock_init(tmio-lock);
- tmio-ccr = ioremap(config-start, resource_size(config));
+ tmio-ccr = devm_ioremap(dev-dev, config-start,
+ resource_size(config));
You should use devm_ioremap_resource() rather than devm_ioremap().
...
@@ -259,16 +258,8 @@ err_add_hcd:
tmio_stop_hc(dev);
if (cell-disable)
cell-disable(dev);
-err_enable:
- dma_release_declared_memory(dev-dev);
-err_dma_declare:
- iounmap(hcd-regs);
-err_ioremap_regs:
- iounmap(tmio-ccr);
-err_ioremap_ccr:
- usb_put_hcd(hcd);
-err_usb_create_hcd:
-
+err:
+ usb_put_hcr(hcd);
There is no function named usb_put_hcr(). Did you try to compile
this patch?
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb-storage/SCSI: Add broken_fua blacklist flag
On Mon, 30 Jun 2014, Alan Stern wrote: Some buggy JMicron USB-ATA bridges don't know how to translate the FUA bit in READs or WRITEs. This patch adds an entry in unusual_devs.h and a blacklist flag to tell the sd driver not to use FUA. Signed-off-by: Alan Stern st...@rowland.harvard.edu Reported-by: Michael Büsch m...@bues.ch Tested-by: Michael Büsch m...@bues.ch Acked-by: James Bottomley james.bottom...@hansenpartnership.com CC: Matthew Dharm mdharm-...@one-eyed-alien.net CC: sta...@vger.kernel.org Greg: I forgot to add your name to the recipient list on this patch. Since James has already Ack'ed it, would you like to take it for your USB tree? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: host: ohci-tmio: Use managed interfaces for resource allocation
On Mon, 30 Jun 2014, Alan Stern wrote:
On Sun, 29 Jun 2014, Himangi Saraogi wrote:
This patch moves resources allocated using ioremap or
dma_declare_coherent_memory to the corresponding managed interface. The
function calls to free the allocated resources are removed in the probe
and remove functions as they are no longer required. Also, some labels
are done away with and a new label err added to make it less specific to
the context.
Signed-off-by: Himangi Saraogi himangi...@gmail.com
Acked-by: Julia Lawall julia.law...@lip6.fr
---
drivers/usb/host/ohci-tmio.c | 36
1 file changed, 12 insertions(+), 24 deletions(-)
diff --git a/drivers/usb/host/ohci-tmio.c b/drivers/usb/host/ohci-tmio.c
index bb40958..1a6034f 100644
--- a/drivers/usb/host/ohci-tmio.c
+++ b/drivers/usb/host/ohci-tmio.c
@@ -203,10 +203,8 @@ static int ohci_hcd_tmio_drv_probe(struct
platform_device *dev)
return -EINVAL;
hcd = usb_create_hcd(ohci_tmio_hc_driver, dev-dev,
dev_name(dev-dev));
- if (!hcd) {
- ret = -ENOMEM;
- goto err_usb_create_hcd;
- }
+ if (!hcd)
+ return -ENOMEM;
hcd-rsrc_start = regs-start;
hcd-rsrc_len = resource_size(regs);
@@ -215,30 +213,31 @@ static int ohci_hcd_tmio_drv_probe(struct
platform_device *dev)
spin_lock_init(tmio-lock);
- tmio-ccr = ioremap(config-start, resource_size(config));
+ tmio-ccr = devm_ioremap(dev-dev, config-start,
+resource_size(config));
You should use devm_ioremap_resource() rather than devm_ioremap().
Even if there was no request_mem_region in the original code?
julia
...
@@ -259,16 +258,8 @@ err_add_hcd:
tmio_stop_hc(dev);
if (cell-disable)
cell-disable(dev);
-err_enable:
- dma_release_declared_memory(dev-dev);
-err_dma_declare:
- iounmap(hcd-regs);
-err_ioremap_regs:
- iounmap(tmio-ccr);
-err_ioremap_ccr:
- usb_put_hcd(hcd);
-err_usb_create_hcd:
-
+err:
+ usb_put_hcr(hcd);
There is no function named usb_put_hcr(). Did you try to compile
this patch?
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[Patch v7 2/3] usb: phy: Add Qualcomm DWC3 HS/SS PHY drivers
From: Ivan T. Ivanov iiva...@mm-sol.com These drivers handles control and configuration of the HS and SS USB PHY transceivers. They are part of the driver which manage Synopsys DesignWare USB3 controller stack inside Qualcomm SoC's. Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com Signed-off-by: Andy Gross agr...@codeaurora.org --- drivers/usb/phy/Kconfig | 11 ++ drivers/usb/phy/Makefile |2 + drivers/usb/phy/phy-qcom-hsusb.c | 348 ++ drivers/usb/phy/phy-qcom-ssusb.c | 385 ++ 4 files changed, 746 insertions(+) create mode 100644 drivers/usb/phy/phy-qcom-hsusb.c create mode 100644 drivers/usb/phy/phy-qcom-ssusb.c diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig index e253fa0..5580c2f 100644 --- a/drivers/usb/phy/Kconfig +++ b/drivers/usb/phy/Kconfig @@ -229,4 +229,15 @@ config USB_ULPI_VIEWPORT Provides read/write operations to the ULPI phy register set for controllers with a viewport register (e.g. Chipidea/ARC controllers). +config USB_QCOM_DWC3_PHY + tristate Qualcomm USB controller DWC3 PHY wrappers support + depends on (USB || USB_GADGET) ARCH_QCOM + select USB_PHY + help + Enable this to support the DWC3 USB PHY transceivers on QCOM chips + with DWC3 USB core. It handles PHY initialization, clock + management required after resetting the hardware and power + management. This driver is required even for peripheral only or + host only mode configurations. + endmenu diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile index 24a9133..ef09c9c 100644 --- a/drivers/usb/phy/Makefile +++ b/drivers/usb/phy/Makefile @@ -29,3 +29,5 @@ obj-$(CONFIG_USB_RCAR_GEN2_PHY) += phy-rcar-gen2-usb.o obj-$(CONFIG_USB_ULPI) += phy-ulpi.o obj-$(CONFIG_USB_ULPI_VIEWPORT)+= phy-ulpi-viewport.o obj-$(CONFIG_KEYSTONE_USB_PHY) += phy-keystone.o +obj-$(CONFIG_USB_QCOM_DWC3_PHY)+= phy-qcom-hsusb.o +obj-$(CONFIG_USB_QCOM_DWC3_PHY)+= phy-qcom-ssusb.o diff --git a/drivers/usb/phy/phy-qcom-hsusb.c b/drivers/usb/phy/phy-qcom-hsusb.c new file mode 100644 index 000..7a44b13 --- /dev/null +++ b/drivers/usb/phy/phy-qcom-hsusb.c @@ -0,0 +1,348 @@ +/* Copyright (c) 2013-2014, Code Aurora Forum. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include linux/clk.h +#include linux/err.h +#include linux/io.h +#include linux/module.h +#include linux/of.h +#include linux/platform_device.h +#include linux/regulator/consumer.h +#include linux/usb/phy.h + +/** + * USB QSCRATCH Hardware registers + */ +#define QSCRATCH_CTRL_REG (0x04) +#define QSCRATCH_GENERAL_CFG (0x08) +#define PHY_CTRL_REG (0x10) +#define PARAMETER_OVERRIDE_X_REG (0x14) +#define CHARGING_DET_CTRL_REG (0x18) +#define CHARGING_DET_OUTPUT_REG(0x1c) +#define ALT_INTERRUPT_EN_REG (0x20) +#define PHY_IRQ_STAT_REG (0x24) +#define CGCTL_REG (0x28) + +#define PHY_3P3_VOL_MIN305 /* uV */ +#define PHY_3P3_VOL_MAX330 /* uV */ +#define PHY_3P3_HPM_LOAD 16000 /* uA */ + +#define PHY_1P8_VOL_MIN180 /* uV */ +#define PHY_1P8_VOL_MAX180 /* uV */ +#define PHY_1P8_HPM_LOAD 19000 /* uA */ + +/* TODO: these are suspicious */ +#define USB_VDDCX_NO 1 /* index */ +#define USB_VDDCX_MIN 5 /* index */ +#define USB_VDDCX_MAX 7 /* index */ + +/* PHY_CTRL_REG */ +#define HSUSB_CTRL_DMSEHV_CLAMPBIT(24) +#define HSUSB_CTRL_USB2_SUSPENDBIT(23) +#define HSUSB_CTRL_UTMI_CLK_EN BIT(21) +#defineHSUSB_CTRL_UTMI_OTG_VBUS_VALID BIT(20) +#define HSUSB_CTRL_USE_CLKCORE BIT(18) +#define HSUSB_CTRL_DPSEHV_CLAMPBIT(17) +#define HSUSB_CTRL_COMMONONN BIT(11) +#define HSUSB_CTRL_ID_HV_CLAMP BIT(9) +#define HSUSB_CTRL_OTGSESSVLD_CLAMPBIT(8) +#define HSUSB_CTRL_CLAMP_ENBIT(7) +#define HSUSB_CTRL_RETENABLEN BIT(1) +#define HSYSB_CTRL_POR BIT(0) + + +/* QSCRATCH_GENERAL_CFG */ +#define HSUSB_GCFG_XHCI_REVBIT(2) + +struct
[Patch v7 3/3] usb: dwc3: qcom: Add device tree binding
From: Ivan T. Ivanov iiva...@mm-sol.com
QCOM USB3.0 core wrapper consist of USB3.0 IP from Synopsys
(SNPS) and HS, SS PHY's control and configuration registers.
It could operate in device mode (SS, HS, FS) and host
mode (SS, HS, FS, LS).
Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
Signed-off-by: Andy Gross agr...@codeaurora.org
---
.../devicetree/bindings/usb/qcom,dwc3.txt | 104
1 file changed, 104 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/qcom,dwc3.txt
diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
new file mode 100644
index 000..105b6b7
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
@@ -0,0 +1,104 @@
+Qualcomm SuperSpeed DWC3 USB SoC controller
+
+
+QCOM DWC3 Highspeed USB PHY
+
+Required properities:
+- compatible: should contain qcom,dwc3-hsphy;
+- reg: offset and length of the register set in the memory map
+- clocks: A list of phandle + clock-specifier pairs for the
+ clocks listed in clock-names
+- clock-names: Should contain the following:
+ utmi UTMI clock
+- v1p8-supply: phandle to the regulator for the 1.8v supply to HSPHY.
+- v3p3-supply: phandle to the regulator for the 3.3v supply to HSPHY.
+- vbus-supply: phandle to the regulator for the vbus supply for host
+ mode.
+- vddcx-supply: phandle to the regulator for the vdd supply for HSPHY
+digital circuit operation.
+
+Optional clocks:
+ xo External reference clock
+
+
+QCOM DWC3 Superspeed USB PHY
+=
+Required properities:
+- compatible: should contain qcom,dwc3-ssphy;
+- reg: offset and length of the register set in the memory map
+- clocks: A list of phandle + clock-specifier pairs for the
+ clocks listed in clock-names
+- clock-names: Should contain the following:
+ refReference clock used in host mode.
+- v1p8-supply: phandle to the regulator for the 1.8v supply to HSPHY.
+- vddcx-supply: phandle to the regulator for the vdd supply for HSPHY
+digital circuit operation.
+
+Optional clocks:
+ xo External reference clock
+
+QCOM DWC3 controller wrapper
+===
+Required properties:
+- compatible: should contain qcom,dwc3
+- clocks: A list of phandle + clock-specifier pairs for the
+ clocks listed in clock-names
+- clock-names: Should contain the following:
+ core Master/Core clock, have to be = 125 MHz for SS
+ operation and = 60MHz for HS operation
+
+Optional clocks:
+ iface System bus AXI clock. Not present on all platforms
+ sleep Sleep clock, used when USB3 core goes into low
+ power mode (U3).
+
+Optional regulator:
+- gdsc-supply: phandle to the regulator from globally distributed
+ switch controller
+
+Required child node:
+A child node must exist to represent the core DWC3 IP block. The name of
+the node is not important. The content of the node is defined in dwc3.txt.
+
+Example device nodes:
+
+ hs_phy_0: phy@110f8800 {
+ compatible = qcom,dwc3-hsphy;
+ reg = 0x110f8800 0x30;
+ clocks = gcc USB30_0_UTMI_CLK;
+ clock-names = utmi;
+
+ status = disabled;
+ };
+
+ ss_phy_0: phy@110f8830 {
+ compatible = qcom,dwc3-ssphy;
+ reg = 0x110f8830 0x30;
+
+ clocks = gcc USB30_0_MASTER_CLK;
+ clock-names = ref;
+
+ status = disabled;
+ };
+
+ usb3_0: usb30@0 {
+ compatible = qcom,dwc3;
+ #address-cells = 1;
+ #size-cells = 1;
+ clocks = gcc USB30_0_MASTER_CLK;
+ clock-names = core;
+
+ ranges;
+
+ status = disabled;
+
+ dwc3@1100 {
+ compatible = snps,dwc3;
+ reg = 0x1100 0xcd00;
+ interrupts = 0 110 0x4;
+ usb-phy = hs_phy_0, ss_phy_0;
+ phy-names = usb2-phy, usb3-phy;
+ tx-fifo-resize;
+ dr_mode = host;
+ };
+ };
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line unsubscribe
[Patch v7 1/3] usb: dwc3: Add Qualcomm DWC3 glue layer driver
From: Ivan T. Ivanov iiva...@mm-sol.com
DWC3 glue layer is hardware layer around Synopsys DesignWare
USB3 core. Its purpose is to supply Synopsys IP with required
clocks, voltages and interface it with the rest of the SoC.
Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
Signed-off-by: Andy Gross agr...@codeaurora.org
---
drivers/usb/dwc3/Kconfig |9 +++
drivers/usb/dwc3/Makefile|1 +
drivers/usb/dwc3/dwc3-qcom.c | 153 ++
3 files changed, 163 insertions(+)
create mode 100644 drivers/usb/dwc3/dwc3-qcom.c
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 8eb996e..29fcbfd 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -79,6 +79,15 @@ config USB_DWC3_KEYSTONE
Support of USB2/3 functionality in TI Keystone2 platforms.
Say 'Y' or 'M' here if you have one such device
+config USB_DWC3_QCOM
+ tristate Qualcomm Platforms
+ default USB_DWC3
+ select USB_QCOM_DWC3_PHY
+ help
+ Recent Qualcomm SoCs ship with one DesignWare Core USB3 IP inside,
+ say 'Y' or 'M' if you have one such device.
+
+
comment Debugging features
config USB_DWC3_DEBUG
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index 10ac3e7..0da8e75 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -33,3 +33,4 @@ obj-$(CONFIG_USB_DWC3_OMAP) += dwc3-omap.o
obj-$(CONFIG_USB_DWC3_EXYNOS) += dwc3-exynos.o
obj-$(CONFIG_USB_DWC3_PCI) += dwc3-pci.o
obj-$(CONFIG_USB_DWC3_KEYSTONE)+= dwc3-keystone.o
+obj-$(CONFIG_USB_DWC3_QCOM)+= dwc3-qcom.o
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
new file mode 100644
index 000..e99764a
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -0,0 +1,153 @@
+/* Copyright (c) 2013-2014, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/clk.h
+#include linux/err.h
+#include linux/io.h
+#include linux/module.h
+#include linux/of.h
+#include linux/of_platform.h
+#include linux/platform_device.h
+#include linux/regulator/consumer.h
+#include linux/usb/phy.h
+
+#include core.h
+
+
+struct dwc3_qcom {
+ struct device *dev;
+
+ struct clk *core_clk;
+ struct clk *iface_clk;
+ struct clk *sleep_clk;
+
+ struct regulator*gdsc;
+};
+
+static int dwc3_qcom_probe(struct platform_device *pdev)
+{
+ struct device_node *node = pdev-dev.of_node;
+ struct dwc3_qcom *qdwc;
+ int ret = 0;
+
+ qdwc = devm_kzalloc(pdev-dev, sizeof(*qdwc), GFP_KERNEL);
+ if (!qdwc)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, qdwc);
+
+ qdwc-dev = pdev-dev;
+
+ qdwc-gdsc = devm_regulator_get(qdwc-dev, gdsc);
+
+ qdwc-core_clk = devm_clk_get(qdwc-dev, core);
+ if (IS_ERR(qdwc-core_clk)) {
+ dev_dbg(qdwc-dev, failed to get core clock\n);
+ return PTR_ERR(qdwc-core_clk);
+ }
+
+ qdwc-iface_clk = devm_clk_get(qdwc-dev, iface);
+ if (IS_ERR(qdwc-iface_clk)) {
+ dev_dbg(qdwc-dev, failed to get iface clock, skipping\n);
+ qdwc-iface_clk = NULL;
+ }
+
+ qdwc-sleep_clk = devm_clk_get(qdwc-dev, sleep);
+ if (IS_ERR(qdwc-sleep_clk)) {
+ dev_dbg(qdwc-dev, failed to get sleep clock, skipping\n);
+ qdwc-sleep_clk = NULL;
+ }
+
+ if (!IS_ERR(qdwc-gdsc)) {
+ ret = regulator_enable(qdwc-gdsc);
+ if (ret)
+ dev_err(qdwc-dev, cannot enable gdsc\n);
+ }
+
+ clk_prepare_enable(qdwc-core_clk);
+
+ if (qdwc-iface_clk)
+ clk_prepare_enable(qdwc-iface_clk);
+
+ if (qdwc-sleep_clk)
+ clk_prepare_enable(qdwc-sleep_clk);
+
+ ret = of_platform_populate(node, NULL, NULL, qdwc-dev);
+ if (ret) {
+ dev_err(qdwc-dev, failed to register core - %d\n, ret);
+ dev_dbg(qdwc-dev, failed to add create dwc3 core\n);
+ goto dis_clks;
+ }
+
+ return 0;
+
+dis_clks:
+ if (qdwc-sleep_clk)
+ clk_disable_unprepare(qdwc-sleep_clk);
+
+ if (qdwc-iface_clk)
+ clk_disable_unprepare(qdwc-iface_clk);
+
+ clk_disable_unprepare(qdwc-core_clk);
+
+ if (!IS_ERR(qdwc-gdsc)) {
+ ret = regulator_disable(qdwc-gdsc);
+
[Patch v7 0/3] DWC3 USB support for Qualcomm platform
These patches add basic support for USB3.0 controllers found on MSM platforms. USB3.0 core is based on Synopsys DesignWare SuperSpeed IP. This work was started by Ivan Ivanov and went through a number of iterations. I picked these patches up and did a little rework to get them working. Changes since v6: * Renamed all MSM references to qcom, including file names * Removed direct TCSR manipulation. This done from new TCSR driver. * Added defines for register bits * XO clk is optional on some platforms. Corrected logic to handle this. * Ignore set_voltage failures. This allows us to support dummy regulators on platforms where there is no voltage control. * Reworked devicetree binding to remove TCSR reg resources Changes since v5: * devicetree bindings descriptions fixes * Fixed NULL pointer dereferences in dev_prink's * Removed extra space in sleep clock name Changes since v4: * Substitute references to wc3 with just dw in USB PHY drivers and file names. This is to indicate that the PHY's are DesignWare, but not necessarily related to DWC3 IP core. Changes since v3: * Remove _clk suffix from clock names * Clarify required child node for qcom,dwc3 * Fix comments in functions headers * Use dbg instead err in drivers probe functions. Changes since v2: * Several improvements in devicetree bindings description * Disable regulators in glue layer if there is error during ioremap. Changes since first version: * Split devicetree bindings description file to separate patch * Address comments for device bindings description * Fix typo in 'gdsc' requlator name. Ivan T. Ivanov (3): usb: dwc3: Add Qualcomm DWC3 glue layer driver usb: phy: Add Qualcomm DWC3 HS/SS PHY drivers usb: dwc3: qcom: Add device tree binding .../devicetree/bindings/usb/qcom,dwc3.txt | 104 ++ drivers/usb/dwc3/Kconfig |9 + drivers/usb/dwc3/Makefile |1 + drivers/usb/dwc3/dwc3-qcom.c | 153 drivers/usb/phy/Kconfig| 11 + drivers/usb/phy/Makefile |2 + drivers/usb/phy/phy-qcom-hsusb.c | 348 ++ drivers/usb/phy/phy-qcom-ssusb.c | 385 8 files changed, 1013 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/qcom,dwc3.txt create mode 100644 drivers/usb/dwc3/dwc3-qcom.c create mode 100644 drivers/usb/phy/phy-qcom-hsusb.c create mode 100644 drivers/usb/phy/phy-qcom-ssusb.c -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: host: ohci-tmio: Use managed interfaces for resource allocation
On Mon, 30 Jun 2014, Julia Lawall wrote: @@ -215,30 +213,31 @@ static int ohci_hcd_tmio_drv_probe(struct platform_device *dev) spin_lock_init(tmio-lock); - tmio-ccr = ioremap(config-start, resource_size(config)); + tmio-ccr = devm_ioremap(dev-dev, config-start, + resource_size(config)); You should use devm_ioremap_resource() rather than devm_ioremap(). Even if there was no request_mem_region in the original code? Hmmm, that does seem strange. Looking at some of the other OHCI platform drivers, I see that one of them (ohci-sa.c) calls request_mem_region without ioremap and two of them (ohci-omap3.c, ohci-tmio.c) call ioremap without request_mem_region. (ohci-ppc-of.c also calls request_mem_region without ioremap, but for a totally different reason.) In the case of ohci-sa this appears to be a platform-specific thing. Are the other two simply buggy? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb-storage/SCSI: Add broken_fua blacklist flag
On Mon, Jun 30, 2014 at 11:16:51AM -0400, Alan Stern wrote: On Mon, 30 Jun 2014, Alan Stern wrote: Some buggy JMicron USB-ATA bridges don't know how to translate the FUA bit in READs or WRITEs. This patch adds an entry in unusual_devs.h and a blacklist flag to tell the sd driver not to use FUA. Signed-off-by: Alan Stern st...@rowland.harvard.edu Reported-by: Michael Büsch m...@bues.ch Tested-by: Michael Büsch m...@bues.ch Acked-by: James Bottomley james.bottom...@hansenpartnership.com CC: Matthew Dharm mdharm-...@one-eyed-alien.net CC: sta...@vger.kernel.org Greg: I forgot to add your name to the recipient list on this patch. Since James has already Ack'ed it, would you like to take it for your USB tree? Sure, I can take it, thanks. greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] usb: dwc3: Kconfig: Fix dependency for Exynos DWC3
On Mon, Jun 30, 2014 at 02:33:14PM +0530, Sachin Kamat wrote: Make the config depend on ARCH_EXYNOS5 instead of ARCH_EXYNOS as this IP is available only on Exynos5 platforms. Signed-off-by: Sachin Kamat sachin.ka...@samsung.com --- drivers/usb/dwc3/Kconfig |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig index 261c3b428220..24a4f8ed9fd1 100644 --- a/drivers/usb/dwc3/Kconfig +++ b/drivers/usb/dwc3/Kconfig @@ -55,7 +55,7 @@ config USB_DWC3_OMAP config USB_DWC3_EXYNOS tristate Samsung Exynos Platform - depends on ARCH_EXYNOS || COMPILE_TEST + depends on ARCH_EXYNOS5 || COMPILE_TEST what happens when you have exynos[6789] ? Are we going to continually increase this line ? To me this is a pointless change. -- balbi signature.asc Description: Digital signature
Re: [Patch v7 1/3] usb: dwc3: Add Qualcomm DWC3 glue layer driver
Hi,
On Mon, Jun 30, 2014 at 11:03:51AM -0500, Andy Gross wrote:
+static int dwc3_qcom_probe(struct platform_device *pdev)
+{
+ struct device_node *node = pdev-dev.of_node;
+ struct dwc3_qcom *qdwc;
+ int ret = 0;
+
+ qdwc = devm_kzalloc(pdev-dev, sizeof(*qdwc), GFP_KERNEL);
+ if (!qdwc)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, qdwc);
+
+ qdwc-dev = pdev-dev;
+
+ qdwc-gdsc = devm_regulator_get(qdwc-dev, gdsc);
+
+ qdwc-core_clk = devm_clk_get(qdwc-dev, core);
+ if (IS_ERR(qdwc-core_clk)) {
+ dev_dbg(qdwc-dev, failed to get core clock\n);
+ return PTR_ERR(qdwc-core_clk);
+ }
+
+ qdwc-iface_clk = devm_clk_get(qdwc-dev, iface);
+ if (IS_ERR(qdwc-iface_clk)) {
+ dev_dbg(qdwc-dev, failed to get iface clock, skipping\n);
+ qdwc-iface_clk = NULL;
so this clock and sleep_clk are optional, that's fine...
+ }
+
+ qdwc-sleep_clk = devm_clk_get(qdwc-dev, sleep);
+ if (IS_ERR(qdwc-sleep_clk)) {
+ dev_dbg(qdwc-dev, failed to get sleep clock, skipping\n);
+ qdwc-sleep_clk = NULL;
+ }
+
+ if (!IS_ERR(qdwc-gdsc)) {
+ ret = regulator_enable(qdwc-gdsc);
+ if (ret)
+ dev_err(qdwc-dev, cannot enable gdsc\n);
+ }
+
+ clk_prepare_enable(qdwc-core_clk);
+
+ if (qdwc-iface_clk)
+ clk_prepare_enable(qdwc-iface_clk);
... right here you can drop the NULL check because clk_prepare_enable()
is safe against NULL pointers.
--
balbi
signature.asc
Description: Digital signature
Re: [Patch v7 2/3] usb: phy: Add Qualcomm DWC3 HS/SS PHY drivers
Hi,
first of all, since this is a brand new PHY driver, could you guys use
the generic phy framework instead ? (drivers/phy)
On Mon, Jun 30, 2014 at 11:03:52AM -0500, Andy Gross wrote:
diff --git a/drivers/usb/phy/phy-qcom-hsusb.c
b/drivers/usb/phy/phy-qcom-hsusb.c
new file mode 100644
index 000..7a44b13
--- /dev/null
+++ b/drivers/usb/phy/phy-qcom-hsusb.c
@@ -0,0 +1,348 @@
+/* Copyright (c) 2013-2014, Code Aurora Forum. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/clk.h
+#include linux/err.h
+#include linux/io.h
+#include linux/module.h
+#include linux/of.h
+#include linux/platform_device.h
+#include linux/regulator/consumer.h
+#include linux/usb/phy.h
+
+/**
+ * USB QSCRATCH Hardware registers
+ */
+#define QSCRATCH_CTRL_REG(0x04)
+#define QSCRATCH_GENERAL_CFG (0x08)
+#define PHY_CTRL_REG (0x10)
+#define PARAMETER_OVERRIDE_X_REG (0x14)
+#define CHARGING_DET_CTRL_REG(0x18)
+#define CHARGING_DET_OUTPUT_REG (0x1c)
+#define ALT_INTERRUPT_EN_REG (0x20)
+#define PHY_IRQ_STAT_REG (0x24)
+#define CGCTL_REG(0x28)
+
+#define PHY_3P3_VOL_MIN 305 /* uV */
+#define PHY_3P3_VOL_MAX 330 /* uV */
+#define PHY_3P3_HPM_LOAD 16000 /* uA */
+
+#define PHY_1P8_VOL_MIN 180 /* uV */
+#define PHY_1P8_VOL_MAX 180 /* uV */
+#define PHY_1P8_HPM_LOAD 19000 /* uA */
+
+/* TODO: these are suspicious */
+#define USB_VDDCX_NO 1 /* index */
+#define USB_VDDCX_MIN5 /* index */
+#define USB_VDDCX_MAX7 /* index */
+
+/* PHY_CTRL_REG */
+#define HSUSB_CTRL_DMSEHV_CLAMP BIT(24)
+#define HSUSB_CTRL_USB2_SUSPEND BIT(23)
+#define HSUSB_CTRL_UTMI_CLK_EN BIT(21)
+#define HSUSB_CTRL_UTMI_OTG_VBUS_VALID BIT(20)
+#define HSUSB_CTRL_USE_CLKCORE BIT(18)
+#define HSUSB_CTRL_DPSEHV_CLAMP BIT(17)
+#define HSUSB_CTRL_COMMONONN BIT(11)
+#define HSUSB_CTRL_ID_HV_CLAMP BIT(9)
+#define HSUSB_CTRL_OTGSESSVLD_CLAMP BIT(8)
+#define HSUSB_CTRL_CLAMP_EN BIT(7)
+#define HSUSB_CTRL_RETENABLENBIT(1)
+#define HSYSB_CTRL_POR BIT(0)
+
+
+/* QSCRATCH_GENERAL_CFG */
+#define HSUSB_GCFG_XHCI_REV BIT(2)
+
+struct qcom_dwc3_hs_phy {
+ struct usb_phy phy;
+ void __iomem*base;
+ struct device *dev;
+
+ struct clk *xo_clk;
+ struct clk *utmi_clk;
+
+ struct regulator*v3p3;
+ struct regulator*v1p8;
+ struct regulator*vddcx;
+ struct regulator*vbus;
+};
+
+#define phy_to_dw_phy(x)container_of((x), struct
qcom_dwc3_hs_phy, phy)
+
+/**
+ * Write register.
+ *
+ * @base - QCOM DWC3 PHY base virtual address.
+ * @offset - register offset.
+ * @val - value to write.
+ */
+static inline void qcom_dwc3_hs_write(void __iomem *base, u32 offset, u32
val)
+{
+ writel(val, base + offset);
+}
+
+/**
+ * Write register and read back masked value to confirm it is written
+ *
+ * @base - QCOM DWC3 PHY base virtual address.
+ * @offset - register offset.
+ * @mask - register bitmask specifying what should be updated
+ * @val - value to write.
+ */
+static inline void qcom_dwc3_hs_write_readback(void __iomem *base, u32
offset,
+ const u32 mask, u32 val)
+{
+ u32 write_val, tmp = readl(base + offset);
+
+ tmp = ~mask; /* retain other bits */
+ write_val = tmp | val;
+
+ writel(write_val, base + offset);
+
+ /* Read back to see if val was written */
+ tmp = readl(base + offset);
+ tmp = mask;/* clear other bits */
+
+ if (tmp != val)
+ pr_err(write: %x to QSCRATCH: %x FAILED\n, val, offset);
really nit-picking - and I'm not even sure I care - but passing a dev
pointer to use dev_err() here might be a good idea.
+}
+
+static void qcom_dwc3_hs_phy_shutdown(struct usb_phy *x)
+{
+ struct qcom_dwc3_hs_phy *phy = phy_to_dw_phy(x);
+ int ret;
+
+ ret = regulator_set_voltage(phy-v3p3, 0,
Re: [PATCH 1/3] phy: omap-usb2: Manage PHY 3.3V supply regulator
Hi,
On Mon, Jun 30, 2014 at 02:00:36PM +0300, Roger Quadros wrote:
On some SoCs e.g. J6 the 3.3V supply to the USB2 PHY can be
powered down when the PHY is not in use. Add regulator
management code to control this power line.
Signed-off-by: Roger Quadros rog...@ti.com
Reviewed-by: Felipe Balbi ba...@ti.com
---
drivers/phy/phy-omap-usb2.c | 25 +
include/linux/phy/omap_usb.h | 1 +
2 files changed, 26 insertions(+)
diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
index 7007c11..2afc79c 100644
--- a/drivers/phy/phy-omap-usb2.c
+++ b/drivers/phy/phy-omap-usb2.c
@@ -30,6 +30,7 @@
#include linux/phy/omap_control_phy.h
#include linux/phy/phy.h
#include linux/of_platform.h
+#include linux/regulator/consumer.h
#define USB2PHY_DISCON_BYP_LATCH (1 31)
#define USB2PHY_ANA_CONFIG1 0x4c
@@ -107,6 +108,14 @@ static int omap_usb_power_off(struct phy *x)
omap_control_phy_power(phy-control_dev, 0);
+ if (phy-pwr) {
+ int ret;
+
+ ret = regulator_disable(phy-pwr);
+ if (ret)
+ return ret;
+ }
+
return 0;
}
@@ -114,6 +123,14 @@ static int omap_usb_power_on(struct phy *x)
{
struct omap_usb *phy = phy_get_drvdata(x);
+ if (phy-pwr) {
+ int ret;
+
+ ret = regulator_enable(phy-pwr);
+ if (ret)
+ return ret;
+ }
+
omap_control_phy_power(phy-control_dev, 1);
return 0;
@@ -253,6 +270,14 @@ static int omap_usb2_probe(struct platform_device *pdev)
phy-control_dev = control_pdev-dev;
omap_control_phy_power(phy-control_dev, 0);
+ /* phy-supply */
+ phy-pwr = devm_regulator_get_optional(phy-dev, phy);
+ if (IS_ERR(phy-pwr)) {
+ if (PTR_ERR(phy-pwr) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ phy-pwr = NULL;
+ }
+
otg-set_host = omap_usb_set_host;
otg-set_peripheral = omap_usb_set_peripheral;
if (phy_data-flags OMAP_USB2_HAS_SET_VBUS)
diff --git a/include/linux/phy/omap_usb.h b/include/linux/phy/omap_usb.h
index dc2c541..e2c46df 100644
--- a/include/linux/phy/omap_usb.h
+++ b/include/linux/phy/omap_usb.h
@@ -40,6 +40,7 @@ struct omap_usb {
struct clk *wkupclk;
struct clk *optclk;
u8 flags;
+ struct regulator*pwr;
};
struct usb_phy_data {
--
1.8.3.2
--
balbi
signature.asc
Description: Digital signature
Re: [RFC PATCH 0/3] ehci_msm fixes for APQ8064 USB host support.
Hi, On Wed, Jun 18, 2014 at 06:00:01PM +0100, Srinivas Kandagatla wrote: While testing usb host on Qualcomm APQ8064, I encountered few issues. These patches fixes those issues. Without these patches USB is not functional on AQ8064. All the patches are tested on IFC6410. please resend without RFC so I can apply. -- balbi signature.asc Description: Digital signature
Re: [RFC PATCH 3/3] usb: phy: msm: Do not do runtime pm if the phy is not idle
On Mon, Jun 30, 2014 at 11:23:43AM +0100, Srinivas Kandagatla wrote: Hi Felipe, On 27/06/14 16:54, Felipe Balbi wrote: Hi, On Wed, Jun 18, 2014 at 06:01:08PM +0100, Srinivas Kandagatla wrote: Use case is when the phy is configured in host mode and a usb device is attached to board before bootup. On bootup, with the existing code and runtime pm enabled, the driver would decrement the pm usage count without checking the current state of the phy. This pm usage count decrement would trigger the runtime pm which than would abort the usb enumeration which was in progress. In my case a usb stick gets detected and then immediatly the driver goes to low power mode which is not correct. log: [1.631412] msm_hsusb_host 1252.usb: EHCI Host Controller [1.636556] msm_hsusb_host 1252.usb: new USB bus registered, assigned bus number 1 [1.642563] msm_hsusb_host 1252.usb: irq 220, io mem 0x1252 [1.658197] msm_hsusb_host 1252.usb: USB 2.0 started, EHCI 1.00 [1.659473] hub 1-0:1.0: USB hub found [1.663415] hub 1-0:1.0: 1 port detected ... [1.973352] usb 1-1: new high-speed USB device number 2 using msm_hsusb_host [2.107707] usb-storage 1-1:1.0: USB Mass Storage device detected [2.108993] scsi0 : usb-storage 1-1:1.0 [2.678341] msm_otg 1252.phy: USB in low power mode [3.168977] usb 1-1: USB disconnect, device number 2 This issue was detected on IFC6410 board. This patch fixes the intial runtime pm trigger by checking the phy state and decrementing the pm use count only when the phy state is IDLE. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org --- drivers/usb/phy/phy-msm-usb.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c index 3bb559d..78cc870 100644 --- a/drivers/usb/phy/phy-msm-usb.c +++ b/drivers/usb/phy/phy-msm-usb.c @@ -1229,7 +1229,9 @@ static void msm_otg_sm_work(struct work_struct *w) motg-chg_state = USB_CHG_STATE_UNDEFINED; motg-chg_type = USB_INVALID_CHARGER; } - pm_runtime_put_sync(otg-phy-dev); + + if (otg-phy-state == OTG_STATE_B_IDLE) + pm_runtime_put_sync(otg-phy-dev); instead, you might as well just use autosuspend. autosuspend is a good idea and will provide a delay before rumtime suspend, however the bug which is fixed here still needs to be fixed anyway. runtime PM in this driver is not that great, the driver just increments the pm use count if the phy is configured in host or deivce mode. This patch fixes a bug in its state-machine which decrements pm use-count without checking the state. As this is a PHY driver, am not sure moving to autosuspend means we would end up just adding some static inactivity delay? Is it really going to add any value to this PHY driver? yeah, perhaps not... I guess we can apply this as a fix; you're right. -- balbi signature.asc Description: Digital signature
Re: [RFC PATCH 0/3] ehci_msm fixes for APQ8064 USB host support.
Thanks Felipe, On 30/06/14 18:13, Felipe Balbi wrote: Hi, On Wed, Jun 18, 2014 at 06:00:01PM +0100, Srinivas Kandagatla wrote: While testing usb host on Qualcomm APQ8064, I encountered few issues. These patches fixes those issues. Without these patches USB is not functional on AQ8064. All the patches are tested on IFC6410. please resend without RFC so I can apply. I will resend the patches. --srini -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/1] usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers
On Sun, Jun 29, 2014 at 11:33:52PM +0900, Magnus Damm wrote:
On Sat, Jun 28, 2014 at 12:40 AM, Felipe Balbi ba...@ti.com wrote:
On Wed, Jun 18, 2014 at 02:15:42PM +0900, Magnus Damm wrote:
Hi Felipe,
On Fri, Jun 13, 2014 at 11:25 PM, Felipe Balbi ba...@ti.com wrote:
Hi,
On Fri, Jun 13, 2014 at 09:20:31PM +0900, Yoshihiro Shimoda wrote:
The R-Car H2 and M2 SoCs come with an xHCI controller that requires
some specific initializations related to the firmware downloading and
some specific registers. This patch adds the support for this special
configuration as an xHCI quirk executed during probe and start.
Signed-off-by: Yoshihiro Shimoda yoshihiro.shimoda...@renesas.com
---
diff --git a/drivers/usb/host/xhci-rcar.c b/drivers/usb/host/xhci-rcar.c
new file mode 100644
index 000..ff0d1b4
--- /dev/null
+++ b/drivers/usb/host/xhci-rcar.c
@@ -0,0 +1,148 @@
+/*
+ * xHCI host controller driver for R-Car SoCs
+ *
+ * Copyright (C) 2014 Renesas Electronics Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ */
+
+#include linux/firmware.h
+#include linux/module.h
+#include linux/platform_device.h
+#include linux/usb/phy.h
+
+#include xhci.h
+#include xhci-rcar.h
+
+#define FIRMWARE_NAMEr8a779x_usb3_v1.dlmem
+MODULE_FIRMWARE(FIRMWARE_NAME);
+
+/*** Register Offset ***/
+#define RCAR_USB3_INT_ENA0x224 /* Interrupt Enable */
+#define RCAR_USB3_DL_CTRL0x250 /* FW Download Control Status */
+#define RCAR_USB3_FW_DATA0 0x258 /* FW Data0 */
+
+#define RCAR_USB3_LCLK 0xa44 /* LCLK Select */
+#define RCAR_USB3_CONF1 0xa48 /* USB3.0 Configuration1
*/
+#define RCAR_USB3_CONF2 0xa5c /* USB3.0 Configuration2
*/
+#define RCAR_USB3_CONF3 0xaa8 /* USB3.0 Configuration3
*/
+#define RCAR_USB3_RX_POL 0xab0 /* USB3.0 RX Polarity */
+#define RCAR_USB3_TX_POL 0xab8 /* USB3.0 TX Polarity */
+
+/*** Register Settings ***/
+/* Interrupt Enable */
+#define RCAR_USB3_INT_XHC_ENA0x0001
+#define RCAR_USB3_INT_PME_ENA0x0002
+#define RCAR_USB3_INT_HSE_ENA0x0004
+#define RCAR_USB3_INT_ENA_VAL(RCAR_USB3_INT_XHC_ENA | \
+ RCAR_USB3_INT_PME_ENA |
RCAR_USB3_INT_HSE_ENA)
+
+/* FW Download Control Status */
+#define RCAR_USB3_DL_CTRL_ENABLE 0x0001
+#define RCAR_USB3_DL_CTRL_FW_SUCCESS 0x0010
+#define RCAR_USB3_DL_CTRL_FW_SET_DATA0 0x0100
+
+/* LCLK Select */
+#define RCAR_USB3_LCLK_ENA_VAL 0x01030001
+
+/* USB3.0 Configuration */
+#define RCAR_USB3_CONF1_VAL 0x00030204
+#define RCAR_USB3_CONF2_VAL 0x00030300
+#define RCAR_USB3_CONF3_VAL 0x13802007
+
+/* USB3.0 Polarity */
+#define RCAR_USB3_RX_POL_VAL BIT(21)
+#define RCAR_USB3_TX_POL_VAL BIT(4)
+
+void xhci_rcar_start(struct usb_hcd *hcd)
+{
+ u32 temp;
+
+ if (hcd-regs != NULL) {
+ /* Interrupt Enable */
+ temp = readl(hcd-regs + RCAR_USB3_INT_ENA);
+ temp |= RCAR_USB3_INT_ENA_VAL;
+ writel(temp, hcd-regs + RCAR_USB3_INT_ENA);
+ /* LCLK Select */
+ writel(RCAR_USB3_LCLK_ENA_VAL, hcd-regs +
RCAR_USB3_LCLK);
+ /* USB3.0 Configuration */
+ writel(RCAR_USB3_CONF1_VAL, hcd-regs + RCAR_USB3_CONF1);
+ writel(RCAR_USB3_CONF2_VAL, hcd-regs + RCAR_USB3_CONF2);
+ writel(RCAR_USB3_CONF3_VAL, hcd-regs + RCAR_USB3_CONF3);
+ /* USB3.0 Polarity */
+ writel(RCAR_USB3_RX_POL_VAL, hcd-regs +
RCAR_USB3_RX_POL);
+ writel(RCAR_USB3_TX_POL_VAL, hcd-regs +
RCAR_USB3_TX_POL);
+ }
+}
+
+static int xhci_rcar_download_firmware(struct device *dev, void
__iomem *regs)
+{
+ const struct firmware *fw;
+ int retval, index, j, time;
+ int timeout = 1;
+ u32 data, val, temp;
+
+ /* request R-Car USB3.0 firmware */
+ retval = request_firmware(fw, FIRMWARE_NAME, dev);
+ if (retval)
+ return retval;
+
+ /* download R-Car USB3.0 firmware */
+ temp = readl(regs + RCAR_USB3_DL_CTRL);
+ temp |= RCAR_USB3_DL_CTRL_ENABLE;
+ writel(temp, regs + RCAR_USB3_DL_CTRL);
+
+ for (index = 0; index fw-size; index += 4) {
+ /* to avoid reading beyond the end of the buffer */
+ for (data = 0, j = 3; j = 0; j--) {
+ if ((j + index) fw-size)
+ data |= fw-data[index + j] (8 * j);
+ }
+
Re: [PATCH v4 1/7] dma: cppi41: handle 0-length packets
On Mon, May 26, 2014 at 02:52:34PM +0200, Daniel Mack wrote: When a 0-length packet is received on the bus, desc-pd0 yields 1, which confuses the driver's users. This information is clearly wrong and not in accordance to the datasheet, but it's been observed on an AM335x board, very reproducible. Fix this by looking at bit 19 in PD2 of the completed packet. This bit will tell us if a zero-length packet was received on a queue. If it's set, ignore the value in PD0 and report a total length of 0 instead. Signed-off-by: Daniel Mack zon...@gmail.com Does this create dependency on the following patches ? Since they're in the same series, I suppose it does... Please clarify so we don't cause regressions upstream due to ordering. -- balbi signature.asc Description: Digital signature
[PATCH v1 0/3] ehci_msm fixes for APQ8064 USB host support.
While testing usb host on Qualcomm APQ8064, I encountered few issues. These patches fixes those issues. Without these patches USB is not functional on AQ8064. All the patches are tested on IFC6410. Resending the patches by removing the RFC in the subject. Can you please consider these patches for next rc, as they are just fixes. Thanks, srini Srinivas Kandagatla (3): usb: Kconfig: make EHCI_MSM selectable for QCOM SOCs usb: phy: msm: Make phy_reset clk and reset line optional. usb: phy: msm: Do not do runtime pm if the phy is not idle drivers/usb/host/Kconfig | 2 +- drivers/usb/phy/phy-msm-usb.c | 14 -- 2 files changed, 9 insertions(+), 7 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v1 1/3] usb: Kconfig: make EHCI_MSM selectable for QCOM SOCs
This patch makes the msm ehci driver available to use on QCOM SOCs, which have the same IP. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org --- drivers/usb/host/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 61b7817..03314f8 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -176,7 +176,7 @@ config USB_EHCI_HCD_AT91 config USB_EHCI_MSM tristate Support for Qualcomm QSD/MSM on-chip EHCI USB controller - depends on ARCH_MSM + depends on ARCH_MSM || ARCH_QCOM select USB_EHCI_ROOT_HUB_TT ---help--- Enables support for the USB Host controller present on the -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v1 2/3] usb: phy: msm: Make phy_reset clk and reset line optional.
This patch makes the phy reset clk and reset line optional as this clk
is not available on boards like IFC6410 with APQ8064.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
---
drivers/usb/phy/phy-msm-usb.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index ced34f3..3bb559d 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -279,11 +279,11 @@ static int msm_otg_link_clk_reset(struct msm_otg *motg,
bool assert)
static int msm_otg_phy_clk_reset(struct msm_otg *motg)
{
- int ret;
+ int ret = 0;
- if (motg-pdata-phy_clk_reset)
+ if (motg-pdata-phy_clk_reset motg-phy_reset_clk)
ret = motg-pdata-phy_clk_reset(motg-phy_reset_clk);
- else
+ else if (motg-phy_rst)
ret = reset_control_reset(motg-phy_rst);
if (ret)
@@ -1464,7 +1464,7 @@ static int msm_otg_read_dt(struct platform_device *pdev,
struct msm_otg *motg)
motg-phy_rst = devm_reset_control_get(pdev-dev, phy);
if (IS_ERR(motg-phy_rst))
- return PTR_ERR(motg-phy_rst);
+ motg-phy_rst = NULL;
pdata-mode = of_usb_get_dr_mode(node);
if (pdata-mode == USB_DR_MODE_UNKNOWN)
@@ -1556,7 +1556,7 @@ static int msm_otg_probe(struct platform_device *pdev)
np ? phy : usb_phy_clk);
if (IS_ERR(motg-phy_reset_clk)) {
dev_err(pdev-dev, failed to get usb_phy_clk\n);
- return PTR_ERR(motg-phy_reset_clk);
+ motg-phy_reset_clk = NULL;
}
motg-clk = devm_clk_get(pdev-dev, np ? core : usb_hs_clk);
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v1 3/3] usb: phy: msm: Do not do runtime pm if the phy is not idle
Use case is when the phy is configured in host mode and a usb device is attached to board before bootup. On bootup, with the existing code and runtime pm enabled, the driver would decrement the pm usage count without checking the current state of the phy. This pm usage count decrement would trigger the runtime pm which than would abort the usb enumeration which was in progress. In my case a usb stick gets detected and then immediatly the driver goes to low power mode which is not correct. log: [1.631412] msm_hsusb_host 1252.usb: EHCI Host Controller [1.636556] msm_hsusb_host 1252.usb: new USB bus registered, assigned bus number 1 [1.642563] msm_hsusb_host 1252.usb: irq 220, io mem 0x1252 [1.658197] msm_hsusb_host 1252.usb: USB 2.0 started, EHCI 1.00 [1.659473] hub 1-0:1.0: USB hub found [1.663415] hub 1-0:1.0: 1 port detected ... [1.973352] usb 1-1: new high-speed USB device number 2 using msm_hsusb_host [2.107707] usb-storage 1-1:1.0: USB Mass Storage device detected [2.108993] scsi0 : usb-storage 1-1:1.0 [2.678341] msm_otg 1252.phy: USB in low power mode [3.168977] usb 1-1: USB disconnect, device number 2 This issue was detected on IFC6410 board. This patch fixes the intial runtime pm trigger by checking the phy state and decrementing the pm use count only when the phy state is IDLE. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org --- drivers/usb/phy/phy-msm-usb.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c index 3bb559d..78cc870 100644 --- a/drivers/usb/phy/phy-msm-usb.c +++ b/drivers/usb/phy/phy-msm-usb.c @@ -1229,7 +1229,9 @@ static void msm_otg_sm_work(struct work_struct *w) motg-chg_state = USB_CHG_STATE_UNDEFINED; motg-chg_type = USB_INVALID_CHARGER; } - pm_runtime_put_sync(otg-phy-dev); + + if (otg-phy-state == OTG_STATE_B_IDLE) + pm_runtime_put_sync(otg-phy-dev); break; case OTG_STATE_B_PERIPHERAL: dev_dbg(otg-phy-dev, OTG_STATE_B_PERIPHERAL state\n); -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/5] Add support for SW babble Control
Hi, On Mon, May 26, 2014 at 02:50:07PM +0530, George Cherian wrote: Series add support for SW babble control logic found in new silicon versions of AM335x. Runtime differentiation of silicon version is done by checking the BABBLE_CTL register. For newer silicon the register default value read is 0x4 and for older versions its 0x0. Patch 1 - Handle Babble only if MUSB is in HOST mode Patch 2 - Convert recover work to delayed work. Patch 3 - usb_phy_vbus_(off/_on) are NOPs for am335x PHY so use usb_phy(_shutdown/_init) in musb_platform_reset() Patch 4 - Add return value for musb_platform_reset() in prepration to support SW babble_ctrl Patch 5 - Add and Enable sw babble control for newer silicon v5 - v6 : Squash patch 5 and 6 form v5 to avoid build warnings. v4 - v5 : Added a debug print before resetting MUSB. changed a musb_readb to dsps_readb introduced in Patch#5 of v4. v3 - v4 : Fixes an issue in gagdet mode - BUS RESET should not be handled as a BABBLE. Added a check for the same.(Patch #1) Enable sw babble control properly (Patch #6) v2 - v3 : Modify musb_platform_reset() to return zero on success. this series doesn't apply anymore, can you please rebase on top of my testing/next which I just pushed ? Thank you ps: while doing that, please also add Bin's Tested-by which he has already given ;-) -- balbi signature.asc Description: Digital signature
Re: [Patch v7 3/3] usb: dwc3: qcom: Add device tree binding
On Jun 30, 2014, at 11:03 AM, Andy Gross agr...@codeaurora.org wrote:
From: Ivan T. Ivanov iiva...@mm-sol.com
QCOM USB3.0 core wrapper consist of USB3.0 IP from Synopsys
(SNPS) and HS, SS PHY's control and configuration registers.
It could operate in device mode (SS, HS, FS) and host
mode (SS, HS, FS, LS).
Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
Signed-off-by: Andy Gross agr...@codeaurora.org
---
.../devicetree/bindings/usb/qcom,dwc3.txt | 104
1 file changed, 104 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/qcom,dwc3.txt
diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
new file mode 100644
index 000..105b6b7
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
@@ -0,0 +1,104 @@
+Qualcomm SuperSpeed DWC3 USB SoC controller
+
+
+QCOM DWC3 Highspeed USB PHY
+
+Required properities:
+- compatible:should contain qcom,dwc3-hsphy;
+- reg: offset and length of the register set in the
memory map
+- clocks:A list of phandle + clock-specifier pairs for the
+ clocks listed in clock-names
+- clock-names: Should contain the following:
+ utmi UTMI clock
+- v1p8-supply: phandle to the regulator for the 1.8v supply to HSPHY.
+- v3p3-supply: phandle to the regulator for the 3.3v supply to HSPHY.
+- vbus-supply: phandle to the regulator for the vbus supply for host
+ mode.
+- vddcx-supply: phandle to the regulator for the vdd supply for HSPHY
+digital circuit operation.
+
+Optional clocks:
+ xo External reference clock
+
+
+QCOM DWC3 Superspeed USB PHY
+=
+Required properities:
+- compatible:should contain qcom,dwc3-ssphy;
+- reg: offset and length of the register set in the
memory map
+- clocks:A list of phandle + clock-specifier pairs for the
+ clocks listed in clock-names
+- clock-names: Should contain the following:
+ ref Reference clock used in host mode.
+- v1p8-supply: phandle to the regulator for the 1.8v supply to HSPHY.
+- vddcx-supply: phandle to the regulator for the vdd supply for HSPHY
+digital circuit operation.
+
+Optional clocks:
+ xo External reference clock
+
+QCOM DWC3 controller wrapper
+===
+Required properties:
+- compatible:should contain qcom,dwc3
+- clocks:A list of phandle + clock-specifier pairs for the
+ clocks listed in clock-names
+- clock-names: Should contain the following:
+ core Master/Core clock, have to be = 125 MHz for SS
+ operation and = 60MHz for HS operation
+
+Optional clocks:
+ ifaceSystem bus AXI clock. Not present on all platforms
+ sleepSleep clock, used when USB3 core goes into low
+ power mode (U3).
We should encode the an optional port number here in the cases that we have
multiple controllers and need to know about it so we can set TCSR correctly.
+
+Optional regulator:
+- gdsc-supply: phandle to the regulator from globally distributed
+ switch controller
+
+Required child node:
+A child node must exist to represent the core DWC3 IP block. The name of
+the node is not important. The content of the node is defined in dwc3.txt.
+
+Example device nodes:
+
+ hs_phy_0: phy@110f8800 {
+ compatible = qcom,dwc3-hsphy;
+ reg = 0x110f8800 0x30;
+ clocks = gcc USB30_0_UTMI_CLK;
+ clock-names = utmi;
+
+ status = disabled;
+ };
+
+ ss_phy_0: phy@110f8830 {
+ compatible = qcom,dwc3-ssphy;
+ reg = 0x110f8830 0x30;
+
+ clocks = gcc USB30_0_MASTER_CLK;
+ clock-names = ref;
+
+ status = disabled;
+ };
+
+ usb3_0: usb30@0 {
+ compatible = qcom,dwc3;
+ #address-cells = 1;
+ #size-cells = 1;
+ clocks = gcc USB30_0_MASTER_CLK;
+ clock-names = core;
+
+ ranges;
+
+ status = disabled;
+
+ dwc3@1100 {
+ compatible = snps,dwc3;
+ reg = 0x1100 0xcd00;
+ interrupts = 0 110 0x4;
+ usb-phy = hs_phy_0, ss_phy_0;
+
Re: [PATCH v3 4/4] usb: gadget: pxa27x_udc: use devm_* helpers
On Mon, Jun 30, 2014 at 12:31:19AM +0400, Sergei Shtylyov wrote: Hello. On 06/29/2014 06:01 PM, Robert Jarzmik wrote: Use devm_* helpers in the probe function to simplify the error path and the remove path. Signed-off-by: Robert Jarzmik robert.jarz...@free.fr Cc: Sergei Shtylyov sergei.shtyl...@cogentembedded.com --- Since V1: Addressed Sergei's comments --- drivers/usb/gadget/pxa27x_udc.c | 57 +++-- 1 file changed, 15 insertions(+), 42 deletions(-) diff --git a/drivers/usb/gadget/pxa27x_udc.c b/drivers/usb/gadget/pxa27x_udc.c index f814795..04aced8 100644 --- a/drivers/usb/gadget/pxa27x_udc.c +++ b/drivers/usb/gadget/pxa27x_udc.c @@ -28,12 +28,11 @@ #include linux/of_gpio.h #include asm/byteorder.h -#include mach/hardware.h #include linux/usb.h #include linux/usb/ch9.h #include linux/usb/gadget.h -#include mach/udc.h +#include linux/platform_data/pxa2xx_udc.h #include pxa27x_udc.h These seem like unrelated changes that should be in a separate patch? agreed. Should be simple to split it up. -- balbi signature.asc Description: Digital signature
Re: [PATCH v1 1/3] usb: Kconfig: make EHCI_MSM selectable for QCOM SOCs
On Mon, Jun 30, 2014 at 06:29:34PM +0100, Srinivas Kandagatla wrote: This patch makes the msm ehci driver available to use on QCOM SOCs, which have the same IP. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org Alan, this is ok to me: Acked-by: Felipe Balbi ba...@ti.com --- drivers/usb/host/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 61b7817..03314f8 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -176,7 +176,7 @@ config USB_EHCI_HCD_AT91 config USB_EHCI_MSM tristate Support for Qualcomm QSD/MSM on-chip EHCI USB controller - depends on ARCH_MSM + depends on ARCH_MSM || ARCH_QCOM select USB_EHCI_ROOT_HUB_TT ---help--- Enables support for the USB Host controller present on the -- 1.9.1 -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 0/2] usb musb/cppi41: Address issues with isochronous audio endpoints
Hi, On Fri, Jun 20, 2014 at 09:14:34AM +0530, George Cherian wrote: On 6/20/2014 3:50 AM, Daniel Mack wrote: Hi, I've been debugging issues with musb in host mode and both full-speed and high-speed USB audio devices with cppi41 DMA mode enabled. The effect that was observed with full-speed devices was that CPU load went up to 100% due to the dma channels dma_completion work struct. For FS devices, the MUSB_TXCSR_TXPKTRDY bit that signals the FIFO's emptyness takes a long time to be cleared, and if the worker function determines that it's still set, it will re-queue the work immediately, which effectively results in a busy poll that renders the system unusable. There are audible crackles on the output, and every bit of extra load will distort the audio stream even more. The work struct was introduced by 1af54b7a40 (usb: musb: musb_cppi41: Handle ISOCH differently and not use the hrtimer.), apparantly in order to mitigate an unreliable behaviour of the driver. Geroge, do you recall which problems you saw, which device you tested with and what the effect of this patch was for you? I'm asking because I suspect the issue in fact lies in the hrtimer interval setup and can hence be fixed well without the extra worker. Sebastian's patch to implement that timer (a655f481d: usb: musb: musb_cppi41: handle pre-mature TX complete interrupt) expressed some uncertainty about the chosen value of 140us, and suspected that there's a relation between the dma burst size and the minimum time value. Hence, I'm sending two patches. The first one reverts 1af54b7a40 as it causes trouble with FS audio devices, and the seconds addresses the issue by tweaking the hrtimer settings instead. This seems to work fine with both FS and HS audio devices now. George, could you give this a try with your original test case? Yes, I would do by this weekend and update. Sorry that am lil busy with some trainings. Do we have any new developments here ? -- balbi signature.asc Description: Digital signature
Re: [PATCH v1 3/3] usb: phy: msm: Do not do runtime pm if the phy is not idle
On Mon, Jun 30, 2014 at 06:29:57PM +0100, Srinivas Kandagatla wrote: Use case is when the phy is configured in host mode and a usb device is attached to board before bootup. On bootup, with the existing code and runtime pm enabled, the driver would decrement the pm usage count without checking the current state of the phy. This pm usage count decrement would trigger the runtime pm which than would abort the usb enumeration which was in progress. In my case a usb stick gets detected and then immediatly the driver goes to low power mode which is not correct. log: [1.631412] msm_hsusb_host 1252.usb: EHCI Host Controller [1.636556] msm_hsusb_host 1252.usb: new USB bus registered, assigned bus number 1 [1.642563] msm_hsusb_host 1252.usb: irq 220, io mem 0x1252 [1.658197] msm_hsusb_host 1252.usb: USB 2.0 started, EHCI 1.00 [1.659473] hub 1-0:1.0: USB hub found [1.663415] hub 1-0:1.0: 1 port detected ... [1.973352] usb 1-1: new high-speed USB device number 2 using msm_hsusb_host [2.107707] usb-storage 1-1:1.0: USB Mass Storage device detected [2.108993] scsi0 : usb-storage 1-1:1.0 [2.678341] msm_otg 1252.phy: USB in low power mode [3.168977] usb 1-1: USB disconnect, device number 2 This issue was detected on IFC6410 board. This patch fixes the intial runtime pm trigger by checking the phy state and decrementing the pm use count only when the phy state is IDLE. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org --- drivers/usb/phy/phy-msm-usb.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c index 3bb559d..78cc870 100644 --- a/drivers/usb/phy/phy-msm-usb.c +++ b/drivers/usb/phy/phy-msm-usb.c @@ -1229,7 +1229,9 @@ static void msm_otg_sm_work(struct work_struct *w) motg-chg_state = USB_CHG_STATE_UNDEFINED; motg-chg_type = USB_INVALID_CHARGER; } - pm_runtime_put_sync(otg-phy-dev); + + if (otg-phy-state == OTG_STATE_B_IDLE) + pm_runtime_put_sync(otg-phy-dev); break; case OTG_STATE_B_PERIPHERAL: dev_dbg(otg-phy-dev, OTG_STATE_B_PERIPHERAL state\n); does this need to go on this -rc or can it wait until v3.17 merge window? Also, how far back should this be backported ? -- balbi signature.asc Description: Digital signature
Re: [PATCH v1 3/3] usb: phy: msm: Do not do runtime pm if the phy is not idle
On 30/06/14 18:56, Felipe Balbi wrote: On Mon, Jun 30, 2014 at 06:29:57PM +0100, Srinivas Kandagatla wrote: Use case is when the phy is configured in host mode and a usb device is attached to board before bootup. On bootup, with the existing code and runtime pm enabled, the driver would decrement the pm usage count without checking the current state of the phy. This pm usage count decrement would trigger the runtime pm which than would abort the usb enumeration which was in progress. In my case a usb stick gets detected and then immediatly the driver goes to low power mode which is not correct. log: [1.631412] msm_hsusb_host 1252.usb: EHCI Host Controller [1.636556] msm_hsusb_host 1252.usb: new USB bus registered, assigned bus number 1 [1.642563] msm_hsusb_host 1252.usb: irq 220, io mem 0x1252 [1.658197] msm_hsusb_host 1252.usb: USB 2.0 started, EHCI 1.00 [1.659473] hub 1-0:1.0: USB hub found [1.663415] hub 1-0:1.0: 1 port detected ... [1.973352] usb 1-1: new high-speed USB device number 2 using msm_hsusb_host [2.107707] usb-storage 1-1:1.0: USB Mass Storage device detected [2.108993] scsi0 : usb-storage 1-1:1.0 [2.678341] msm_otg 1252.phy: USB in low power mode [3.168977] usb 1-1: USB disconnect, device number 2 This issue was detected on IFC6410 board. This patch fixes the intial runtime pm trigger by checking the phy state and decrementing the pm use count only when the phy state is IDLE. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org --- drivers/usb/phy/phy-msm-usb.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c index 3bb559d..78cc870 100644 --- a/drivers/usb/phy/phy-msm-usb.c +++ b/drivers/usb/phy/phy-msm-usb.c @@ -1229,7 +1229,9 @@ static void msm_otg_sm_work(struct work_struct *w) motg-chg_state = USB_CHG_STATE_UNDEFINED; motg-chg_type = USB_INVALID_CHARGER; } - pm_runtime_put_sync(otg-phy-dev); + + if (otg-phy-state == OTG_STATE_B_IDLE) + pm_runtime_put_sync(otg-phy-dev); break; case OTG_STATE_B_PERIPHERAL: dev_dbg(otg-phy-dev, OTG_STATE_B_PERIPHERAL state\n); does this need to go on this -rc or can it wait until v3.17 merge Getting it to rc would be nice :-), but I will leave it up to you. window? Also, how far back should this be backported ? This patch is required if runtime pm is activated. --srini -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2][for 3.16 0/3] Gadget directory cleanup
Hi, On Tue, Jun 17, 2014 at 02:19:14PM +0200, Andrzej Pietrasiewicz wrote: This is a follow-up to this thread: http://www.spinics.net/lists/linux-usb/msg107611.html This refactoring has been accepted well, so I am sending it once 3.16-rc1 is out in order to make it easy to apply it. The cleanup rearranges the way source code files are located in the drivers/usb/gadget directory. New subdirectories are introduced: - udc for UDC chip drivers - function for actual usb functions' implementations - legacy for gadgets compiled as statically composed modules while at the gadget's root level there are files related to the composite framework. These are the reasons for doing such a change: - there are ~130 source code files already in a clean tree - when all gadgets and some udc drivers are built as modules then, including the by-products of the build (*.o, *.ko, *.mod.c, .*.cmd), the number easily grows to ~500 files - files serving different purpose (udc chip drivers, actual gadget functions' implementations, legacy gadgets) are located side-by-side which might cause confusion - gadget's Kconfig and Makefile tend to be lengthy. After the patches are applied the gadget subdirectory looks much cleaner. Rebased onto Greg's usb-next. please rebase on my testing/next, doesn't apply. -- balbi signature.asc Description: Digital signature
Re: [PATCH v1 1/3] usb: Kconfig: make EHCI_MSM selectable for QCOM SOCs
On Mon, 30 Jun 2014, Felipe Balbi wrote: On Mon, Jun 30, 2014 at 06:29:34PM +0100, Srinivas Kandagatla wrote: This patch makes the msm ehci driver available to use on QCOM SOCs, which have the same IP. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org Alan, this is ok to me: Acked-by: Felipe Balbi ba...@ti.com Greg, it's okay with me too. Acked-by: Alan Stern st...@rowland.harvard.edu --- drivers/usb/host/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 61b7817..03314f8 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -176,7 +176,7 @@ config USB_EHCI_HCD_AT91 config USB_EHCI_MSM tristate Support for Qualcomm QSD/MSM on-chip EHCI USB controller - depends on ARCH_MSM + depends on ARCH_MSM || ARCH_QCOM select USB_EHCI_ROOT_HUB_TT ---help--- Enables support for the USB Host controller present on the -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch V3 0/2] usb: gadget: zero: Add support for interrupt EP
Hi, On Mon, Jun 09, 2014 at 10:44:38AM +0530, Amit Virdi wrote: Felipe, Alan, On 5/23/2014 12:01 PM, Amit VIRDI wrote: This patchset adds support for interrupt EP and the corresponding test cases to gadget zero. The code has been rebased and tested on Kernel v3.15-rc5 V2 - V3 - Rectified wMaxPacketSize for FS from 1023 to 64 - Modified default value of interrupt interval for FS to 1 ms V1 - V2 - Modified the alternate interface from having 6 EPs (2 - Interrupt, 2 Bulk, 2 Isoc) to 2 EPs (Interrupt only) RFC - V1 - Added support for configuring interrupt EP attributes from configfs interface Amit Virdi (2): usb: gadget: zero: Add support for interrupt EP usbtest: Add interrupt EP testcases Any comment on this patchset? can you send some logs of this patchset working ? What output should we expect ? How to run it ? (sure, we could just read the source code and figure it out, but it's good 'documentation' to put on the commit log) cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: gadget: add claimed field in struct usb_ep
On Tue, Jun 24, 2014 at 02:16:35PM +0200, Robert Baldyga wrote: On 06/23/2014 08:27 PM, Felipe Balbi wrote: Hi, On Mon, Jun 23, 2014 at 08:07:43AM +0200, Robert Baldyga wrote: On 06/19/2014 05:08 PM, Felipe Balbi wrote: On Mon, Jun 16, 2014 at 10:20:36AM +0200, Robert Baldyga wrote: This field allows to mark ep as claimed in more clear way. Claiming endpoint by setting driver_data to non-null value is leaky solution and makes code unreadable. how come ? How can it be unreadable ? how can it be leaky ? What if gadget will not assign any value to driver_data (just like Gadget Zero do)? Endpoint will be seen as not used, and autoconfig will huh ??? The gadget isn't the endpoint user, the function is. Look at f_sourcesink.c and f_loopback.c. If the function doesn't set anything to driver_data, then that's a bug on the function which needs fixing. Moreover, if there's a function which doesn't set driver_data, we could just as well have a function which doesn't set claimed, so the problem is the same. I mean the function, not the gadget. Sorry for confusion. Mechanism I developed marks endpoint as claimed *inside autoconfig function*. It's significant difference, because there's not possible to forget to mark that endpoint is claimed. When endpoint is returned from autoconfig function, it belongs to function, and the function doesn't need to do anything to claim obtained endpoint - it's already done. we still might need to keep driver_data though. Some functions might need it. But now that you explained your goal, I can see how that might help. Please send a complete patchset also with the implementation for ep_autoconfig so we can all review your idea. cheers -- balbi signature.asc Description: Digital signature
[GIT PULL] USB fixes for v3.16-rc4
Hi Greg, Here's my second set of fixes. Note the revert of the patch Michal asked to revert. Please consider merging to your usb-linus branch. Let me know if you want any changes to this pull request. cheers The following changes since commit 5d881802c407d83c169c875dad88fe2bba066c33: usb: musb: core: Handle Babble condition only in HOST mode (2014-06-19 15:43:07 -0500) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git tags/fixes-for-v3.16-rc4 for you to fetch changes up to 8035691365b80428c58908215d4408559afe7cb3: usb: musb: dsps: fix the base address for accessing the mode register (2014-06-30 13:31:48 -0500) usb: fixes for v3.16-rc4 A few more fixes for this RC cycle. There's a revert of a previous patch which ended up being the wrong version, so we reverted that commit and applied a better fix. CPPI41 got a race condition fix which was found by Thomas Gleixner. The MSM PHY driver got a runtime pm usage fix so that it wouldn't kill the PHY while it was still being used. We also have a fix for a panic caused when removing musb_am335x driver. Other than that, a few other minor fixes. Signed-of-by: Felipe Balbi ba...@ti.com Andreas Larsson (1): usb: gadget: gr_udc: Fix check for invalid number of microframes Ezequiel Garcia (1): usb: musb: Fix panic upon musb_am335x module removal Felipe Balbi (1): Revert tools: ffs-test: convert to new descriptor format fixing compilation error Lothar Waßmann (1): usb: musb: dsps: fix the base address for accessing the mode register Michal Nazarewicz (2): usb: gadget: f_fs: resurect usb_functionfs_descs_head structure tools: ffs-test: fix header values endianess Srinivas Kandagatla (1): usb: phy: msm: Do not do runtime pm if the phy is not idle Thomas Gleixner (1): usb: musb: Ensure that cppi41 timer gets armed on premature DMA TX irq drivers/usb/gadget/gr_udc.c | 5 +++-- drivers/usb/musb/musb_am335x.c | 23 ++- drivers/usb/musb/musb_cppi41.c | 2 +- drivers/usb/musb/musb_dsps.c| 9 - drivers/usb/phy/phy-msm-usb.c | 4 +++- include/uapi/linux/usb/functionfs.h | 9 - tools/usb/Makefile | 6 +- tools/usb/ffs-test.c| 24 8 files changed, 30 insertions(+), 52 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 05/12] usb: phy: add the Berlin USB PHY driver
On Mon, Jun 30, 2014 at 04:52:11PM +0200, Antoine Ténart wrote: Felipe, On Fri, Jun 27, 2014 at 06:04:33PM -0500, Felipe Balbi wrote: On Fri, Jun 27, 2014 at 06:05:57PM +0200, Antoine Ténart wrote: Hi Felipe, On Fri, Jun 27, 2014 at 10:56:22AM -0500, Felipe Balbi wrote: On Tue, Jun 24, 2014 at 12:35:14PM +0200, Antoine Ténart wrote: Add the driver driving the Marvell Berlin USB PHY. This allows to initialize the PHY and to use it from the USB driver later. Signed-off-by: Antoine Ténart antoine.ten...@free-electrons.com since this is a brand new driver, it should go to drivers/phy instead. This PHY is used by a ChipIdea USB driver, which uses the provided common function for ChipIdea. These functions use the usb_phy framework. That's why this PHY driver is there. right, but you can add support for the new PHY layer which would help other users convert their PHY drivers to the new PHY framework. Adding the support for the new PHY layer in the common CI code is not complicated, but these functions also use some parts from usb hcd or usb otg which are only usb_phy compatible. Shouldn't these parts add the new PHY support first, to avoid ending up with a fairly big series, quite long to do and complicated to review? sure, patches are welcome. :-) -- balbi signature.asc Description: Digital signature
Re: [RFC/PATCH] usb: musb: Prevent musb_am335x from being removed
Hi, On Thu, May 08, 2014 at 02:27:40PM -0300, Ezequiel Garcia wrote: At probe time, the musb_am335x driver registers its childs by calling of_platform_populate(), which registers all childs in the devicetree hierarchy recursively. On the other side, the driver's remove() function uses of_device_unregister() to remove each child in the musb_am335x driver device struct. However, when musb_dsps is loaded, its devices are attached to the musb_am335x driver device struct as musb_am335x childs. Hence, musb_am335x remove() will attempt to unregister the devices registered by musb_dsps, which produces a kernel panic. In other words, the childs in the struct device hierarchy are not the same as the childs in the devicetree hierarchy. Ideally, we should be enforcing the removal of the devices registered by musb_am335x *only*, instead of all the child devices. However, because of the recursive nature of of_platform_populate, this doesn't seem possible. Therefore, as the only solution at hand, this commit disables musb_am335x driver removal capability, preventing it from being ever removed. Just for reference, here's the panic upon module removal: musb-hdrc musb-hdrc.0.auto: remove, state 4 usb usb1: USB disconnect, device number 1 musb-hdrc musb-hdrc.0.auto: USB bus 1 deregistered Unable to handle kernel NULL pointer dereference at virtual address 008c pgd = de11c000 [008c] *pgd=9e174831, *pte=, *ppte= Internal error: Oops: 17 [#1] ARM Modules linked in: musb_am335x(-) musb_dsps musb_hdrc usbcore usb_common CPU: 0 PID: 623 Comm: modprobe Not tainted 3.15.0-rc4-1-g24efd13 #69 task: de1b7500 ti: de122000 task.ti: de122000 PC is at am335x_shutdown+0x10/0x28 LR is at am335x_shutdown+0xc/0x28 pc : [c0327798]lr : [c0327794]psr: a013 sp : de123df8 ip : 0004 fp : 00028f00 r10: r9 : de122000 r8 : c000e6c4 r7 : de0e3c10 r6 : de0e3800 r5 : de624010 r4 : de1ec750 r3 : de0e3810 r2 : r1 : 0001 r0 : Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user Control: 10c5387d Table: 9e11c019 DAC: 0015 Process modprobe (pid: 623, stack limit = 0xde122240) Stack: (0xde123df8 to 0xde124000) 3de0: de0e3810 bf054488 3e00: bf05444c de624010 6013 bf043650 12fc de624010 de0e3810 bf043a20 3e20: de0e3810 bf04b240 c0635b88 c02ca37c c02ca364 c02c8db0 de1b7500 de0e3844 3e40: de0e3810 c02c8e28 c0635b88 de02824c de0e3810 c02c884c de0e3800 de0e3810 3e60: de0e3818 c02c5b20 bf05417c de0e3800 de0e3800 c0635b88 de0f2410 c02ca838 3e80: bf05417c de0e3800 bf055438 c02ca8cc de0e3c10 bf054194 de0e3c10 c02ca37c 3ea0: c02ca364 c02c8db0 de1b7500 de0e3c44 de0e3c10 c02c8e28 c0635b88 de02824c 3ec0: de0e3c10 c02c884c de0e3c10 de0e3c10 de0e3c18 c02c5b20 de0e3c10 de0e3c10 3ee0: bf059000 a013 c02c5bc0 bf05900c de0e3c10 c02c5c48 3f00: de0dd0c0 de1ec970 de0f2410 bf05929c de0f2444 bf05902c de0f2410 c02ca37c 3f20: c02ca364 c02c8db0 bf05929c de0f2410 bf05929c c02c94c8 bf05929c 3f40: 0800 c02c8ab4 bf0592e0 c007fc40 c00dd820 6273756d 336d615f 00783533 3f60: c064a0ac de1b7500 de122000 de1b7500 c000e590 0001 c000e6c4 c0060160 3f80: 00028e70 00028e70 00028ea4 0081 6010 00028e70 00028e70 00028ea4 3fa0: 0081 c000e500 00028e70 00028e70 00028ea4 0800 becb59f8 00027608 3fc0: 00028e70 00028e70 00028ea4 0081 0001 0001 00028f00 3fe0: b6e6b6f0 becb59d4 000160e8 b6e6b6fc 6010 00028ea4 [c0327798] (am335x_shutdown) from [bf054488] (dsps_musb_exit+0x3c/0x4c [musb_dsps]) [bf054488] (dsps_musb_exit [musb_dsps]) from [bf043650] (musb_shutdown+0x80/0x90 [musb_hdrc]) [bf043650] (musb_shutdown [musb_hdrc]) from [bf043a20] (musb_remove+0x24/0x68 [musb_hdrc]) [bf043a20] (musb_remove [musb_hdrc]) from [c02ca37c] (platform_drv_remove+0x18/0x1c) [c02ca37c] (platform_drv_remove) from [c02c8db0] (__device_release_driver+0x70/0xc8) [c02c8db0] (__device_release_driver) from [c02c8e28] (device_release_driver+0x20/0x2c) [c02c8e28] (device_release_driver) from [c02c884c] (bus_remove_device+0xdc/0x10c) [c02c884c] (bus_remove_device) from [c02c5b20] (device_del+0x104/0x198) [c02c5b20] (device_del) from [c02ca838] (platform_device_del+0x14/0x9c) [c02ca838] (platform_device_del) from [c02ca8cc] (platform_device_unregister+0xc/0x20) [c02ca8cc] (platform_device_unregister) from [bf054194] (dsps_remove+0x18/0x38 [musb_dsps]) [bf054194] (dsps_remove [musb_dsps]) from [c02ca37c] (platform_drv_remove+0x18/0x1c) [c02ca37c] (platform_drv_remove) from [c02c8db0] (__device_release_driver+0x70/0xc8) [c02c8db0] (__device_release_driver) from [c02c8e28] (device_release_driver+0x20/0x2c) [c02c8e28] (device_release_driver) from [c02c884c] (bus_remove_device+0xdc/0x10c) [c02c884c] (bus_remove_device) from [c02c5b20] (device_del+0x104/0x198) [c02c5b20]
Re: [RFC/PATCH] usb: musb: Prevent musb_am335x from being removed
On 30 June 2014 15:44, Felipe Balbi ba...@ti.com wrote: Hi, On Thu, May 08, 2014 at 02:27:40PM -0300, Ezequiel Garcia wrote: At probe time, the musb_am335x driver registers its childs by calling of_platform_populate(), which registers all childs in the devicetree hierarchy recursively. On the other side, the driver's remove() function uses of_device_unregister() to remove each child in the musb_am335x driver device struct. However, when musb_dsps is loaded, its devices are attached to the musb_am335x driver device struct as musb_am335x childs. Hence, musb_am335x remove() will attempt to unregister the devices registered by musb_dsps, which produces a kernel panic. In other words, the childs in the struct device hierarchy are not the same as the childs in the devicetree hierarchy. Ideally, we should be enforcing the removal of the devices registered by musb_am335x *only*, instead of all the child devices. However, because of the recursive nature of of_platform_populate, this doesn't seem possible. Therefore, as the only solution at hand, this commit disables musb_am335x driver removal capability, preventing it from being ever removed. Just for reference, here's the panic upon module removal: musb-hdrc musb-hdrc.0.auto: remove, state 4 usb usb1: USB disconnect, device number 1 musb-hdrc musb-hdrc.0.auto: USB bus 1 deregistered Unable to handle kernel NULL pointer dereference at virtual address 008c pgd = de11c000 [008c] *pgd=9e174831, *pte=, *ppte= Internal error: Oops: 17 [#1] ARM Modules linked in: musb_am335x(-) musb_dsps musb_hdrc usbcore usb_common CPU: 0 PID: 623 Comm: modprobe Not tainted 3.15.0-rc4-1-g24efd13 #69 task: de1b7500 ti: de122000 task.ti: de122000 PC is at am335x_shutdown+0x10/0x28 LR is at am335x_shutdown+0xc/0x28 pc : [c0327798]lr : [c0327794]psr: a013 sp : de123df8 ip : 0004 fp : 00028f00 r10: r9 : de122000 r8 : c000e6c4 r7 : de0e3c10 r6 : de0e3800 r5 : de624010 r4 : de1ec750 r3 : de0e3810 r2 : r1 : 0001 r0 : Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user Control: 10c5387d Table: 9e11c019 DAC: 0015 Process modprobe (pid: 623, stack limit = 0xde122240) Stack: (0xde123df8 to 0xde124000) 3de0: de0e3810 bf054488 3e00: bf05444c de624010 6013 bf043650 12fc de624010 de0e3810 bf043a20 3e20: de0e3810 bf04b240 c0635b88 c02ca37c c02ca364 c02c8db0 de1b7500 de0e3844 3e40: de0e3810 c02c8e28 c0635b88 de02824c de0e3810 c02c884c de0e3800 de0e3810 3e60: de0e3818 c02c5b20 bf05417c de0e3800 de0e3800 c0635b88 de0f2410 c02ca838 3e80: bf05417c de0e3800 bf055438 c02ca8cc de0e3c10 bf054194 de0e3c10 c02ca37c 3ea0: c02ca364 c02c8db0 de1b7500 de0e3c44 de0e3c10 c02c8e28 c0635b88 de02824c 3ec0: de0e3c10 c02c884c de0e3c10 de0e3c10 de0e3c18 c02c5b20 de0e3c10 de0e3c10 3ee0: bf059000 a013 c02c5bc0 bf05900c de0e3c10 c02c5c48 3f00: de0dd0c0 de1ec970 de0f2410 bf05929c de0f2444 bf05902c de0f2410 c02ca37c 3f20: c02ca364 c02c8db0 bf05929c de0f2410 bf05929c c02c94c8 bf05929c 3f40: 0800 c02c8ab4 bf0592e0 c007fc40 c00dd820 6273756d 336d615f 00783533 3f60: c064a0ac de1b7500 de122000 de1b7500 c000e590 0001 c000e6c4 c0060160 3f80: 00028e70 00028e70 00028ea4 0081 6010 00028e70 00028e70 00028ea4 3fa0: 0081 c000e500 00028e70 00028e70 00028ea4 0800 becb59f8 00027608 3fc0: 00028e70 00028e70 00028ea4 0081 0001 0001 00028f00 3fe0: b6e6b6f0 becb59d4 000160e8 b6e6b6fc 6010 00028ea4 [c0327798] (am335x_shutdown) from [bf054488] (dsps_musb_exit+0x3c/0x4c [musb_dsps]) [bf054488] (dsps_musb_exit [musb_dsps]) from [bf043650] (musb_shutdown+0x80/0x90 [musb_hdrc]) [bf043650] (musb_shutdown [musb_hdrc]) from [bf043a20] (musb_remove+0x24/0x68 [musb_hdrc]) [bf043a20] (musb_remove [musb_hdrc]) from [c02ca37c] (platform_drv_remove+0x18/0x1c) [c02ca37c] (platform_drv_remove) from [c02c8db0] (__device_release_driver+0x70/0xc8) [c02c8db0] (__device_release_driver) from [c02c8e28] (device_release_driver+0x20/0x2c) [c02c8e28] (device_release_driver) from [c02c884c] (bus_remove_device+0xdc/0x10c) [c02c884c] (bus_remove_device) from [c02c5b20] (device_del+0x104/0x198) [c02c5b20] (device_del) from [c02ca838] (platform_device_del+0x14/0x9c) [c02ca838] (platform_device_del) from [c02ca8cc] (platform_device_unregister+0xc/0x20) [c02ca8cc] (platform_device_unregister) from [bf054194] (dsps_remove+0x18/0x38 [musb_dsps]) [bf054194] (dsps_remove [musb_dsps]) from [c02ca37c] (platform_drv_remove+0x18/0x1c) [c02ca37c] (platform_drv_remove) from [c02c8db0] (__device_release_driver+0x70/0xc8) [c02c8db0] (__device_release_driver) from [c02c8e28] (device_release_driver+0x20/0x2c) [c02c8e28] (device_release_driver) from [c02c884c] (bus_remove_device+0xdc/0x10c) [c02c884c] (bus_remove_device) from
Re: [RFC/PATCH] usb: musb: Prevent musb_am335x from being removed
On Mon, Jun 30, 2014 at 03:50:23PM -0300, Ezequiel García wrote: On 30 June 2014 15:44, Felipe Balbi ba...@ti.com wrote: Hi, On Thu, May 08, 2014 at 02:27:40PM -0300, Ezequiel Garcia wrote: At probe time, the musb_am335x driver registers its childs by calling of_platform_populate(), which registers all childs in the devicetree hierarchy recursively. On the other side, the driver's remove() function uses of_device_unregister() to remove each child in the musb_am335x driver device struct. However, when musb_dsps is loaded, its devices are attached to the musb_am335x driver device struct as musb_am335x childs. Hence, musb_am335x remove() will attempt to unregister the devices registered by musb_dsps, which produces a kernel panic. In other words, the childs in the struct device hierarchy are not the same as the childs in the devicetree hierarchy. Ideally, we should be enforcing the removal of the devices registered by musb_am335x *only*, instead of all the child devices. However, because of the recursive nature of of_platform_populate, this doesn't seem possible. Therefore, as the only solution at hand, this commit disables musb_am335x driver removal capability, preventing it from being ever removed. Just for reference, here's the panic upon module removal: musb-hdrc musb-hdrc.0.auto: remove, state 4 usb usb1: USB disconnect, device number 1 musb-hdrc musb-hdrc.0.auto: USB bus 1 deregistered Unable to handle kernel NULL pointer dereference at virtual address 008c pgd = de11c000 [008c] *pgd=9e174831, *pte=, *ppte= Internal error: Oops: 17 [#1] ARM Modules linked in: musb_am335x(-) musb_dsps musb_hdrc usbcore usb_common CPU: 0 PID: 623 Comm: modprobe Not tainted 3.15.0-rc4-1-g24efd13 #69 task: de1b7500 ti: de122000 task.ti: de122000 PC is at am335x_shutdown+0x10/0x28 LR is at am335x_shutdown+0xc/0x28 pc : [c0327798]lr : [c0327794]psr: a013 sp : de123df8 ip : 0004 fp : 00028f00 r10: r9 : de122000 r8 : c000e6c4 r7 : de0e3c10 r6 : de0e3800 r5 : de624010 r4 : de1ec750 r3 : de0e3810 r2 : r1 : 0001 r0 : Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user Control: 10c5387d Table: 9e11c019 DAC: 0015 Process modprobe (pid: 623, stack limit = 0xde122240) Stack: (0xde123df8 to 0xde124000) 3de0: de0e3810 bf054488 3e00: bf05444c de624010 6013 bf043650 12fc de624010 de0e3810 bf043a20 3e20: de0e3810 bf04b240 c0635b88 c02ca37c c02ca364 c02c8db0 de1b7500 de0e3844 3e40: de0e3810 c02c8e28 c0635b88 de02824c de0e3810 c02c884c de0e3800 de0e3810 3e60: de0e3818 c02c5b20 bf05417c de0e3800 de0e3800 c0635b88 de0f2410 c02ca838 3e80: bf05417c de0e3800 bf055438 c02ca8cc de0e3c10 bf054194 de0e3c10 c02ca37c 3ea0: c02ca364 c02c8db0 de1b7500 de0e3c44 de0e3c10 c02c8e28 c0635b88 de02824c 3ec0: de0e3c10 c02c884c de0e3c10 de0e3c10 de0e3c18 c02c5b20 de0e3c10 de0e3c10 3ee0: bf059000 a013 c02c5bc0 bf05900c de0e3c10 c02c5c48 3f00: de0dd0c0 de1ec970 de0f2410 bf05929c de0f2444 bf05902c de0f2410 c02ca37c 3f20: c02ca364 c02c8db0 bf05929c de0f2410 bf05929c c02c94c8 bf05929c 3f40: 0800 c02c8ab4 bf0592e0 c007fc40 c00dd820 6273756d 336d615f 00783533 3f60: c064a0ac de1b7500 de122000 de1b7500 c000e590 0001 c000e6c4 c0060160 3f80: 00028e70 00028e70 00028ea4 0081 6010 00028e70 00028e70 00028ea4 3fa0: 0081 c000e500 00028e70 00028e70 00028ea4 0800 becb59f8 00027608 3fc0: 00028e70 00028e70 00028ea4 0081 0001 0001 00028f00 3fe0: b6e6b6f0 becb59d4 000160e8 b6e6b6fc 6010 00028ea4 [c0327798] (am335x_shutdown) from [bf054488] (dsps_musb_exit+0x3c/0x4c [musb_dsps]) [bf054488] (dsps_musb_exit [musb_dsps]) from [bf043650] (musb_shutdown+0x80/0x90 [musb_hdrc]) [bf043650] (musb_shutdown [musb_hdrc]) from [bf043a20] (musb_remove+0x24/0x68 [musb_hdrc]) [bf043a20] (musb_remove [musb_hdrc]) from [c02ca37c] (platform_drv_remove+0x18/0x1c) [c02ca37c] (platform_drv_remove) from [c02c8db0] (__device_release_driver+0x70/0xc8) [c02c8db0] (__device_release_driver) from [c02c8e28] (device_release_driver+0x20/0x2c) [c02c8e28] (device_release_driver) from [c02c884c] (bus_remove_device+0xdc/0x10c) [c02c884c] (bus_remove_device) from [c02c5b20] (device_del+0x104/0x198) [c02c5b20] (device_del) from [c02ca838] (platform_device_del+0x14/0x9c) [c02ca838] (platform_device_del) from [c02ca8cc] (platform_device_unregister+0xc/0x20) [c02ca8cc] (platform_device_unregister) from [bf054194] (dsps_remove+0x18/0x38 [musb_dsps]) [bf054194] (dsps_remove [musb_dsps]) from [c02ca37c] (platform_drv_remove+0x18/0x1c) [c02ca37c] (platform_drv_remove) from [c02c8db0]
Re: [PATCH v2 0/2] usb musb/cppi41: Address issues with isochronous audio endpoints
On 06/30/2014 07:54 PM, Felipe Balbi wrote: Hi, On Fri, Jun 20, 2014 at 09:14:34AM +0530, George Cherian wrote: On 6/20/2014 3:50 AM, Daniel Mack wrote: Hi, I've been debugging issues with musb in host mode and both full-speed and high-speed USB audio devices with cppi41 DMA mode enabled. The effect that was observed with full-speed devices was that CPU load went up to 100% due to the dma channels dma_completion work struct. For FS devices, the MUSB_TXCSR_TXPKTRDY bit that signals the FIFO's emptyness takes a long time to be cleared, and if the worker function determines that it's still set, it will re-queue the work immediately, which effectively results in a busy poll that renders the system unusable. There are audible crackles on the output, and every bit of extra load will distort the audio stream even more. The work struct was introduced by 1af54b7a40 (usb: musb: musb_cppi41: Handle ISOCH differently and not use the hrtimer.), apparantly in order to mitigate an unreliable behaviour of the driver. Geroge, do you recall which problems you saw, which device you tested with and what the effect of this patch was for you? I'm asking because I suspect the issue in fact lies in the hrtimer interval setup and can hence be fixed well without the extra worker. Sebastian's patch to implement that timer (a655f481d: usb: musb: musb_cppi41: handle pre-mature TX complete interrupt) expressed some uncertainty about the chosen value of 140us, and suspected that there's a relation between the dma burst size and the minimum time value. Hence, I'm sending two patches. The first one reverts 1af54b7a40 as it causes trouble with FS audio devices, and the seconds addresses the issue by tweaking the hrtimer settings instead. This seems to work fine with both FS and HS audio devices now. George, could you give this a try with your original test case? Yes, I would do by this weekend and update. Sorry that am lil busy with some trainings. Do we have any new developments here ? FWIW, I now got back positive results from Sebastian, who tests these patches extensively. Daniel signature.asc Description: OpenPGP digital signature
[PATCH] usb: gadget: s3c2410: Move to clk_prepare_enable/clk_disable_unprepare
Use clk_prepare_enable/clk_disable_unprepare to make the driver
work properly with common clock framework.
Signed-off-by: Vasily Khoruzhick anars...@gmail.com
---
drivers/usb/gadget/s3c2410_udc.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/s3c2410_udc.c b/drivers/usb/gadget/s3c2410_udc.c
index 7987aa0..357b58e 100644
--- a/drivers/usb/gadget/s3c2410_udc.c
+++ b/drivers/usb/gadget/s3c2410_udc.c
@@ -1788,7 +1788,7 @@ static int s3c2410_udc_probe(struct platform_device *pdev)
return PTR_ERR(usb_bus_clock);
}
- clk_enable(usb_bus_clock);
+ clk_prepare_enable(usb_bus_clock);
udc_clock = clk_get(NULL, usb-device);
if (IS_ERR(udc_clock)) {
@@ -1796,7 +1796,7 @@ static int s3c2410_udc_probe(struct platform_device *pdev)
return PTR_ERR(udc_clock);
}
- clk_enable(udc_clock);
+ clk_prepare_enable(udc_clock);
mdelay(10);
@@ -1952,13 +1952,13 @@ static int s3c2410_udc_remove(struct platform_device
*pdev)
release_mem_region(rsrc_start, rsrc_len);
if (!IS_ERR(udc_clock) udc_clock != NULL) {
- clk_disable(udc_clock);
+ clk_disable_unprepare(udc_clock);
clk_put(udc_clock);
udc_clock = NULL;
}
if (!IS_ERR(usb_bus_clock) usb_bus_clock != NULL) {
- clk_disable(usb_bus_clock);
+ clk_disable_unprepare(usb_bus_clock);
clk_put(usb_bus_clock);
usb_bus_clock = NULL;
}
--
2.0.0
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/7] dma: cppi41: handle 0-length packets
Hi Felipe, Glad to see you find some time for this patch set :) On 06/30/2014 07:19 PM, Felipe Balbi wrote: On Mon, May 26, 2014 at 02:52:34PM +0200, Daniel Mack wrote: When a 0-length packet is received on the bus, desc-pd0 yields 1, which confuses the driver's users. This information is clearly wrong and not in accordance to the datasheet, but it's been observed on an AM335x board, very reproducible. Fix this by looking at bit 19 in PD2 of the completed packet. This bit will tell us if a zero-length packet was received on a queue. If it's set, ignore the value in PD0 and report a total length of 0 instead. Signed-off-by: Daniel Mack zon...@gmail.com Does this create dependency on the following patches ? Since they're in the same series, I suppose it does... Please clarify so we don't cause regressions upstream due to ordering. No, there's no dependency. I sent them in one series as they all contribute to the fact that multiple USB devices would behave incorrectly on musb. All of these patches have to be applied to fix different details, but for this particular one, the order doesn't matter. I also asked Vinod to pick this single patch for his tree, but there was no answer yet. Thanks, Daniel signature.asc Description: OpenPGP digital signature
Re: [PATCH v4 1/7] dma: cppi41: handle 0-length packets
Hi, On Mon, Jun 30, 2014 at 09:19:08PM +0200, Daniel Mack wrote: Hi Felipe, Glad to see you find some time for this patch set :) yeah, have been quite busy lately. On 06/30/2014 07:19 PM, Felipe Balbi wrote: On Mon, May 26, 2014 at 02:52:34PM +0200, Daniel Mack wrote: When a 0-length packet is received on the bus, desc-pd0 yields 1, which confuses the driver's users. This information is clearly wrong and not in accordance to the datasheet, but it's been observed on an AM335x board, very reproducible. Fix this by looking at bit 19 in PD2 of the completed packet. This bit will tell us if a zero-length packet was received on a queue. If it's set, ignore the value in PD0 and report a total length of 0 instead. Signed-off-by: Daniel Mack zon...@gmail.com Does this create dependency on the following patches ? Since they're in the same series, I suppose it does... Please clarify so we don't cause regressions upstream due to ordering. No, there's no dependency. I sent them in one series as they all contribute to the fact that multiple USB devices would behave incorrectly on musb. All of these patches have to be applied to fix different details, but for this particular one, the order doesn't matter. awesome, thanks I also asked Vinod to pick this single patch for his tree, but there was no answer yet. cool. hopefully we can get a better v3.17. -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 0/2] usb musb/cppi41: Address issues with isochronous audio endpoints
On Mon, Jun 30, 2014 at 09:16:01PM +0200, Daniel Mack wrote: On 06/30/2014 07:54 PM, Felipe Balbi wrote: Hi, On Fri, Jun 20, 2014 at 09:14:34AM +0530, George Cherian wrote: On 6/20/2014 3:50 AM, Daniel Mack wrote: Hi, I've been debugging issues with musb in host mode and both full-speed and high-speed USB audio devices with cppi41 DMA mode enabled. The effect that was observed with full-speed devices was that CPU load went up to 100% due to the dma channels dma_completion work struct. For FS devices, the MUSB_TXCSR_TXPKTRDY bit that signals the FIFO's emptyness takes a long time to be cleared, and if the worker function determines that it's still set, it will re-queue the work immediately, which effectively results in a busy poll that renders the system unusable. There are audible crackles on the output, and every bit of extra load will distort the audio stream even more. The work struct was introduced by 1af54b7a40 (usb: musb: musb_cppi41: Handle ISOCH differently and not use the hrtimer.), apparantly in order to mitigate an unreliable behaviour of the driver. Geroge, do you recall which problems you saw, which device you tested with and what the effect of this patch was for you? I'm asking because I suspect the issue in fact lies in the hrtimer interval setup and can hence be fixed well without the extra worker. Sebastian's patch to implement that timer (a655f481d: usb: musb: musb_cppi41: handle pre-mature TX complete interrupt) expressed some uncertainty about the chosen value of 140us, and suspected that there's a relation between the dma burst size and the minimum time value. Hence, I'm sending two patches. The first one reverts 1af54b7a40 as it causes trouble with FS audio devices, and the seconds addresses the issue by tweaking the hrtimer settings instead. This seems to work fine with both FS and HS audio devices now. George, could you give this a try with your original test case? Yes, I would do by this weekend and update. Sorry that am lil busy with some trainings. Do we have any new developments here ? FWIW, I now got back positive results from Sebastian, who tests these patches extensively. cool, I'll add these to my 'testing/next' branch. -- balbi signature.asc Description: Digital signature
Re: [PATCH v4 1/7] dma: cppi41: handle 0-length packets
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/30/2014 09:26 PM, Felipe Balbi wrote: On Mon, Jun 30, 2014 at 09:19:08PM +0200, Daniel Mack wrote: I also asked Vinod to pick this single patch for his tree, but there was no answer yet. cool. hopefully we can get a better v3.17. Ultimately it's your decision of course, but I really think the patches sould make it for v3.16, as they fix real bugs. Thanks, Daniel -BEGIN PGP SIGNATURE- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJTsbqGAAoJELphMRr8Y1QkjBUP/1V8FYuPpNgapV7wbvUND8fE cXckUSPkH9n1M9v6+7Jz/aRJ6KJGy5GBmyJU29v26wq1JlkvrD7o3iOtwPU5JzZr NjaHVkaBF4/8JKiCvOokKf+NiN07BBQdxJjbUaX2Wx0qaBR/2vaI98jFIRLQ8fe0 uTvUpJ1wUTHEppcMfPGEWtLVYJRN9ygQHwZ2XvfAo6wnRHu25/q0skMsy1hL40m6 eAR/dkl6eCC+e6HMR16OEjFizHbeZ1lHOeN95oTwznpIXafboPsap4/ZwePl/jOn 9Bw6q0nEZIE/QoTnWSRnRPo5JXNmGBA4xbB0pzHVA1eodd4GkkXlPQzWRy0UfdWP ouw+806aLIBBNEfXgvXSxzDkkWaVFf0jhYkQt3SNGnbrLClwmGSuownhhR6bYTih FOQoCenU3sR+4VNGvFDjConso2KxHIlxVkS4Zsc2hgpRNiEnrrxJaIGBmPGJgW6W x5MEp1446uegxHdzrkAXvo7Bqkh7SbsncdrymQbFgs/+PiT5hmL3LC8zfaOrqcR+ yOe848gZ2+qASEIxH9XypTGZWLCjvLrKaq0U1NvQRYCUdjNB2qHu5aR0ZHxCFJBT CMiwovnCzRDfbaSwjROj8QLsslAItXPm64i/ubuJYRVz+CZdIj327n/mo1tBzQfN KGxhT1iDNEgxkiiGwDnh =KIqc -END PGP SIGNATURE- -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/10] net2280: Support for PLX USB338x and cleanout code
Hello Felipe Any comment on this patchset? Shall I resend it rebased on v3.16-rc2? Regards On Tue, Jun 10, 2014 at 10:19 PM, Ricardo Ribalda Delgado ricardo.riba...@gmail.com wrote: Ping... On Tue, May 20, 2014 at 6:30 PM, Ricardo Ribalda Delgado ricardo.riba...@gmail.com wrote: The USB 3380 is a PCI Express Gen 2 to USB 3.0 SuperSpeed Peripheral Controller. It shares a lot functionatily with the net228x family. This series of patches includes some resend of previous patches. Please check the changelog for every patch. This time it has been placed under the tear line Ricardo Ribalda Delgado (10): usb: gadget: net2280: Add support for PLX USB338X usb: gadget: net2280: Dont use magic numbers usb: gadget: net2280: Use BIT() macro usb: gadget: net2280: Use true/false instead of 1/0 usb: gadget: net2280: Use module_pci_driver macro usb: gadget: net2280: Refactor queues_show usb: gadget: net2280: Pass checkpacth.pl test usb: gadget: net2280: Code Cleanup usb: gadget: net2280: Use pr_* function usb: gadget: net2280: Use quirks instead of pci id drivers/usb/gadget/Kconfig | 10 +- drivers/usb/gadget/net2280.c | 2752 -- drivers/usb/gadget/net2280.h | 285 +++-- include/linux/usb/usb338x.h | 199 +++ 4 files changed, 2235 insertions(+), 1011 deletions(-) create mode 100644 include/linux/usb/usb338x.h -- 2.0.0.rc2 -- Ricardo Ribalda -- Ricardo Ribalda -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/7] dma: cppi41: handle 0-length packets
Hi, On Mon, Jun 30, 2014 at 09:29:10PM +0200, Daniel Mack wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/30/2014 09:26 PM, Felipe Balbi wrote: On Mon, Jun 30, 2014 at 09:19:08PM +0200, Daniel Mack wrote: I also asked Vinod to pick this single patch for his tree, but there was no answer yet. cool. hopefully we can get a better v3.17. Ultimately it's your decision of course, but I really think the patches sould make it for v3.16, as they fix real bugs. only patchs 4 and 6 seem to be -rc material. Will it work if I apply only those during this -rc ? -- balbi signature.asc Description: Digital signature
Re: [PATCH 00/10] net2280: Support for PLX USB338x and cleanout code
On Mon, Jun 30, 2014 at 09:30:49PM +0200, Ricardo Ribalda Delgado wrote: Hello Felipe Any comment on this patchset? Shall I resend it rebased on v3.16-rc2? they are all in my 'testing/next' branch. Should be available on 'next' next week if nothing wrong is found. cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH v4 1/7] dma: cppi41: handle 0-length packets
On 06/30/2014 09:31 PM, Felipe Balbi wrote: On Mon, Jun 30, 2014 at 09:29:10PM +0200, Daniel Mack wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/30/2014 09:26 PM, Felipe Balbi wrote: On Mon, Jun 30, 2014 at 09:19:08PM +0200, Daniel Mack wrote: I also asked Vinod to pick this single patch for his tree, but there was no answer yet. cool. hopefully we can get a better v3.17. Ultimately it's your decision of course, but I really think the patches sould make it for v3.16, as they fix real bugs. only patchs 4 and 6 seem to be -rc material. Will it work if I apply only those during this -rc ? Hmm, I'd say #3 and #5 are -rc material, too. #3 fixes a reproducible kernel crash when unplugging streaming audio devices, and #5 causes data stream starvation which makes using many Wifi adapters with musb impossible, #1 should better go through Vinod's tree, and #2 and #7 are just cosmetic things. Daniel -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] usb: gadget: pxa27x_udc: use devm_* helpers
Felipe Balbi ba...@ti.com writes: @@ -28,12 +28,11 @@ #include linux/of_gpio.h #include asm/byteorder.h -#include mach/hardware.h #include linux/usb.h #include linux/usb/ch9.h #include linux/usb/gadget.h -#include mach/udc.h +#include linux/platform_data/pxa2xx_udc.h #include pxa27x_udc.h These seem like unrelated changes that should be in a separate patch? agreed. Should be simple to split it up. Yeah, sure. -- Robert -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v2] HID: leds: Use attribute-groups in MSI GT683R driver
On Mon, Jun 30, 2014 at 4:33 AM, Jiri Kosina jkos...@suse.cz wrote: On Mon, 30 Jun 2014, Johan Hovold wrote: I think the better place is HID/input tree, since this patch depends on the initial one which is not in my tree. I'm going to merge Johan's whole patchset and this patch probably depends Johan's work too. Dmitry has ACKed the input-patch and Bryan has applied that one and the leds-patches to his tree (of which the first one is a dependency of this patch). Jiri, are you saying that the gt683r-driver should go in through his tree as well, that is all three patches including the first that you have already applied? I just assumed your for-next branch was immutable, but perhaps I was mistaken. Well, for-next branch is a collection of all the topic branches I am queuing for the following merge window. I am never really rebasing it, but I can definitely not include 'for-3.17/hid-gt683r' topic branch in the pile I will be sending to Linus (all the scheduled branches are getting merged into 'for-linus' only when merge window open). So the only potential conflict between hid.git and Bryan's tree would be in linux-next (and probably there will be none, git can handle duplicate patches nicely). So once Bryan confirms he's queued it (please preserve my Signoff from my tree), then I will just not include for-3.17/hid-gt683r branch in pull request to Linus and all is fine. I'm OK to merge Janne's first patch for HID GT683R through my tree with you guys' SOB. I'm also OK to merge this incremental patchset here. Please confirm it if I didn't misunderstand here. Also Janne or someone, can you post the original first patch to me or point me where is it? Thanks, -Bryan -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3] USB: ehci-pci: USB host controller support for Intel Quark X1000
/*
-*/
+#define PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC0x0939
+static inline bool is_intel_quark_x1000(struct pci_dev *pdev) {
+ return pdev-vendor == PCI_VENDOR_ID_INTEL
+ pdev-device == PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC;
+}
Why not just put this check inline into ehci_pci_reinit()?
Alan Stern said it is not a problem, I think so also since it just a inline
subroutine.
+
+/*
+ * The offset of in/out threshold register is 0x84.
+ * And it is the register of 'hostpc'
+ * in memory-mapped EHCI controller.
+*/
The preferred multi-line kernel style is this:
/*
* bla
* bla
*/
I will improve it.
+#defineintel_quark_x1000_insnreg01 hostpc
+
+/* The maximal ehci packet buffer size is 512 bytes */
s/ehci/EHCI/
+#define INTEL_QUARK_X1000_EHCI_MAX_PACKET_BUFFER_SIZE 512
+
+/* The threshold value set the register is in DWORD */
+#define INTEL_QUARK_X1000_EHCI_THRESHOLD(size) ((size)/4u)
+#define INTEL_QUARK_X1000_EHCI_THRESHOLD_OUT_SHIFT 16
+#define INTEL_QUARK_X1000_EHCI_THRESHOLD_IN_SHIFT 0
+
Too many empty lines...
/* called after powerup, by probe or system-pm wakeup */
static int ehci_pci_reinit(struct ehci_hcd *ehci, struct pci_dev *pdev)
{
int retval;
+ u32 val;
+ u32 thr;
Why not declare these where they are used?
All will be removed as Alan Stern's suggestion.
+ /* Reset the threshold limit */
+ if (is_intel_quark_x1000(pdev)) {
+ /*
+ * In order to support the isochronous/interrupt
+ * transactions, 508 bytes should be used as
+ * max threshold values to maximize the
+ * performance
+ */
Same comment about the comment style...
+ thr = INTEL_QUARK_X1000_EHCI_THRESHOLD(
+ INTEL_QUARK_X1000_EHCI_MAX_PACKET_BUFFER_SIZE - 4
+ );
+ val = thrINTEL_QUARK_X1000_EHCI_THRESHOLD_OUT_SHIFT |
+ thrINTEL_QUARK_X1000_EHCI_THRESHOLD_IN_SHIFT;
Please surround with spaces for consistency.
The above code will be removed as Alan Stern's suggestion.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3] USB: ehci-pci: USB host controller support for Intel Quark X1000
/*
-*/
+#define PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC0x0939
+static inline bool is_intel_quark_x1000(struct pci_dev *pdev) {
+ return pdev-vendor == PCI_VENDOR_ID_INTEL
+ pdev-device == PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC;
+}
Whether to put this test directly into ehci_pci_reset() or leave it as a
separate
subroutine is up to you. I don't care either way.
I will just keep it.
+
+/*
+ * The offset of in/out threshold register is 0x84.
+ * And it is the register of 'hostpc'
+ * in memory-mapped EHCI controller.
+*/
0x84 is the same as offset of the hostpc register in the Intel Moorestown
controller. hostpc is not present in general EHCI controllers.
OK, I will improve the comments.
+#defineintel_quark_x1000_insnreg01 hostpc
+
+/* The maximal ehci packet buffer size is 512 bytes */
+#define INTEL_QUARK_X1000_EHCI_MAX_PACKET_BUFFER_SIZE 512
+
+/* The threshold value set the register is in DWORD */
+#define INTEL_QUARK_X1000_EHCI_THRESHOLD(size) ((size)/4u)
+#define INTEL_QUARK_X1000_EHCI_THRESHOLD_OUT_SHIFT 16
+#define INTEL_QUARK_X1000_EHCI_THRESHOLD_IN_SHIFT 0
+
/* called after powerup, by probe or system-pm wakeup */ static
int ehci_pci_reinit(struct ehci_hcd *ehci, struct pci_dev *pdev) {
int retval;
+ u32 val;
+ u32 thr;
/* we expect static quirk code to handle the extended capabilities
* (currently just BIOS handoff) allowed starting with EHCI 0.96 @@
-50,6 +74,22 @@ static int ehci_pci_reinit(struct ehci_hcd *ehci, struct
pci_dev *pdev)
if (!retval)
ehci_dbg(ehci, MWI active\n);
+ /* Reset the threshold limit */
+ if (is_intel_quark_x1000(pdev)) {
+ /*
+ * In order to support the isochronous/interrupt
+ * transactions, 508 bytes should be used as
+ * max threshold values to maximize the
+ * performance
+ */
+ thr = INTEL_QUARK_X1000_EHCI_THRESHOLD(
+ INTEL_QUARK_X1000_EHCI_MAX_PACKET_BUFFER_SIZE - 4
+ );
+ val = thrINTEL_QUARK_X1000_EHCI_THRESHOLD_OUT_SHIFT |
+ thrINTEL_QUARK_X1000_EHCI_THRESHOLD_IN_SHIFT;
+ ehci_writel(ehci, val, ehci-regs-intel_quark_x1000_insnreg01);
I saw what other people told you about the original patch version, and I
disagree with them. It is not necessary to include a detailed calculation
like
this, it only makes the code harder to read. It will be better to have a
single
#define with a comment explaining it, like
this:
/* Maximum usable threshold value is 0x7f dwords for both IN and OUT */
#define INTEL_QUARK_X1000_EHCI_MAX_THRESHOLD 0x007f007f
Then here, just use INTEL_QUARK_X1000_EHCI_MAX_THRESHOLD instead of
val. The comment can simply say:
/*
* For the Intel QUARK X1000, raise the I/O threshold to the
* maximum usable value in order to improve performance.
*/
I think so also. It is not necessary to make so complicated. I will adopt your
suggestions, it is more simple and clearly.
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 07/12] usb: chipidea: add a generic driver
On Mon, Jun 30, 2014 at 03:33:13PM +0200, Antoine Ténart wrote:
Peter,
On Fri, Jun 27, 2014 at 11:25:07AM +0800, Peter Chen wrote:
On Tue, Jun 24, 2014 at 12:35:16PM +0200, Antoine Ténart wrote:
ifneq ($(CONFIG_OF),)
obj-$(CONFIG_USB_CHIPIDEA) += usbmisc_imx.o ci_hdrc_imx.o
+ obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_generic.o
endif
As a generic driver, you may need to support both dt and non-dt
solution.
Since the dt is now the best practice and since there is no need (yet)
for a non-dt usage of this driver shouldn't we let anyone needing it
implement it when the time comes?
No, at least your code structure should support both dt and non-dt,
and let the compile pass for non-dt platform if you don't have one.
Then, someone with non-dt platform can change few to support it.
A good example is: drivers/usb/host/ehci-platform.c
+static int ci_hdrc_generic_probe(struct platform_device *pdev)
+{
+ struct device *dev = pdev-dev;
+ struct ci_hdrc_generic_priv *priv;
+ struct ci_hdrc_platform_data ci_pdata = {
+ .name = ci_hdrc,
How about this using dev_name(pdev-dev) for name?
Yes, why not. I don't have a strong preference.
+
+clk_err:
+ clk_disable_unprepare(priv-clk);
You may need to add if (!IS_ERR(priv-clk))
Right! I'll update this.
+
+static const struct of_device_id ci_hdrc_generic_of_match[] = {
+ { .compatible = chipidea-usb },
+ { }
+};
Even as a generic driver, you can also use your own compatible string.
Well, there is nothing specific about the Berlin CI. Some subsystems
use the 'generic' keyword in these cases. Do you see a particular reason
I should use some Berlin related compatible here?
Not must, one suggestion is: can you change the compatible string
to chipidea-usb-generic?
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] usb: dwc3: Kconfig: Fix dependency for Exynos DWC3
On Mon, Jun 30, 2014 at 10:19 PM, Felipe Balbi ba...@ti.com wrote: On Mon, Jun 30, 2014 at 02:33:14PM +0530, Sachin Kamat wrote: Make the config depend on ARCH_EXYNOS5 instead of ARCH_EXYNOS as this IP is available only on Exynos5 platforms. Signed-off-by: Sachin Kamat sachin.ka...@samsung.com --- drivers/usb/dwc3/Kconfig |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig index 261c3b428220..24a4f8ed9fd1 100644 --- a/drivers/usb/dwc3/Kconfig +++ b/drivers/usb/dwc3/Kconfig @@ -55,7 +55,7 @@ config USB_DWC3_OMAP config USB_DWC3_EXYNOS tristate Samsung Exynos Platform - depends on ARCH_EXYNOS || COMPILE_TEST + depends on ARCH_EXYNOS5 || COMPILE_TEST what happens when you have exynos[6789] ? Are we going to continually increase this line ? To me this is a pointless change. I am not sure if the IP would remain same in future Exynos SoCs. However for what we know for now is that this IP exists only on Exynos5 and it makes sense that the dependency reflect the same. I am not introducing any new dependency, just making the existing one accurate. If you feel this is a pointless change, please feel free to drop this patch. Regards, Sachin. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 0/6] usb: gadget: Delete __init marker for probe
On Sun, May 18, 2014 at 03:19:01PM +0800, Peter Chen wrote: The probe function may be probed deferal and called after .init section has freed. Peter Chen (6): usb: gadget: atmel_usba_udc: delete __init marker for probe usb: gadget: fsl_udc_core: delete __init marker for probe usb: gadget: lpc32xx: delete __init marker for probe usb: gadget: m66592-udc: delete __init marker for probe usb: gadget: fusb300_udc: delete __init marker for probe usb: gadget: r8a66597-udc: delete __init marker for probe drivers/usb/gadget/atmel_usba_udc.c |2 +- drivers/usb/gadget/fsl_udc_core.c |6 +++--- drivers/usb/gadget/fusb300_udc.c|2 +- drivers/usb/gadget/lpc32xx_udc.c|2 +- drivers/usb/gadget/m66592-udc.c |2 +- drivers/usb/gadget/r8a66597-udc.c |4 ++-- 6 files changed, 9 insertions(+), 9 deletions(-) -- 1.7.8 Hi Felipe, How about the this patch set? -- ping again... Peter -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 0/5] usb: gadget: cleanup gadget driver .unbind usage at udc driver
-Original Message- From: Peter Chen [mailto:peter.c...@freescale.com] Sent: Saturday, June 21, 2014 9:41 PM To: ba...@ti.com Cc: linux-usb@vger.kernel.org Subject: Re: [PATCH 0/5] usb: gadget: cleanup gadget driver .unbind usage at udc driver On Wed, May 21, 2014 at 09:04:16AM +0800, Peter Chen wrote: Hi Felipe, This patch set cleans up unnecessary .unbind calling at udc driver, the udc core covers gadget driver's .unbind calling well. Peter Chen (5): usb: gadget: fsl_udc_core: should not call gadget driver's .unbind usb: gadget: fusb300_udc: should not call gadget driver's .unbind usb: gadget: m66592-udc: should not call gadget driver's .unbind usb: gadget: net2272: do not need to judge gadget driver's .unbind usb: gadget: omap_udc: should not call gadget driver's .unbind drivers/usb/gadget/fsl_udc_core.c |1 - drivers/usb/gadget/fusb300_udc.c |2 -- drivers/usb/gadget/m66592-udc.c |2 -- drivers/usb/gadget/net2272.c |2 +- drivers/usb/gadget/omap_udc.c |5 + 5 files changed, 2 insertions(+), 10 deletions(-) -- 1.7.8 Hi Felipe, How about this patch set? ping again. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] USB fixes for v3.16-rc4
On Mon, Jun 30, 2014 at 01:40:35PM -0500, Felipe Balbi wrote: Hi Greg, Here's my second set of fixes. Note the revert of the patch Michal asked to revert. Please consider merging to your usb-linus branch. Let me know if you want any changes to this pull request. cheers The following changes since commit 5d881802c407d83c169c875dad88fe2bba066c33: usb: musb: core: Handle Babble condition only in HOST mode (2014-06-19 15:43:07 -0500) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git tags/fixes-for-v3.16-rc4 Pulled and pushed out, thanks. greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch v7 3/3] usb: dwc3: qcom: Add device tree binding
On Mon, Jun 30, 2014 at 11:03 AM, Andy Gross agr...@codeaurora.org wrote:
From: Ivan T. Ivanov iiva...@mm-sol.com
Please copy the right lists and maintainers.
QCOM USB3.0 core wrapper consist of USB3.0 IP from Synopsys
(SNPS) and HS, SS PHY's control and configuration registers.
It could operate in device mode (SS, HS, FS) and host
mode (SS, HS, FS, LS).
Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
Signed-off-by: Andy Gross agr...@codeaurora.org
---
.../devicetree/bindings/usb/qcom,dwc3.txt | 104
1 file changed, 104 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/qcom,dwc3.txt
diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
new file mode 100644
index 000..105b6b7
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
@@ -0,0 +1,104 @@
+Qualcomm SuperSpeed DWC3 USB SoC controller
+
+
+QCOM DWC3 Highspeed USB PHY
+
+Required properities:
+- compatible: should contain qcom,dwc3-hsphy;
+- reg: offset and length of the register set in the memory
map
+- clocks: A list of phandle + clock-specifier pairs for the
+ clocks listed in clock-names
+- clock-names: Should contain the following:
+ utmi UTMI clock
+- v1p8-supply: phandle to the regulator for the 1.8v supply to HSPHY.
+- v3p3-supply: phandle to the regulator for the 3.3v supply to HSPHY.
+- vbus-supply: phandle to the regulator for the vbus supply for host
+ mode.
+- vddcx-supply: phandle to the regulator for the vdd supply for HSPHY
+digital circuit operation.
+
+Optional clocks:
+ xo External reference clock
+
+
+QCOM DWC3 Superspeed USB PHY
+=
+Required properities:
+- compatible: should contain qcom,dwc3-ssphy;
+- reg: offset and length of the register set in the memory
map
+- clocks: A list of phandle + clock-specifier pairs for the
+ clocks listed in clock-names
+- clock-names: Should contain the following:
+ refReference clock used in host mode.
+- v1p8-supply: phandle to the regulator for the 1.8v supply to HSPHY.
+- vddcx-supply: phandle to the regulator for the vdd supply for HSPHY
+digital circuit operation.
+
+Optional clocks:
+ xo External reference clock
+
+QCOM DWC3 controller wrapper
+===
+Required properties:
+- compatible: should contain qcom,dwc3
+- clocks: A list of phandle + clock-specifier pairs for the
+ clocks listed in clock-names
+- clock-names: Should contain the following:
+ core Master/Core clock, have to be = 125 MHz for SS
+ operation and = 60MHz for HS operation
+
+Optional clocks:
+ iface System bus AXI clock. Not present on all platforms
Really?, some platforms have a clockless bus?
+ sleep Sleep clock, used when USB3 core goes into low
+ power mode (U3).
+
+Optional regulator:
+- gdsc-supply: phandle to the regulator from globally distributed
+ switch controller
The name should reflect the name of the input, not the source.
+
+Required child node:
+A child node must exist to represent the core DWC3 IP block. The name of
+the node is not important. The content of the node is defined in dwc3.txt.
+
+Example device nodes:
+
+ hs_phy_0: phy@110f8800 {
+ compatible = qcom,dwc3-hsphy;
+ reg = 0x110f8800 0x30;
+ clocks = gcc USB30_0_UTMI_CLK;
+ clock-names = utmi;
+
+ status = disabled;
+ };
+
+ ss_phy_0: phy@110f8830 {
+ compatible = qcom,dwc3-ssphy;
+ reg = 0x110f8830 0x30;
+
+ clocks = gcc USB30_0_MASTER_CLK;
+ clock-names = ref;
+
+ status = disabled;
+ };
+
+ usb3_0: usb30@0 {
+ compatible = qcom,dwc3;
+ #address-cells = 1;
+ #size-cells = 1;
+ clocks = gcc USB30_0_MASTER_CLK;
+ clock-names = core;
+
+ ranges;
+
+ status = disabled;
+
+ dwc3@1100 {
+ compatible = snps,dwc3;
This sub-node is just wrong. Why can't you have a single node with '
qcom,dwc3, snps,dwc3 ' for the compatible property? All you are
adding here is clocks. Does the Synopsys block have no clocks?
I
Re: [PATCH v4 1/7] dma: cppi41: handle 0-length packets
On Mon, Jun 30, 2014 at 09:40:06PM +0200, Daniel Mack wrote: On 06/30/2014 09:31 PM, Felipe Balbi wrote: On Mon, Jun 30, 2014 at 09:29:10PM +0200, Daniel Mack wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/30/2014 09:26 PM, Felipe Balbi wrote: On Mon, Jun 30, 2014 at 09:19:08PM +0200, Daniel Mack wrote: I also asked Vinod to pick this single patch for his tree, but there was no answer yet. cool. hopefully we can get a better v3.17. Ultimately it's your decision of course, but I really think the patches sould make it for v3.16, as they fix real bugs. only patchs 4 and 6 seem to be -rc material. Will it work if I apply only those during this -rc ? Hmm, I'd say #3 and #5 are -rc material, too. #3 fixes a reproducible kernel crash when unplugging streaming audio devices, and #5 causes data stream starvation which makes using many Wifi adapters with musb impossible, #1 should better go through Vinod's tree, and #2 and #7 are just cosmetic things. Sorry been bit busy, I will pick this one and queue it for fixes. And yes, rest would make sense to go thru usb tree -- ~Vinod -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
