This patch provides option for hci transport driver protocol implementation
to have a callback for hci open.
Signed-off-by: Suraj Sumangala <[email protected]>
---
drivers/bluetooth/hci_ldisc.c | 5 ++++-
drivers/bluetooth/hci_uart.h | 1 +
2 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 998833d..5e02501 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -162,9 +162,12 @@ restart:
/* Initialize device */
static int hci_uart_open(struct hci_dev *hdev)
{
+ struct hci_uart *hu = (struct hci_uart *) hdev->driver_data;
+
BT_DBG("%s %p", hdev->name, hdev);
- /* Nothing to do for UART driver */
+ if (hu->proto->hci_open)
+ hu->proto->hci_open(hu);
set_bit(HCI_RUNNING, &hdev->flags);
diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
index 99fb352..d0198ec 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -51,6 +51,7 @@ struct hci_uart;
struct hci_uart_proto {
unsigned int id;
int (*open)(struct hci_uart *hu);
+ int (*hci_open)(struct hci_uart *hu);
int (*close)(struct hci_uart *hu);
int (*flush)(struct hci_uart *hu);
int (*recv)(struct hci_uart *hu, void *data, int len);
--
1.7.0.4
*ping*
On 9/21/2010 7:03 PM, Suraj Sumangala wrote:
> This patch provides option for hci transport driver protocol implementation
> to have a callback for hci open.
>
> Signed-off-by: Suraj Sumangala<[email protected]>
> ---
> drivers/bluetooth/hci_ldisc.c | 5 ++++-
> drivers/bluetooth/hci_uart.h | 1 +
> 2 files changed, 5 insertions(+), 1 deletions(-)
>
Regards
Suraj
Hi Pavan,
On 9/23/2010 2:52 AM, Pavan Savoy wrote:
> Suraj,
>
> On Tue, Sep 21, 2010 at 11:01 PM, Suraj Sumangala<[email protected]> wrote:
>> Hi Pavan,
>>
>> On 9/22/2010 1:22 AM, Pavan Savoy wrote:
>>>
>>> On Tue, Sep 21, 2010 at 8:33 AM, Suraj Sumangala<[email protected]>
>>> wrote:
>>>>
>>>> This patch enables HCI_UART_ATH3K transport driver to support
>>>> sending Vendor specific hci commands during hci open
>>>> to enable or disable power management feature.
>>>
>>> Why? shouldn't this be done from the hciattach? like for the other
>>> manufacturers?
>>> If you want it to be sent before hci0 interface is exposed, send it
>>> over ttyXX, you have your _init function and if you require it to be
>>> sent after the hci0 is exposed - do it in the _post function.
>>>
>> We are already using the _init and _post of hciattach.
>>
>> The mentioned feature will get disabled in the controller on receiving a HCI
>> RESET command.
>>
>> If the user does an HCI close, this feature will be disabled and we need to
>> enable it again when the user opens the HCI device again.
>>
>> I guess the "hdev->driver_init" queue is provided for that reason.
>>
>> An hciattach is called only once but hci open/close can be done multiple
>> times.
>
> Well this is debatable, I guess we had this discussion when our
> manufacturer came up with PM feature like this - As to what is the
> right BT on procedure?
> Should hciattach be terminated when BT is Off or is it just a
> hciconfig hci0 down - So we decided to get rid of hciattach way of
> doing things.
Yes, it would work fine for me too on a customer platform. but my idea
was to get it working on a generic x86 system running a normal distribution.
Moreover, if you looks at the "hdev->driver_init" queue, it looks like
it is meant for exactly this requirement. Bluez already have code in
hci_init_req() to take care of this scenario.
>
> Also HCI_QUIRK_NO_RESET allows you to not to have reset .. in certain
> cases, I guess with this your chip's firmware is able to remember the
> PM settings previously sent across?
HCI_QUIRK_NO_RESET only lets you not send a HCI reset during
initialization. I have no issues there, I am using the _post callback in
hciattach to configure PM.
I am doing the testing in Ubuntu 10.04 using the Blueman as my Bluetooth
agent.
From the UI if the user does a BT off. It sends an HCI reset command
followed by ah HCI close. This is causing the firmware forget the PM
parameters.
>
>> Regards
>> Suraj
>>
Regards
Suraj
Suraj,
On Tue, Sep 21, 2010 at 11:01 PM, Suraj Sumangala <[email protected]> wrote:
> Hi Pavan,
>
> On 9/22/2010 1:22 AM, Pavan Savoy wrote:
>>
>> On Tue, Sep 21, 2010 at 8:33 AM, Suraj Sumangala<[email protected]>
>> wrote:
>>>
>>> This patch enables HCI_UART_ATH3K transport driver to support
>>> sending Vendor specific hci commands during hci open
>>> to enable or disable power management feature.
>>
>> Why? shouldn't this be done from the hciattach? like for the other
>> manufacturers?
>> If you want it to be sent before hci0 interface is exposed, send it
>> over ttyXX, you have your _init function and if you require it to be
>> sent after the hci0 is exposed - do it in the _post function.
>>
> We are already using the _init and _post of hciattach.
>
> The mentioned feature will get disabled in the controller on receiving a HCI
> RESET command.
>
> If the user does an HCI close, this feature will be disabled and we need to
> enable it again when the user opens the HCI device again.
>
> I guess the "hdev->driver_init" queue is provided for that reason.
>
> An hciattach is called only once but hci open/close can be done multiple
> times.
Well this is debatable, I guess we had this discussion when our
manufacturer came up with PM feature like this - As to what is the
right BT on procedure?
Should hciattach be terminated when BT is Off or is it just a
hciconfig hci0 down - So we decided to get rid of hciattach way of
doing things.
Also HCI_QUIRK_NO_RESET allows you to not to have reset .. in certain
cases, I guess with this your chip's firmware is able to remember the
PM settings previously sent across?
> Regards
> Suraj
>
Hi Pavan,
On 9/22/2010 1:22 AM, Pavan Savoy wrote:
> On Tue, Sep 21, 2010 at 8:33 AM, Suraj Sumangala<[email protected]> wrote:
>> This patch enables HCI_UART_ATH3K transport driver to support
>> sending Vendor specific hci commands during hci open
>> to enable or disable power management feature.
>
> Why? shouldn't this be done from the hciattach? like for the other
> manufacturers?
> If you want it to be sent before hci0 interface is exposed, send it
> over ttyXX, you have your _init function and if you require it to be
> sent after the hci0 is exposed - do it in the _post function.
>
We are already using the _init and _post of hciattach.
The mentioned feature will get disabled in the controller on receiving a
HCI RESET command.
If the user does an HCI close, this feature will be disabled and we need
to enable it again when the user opens the HCI device again.
I guess the "hdev->driver_init" queue is provided for that reason.
An hciattach is called only once but hci open/close can be done multiple
times.
Regards
Suraj
On Tue, Sep 21, 2010 at 8:33 AM, Suraj Sumangala <[email protected]> wrote:
> This patch enables HCI_UART_ATH3K transport driver to support
> sending Vendor specific hci commands during hci open
> to enable or disable power management feature.
Why? shouldn't this be done from the hciattach? like for the other
manufacturers?
If you want it to be sent before hci0 interface is exposed, send it
over ttyXX, you have your _init function and if you require it to be
sent after the hci0 is exposed - do it in the _post function.
> Signed-off-by: Suraj Sumangala <[email protected]>
> ---
> drivers/bluetooth/hci_ath.c | 59 +++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 59 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_ath.c b/drivers/bluetooth/hci_ath.c
> index 6a160c1..d4b0653 100644
> --- a/drivers/bluetooth/hci_ath.c
> +++ b/drivers/bluetooth/hci_ath.c
> @@ -49,6 +49,37 @@ struct ath_struct {
> struct work_struct ctxtsw;
> };
>
> +/* Form HCI command */
> +static struct sk_buff *form_hci_cmd(struct hci_dev *hdev, __u16 opcode,
> + __u32 plen, void *param)
> +{
> + int len = HCI_COMMAND_HDR_SIZE + plen;
> + struct hci_command_hdr *hdr;
> + struct sk_buff *skb;
> +
> + BT_DBG("%s opcode 0x%x plen %d", hdev->name, opcode, plen);
> +
> + skb = bt_skb_alloc(len, GFP_ATOMIC);
> + if (!skb) {
> + BT_ERR("%s no memory for command", hdev->name);
> + return NULL;
> + }
> +
> + hdr = (struct hci_command_hdr *) skb_put(skb, HCI_COMMAND_HDR_SIZE);
> + hdr->opcode = cpu_to_le16(opcode);
> + hdr->plen = plen;
> +
> + if (plen)
> + memcpy(skb_put(skb, plen), param, plen);
> +
> + BT_DBG("skb len %d", skb->len);
> +
> + bt_cb(skb)->pkt_type = HCI_COMMAND_PKT;
> + skb->dev = (void *) hdev;
> +
> + return skb;
> +}
> +
> static int ath_wakeup_ar3k(struct tty_struct *tty)
> {
> struct termios settings;
> @@ -81,6 +112,26 @@ static int ath_wakeup_ar3k(struct tty_struct *tty)
> return status;
> }
>
> +#define HCI_OP_SLEEP_SET 0xFC04
> +static void ath_hci_init_driver(struct hci_uart *hu)
> +{
> + struct ath_struct *ath = hu->priv;
> + struct hci_dev *hdev = hu->hdev;
> + struct sk_buff *skb;
> +
> + if (!hdev)
> + return;
> +
> + if (!ath)
> + return;
> +
> + skb = form_hci_cmd(hdev, HCI_OP_SLEEP_SET, 1, &ath->cur_sleep);
> + if (!skb)
> + return;
> +
> + skb_queue_head(&hdev->driver_init, skb);
> +}
> +
> static void ath_hci_uart_work(struct work_struct *work)
> {
> int status;
> @@ -126,6 +177,13 @@ static int ath_open(struct hci_uart *hu)
> return 0;
> }
>
> +static int ath_hci_open(struct hci_uart *hu)
> +{
> + ath_hci_init_driver(hu);
> +
> + return 0;
> +}
> +
> /* Flush protocol data */
> static int ath_flush(struct hci_uart *hu)
> {
> @@ -210,6 +268,7 @@ static int ath_recv(struct hci_uart *hu, void *data, int count)
> static struct hci_uart_proto athp = {
> .id = HCI_UART_ATH3K,
> .open = ath_open,
> + .hci_open = ath_hci_open,
> .close = ath_close,
> .recv = ath_recv,
> .enqueue = ath_enqueue,
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
This patch enables HCI_UART_ATH3K transport driver to support
sending Vendor specific hci commands during hci open
to enable or disable power management feature.
Signed-off-by: Suraj Sumangala <[email protected]>
---
drivers/bluetooth/hci_ath.c | 59 +++++++++++++++++++++++++++++++++++++++++++
1 files changed, 59 insertions(+), 0 deletions(-)
diff --git a/drivers/bluetooth/hci_ath.c b/drivers/bluetooth/hci_ath.c
index 6a160c1..d4b0653 100644
--- a/drivers/bluetooth/hci_ath.c
+++ b/drivers/bluetooth/hci_ath.c
@@ -49,6 +49,37 @@ struct ath_struct {
struct work_struct ctxtsw;
};
+/* Form HCI command */
+static struct sk_buff *form_hci_cmd(struct hci_dev *hdev, __u16 opcode,
+ __u32 plen, void *param)
+{
+ int len = HCI_COMMAND_HDR_SIZE + plen;
+ struct hci_command_hdr *hdr;
+ struct sk_buff *skb;
+
+ BT_DBG("%s opcode 0x%x plen %d", hdev->name, opcode, plen);
+
+ skb = bt_skb_alloc(len, GFP_ATOMIC);
+ if (!skb) {
+ BT_ERR("%s no memory for command", hdev->name);
+ return NULL;
+ }
+
+ hdr = (struct hci_command_hdr *) skb_put(skb, HCI_COMMAND_HDR_SIZE);
+ hdr->opcode = cpu_to_le16(opcode);
+ hdr->plen = plen;
+
+ if (plen)
+ memcpy(skb_put(skb, plen), param, plen);
+
+ BT_DBG("skb len %d", skb->len);
+
+ bt_cb(skb)->pkt_type = HCI_COMMAND_PKT;
+ skb->dev = (void *) hdev;
+
+ return skb;
+}
+
static int ath_wakeup_ar3k(struct tty_struct *tty)
{
struct termios settings;
@@ -81,6 +112,26 @@ static int ath_wakeup_ar3k(struct tty_struct *tty)
return status;
}
+#define HCI_OP_SLEEP_SET 0xFC04
+static void ath_hci_init_driver(struct hci_uart *hu)
+{
+ struct ath_struct *ath = hu->priv;
+ struct hci_dev *hdev = hu->hdev;
+ struct sk_buff *skb;
+
+ if (!hdev)
+ return;
+
+ if (!ath)
+ return;
+
+ skb = form_hci_cmd(hdev, HCI_OP_SLEEP_SET, 1, &ath->cur_sleep);
+ if (!skb)
+ return;
+
+ skb_queue_head(&hdev->driver_init, skb);
+}
+
static void ath_hci_uart_work(struct work_struct *work)
{
int status;
@@ -126,6 +177,13 @@ static int ath_open(struct hci_uart *hu)
return 0;
}
+static int ath_hci_open(struct hci_uart *hu)
+{
+ ath_hci_init_driver(hu);
+
+ return 0;
+}
+
/* Flush protocol data */
static int ath_flush(struct hci_uart *hu)
{
@@ -210,6 +268,7 @@ static int ath_recv(struct hci_uart *hu, void *data, int count)
static struct hci_uart_proto athp = {
.id = HCI_UART_ATH3K,
.open = ath_open,
+ .hci_open = ath_hci_open,
.close = ath_close,
.recv = ath_recv,
.enqueue = ath_enqueue,
--
1.7.0.4
Hi Suraj,
> This patch provides option for hci transport driver protocol implementation
> to have a callback for hci open.
>
> Signed-off-by: Suraj Sumangala <[email protected]>
> ---
> drivers/bluetooth/hci_ldisc.c | 5 ++++-
> drivers/bluetooth/hci_uart.h | 1 +
> 2 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> index 998833d..5e02501 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -162,9 +162,12 @@ restart:
> /* Initialize device */
> static int hci_uart_open(struct hci_dev *hdev)
> {
> + struct hci_uart *hu = (struct hci_uart *) hdev->driver_data;
> +
> BT_DBG("%s %p", hdev->name, hdev);
>
> - /* Nothing to do for UART driver */
> + if (hu->proto->hci_open)
> + hu->proto->hci_open(hu);
>
> set_bit(HCI_RUNNING, &hdev->flags);
>
> diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
> index 99fb352..d0198ec 100644
> --- a/drivers/bluetooth/hci_uart.h
> +++ b/drivers/bluetooth/hci_uart.h
> @@ -51,6 +51,7 @@ struct hci_uart;
> struct hci_uart_proto {
> unsigned int id;
> int (*open)(struct hci_uart *hu);
> + int (*hci_open)(struct hci_uart *hu);
> int (*close)(struct hci_uart *hu);
> int (*flush)(struct hci_uart *hu);
> int (*recv)(struct hci_uart *hu, void *data, int len);
I don't like this. It is not what any other driver is doing. For me this
is just hacking around something that would need to be solved for all
Bluetooth transport drivers. Just trying to solve this for UART based
drivers is not going to help.
So we talked about adding a setup stage to the HCI driver interface.
That is the way you should proceed if doing this in hciattach is not
enough for you.
Regards
Marcel
Hi,
On 9/30/2010 2:01 PM, Suraj Sumangala wrote:
> *ping*
>
> On 9/21/2010 7:03 PM, Suraj Sumangala wrote:
>> This patch provides option for hci transport driver protocol implementation
>> to have a callback for hci open.
>>
>> Signed-off-by: Suraj Sumangala<[email protected]>
>> ---
>> drivers/bluetooth/hci_ldisc.c | 5 ++++-
>> drivers/bluetooth/hci_uart.h | 1 +
>> 2 files changed, 5 insertions(+), 1 deletions(-)
>>
>
> Regards
> Suraj
Is there any comments on this patch?
Regards
Suraj