[v1] Bluetooth: btmtk: Fix btmtk.c undefined reference build error

[PATCH] Bluetooth: btmtk: Fix btmtk.c undefined reference build error

Posted by Chris Lu 1 year, 7 months ago

MediaTek move some usb interface related function to btmtk.c which
may cause build failed if BT USB Kconfig wasn't enabled.
Fix undefined reference by adding config check.

Fixes: 39a9e1c69e74 Bluetooth: btmtk: move btusb_mtk_hci_wmt_sync to btmtk.c
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202407091928.AH0aGZnx-lkp@intel.com/
Signed-off-by: Chris Lu <chris.lu@mediatek.com>
---
 drivers/bluetooth/btmtk.c | 2 ++
 drivers/bluetooth/btmtk.h | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
index b7c348687a77..9789296ad4f6 100644
--- a/drivers/bluetooth/btmtk.c
+++ b/drivers/bluetooth/btmtk.c
@@ -437,6 +437,7 @@ int btmtk_process_coredump(struct hci_dev *hdev, struct sk_buff *skb)
 }
 EXPORT_SYMBOL_GPL(btmtk_process_coredump);
 
+#if IS_ENABLED(CONFIG_BT_HCIBTUSB_MTK)
 static void btmtk_usb_wmt_recv(struct urb *urb)
 {
 	struct hci_dev *hdev = urb->context;
@@ -1487,6 +1488,7 @@ int btmtk_usb_shutdown(struct hci_dev *hdev)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(btmtk_usb_shutdown);
+#endif
 
 MODULE_AUTHOR("Sean Wang <sean.wang@mediatek.com>");
 MODULE_AUTHOR("Mark Chen <mark-yw.chen@mediatek.com>");
diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h
index 453ed5131a37..890dbe9beff8 100644
--- a/drivers/bluetooth/btmtk.h
+++ b/drivers/bluetooth/btmtk.h
@@ -165,6 +165,7 @@ struct btmtk_data {
 	btmtk_reset_sync_func_t reset_sync;
 	struct btmtk_coredump_info cd_info;
 
+#if IS_ENABLED(CONFIG_BT_HCIBTUSB_MTK)
 	struct usb_device *udev;
 	struct usb_interface *intf;
 	struct usb_anchor *ctrl_anchor;
@@ -177,6 +178,7 @@ struct btmtk_data {
 
 	/* spinlock for ISO data transmission */
 	spinlock_t isorxlock;
+#endif
 };
 
 typedef int (*wmt_cmd_sync_func_t)(struct hci_dev *,
@@ -202,6 +204,7 @@ int btmtk_process_coredump(struct hci_dev *hdev, struct sk_buff *skb);
 void btmtk_fw_get_filename(char *buf, size_t size, u32 dev_id, u32 fw_ver,
 			   u32 fw_flavor);
 
+#if IS_ENABLED(CONFIG_BT_HCIBTUSB_MTK)
 int btmtk_usb_subsys_reset(struct hci_dev *hdev, u32 dev_id);
 
 int btmtk_usb_recv_acl(struct hci_dev *hdev, struct sk_buff *skb);
@@ -216,6 +219,7 @@ int btmtk_usb_suspend(struct hci_dev *hdev);
 int btmtk_usb_setup(struct hci_dev *hdev);
 
 int btmtk_usb_shutdown(struct hci_dev *hdev);
+#endif
 #else
 
 static inline int btmtk_set_bdaddr(struct hci_dev *hdev,
-- 
2.18.0

Re: [PATCH] Bluetooth: btmtk: Fix btmtk.c undefined reference build error

Posted by Luiz Augusto von Dentz 1 year, 7 months ago

Hi Chris,

On Wed, Jul 10, 2024 at 3:38 AM Chris Lu <chris.lu@mediatek.com> wrote:
>
> MediaTek move some usb interface related function to btmtk.c which
> may cause build failed if BT USB Kconfig wasn't enabled.
> Fix undefined reference by adding config check.
>
> Fixes: 39a9e1c69e74 Bluetooth: btmtk: move btusb_mtk_hci_wmt_sync to btmtk.c
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202407091928.AH0aGZnx-lkp@intel.com/
> Signed-off-by: Chris Lu <chris.lu@mediatek.com>
> ---
>  drivers/bluetooth/btmtk.c | 2 ++
>  drivers/bluetooth/btmtk.h | 4 ++++
>  2 files changed, 6 insertions(+)
>
> diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
> index b7c348687a77..9789296ad4f6 100644
> --- a/drivers/bluetooth/btmtk.c
> +++ b/drivers/bluetooth/btmtk.c
> @@ -437,6 +437,7 @@ int btmtk_process_coredump(struct hci_dev *hdev, struct sk_buff *skb)
>  }
>  EXPORT_SYMBOL_GPL(btmtk_process_coredump);
>
> +#if IS_ENABLED(CONFIG_BT_HCIBTUSB_MTK)
>  static void btmtk_usb_wmt_recv(struct urb *urb)
>  {
>         struct hci_dev *hdev = urb->context;
> @@ -1487,6 +1488,7 @@ int btmtk_usb_shutdown(struct hci_dev *hdev)
>         return 0;
>  }
>  EXPORT_SYMBOL_GPL(btmtk_usb_shutdown);
> +#endif
>
>  MODULE_AUTHOR("Sean Wang <sean.wang@mediatek.com>");
>  MODULE_AUTHOR("Mark Chen <mark-yw.chen@mediatek.com>");
> diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h
> index 453ed5131a37..890dbe9beff8 100644
> --- a/drivers/bluetooth/btmtk.h
> +++ b/drivers/bluetooth/btmtk.h
> @@ -165,6 +165,7 @@ struct btmtk_data {
>         btmtk_reset_sync_func_t reset_sync;
>         struct btmtk_coredump_info cd_info;
>
> +#if IS_ENABLED(CONFIG_BT_HCIBTUSB_MTK)
>         struct usb_device *udev;
>         struct usb_interface *intf;
>         struct usb_anchor *ctrl_anchor;
> @@ -177,6 +178,7 @@ struct btmtk_data {
>
>         /* spinlock for ISO data transmission */
>         spinlock_t isorxlock;
> +#endif

If I recall this correctly it is not a good practice to use #if inside
structs as it would change its overall size, memory layout, etc, and
it also makes it difficult to use if (IS_DEFINED) and let the compiler
eliminate dead code instead of using the preprocessor like you doing.

>  };
>
>  typedef int (*wmt_cmd_sync_func_t)(struct hci_dev *,
> @@ -202,6 +204,7 @@ int btmtk_process_coredump(struct hci_dev *hdev, struct sk_buff *skb);
>  void btmtk_fw_get_filename(char *buf, size_t size, u32 dev_id, u32 fw_ver,
>                            u32 fw_flavor);
>
> +#if IS_ENABLED(CONFIG_BT_HCIBTUSB_MTK)
>  int btmtk_usb_subsys_reset(struct hci_dev *hdev, u32 dev_id);
>
>  int btmtk_usb_recv_acl(struct hci_dev *hdev, struct sk_buff *skb);
> @@ -216,6 +219,7 @@ int btmtk_usb_suspend(struct hci_dev *hdev);
>  int btmtk_usb_setup(struct hci_dev *hdev);
>
>  int btmtk_usb_shutdown(struct hci_dev *hdev);
> +#endif
>  #else
>
>  static inline int btmtk_set_bdaddr(struct hci_dev *hdev,
> --
> 2.18.0
>


-- 
Luiz Augusto von Dentz

Re: [PATCH] Bluetooth: btmtk: Fix btmtk.c undefined reference build error

Posted by Chris Lu (陸稚泓) 1 year, 7 months ago

Hi Luiz,

On Wed, 2024-07-10 at 08:20 -0400, Luiz Augusto von Dentz wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  Hi Chris,
> 
> On Wed, Jul 10, 2024 at 3:38 AM Chris Lu <chris.lu@mediatek.com>
> wrote:
> >
> > MediaTek move some usb interface related function to btmtk.c which
> > may cause build failed if BT USB Kconfig wasn't enabled.
> > Fix undefined reference by adding config check.
> >
> > Fixes: 39a9e1c69e74 Bluetooth: btmtk: move btusb_mtk_hci_wmt_sync
> to btmtk.c
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: 
> https://lore.kernel.org/oe-kbuild-all/202407091928.AH0aGZnx-lkp@intel.com/
> > Signed-off-by: Chris Lu <chris.lu@mediatek.com>
> > ---
> >  drivers/bluetooth/btmtk.c | 2 ++
> >  drivers/bluetooth/btmtk.h | 4 ++++
> >  2 files changed, 6 insertions(+)
> >
> > diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
> > index b7c348687a77..9789296ad4f6 100644
> > --- a/drivers/bluetooth/btmtk.c
> > +++ b/drivers/bluetooth/btmtk.c
> > @@ -437,6 +437,7 @@ int btmtk_process_coredump(struct hci_dev
> *hdev, struct sk_buff *skb)
> >  }
> >  EXPORT_SYMBOL_GPL(btmtk_process_coredump);
> >
> > +#if IS_ENABLED(CONFIG_BT_HCIBTUSB_MTK)
> >  static void btmtk_usb_wmt_recv(struct urb *urb)
> >  {
> >         struct hci_dev *hdev = urb->context;
> > @@ -1487,6 +1488,7 @@ int btmtk_usb_shutdown(struct hci_dev *hdev)
> >         return 0;
> >  }
> >  EXPORT_SYMBOL_GPL(btmtk_usb_shutdown);
> > +#endif
> >
> >  MODULE_AUTHOR("Sean Wang <sean.wang@mediatek.com>");
> >  MODULE_AUTHOR("Mark Chen <mark-yw.chen@mediatek.com>");
> > diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h
> > index 453ed5131a37..890dbe9beff8 100644
> > --- a/drivers/bluetooth/btmtk.h
> > +++ b/drivers/bluetooth/btmtk.h
> > @@ -165,6 +165,7 @@ struct btmtk_data {
> >         btmtk_reset_sync_func_t reset_sync;
> >         struct btmtk_coredump_info cd_info;
> >
> > +#if IS_ENABLED(CONFIG_BT_HCIBTUSB_MTK)
> >         struct usb_device *udev;
> >         struct usb_interface *intf;
> >         struct usb_anchor *ctrl_anchor;
> > @@ -177,6 +178,7 @@ struct btmtk_data {
> >
> >         /* spinlock for ISO data transmission */
> >         spinlock_t isorxlock;
> > +#endif
> 
> If I recall this correctly it is not a good practice to use #if
> inside
> structs as it would change its overall size, memory layout, etc, and
> it also makes it difficult to use if (IS_DEFINED) and let the
> compiler
> eliminate dead code instead of using the preprocessor like you doing.
> 
> 

"#if" can be remove here, usb.h was included in each c file in case 
making certain compiler error as you mentioned. It should be fine not
to separate these element. I'm able to build pass after remove the
"#if" here locally with same Kconfig.

I saw someone sent another patch for this issue by adding inline  to
each function. It would be another way to avoid undefined reference.


Thanks,
Chris Lu
> >  };
> >
> >  typedef int (*wmt_cmd_sync_func_t)(struct hci_dev *,
> > @@ -202,6 +204,7 @@ int btmtk_process_coredump(struct hci_dev
> *hdev, struct sk_buff *skb);
> >  void btmtk_fw_get_filename(char *buf, size_t size, u32 dev_id, u32
> fw_ver,
> >                            u32 fw_flavor);
> >
> > +#if IS_ENABLED(CONFIG_BT_HCIBTUSB_MTK)
> >  int btmtk_usb_subsys_reset(struct hci_dev *hdev, u32 dev_id);
> >
> >  int btmtk_usb_recv_acl(struct hci_dev *hdev, struct sk_buff *skb);
> > @@ -216,6 +219,7 @@ int btmtk_usb_suspend(struct hci_dev *hdev);
> >  int btmtk_usb_setup(struct hci_dev *hdev);
> >
> >  int btmtk_usb_shutdown(struct hci_dev *hdev);
> > +#endif
> >  #else
> >
> >  static inline int btmtk_set_bdaddr(struct hci_dev *hdev,
> > --
> > 2.18.0
> >
> 
> 
> -- 
> Luiz Augusto von Dentz