linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Calvin Owens <calvin@wbinvd.org>
To: Paul Menzel <pmenzel@molgen.mpg.de>
Cc: linux-kernel@vger.kernel.org, linux-bluetooth@vger.kernel.org,
	Luiz Augusto von Dentz <luiz.von.dentz@intel.com>,
	oe-kbuild-all@lists.linux.dev,
	Marcel Holtmann <marcel@holtmann.org>,
	Sean Wang <sean.wang@mediatek.com>,
	linux-mediatek@lists.infradead.org
Subject: Re: [PATCH] Bluetooth: btmtksdio: Fix build after header cleanup
Date: Sat, 30 Aug 2025 12:32:15 -0700	[thread overview]
Message-ID: <aLNRvzXE4O9dKZoN@mozart.vkv.me> (raw)
In-Reply-To: <84fd4012-966b-4983-b015-ffce06509b5e@molgen.mpg.de>

On Saturday 08/30 at 07:11 +0200, Paul Menzel wrote:
> Dear Calvin,
> 
> 
> Thank you for your patch, and addressing the regression right away.
> 
> Am 30.08.25 um 02:50 schrieb Calvin Owens:
> > Syzbot found a randconfig which fails after my recent patch:
> > 
> >      drivers/bluetooth/btmtksdio.c:442:33: error: array type has incomplete element type ‘struct h4_recv_pkt’
> >        442 | static const struct h4_recv_pkt mtk_recv_pkts[] = {
> >            |                                 ^~~~~~~~~~~~~
> >      drivers/bluetooth/btmtksdio.c:443:11: error: ‘H4_RECV_ACL’ undeclared here (not in a function)
> >        443 |         { H4_RECV_ACL,      .recv = btmtksdio_recv_acl },
> >            |           ^~~~~~~~~~~
> >      drivers/bluetooth/btmtksdio.c:444:11: error: ‘H4_RECV_SCO’ undeclared here (not in a function)
> >        444 |         { H4_RECV_SCO,      .recv = hci_recv_frame },
> >            |           ^~~~~~~~~~~
> >      drivers/bluetooth/btmtksdio.c:445:11: error: ‘H4_RECV_EVENT’ undeclared here (not in a function)
> >        445 |         { H4_RECV_EVENT,    .recv = btmtksdio_recv_event },
> > 
> > ...because we can have BT_MTKSDIO=y with BT_HCIUART_H4=n, and the
> > definitions used here are gated on BT_HCIUART_H4 in hci_uart.h.
> 
> The drivers below seem to be affected:
> 
>     drivers/bluetooth/bpa10x.c:     { H4_RECV_EVENT,   .recv =
> hci_recv_frame },
>     drivers/bluetooth/btmtksdio.c:  { H4_RECV_EVENT,    .recv =
> btmtksdio_recv_event },
>     drivers/bluetooth/btmtkuart.c:  { H4_RECV_EVENT,    .recv =
> btmtkuart_recv_event },
>     drivers/bluetooth/btnxpuart.c:  { H4_RECV_EVENT,        .recv =
> hci_recv_frame },
>
> > I think the simplest way to fix this is to remove the gate on the
> > definitions in hci_uart.h. Since the constants are macros, there's no
> > runtime cost to doing so, and nothing seems to rely on their absence in
> > the BT_HCIUART_H4=n case.
> 
> Looking at the implementation, it looks like they only work with the H4
> protocol? So maybe, that should be denoted in the Kconfig files?

Thanks for looking Paul.

Yes, my fix will cause a link error with other randconfigs, which my
'make randconfig drivers/bluetooth/' test loop missed after I made the
function prototype always defined, whoops.

We do need the dependencies here, as you note. The btmtksdio case syzbot
found is the odd one out because it only uses the constants, and doesn't
call h4_recv_buf().

Hopefully this gets it all:

-----8<-----
From: Calvin Owens <calvin@wbinvd.org>
Subject: [PATCH v2] Bluetooth: Fix build after header cleanup

Some Kconfig dependencies are needed after my recent cleanup, since
the core code has its own option.

Since btmtksdio does not actually call h4_recv_buf(), move the
definitions it uses outside the BT_HCIUART_H4 gate in hci_uart.h to
avoid adding a dependency for btmtksdio.

The rest I touched (bpa10x, btmtkuart, and btnxpuart) do really call
h4_recv_buf(), so the dependency is required, add it for them.

Fixes: 74bcec450eea ("Bluetooth: remove duplicate h4_recv_buf() in header")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202508300413.OnIedvRh-lkp@intel.com/
Signed-off-by: Calvin Owens <calvin@wbinvd.org>
---
 drivers/bluetooth/Kconfig    | 6 ++++++
 drivers/bluetooth/hci_uart.h | 8 ++++----
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index 4ab32abf0f48..7df69ccb6600 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -312,7 +312,9 @@ config BT_HCIBCM4377
 
 config BT_HCIBPA10X
 	tristate "HCI BPA10x USB driver"
+	depends on BT_HCIUART
 	depends on USB
+	select BT_HCIUART_H4
 	help
 	  Bluetooth HCI BPA10x USB driver.
 	  This driver provides support for the Digianswer BPA 100/105 Bluetooth
@@ -437,8 +439,10 @@ config BT_MTKSDIO
 
 config BT_MTKUART
 	tristate "MediaTek HCI UART driver"
+	depends on BT_HCIUART
 	depends on SERIAL_DEV_BUS
 	depends on USB || !BT_HCIBTUSB_MTK
+	select BT_HCIUART_H4
 	select BT_MTK
 	help
 	  MediaTek Bluetooth HCI UART driver.
@@ -483,7 +487,9 @@ config BT_VIRTIO
 
 config BT_NXPUART
 	tristate "NXP protocol support"
+	depends on BT_HCIUART
 	depends on SERIAL_DEV_BUS
+	select BT_HCIUART_H4
 	select CRC32
 	select CRC8
 	help
diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
index 5ea5dd80e297..cbbe79b241ce 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -121,10 +121,6 @@ void hci_uart_set_flow_control(struct hci_uart *hu, bool enable);
 void hci_uart_set_speeds(struct hci_uart *hu, unsigned int init_speed,
 			 unsigned int oper_speed);
 
-#ifdef CONFIG_BT_HCIUART_H4
-int h4_init(void);
-int h4_deinit(void);
-
 struct h4_recv_pkt {
 	u8  type;	/* Packet type */
 	u8  hlen;	/* Header length */
@@ -162,6 +158,10 @@ struct h4_recv_pkt {
 	.lsize = 2, \
 	.maxlen = HCI_MAX_FRAME_SIZE \
 
+#ifdef CONFIG_BT_HCIUART_H4
+int h4_init(void);
+int h4_deinit(void);
+
 struct sk_buff *h4_recv_buf(struct hci_dev *hdev, struct sk_buff *skb,
 			    const unsigned char *buffer, int count,
 			    const struct h4_recv_pkt *pkts, int pkts_count);
-- 
2.47.2


  reply	other threads:[~2025-08-30 19:32 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <202508300413.OnIedvRh-lkp@intel.com>
2025-08-30  0:50 ` [PATCH] Bluetooth: btmtksdio: Fix build after header cleanup Calvin Owens
2025-08-30  5:11   ` Paul Menzel
2025-08-30 19:32     ` Calvin Owens [this message]
2025-09-02 15:29       ` Luiz Augusto von Dentz
2025-09-02 15:43         ` [PATCH v2] Bluetooth: " Calvin Owens
2025-09-02 16:20           ` patchwork-bot+bluetooth
2025-09-03 14:04           ` Calvin Owens
2025-09-03 14:06             ` [PATCH v3] " Calvin Owens
2025-09-03 15:29               ` Paul Menzel

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=aLNRvzXE4O9dKZoN@mozart.vkv.me \
    --to=calvin@wbinvd.org \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=luiz.von.dentz@intel.com \
    --cc=marcel@holtmann.org \
    --cc=oe-kbuild-all@lists.linux.dev \
    --cc=pmenzel@molgen.mpg.de \
    --cc=sean.wang@mediatek.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).