[PATCH v4 1/3] Bluetooth: Fix possible race with userspace of sysfs isoc_alt

[PATCH v4 1/3] Bluetooth: Fix possible race with userspace of sysfs isoc_alt

[Hsin-chen Chuang]

From: Hsin-chen Chuang <chharry@chromium.org>

Expose the isoc_alt attr with device group to avoid the racing.

Now we create a dev node for btusb. The isoc_alt attr belongs to it and
it also becomes the parent device of hci dev.

Fixes: b16b327edb4d ("Bluetooth: btusb: add sysfs attribute to control USB alt setting")
Signed-off-by: Hsin-chen Chuang <chharry@chromium.org>
---

Changes in v4:
- Create a dev node for btusb. It's now hci dev's parent and the
  isoc_alt now belongs to it.
- Since the changes is almost limitted in btusb, no need to add the
  callbacks in hdev anymore.

Changes in v3:
- Make the attribute exported only when the isoc_alt is available.
- In btusb_probe, determine data->isoc before calling hci_alloc_dev_priv
  (which calls hci_init_sysfs).
- Since hci_init_sysfs is called before btusb could modify the hdev,
  add new argument add_isoc_alt_attr for btusb to inform hci_init_sysfs.

 drivers/bluetooth/btusb.c        | 98 +++++++++++++++++++++++++-------
 include/net/bluetooth/hci_core.h |  1 +
 net/bluetooth/hci_sysfs.c        |  3 +-
 3 files changed, 79 insertions(+), 23 deletions(-)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 1caf7a071a73..cb3db18bb72c 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -920,6 +920,8 @@ struct btusb_data {
 	int oob_wake_irq;   /* irq for out-of-band wake-on-bt */
 
 	struct qca_dump_info qca_dump;
+
+	struct device dev;
 };
 
 static void btusb_reset(struct hci_dev *hdev)
@@ -3693,6 +3695,9 @@ static ssize_t isoc_alt_store(struct device *dev,
 	int alt;
 	int ret;
 
+	if (!data->hdev)
+		return -ENODEV;
+
 	if (kstrtoint(buf, 10, &alt))
 		return -EINVAL;
 
@@ -3702,6 +3707,34 @@ static ssize_t isoc_alt_store(struct device *dev,
 
 static DEVICE_ATTR_RW(isoc_alt);
 
+static struct attribute *btusb_sysfs_attrs[] = {
+	NULL,
+};
+ATTRIBUTE_GROUPS(btusb_sysfs);
+
+static void btusb_sysfs_release(struct device *dev)
+{
+	// Resource release is managed in btusb_disconnect
+}
+
+static const struct device_type btusb_sysfs = {
+	.name    = "btusb",
+	.release = btusb_sysfs_release,
+	.groups  = btusb_sysfs_groups,
+};
+
+static struct attribute *btusb_sysfs_isoc_alt_attrs[] = {
+	&dev_attr_isoc_alt.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(btusb_sysfs_isoc_alt);
+
+static const struct device_type btusb_sysfs_isoc_alt = {
+	.name    = "btusb",
+	.release = btusb_sysfs_release,
+	.groups  = btusb_sysfs_isoc_alt_groups,
+};
+
 static int btusb_probe(struct usb_interface *intf,
 		       const struct usb_device_id *id)
 {
@@ -3821,16 +3854,47 @@ static int btusb_probe(struct usb_interface *intf,
 
 	data->recv_acl = hci_recv_frame;
 
+	if (id->driver_info & BTUSB_AMP) {
+		/* AMP controllers do not support SCO packets */
+		data->isoc = NULL;
+	} else {
+		/* Interface orders are hardcoded in the specification */
+		data->isoc = usb_ifnum_to_if(data->udev, ifnum_base + 1);
+		data->isoc_ifnum = ifnum_base + 1;
+	}
+
+	if (id->driver_info & BTUSB_BROKEN_ISOC)
+		data->isoc = NULL;
+
+	/* Init a dev for btusb. The attr depends on the support of isoc. */
+	if (data->isoc)
+		data->dev.type = &btusb_sysfs_isoc_alt;
+	else
+		data->dev.type = &btusb_sysfs;
+	data->dev.class = &bt_class;
+	data->dev.parent = &intf->dev;
+
+	err = dev_set_name(&data->dev, "btusb%s", dev_name(&intf->dev));
+	if (err)
+		return err;
+
+	dev_set_drvdata(&data->dev, data);
+	err = device_register(&data->dev);
+	if (err < 0)
+		goto out_put_sysfs;
+
 	hdev = hci_alloc_dev_priv(priv_size);
-	if (!hdev)
-		return -ENOMEM;
+	if (!hdev) {
+		err = -ENOMEM;
+		goto out_free_sysfs;
+	}
 
 	hdev->bus = HCI_USB;
 	hci_set_drvdata(hdev, data);
 
 	data->hdev = hdev;
 
-	SET_HCIDEV_DEV(hdev, &intf->dev);
+	SET_HCIDEV_DEV(hdev, &data->dev);
 
 	reset_gpio = gpiod_get_optional(&data->udev->dev, "reset",
 					GPIOD_OUT_LOW);
@@ -3969,15 +4033,6 @@ static int btusb_probe(struct usb_interface *intf,
 		hci_set_msft_opcode(hdev, 0xFD70);
 	}
 
-	if (id->driver_info & BTUSB_AMP) {
-		/* AMP controllers do not support SCO packets */
-		data->isoc = NULL;
-	} else {
-		/* Interface orders are hardcoded in the specification */
-		data->isoc = usb_ifnum_to_if(data->udev, ifnum_base + 1);
-		data->isoc_ifnum = ifnum_base + 1;
-	}
-
 	if (IS_ENABLED(CONFIG_BT_HCIBTUSB_RTL) &&
 	    (id->driver_info & BTUSB_REALTEK)) {
 		btrtl_set_driver_name(hdev, btusb_driver.name);
@@ -4010,9 +4065,6 @@ static int btusb_probe(struct usb_interface *intf,
 			set_bit(HCI_QUIRK_FIXUP_BUFFER_SIZE, &hdev->quirks);
 	}
 
-	if (id->driver_info & BTUSB_BROKEN_ISOC)
-		data->isoc = NULL;
-
 	if (id->driver_info & BTUSB_WIDEBAND_SPEECH)
 		set_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED, &hdev->quirks);
 
@@ -4065,10 +4117,6 @@ static int btusb_probe(struct usb_interface *intf,
 						 data->isoc, data);
 		if (err < 0)
 			goto out_free_dev;
-
-		err = device_create_file(&intf->dev, &dev_attr_isoc_alt);
-		if (err)
-			goto out_free_dev;
 	}
 
 	if (IS_ENABLED(CONFIG_BT_HCIBTUSB_BCM) && data->diag) {
@@ -4099,6 +4147,13 @@ static int btusb_probe(struct usb_interface *intf,
 	if (data->reset_gpio)
 		gpiod_put(data->reset_gpio);
 	hci_free_dev(hdev);
+
+out_free_sysfs:
+	device_del(&data->dev);
+
+out_put_sysfs:
+	put_device(&data->dev);
+
 	return err;
 }
 
@@ -4115,10 +4170,8 @@ static void btusb_disconnect(struct usb_interface *intf)
 	hdev = data->hdev;
 	usb_set_intfdata(data->intf, NULL);
 
-	if (data->isoc) {
-		device_remove_file(&intf->dev, &dev_attr_isoc_alt);
+	if (data->isoc)
 		usb_set_intfdata(data->isoc, NULL);
-	}
 
 	if (data->diag)
 		usb_set_intfdata(data->diag, NULL);
@@ -4150,6 +4203,7 @@ static void btusb_disconnect(struct usb_interface *intf)
 		gpiod_put(data->reset_gpio);
 
 	hci_free_dev(hdev);
+	device_unregister(&data->dev);
 }
 
 #ifdef CONFIG_PM
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 05919848ea95..776dd6183509 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1843,6 +1843,7 @@ int hci_get_adv_monitor_offload_ext(struct hci_dev *hdev);
 
 void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb);
 
+extern const struct class bt_class;
 void hci_init_sysfs(struct hci_dev *hdev);
 void hci_conn_init_sysfs(struct hci_conn *conn);
 void hci_conn_add_sysfs(struct hci_conn *conn);
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index 041ce9adc378..aab3ffaa264c 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -6,9 +6,10 @@
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>
 
-static const struct class bt_class = {
+const struct class bt_class = {
 	.name = "bluetooth",
 };
+EXPORT_SYMBOL(bt_class);
 
 static void bt_link_release(struct device *dev)
 {
-- 
2.48.1.502.g6dc24dfdaf-goog

[PATCH v4 2/3] Bluetooth: Always allow SCO packets for user channel

[Hsin-chen Chuang]

From: Hsin-chen Chuang <chharry@chromium.org>

The SCO packets from Bluetooth raw socket are now rejected because
hci_conn_num is left 0. This patch allows such the usecase to enable
the userspace SCO support.

Fixes: b16b327edb4d ("Bluetooth: btusb: add sysfs attribute to control USB alt setting")
Signed-off-by: Hsin-chen Chuang <chharry@chromium.org>
---

(no changes since v2)

Changes in v2:
- Check HCI_USER_CHANNEL rather than just remove the hci_conn_num check

 drivers/bluetooth/btusb.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index cb3db18bb72c..01f84da27a30 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2132,7 +2132,8 @@ static int btusb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
 		return submit_or_queue_tx_urb(hdev, urb);
 
 	case HCI_SCODATA_PKT:
-		if (hci_conn_num(hdev, SCO_LINK) < 1)
+		if (!hci_dev_test_flag(hdev, HCI_USER_CHANNEL) &&
+		    hci_conn_num(hdev, SCO_LINK) < 1)
 			return -ENODEV;
 
 		urb = alloc_isoc_urb(hdev, skb);
@@ -2606,7 +2607,8 @@ static int btusb_send_frame_intel(struct hci_dev *hdev, struct sk_buff *skb)
 		return submit_or_queue_tx_urb(hdev, urb);
 
 	case HCI_SCODATA_PKT:
-		if (hci_conn_num(hdev, SCO_LINK) < 1)
+		if (!hci_dev_test_flag(hdev, HCI_USER_CHANNEL) &&
+		    hci_conn_num(hdev, SCO_LINK) < 1)
 			return -ENODEV;
 
 		urb = alloc_isoc_urb(hdev, skb);
-- 
2.48.1.502.g6dc24dfdaf-goog

[PATCH v4 3/3] Bluetooth: Add ABI doc for sysfs isoc_alt

[Hsin-chen Chuang]

From: Hsin-chen Chuang <chharry@chromium.org>

The functionality was completed in commit c04f2e4f4ea1 ("Bluetooth: Fix
possible race with userspace of sysfs isoc_alt")

Fixes: c04f2e4f4ea1 ("Bluetooth: Fix possible race with userspace of sysfs isoc_alt")
Signed-off-by: Hsin-chen Chuang <chharry@chromium.org>
---

(no changes since v1)

 Documentation/ABI/stable/sysfs-class-bluetooth | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/ABI/stable/sysfs-class-bluetooth b/Documentation/ABI/stable/sysfs-class-bluetooth
index 36be02471174..c1024c7c4634 100644
--- a/Documentation/ABI/stable/sysfs-class-bluetooth
+++ b/Documentation/ABI/stable/sysfs-class-bluetooth
@@ -7,3 +7,16 @@ Description: 	This write-only attribute allows users to trigger the vendor reset
 		The reset may or may not be done through the device transport
 		(e.g., UART/USB), and can also be done through an out-of-band
 		approach such as GPIO.
+
+What:		/sys/class/bluetooth/btusb<usb-intf>/isoc_alt
+Date:		13-Feb-2025
+KernelVersion:	6.13
+Contact:	linux-bluetooth@vger.kernel.org
+Description:	This attribute allows users to configure the USB Alternate setting
+		for the specific HCI device. Reading this attribute returns the
+		current setting, and writing any supported numbers would change
+		the setting. See the USB Alternate setting definition in Bluetooth
+		core spec 5, vol 4, part B, table 2.1.
+		If the HCI device is not yet init-ed, the write fails with -ENODEV.
+		If the data is not a valid number, the write fails with -EINVAL.
+		The other failures are vendor specific.
-- 
2.48.1.502.g6dc24dfdaf-goog

RE: [v4,1/3] Bluetooth: Fix possible race with userspace of sysfs isoc_alt

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2086 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=933448

---Test result---

Test Summary:
CheckPatch                    PENDING   0.19 seconds
GitLint                       PENDING   0.19 seconds
SubjectPrefix                 PASS      1.06 seconds
BuildKernel                   PASS      24.38 seconds
CheckAllWarning               PASS      26.74 seconds
CheckSparse                   PASS      30.64 seconds
BuildKernel32                 PASS      24.08 seconds
TestRunnerSetup               PASS      429.51 seconds
TestRunner_l2cap-tester       PASS      20.95 seconds
TestRunner_iso-tester         PASS      29.73 seconds
TestRunner_bnep-tester        PASS      4.81 seconds
TestRunner_mgmt-tester        PASS      120.39 seconds
TestRunner_rfcomm-tester      PASS      7.66 seconds
TestRunner_sco-tester         PASS      9.48 seconds
TestRunner_ioctl-tester       PASS      8.20 seconds
TestRunner_mesh-tester        FAIL      6.10 seconds
TestRunner_smp-tester         PASS      7.56 seconds
TestRunner_userchan-tester    PASS      5.09 seconds
IncrementalBuild              PENDING   0.74 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
BUG: KASAN: slab-use-after-free in run_timer_softirq+0x76f/0x7d0
WARNING: CPU: 0 PID: 66 at kernel/workqueue.c:2257 __queue_work+0x687/0xb40
Total: 10, Passed: 9 (90.0%), Failed: 1, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Failed       0.114 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

Re: [PATCH v4 1/3] Bluetooth: Fix possible race with userspace of sysfs isoc_alt

[Greg KH]

On Thu, Feb 13, 2025 at 11:43:59AM +0800, Hsin-chen Chuang wrote:
> From: Hsin-chen Chuang <chharry@chromium.org>
> 
> Expose the isoc_alt attr with device group to avoid the racing.
> 
> Now we create a dev node for btusb. The isoc_alt attr belongs to it and
> it also becomes the parent device of hci dev.
> 
> Fixes: b16b327edb4d ("Bluetooth: btusb: add sysfs attribute to control USB alt setting")
> Signed-off-by: Hsin-chen Chuang <chharry@chromium.org>
> ---
> 
> Changes in v4:
> - Create a dev node for btusb. It's now hci dev's parent and the
>   isoc_alt now belongs to it.
> - Since the changes is almost limitted in btusb, no need to add the
>   callbacks in hdev anymore.
> 
> Changes in v3:
> - Make the attribute exported only when the isoc_alt is available.
> - In btusb_probe, determine data->isoc before calling hci_alloc_dev_priv
>   (which calls hci_init_sysfs).
> - Since hci_init_sysfs is called before btusb could modify the hdev,
>   add new argument add_isoc_alt_attr for btusb to inform hci_init_sysfs.
> 
>  drivers/bluetooth/btusb.c        | 98 +++++++++++++++++++++++++-------
>  include/net/bluetooth/hci_core.h |  1 +
>  net/bluetooth/hci_sysfs.c        |  3 +-
>  3 files changed, 79 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 1caf7a071a73..cb3db18bb72c 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -920,6 +920,8 @@ struct btusb_data {
>  	int oob_wake_irq;   /* irq for out-of-band wake-on-bt */
>  
>  	struct qca_dump_info qca_dump;
> +
> +	struct device dev;
>  };
>  
>  static void btusb_reset(struct hci_dev *hdev)
> @@ -3693,6 +3695,9 @@ static ssize_t isoc_alt_store(struct device *dev,
>  	int alt;
>  	int ret;
>  
> +	if (!data->hdev)
> +		return -ENODEV;
> +
>  	if (kstrtoint(buf, 10, &alt))
>  		return -EINVAL;
>  
> @@ -3702,6 +3707,34 @@ static ssize_t isoc_alt_store(struct device *dev,
>  
>  static DEVICE_ATTR_RW(isoc_alt);
>  
> +static struct attribute *btusb_sysfs_attrs[] = {
> +	NULL,
> +};
> +ATTRIBUTE_GROUPS(btusb_sysfs);
> +
> +static void btusb_sysfs_release(struct device *dev)
> +{
> +	// Resource release is managed in btusb_disconnect

That is NOT how the driver model works, do NOT provide an empty
release() function just to silence the driver core from complaining
about it.

If for some reason the code thinks it is handling devices differently
than it should be, fix that instead.

thanks,

greg k-h

Re: [PATCH v4 3/3] Bluetooth: Add ABI doc for sysfs isoc_alt

[Greg KH]

On Thu, Feb 13, 2025 at 11:44:01AM +0800, Hsin-chen Chuang wrote:
> From: Hsin-chen Chuang <chharry@chromium.org>
> 
> The functionality was completed in commit c04f2e4f4ea1 ("Bluetooth: Fix
> possible race with userspace of sysfs isoc_alt")
> 
> Fixes: c04f2e4f4ea1 ("Bluetooth: Fix possible race with userspace of sysfs isoc_alt")

Where is this git id?  I don't see it in linux-next, are you sure it is
correct?

thanks,

greg k-h

Re: [PATCH v4 3/3] Bluetooth: Add ABI doc for sysfs isoc_alt

[Hsin-chen Chuang]

On Thu, Feb 13, 2025 at 6:01 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Feb 13, 2025 at 11:44:01AM +0800, Hsin-chen Chuang wrote:
> > From: Hsin-chen Chuang <chharry@chromium.org>
> >
> > The functionality was completed in commit c04f2e4f4ea1 ("Bluetooth: Fix
> > possible race with userspace of sysfs isoc_alt")
> >
> > Fixes: c04f2e4f4ea1 ("Bluetooth: Fix possible race with userspace of sysfs isoc_alt")
>
> Where is this git id?  I don't see it in linux-next, are you sure it is
> correct?

This hash is the first patch of this series. So I guess I should send
this patch after the ancestors are all applied then.

>
> thanks,
>
> greg k-h

-- 
Best Regards,
Hsin-chen

Re: [PATCH v4 3/3] Bluetooth: Add ABI doc for sysfs isoc_alt

[Greg KH]

On Thu, Feb 13, 2025 at 06:07:59PM +0800, Hsin-chen Chuang wrote:
> On Thu, Feb 13, 2025 at 6:01 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Thu, Feb 13, 2025 at 11:44:01AM +0800, Hsin-chen Chuang wrote:
> > > From: Hsin-chen Chuang <chharry@chromium.org>
> > >
> > > The functionality was completed in commit c04f2e4f4ea1 ("Bluetooth: Fix
> > > possible race with userspace of sysfs isoc_alt")
> > >
> > > Fixes: c04f2e4f4ea1 ("Bluetooth: Fix possible race with userspace of sysfs isoc_alt")
> >
> > Where is this git id?  I don't see it in linux-next, are you sure it is
> > correct?
> 
> This hash is the first patch of this series.

That's not a valid git id as it only exists on your local system and no
where else :(

> So I guess I should send this patch after the ancestors are all
> applied then.

Or better yet, include the api addition as part of the patch that
actually adds the new api.

thanks,

greg k-h

Re: [PATCH v4 3/3] Bluetooth: Add ABI doc for sysfs isoc_alt

[Hsin-chen Chuang]

Got it. Thanks for the feedback

On Thu, Feb 13, 2025 at 6:13 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Feb 13, 2025 at 06:07:59PM +0800, Hsin-chen Chuang wrote:
> > On Thu, Feb 13, 2025 at 6:01 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Thu, Feb 13, 2025 at 11:44:01AM +0800, Hsin-chen Chuang wrote:
> > > > From: Hsin-chen Chuang <chharry@chromium.org>
> > > >
> > > > The functionality was completed in commit c04f2e4f4ea1 ("Bluetooth: Fix
> > > > possible race with userspace of sysfs isoc_alt")
> > > >
> > > > Fixes: c04f2e4f4ea1 ("Bluetooth: Fix possible race with userspace of sysfs isoc_alt")
> > >
> > > Where is this git id?  I don't see it in linux-next, are you sure it is
> > > correct?
> >
> > This hash is the first patch of this series.
>
> That's not a valid git id as it only exists on your local system and no
> where else :(
>
> > So I guess I should send this patch after the ancestors are all
> > applied then.
>
> Or better yet, include the api addition as part of the patch that
> actually adds the new api.
>
> thanks,
>
> greg k-h

Re: [PATCH v4 1/3] Bluetooth: Fix possible race with userspace of sysfs isoc_alt

[Hsin-chen Chuang]

The btusb driver data is allocated by devm_kzalloc and is
automatically freed on driver detach, so I guess we don't have
anything to do here.
Or perhaps we should move btusb_disconnect's content here? Luiz, what
do you think?

On Thu, Feb 13, 2025 at 6:01 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Feb 13, 2025 at 11:43:59AM +0800, Hsin-chen Chuang wrote:
> > From: Hsin-chen Chuang <chharry@chromium.org>
> >
> > Expose the isoc_alt attr with device group to avoid the racing.
> >
> > Now we create a dev node for btusb. The isoc_alt attr belongs to it and
> > it also becomes the parent device of hci dev.
> >
> > Fixes: b16b327edb4d ("Bluetooth: btusb: add sysfs attribute to control USB alt setting")
> > Signed-off-by: Hsin-chen Chuang <chharry@chromium.org>
> > ---
> >
> > Changes in v4:
> > - Create a dev node for btusb. It's now hci dev's parent and the
> >   isoc_alt now belongs to it.
> > - Since the changes is almost limitted in btusb, no need to add the
> >   callbacks in hdev anymore.
> >
> > Changes in v3:
> > - Make the attribute exported only when the isoc_alt is available.
> > - In btusb_probe, determine data->isoc before calling hci_alloc_dev_priv
> >   (which calls hci_init_sysfs).
> > - Since hci_init_sysfs is called before btusb could modify the hdev,
> >   add new argument add_isoc_alt_attr for btusb to inform hci_init_sysfs.
> >
> >  drivers/bluetooth/btusb.c        | 98 +++++++++++++++++++++++++-------
> >  include/net/bluetooth/hci_core.h |  1 +
> >  net/bluetooth/hci_sysfs.c        |  3 +-
> >  3 files changed, 79 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index 1caf7a071a73..cb3db18bb72c 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -920,6 +920,8 @@ struct btusb_data {
> >       int oob_wake_irq;   /* irq for out-of-band wake-on-bt */
> >
> >       struct qca_dump_info qca_dump;
> > +
> > +     struct device dev;
> >  };
> >
> >  static void btusb_reset(struct hci_dev *hdev)
> > @@ -3693,6 +3695,9 @@ static ssize_t isoc_alt_store(struct device *dev,
> >       int alt;
> >       int ret;
> >
> > +     if (!data->hdev)
> > +             return -ENODEV;
> > +
> >       if (kstrtoint(buf, 10, &alt))
> >               return -EINVAL;
> >
> > @@ -3702,6 +3707,34 @@ static ssize_t isoc_alt_store(struct device *dev,
> >
> >  static DEVICE_ATTR_RW(isoc_alt);
> >
> > +static struct attribute *btusb_sysfs_attrs[] = {
> > +     NULL,
> > +};
> > +ATTRIBUTE_GROUPS(btusb_sysfs);
> > +
> > +static void btusb_sysfs_release(struct device *dev)
> > +{
> > +     // Resource release is managed in btusb_disconnect
>
> That is NOT how the driver model works, do NOT provide an empty
> release() function just to silence the driver core from complaining
> about it.
>
> If for some reason the code thinks it is handling devices differently
> than it should be, fix that instead.
>
> thanks,
>
> greg k-h

-- 
Best Regards,
Hsin-chen

Re: [PATCH v4 1/3] Bluetooth: Fix possible race with userspace of sysfs isoc_alt

[Greg KH]

A: http://en.wikipedia.org/wiki/Top_post
Q: Were do I find info about this thing called top-posting?
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Thu, Feb 13, 2025 at 07:57:15PM +0800, Hsin-chen Chuang wrote:
> The btusb driver data is allocated by devm_kzalloc and is
> automatically freed on driver detach, so I guess we don't have
> anything to do here.

What?  A struct device should NEVER be allocated with devm_kzalloc.
That's just not going to work at all.

> Or perhaps we should move btusb_disconnect's content here? Luiz, what
> do you think?

I think something is really wrong here.  Why are you adding a new struct
device to the system?  What requires that?  What is this new device
going to be used for?

confused,


greg k-h

Re: [PATCH v4 1/3] Bluetooth: Fix possible race with userspace of sysfs isoc_alt

[Hsin-chen Chuang]

On Thu, Feb 13, 2025 at 8:10 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> A: http://en.wikipedia.org/wiki/Top_post
> Q: Were do I find info about this thing called top-posting?
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?
> A: Top-posting.
> Q: What is the most annoying thing in e-mail?
>
> A: No.
> Q: Should I include quotations after my reply?
>
> http://daringfireball.net/2007/07/on_top
>
> On Thu, Feb 13, 2025 at 07:57:15PM +0800, Hsin-chen Chuang wrote:
> > The btusb driver data is allocated by devm_kzalloc and is
> > automatically freed on driver detach, so I guess we don't have
> > anything to do here.
>
> What?  A struct device should NEVER be allocated with devm_kzalloc.
> That's just not going to work at all.

Noted. Perhaps that needs to be refactored together.

>
> > Or perhaps we should move btusb_disconnect's content here? Luiz, what
> > do you think?
>
> I think something is really wrong here.  Why are you adding a new struct
> device to the system?  What requires that?  What is this new device
> going to be used for?

The new device is only for exposing a new sysfs attribute.

So originally we had a device called hci_dev, indicating the
implementation of the Bluetooth HCI layer. hci_dev is directly the
child of the usb_interface (the Bluetooth chip connected through USB).
Now I would like to add an attribute for something that's not defined
in the HCI layer, but lower layer only in Bluetooth USB.
Thus we want to rephrase the structure: usb_interface -> btusb (new
device) -> hci_dev, and then we could place the new attribute in the
new device.

Basically I kept the memory management in btusb unchanged in this
patch, as the new device is only used for a new attribute.
Would you suggest we revise the memory management since we added a
device in this module?

>
> confused,
>
>
> greg k-h

-- 
Best Regards,
Hsin-chen

Re: [PATCH v4 1/3] Bluetooth: Fix possible race with userspace of sysfs isoc_alt

[Greg KH]

On Thu, Feb 13, 2025 at 09:33:34PM +0800, Hsin-chen Chuang wrote:
> On Thu, Feb 13, 2025 at 8:10 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > A: http://en.wikipedia.org/wiki/Top_post
> > Q: Were do I find info about this thing called top-posting?
> > A: Because it messes up the order in which people normally read text.
> > Q: Why is top-posting such a bad thing?
> > A: Top-posting.
> > Q: What is the most annoying thing in e-mail?
> >
> > A: No.
> > Q: Should I include quotations after my reply?
> >
> > http://daringfireball.net/2007/07/on_top
> >
> > On Thu, Feb 13, 2025 at 07:57:15PM +0800, Hsin-chen Chuang wrote:
> > > The btusb driver data is allocated by devm_kzalloc and is
> > > automatically freed on driver detach, so I guess we don't have
> > > anything to do here.
> >
> > What?  A struct device should NEVER be allocated with devm_kzalloc.
> > That's just not going to work at all.
> 
> Noted. Perhaps that needs to be refactored together.
> 
> >
> > > Or perhaps we should move btusb_disconnect's content here? Luiz, what
> > > do you think?
> >
> > I think something is really wrong here.  Why are you adding a new struct
> > device to the system?  What requires that?  What is this new device
> > going to be used for?
> 
> The new device is only for exposing a new sysfs attribute.

That feels crazy.

> So originally we had a device called hci_dev, indicating the
> implementation of the Bluetooth HCI layer. hci_dev is directly the
> child of the usb_interface (the Bluetooth chip connected through USB).
> Now I would like to add an attribute for something that's not defined
> in the HCI layer, but lower layer only in Bluetooth USB.
> Thus we want to rephrase the structure: usb_interface -> btusb (new
> device) -> hci_dev, and then we could place the new attribute in the
> new device.
> 
> Basically I kept the memory management in btusb unchanged in this
> patch, as the new device is only used for a new attribute.
> Would you suggest we revise the memory management since we added a
> device in this module?

If you add a new device in the tree, it HAS to work properly with the
driver core (i.e. life cycles are unique, you can't have empty release
functions, etc.)  Put it on the proper bus it belongs to, bind the
needed drivers to it, and have it work that way, don't make a "fake"
device for no good reason.

thanks,

greg k-h

Re: [PATCH v4 1/3] Bluetooth: Fix possible race with userspace of sysfs isoc_alt

[Hsin-chen Chuang]

On Thu, Feb 13, 2025 at 9:45 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Feb 13, 2025 at 09:33:34PM +0800, Hsin-chen Chuang wrote:
> > On Thu, Feb 13, 2025 at 8:10 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > A: http://en.wikipedia.org/wiki/Top_post
> > > Q: Were do I find info about this thing called top-posting?
> > > A: Because it messes up the order in which people normally read text.
> > > Q: Why is top-posting such a bad thing?
> > > A: Top-posting.
> > > Q: What is the most annoying thing in e-mail?
> > >
> > > A: No.
> > > Q: Should I include quotations after my reply?
> > >
> > > http://daringfireball.net/2007/07/on_top
> > >
> > > On Thu, Feb 13, 2025 at 07:57:15PM +0800, Hsin-chen Chuang wrote:
> > > > The btusb driver data is allocated by devm_kzalloc and is
> > > > automatically freed on driver detach, so I guess we don't have
> > > > anything to do here.
> > >
> > > What?  A struct device should NEVER be allocated with devm_kzalloc.
> > > That's just not going to work at all.
> >
> > Noted. Perhaps that needs to be refactored together.
> >
> > >
> > > > Or perhaps we should move btusb_disconnect's content here? Luiz, what
> > > > do you think?
> > >
> > > I think something is really wrong here.  Why are you adding a new struct
> > > device to the system?  What requires that?  What is this new device
> > > going to be used for?
> >
> > The new device is only for exposing a new sysfs attribute.
>
> That feels crazy.
>
> > So originally we had a device called hci_dev, indicating the
> > implementation of the Bluetooth HCI layer. hci_dev is directly the
> > child of the usb_interface (the Bluetooth chip connected through USB).
> > Now I would like to add an attribute for something that's not defined
> > in the HCI layer, but lower layer only in Bluetooth USB.
> > Thus we want to rephrase the structure: usb_interface -> btusb (new
> > device) -> hci_dev, and then we could place the new attribute in the
> > new device.
> >
> > Basically I kept the memory management in btusb unchanged in this
> > patch, as the new device is only used for a new attribute.
> > Would you suggest we revise the memory management since we added a
> > device in this module?
>
> If you add a new device in the tree, it HAS to work properly with the
> driver core (i.e. life cycles are unique, you can't have empty release
> functions, etc.)  Put it on the proper bus it belongs to, bind the
> needed drivers to it, and have it work that way, don't make a "fake"
> device for no good reason.

Got it. Thanks for the info.

Hi Luiz, I will work on v5 to make btusb resources managed by device.
Any concern?

>
> thanks,

>
> greg k-h

--
Best Regards,
Hsin-chen

Re: [PATCH v4 1/3] Bluetooth: Fix possible race with userspace of sysfs isoc_alt

[Luiz Augusto von Dentz]

Hi Greg,

On Thu, Feb 13, 2025 at 8:45 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Feb 13, 2025 at 09:33:34PM +0800, Hsin-chen Chuang wrote:
> > On Thu, Feb 13, 2025 at 8:10 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > A: http://en.wikipedia.org/wiki/Top_post
> > > Q: Were do I find info about this thing called top-posting?
> > > A: Because it messes up the order in which people normally read text.
> > > Q: Why is top-posting such a bad thing?
> > > A: Top-posting.
> > > Q: What is the most annoying thing in e-mail?
> > >
> > > A: No.
> > > Q: Should I include quotations after my reply?
> > >
> > > http://daringfireball.net/2007/07/on_top
> > >
> > > On Thu, Feb 13, 2025 at 07:57:15PM +0800, Hsin-chen Chuang wrote:
> > > > The btusb driver data is allocated by devm_kzalloc and is
> > > > automatically freed on driver detach, so I guess we don't have
> > > > anything to do here.
> > >
> > > What?  A struct device should NEVER be allocated with devm_kzalloc.
> > > That's just not going to work at all.
> >
> > Noted. Perhaps that needs to be refactored together.
> >
> > >
> > > > Or perhaps we should move btusb_disconnect's content here? Luiz, what
> > > > do you think?
> > >
> > > I think something is really wrong here.  Why are you adding a new struct
> > > device to the system?  What requires that?  What is this new device
> > > going to be used for?
> >
> > The new device is only for exposing a new sysfs attribute.
>
> That feels crazy.
>
> > So originally we had a device called hci_dev, indicating the
> > implementation of the Bluetooth HCI layer. hci_dev is directly the
> > child of the usb_interface (the Bluetooth chip connected through USB).
> > Now I would like to add an attribute for something that's not defined
> > in the HCI layer, but lower layer only in Bluetooth USB.
> > Thus we want to rephrase the structure: usb_interface -> btusb (new
> > device) -> hci_dev, and then we could place the new attribute in the
> > new device.
> >
> > Basically I kept the memory management in btusb unchanged in this
> > patch, as the new device is only used for a new attribute.
> > Would you suggest we revise the memory management since we added a
> > device in this module?
>
> If you add a new device in the tree, it HAS to work properly with the
> driver core (i.e. life cycles are unique, you can't have empty release
> functions, etc.)  Put it on the proper bus it belongs to, bind the
> needed drivers to it, and have it work that way, don't make a "fake"
> device for no good reason.

Well we could just introduce it to USB device, since alternate setting
is a concept that is coming from there, but apparently the likes of
/sys/bus/usb/devices/usbX/bAlternateSetting is read-only, some
Bluetooth profiles (HFP) requires switching the alternate setting and
because Google is switching to handle this via userspace thus why
there was this request to add a new sysfs to control it.

-- 
Luiz Augusto von Dentz

Re: [PATCH v4 1/3] Bluetooth: Fix possible race with userspace of sysfs isoc_alt

[Greg KH]

On Thu, Feb 13, 2025 at 09:22:28AM -0500, Luiz Augusto von Dentz wrote:
> Hi Greg,
> 
> On Thu, Feb 13, 2025 at 8:45 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Thu, Feb 13, 2025 at 09:33:34PM +0800, Hsin-chen Chuang wrote:
> > > On Thu, Feb 13, 2025 at 8:10 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > A: http://en.wikipedia.org/wiki/Top_post
> > > > Q: Were do I find info about this thing called top-posting?
> > > > A: Because it messes up the order in which people normally read text.
> > > > Q: Why is top-posting such a bad thing?
> > > > A: Top-posting.
> > > > Q: What is the most annoying thing in e-mail?
> > > >
> > > > A: No.
> > > > Q: Should I include quotations after my reply?
> > > >
> > > > http://daringfireball.net/2007/07/on_top
> > > >
> > > > On Thu, Feb 13, 2025 at 07:57:15PM +0800, Hsin-chen Chuang wrote:
> > > > > The btusb driver data is allocated by devm_kzalloc and is
> > > > > automatically freed on driver detach, so I guess we don't have
> > > > > anything to do here.
> > > >
> > > > What?  A struct device should NEVER be allocated with devm_kzalloc.
> > > > That's just not going to work at all.
> > >
> > > Noted. Perhaps that needs to be refactored together.
> > >
> > > >
> > > > > Or perhaps we should move btusb_disconnect's content here? Luiz, what
> > > > > do you think?
> > > >
> > > > I think something is really wrong here.  Why are you adding a new struct
> > > > device to the system?  What requires that?  What is this new device
> > > > going to be used for?
> > >
> > > The new device is only for exposing a new sysfs attribute.
> >
> > That feels crazy.
> >
> > > So originally we had a device called hci_dev, indicating the
> > > implementation of the Bluetooth HCI layer. hci_dev is directly the
> > > child of the usb_interface (the Bluetooth chip connected through USB).
> > > Now I would like to add an attribute for something that's not defined
> > > in the HCI layer, but lower layer only in Bluetooth USB.
> > > Thus we want to rephrase the structure: usb_interface -> btusb (new
> > > device) -> hci_dev, and then we could place the new attribute in the
> > > new device.
> > >
> > > Basically I kept the memory management in btusb unchanged in this
> > > patch, as the new device is only used for a new attribute.
> > > Would you suggest we revise the memory management since we added a
> > > device in this module?
> >
> > If you add a new device in the tree, it HAS to work properly with the
> > driver core (i.e. life cycles are unique, you can't have empty release
> > functions, etc.)  Put it on the proper bus it belongs to, bind the
> > needed drivers to it, and have it work that way, don't make a "fake"
> > device for no good reason.
> 
> Well we could just introduce it to USB device, since alternate setting
> is a concept that is coming from there, but apparently the likes of
> /sys/bus/usb/devices/usbX/bAlternateSetting is read-only, some
> Bluetooth profiles (HFP) requires switching the alternate setting and
> because Google is switching to handle this via userspace thus why
> there was this request to add a new sysfs to control it.

That's fine, just don't add devices where there shouldn't be devices, or
if you do, make them actually work properly (i.e. do NOT have empty
release callbacks...)

If you want to switch alternate settings in a USB device, do it the
normal way from userspace that has been there for decades!  Don't make
up some other random sysfs file for this please, that would be crazy,
and wrong.

So what's wrong with the current api we have today that doesn't work for
bluetooth devices?

thanks,

greg k-h

Re: [PATCH v4 1/3] Bluetooth: Fix possible race with userspace of sysfs isoc_alt

[Luiz Augusto von Dentz]

Hi Greg,

On Thu, Feb 13, 2025 at 10:39 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Feb 13, 2025 at 09:22:28AM -0500, Luiz Augusto von Dentz wrote:
> > Hi Greg,
> >
> > On Thu, Feb 13, 2025 at 8:45 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Thu, Feb 13, 2025 at 09:33:34PM +0800, Hsin-chen Chuang wrote:
> > > > On Thu, Feb 13, 2025 at 8:10 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > >
> > > > > A: http://en.wikipedia.org/wiki/Top_post
> > > > > Q: Were do I find info about this thing called top-posting?
> > > > > A: Because it messes up the order in which people normally read text.
> > > > > Q: Why is top-posting such a bad thing?
> > > > > A: Top-posting.
> > > > > Q: What is the most annoying thing in e-mail?
> > > > >
> > > > > A: No.
> > > > > Q: Should I include quotations after my reply?
> > > > >
> > > > > http://daringfireball.net/2007/07/on_top
> > > > >
> > > > > On Thu, Feb 13, 2025 at 07:57:15PM +0800, Hsin-chen Chuang wrote:
> > > > > > The btusb driver data is allocated by devm_kzalloc and is
> > > > > > automatically freed on driver detach, so I guess we don't have
> > > > > > anything to do here.
> > > > >
> > > > > What?  A struct device should NEVER be allocated with devm_kzalloc.
> > > > > That's just not going to work at all.
> > > >
> > > > Noted. Perhaps that needs to be refactored together.
> > > >
> > > > >
> > > > > > Or perhaps we should move btusb_disconnect's content here? Luiz, what
> > > > > > do you think?
> > > > >
> > > > > I think something is really wrong here.  Why are you adding a new struct
> > > > > device to the system?  What requires that?  What is this new device
> > > > > going to be used for?
> > > >
> > > > The new device is only for exposing a new sysfs attribute.
> > >
> > > That feels crazy.
> > >
> > > > So originally we had a device called hci_dev, indicating the
> > > > implementation of the Bluetooth HCI layer. hci_dev is directly the
> > > > child of the usb_interface (the Bluetooth chip connected through USB).
> > > > Now I would like to add an attribute for something that's not defined
> > > > in the HCI layer, but lower layer only in Bluetooth USB.
> > > > Thus we want to rephrase the structure: usb_interface -> btusb (new
> > > > device) -> hci_dev, and then we could place the new attribute in the
> > > > new device.
> > > >
> > > > Basically I kept the memory management in btusb unchanged in this
> > > > patch, as the new device is only used for a new attribute.
> > > > Would you suggest we revise the memory management since we added a
> > > > device in this module?
> > >
> > > If you add a new device in the tree, it HAS to work properly with the
> > > driver core (i.e. life cycles are unique, you can't have empty release
> > > functions, etc.)  Put it on the proper bus it belongs to, bind the
> > > needed drivers to it, and have it work that way, don't make a "fake"
> > > device for no good reason.
> >
> > Well we could just introduce it to USB device, since alternate setting
> > is a concept that is coming from there, but apparently the likes of
> > /sys/bus/usb/devices/usbX/bAlternateSetting is read-only, some
> > Bluetooth profiles (HFP) requires switching the alternate setting and
> > because Google is switching to handle this via userspace thus why
> > there was this request to add a new sysfs to control it.
>
> That's fine, just don't add devices where there shouldn't be devices, or
> if you do, make them actually work properly (i.e. do NOT have empty
> release callbacks...)
>
> If you want to switch alternate settings in a USB device, do it the
> normal way from userspace that has been there for decades!  Don't make
> up some other random sysfs file for this please, that would be crazy,
> and wrong.
>
> So what's wrong with the current api we have today that doesn't work for
> bluetooth devices?

Perhaps it is just lack of knowledge then, how userspace can request
to switch alternate settings? If I recall correctly Hsin-chen tried
with libusb, or something like that, but it didn't work for some
reason.

@Hsin-chen Chuang I hope you can fill in the details since for the
most part this is not a BlueZ feature. I'm just explaining what you
guys are after but ultimately it is up to Google to drive this.

-- 
Luiz Augusto von Dentz

Re: [PATCH v4 1/3] Bluetooth: Fix possible race with userspace of sysfs isoc_alt

[Greg KH]

On Thu, Feb 13, 2025 at 10:56:46AM -0500, Luiz Augusto von Dentz wrote:
> Hi Greg,
> 
> On Thu, Feb 13, 2025 at 10:39 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Thu, Feb 13, 2025 at 09:22:28AM -0500, Luiz Augusto von Dentz wrote:
> > > Hi Greg,
> > >
> > > On Thu, Feb 13, 2025 at 8:45 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Thu, Feb 13, 2025 at 09:33:34PM +0800, Hsin-chen Chuang wrote:
> > > > > On Thu, Feb 13, 2025 at 8:10 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > > >
> > > > > > A: http://en.wikipedia.org/wiki/Top_post
> > > > > > Q: Were do I find info about this thing called top-posting?
> > > > > > A: Because it messes up the order in which people normally read text.
> > > > > > Q: Why is top-posting such a bad thing?
> > > > > > A: Top-posting.
> > > > > > Q: What is the most annoying thing in e-mail?
> > > > > >
> > > > > > A: No.
> > > > > > Q: Should I include quotations after my reply?
> > > > > >
> > > > > > http://daringfireball.net/2007/07/on_top
> > > > > >
> > > > > > On Thu, Feb 13, 2025 at 07:57:15PM +0800, Hsin-chen Chuang wrote:
> > > > > > > The btusb driver data is allocated by devm_kzalloc and is
> > > > > > > automatically freed on driver detach, so I guess we don't have
> > > > > > > anything to do here.
> > > > > >
> > > > > > What?  A struct device should NEVER be allocated with devm_kzalloc.
> > > > > > That's just not going to work at all.
> > > > >
> > > > > Noted. Perhaps that needs to be refactored together.
> > > > >
> > > > > >
> > > > > > > Or perhaps we should move btusb_disconnect's content here? Luiz, what
> > > > > > > do you think?
> > > > > >
> > > > > > I think something is really wrong here.  Why are you adding a new struct
> > > > > > device to the system?  What requires that?  What is this new device
> > > > > > going to be used for?
> > > > >
> > > > > The new device is only for exposing a new sysfs attribute.
> > > >
> > > > That feels crazy.
> > > >
> > > > > So originally we had a device called hci_dev, indicating the
> > > > > implementation of the Bluetooth HCI layer. hci_dev is directly the
> > > > > child of the usb_interface (the Bluetooth chip connected through USB).
> > > > > Now I would like to add an attribute for something that's not defined
> > > > > in the HCI layer, but lower layer only in Bluetooth USB.
> > > > > Thus we want to rephrase the structure: usb_interface -> btusb (new
> > > > > device) -> hci_dev, and then we could place the new attribute in the
> > > > > new device.
> > > > >
> > > > > Basically I kept the memory management in btusb unchanged in this
> > > > > patch, as the new device is only used for a new attribute.
> > > > > Would you suggest we revise the memory management since we added a
> > > > > device in this module?
> > > >
> > > > If you add a new device in the tree, it HAS to work properly with the
> > > > driver core (i.e. life cycles are unique, you can't have empty release
> > > > functions, etc.)  Put it on the proper bus it belongs to, bind the
> > > > needed drivers to it, and have it work that way, don't make a "fake"
> > > > device for no good reason.
> > >
> > > Well we could just introduce it to USB device, since alternate setting
> > > is a concept that is coming from there, but apparently the likes of
> > > /sys/bus/usb/devices/usbX/bAlternateSetting is read-only, some
> > > Bluetooth profiles (HFP) requires switching the alternate setting and
> > > because Google is switching to handle this via userspace thus why
> > > there was this request to add a new sysfs to control it.
> >
> > That's fine, just don't add devices where there shouldn't be devices, or
> > if you do, make them actually work properly (i.e. do NOT have empty
> > release callbacks...)
> >
> > If you want to switch alternate settings in a USB device, do it the
> > normal way from userspace that has been there for decades!  Don't make
> > up some other random sysfs file for this please, that would be crazy,
> > and wrong.
> >
> > So what's wrong with the current api we have today that doesn't work for
> > bluetooth devices?
> 
> Perhaps it is just lack of knowledge then, how userspace can request
> to switch alternate settings? If I recall correctly Hsin-chen tried
> with libusb, or something like that, but it didn't work for some
> reason.

The USBDEVFS_SETINTERFACE usbfs ioctl should do this.  If not, pleaase
let us know.  libusb supports this through the
libusb_set_interface_alt_setting() function.

Here's an 11 year old stackoverflow question about this:
	https://stackoverflow.com/questions/17546709/what-does-libusb-set-interface-alt-setting-do

thanks,

greg k-h