Re: [PATCH usb-linus] USB: keyspan: fix typo causing GPF on open
Richard writes: > Bjørn: > > I patched keyspan.c using your below supplied diff in 3.6.6 (I'm not > using git.) The patch WORKS for me. (I tested using minicom and the > two programs that usually access the Keyspan serial device.) Thanks for testing. Good to know that this really was the problem you were facing. As you might have guessed, neither Johan nor I have the hardware to test this driver. So your bug report and testing is extremely valuable. > Thank you for the quick fix. And thank you for an excellent bug report and quick testing. The fact that you could describe exactly which stable releases this appeared in narrowed it down to just a single commit. And describing exactly what happened narrowed it down to just a few possible places within that commit. This was the main reason the bug could be fixed this fast. > Will this show up in 3.6.7? That's for Greg to answer. But don't be surprised if this came in too late for 3.6.7. All fixes for stable has to go through mainline first. Bjørn -- 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 enclosures seem to require read(16) with >2TB drives
On Nov 09 Elliott, Robert (Server Storage) wrote: > I recommend broadening this patch. T10 is discussing making READ (10), WRITE > (10), etc. obsolete in SBC-4 in favor of their 16-byte CDB counterparts. > > The algorithm should be: > 1. During discovery, determine if 16-byte CDBs are supported. There are > several ways to determine this: > a) REPORT SUPPORTED OPERATION CODES command succeeds and reports that READ > (16) et al are supported. > b) READ (16) command specifying a Transfer Length of zero succeeds. > c) READ CAPACITY (16) command succeeds and reports that the capacity is > 2 > TiB. > d) (future) INQUIRY command succeeds fetching the Block Device > Characteristics VPD page and notices a new field added by the SBC-4 > simplified SCSI feature set proposal. > > Since REPORT SUPPORTED OPERATION CODES is optional, it won't always work. > READ CAPACITY (16) used to be optional for < 2 TiB drives, so it won't always > work either. READ (16) will always work, but requires the drive to be spun up > beforehand (e.g., with START STOP UNIT). This won't work. It will crash badly written device firmwares. Instead, try the (10) commands on the SBC-4 device and let it respond that it does not implement these commands. Or have other means to be certain of SBC-4 compliance without issuing commands that were optional in or not defined by earlier revisions of the spec. I wonder whether testing for INQUIRY_data.VERSION >= something is a sufficiently safe test. > 2. if 16-byte CDBs are supported, then use them; only drop down to 10-byte > CDBs if 16-byte CDBs are unavailable. Don't make the decision by comparing > the LBA on every IO. -- Stefan Richter -=-===-- =-== -=-== http://arcgraph.de/sr/ -- 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 v5 0/6] solve deadlock caused by memory allocation with I/O
This patchset try to solve one deadlock problem which might be caused by memory allocation with block I/O during runtime PM and block device error handling path. Traditionly, the problem is addressed by passing GFP_NOIO statically to mm, but that is not a effective solution, see detailed description in patch 1's commit log. This patch set introduces one process flag and trys to fix the deadlock problem on block device/network device during runtime PM or usb bus reset. The 1st one is the change on include/sched.h and mm. The 2nd patch introduces the flag of memalloc_noio on 'dev_pm_info', and pm_runtime_set_memalloc_noio(), so that PM Core can teach mm to not allocate mm with GFP_IO during the runtime_resume callback only on device with the flag set. The following 2 patches apply the introduced pm_runtime_set_memalloc_noio() to mark all devices as memalloc_noio_resume in the path from the block or network device to the root device in device tree. The last 2 patches are applied again PM and USB subsystem to demonstrate how to use the introduced mechanism to fix the deadlock problem. Change logs: V5: - don't clear GFP_FS - coding style fix - add comments - see details in individual change logs V4: - patches from the 2nd to the 6th changed - call pm_runtime_set_memalloc_noio() after device_add() as pointed by Alan - set PF_MEMALLOC_NOIO during runtime_suspend() V3: - patch 2/6 and 5/6 changed, see their commit log - remove RFC from title since several guys have expressed that it is a reasonable solution V2: - remove changes on 'may_writepage' and 'may_swap'(1/6) - unset GFP_IOFS in try_to_free_pages() path(1/6) - introduce pm_runtime_set_memalloc_noio() - only apply the meachnism on block/network device and its ancestors for runtime resume context V1: - take Minchan's change to avoid the check in alloc_page hot path - change the helpers' style into save/restore as suggested by Alan - memory allocation with no io in usb bus reset path for all devices as suggested by Greg and Oliver block/genhd.c| 10 + drivers/base/power/runtime.c | 92 +- drivers/usb/core/hub.c | 13 ++ include/linux/pm.h |1 + include/linux/pm_runtime.h |3 ++ include/linux/sched.h| 22 ++ mm/page_alloc.c |9 - mm/vmscan.c |4 +- net/core/net-sysfs.c |5 +++ 9 files changed, 154 insertions(+), 5 deletions(-) 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
[PATCH v5 3/6] block/genhd.c: apply pm_runtime_set_memalloc_noio on block devices
This patch applyes the introduced pm_runtime_set_memalloc_noio on
block device so that PM core will teach mm to not allocate memory with
GFP_IOFS when calling the runtime_resume and runtime_suspend callback
for block devices and its ancestors.
Cc: Jens Axboe
Signed-off-by: Ming Lei
---
v5:
- fix code style and one typo
v4:
- call pm_runtime_set_memalloc_noio(ddev, true) after device_add
---
block/genhd.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/block/genhd.c b/block/genhd.c
index 9e02cd6..085cce4 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include "blk.h"
@@ -532,6 +533,14 @@ static void register_disk(struct gendisk *disk)
return;
}
}
+
+ /*
+* avoid probable deadlock caused by allocating memory with
+* GFP_KERNEL in runtime_resume callback of its all ancestor
+* devices
+*/
+ pm_runtime_set_memalloc_noio(ddev, true);
+
disk->part0.holder_dir = kobject_create_and_add("holders", &ddev->kobj);
disk->slave_dir = kobject_create_and_add("slaves", &ddev->kobj);
@@ -661,6 +670,7 @@ void del_gendisk(struct gendisk *disk)
disk->driverfs_dev = NULL;
if (!sysfs_deprecated)
sysfs_remove_link(block_depr, dev_name(disk_to_dev(disk)));
+ pm_runtime_set_memalloc_noio(disk_to_dev(disk), false);
device_del(disk_to_dev(disk));
}
EXPORT_SYMBOL(del_gendisk);
--
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 v5 4/6] net/core: apply pm_runtime_set_memalloc_noio on network devices
Deadlock might be caused by allocating memory with GFP_KERNEL in runtime_resume and runtime_suspend callback of network devices in iSCSI situation, so mark network devices and its ancestor as 'memalloc_noio' with the introduced pm_runtime_set_memalloc_noio(). Cc: "David S. Miller" Cc: Eric Dumazet Cc: David Decotigny Cc: Tom Herbert Cc: Ingo Molnar Signed-off-by: Ming Lei --- v4: - call pm_runtime_set_memalloc_noio(ddev, true) after device_add --- net/core/net-sysfs.c |5 + 1 file changed, 5 insertions(+) diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c index bcf02f6..a55d255 100644 --- a/net/core/net-sysfs.c +++ b/net/core/net-sysfs.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include "net-sysfs.h" @@ -1386,6 +1387,8 @@ void netdev_unregister_kobject(struct net_device * net) remove_queue_kobjects(net); + pm_runtime_set_memalloc_noio(dev, false); + device_del(dev); } @@ -1421,6 +1424,8 @@ int netdev_register_kobject(struct net_device *net) return error; } + pm_runtime_set_memalloc_noio(dev, true); + return error; } -- 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 v5 5/6] PM / Runtime: force memory allocation with no I/O during Runtime PM callbcack
This patch applies the introduced memalloc_noio_save() and
memalloc_noio_restore() to force memory allocation with no I/O
during runtime_resume/runtime_suspend callback on device with
the flag of 'memalloc_noio' set.
Cc: Alan Stern
Cc: Oliver Neukum
Cc: Rafael J. Wysocki
Signed-off-by: Ming Lei
---
v5:
- use inline memalloc_noio_save()
v4:
- runtime_suspend need this too because rpm_resume may wait for
completion of concurrent runtime_suspend, so deadlock still may
be triggered in runtime_suspend path.
---
drivers/base/power/runtime.c | 32 ++--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 3e198a0..96d99ea 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -371,6 +371,7 @@ static int rpm_suspend(struct device *dev, int rpmflags)
int (*callback)(struct device *);
struct device *parent = NULL;
int retval;
+ unsigned int noio_flag;
trace_rpm_suspend(dev, rpmflags);
@@ -480,7 +481,20 @@ static int rpm_suspend(struct device *dev, int rpmflags)
if (!callback && dev->driver && dev->driver->pm)
callback = dev->driver->pm->runtime_suspend;
- retval = rpm_callback(callback, dev);
+ /*
+* Deadlock might be caused if memory allocation with GFP_KERNEL
+* happens inside runtime_suspend callback of one block device's
+* ancestor or the block device itself. Network device might be
+* thought as part of iSCSI block device, so network device and
+* its ancestor should be marked as memalloc_noio.
+*/
+ if (dev->power.memalloc_noio) {
+ noio_flag = memalloc_noio_save();
+ retval = rpm_callback(callback, dev);
+ memalloc_noio_restore(noio_flag);
+ } else {
+ retval = rpm_callback(callback, dev);
+ }
if (retval)
goto fail;
@@ -563,6 +577,7 @@ static int rpm_resume(struct device *dev, int rpmflags)
int (*callback)(struct device *);
struct device *parent = NULL;
int retval = 0;
+ unsigned int noio_flag;
trace_rpm_resume(dev, rpmflags);
@@ -712,7 +727,20 @@ static int rpm_resume(struct device *dev, int rpmflags)
if (!callback && dev->driver && dev->driver->pm)
callback = dev->driver->pm->runtime_resume;
- retval = rpm_callback(callback, dev);
+ /*
+* Deadlock might be caused if memory allocation with GFP_KERNEL
+* happens inside runtime_resume callback of one block device's
+* ancestor or the block device itself. Network device might be
+* thought as part of iSCSI block device, so network device and
+* its ancestor should be marked as memalloc_noio.
+*/
+ if (dev->power.memalloc_noio) {
+ noio_flag = memalloc_noio_save();
+ retval = rpm_callback(callback, dev);
+ memalloc_noio_restore(noio_flag);
+ } else {
+ retval = rpm_callback(callback, dev);
+ }
if (retval) {
__update_runtime_status(dev, RPM_SUSPENDED);
pm_runtime_cancel_pending(dev);
--
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 v5 6/6] USB: forbid memory allocation with I/O during bus reset
If one storage interface or usb network interface(iSCSI case)
exists in current configuration, memory allocation with
GFP_KERNEL during usb_device_reset() might trigger I/O transfer
on the storage interface itself and cause deadlock because
the 'us->dev_mutex' is held in .pre_reset() and the storage
interface can't do I/O transfer when the reset is triggered
by other interface, or the error handling can't be completed
if the reset is triggered by the storage itself(error handling path).
Cc: Alan Stern
Cc: Oliver Neukum
Signed-off-by: Ming Lei
---
v5:
- use inline memalloc_noio_save()
v4:
- mark current memalloc_noio for every usb device reset
---
drivers/usb/core/hub.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 90accde..2d5cc1c 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -5040,6 +5040,7 @@ int usb_reset_device(struct usb_device *udev)
{
int ret;
int i;
+ unsigned int noio_flag;
struct usb_host_config *config = udev->actconfig;
if (udev->state == USB_STATE_NOTATTACHED ||
@@ -5049,6 +5050,17 @@ int usb_reset_device(struct usb_device *udev)
return -EINVAL;
}
+ /*
+* Don't allocate memory with GFP_KERNEL in current
+* context to avoid possible deadlock if usb mass
+* storage interface or usbnet interface(iSCSI case)
+* is included in current configuration. The easist
+* approach is to do it for every device reset,
+* because the device 'memalloc_noio' flag may have
+* not been set before reseting the usb device.
+*/
+ noio_flag = memalloc_noio_save();
+
/* Prevent autosuspend during the reset */
usb_autoresume_device(udev);
@@ -5093,6 +5105,7 @@ int usb_reset_device(struct usb_device *udev)
}
usb_autosuspend_device(udev);
+ memalloc_noio_restore(noio_flag);
return ret;
}
EXPORT_SYMBOL_GPL(usb_reset_device);
--
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 v5 1/6] mm: teach mm by current context info to not do I/O during memory allocation
This patch introduces PF_MEMALLOC_NOIO on process flag('flags' field of
'struct task_struct'), so that the flag can be set by one task
to avoid doing I/O inside memory allocation in the task's context.
The patch trys to solve one deadlock problem caused by block device,
and the problem may happen at least in the below situations:
- during block device runtime resume, if memory allocation with
GFP_KERNEL is called inside runtime resume callback of any one
of its ancestors(or the block device itself), the deadlock may be
triggered inside the memory allocation since it might not complete
until the block device becomes active and the involed page I/O finishes.
The situation is pointed out first by Alan Stern. It is not a good
approach to convert all GFP_KERNEL[1] in the path into GFP_NOIO because
several subsystems may be involved(for example, PCI, USB and SCSI may
be involved for usb mass stoarage device, network devices involved too
in the iSCSI case)
- during block device runtime suspend, because runtime resume need
to wait for completion of concurrent runtime suspend.
- during error handling of usb mass storage deivce, USB bus reset
will be put on the device, so there shouldn't have any
memory allocation with GFP_KERNEL during USB bus reset, otherwise
the deadlock similar with above may be triggered. Unfortunately, any
usb device may include one mass storage interface in theory, so it
requires all usb interface drivers to handle the situation. In fact,
most usb drivers don't know how to handle bus reset on the device
and don't provide .pre_set() and .post_reset() callback at all, so
USB core has to unbind and bind driver for these devices. So it
is still not practical to resort to GFP_NOIO for solving the problem.
Also the introduced solution can be used by block subsystem or block
drivers too, for example, set the PF_MEMALLOC_NOIO flag before doing
actual I/O transfer.
It is not a good idea to convert all these GFP_KERNEL in the
affected path into GFP_NOIO because these functions doing that may be
implemented as library and will be called in many other contexts.
In fact, memalloc_noio_flags() can convert some of current static GFP_NOIO
allocation into GFP_KERNEL back in other non-affected contexts, at least
almost all GFP_NOIO in USB subsystem can be converted into GFP_KERNEL
after applying the approach and make allocation with GFP_NOIO
only happen in runtime resume/bus reset/block I/O transfer contexts
generally.
[1], several GFP_KERNEL allocation examples in runtime resume path
- pci subsystem
acpi_os_allocate
<-acpi_ut_allocate
<-ACPI_ALLOCATE_ZEROED
<-acpi_evaluate_object
<-__acpi_bus_set_power
<-acpi_bus_set_power
<-acpi_pci_set_power_state
<-platform_pci_set_power_state
<-pci_platform_power_transition
<-__pci_complete_power_transition
<-pci_set_power_state
<-pci_restore_standard_config
<-pci_pm_runtime_resume
- usb subsystem
usb_get_status
<-finish_port_resume
<-usb_port_resume
<-generic_resume
<-usb_resume_device
<-usb_resume_both
<-usb_runtime_resume
- some individual usb drivers
usblp, uvc, gspca, most of dvb-usb-v2 media drivers, cpia2, az6007,
That is just what I have found. Unfortunately, this allocation can
only be found by human being now, and there should be many not found
since any function in the resume path(call tree) may allocate memory
with GFP_KERNEL.
Cc: Alan Stern
Cc: Oliver Neukum
Cc: Jiri Kosina
Cc: Andrew Morton
Cc: Mel Gorman
Cc: KAMEZAWA Hiroyuki
Cc: Michal Hocko
Cc: Ingo Molnar
Cc: Peter Zijlstra
Cc: "Rafael J. Wysocki"
Signed-off-by: Minchan Kim
Signed-off-by: Ming Lei
---
v5:
- use inline instead of macro to define memalloc_noio_*
- replace memalloc_noio() with memalloc_noio_flags() to
make code neater
- don't clear GFP_FS because no GFP_IO means
that allocation won't enter device driver as pointed by
Andrew Morton
v4:
- fix comment
v3:
- no change
v2:
- remove changes on 'may_writepage' and 'may_swap' because that
isn't related with the patchset, and can't introduce I/O in
allocation path if GFP_IOFS is unset, so handing 'may_swap'
and may_writepage on GFP_NOIO
[PATCH v5 2/6] PM / Runtime: introduce pm_runtime_set_memalloc_noio()
The patch introduces the flag of memalloc_noio in 'struct dev_pm_info'
to help PM core to teach mm not allocating memory with GFP_KERNEL
flag for avoiding probable deadlock.
As explained in the comment, any GFP_KERNEL allocation inside
runtime_resume() or runtime_suspend() on any one of device in
the path from one block or network device to the root device
in the device tree may cause deadlock, the introduced
pm_runtime_set_memalloc_noio() sets or clears the flag on
device in the path recursively.
Cc: Alan Stern
Cc: "Rafael J. Wysocki"
Signed-off-by: Ming Lei
---
v5:
- fix code style error
- add comment on clear the device memalloc_noio flag
v4:
- rename memalloc_noio_resume as memalloc_noio
- remove pm_runtime_get_memalloc_noio()
- add comments on pm_runtime_set_memalloc_noio
v3:
- introduce pm_runtime_get_memalloc_noio()
- hold one global lock on pm_runtime_set_memalloc_noio
- hold device power lock when accessing memalloc_noio_resume
flag suggested by Alan Stern
- implement pm_runtime_set_memalloc_noio without recursion
suggested by Alan Stern
v2:
- introduce pm_runtime_set_memalloc_noio()
---
drivers/base/power/runtime.c | 60 ++
include/linux/pm.h |1 +
include/linux/pm_runtime.h |3 +++
3 files changed, 64 insertions(+)
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 3148b10..3e198a0 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -124,6 +124,66 @@ unsigned long pm_runtime_autosuspend_expiration(struct
device *dev)
}
EXPORT_SYMBOL_GPL(pm_runtime_autosuspend_expiration);
+static int dev_memalloc_noio(struct device *dev, void *data)
+{
+ return dev->power.memalloc_noio;
+}
+
+/*
+ * pm_runtime_set_memalloc_noio - Set a device's memalloc_noio flag.
+ * @dev: Device to handle.
+ * @enable: True for setting the flag and False for clearing the flag.
+ *
+ * Set the flag for all devices in the path from the device to the
+ * root device in the device tree if @enable is true, otherwise clear
+ * the flag for devices in the path whose siblings don't set the flag.
+ *
+ * The function should only be called by block device, or network
+ * device driver for solving the deadlock problem during runtime
+ * resume/suspend:
+ *
+ * If memory allocation with GFP_KERNEL is called inside runtime
+ * resume/suspend callback of any one of its ancestors(or the
+ * block device itself), the deadlock may be triggered inside the
+ * memory allocation since it might not complete until the block
+ * device becomes active and the involed page I/O finishes. The
+ * situation is pointed out first by Alan Stern. Network device
+ * are involved in iSCSI kind of situation.
+ *
+ * The lock of dev_hotplug_mutex is held in the function for handling
+ * hotplug race because pm_runtime_set_memalloc_noio() may be called
+ * in async probe().
+ *
+ * The function should be called between device_add() and device_del()
+ * on the affected device(block/network device).
+ */
+void pm_runtime_set_memalloc_noio(struct device *dev, bool enable)
+{
+ static DEFINE_MUTEX(dev_hotplug_mutex);
+
+ mutex_lock(&dev_hotplug_mutex);
+ for (;;) {
+ /* hold power lock since bitfield is not SMP-safe. */
+ spin_lock_irq(&dev->power.lock);
+ dev->power.memalloc_noio = enable;
+ spin_unlock_irq(&dev->power.lock);
+
+ dev = dev->parent;
+
+ /*
+* clear flag of the parent device only if all the
+* children don't set the flag because ancestor's
+* flag was set by any one of the descendants.
+*/
+ if (!dev || (!enable &&
+device_for_each_child(dev, NULL,
+ dev_memalloc_noio)))
+ break;
+ }
+ mutex_unlock(&dev_hotplug_mutex);
+}
+EXPORT_SYMBOL_GPL(pm_runtime_set_memalloc_noio);
+
/**
* rpm_check_suspend_allowed - Test whether a device may be suspended.
* @dev: Device to test.
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 03d7bb1..1a8a69d 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -538,6 +538,7 @@ struct dev_pm_info {
unsigned intirq_safe:1;
unsigned intuse_autosuspend:1;
unsigned inttimer_autosuspends:1;
+ unsigned intmemalloc_noio:1;
enum rpm_requestrequest;
enum rpm_status runtime_status;
int runtime_error;
diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index f271860..775e063 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -47,6 +47,7 @@ extern void pm_runtime_set_autosuspend_delay(struct devi
Re: [PATCH v5 1/6] mm: teach mm by current context info to not do I/O during memory allocation
On Sun, Nov 11, 2012 at 8:34 PM, Ming Lei wrote:
> +/* GFP_NOIO isn't allowed if PF_MEMALLOC_NOIO is set in current->flags */
> +static inline gfp_t memalloc_noio_flags(gfp_t flags)
> +{
> + if (unlikely(current->flags & PF_MEMALLOC_NOIO))
> + flags &= ~GFP_NOIO;
> + return flags;
Sorry, the above is wrong, and GFP_IO should be cleared, and I will
resend this one.
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
[PATCH v5 1/6] mm: teach mm by current context info to not do I/O during memory allocation
This patch introduces PF_MEMALLOC_NOIO on process flag('flags' field of
'struct task_struct'), so that the flag can be set by one task
to avoid doing I/O inside memory allocation in the task's context.
The patch trys to solve one deadlock problem caused by block device,
and the problem may happen at least in the below situations:
- during block device runtime resume, if memory allocation with
GFP_KERNEL is called inside runtime resume callback of any one
of its ancestors(or the block device itself), the deadlock may be
triggered inside the memory allocation since it might not complete
until the block device becomes active and the involed page I/O finishes.
The situation is pointed out first by Alan Stern. It is not a good
approach to convert all GFP_KERNEL[1] in the path into GFP_NOIO because
several subsystems may be involved(for example, PCI, USB and SCSI may
be involved for usb mass stoarage device, network devices involved too
in the iSCSI case)
- during block device runtime suspend, because runtime resume need
to wait for completion of concurrent runtime suspend.
- during error handling of usb mass storage deivce, USB bus reset
will be put on the device, so there shouldn't have any
memory allocation with GFP_KERNEL during USB bus reset, otherwise
the deadlock similar with above may be triggered. Unfortunately, any
usb device may include one mass storage interface in theory, so it
requires all usb interface drivers to handle the situation. In fact,
most usb drivers don't know how to handle bus reset on the device
and don't provide .pre_set() and .post_reset() callback at all, so
USB core has to unbind and bind driver for these devices. So it
is still not practical to resort to GFP_NOIO for solving the problem.
Also the introduced solution can be used by block subsystem or block
drivers too, for example, set the PF_MEMALLOC_NOIO flag before doing
actual I/O transfer.
It is not a good idea to convert all these GFP_KERNEL in the
affected path into GFP_NOIO because these functions doing that may be
implemented as library and will be called in many other contexts.
In fact, memalloc_noio_flags() can convert some of current static GFP_NOIO
allocation into GFP_KERNEL back in other non-affected contexts, at least
almost all GFP_NOIO in USB subsystem can be converted into GFP_KERNEL
after applying the approach and make allocation with GFP_NOIO
only happen in runtime resume/bus reset/block I/O transfer contexts
generally.
[1], several GFP_KERNEL allocation examples in runtime resume path
- pci subsystem
acpi_os_allocate
<-acpi_ut_allocate
<-ACPI_ALLOCATE_ZEROED
<-acpi_evaluate_object
<-__acpi_bus_set_power
<-acpi_bus_set_power
<-acpi_pci_set_power_state
<-platform_pci_set_power_state
<-pci_platform_power_transition
<-__pci_complete_power_transition
<-pci_set_power_state
<-pci_restore_standard_config
<-pci_pm_runtime_resume
- usb subsystem
usb_get_status
<-finish_port_resume
<-usb_port_resume
<-generic_resume
<-usb_resume_device
<-usb_resume_both
<-usb_runtime_resume
- some individual usb drivers
usblp, uvc, gspca, most of dvb-usb-v2 media drivers, cpia2, az6007,
That is just what I have found. Unfortunately, this allocation can
only be found by human being now, and there should be many not found
since any function in the resume path(call tree) may allocate memory
with GFP_KERNEL.
Cc: Alan Stern
Cc: Oliver Neukum
Cc: Jiri Kosina
Cc: Andrew Morton
Cc: Mel Gorman
Cc: KAMEZAWA Hiroyuki
Cc: Michal Hocko
Cc: Ingo Molnar
Cc: Peter Zijlstra
Cc: "Rafael J. Wysocki"
Signed-off-by: Minchan Kim
Signed-off-by: Ming Lei
---
v5:
- use inline instead of macro to define memalloc_noio_*
- replace memalloc_noio() with memalloc_noio_flags() to
make code neater
- don't clear GFP_FS because no GFP_IO means
that allocation won't enter device driver as pointed by
Andrew Morton
v4:
- fix comment
v3:
- no change
v2:
- remove changes on 'may_writepage' and 'may_swap' because that
isn't related with the patchset, and can't introduce I/O in
allocation path if GFP_IOFS is unset, so handing 'may_swap'
and may_writepage on GFP_NOIO
Re: [RFC PATCH 4/5] usb: add runtime pm support for usb port device
On 2012/11/10 0:07, Alan Stern wrote: On Fri, 9 Nov 2012, Lan Tianyu wrote: When pm qos flags is changing, the pm core will keep device not in RPM_SUSPEND via pm_runtime_get_sync/put(dev). When the flags are changed, this will affect usb device status. If NO_POWER_OFF flag was set when the device was power off, it would need to be resumed and suspended again without power off. If NO_POWER_OFF flag was cleared when the device was suspended without power off, it would need to be resumed and suspended again with power off inorder to save more power. This patch is to add runtime pm callback for usb port device. In the callback, usb device attached to the port will be resumed and suspended. No, this is completely the wrong way around. The device's resume routine should force the port to resume, not the reverse. Think of the port device as a parent of the USB device (even though it's not represented that way in the device hierarchy) and you'll see why. Hi Alan: Great thanks for your review and good suggestions. I think the main problem is that when the port device resumed(you said turn on/off power in the port's runtime callback ) and device powered on again, If we did't resume the device and suspend again, the device would be at uninitalized state(I guess it should be unattached state actually) until resume it, right? This will consume more power than suspend it agian. So from my opinion, the device should be resumed when port resumes. After autosuspend delay timeout, the device will suspend again. If I remember correctly, pcie port and sata port do the same thing that port will resume its children when it resumes. Also, all this port stuff is making hub.c a real mess. Before going any farther, can you split it all out into a separate source file? Ok. I will do this. Alan Stern -- 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: [RFC PATCH 2/5] usb: add usb port auto power off mechanism
On 2012/11/10 0:01, Alan Stern wrote: On Fri, 9 Nov 2012, Lan Tianyu wrote: This patch is to add usb port auto power off mechanism. When usb device is suspending, usb core will send clear usb port's POWER feature requst to power off port if all conditions were met. Actually, this is the wrong approach. What you should do, if all the conditions are met, is call pm_runtime_put(&port_dev->dev). Then the port's runtime suspend routine should be responsible for turning off the power. You mean just send set/clear PORT_POWER request in the port's runtime pm callback? Yeah, this has been suggested Rafael. But my previous opinion, usb_port_suspend/resume() has done a lot port's suspend/resume work.(e,g remote wakeup and link power management). If we added port's runtime pm callback, I thought we should move the content of usb_port_suspend/resume() to the callback. But usb_port_suspend/resume() are also used by system pm. So this maybe be difficult to move the code. From your response, this doesn't matter. But I think we also should do a check of runtime pm or system pm before calling pm_runtime_get_sync/put() in the usb_port_suspend/resume(). Since it's not reasonable to call runtime function during system pm, Right? About port's runtime callback, I have saided my concern about the status of device after port being resumed at my previous rely. Similarly, whenever you want to make sure that a port is powered, you should call pm_runtime_get_sync(&port_dev->dev). Alan Stern -- 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: [RFC PATCH 5/5] usb: add usb port's pm qos flags request to change NO_POWER_OFF flag
On 2012/11/10 0:10, Alan Stern wrote: On Fri, 9 Nov 2012, Lan Tianyu wrote: Some usb devices can't be resumed correctly after power off. This patch is to add pm qos flags request to change NO_POWER_OFF and provide usb_device_allow_power_off() for device drivers to allow or prohibit usb core to power off the device. If the reason for adding this request is to prevent the device from being powered off, then shouldn't the PM QOS flag be associated with device rather than with the port? Hi Alan: Great thanks for your review and good suggestions. For pm qos flags, If put it on the port device, there is an advantage that it can cover port without device or unused port and this part didn't present in my this patchset. In fact, it seems reasonable to have such a flag for both the device and the port. You mean for usb device or usb interface device? I think port and device are bound. Power off port also means to power off device. So prevent device from power off also means to prevent port from power off. They just like a power domain. Because power is on the port and so I thought we just needed to have pm qos flag on the port. This maybe a simple version. If necessary, I also could add pm qos flag for usb or usb interface device later. Alan Stern -- 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: [RFC PATCH 4/5] usb: add runtime pm support for usb port device
On Sun, 11 Nov 2012, Lan Tianyu wrote: > Hi Alan: > Great thanks for your review and good suggestions. > I think the main problem is that when the port device resumed(you said > turn on/off power in the port's runtime callback ) and device powered on > again, If we did't resume the device and suspend again, the device would > be at uninitalized state(I guess it should be unattached state actually) > until resume it, right? No. The device will be in the POWERED state at first, but it will quickly change into the SUSPENDED state. > This will consume more power than suspend it > agian. No it won't, because the device will suspend itself after 3 ms. > So from my opinion, the device should be resumed when port > resumes. After autosuspend delay timeout, the device will suspend again. > If I remember correctly, pcie port and sata port do the same thing that > port will resume its children when it resumes. That doesn't mean USB has to do the same thing. 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: [RFC PATCH 2/5] usb: add usb port auto power off mechanism
On Sun, 11 Nov 2012, Lan Tianyu wrote: > > What you should do, if all the conditions are met, is call > > pm_runtime_put(&port_dev->dev). Then the port's runtime suspend > > routine should be responsible for turning off the power. > > You mean just send set/clear PORT_POWER request in the port's runtime pm > callback? Yeah, this has been suggested Rafael. Yes, that's what I mean. > But my previous opinion, usb_port_suspend/resume() has done a lot port's > suspend/resume work.(e,g remote wakeup and link power management). If we > added port's runtime pm callback, I thought we should move the content > of usb_port_suspend/resume() to the callback. But No, that won't work. The work you're talking about has to be done when the device is suspended/resumed, not when the port is powered off/on. > usb_port_suspend/resume() are also used by system pm. So this maybe be > difficult to move the code. From your response, this doesn't matter. But > I think we also should do a check of runtime pm or system pm before > calling pm_runtime_get_sync/put() in the usb_port_suspend/resume(). > Since it's not reasonable to call runtime function during system pm, Right? In fact it won't matter, because the PM core will increment the port's usage counter at the start of a system suspend. So this test isn't needed. 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: [RFC PATCH 5/5] usb: add usb port's pm qos flags request to change NO_POWER_OFF flag
On Sun, 11 Nov 2012, Lan Tianyu wrote: > On 2012/11/10 0:10, Alan Stern wrote: > > On Fri, 9 Nov 2012, Lan Tianyu wrote: > > > >> Some usb devices can't be resumed correctly after power off. This > >> patch is to add pm qos flags request to change NO_POWER_OFF and > >> provide usb_device_allow_power_off() for device drivers to allow or > >> prohibit usb core to power off the device. > > > > If the reason for adding this request is to prevent the device from > > being powered off, then shouldn't the PM QOS flag be associated with > > device rather than with the port? > Hi Alan: > Great thanks for your review and good suggestions. > For pm qos flags, If put it on the port device, there is an advantage > that it can cover port without device or unused port and this part > didn't present in my this patchset. Right. That's a big advantage. > > In fact, it seems reasonable to have such a flag for both the device > > and the port. > You mean for usb device or usb interface device? USB device. > I think port and device are bound. Power off port also means to power > off device. So prevent device from power off also means to prevent port > from power off. They just like a power domain. Because power is on the > port and so I thought we just needed to have pm qos flag on the port. Consider what happens when the device is unplugged. The restriction against powering off the device won't be needed any more, but the PM QOS flag will still be set for the port. > This maybe a simple version. If necessary, I also could add pm qos flag > for usb or usb interface device later. Okay. 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: fix endpoint-disabling for failed config changes
At 08:37 PM 11/10/2012 -0500, Alan Stern wrote: >Does the patch fix the problem? Morning and coffee motivated an attempt to build a kernel and try the patch. However it fails to apply for 3.6.6 --even caffine is not enough to inspire me to consider a -rc for production. Any chance for a 3.6 patch? -- 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: fix endpoint-disabling for failed config changes
Just started a copies of /dev/urandom to the new drive mirror pairs (in preparation for crypt'ing them). This will take four days so I can't touch the system for a week now. No hurry on a 3.6 patch if you were contemplating one. Maybe 3.7 will be out by then anyway. >At 08:37 PM 11/10/2012 -0500, Alan Stern wrote: >>Does the patch fix the problem? > >Will let you and the list know. Want >to wait for a full kernel release. > >The drives go active if the power to >the docks is toggled after they spin >up--going with that for now. > >Will finish configuring the array >and offline the mirror copies before >upgrading the kernel. -- 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: XHCI: xhci-ring: Remove unused dma address calculation in inc_enq and inc_deq function
> It looks like your mail client attempted to line wrap it. You might > want to use mutt, thunderbird, or maybe even the plain text gmail > interface to resend this. If anyone is using Outlook, see this: https://lkml.org/lkml/2011/1/25/587 Yuhong Bao-- 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: xceiv: patches for v3.8 merge window
On Fri, Nov 09, 2012 at 11:25:53PM +0200, Felipe Balbi wrote: > Hi Greg, > > Here are the transceiver patches for v3.8 merge window. Let me know > if you want any changes. > > Have a nice weekend > > ps: for samsung folks, I'm sorry but I couldn't wait any longer. > > The following changes since commit ddffeb8c4d0331609ef2581d84de4d763607bd37: > > Linux 3.7-rc1 (2012-10-14 14:41:04 -0700) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git > tags/xceiv-for-v3.8 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: [GIT PULL] usb: musb: patches for v3.8 merge window
On Fri, Nov 09, 2012 at 11:27:32PM +0200, Felipe Balbi wrote:
> Hi Greg,
>
> MUSB changes for v3.8 follow. This will conflict with your usb-next branch,
> below you can find the resolution I used:
>
> diff --cc drivers/usb/musb/musb_dsps.c
> index ff5f112,e770f79..cf08966a
> --- a/drivers/usb/musb/musb_dsps.c
> +++ b/drivers/usb/musb/musb_dsps.c
> @@@ -458,14 -489,24 +489,24 @@@ static int __devinit dsps_create_musb_p
> struct platform_device *musb;
> struct resource *res;
> struct resource resources[2];
> -char res_name[10];
> +char res_name[11];
> - int ret, musbid;
> + int ret;
>
> - /* get memory resource */
> - snprintf(res_name, sizeof(res_name), "musb%d", id);
> - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, res_name);
> + resources[0].start = dsps_control_module_phys[id];
> + resources[0].end = resources[0].start + SZ_4 - 1;
> + resources[0].flags = IORESOURCE_MEM;
> +
> + glue->usb_ctrl[id] = devm_request_and_ioremap(&pdev->dev, resources);
> + if (glue->usb_ctrl[id] == NULL) {
> + dev_err(dev, "Failed to obtain usb_ctrl%d memory\n", id);
> + ret = -ENODEV;
> + goto err0;
> + }
> +
> + /* first resource is for usbss, so start index from 1 */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, id + 1);
> if (!res) {
> - dev_err(dev, "%s get mem resource failed\n", res_name);
> + dev_err(dev, "failed to get memory for instance %d\n", id);
> ret = -ENODEV;
> goto err0;
> }
>
> The following changes since commit ddffeb8c4d0331609ef2581d84de4d763607bd37:
>
> Linux 3.7-rc1 (2012-10-14 14:41:04 -0700)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git
> tags/musb-for-v3.8
Thanks for the merge resolution, that helped out. Now pulled and pushed
out.
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: [GIT PULL] usb: dwc3: patches for v3.8 merge window
On Fri, Nov 09, 2012 at 11:29:05PM +0200, Felipe Balbi wrote:
> Hi Greg,
>
> Here are dwc3 patches for v3.8. This will also conflict with your usb-next
> branch and below you can find my resolution:
>
> diff --cc drivers/usb/dwc3/core.c
> index c14ebc9,2bd007d..4174054
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@@ -408,11 -355,6 +355,10 @@@ err0
> static void dwc3_core_exit(struct dwc3 *dwc)
> {
> dwc3_event_buffers_cleanup(dwc);
> - dwc3_free_event_buffers(dwc);
> +
> +usb_phy_shutdown(dwc->usb2_phy);
> +usb_phy_shutdown(dwc->usb3_phy);
> +
> }
>
> #define DWC3_ALIGN_MASK (16 - 1)
>
> The following changes since commit ddffeb8c4d0331609ef2581d84de4d763607bd37:
>
> Linux 3.7-rc1 (2012-10-14 14:41:04 -0700)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git
> tags/dwc3-for-v3.8
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: [GIT PULL] USB patches
On Fri, Nov 09, 2012 at 11:30:36PM +0200, Felipe Balbi wrote: > Hi Greg, > > Here are gadget patches for v3.8, and it also conflicts with your > usb-next branch. Below you can find my resolution (suggested > by Kuninori): > > diff --cc drivers/usb/renesas_usbhs/fifo.c > index c021b20,72ad375..9538f0f > --- a/drivers/usb/renesas_usbhs/fifo.c > +++ b/drivers/usb/renesas_usbhs/fifo.c > @@@ -795,7 -798,7 +798,8 @@@ static void xfer_work(struct work_struc > dev_dbg(dev, " %s %d (%d/ %d)\n", > fifo->name, usbhs_pipe_number(pipe), pkt->length, pkt->zero); > > + usbhs_pipe_set_trans_count_if_bulk(pipe, pkt->trans); > +usbhs_pipe_enable(pipe); > usbhsf_dma_start(pipe, fifo); > dma_async_issue_pending(chan); > } > > The following changes since commit ddffeb8c4d0331609ef2581d84de4d763607bd37: > > Linux 3.7-rc1 (2012-10-14 14:41:04 -0700) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git > tags/gadget-for-v3.8 Thanks for the resolution, I would have done it backwards :) Now pulled and pushed out. 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: [RFC PATCH 5/5] usb: add usb port's pm qos flags request to change NO_POWER_OFF flag
On 2012年11月11日 23:55, Alan Stern wrote: > On Sun, 11 Nov 2012, Lan Tianyu wrote: > >> On 2012/11/10 0:10, Alan Stern wrote: >>> On Fri, 9 Nov 2012, Lan Tianyu wrote: >>> Some usb devices can't be resumed correctly after power off. This patch is to add pm qos flags request to change NO_POWER_OFF and provide usb_device_allow_power_off() for device drivers to allow or prohibit usb core to power off the device. >>> >>> If the reason for adding this request is to prevent the device from >>> being powered off, then shouldn't the PM QOS flag be associated with >>> device rather than with the port? >> Hi Alan: >> Great thanks for your review and good suggestions. >> For pm qos flags, If put it on the port device, there is an advantage >> that it can cover port without device or unused port and this part >> didn't present in my this patchset. > > Right. That's a big advantage. > >>> In fact, it seems reasonable to have such a flag for both the device >>> and the port. >> You mean for usb device or usb interface device? > > USB device. > >> I think port and device are bound. Power off port also means to power >> off device. So prevent device from power off also means to prevent port >> from power off. They just like a power domain. Because power is on the >> port and so I thought we just needed to have pm qos flag on the port. > > Consider what happens when the device is unplugged. The restriction > against powering off the device won't be needed any more, but the PM > QOS flag will still be set for the port. You are saying outside port. For outside port, currently our plan is to set NO_POWER_OFF flag defaultly. So it doesn't matter after device unplugged. Moreover, we can change the port's pm qos flag when device is disconnected. > >> This maybe a simple version. If necessary, I also could add pm qos flag >> for usb or usb interface device later. > > Okay. > > Alan Stern > -- Best regards Tianyu Lan -- 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: fix endpoint-disabling for failed config changes
On Sat, Nov 10, 2012 at 08:37:58PM -0500, Alan Stern wrote: > On Sat, 10 Nov 2012 starlight.201...@binnacle.cx wrote: > > > Hello, > > > > I'm wondering how long it will be before this > > patch appears in a 3.6.x release. Any idea? > > Probably a week or two. It's up to Greg KH. As it's really late in the 3.7 release cycle, I'm going to hold off on this patch until the 3.8-rc1 merge window. At that point it will get backported to 3.6 and other kernels. So it will be a few weeks before that happens, sorry. 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: [RFC PATCH 4/5] usb: add runtime pm support for usb port device
On 2012年11月11日 23:46, Alan Stern wrote: > On Sun, 11 Nov 2012, Lan Tianyu wrote: > >> Hi Alan: >> Great thanks for your review and good suggestions. >> I think the main problem is that when the port device resumed(you said >> turn on/off power in the port's runtime callback ) and device powered on >> again, If we did't resume the device and suspend again, the device would >> be at uninitalized state(I guess it should be unattached state actually) >> until resume it, right? > > No. The device will be in the POWERED state at first, but it will > quickly change into the SUSPENDED state. > >> This will consume more power than suspend it >> agian. > > No it won't, because the device will suspend itself after 3 ms. But the premise is "they see a constant Idle state on their upstream facing bus lines for more than 3.0 ms", that means the port's suspend feature should be set to prevent all the bus activity(e.g sof), right? Now I have another concern that the device will maintain address 0 after repower and not reset-resume. What happen if another device was plugged in? > >> So from my opinion, the device should be resumed when port >> resumes. After autosuspend delay timeout, the device will suspend again. >> If I remember correctly, pcie port and sata port do the same thing that >> port will resume its children when it resumes. > > That doesn't mean USB has to do the same thing. > > Alan Stern > -- Best regards Tianyu Lan -- 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 5/5] usb: add usb port's pm qos flags request to change NO_POWER_OFF flag
On Mon, 12 Nov 2012, Lan Tianyu wrote: > >> I think port and device are bound. Power off port also means to power > >> off device. So prevent device from power off also means to prevent port > >> from power off. They just like a power domain. Because power is on the > >> port and so I thought we just needed to have pm qos flag on the port. > > > > Consider what happens when the device is unplugged. The restriction > > against powering off the device won't be needed any more, but the PM > > QOS flag will still be set for the port. > You are saying outside port. For outside port, currently our plan is to > set NO_POWER_OFF flag defaultly. So it doesn't matter after device > unplugged. What if the user changes the NO_POWER_OFF flag? It's only a default, so it can be changed. Once we have the PM QOS flag, do we need the NO_POWER_OFF flag any more? > Moreover, we can change the port's pm qos flag when device is > disconnected. But what if the user wants that port to remain powered on even after the device is disconnected? It seems to me the user might have good reasons to want to change the power-off behavior for a particular device, _or_ to change the power-off behavior for a particular port regardless of what device is plugged into it. 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: [RFC PATCH 4/5] usb: add runtime pm support for usb port device
On Mon, 12 Nov 2012, Lan Tianyu wrote: > >> This will consume more power than suspend it > >> agian. > > > > No it won't, because the device will suspend itself after 3 ms. > But the premise is "they see a constant Idle state on their upstream > facing bus lines for more than 3.0 ms", that means the port's suspend > feature should be set to prevent all the bus activity(e.g sof), right? No, it only means that the port doesn't transmit packets. This could be because the port's suspend feature is set, or it could be because the port's enable feature is clear. > Now I have another concern that the device will maintain address 0 after > repower and not reset-resume. What happen if another device was plugged in? If the port isn't enabled, it doesn't matter. 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: [RFC PATCH 5/5] usb: add usb port's pm qos flags request to change NO_POWER_OFF flag
On 2012年11月12日 10:41, Alan Stern wrote: > On Mon, 12 Nov 2012, Lan Tianyu wrote: > I think port and device are bound. Power off port also means to power off device. So prevent device from power off also means to prevent port from power off. They just like a power domain. Because power is on the port and so I thought we just needed to have pm qos flag on the port. >>> >>> Consider what happens when the device is unplugged. The restriction >>> against powering off the device won't be needed any more, but the PM >>> QOS flag will still be set for the port. >> You are saying outside port. For outside port, currently our plan is to >> set NO_POWER_OFF flag defaultly. So it doesn't matter after device >> unplugged. > > What if the user changes the NO_POWER_OFF flag? It's only a default, > so it can be changed. > > Once we have the PM QOS flag, do we need the NO_POWER_OFF flag any > more? Sorry, I didn't express clear. I mean the NO_POWER_OFF flag in the PM qos flags. > >> Moreover, we can change the port's pm qos flag when device is >> disconnected. > > But what if the user wants that port to remain powered on even after > the device is disconnected? It seems to me the user might have good > reasons to want to change the power-off behavior for a particular > device, _or_ to change the power-off behavior for a particular port > regardless of what device is plugged into it. Yeah. You are right. User can keep their choice. In the pm core, there is already a pm qos flags request which represent user choice. I add the new request to represent driver's choice. Only when both two request are not set pm qos NO_POWER_OFF flag, the effective flag maintains 0. If user set the flag but driver says the device can power off. The result is that the effective flag is set so the device will not be power off. As long as user set the flag, the device or port will not power off. >From my opinion, we can clear the flag of the new request I added when a device is disconnected. So when user also keep the flag unset and device is disconnected, then the port can be power off. > > Alan Stern > -- Best regards Tianyu Lan -- 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 0/5] usb: phy: samsung: Introducing usb phy driver for samsung SoCs
Felipe Balbi wrote:
>
> Hi,
>
Hi :-)
[...]
> Sure, but I still need Kukjin's 'say-so' for the arch/arm/plat-samsung
> and arch/arm/mach-exynos part.
>
Basically, this approach looks OK to me.
BTW, I have some comments and need to update...
>From 4th patch...
> diff --git a/arch/arm/mach-s3c64xx/mach-smdk6410.c b/arch/arm/mach-
> s3c64xx/mach-smdk6410.c
[...]
> @@ -72,6 +73,8 @@
> #include
> #include
> #include
> +#include
Why? In addition, this causes build error with s3c6400_defconfig.
[...]
> diff --git a/arch/arm/plat-samsung/devs.c b/arch/arm/plat-samsung/devs.c
> index 03f654d..9cdb666 100644
[...]
> @@ -1370,6 +1371,30 @@ struct platform_device s5p_device_mixer = {
>
> /* USB */
>
> +#ifdef CONFIG_S3C_DEV_USB_HSOTG
> +/* USB PHY*/
> +static struct resource samsung_usbphy_resource[] = {
> + [0] = {
> + .start = S3C_PA_USB_PHY,
> + .end = S3C_PA_USB_PHY + SZ_16 - 1,
> + .flags = IORESOURCE_MEM,
> + },
> +};
+static struct resource samsung_usbphy_resource[] = {
+ [0] = DEFINED_RES_MEM(S3C_PA_USB_PHY, SZ_16),
+};
[...]
Happens build error with s5pv210_defconfig
arch/arm/plat-samsung/devs.c:1375: error: 'S3C_PA_USB_PHY' undeclared here
(not in a function)
make[2]: *** [arch/arm/plat-samsung/devs.o] Error 1
make[1]: *** [arch/arm/plat-samsung] Error 2
make[1]: *** Waiting for unfinished jobs
And another build error with exynos_defconfig...
arch/arm/mach-exynos/built-in.o: In function `.LANCHOR1':
setup-i2c0.c:(.data+0x8080): undefined reference to
`s5p_usb_phy_pmu_isolation'
>From 5th patch
If possible, please to use 'ARM: [sub-arch dir name]: [subject]' format.
So I preferred to use 'ARM: EXYNOS: Enabling samsung-usbphy driver for
EXYNOS4210' on that.
Please make sure your patch has no problem for kernel compilation before
submitting...
Thanks.
Best regards,
Kgene.
--
Kukjin Kim , Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
--
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
