Messages in this thread | | | Subject | Re: [PATCH 2/2] Bluetooth: BT_HCIUART now depends on SERIAL_DEV_BUS | From | Hans de Goede <> | Date | Thu, 12 Oct 2017 10:27:01 +0200 |
| |
Hi,
On 11-10-17 20:10, Marcel Holtmann wrote: > Hi Hans, > >>> It is no longer possible to build BT_HCIUART into the kernel >>> when SERIAL_DEV_BUS is a loadable module, even if none of the >>> SERIAL_DEV_BUS based implementations are selected: >>> drivers/bluetooth/hci_ldisc.o: In function `hci_uart_set_flow_control': >>> hci_ldisc.c:(.text+0xb40): undefined reference to `serdev_device_set_flow_control' >>> hci_ldisc.c:(.text+0xb5c): undefined reference to `serdev_device_set_tiocm' >>> This adds a dependency to avoid the broken configuration. >>> Fixes: 7841d554809b ("Bluetooth: hci_uart_set_flow_control: Fix NULL deref when using serdev") >>> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >> >> Another one I have on my TODO after the buildbot errors. In this case >> I do not believe this is the proper fix though. >> >> As pointed out in another thread discussing the series introducing >> this problem, hci_ldisc.c really should not depend on serdev, >> so the proper fix would be to have hci_bcm.c directly call >> the serdev flowcontrol and rts functions when the hci is >> backed by a serdev device, like hci_bcm.c is already doing >> when setting the baudrate, see host_set_baudrate in hci_bcm.c, >> so a similar host_set_flow_control should be added after which >> the changes to hci_ldisc.c can be reverted. >> >> If I understood Marcel correctly he prefers a single patch >> fixing this which also removes the changes from hci_ldisc.c, >> rather then a separate revert. > > actually two patches is fine, but I want them as a patch series so they are applied in a row. I think best is first to revert the btusb.c changes and then apply the new ACPI PNP id.
This is actually about a different series, but I assume the same applies for both series.
My remark here was not about the btusb 0000:0000 USB ids handling + ACPI PNP ids, but about fixing hci_ldisc.c now depending on serdev.
So since you now have accepted Arnd patch making hci_ldisc.c now depending on serdev more or less official (which I'm fine with), can I assume that this is going to be the final (for now / for 4.15) state of the deps situation surrounding hci_ldisc.c ?
The reason I'm asking this is because my plan was to undo the changes introducing the hci_ldisc.c dependency on serdev and instead adding a host_set_flow_control helper to hci_bcm.c, so have hci_bcm.c directly call the serdev flowcontrol funcs instead of having it depend on hci_ldisc.c for this in the same way as it is already directly calling serdev_device_set_baudrate.
If we are going to let hci_ldisc.c deal with tty vs serdev backed devices for flowcontrol, it makes sense to do the same for baudrate and make it save to call hci_uart_set_baudrate on a serdev backed hci_uart and drop the host_set_baudrate helper from hci_bcm.c. I can write and test a patch for this ...
Either way let me know how you want to proceed. I will let this rest until I hear back from you.
Regards,
Hans
| |