Re: [PATCH 2/3] usb: Take attribute avoid_reset_quirk out of usb device's attribute group

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx> writes:
> On Mon, Jul 30, 2012 at 09:28:36AM +0200, Oliver Neukum wrote:
>> On Monday 30 July 2012 11:34:10 Lan Tianyu wrote:
>> > The hub is always supposed to support reset and its persist is enabled.
>> 
>> By default, not necessarily always. User space may disable it.
>> 
>> > So hub doesn't need attribute avoid_reset_quirk. The patch is to take
>> > attribute avoid_reset_quirk out of usb device's attribute group and
>> > add or remove it in the usb_create/remove_sysfs_dev_files() if the device
>> > is not a usb hub.
>> 
>> Why? What is gained doing so? Without further explanation about the need
>> or benefit of doing so, why do you want to make hubs different from
>> other devices?
>
> Along those lines, Tianyu, can you share what your master plan for
> implementing the automatic powering off of ports?  I think it would help
> to understand the bigger picture when looking at small patches like
> these.
>
> So far, the discussion on the mailing list seems to boil down to:
>
> Issues
> ------
>
>  - We need to issue a reset resume for all suspended devices that have
>    been powered off.
>
>  - We can't power off ports with connected devices that require firmware
>    (e.g. bluetooth and 3G modems).  The firmware would be lost when
>    they're reset.

I don't think that is limited to devices needing firmware.  Even modems
with persistent firmware will keep lots of internal state which the
driver cannot re-establish.  And even if you could reset the device
state, the mobile network kept some state which also was lost when you
powered off the modem.

I really think this poweroff-on-suspend thought is a bit simplistic. It
may work for some simple devices, but there are certainly a number of
devices where suspend cannot be replaced by poweroff.

>  - Many drivers may turn on remote wakeup when a device is suspended,
>    but userspace may not need the wakeup.  An example would be if the
>    user clicked "Disable bluetooth" from ConnMan.  It obviously wouldn't
>    care about remote wakeups from the bluetooth device.

This I did not understand.  Do you have some way to override the drivers
decision to enable remote wakeup?  Based on what?  Do you want to power
off a device where the driver requested remote wakeup?

I fail to see how that is going to work.  You need to cooperate with the
driver.

> Possible solutions
> ------------------
>
>  - We need a new interface driver flag to indicate a driver is fine with
>    the port being powered off.  Something like "supports_power_off".

May work for some devices.  But what are the use cases? Which devices
will work better with such driver support than they will if you simply
unload the driver?

>  - In addition to the port power sysfs values of "on" or "off", we also
>    need an "auto" value that attempts to apply a smart in-kernel policy
>    to when to power off the port.  That policy might look like:
>
>    1. If the device is internal and unpluggable, power off the port

How do you detect the "unpluggable"?  Did you consider rfkill switches
"plugging" and "unplugging" internal devices?

>    2. If the device is internal and suspended, power off the port if all
>       the following are true:
>
>       a) all interface drivers have supports_power_off set, or no
> 	 interface drivers are bound and usbfs has not claimed the
> 	 device.
>       b) remote wakeup is disabled
>       c) USB_QUIRK_RESET_MORPHS is not set

Why can't that be a userspace decision?  I.e. drop all this policy in
the kernel stuff, and just implement:

>  - If userspace wants a port to be powered off, and one of the interface
>    drivers doesn't set supports_power_off or the driver enables remote
>    wakeup, then userspace will need to unbind or unload the driver.
>
>
> So far, the discussion has been mainly focused on figuring out a smart
> policy for internal USB ports.  However, I'd like to see the "auto"
> in-kernel policy extended to external USB ports.  Perhaps we need to
> expose the ACPI internal/external port and connectable/unconnectable
> values through sysfs, and apply the policy to both internal and external
> devices?
>
> Then userspace could look at whether a port is internal or external, and
> decide when it makes sense to auto-power-off the port.  It could decide
> to set an "auto" policy on all ports when the screen blanks.  When the
> user starts interacting with the system and the screen turns back on,
> userspace could change the policy back to "on" for external ports and
> internal connectable ports.

Yes, this is the way to go IMHO. 

BTW, what if the interaction started with a USB keyboard being plugged
in?  No problem for you - that was the userspace daemon bad coice of
policy :-)

> Then policy #1 would just change to "If the device is disconnected,
> power off the port" and policy #2 would apply to both internal and
> external suspended ports.
>
> Thoughts?

I really think the kernel should limit itself to providing the basic
functionality (driver support flag and port power switch), and leave any
policy decisions for userspace to screw up.

It's not like the kernel decides whether to enable autosuspend in the
first place...



Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux