linux-kernel - Re: [PATCHv2 4/4] Bluetooth: btwilink: drop superseded driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <299B10E1-E7A8-4346-BED9-A48446C4D866@holtmann.org>
Date:   Wed, 23 Oct 2019 14:42:43 +0200
From:   Marcel Holtmann <marcel@...tmann.org>
To:     Sebastian Reichel <sre@...nel.org>
Cc:     Tony Lindgren <tony@...mide.com>, Adam Ford <aford173@...il.com>,
        Enric Balletbo i Serra <enric.balletbo@...labora.com>,
        linux-bluetooth@...r.kernel.org, linux-omap@...r.kernel.org,
        linux-kernel@...r.kernel.org, kernel@...labora.com
Subject: Re: [PATCHv2 4/4] Bluetooth: btwilink: drop superseded driver

Hi Sebastian,

>>>>> All users of this driver have been converted to the serdev based
>>>>> hci_ll driver. The unused driver can be safely dropped now.
>>>>> 
>>>>> Signed-off-by: Sebastian Reichel <sebastian.reichel@...labora.com>
>>>>> ---
>>>>> drivers/bluetooth/Kconfig    |  11 --
>>>>> drivers/bluetooth/Makefile   |   1 -
>>>>> drivers/bluetooth/btwilink.c | 337 -----------------------------------
>>>>> 3 files changed, 349 deletions(-)
>>>>> delete mode 100644 drivers/bluetooth/btwilink.c
>>>> 
>>>> patch has been applied to bluetooth-next tree.
>>>> 
>>>> However what I really like to see is that you re-introduce a
>>>> btwilink driver that is purely serdev based and doesn’t rely on
>>>> any hci_uart/hci_ldisc code. A clean serdev only driver is that
>>>> best and easier to maintain long term.
>>> 
>>> So basically move the serdev implementation from hci_ll.c into its
>>> own driver and make hci_ll hci_uart based only? That effectively
>>> means, that we have two implementations of the protocol. I don't
>>> think this will improve maintainability, since then bugs needs to
>>> be fixed in two places? Note, that we have a couple of drivers
>>> with serdev+hci_uart by now:
>>> 
>>> for file in $(grep -l serdev drivers/bluetooth/hci_*c) ; grep -l hci_uart_register_proto "${file}"
>>> hci_bcm.c
>>> hci_h5.c
>>> hci_ldisc.c
>>> hci_ll.c
>>> hci_mrvl.c
>>> hci_qca.c
>> 
>> I would like to have something similar to btmtkuart.c which is a
>> pure serdev driver that doesn’t depend on any hci_ldisc.c
>> framework. If we have this, then we would just drop hci_ll.c from
>> the kernel and focus on the serdev only version. As noted, there
>> is no need for any other driver at that point since everything is
>> probed anyway. Users will not even notice the difference.
> 
> This can be achieved by just removing the hci_uart part from
> hci_ll. But AFAIK there are some non-wilink based TI HCILL
> devices, which do not require any extra platform data and might
> still use the hci_uart part.

the hci_ldisc and hci_uart driver abstraction provides enqueue and dequeue handling that just needs to be removed and done natively in the serdev driver. This should be all straight forward and we can keep the current hci_ll.c around for a bit to see what users we have. However I really want to migrate everything over to native serdev drivers that are standalone. The TI HCILL is complex enough to warrant a separate driver.

Regards

Marcel