Re: [PATCH] staging: dwc2: fix wrong setting of DMA masks
Hi Paul,
+ /* Set device flags indicating whether the HCD supports DMA */
+ if (hsotg-core_params-dma_enable 0) {
+ if (dma_set_mask(hsotg-dev, DMA_BIT_MASK(32)) 0)
+ dev_warn(hsotg-dev,
+ can't enable workaround for 4GB RAM\n);
Did you intentionally change the workaround here? It used to be 31 bits
and 2GB RAM. Perhaps this deserves a mention in the commit message?
Other than that, the patch looks ok to me.
Gr.
Matthijs
--
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: dwc2: Host channel not always released on dequeue
Hi Paul,
I would prefer to keep that part of the code as-is for now. I haven't
quite convinced myself that the function can never be called with
halt_status = DWC2_HC_XFER_URB_DEQUEUE.
Ok. For arguments sake, it should be easy to convince yourself:
dwc2_hc_chhltd_intr is a static function only referenced from
dwc2_hc_n_intr. The latter basically does:
static void dwc2_hc_n_intr(struct dwc2_hsotg *hsotg, int chnum)
{
(...)
if (chan-halt_status == DWC2_HC_XFER_URB_DEQUEUE) {
(...)
return;
}
(...)
dwc2_hc_chhltd_intr(hsotg, chan, chnum, qtd);
(...)
}
This of course only holds provided that halt_status doesn't get set to
DWC2_HC_XFER_URB_DEQUEUE in the meanwhile, which only happens in
dwc2_hcd_urb_enqueue. AFAIU it should be safe to assume the driver
doesn't start dequeuing urbs on its own in the interrupt handler.
Why change this function? AFAICS it's never called with qtd = NULL with
the patch?
You're probably right, but I feel better adding a little defensive
programming here, just in case.
Absolutely right, I was just wondering :-)
Gr.
Matthijs
--
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 port power-off and system suspend
On 2013/3/23 1:35, Alan Stern wrote: Tianyu:
Did you ever test the port power-off mechanism with system suspend?
Right now it doesn't seem like it would work, because it relies on
runtime PM to turn off the port power, and runtime PM doesn't operate
normally during a system suspend.
Alan Stern
Hi Alan:
You are right. Current the port power-off can't work during
system suspend. I have already worked on a patch of adding usb
port system pm support. But there is a still problem that how to
control power off mechanism from kernel side. Before, we have a solution
for runtime pm that increase/decrease port dev's usage count.
http://marc.info/?l=linux-usbm=135772122031446w=2
But now, we have another user, System pm. So I think we should find
a solution to cover these two cases. Maybe add a new pm qos request
for NO_POWER_OFF flag? I'd like to see your opinion.
I have a patch that can work and it still needs to add some condition
checks in the usb_port_system_suspend(). Dev's system wakeup disable
and persist enable.
From c3abd373707910672d6fd2e96da78879b3e22417 Mon Sep 17 00:00:00 2001
From: Lan Tianyu tianyu@intel.com
Date: Tue, 26 Feb 2013 11:12:09 +0800
Subject: [PATCH] usb: Add usb port system pm support
This patch is to add usb port system pm support. Add
usb port's system suspend/resume callbacks and call
usb_port_runtime_resume/suspend() to power off these
ports without pm qos NO_POWER_OFF flag setting.
During system pm, usb port should be powered off after
dev being suspended and powered on before dev being
resumed. Usb ports and devs enable async suspend. In
order to keeping the order, call device_pm_wait_for_dev()
in the usb_port_system_suspend() and usb_port_resume().
Usb_port_system_suspend() ignores EAGAIN from usb_port_runtime_suspend().
EAGAIN is caused by pm qos NO_POWER_OFF setting. This is
not an error for usb port system pm.
---
drivers/usb/core/hub.c |4
drivers/usb/core/port.c | 31 +++
2 files changed, 35 insertions(+)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 53c1555..8868f15 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3162,6 +3162,10 @@ int usb_port_resume(struct usb_device *udev,
pm_message_t msg)
int status;
u16 portchange, portstatus;
+ /* Wait for usb port system resume finishing */
+ if (!PMSG_IS_AUTO(msg))
+ device_pm_wait_for_dev(udev-dev, port_dev-dev);
+
if (port_dev-did_runtime_put) {
status = pm_runtime_get_sync(port_dev-dev);
port_dev-did_runtime_put = false;
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 797f9d5..e5c6ec5 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -84,6 +84,9 @@ static int usb_port_runtime_resume(struct device *dev)
if (!hub)
return -EINVAL;
+ if (port_dev-power_is_on)
+ return 0;
+
usb_autopm_get_interface(intf);
set_bit(port1, hub-busy_bits);
@@ -127,6 +130,9 @@ static int usb_port_runtime_suspend(struct device *dev)
== PM_QOS_FLAGS_ALL)
return -EAGAIN;
+ if (!port_dev-power_is_on)
+ return 0;
+
usb_autopm_get_interface(intf);
set_bit(port1, hub-busy_bits);
retval = usb_hub_set_port_power(hdev, port1, false);
@@ -136,9 +142,34 @@ static int usb_port_runtime_suspend(struct device *dev)
usb_autopm_put_interface(intf);
return retval;
}
+#else
+static int usb_port_runtime_suspend(struct device *dev) { return 0; }
+static int usb_port_runtime_resume(struct device *dev) { return 0; }
#endif
+static int usb_port_system_suspend(struct device *dev)
+{
+ struct usb_port *port_dev = to_usb_port(dev);
+ int retval;
+
+ if (port_dev-child)
+ device_pm_wait_for_dev(dev, port_dev-child-dev);
+
+ retval = usb_port_runtime_suspend(dev);
+ if (retval 0 retval != -EAGAIN)
+ return retval;
+
+ return 0;
+}
+
+static int usb_port_system_resume(struct device *dev)
+{
+ return usb_port_runtime_resume(dev);
+}
+
static const struct dev_pm_ops usb_port_pm_ops = {
+ .suspend = usb_port_system_suspend,
+ .resume = usb_port_system_resume,
#ifdef CONFIG_USB_SUSPEND
.runtime_suspend = usb_port_runtime_suspend,
.runtime_resume = usb_port_runtime_resume,
--
1.7.10.4
--
Best Regards
Tianyu Lan
linux kernel enabling team
--
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] staging: dwc2: fix wrong setting of DMA masks
Hi,
On Fri, Mar 22, 2013 at 05:27:26PM -0700, Paul Zimmerman wrote:
We were setting the DMA masks in dwc2_driver_probe(), but that is
before the driver parameters have been set to their default values.
That meant the DMA masks could be set wrong. Fix it by moving the
DMA mask setting into dwc2_hcd_init(), after the driver parameters
have been set.
This required a bit of refactoring of the dwc2_hcd_init() code as
well. While there, also remove the unneeded struct device *dev
parameter to dwc2_hcd_init() and dwc2_hcd_remove(), and pass in the
value through the hsotg-dev member.
looks like this can be easily split into a proper series:
patch 1: remove struct device *dev parameter from both functions
patch 2: refactor dwc2_hcd_init()
patch 3: fix setting of DMA masks.
This will make your patches a lot easier to review and isolates the only
change which can cause a regression into its own patch. Should anything
turn out to be wrong in your DMA masks change, it can be easily reverted
without removing the other two cleanups.
diff --git a/drivers/staging/dwc2/hcd.c b/drivers/staging/dwc2/hcd.c
index 01dbdd8..1a78924 100644
--- a/drivers/staging/dwc2/hcd.c
+++ b/drivers/staging/dwc2/hcd.c
@@ -2682,7 +2682,7 @@ static void dwc2_set_uninitialized(int *p, int size)
* USB bus with the core and calls the hc_driver-start() function. It
returns
* a negative error on failure.
*/
-int dwc2_hcd_init(struct device *dev, struct dwc2_hsotg *hsotg, int irq,
+int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,
struct dwc2_core_params *params)
{
struct usb_hcd *hcd;
@@ -2691,7 +2691,7 @@ int dwc2_hcd_init(struct device *dev, struct dwc2_hsotg
*hsotg, int irq,
int i, num_channels;
int retval = -ENOMEM;
- dev_dbg(dev, DWC OTG HCD INIT\n);
+ dev_dbg(hsotg-dev, DWC OTG HCD INIT\n);
I would even have a patch before removing the parameter which just
converts dev_* messages to use hsotg-dev, then remove the parameter,
but that's just me. I really, really enjoy 'atomic' changes.
--
balbi
signature.asc
Description: Digital signature
Re: [BUG] bisected: PandaBoard smsc95xx ethernet driver error from USB timeout
On Fri, Mar 22, 2013 at 4:28 AM, Frank Rowand frank.row...@am.sony.com wrote: I play upstream kernel on Pandaboard A1 frequently, looks not see the failure problem before. Maybe the problem is config dependent. If you may share your config file, I'd like to do the test too. 3.9-rc2-20130314 doesn't have the problem observed on my Pandaboard A1, but I only tested booting from MMC, not from NFS. Thanks, -- Ming Lei -- 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
