public inbox for linux-mediatek@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH v2] Bluetooth: btmtk: add event filter to filter specific event
@ 2026-04-07 11:48 Chris Lu
  2026-04-07 14:54 ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 3+ messages in thread
From: Chris Lu @ 2026-04-07 11:48 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg, Luiz Von Dentz
  Cc: Sean Wang, Will Lee, SS Wu, Steve Lee, linux-bluetooth,
	linux-kernel, linux-mediatek, Chris Lu

Add an event filter to filter event with specific opcode to prevent BT
stack from receiving unexpected event.

Event with opcode 0xfc5d is generated when MediaTek's Bluetooth enable
firmware logs and is not expected to be sent to userspace.

Signed-off-by: Chris Lu <chris.lu@mediatek.com>
---
v1 -> v2: update commit message
---
 drivers/bluetooth/btmtk.c | 22 ++++++++++++++++++++++
 drivers/bluetooth/btmtk.h |  7 +++++++
 drivers/bluetooth/btusb.c |  2 ++
 3 files changed, 31 insertions(+)

diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
index 55516b4602db..302d6ddf9062 100644
--- a/drivers/bluetooth/btmtk.c
+++ b/drivers/bluetooth/btmtk.c
@@ -1503,6 +1503,28 @@ int btmtk_usb_shutdown(struct hci_dev *hdev)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(btmtk_usb_shutdown);
+
+int btmtk_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct hci_event_hdr *hdr = (void *)skb->data;
+
+	if (hdr->evt == HCI_EV_CMD_COMPLETE) {
+		struct hci_ev_cmd_complete *ec;
+		u16 opcode;
+
+		ec = (void *)(skb->data + HCI_EVENT_HDR_SIZE);
+		opcode = __le16_to_cpu(ec->opcode);
+
+		/* Filter vendor opcode */
+		if (opcode == 0xfc5d) {
+			kfree_skb(skb);
+			return 0;
+		}
+	}
+
+	return hci_recv_frame(hdev, skb);
+}
+EXPORT_SYMBOL_GPL(btmtk_recv_event);
 #endif
 
 MODULE_AUTHOR("Sean Wang <sean.wang@mediatek.com>");
diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h
index adaf385626ee..08b73927c8f3 100644
--- a/drivers/bluetooth/btmtk.h
+++ b/drivers/bluetooth/btmtk.h
@@ -218,6 +218,8 @@ int btmtk_usb_suspend(struct hci_dev *hdev);
 int btmtk_usb_setup(struct hci_dev *hdev);
 
 int btmtk_usb_shutdown(struct hci_dev *hdev);
+
+int btmtk_recv_event(struct hci_dev *hdev, struct sk_buff *skb);
 #else
 
 static inline int btmtk_set_bdaddr(struct hci_dev *hdev,
@@ -296,4 +298,9 @@ static inline int btmtk_usb_shutdown(struct hci_dev *hdev)
 {
 	return -EOPNOTSUPP;
 }
+
+static inline int btmtk_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	return -EOPNOTSUPP;
+}
 #endif
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index f9d515ee9124..daf8a387e660 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -4150,6 +4150,8 @@ static int btusb_probe(struct usb_interface *intf,
 	} else if (id->driver_info & BTUSB_MEDIATEK) {
 		/* Allocate extra space for Mediatek device */
 		priv_size += sizeof(struct btmtk_data);
+
+		data->recv_event = btmtk_recv_event;
 	}
 
 	data->recv_acl = hci_recv_frame;
-- 
2.45.2



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] Bluetooth: btmtk: add event filter to filter specific event
  2026-04-07 11:48 [PATCH v2] Bluetooth: btmtk: add event filter to filter specific event Chris Lu
@ 2026-04-07 14:54 ` Luiz Augusto von Dentz
  2026-04-08 11:28   ` Chris Lu (陸稚泓)
  0 siblings, 1 reply; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2026-04-07 14:54 UTC (permalink / raw)
  To: Chris Lu
  Cc: Marcel Holtmann, Johan Hedberg, Sean Wang, Will Lee, SS Wu,
	Steve Lee, linux-bluetooth, linux-kernel, linux-mediatek

Hi Chris,

On Tue, Apr 7, 2026 at 7:48 AM Chris Lu <chris.lu@mediatek.com> wrote:
>
> Add an event filter to filter event with specific opcode to prevent BT
> stack from receiving unexpected event.
>
> Event with opcode 0xfc5d is generated when MediaTek's Bluetooth enable
> firmware logs and is not expected to be sent to userspace.
>
> Signed-off-by: Chris Lu <chris.lu@mediatek.com>
> ---
> v1 -> v2: update commit message
> ---
>  drivers/bluetooth/btmtk.c | 22 ++++++++++++++++++++++
>  drivers/bluetooth/btmtk.h |  7 +++++++
>  drivers/bluetooth/btusb.c |  2 ++
>  3 files changed, 31 insertions(+)
>
> diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
> index 55516b4602db..302d6ddf9062 100644
> --- a/drivers/bluetooth/btmtk.c
> +++ b/drivers/bluetooth/btmtk.c
> @@ -1503,6 +1503,28 @@ int btmtk_usb_shutdown(struct hci_dev *hdev)
>         return 0;
>  }
>  EXPORT_SYMBOL_GPL(btmtk_usb_shutdown);
> +
> +int btmtk_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +       struct hci_event_hdr *hdr = (void *)skb->data;
> +
> +       if (hdr->evt == HCI_EV_CMD_COMPLETE) {
> +               struct hci_ev_cmd_complete *ec;
> +               u16 opcode;
> +
> +               ec = (void *)(skb->data + HCI_EVENT_HDR_SIZE);
> +               opcode = __le16_to_cpu(ec->opcode);
> +
> +               /* Filter vendor opcode */
> +               if (opcode == 0xfc5d) {
> +                       kfree_skb(skb);
> +                       return 0;
> +               }
> +       }
> +
> +       return hci_recv_frame(hdev, skb);
> +}
> +EXPORT_SYMBOL_GPL(btmtk_recv_event);
>  #endif
>
>  MODULE_AUTHOR("Sean Wang <sean.wang@mediatek.com>");
> diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h
> index adaf385626ee..08b73927c8f3 100644
> --- a/drivers/bluetooth/btmtk.h
> +++ b/drivers/bluetooth/btmtk.h
> @@ -218,6 +218,8 @@ int btmtk_usb_suspend(struct hci_dev *hdev);
>  int btmtk_usb_setup(struct hci_dev *hdev);
>
>  int btmtk_usb_shutdown(struct hci_dev *hdev);
> +
> +int btmtk_recv_event(struct hci_dev *hdev, struct sk_buff *skb);
>  #else
>
>  static inline int btmtk_set_bdaddr(struct hci_dev *hdev,
> @@ -296,4 +298,9 @@ static inline int btmtk_usb_shutdown(struct hci_dev *hdev)
>  {
>         return -EOPNOTSUPP;
>  }
> +
> +static inline int btmtk_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +       return -EOPNOTSUPP;
> +}
>  #endif
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index f9d515ee9124..daf8a387e660 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -4150,6 +4150,8 @@ static int btusb_probe(struct usb_interface *intf,
>         } else if (id->driver_info & BTUSB_MEDIATEK) {
>                 /* Allocate extra space for Mediatek device */
>                 priv_size += sizeof(struct btmtk_data);
> +
> +               data->recv_event = btmtk_recv_event;
>         }
>
>         data->recv_acl = hci_recv_frame;
> --
> 2.45.2


https://sashiko.dev/#/patchset/20260407114851.3087886-1-chris.lu%40mediatek.com

Both issues seem valid to me. That said, I wonder if it wouldn't have
been better to allow drivers to register with its own struct
hci_ev/struct hci_cc tables, that way the driver can extend event
parsing rather than intercepting it and performing custom parsing
which can lead to bugs like the ones mentioned above.


-- 
Luiz Augusto von Dentz


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] Bluetooth: btmtk: add event filter to filter specific event
  2026-04-07 14:54 ` Luiz Augusto von Dentz
@ 2026-04-08 11:28   ` Chris Lu (陸稚泓)
  0 siblings, 0 replies; 3+ messages in thread
From: Chris Lu (陸稚泓) @ 2026-04-08 11:28 UTC (permalink / raw)
  To: luiz.dentz@gmail.com
  Cc: Will-CY Lee (李政穎),
	Steve Lee (李視誠), marcel@holtmann.org,
	SS Wu (巫憲欣), linux-kernel@vger.kernel.org,
	johan.hedberg@gmail.com, Sean Wang,
	linux-bluetooth@vger.kernel.org,
	linux-mediatek@lists.infradead.org

Hi Luiz,

Thanks for your reply.

On Tue, 2026-04-07 at 10:54 -0400, Luiz Augusto von Dentz wrote:
> Hi Chris,
> 
> On Tue, Apr 7, 2026 at 7:48 AM Chris Lu <chris.lu@mediatek.com>
> wrote:
> > 
> > Add an event filter to filter event with specific opcode to prevent
> > BT
> > stack from receiving unexpected event.
> > 
> > Event with opcode 0xfc5d is generated when MediaTek's Bluetooth
> > enable
> > firmware logs and is not expected to be sent to userspace.
> > 
> > Signed-off-by: Chris Lu <chris.lu@mediatek.com>
> > ---
> > v1 -> v2: update commit message
> > ---
> >  drivers/bluetooth/btmtk.c | 22 ++++++++++++++++++++++
> >  drivers/bluetooth/btmtk.h |  7 +++++++
> >  drivers/bluetooth/btusb.c |  2 ++
> >  3 files changed, 31 insertions(+)
> > 
> > diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
> > index 55516b4602db..302d6ddf9062 100644
> > --- a/drivers/bluetooth/btmtk.c
> > +++ b/drivers/bluetooth/btmtk.c
> > @@ -1503,6 +1503,28 @@ int btmtk_usb_shutdown(struct hci_dev *hdev)
> >         return 0;
> >  }
> >  EXPORT_SYMBOL_GPL(btmtk_usb_shutdown);
> > +
> > +int btmtk_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
> > +{
> > +       struct hci_event_hdr *hdr = (void *)skb->data;
> > +
> > +       if (hdr->evt == HCI_EV_CMD_COMPLETE) {
> > +               struct hci_ev_cmd_complete *ec;
> > +               u16 opcode;
> > +
> > +               ec = (void *)(skb->data + HCI_EVENT_HDR_SIZE);
> > +               opcode = __le16_to_cpu(ec->opcode);
> > +
> > +               /* Filter vendor opcode */
> > +               if (opcode == 0xfc5d) {
> > +                       kfree_skb(skb);
> > +                       return 0;
> > +               }
> > +       }
> > +
> > +       return hci_recv_frame(hdev, skb);
> > +}
> > +EXPORT_SYMBOL_GPL(btmtk_recv_event);
> >  #endif
> > 
> >  MODULE_AUTHOR("Sean Wang <sean.wang@mediatek.com>");
> > diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h
> > index adaf385626ee..08b73927c8f3 100644
> > --- a/drivers/bluetooth/btmtk.h
> > +++ b/drivers/bluetooth/btmtk.h
> > @@ -218,6 +218,8 @@ int btmtk_usb_suspend(struct hci_dev *hdev);
> >  int btmtk_usb_setup(struct hci_dev *hdev);
> > 
> >  int btmtk_usb_shutdown(struct hci_dev *hdev);
> > +
> > +int btmtk_recv_event(struct hci_dev *hdev, struct sk_buff *skb);
> >  #else
> > 
> >  static inline int btmtk_set_bdaddr(struct hci_dev *hdev,
> > @@ -296,4 +298,9 @@ static inline int btmtk_usb_shutdown(struct
> > hci_dev *hdev)
> >  {
> >         return -EOPNOTSUPP;
> >  }
> > +
> > +static inline int btmtk_recv_event(struct hci_dev *hdev, struct
> > sk_buff *skb)
> > +{
> > +       return -EOPNOTSUPP;
> > +}
> >  #endif
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index f9d515ee9124..daf8a387e660 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -4150,6 +4150,8 @@ static int btusb_probe(struct usb_interface
> > *intf,
> >         } else if (id->driver_info & BTUSB_MEDIATEK) {
> >                 /* Allocate extra space for Mediatek device */
> >                 priv_size += sizeof(struct btmtk_data);
> > +
> > +               data->recv_event = btmtk_recv_event;
> >         }
> > 
> >         data->recv_acl = hci_recv_frame;
> > --
> > 2.45.2
> 
> 
> https://urldefense.com/v3/__https://sashiko.dev/*/patchset/20260407114851.3087886-1-chris.lu*40mediatek.com__;IyU!!CTRNKA9wMg0ARbw!j4c9a9a8N0yC-uPVxzP3piQN8Ga4aEG1gE7ZMtTFeFYFzkNiq08Zd_XHi9ImhiQTSWTjKP-XoRutGQU0yKk$
>  
> 
> Both issues seem valid to me. That said, I wonder if it wouldn't have
> been better to allow drivers to register with its own struct
> hci_ev/struct hci_cc tables, that way the driver can extend event
> parsing rather than intercepting it and performing custom parsing
> which can lead to bugs like the ones mentioned above.
> 
> 

For the approach you mentioned, I think it's a better long-term
direction but it seems that this mechanism hasn't been implement yet,
or is there already existing an structure/API in kernel that I can
refer to?

So far I refer to the methods used by other vendor to attach the event
filter in driver setup stage. I'll send a v3 addressing the review
comment in above website after doing some verification locally.

Thanks,
Chris Lu

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-04-08 11:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-07 11:48 [PATCH v2] Bluetooth: btmtk: add event filter to filter specific event Chris Lu
2026-04-07 14:54 ` Luiz Augusto von Dentz
2026-04-08 11:28   ` Chris Lu (陸稚泓)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox