Re: [PATCH v2] Bluetooth: btmtk: add event filter to filter specific event

["Chris Lu (陸稚泓)" <Chris.Lu@mediatek.com>]

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