Hi,
This moves the remaining users of btwilink to the "new" serdev based hci_ll
driver and drops the btwilink driver afterwards. The patches were only compile
tested by me, but Enric tested the IGEP platform and Adam will test the LogicPD
platform.
I kept the TI_ST driver for now, since I plan to send a second patchset for the
FM radio driver. Once the FM driver has been converted to also use hci_ll, we
can remove TI_ST completly.
My suggestion is for the patch handling is, that everything simply goes through
Tony's tree.
Changes since PATCHv1 [0]
* rebase to 5.4-rc1
* move FM radio patches into separate patchset
[0] https://lore.kernel.org/lkml/[email protected]/
-- Sebastian
Sebastian Reichel (4):
ARM: dts: LogicPD Torpedo: Add WiLink UART node
ARM: dts: IGEP: Add WiLink UART node
ARM: OMAP2+: pdata-quirks: drop TI_ST/KIM support
Bluetooth: btwilink: drop superseded driver
.../boot/dts/logicpd-torpedo-37xx-devkit.dts | 8 +
arch/arm/boot/dts/omap3-igep0020-rev-f.dts | 8 +
arch/arm/boot/dts/omap3-igep0030-rev-g.dts | 8 +
arch/arm/mach-omap2/pdata-quirks.c | 52 ---
drivers/bluetooth/Kconfig | 11 -
drivers/bluetooth/Makefile | 1 -
drivers/bluetooth/btwilink.c | 337 ------------------
7 files changed, 24 insertions(+), 401 deletions(-)
delete mode 100644 drivers/bluetooth/btwilink.c
--
2.23.0
From: Sebastian Reichel <[email protected]>
All TI_ST users have been migrated to the new serdev based HCILL
bluetooth driver. That driver is initialized from DT and does not
need any platform quirks.
Signed-off-by: Sebastian Reichel <[email protected]>
---
arch/arm/mach-omap2/pdata-quirks.c | 52 ------------------------------
1 file changed, 52 deletions(-)
diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c
index d942a3357090..02abeb44cab2 100644
--- a/arch/arm/mach-omap2/pdata-quirks.c
+++ b/arch/arm/mach-omap2/pdata-quirks.c
@@ -11,7 +11,6 @@
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/of_platform.h>
-#include <linux/ti_wilink_st.h>
#include <linux/wl12xx.h>
#include <linux/mmc/card.h>
#include <linux/mmc/host.h>
@@ -139,53 +138,6 @@ static void __init omap3_sbc_t3530_legacy_init(void)
omap3_sbc_t3x_usb_hub_init(167, "sb-t35 usb hub");
}
-static struct ti_st_plat_data wilink_pdata = {
- .nshutdown_gpio = 137,
- .dev_name = "/dev/ttyO1",
- .flow_cntrl = 1,
- .baud_rate = 300000,
-};
-
-static struct platform_device wl18xx_device = {
- .name = "kim",
- .id = -1,
- .dev = {
- .platform_data = &wilink_pdata,
- }
-};
-
-static struct ti_st_plat_data wilink7_pdata = {
- .nshutdown_gpio = 162,
- .dev_name = "/dev/ttyO1",
- .flow_cntrl = 1,
- .baud_rate = 3000000,
-};
-
-static struct platform_device wl128x_device = {
- .name = "kim",
- .id = -1,
- .dev = {
- .platform_data = &wilink7_pdata,
- }
-};
-
-static struct platform_device btwilink_device = {
- .name = "btwilink",
- .id = -1,
-};
-
-static void __init omap3_igep0020_rev_f_legacy_init(void)
-{
- platform_device_register(&wl18xx_device);
- platform_device_register(&btwilink_device);
-}
-
-static void __init omap3_igep0030_rev_g_legacy_init(void)
-{
- platform_device_register(&wl18xx_device);
- platform_device_register(&btwilink_device);
-}
-
static void __init omap3_evm_legacy_init(void)
{
hsmmc2_internal_input_clk();
@@ -299,8 +251,6 @@ static void __init omap3_tao3530_legacy_init(void)
static void __init omap3_logicpd_torpedo_init(void)
{
omap3_gpio126_127_129();
- platform_device_register(&wl128x_device);
- platform_device_register(&btwilink_device);
}
/* omap3pandora legacy devices */
@@ -679,8 +629,6 @@ static struct pdata_init pdata_quirks[] __initdata = {
{ "nokia,omap3-n900", nokia_n900_legacy_init, },
{ "nokia,omap3-n9", hsmmc2_internal_input_clk, },
{ "nokia,omap3-n950", hsmmc2_internal_input_clk, },
- { "isee,omap3-igep0020-rev-f", omap3_igep0020_rev_f_legacy_init, },
- { "isee,omap3-igep0030-rev-g", omap3_igep0030_rev_g_legacy_init, },
{ "logicpd,dm3730-torpedo-devkit", omap3_logicpd_torpedo_init, },
{ "ti,omap3-evm-37xx", omap3_evm_legacy_init, },
{ "ti,am3517-evm", am3517_evm_legacy_init, },
--
2.23.0
From: Sebastian Reichel <[email protected]>
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 <[email protected]>
---
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
diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index aae665a3a254..f7aa2dc1ff85 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -380,17 +380,6 @@ config BT_ATH3K
Say Y here to compile support for "Atheros firmware download driver"
into the kernel or say M to compile it as module (ath3k).
-config BT_WILINK
- tristate "Texas Instruments WiLink7 driver"
- depends on TI_ST
- help
- This enables the Bluetooth driver for Texas Instrument's BT/FM/GPS
- combo devices. This makes use of shared transport line discipline
- core driver to communicate with the BT core of the combo chip.
-
- Say Y here to compile support for Texas Instrument's WiLink7 driver
- into the kernel or say M to compile it as module (btwilink).
-
config BT_MTKSDIO
tristate "MediaTek HCI SDIO driver"
depends on MMC
diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
index 34887b9b3a85..1a58a3ae142c 100644
--- a/drivers/bluetooth/Makefile
+++ b/drivers/bluetooth/Makefile
@@ -19,7 +19,6 @@ obj-$(CONFIG_BT_INTEL) += btintel.o
obj-$(CONFIG_BT_ATH3K) += ath3k.o
obj-$(CONFIG_BT_MRVL) += btmrvl.o
obj-$(CONFIG_BT_MRVL_SDIO) += btmrvl_sdio.o
-obj-$(CONFIG_BT_WILINK) += btwilink.o
obj-$(CONFIG_BT_MTKSDIO) += btmtksdio.o
obj-$(CONFIG_BT_MTKUART) += btmtkuart.o
obj-$(CONFIG_BT_QCOMSMD) += btqcomsmd.o
diff --git a/drivers/bluetooth/btwilink.c b/drivers/bluetooth/btwilink.c
deleted file mode 100644
index e55f06e4270f..000000000000
--- a/drivers/bluetooth/btwilink.c
+++ /dev/null
@@ -1,337 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Texas Instrument's Bluetooth Driver For Shared Transport.
- *
- * Bluetooth Driver acts as interface between HCI core and
- * TI Shared Transport Layer.
- *
- * Copyright (C) 2009-2010 Texas Instruments
- * Author: Raja Mani <[email protected]>
- * Pavan Savoy <[email protected]>
- */
-
-#include <linux/platform_device.h>
-#include <net/bluetooth/bluetooth.h>
-#include <net/bluetooth/hci_core.h>
-#include <net/bluetooth/hci.h>
-
-#include <linux/ti_wilink_st.h>
-#include <linux/module.h>
-
-/* Bluetooth Driver Version */
-#define VERSION "1.0"
-#define MAX_BT_CHNL_IDS 3
-
-/* Number of seconds to wait for registration completion
- * when ST returns PENDING status.
- */
-#define BT_REGISTER_TIMEOUT 6000 /* 6 sec */
-
-/**
- * struct ti_st - driver operation structure
- * @hdev: hci device pointer which binds to bt driver
- * @reg_status: ST registration callback status
- * @st_write: write function provided by the ST driver
- * to be used by the driver during send_frame.
- * @wait_reg_completion - completion sync between ti_st_open
- * and st_reg_completion_cb.
- */
-struct ti_st {
- struct hci_dev *hdev;
- int reg_status;
- long (*st_write) (struct sk_buff *);
- struct completion wait_reg_completion;
-};
-
-/* Increments HCI counters based on pocket ID (cmd,acl,sco) */
-static inline void ti_st_tx_complete(struct ti_st *hst, int pkt_type)
-{
- struct hci_dev *hdev = hst->hdev;
-
- /* Update HCI stat counters */
- switch (pkt_type) {
- case HCI_COMMAND_PKT:
- hdev->stat.cmd_tx++;
- break;
-
- case HCI_ACLDATA_PKT:
- hdev->stat.acl_tx++;
- break;
-
- case HCI_SCODATA_PKT:
- hdev->stat.sco_tx++;
- break;
- }
-}
-
-/* ------- Interfaces to Shared Transport ------ */
-
-/* Called by ST layer to indicate protocol registration completion
- * status.ti_st_open() function will wait for signal from this
- * API when st_register() function returns ST_PENDING.
- */
-static void st_reg_completion_cb(void *priv_data, int data)
-{
- struct ti_st *lhst = priv_data;
-
- /* Save registration status for use in ti_st_open() */
- lhst->reg_status = data;
- /* complete the wait in ti_st_open() */
- complete(&lhst->wait_reg_completion);
-}
-
-/* Called by Shared Transport layer when receive data is available */
-static long st_receive(void *priv_data, struct sk_buff *skb)
-{
- struct ti_st *lhst = priv_data;
- int err;
-
- if (!skb)
- return -EFAULT;
-
- if (!lhst) {
- kfree_skb(skb);
- return -EFAULT;
- }
-
- /* Forward skb to HCI core layer */
- err = hci_recv_frame(lhst->hdev, skb);
- if (err < 0) {
- BT_ERR("Unable to push skb to HCI core(%d)", err);
- return err;
- }
-
- lhst->hdev->stat.byte_rx += skb->len;
-
- return 0;
-}
-
-/* ------- Interfaces to HCI layer ------ */
-/* protocol structure registered with shared transport */
-static struct st_proto_s ti_st_proto[MAX_BT_CHNL_IDS] = {
- {
- .chnl_id = HCI_EVENT_PKT, /* HCI Events */
- .hdr_len = sizeof(struct hci_event_hdr),
- .offset_len_in_hdr = offsetof(struct hci_event_hdr, plen),
- .len_size = 1, /* sizeof(plen) in struct hci_event_hdr */
- .reserve = 8,
- },
- {
- .chnl_id = HCI_ACLDATA_PKT, /* ACL */
- .hdr_len = sizeof(struct hci_acl_hdr),
- .offset_len_in_hdr = offsetof(struct hci_acl_hdr, dlen),
- .len_size = 2, /* sizeof(dlen) in struct hci_acl_hdr */
- .reserve = 8,
- },
- {
- .chnl_id = HCI_SCODATA_PKT, /* SCO */
- .hdr_len = sizeof(struct hci_sco_hdr),
- .offset_len_in_hdr = offsetof(struct hci_sco_hdr, dlen),
- .len_size = 1, /* sizeof(dlen) in struct hci_sco_hdr */
- .reserve = 8,
- },
-};
-
-/* Called from HCI core to initialize the device */
-static int ti_st_open(struct hci_dev *hdev)
-{
- unsigned long timeleft;
- struct ti_st *hst;
- int err, i;
-
- BT_DBG("%s %p", hdev->name, hdev);
-
- /* provide contexts for callbacks from ST */
- hst = hci_get_drvdata(hdev);
-
- for (i = 0; i < MAX_BT_CHNL_IDS; i++) {
- ti_st_proto[i].priv_data = hst;
- ti_st_proto[i].max_frame_size = HCI_MAX_FRAME_SIZE;
- ti_st_proto[i].recv = st_receive;
- ti_st_proto[i].reg_complete_cb = st_reg_completion_cb;
-
- /* Prepare wait-for-completion handler */
- init_completion(&hst->wait_reg_completion);
- /* Reset ST registration callback status flag,
- * this value will be updated in
- * st_reg_completion_cb()
- * function whenever it called from ST driver.
- */
- hst->reg_status = -EINPROGRESS;
-
- err = st_register(&ti_st_proto[i]);
- if (!err)
- goto done;
-
- if (err != -EINPROGRESS) {
- BT_ERR("st_register failed %d", err);
- return err;
- }
-
- /* ST is busy with either protocol
- * registration or firmware download.
- */
- BT_DBG("waiting for registration "
- "completion signal from ST");
- timeleft = wait_for_completion_timeout
- (&hst->wait_reg_completion,
- msecs_to_jiffies(BT_REGISTER_TIMEOUT));
- if (!timeleft) {
- BT_ERR("Timeout(%d sec),didn't get reg "
- "completion signal from ST",
- BT_REGISTER_TIMEOUT / 1000);
- return -ETIMEDOUT;
- }
-
- /* Is ST registration callback
- * called with ERROR status?
- */
- if (hst->reg_status != 0) {
- BT_ERR("ST registration completed with invalid "
- "status %d", hst->reg_status);
- return -EAGAIN;
- }
-
-done:
- hst->st_write = ti_st_proto[i].write;
- if (!hst->st_write) {
- BT_ERR("undefined ST write function");
- for (i = 0; i < MAX_BT_CHNL_IDS; i++) {
- /* Undo registration with ST */
- err = st_unregister(&ti_st_proto[i]);
- if (err)
- BT_ERR("st_unregister() failed with "
- "error %d", err);
- hst->st_write = NULL;
- }
- return -EIO;
- }
- }
- return 0;
-}
-
-/* Close device */
-static int ti_st_close(struct hci_dev *hdev)
-{
- int err, i;
- struct ti_st *hst = hci_get_drvdata(hdev);
-
- for (i = MAX_BT_CHNL_IDS-1; i >= 0; i--) {
- err = st_unregister(&ti_st_proto[i]);
- if (err)
- BT_ERR("st_unregister(%d) failed with error %d",
- ti_st_proto[i].chnl_id, err);
- }
-
- hst->st_write = NULL;
-
- return err;
-}
-
-static int ti_st_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
-{
- struct ti_st *hst;
- long len;
- int pkt_type;
-
- hst = hci_get_drvdata(hdev);
-
- /* Prepend skb with frame type */
- memcpy(skb_push(skb, 1), &hci_skb_pkt_type(skb), 1);
-
- BT_DBG("%s: type %d len %d", hdev->name, hci_skb_pkt_type(skb),
- skb->len);
-
- /* Insert skb to shared transport layer's transmit queue.
- * Freeing skb memory is taken care in shared transport layer,
- * so don't free skb memory here.
- */
- pkt_type = hci_skb_pkt_type(skb);
- len = hst->st_write(skb);
- if (len < 0) {
- BT_ERR("ST write failed (%ld)", len);
- /* Try Again, would only fail if UART has gone bad */
- return -EAGAIN;
- }
-
- /* ST accepted our skb. So, Go ahead and do rest */
- hdev->stat.byte_tx += len;
- ti_st_tx_complete(hst, pkt_type);
-
- return 0;
-}
-
-static int bt_ti_probe(struct platform_device *pdev)
-{
- struct ti_st *hst;
- struct hci_dev *hdev;
- int err;
-
- hst = devm_kzalloc(&pdev->dev, sizeof(struct ti_st), GFP_KERNEL);
- if (!hst)
- return -ENOMEM;
-
- /* Expose "hciX" device to user space */
- hdev = hci_alloc_dev();
- if (!hdev)
- return -ENOMEM;
-
- BT_DBG("hdev %p", hdev);
-
- hst->hdev = hdev;
- hdev->bus = HCI_UART;
- hci_set_drvdata(hdev, hst);
- hdev->open = ti_st_open;
- hdev->close = ti_st_close;
- hdev->flush = NULL;
- hdev->send = ti_st_send_frame;
-
- err = hci_register_dev(hdev);
- if (err < 0) {
- BT_ERR("Can't register HCI device error %d", err);
- hci_free_dev(hdev);
- return err;
- }
-
- BT_DBG("HCI device registered (hdev %p)", hdev);
-
- dev_set_drvdata(&pdev->dev, hst);
- return 0;
-}
-
-static int bt_ti_remove(struct platform_device *pdev)
-{
- struct hci_dev *hdev;
- struct ti_st *hst = dev_get_drvdata(&pdev->dev);
-
- if (!hst)
- return -EFAULT;
-
- BT_DBG("%s", hst->hdev->name);
-
- hdev = hst->hdev;
- ti_st_close(hdev);
- hci_unregister_dev(hdev);
-
- hci_free_dev(hdev);
-
- dev_set_drvdata(&pdev->dev, NULL);
- return 0;
-}
-
-static struct platform_driver btwilink_driver = {
- .probe = bt_ti_probe,
- .remove = bt_ti_remove,
- .driver = {
- .name = "btwilink",
- },
-};
-
-module_platform_driver(btwilink_driver);
-
-/* ------ Module Info ------ */
-
-MODULE_AUTHOR("Raja Mani <[email protected]>");
-MODULE_DESCRIPTION("Bluetooth Driver for TI Shared Transport" VERSION);
-MODULE_VERSION(VERSION);
-MODULE_LICENSE("GPL");
--
2.23.0
From: Sebastian Reichel <[email protected]>
Add a node for the UART part of WiLink chip.
Tested-by: Enric Balletbo i Serra <[email protected]>
Signed-off-by: Sebastian Reichel <[email protected]>
---
arch/arm/boot/dts/omap3-igep0020-rev-f.dts | 8 ++++++++
arch/arm/boot/dts/omap3-igep0030-rev-g.dts | 8 ++++++++
2 files changed, 16 insertions(+)
diff --git a/arch/arm/boot/dts/omap3-igep0020-rev-f.dts b/arch/arm/boot/dts/omap3-igep0020-rev-f.dts
index 03dcd05fb8a0..001decc20b3d 100644
--- a/arch/arm/boot/dts/omap3-igep0020-rev-f.dts
+++ b/arch/arm/boot/dts/omap3-igep0020-rev-f.dts
@@ -49,3 +49,11 @@
interrupts = <17 IRQ_TYPE_EDGE_RISING>; /* gpio 177 */
};
};
+
+&uart2 {
+ bluetooth {
+ compatible = "ti,wl1835-st";
+ enable-gpios = <&gpio5 9 GPIO_ACTIVE_HIGH>; /* gpio 137 */
+ max-speed = <300000>;
+ };
+};
diff --git a/arch/arm/boot/dts/omap3-igep0030-rev-g.dts b/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
index 060acd1e803a..9a8975799e16 100644
--- a/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
+++ b/arch/arm/boot/dts/omap3-igep0030-rev-g.dts
@@ -71,3 +71,11 @@
interrupts = <8 IRQ_TYPE_EDGE_RISING>; /* gpio 136 */
};
};
+
+&uart2 {
+ bluetooth {
+ compatible = "ti,wl1835-st";
+ enable-gpios = <&gpio5 9 GPIO_ACTIVE_HIGH>; /* gpio 137 */
+ max-speed = <300000>;
+ };
+};
--
2.23.0
* Sebastian Reichel <[email protected]> [191003 06:42]:
> Hi,
>
> This moves the remaining users of btwilink to the "new" serdev based hci_ll
> driver and drops the btwilink driver afterwards. The patches were only compile
> tested by me, but Enric tested the IGEP platform and Adam will test the LogicPD
> platform.
>
> I kept the TI_ST driver for now, since I plan to send a second patchset for the
> FM radio driver. Once the FM driver has been converted to also use hci_ll, we
> can remove TI_ST completly.
>
> My suggestion is for the patch handling is, that everything simply goes through
> Tony's tree.
Sounds good to me, good to see kim gone with patch 3/4 :)
Marcel, care to ack the old driver removal patch?
Regards,
Tony
> Changes since PATCHv1 [0]
> * rebase to 5.4-rc1
> * move FM radio patches into separate patchset
>
> [0] https://lore.kernel.org/lkml/[email protected]/
>
> -- Sebastian
>
> Sebastian Reichel (4):
> ARM: dts: LogicPD Torpedo: Add WiLink UART node
> ARM: dts: IGEP: Add WiLink UART node
> ARM: OMAP2+: pdata-quirks: drop TI_ST/KIM support
> Bluetooth: btwilink: drop superseded driver
>
> .../boot/dts/logicpd-torpedo-37xx-devkit.dts | 8 +
> arch/arm/boot/dts/omap3-igep0020-rev-f.dts | 8 +
> arch/arm/boot/dts/omap3-igep0030-rev-g.dts | 8 +
> arch/arm/mach-omap2/pdata-quirks.c | 52 ---
> drivers/bluetooth/Kconfig | 11 -
> drivers/bluetooth/Makefile | 1 -
> drivers/bluetooth/btwilink.c | 337 ------------------
> 7 files changed, 24 insertions(+), 401 deletions(-)
> delete mode 100644 drivers/bluetooth/btwilink.c
>
> --
> 2.23.0
>
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 <[email protected]>
> ---
> 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.
Regards
Marcel
Hi,
On Wed, Oct 16, 2019 at 09:15:03PM +0200, Marcel Holtmann wrote:
> > 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 <[email protected]>
> > ---
> > 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
-- Sebastian
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 <[email protected]>
>>> ---
>>> 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.
Regards
Marcel
* Sebastian Reichel <[email protected]> [191020 20:34]:
> Hi Tony,
>
> On Tue, Oct 08, 2019 at 07:31:16AM -0700, Tony Lindgren wrote:
> > * Sebastian Reichel <[email protected]> [191003 06:42]:
> > > This moves the remaining users of btwilink to the "new" serdev based hci_ll
> > > driver and drops the btwilink driver afterwards. The patches were only compile
> > > tested by me, but Enric tested the IGEP platform and Adam will test the LogicPD
> > > platform.
> > >
> > > I kept the TI_ST driver for now, since I plan to send a second patchset for the
> > > FM radio driver. Once the FM driver has been converted to also use hci_ll, we
> > > can remove TI_ST completly.
> > >
> > > My suggestion is for the patch handling is, that everything simply goes through
> > > Tony's tree.
> >
> > Sounds good to me, good to see kim gone with patch 3/4 :)
> >
> > Marcel, care to ack the old driver removal patch?
>
> Looks like Marcel missed the extra messages and merged the 4th
> patch, so I guess you can just merge patches 1-3. Technically that
> might lead to temporarily missing BT support on those two devices
> when the BT tree is merged before ARM tree during the merge window.
> Not a big issue I guess.
Sure no problem. Applying dts changes into omap-for-v5.5/dt
and the pdata quirk removal to omap-for-v5.5/soc.
I guess ti_wilink_st.h will need some follow-up patch to
remove unused platform data, but best to wait on that.
Regards,
Tony
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 <[email protected]>
>>>>> ---
>>>>> 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