Hi Luiz,

A follow-up on my previous reply.

Looks like the dump lifetime is fixed/hardcoded to 5 minutes by the devcoredump base framework on purpose. Any change to this framework, to allow btnxpuart driver control the dump lifetime may not get approved by devcoredump maintainers.
https://github.com/bluez/bluetooth-next/commit/3b9c181bcde8555ca81b2394c2dc2201cefc2dd4#diff-dd2971a4abc5dbad2b03c42f25b2d8ef9d9417189070012500f315ee22fd17f9

Anyways, for Bluetooth drivers that relay on hci_devcd layer, calling the new hci_devcd_unregister() API fixes the issue mentioned in the commit message, properly cleaning up the dump data if driver is removed before the dump lifetime is complete, no matter who controls the dump lifetime.

Please let me know if I'm missing something.

Thanks,
Neeraj
 

Hi Luiz,

Thank you for reviewing this patch.

quoted

On Thu, Jul 3, 2025 at 9:17 AM Neeraj Sanjay Kale
[off-list ref] wrote:

quoted

This adds hci_devcd_unregister() which can be called when driver is
removed, which will cleanup the devcoredump data and cancel delayed
dump_timeout work.

With BTNXPUART driver, it is observed that after FW dump, if driver
is removed and re-loaded, it creates hci1 interface instead of hci0
interface.

But after DEVCD_TIMEOUT (5 minutes) if driver is re-loaded, hci0 is
created. This is because after FW dump, hci0 is not unregistered
properly for DEVCD_TIMEOUT.

With this patch, BTNXPUART is able to create hci0 after every FW
dump and driver reload.

Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
---
 include/net/bluetooth/coredump.h | 3 +++
 net/bluetooth/coredump.c         | 8 ++++++++
 2 files changed, 11 insertions(+)

diff --git a/include/net/bluetooth/coredump.h

b/include/net/bluetooth/coredump.h
index 72f51b587a04..bc8856e4bfe7 100644

--- a/include/net/bluetooth/coredump.h
+++ b/include/net/bluetooth/coredump.h

@@ -66,6 +66,7 @@ void hci_devcd_timeout(struct work_struct *work);

 int hci_devcd_register(struct hci_dev *hdev, coredump_t coredump,
                       dmp_hdr_t dmp_hdr, notify_change_t

notify_change);
+void hci_devcd_unregister(struct hci_dev *hdev);
 int hci_devcd_init(struct hci_dev *hdev, u32 dump_size);  int
hci_devcd_append(struct hci_dev *hdev, struct sk_buff *skb);  int
hci_devcd_append_pattern(struct hci_dev *hdev, u8 pattern, u32 len);

@@ -85,6 +86,8 @@ static inline int hci_devcd_register(struct

hci_dev

*hdev, coredump_t coredump,

quoted

        return -EOPNOTSUPP;
 }

+static inline void hci_devcd_unregister(struct hci_dev *hdev) {}
+
 static inline int hci_devcd_init(struct hci_dev *hdev, u32
dump_size) {
        return -EOPNOTSUPP;

diff --git a/net/bluetooth/coredump.c b/net/bluetooth/coredump.c
index 819eacb38762..dd7bd40e3eba 100644
--- a/net/bluetooth/coredump.c
+++ b/net/bluetooth/coredump.c

@@ -442,6 +442,14 @@ int hci_devcd_register(struct hci_dev *hdev,

coredump_t coredump,  }  EXPORT_SYMBOL(hci_devcd_register);

+void hci_devcd_unregister(struct hci_dev *hdev) {
+       cancel_delayed_work(&hdev->dump.dump_timeout);
+       skb_queue_purge(&hdev->dump.dump_q);
+       dev_coredump_put(&hdev->dev); }
+EXPORT_SYMBOL_GPL(hci_devcd_unregister);

The fact that the dump lives inside hdev is sort of the source of
these problems, specially if the dumps are not HCI traffic it might be
better off having the driver control its lifetime and not use
hdev->workqueue to schedule it.

Are you are talking about "hdev->dump.dump_timeout"? it does not control
the dump lifetime.
It simply makes sure that once FW dump is started, it should be complete
within "dump_timeout" seconds.

The actual cleaning up of dump data is done by the " devcd->del_wk" which
is delay-scheduled by 5 minutes in dev_coredumpm_timeout(), which is part
of the devcoredump base.

Sure, with some modification, the driver can control the dump lifetime
instead of hardcode DEVCD_TIMEOUT, but during driver exit, there is a need
for "dev_coredump_put()" API to be called anyway.

Please let me know your thoughts on this.

quoted

quoted

 static inline bool hci_devcd_enabled(struct hci_dev *hdev)  {
        return hdev->dump.supported;
--

Thanks,
Neeraj