The device is initially powered, however the display is off. With this
patch, the shadow_power variable is initialized to 1 (as the device is
initially powered). I think the attribute of "powered" may be adding
to the confusion. It really means "turn on/off the display", which is
independent of if the actual device is powered or suspended (of
course, the display can't be on if the device is suspended).
However, more is needed, than just initializing shadow_power to 1. As
the display starts in the off state, the device could be suspended
immediately after initialization completes (if usb autosuspend is
enabled, and in this case the suspend() function is called then
setting shadow_power to 0). The device really only needs to be powered
when the user has turned on the display. When update_display_powered()
is called to turn on the device (this function sends a msg to the
device to turn on/off its display), it *should* call
usb_autopm_get_interface() if it hasn't before. This is because if usb
autosuspend is subsequently enabled on the device, since the reference
count is 0, the device could be suspended. This is not what the user
would want. Only when the display on the device is off should it be
suspended.
Also, the old logic would call usb_autopm_put_interface() if
shadow_power == 1 and the user requested the display to be off ("echo
0 > /sys/$DEVICE/powered"). If usb autosuspend was disabled the
suspend() function would never be called, and the shadow_power
variable would still be 1. So the user could "echo 0" multiple times
(even though the display is already off) and this would really screw
up the reference count (it would be negative from calling
usb_autopm_put_interface() multiple times).
This is why there is a separate variable to track if we have called
usb_autopm_get/put_interface().
Harry Metzger
On Mon, Mar 8, 2010 at 9:46 AM, Oliver Neukum <oliver@xxxxxxxxxx> wrote:
> Am Montag, 8. März 2010 15:10:28 schrieb Harrison Metzger:
>> The problem was that the update_display_{visual,mode}() functions
>> (which send a control msg to the actual device) checks to make sure
>> mydev->shadow_power == 1, otherwise it returns immediately. The
>> mydev->shadow_power variable is only set in the resume(),
>> reset_resume(), and suspend() functions. So, when autosuspend is
>> disabled, resume() is never called, which means mydev->shadow_power is
>> still set to 0, and the update_display_{visual,mode}() functions
>> simply return and do not send a msg to the device.
>
> Wouldn't it be easier to initialize shadow_power as 1?
> Is the display initially powered?
>
> Regards
> Oliver
>
--
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
