Re: [PATCH] Bluetooth: Add tty HCI driver
Hi Dotan, >> tty_hci driver exposes a /dev/hci_tty character device node, that >> intends to emulate a generic /dev/ttyX device that would be used by >> the user-space Bluetooth stacks to send/receive data to/from the WL >> combo-connectivity chipsets. > > We have an in kernel bluetooth stack, why do we care about this - > how will people test and debug driver interactions with binary only > bluetooth stacks in userspace ? NAK to this driver. If you want to run an external Bluetooth stack, then use HCI User Channel. The functionality already exists. >>> >>> This is a pretty old driver that has been written a couple of years back >>> and is >> being used by customers (including android that use a userspace based stack). >>> If there is another driver already available that provides same >>> functionality, >> of course we will be happy to use it instead. >>> Any idea of where we can get info on the HCI User Channel? >> >> search for my BlueZ for Android presentation from Android Builders Summit >> from 2014. It has an explanation on how HCI User Channel works. >> >> Otherwise look into bluez.git since it contains a bunch of examples on how to >> use the HCI User Channel. It is a generic functionality for raw access to >> the HCI >> layer. Look for HCI_CHANNEL_USER. >> We do not want to have two drivers for the same hardware. Work with the Bluetooth stack that is present in the kernel and please stop hacking >> around it. >>> There are in fact quite a few products that use commercial user space based >> Bluetooth stacks. >>> One reason is that bluez has not been certified but I assume they have >> other reasons as well. >> >> Please stop this FUD. BlueZ has been qualified multiple times. >> >> And let me tell you this, when doing BlueZ for Android, we went through a >> lengthy period of qualification and fixing PTS with 65 errata today. I don't >> know if any commercial stack was actually such a good citizen in ensuring >> that >> the certification tools get fixed. And we documented all of it. >> > > No argue here, as a matter of fact, we (TI) certified BlueZ several times in > the past. Our customers are using large variety of Kernel versions and might > use a different version of BlueZ. Our BQE requested us to certify the stack > per kernel and per BlueZ version which is practically impossible (due to > certification costs and the time it takes). > If you are familiar with a way to bypass it, that would be great. that is why we documented everything for BlueZ for Android in such detail so that the whole host stack can be qualified using PTS. It still means that you have to run 1000+ test cases, but at least you have the full documentation with detailed instructions for it. Automating PTS is work in progress actually. So that you can just start the process and once it is done, you get your evidence for qualification. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] Bluetooth: Add tty HCI driver
Hi Marcel
>-Original Message-
>From: Marcel Holtmann [mailto:mar...@holtmann.org]
>Sent: Thursday, April 30, 2015 5:58 PM
>To: Reizer, Eyal
>Cc: One Thousand Gnomes; Eyal Reizer; linux-kernel@vger.kernel.org; Pavan
>Savoy; Mahaveer, Vishal; linux-blueto...@vger.kernel.org; Ziv, Dotan
>Subject: Re: [PATCH] Bluetooth: Add tty HCI driver
>
>Hi Eyal,
>
>>>>> tty_hci driver exposes a /dev/hci_tty character device node, that
>>>>> intends to emulate a generic /dev/ttyX device that would be used by
>>>>> the user-space Bluetooth stacks to send/receive data to/from the WL
>>>>> combo-connectivity chipsets.
>>>>
>>>> We have an in kernel bluetooth stack, why do we care about this -
>>>> how will people test and debug driver interactions with binary only
>>>> bluetooth stacks in userspace ?
>>>
>>> NAK to this driver.
>>>
>>> If you want to run an external Bluetooth stack, then use HCI User
>>> Channel. The functionality already exists.
>>
>> This is a pretty old driver that has been written a couple of years back and
>> is
>being used by customers (including android that use a userspace based stack).
>> If there is another driver already available that provides same
>> functionality,
>of course we will be happy to use it instead.
>> Any idea of where we can get info on the HCI User Channel?
>
>search for my BlueZ for Android presentation from Android Builders Summit
>from 2014. It has an explanation on how HCI User Channel works.
>
>Otherwise look into bluez.git since it contains a bunch of examples on how to
>use the HCI User Channel. It is a generic functionality for raw access to the
>HCI
>layer. Look for HCI_CHANNEL_USER.
>
>>> We do not want to have two drivers for the same hardware. Work with
>>> the Bluetooth stack that is present in the kernel and please stop hacking
>around it.
>>>
>> There are in fact quite a few products that use commercial user space based
>Bluetooth stacks.
>> One reason is that bluez has not been certified but I assume they have
>other reasons as well.
>
>Please stop this FUD. BlueZ has been qualified multiple times.
>
>And let me tell you this, when doing BlueZ for Android, we went through a
>lengthy period of qualification and fixing PTS with 65 errata today. I don't
>know if any commercial stack was actually such a good citizen in ensuring that
>the certification tools get fixed. And we documented all of it.
>
No argue here, as a matter of fact, we (TI) certified BlueZ several times in
the past. Our customers are using large variety of Kernel versions and might
use a different version of BlueZ. Our BQE requested us to certify the stack per
kernel and per BlueZ version which is practically impossible (due to
certification costs and the time it takes).
If you are familiar with a way to bypass it, that would be great.
>Clone the bluez.git tree and look for android/{pts,pics,pixit}-*.txt files.
>They
>contain all details for qualifying BlueZ for Android stack.
>
>Regards
>
>Marcel
Regards,
Dotan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Bluetooth: Add tty HCI driver
Hi Eyal,
tty_hci driver exposes a /dev/hci_tty character device node, that
intends to emulate a generic /dev/ttyX device that would be used by
the user-space Bluetooth stacks to send/receive data to/from the WL
combo-connectivity chipsets.
>>>
>>> We have an in kernel bluetooth stack, why do we care about this - how
>>> will people test and debug driver interactions with binary only
>>> bluetooth stacks in userspace ?
>>
>> NAK to this driver.
>>
>> If you want to run an external Bluetooth stack, then use HCI User Channel.
>> The
>> functionality already exists.
>
> This is a pretty old driver that has been written a couple of years back and
> is being used by customers (including android that use a userspace based
> stack).
> If there is another driver already available that provides same
> functionality, of course we will be happy to use it instead.
> Any idea of where we can get info on the HCI User Channel?
search for my BlueZ for Android presentation from Android Builders Summit from
2014. It has an explanation on how HCI User Channel works.
Otherwise look into bluez.git since it contains a bunch of examples on how to
use the HCI User Channel. It is a generic functionality for raw access to the
HCI layer. Look for HCI_CHANNEL_USER.
>> We do not want to have two drivers for the same hardware. Work with the
>> Bluetooth stack that is present in the kernel and please stop hacking around
>> it.
>>
> There are in fact quite a few products that use commercial user space based
> Bluetooth stacks.
> One reason is that bluez has not been certified but I assume they have other
> reasons as well.
Please stop this FUD. BlueZ has been qualified multiple times.
And let me tell you this, when doing BlueZ for Android, we went through a
lengthy period of qualification and fixing PTS with 65 errata today. I don't
know if any commercial stack was actually such a good citizen in ensuring that
the certification tools get fixed. And we documented all of it.
Clone the bluez.git tree and look for android/{pts,pics,pixit}-*.txt files.
They contain all details for qualifying BlueZ for Android stack.
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] Bluetooth: Add tty HCI driver
Hi, > -Original Message- > From: Marcel Holtmann [mailto:mar...@holtmann.org] > Sent: Thursday, April 30, 2015 5:25 PM > To: One Thousand Gnomes > Cc: Eyal Reizer; linux-kernel@vger.kernel.org; Reizer, Eyal; Pavan Savoy; > Mahaveer, Vishal; linux-blueto...@vger.kernel.org > Subject: Re: [PATCH] Bluetooth: Add tty HCI driver > > Hi, > > >> tty_hci driver exposes a /dev/hci_tty character device node, that > >> intends to emulate a generic /dev/ttyX device that would be used by > >> the user-space Bluetooth stacks to send/receive data to/from the WL > >> combo-connectivity chipsets. > > > > We have an in kernel bluetooth stack, why do we care about this - how > > will people test and debug driver interactions with binary only > > bluetooth stacks in userspace ? > > NAK to this driver. > > If you want to run an external Bluetooth stack, then use HCI User Channel. The > functionality already exists. This is a pretty old driver that has been written a couple of years back and is being used by customers (including android that use a userspace based stack). If there is another driver already available that provides same functionality, of course we will be happy to use it instead. Any idea of where we can get info on the HCI User Channel? > > We do not want to have two drivers for the same hardware. Work with the > Bluetooth stack that is present in the kernel and please stop hacking around > it. > There are in fact quite a few products that use commercial user space based Bluetooth stacks. One reason is that bluez has not been certified but I assume they have other reasons as well. Best Regards, Eyal -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Bluetooth: Add tty HCI driver
Hi, >> tty_hci driver exposes a /dev/hci_tty character device node, that intends >> to emulate a generic /dev/ttyX device that would be used by the user-space >> Bluetooth stacks to send/receive data to/from the WL combo-connectivity >> chipsets. > > We have an in kernel bluetooth stack, why do we care about this - how > will people test and debug driver interactions with binary only bluetooth > stacks in userspace ? NAK to this driver. If you want to run an external Bluetooth stack, then use HCI User Channel. The functionality already exists. We do not want to have two drivers for the same hardware. Work with the Bluetooth stack that is present in the kernel and please stop hacking around it. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Bluetooth: Add tty HCI driver
On Thu, 30 Apr 2015 10:48:09 +0300
Eyal Reizer wrote:
> tty_hci driver exposes a /dev/hci_tty character device node, that intends
> to emulate a generic /dev/ttyX device that would be used by the user-space
> Bluetooth stacks to send/receive data to/from the WL combo-connectivity
> chipsets.
We have an in kernel bluetooth stack, why do we care about this - how
will people test and debug driver interactions with binary only bluetooth
stacks in userspace ?
That aside
Either
- Use the tty layer to provide your interface with a correct "emulation",
or
- Don't pretend to be a tty device in the first place
Your code behaviour is nothing like a tty or indeed much like anything
else sane.
> +int hci_tty_open(struct inode *inod, struct file *file)
> +{
> + int i = 0, err = 0;
> + unsigned long timeleft;
> + struct ti_st *hst;
> +
> + pr_info("inside %s (%p, %p)\n", __func__, inod, file);
> +
> + hst = kzalloc(sizeof(*hst), GFP_KERNEL);
> + file->private_data = hst;
> + hst = file->private_data;
Just crashed if you ran out of memory
> + pr_debug("waiting for registration completion signal from ST");
> + timeleft = wait_for_completion_timeout
> + (>wait_reg_completion,
> + msecs_to_jiffies(BT_REGISTER_TIMEOUT));
> + if (!timeleft) {
> + pr_err("Timeout(%d sec),didn't get reg completion
> signal from ST",
> +BT_REGISTER_TIMEOUT / 1000);
> + err = -ETIMEDOUT;
> + goto error;
Without de-registering stuff ?
+
> +/** hci_tty_read Function
> + *
> + * Parameters :
> + * @file : File pointer for BT char driver
> + * @data : Data which needs to be passed to APP
> + * @size : Length of the data passesd
> + * offset :
> + * Returns Size of packet received - on success
> + * else suitable error code
> + */
> +ssize_t hci_tty_read(struct file *file, char __user *data, size_t size,
> + loff_t *offset)
> +{
> + int len = 0, tout;
> + struct sk_buff *skb = NULL, *rskb = NULL;
> + struct ti_st*hst;
> +
> + pr_debug("inside %s (%p, %p, %u, %p)\n",
> + __func__, file, data, size, offset);
> +
> + /* Validate input parameters */
> + if ((NULL == file) || (((NULL == data) || (0 == size {
> + pr_err("Invalid input params passed to %s", __func__);
> + return -EINVAL;
> + }
Why ... if they are broken here then the kernel is already crashed.
> +
> + hst = file->private_data;
> +
> + /* cannot come here if poll-ed before reading
> + * if not poll-ed wait on the same wait_q
> + */
> + tout = wait_event_interruptible_timeout(hst->data_q,
> + !skb_queue_empty(>rx_list),
> + msecs_to_jiffies(1000));
> + /* Check for timed out condition */
> + if (0 == tout) {
> + pr_err("Device Read timed out\n");
> + return -ETIMEDOUT;
> + }
> +
> + /* hst->rx_list not empty skb already present */
> + skb = skb_dequeue(>rx_list);
> + if (!skb) {
> + pr_err("dequed skb is null?\n");
> + return -EIO;
> + }
> +
> +#ifdef VERBOSE
> + print_hex_dump(KERN_INFO, ">in>", DUMP_PREFIX_NONE,
> +16, 1, skb->data, skb->len, 0);
> +#endif
> +
> + /* Forward the data to the user */
> + if (skb->len >= size) {
> + pr_err("FIONREAD not done before read\n");
> + return -ENOMEM;
Even if the user did an FIONREAD this wouldn't work with two threads.
ENOMEM is a nonsense return for it
The semantics of read/write on a tty are not those you have implemented
You don't want to allow users to fill the log with error messages
> + }
> + /* returning skb */
> + rskb = alloc_skb(size, GFP_KERNEL);
> + if (!rskb)
> + return -ENOMEM;
> +
> + /* cb[0] has the pkt_type 0x04 or 0x02 or 0x03 */
> + memcpy(skb_put(rskb, 1), >cb[0], 1);
> + memcpy(skb_put(rskb, skb->len), skb->data, skb->len);
> +
> + if (copy_to_user(data, rskb->data, rskb->len)) {
> + pr_err("unable to copy to user space\n");
Same problem with error messages
> + /* Queue the skb back to head */
> + skb_queue_head(>rx_list, skb);
You've just re-ordered your list if threaded
> +/** hci_tty_ioctl Function
> + * This will peform the functions as directed by the command and command
> + * argument.
> + *
> + * Parameters :
> + * @file : File pointer for BT char driver
> + * @cmd : IOCTL Command
> + * @arg : Command argument for IOCTL command
> + * Returns 0 on success
> + * else suitable error code
> + */
> +static long hci_tty_ioctl(struct file *file,
> + unsigned int cmd, unsigned long arg)
> +{
> + struct sk_buff *skb = NULL;
> + int
[PATCH] Bluetooth: Add tty HCI driver
tty_hci driver exposes a /dev/hci_tty character device node, that intends
to emulate a generic /dev/ttyX device that would be used by the user-space
Bluetooth stacks to send/receive data to/from the WL combo-connectivity
chipsets.
The device driver has no internal logic of its own to intrepret data & all
such logic is handled by the user-space stack.
Signed-off-by: Pavan Savoy
[Fixed checkpatch warnings]
Signed-off-by: Vishal Mahaveer
[Fixed checkpatch --strict warnings]
Signed-off-by: Eyal Reizer
---
drivers/misc/ti-st/Kconfig |8 +
drivers/misc/ti-st/Makefile |1 +
drivers/misc/ti-st/tty_hci.c | 538 ++
3 files changed, 547 insertions(+)
create mode 100644 drivers/misc/ti-st/tty_hci.c
diff --git a/drivers/misc/ti-st/Kconfig b/drivers/misc/ti-st/Kconfig
index f34dcc5..f2df2c7 100644
--- a/drivers/misc/ti-st/Kconfig
+++ b/drivers/misc/ti-st/Kconfig
@@ -14,4 +14,12 @@ config TI_ST
are returned to relevant protocol drivers based on their
packet types.
+config ST_HCI
+ tristate "HCI TTY emulation driver for Bluetooth"
+ depends on TI_ST
+ help
+ This enables the TTY device like emulation for HCI used by
+ user-space Bluetooth stacks.
+ It will provide a character device for user space Bluetooth stack to
+ send/receive data over shared transport.
endmenu
diff --git a/drivers/misc/ti-st/Makefile b/drivers/misc/ti-st/Makefile
index 78d7ebb..4546219 100644
--- a/drivers/misc/ti-st/Makefile
+++ b/drivers/misc/ti-st/Makefile
@@ -4,3 +4,4 @@
#
obj-$(CONFIG_TI_ST)+= st_drv.o
st_drv-objs:= st_core.o st_kim.o st_ll.o
+obj-$(CONFIG_ST_HCI) += tty_hci.o
diff --git a/drivers/misc/ti-st/tty_hci.c b/drivers/misc/ti-st/tty_hci.c
new file mode 100644
index 000..7a6669f
--- /dev/null
+++ b/drivers/misc/ti-st/tty_hci.c
@@ -0,0 +1,538 @@
+/*
+ * TTY emulation for user-space Bluetooth stacks over HCI-H4
+ * Copyright (C) 2011-2012 Texas Instruments
+ * Author: Pavan Savoy
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+/** define one of the following for debugging
+#define DEBUG
+#define VERBOSE
+*/
+
+#define pr_fmt(fmt) "(hci_tty): " fmt
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+/* 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;
+ char reg_status;
+ long (*st_write)(struct sk_buff *);
+ struct completion wait_reg_completion;
+ wait_queue_head_t data_q;
+ struct sk_buff_head rx_list;
+};
+
+#define DEVICE_NAME "hci_tty"
+
+/***Functions called from ST driver**/
+/* Called by Shared Transport layer when receive data is
+ * available */
+static long st_receive(void *priv_data, struct sk_buff *skb)
+{
+ struct ti_st*hst = (void *)priv_data;
+
+ pr_debug("@ %s", __func__);
+#ifdef VERBOSE
+ print_hex_dump(KERN_INFO, ">rx>", DUMP_PREFIX_NONE,
+ 16, 1, skb->data, skb->len, 0);
+#endif
+ skb_queue_tail(>rx_list, skb);
+ wake_up_interruptible(>data_q);
+ return 0;
+}
+
+/* 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, char data)
+{
+ struct ti_st*lhst = (void *)priv_data;
+
+ pr_info("@ %s\n", __func__);
+ /* Save registration status for use in ti_st_open() */
+ lhst->reg_status = data;
+ /* complete the wait in ti_st_open() */
+ complete(>wait_reg_completion);
+}
+
+/* protocol structure registered with shared transport */
+#define MAX_BT_CHNL_IDS 3
+static struct st_proto_s ti_st_proto[MAX_BT_CHNL_IDS] = {
+ {
+ .chnl_id = 0x04, /* HCI Events */
+ .hdr_len = 2,
+ .offset_len_in_hdr = 1,
+ .len_size =
[PATCH] Bluetooth: Add tty HCI driver
tty_hci driver exposes a /dev/hci_tty character device node, that intends
to emulate a generic /dev/ttyX device that would be used by the user-space
Bluetooth stacks to send/receive data to/from the WL combo-connectivity
chipsets.
The device driver has no internal logic of its own to intrepret data all
such logic is handled by the user-space stack.
Signed-off-by: Pavan Savoy pavan_sa...@ti.com
[Fixed checkpatch warnings]
Signed-off-by: Vishal Mahaveer vish...@ti.com
[Fixed checkpatch --strict warnings]
Signed-off-by: Eyal Reizer ey...@ti.com
---
drivers/misc/ti-st/Kconfig |8 +
drivers/misc/ti-st/Makefile |1 +
drivers/misc/ti-st/tty_hci.c | 538 ++
3 files changed, 547 insertions(+)
create mode 100644 drivers/misc/ti-st/tty_hci.c
diff --git a/drivers/misc/ti-st/Kconfig b/drivers/misc/ti-st/Kconfig
index f34dcc5..f2df2c7 100644
--- a/drivers/misc/ti-st/Kconfig
+++ b/drivers/misc/ti-st/Kconfig
@@ -14,4 +14,12 @@ config TI_ST
are returned to relevant protocol drivers based on their
packet types.
+config ST_HCI
+ tristate HCI TTY emulation driver for Bluetooth
+ depends on TI_ST
+ help
+ This enables the TTY device like emulation for HCI used by
+ user-space Bluetooth stacks.
+ It will provide a character device for user space Bluetooth stack to
+ send/receive data over shared transport.
endmenu
diff --git a/drivers/misc/ti-st/Makefile b/drivers/misc/ti-st/Makefile
index 78d7ebb..4546219 100644
--- a/drivers/misc/ti-st/Makefile
+++ b/drivers/misc/ti-st/Makefile
@@ -4,3 +4,4 @@
#
obj-$(CONFIG_TI_ST)+= st_drv.o
st_drv-objs:= st_core.o st_kim.o st_ll.o
+obj-$(CONFIG_ST_HCI) += tty_hci.o
diff --git a/drivers/misc/ti-st/tty_hci.c b/drivers/misc/ti-st/tty_hci.c
new file mode 100644
index 000..7a6669f
--- /dev/null
+++ b/drivers/misc/ti-st/tty_hci.c
@@ -0,0 +1,538 @@
+/*
+ * TTY emulation for user-space Bluetooth stacks over HCI-H4
+ * Copyright (C) 2011-2012 Texas Instruments
+ * Author: Pavan Savoy pavan_sa...@ti.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+/** define one of the following for debugging
+#define DEBUG
+#define VERBOSE
+*/
+
+#define pr_fmt(fmt) (hci_tty): fmt
+#include linux/module.h
+#include linux/cdev.h
+#include linux/fs.h
+#include linux/device.h
+
+#include linux/uaccess.h
+#include linux/tty.h
+#include linux/sched.h
+
+#include linux/delay.h
+#include linux/firmware.h
+#include linux/platform_device.h
+#include linux/poll.h
+#include linux/skbuff.h
+#include linux/interrupt.h
+
+#include linux/ti_wilink_st.h
+
+/* 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;
+ char reg_status;
+ long (*st_write)(struct sk_buff *);
+ struct completion wait_reg_completion;
+ wait_queue_head_t data_q;
+ struct sk_buff_head rx_list;
+};
+
+#define DEVICE_NAME hci_tty
+
+/***Functions called from ST driver**/
+/* Called by Shared Transport layer when receive data is
+ * available */
+static long st_receive(void *priv_data, struct sk_buff *skb)
+{
+ struct ti_st*hst = (void *)priv_data;
+
+ pr_debug(@ %s, __func__);
+#ifdef VERBOSE
+ print_hex_dump(KERN_INFO, rx, DUMP_PREFIX_NONE,
+ 16, 1, skb-data, skb-len, 0);
+#endif
+ skb_queue_tail(hst-rx_list, skb);
+ wake_up_interruptible(hst-data_q);
+ return 0;
+}
+
+/* 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, char data)
+{
+ struct ti_st*lhst = (void *)priv_data;
+
+ pr_info(@ %s\n, __func__);
+ /* 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);
+}
+
+/* protocol structure registered with
Re: [PATCH] Bluetooth: Add tty HCI driver
On Thu, 30 Apr 2015 10:48:09 +0300
Eyal Reizer eyalrei...@gmail.com wrote:
tty_hci driver exposes a /dev/hci_tty character device node, that intends
to emulate a generic /dev/ttyX device that would be used by the user-space
Bluetooth stacks to send/receive data to/from the WL combo-connectivity
chipsets.
We have an in kernel bluetooth stack, why do we care about this - how
will people test and debug driver interactions with binary only bluetooth
stacks in userspace ?
That aside
Either
- Use the tty layer to provide your interface with a correct emulation,
or
- Don't pretend to be a tty device in the first place
Your code behaviour is nothing like a tty or indeed much like anything
else sane.
+int hci_tty_open(struct inode *inod, struct file *file)
+{
+ int i = 0, err = 0;
+ unsigned long timeleft;
+ struct ti_st *hst;
+
+ pr_info(inside %s (%p, %p)\n, __func__, inod, file);
+
+ hst = kzalloc(sizeof(*hst), GFP_KERNEL);
+ file-private_data = hst;
+ hst = file-private_data;
Just crashed if you ran out of memory
+ pr_debug(waiting for registration completion signal from ST);
+ timeleft = wait_for_completion_timeout
+ (hst-wait_reg_completion,
+ msecs_to_jiffies(BT_REGISTER_TIMEOUT));
+ if (!timeleft) {
+ pr_err(Timeout(%d sec),didn't get reg completion
signal from ST,
+BT_REGISTER_TIMEOUT / 1000);
+ err = -ETIMEDOUT;
+ goto error;
Without de-registering stuff ?
+
+/** hci_tty_read Function
+ *
+ * Parameters :
+ * @file : File pointer for BT char driver
+ * @data : Data which needs to be passed to APP
+ * @size : Length of the data passesd
+ * offset :
+ * Returns Size of packet received - on success
+ * else suitable error code
+ */
+ssize_t hci_tty_read(struct file *file, char __user *data, size_t size,
+ loff_t *offset)
+{
+ int len = 0, tout;
+ struct sk_buff *skb = NULL, *rskb = NULL;
+ struct ti_st*hst;
+
+ pr_debug(inside %s (%p, %p, %u, %p)\n,
+ __func__, file, data, size, offset);
+
+ /* Validate input parameters */
+ if ((NULL == file) || (((NULL == data) || (0 == size {
+ pr_err(Invalid input params passed to %s, __func__);
+ return -EINVAL;
+ }
Why ... if they are broken here then the kernel is already crashed.
+
+ hst = file-private_data;
+
+ /* cannot come here if poll-ed before reading
+ * if not poll-ed wait on the same wait_q
+ */
+ tout = wait_event_interruptible_timeout(hst-data_q,
+ !skb_queue_empty(hst-rx_list),
+ msecs_to_jiffies(1000));
+ /* Check for timed out condition */
+ if (0 == tout) {
+ pr_err(Device Read timed out\n);
+ return -ETIMEDOUT;
+ }
+
+ /* hst-rx_list not empty skb already present */
+ skb = skb_dequeue(hst-rx_list);
+ if (!skb) {
+ pr_err(dequed skb is null?\n);
+ return -EIO;
+ }
+
+#ifdef VERBOSE
+ print_hex_dump(KERN_INFO, in, DUMP_PREFIX_NONE,
+16, 1, skb-data, skb-len, 0);
+#endif
+
+ /* Forward the data to the user */
+ if (skb-len = size) {
+ pr_err(FIONREAD not done before read\n);
+ return -ENOMEM;
Even if the user did an FIONREAD this wouldn't work with two threads.
ENOMEM is a nonsense return for it
The semantics of read/write on a tty are not those you have implemented
You don't want to allow users to fill the log with error messages
+ }
+ /* returning skb */
+ rskb = alloc_skb(size, GFP_KERNEL);
+ if (!rskb)
+ return -ENOMEM;
+
+ /* cb[0] has the pkt_type 0x04 or 0x02 or 0x03 */
+ memcpy(skb_put(rskb, 1), skb-cb[0], 1);
+ memcpy(skb_put(rskb, skb-len), skb-data, skb-len);
+
+ if (copy_to_user(data, rskb-data, rskb-len)) {
+ pr_err(unable to copy to user space\n);
Same problem with error messages
+ /* Queue the skb back to head */
+ skb_queue_head(hst-rx_list, skb);
You've just re-ordered your list if threaded
+/** hci_tty_ioctl Function
+ * This will peform the functions as directed by the command and command
+ * argument.
+ *
+ * Parameters :
+ * @file : File pointer for BT char driver
+ * @cmd : IOCTL Command
+ * @arg : Command argument for IOCTL command
+ * Returns 0 on success
+ * else suitable error code
+ */
+static long hci_tty_ioctl(struct file *file,
+ unsigned int cmd, unsigned long arg)
+{
+ struct sk_buff *skb = NULL;
+ int retcode = 0;
+ struct ti_st*hst;
+
+ pr_debug(inside %s (%p, %u, %lx), __func__, file, cmd,
RE: [PATCH] Bluetooth: Add tty HCI driver
Hi Marcel
-Original Message-
From: Marcel Holtmann [mailto:mar...@holtmann.org]
Sent: Thursday, April 30, 2015 5:58 PM
To: Reizer, Eyal
Cc: One Thousand Gnomes; Eyal Reizer; linux-kernel@vger.kernel.org; Pavan
Savoy; Mahaveer, Vishal; linux-blueto...@vger.kernel.org; Ziv, Dotan
Subject: Re: [PATCH] Bluetooth: Add tty HCI driver
Hi Eyal,
tty_hci driver exposes a /dev/hci_tty character device node, that
intends to emulate a generic /dev/ttyX device that would be used by
the user-space Bluetooth stacks to send/receive data to/from the WL
combo-connectivity chipsets.
We have an in kernel bluetooth stack, why do we care about this -
how will people test and debug driver interactions with binary only
bluetooth stacks in userspace ?
NAK to this driver.
If you want to run an external Bluetooth stack, then use HCI User
Channel. The functionality already exists.
This is a pretty old driver that has been written a couple of years back and
is
being used by customers (including android that use a userspace based stack).
If there is another driver already available that provides same
functionality,
of course we will be happy to use it instead.
Any idea of where we can get info on the HCI User Channel?
search for my BlueZ for Android presentation from Android Builders Summit
from 2014. It has an explanation on how HCI User Channel works.
Otherwise look into bluez.git since it contains a bunch of examples on how to
use the HCI User Channel. It is a generic functionality for raw access to the
HCI
layer. Look for HCI_CHANNEL_USER.
We do not want to have two drivers for the same hardware. Work with
the Bluetooth stack that is present in the kernel and please stop hacking
around it.
There are in fact quite a few products that use commercial user space based
Bluetooth stacks.
One reason is that bluez has not been certified but I assume they have
other reasons as well.
Please stop this FUD. BlueZ has been qualified multiple times.
And let me tell you this, when doing BlueZ for Android, we went through a
lengthy period of qualification and fixing PTS with 65 errata today. I don't
know if any commercial stack was actually such a good citizen in ensuring that
the certification tools get fixed. And we documented all of it.
No argue here, as a matter of fact, we (TI) certified BlueZ several times in
the past. Our customers are using large variety of Kernel versions and might
use a different version of BlueZ. Our BQE requested us to certify the stack per
kernel and per BlueZ version which is practically impossible (due to
certification costs and the time it takes).
If you are familiar with a way to bypass it, that would be great.
Clone the bluez.git tree and look for android/{pts,pics,pixit}-*.txt files.
They
contain all details for qualifying BlueZ for Android stack.
Regards
Marcel
Regards,
Dotan
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] Bluetooth: Add tty HCI driver
Hi, -Original Message- From: Marcel Holtmann [mailto:mar...@holtmann.org] Sent: Thursday, April 30, 2015 5:25 PM To: One Thousand Gnomes Cc: Eyal Reizer; linux-kernel@vger.kernel.org; Reizer, Eyal; Pavan Savoy; Mahaveer, Vishal; linux-blueto...@vger.kernel.org Subject: Re: [PATCH] Bluetooth: Add tty HCI driver Hi, tty_hci driver exposes a /dev/hci_tty character device node, that intends to emulate a generic /dev/ttyX device that would be used by the user-space Bluetooth stacks to send/receive data to/from the WL combo-connectivity chipsets. We have an in kernel bluetooth stack, why do we care about this - how will people test and debug driver interactions with binary only bluetooth stacks in userspace ? NAK to this driver. If you want to run an external Bluetooth stack, then use HCI User Channel. The functionality already exists. This is a pretty old driver that has been written a couple of years back and is being used by customers (including android that use a userspace based stack). If there is another driver already available that provides same functionality, of course we will be happy to use it instead. Any idea of where we can get info on the HCI User Channel? We do not want to have two drivers for the same hardware. Work with the Bluetooth stack that is present in the kernel and please stop hacking around it. There are in fact quite a few products that use commercial user space based Bluetooth stacks. One reason is that bluez has not been certified but I assume they have other reasons as well. Best Regards, Eyal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Bluetooth: Add tty HCI driver
Hi, tty_hci driver exposes a /dev/hci_tty character device node, that intends to emulate a generic /dev/ttyX device that would be used by the user-space Bluetooth stacks to send/receive data to/from the WL combo-connectivity chipsets. We have an in kernel bluetooth stack, why do we care about this - how will people test and debug driver interactions with binary only bluetooth stacks in userspace ? NAK to this driver. If you want to run an external Bluetooth stack, then use HCI User Channel. The functionality already exists. We do not want to have two drivers for the same hardware. Work with the Bluetooth stack that is present in the kernel and please stop hacking around it. Regards Marcel -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Bluetooth: Add tty HCI driver
Hi Dotan, tty_hci driver exposes a /dev/hci_tty character device node, that intends to emulate a generic /dev/ttyX device that would be used by the user-space Bluetooth stacks to send/receive data to/from the WL combo-connectivity chipsets. We have an in kernel bluetooth stack, why do we care about this - how will people test and debug driver interactions with binary only bluetooth stacks in userspace ? NAK to this driver. If you want to run an external Bluetooth stack, then use HCI User Channel. The functionality already exists. This is a pretty old driver that has been written a couple of years back and is being used by customers (including android that use a userspace based stack). If there is another driver already available that provides same functionality, of course we will be happy to use it instead. Any idea of where we can get info on the HCI User Channel? search for my BlueZ for Android presentation from Android Builders Summit from 2014. It has an explanation on how HCI User Channel works. Otherwise look into bluez.git since it contains a bunch of examples on how to use the HCI User Channel. It is a generic functionality for raw access to the HCI layer. Look for HCI_CHANNEL_USER. We do not want to have two drivers for the same hardware. Work with the Bluetooth stack that is present in the kernel and please stop hacking around it. There are in fact quite a few products that use commercial user space based Bluetooth stacks. One reason is that bluez has not been certified but I assume they have other reasons as well. Please stop this FUD. BlueZ has been qualified multiple times. And let me tell you this, when doing BlueZ for Android, we went through a lengthy period of qualification and fixing PTS with 65 errata today. I don't know if any commercial stack was actually such a good citizen in ensuring that the certification tools get fixed. And we documented all of it. No argue here, as a matter of fact, we (TI) certified BlueZ several times in the past. Our customers are using large variety of Kernel versions and might use a different version of BlueZ. Our BQE requested us to certify the stack per kernel and per BlueZ version which is practically impossible (due to certification costs and the time it takes). If you are familiar with a way to bypass it, that would be great. that is why we documented everything for BlueZ for Android in such detail so that the whole host stack can be qualified using PTS. It still means that you have to run 1000+ test cases, but at least you have the full documentation with detailed instructions for it. Automating PTS is work in progress actually. So that you can just start the process and once it is done, you get your evidence for qualification. Regards Marcel -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Bluetooth: Add tty HCI driver
Hi Eyal,
tty_hci driver exposes a /dev/hci_tty character device node, that
intends to emulate a generic /dev/ttyX device that would be used by
the user-space Bluetooth stacks to send/receive data to/from the WL
combo-connectivity chipsets.
We have an in kernel bluetooth stack, why do we care about this - how
will people test and debug driver interactions with binary only
bluetooth stacks in userspace ?
NAK to this driver.
If you want to run an external Bluetooth stack, then use HCI User Channel.
The
functionality already exists.
This is a pretty old driver that has been written a couple of years back and
is being used by customers (including android that use a userspace based
stack).
If there is another driver already available that provides same
functionality, of course we will be happy to use it instead.
Any idea of where we can get info on the HCI User Channel?
search for my BlueZ for Android presentation from Android Builders Summit from
2014. It has an explanation on how HCI User Channel works.
Otherwise look into bluez.git since it contains a bunch of examples on how to
use the HCI User Channel. It is a generic functionality for raw access to the
HCI layer. Look for HCI_CHANNEL_USER.
We do not want to have two drivers for the same hardware. Work with the
Bluetooth stack that is present in the kernel and please stop hacking around
it.
There are in fact quite a few products that use commercial user space based
Bluetooth stacks.
One reason is that bluez has not been certified but I assume they have other
reasons as well.
Please stop this FUD. BlueZ has been qualified multiple times.
And let me tell you this, when doing BlueZ for Android, we went through a
lengthy period of qualification and fixing PTS with 65 errata today. I don't
know if any commercial stack was actually such a good citizen in ensuring that
the certification tools get fixed. And we documented all of it.
Clone the bluez.git tree and look for android/{pts,pics,pixit}-*.txt files.
They contain all details for qualifying BlueZ for Android stack.
Regards
Marcel
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
