From: Hans de Goede <hdegoede@redhat.com>
To: Hui Wang <hui.wang@canonical.com>,
Marcel Holtmann <marcel@holtmann.org>,
Johan Hedberg <johan.hedberg@gmail.com>
Cc: linux-bluetooth@vger.kernel.org
Subject: Re: [PATCH 5.12 regression fix] Bluetooth: btusb: Revert Fix the autosuspend enable and disable
Date: Thu, 18 Feb 2021 16:02:05 +0100 [thread overview]
Message-ID: <4510935f-a30b-445d-a048-683619f2855b@redhat.com> (raw)
In-Reply-To: <90d8996d-a376-2e9c-37ce-ce50b8660fd1@canonical.com>
Hi,
On 2/18/21 3:36 PM, Hui Wang wrote:
>
> On 2/18/21 8:37 PM, Hans de Goede wrote:
>> drivers/usb/core/hub.c: usb_new_device() contains the following:
> [...]
>> err = hci_register_dev(hdev);
>> if (err < 0)
>> @@ -4688,9 +4688,6 @@ static void btusb_disconnect(struct usb_interface *intf)
>> gpiod_put(data->reset_gpio);
>> hci_free_dev(hdev);
>> -
>> - if (!enable_autosuspend)
>> - usb_enable_autosuspend(data->udev);
> Hi Hans,
>
> And Do we need to call usb_disable_autosuspend() in the disconnect()? like below:
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 32161dd40ed6..ef831492363c 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -4673,6 +4673,9 @@ static void btusb_disconnect(struct usb_interface *intf)
>
> hci_unregister_dev(hdev);
>
> + if (enable_autosuspend)
> + usb_disable_autosuspend(data->udev);
> +
> if (intf == data->intf) {
> if (data->isoc)
> usb_driver_release_interface(&btusb_driver, data->isoc);
>
>
> Before the btusb_probe() is called, the usb device is autosuspend disabled, suppose users set the btusb.enable_autosuspend=1, the driver btusb will enable the autosuspend on this device. If users remove this driver, the disconnect() will be called, the usb device will keep autosuspend enabled. Next time if users reload this driver by 'sudo modprobe btusb enalbe_autosuspend=0', they will find the device is autosuspend enabled instead of disabled.
The problem with calling usb_disable_autosuspend() is that the auto-suspend setting is a bool,
rather then a counter, so if a udev-rule or the user manually through e.g. :
echo auto > /sys/bus/usb/devices/1-10/power/control
Has enabled autosuspend then we would be disabling it, which is undesirable.
Most USB drivers which have some way of enabling autosuspend by-default
(IOW which call usb_enable_autosuspend()) simply enable it at the end
of a successful probe and leave it as is on remove.
Also keep in mind that remove normally runs on unplug of the device, in
which case it does not matter as the device is going away.
If a user wants to disable autosuspend after loading the btusb module,
the correct way to do this is by simply running e.g. :
echo on > /sys/bus/usb/devices/1-10/power/control
Rather then rmmod-ing and insmod-ing the module with a different module-param value.
Regards,
Hans
next prev parent reply other threads:[~2021-02-18 17:27 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-18 12:37 [PATCH 5.12 regression fix 0/1] Bluetooth: btusb: Revert "Fix the autosuspend enable and disable" Hans de Goede
2021-02-18 12:37 ` [PATCH 5.12 regression fix] Bluetooth: btusb: Revert Fix the autosuspend enable and disable Hans de Goede
2021-02-18 14:36 ` Hui Wang
2021-02-18 15:02 ` Hans de Goede [this message]
2021-02-18 15:43 ` Hui Wang
2021-02-18 20:01 ` Marcel Holtmann
2021-02-18 22:04 ` Hans de Goede
2021-02-18 23:41 ` Luiz Augusto von Dentz
2021-02-19 9:24 ` Hans de Goede
2021-02-25 4:37 ` Hui Wang
2021-02-25 14:17 ` Hans de Goede
2021-02-26 1:03 ` Hui Wang
2021-02-18 15:08 ` Bluetooth: btusb: Revert "Fix the autosuspend enable and disable" bluez.test.bot
2021-02-26 1:05 ` [PATCH 5.12 regression fix] Bluetooth: btusb: Revert Fix the autosuspend enable and disable Hui Wang
2021-02-18 13:32 ` [PATCH 5.12 regression fix 0/1] Bluetooth: btusb: Revert "Fix the autosuspend enable and disable" Hui Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4510935f-a30b-445d-a048-683619f2855b@redhat.com \
--to=hdegoede@redhat.com \
--cc=hui.wang@canonical.com \
--cc=johan.hedberg@gmail.com \
--cc=linux-bluetooth@vger.kernel.org \
--cc=marcel@holtmann.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.