On Fri, May 16, 2014 at 11:32 AM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> On Wed, 7 May 2014, Dan Williams wrote:
>
>> We want to manipulate ->did_runtime_put in usb_port_runtime_resume(),
>> but we don't want that to collide with other updates. Move usb_port
>> flags to new port-bitmap fields in usb_hub. "did_runtime_put" is renamed
>> "child_usage_bits" to reflect that it is strictly standing in for the
>> fact that usb_devices are not the device_model children of their parent
>> port.
>>
>> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
>
>
>> @@ -4628,8 +4628,10 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1,
>> spin_lock_irq(&device_state_lock);
>> if (hdev->state == USB_STATE_NOTATTACHED)
>> status = -ENOTCONN;
>> - else
>> + else {
>> port_dev->child = udev;
>> + set_bit(port1, hub->child_usage_bits);
>> + }
>
> Doesn't this line belong in usb_new_device(), next to the
> pm_runtime_get_sync(&port_dev->dev) line? Or maybe does that line
> belong here?
I believe it belongs in usb_new_device() especially because that
routine initializes the runtime pm state of the device. Were the
usb_device a device-model child of its port then usb_new_device()
would already be implicitly modifying the port_dev usage count.
>> spin_unlock_irq(&device_state_lock);
>> mutex_unlock(&usb_port_peer_mutex);
>>
>> diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
>> index 048c797f394c..b3432deb8355 100644
>> --- a/drivers/usb/core/hub.h
>> +++ b/drivers/usb/core/hub.h
>> @@ -51,6 +51,10 @@ struct usb_hub {
>> device present */
>> unsigned long wakeup_bits[1]; /* ports that have signaled
>> remote wakeup */
>> + unsigned long power_bits[1]; /* ports that are powered */
>> + unsigned long child_usage_bits[1]; /* child pm_runtime
>> + active */
>
> I'd say rather /* ports powered on for children */
Done.
> Alan Stern
Thanks!
--
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
