Re: [PATCH 5/8] Bluetooth: btrsi: add new rsi bluetooth driver

[Amitkumar Karwar]

On Wed, Nov 29, 2017 at 7:03 PM, Marcel Holtmann  wrote:
> Hi Amitkumar,
>
 Redpine bluetooth driver is a thin driver which depends on
 'rsi_91x' driver for transmitting and receiving packets
 to/from device. It creates hci interface when attach() is
 called from 'rsi_91x' module.

 Signed-off-by: Prameela Rani Garnepudi 
 Signed-off-by: Siva Rebbagondla 
 Signed-off-by: Amitkumar Karwar 
 ---
 drivers/bluetooth/Kconfig   |  12 ++
 drivers/bluetooth/Makefile  |   2 +
 drivers/bluetooth/btrsi.c   | 268 
 
 drivers/bluetooth/rsi_hci.h |  51 +
 4 files changed, 333 insertions(+)
 create mode 100644 drivers/bluetooth/btrsi.c
 create mode 100644 drivers/bluetooth/rsi_hci.h

 diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
 index 60e1c7d..ca58d74 100644
 --- a/drivers/bluetooth/Kconfig
 +++ b/drivers/bluetooth/Kconfig
 @@ -378,4 +378,16 @@ config BT_QCOMSMD
Say Y here to compile support for HCI over Qualcomm SMD into the
kernel or say M to compile as a module.

 +config BT_RSI
 + tristate "Redpine HCI support"
 + depends on BT && BT_RFCOMM
>>>
>>> the depends on BT_RFCOMM is wrong. And even the depends on BT is unneeded 
>>> since that is already covered with the above menu clause.
>>>
>>>
 + default m
>>>
>>> No. New drivers are by default off.
>>
>> We made it by default on, as RSI core module depends on this.
>>
>>>
 + help
 +   Redpine BT driver.
 +   This driver handles BT traffic from upper layers and pass
 +   to the RSI_91x coex module for further scheduling to device
 +
 +   Say Y here to compile support for HCI over Qualcomm SMD into the
 +   kernel or say M to compile as a module.
 +
>>>
>>> What I am missing is the depends on the RSI core piece that is needed.
>>
>> Actually RSI core module depends on RSI BT module as per our design.
>> As soon as firmware is downloaded by RSI core module, we get WLAN and
>> BT card ready frames from firmware. BT initialization has to happen at
>> this point. So btrsi module should be loaded first.
>
> that should be solved by the dependencies and not by a default m.

Understood. I will take care of it and submit v3.

>
>>
>>>
 endmenu
 diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
 index 4e4e44d..712af83a 100644
 --- a/drivers/bluetooth/Makefile
 +++ b/drivers/bluetooth/Makefile
 @@ -28,6 +28,8 @@ obj-$(CONFIG_BT_QCA)+= btqca.o

 obj-$(CONFIG_BT_HCIUART_NOKIA)+= hci_nokia.o

 +obj-$(CONFIG_BT_RSI) += btrsi.o
 +
 btmrvl-y  := btmrvl_main.o
 btmrvl-$(CONFIG_DEBUG_FS) += btmrvl_debugfs.o

 diff --git a/drivers/bluetooth/btrsi.c b/drivers/bluetooth/btrsi.c
 new file mode 100644
 index 000..c52f418
 --- /dev/null
 +++ b/drivers/bluetooth/btrsi.c
 @@ -0,0 +1,268 @@
 +/**
 + * Copyright (c) 2017 Redpine Signals Inc.
 + *
 + * Permission to use, copy, modify, and/or distribute this software for 
 any
 + * purpose with or without fee is hereby granted, provided that the above
 + * copyright notice and this permission notice appear in all copies.
 + *
 + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL 
 WARRANTIES
 + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
 + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
 +e* ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
 + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
 + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 + */
 +#include 
 +#include 
 +#include 
 +#include "rsi_hci.h"
 +
 +static struct rsi_mod_ops rsi_bt_ops = {
 + .attach = rsi_hci_attach,
 + .detach = rsi_hci_detach,
 + .recv_pkt = rsi_hci_recv_pkt,
 +};
 +
 +static int rsi_hci_open(struct hci_dev *hdev)
 +{
 + BT_INFO("%s open\n", hdev->name);
>>>
>>> No BT_INFO here. We are not spamming dmesg with pointless information. And 
>>> this applies to all of these.
>>>
 +
 + return 0;
 +}
 +
 +static int rsi_hci_close(struct hci_dev *hdev)
 +{
 + BT_INFO("%s closed\n", hdev->name);
 +
 + return 0;
 +}
 +
 +static int rsi_hci_flush(struct hci_dev *hdev)
 +{
 + BT_ERR("%s flush\n", hdev->name);
 +
 + return 0;
 +}
 +
 +static int rsi_hci_send_pkt(struct hci_dev *hdev, struct sk_buff *skb)
 

Re: [PATCH 5/8] Bluetooth: btrsi: add new rsi bluetooth driver

[Marcel Holtmann]

Hi Amitkumar,

>>> Redpine bluetooth driver is a thin driver which depends on
>>> 'rsi_91x' driver for transmitting and receiving packets
>>> to/from device. It creates hci interface when attach() is
>>> called from 'rsi_91x' module.
>>> 
>>> Signed-off-by: Prameela Rani Garnepudi 
>>> Signed-off-by: Siva Rebbagondla 
>>> Signed-off-by: Amitkumar Karwar 
>>> ---
>>> drivers/bluetooth/Kconfig   |  12 ++
>>> drivers/bluetooth/Makefile  |   2 +
>>> drivers/bluetooth/btrsi.c   | 268 
>>> 
>>> drivers/bluetooth/rsi_hci.h |  51 +
>>> 4 files changed, 333 insertions(+)
>>> create mode 100644 drivers/bluetooth/btrsi.c
>>> create mode 100644 drivers/bluetooth/rsi_hci.h
>>> 
>>> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
>>> index 60e1c7d..ca58d74 100644
>>> --- a/drivers/bluetooth/Kconfig
>>> +++ b/drivers/bluetooth/Kconfig
>>> @@ -378,4 +378,16 @@ config BT_QCOMSMD
>>>Say Y here to compile support for HCI over Qualcomm SMD into the
>>>kernel or say M to compile as a module.
>>> 
>>> +config BT_RSI
>>> + tristate "Redpine HCI support"
>>> + depends on BT && BT_RFCOMM
>> 
>> the depends on BT_RFCOMM is wrong. And even the depends on BT is unneeded 
>> since that is already covered with the above menu clause.
>> 
>> 
>>> + default m
>> 
>> No. New drivers are by default off.
> 
> We made it by default on, as RSI core module depends on this.
> 
>> 
>>> + help
>>> +   Redpine BT driver.
>>> +   This driver handles BT traffic from upper layers and pass
>>> +   to the RSI_91x coex module for further scheduling to device
>>> +
>>> +   Say Y here to compile support for HCI over Qualcomm SMD into the
>>> +   kernel or say M to compile as a module.
>>> +
>> 
>> What I am missing is the depends on the RSI core piece that is needed.
> 
> Actually RSI core module depends on RSI BT module as per our design.
> As soon as firmware is downloaded by RSI core module, we get WLAN and
> BT card ready frames from firmware. BT initialization has to happen at
> this point. So btrsi module should be loaded first.

that should be solved by the dependencies and not by a default m.

> 
>> 
>>> endmenu
>>> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
>>> index 4e4e44d..712af83a 100644
>>> --- a/drivers/bluetooth/Makefile
>>> +++ b/drivers/bluetooth/Makefile
>>> @@ -28,6 +28,8 @@ obj-$(CONFIG_BT_QCA)+= btqca.o
>>> 
>>> obj-$(CONFIG_BT_HCIUART_NOKIA)+= hci_nokia.o
>>> 
>>> +obj-$(CONFIG_BT_RSI) += btrsi.o
>>> +
>>> btmrvl-y  := btmrvl_main.o
>>> btmrvl-$(CONFIG_DEBUG_FS) += btmrvl_debugfs.o
>>> 
>>> diff --git a/drivers/bluetooth/btrsi.c b/drivers/bluetooth/btrsi.c
>>> new file mode 100644
>>> index 000..c52f418
>>> --- /dev/null
>>> +++ b/drivers/bluetooth/btrsi.c
>>> @@ -0,0 +1,268 @@
>>> +/**
>>> + * Copyright (c) 2017 Redpine Signals Inc.
>>> + *
>>> + * Permission to use, copy, modify, and/or distribute this software for any
>>> + * purpose with or without fee is hereby granted, provided that the above
>>> + * copyright notice and this permission notice appear in all copies.
>>> + *
>>> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
>>> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
>>> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
>>> +e* ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
>>> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
>>> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
>>> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>>> + */
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include "rsi_hci.h"
>>> +
>>> +static struct rsi_mod_ops rsi_bt_ops = {
>>> + .attach = rsi_hci_attach,
>>> + .detach = rsi_hci_detach,
>>> + .recv_pkt = rsi_hci_recv_pkt,
>>> +};
>>> +
>>> +static int rsi_hci_open(struct hci_dev *hdev)
>>> +{
>>> + BT_INFO("%s open\n", hdev->name);
>> 
>> No BT_INFO here. We are not spamming dmesg with pointless information. And 
>> this applies to all of these.
>> 
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int rsi_hci_close(struct hci_dev *hdev)
>>> +{
>>> + BT_INFO("%s closed\n", hdev->name);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int rsi_hci_flush(struct hci_dev *hdev)
>>> +{
>>> + BT_ERR("%s flush\n", hdev->name);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int rsi_hci_send_pkt(struct hci_dev *hdev, struct sk_buff *skb)
>>> +{
>>> + struct rsi_hci_adapter *h_adapter;
>> 
>>= hci_get_drvdata(dev);
>> 
>> There is no need to do that separate.
>> 
>>> + struct sk_buff *new_skb = NULL;
>>> +
>>> + int status = 0;
>>> +
>>> + 

Re: [PATCH 5/8] Bluetooth: btrsi: add new rsi bluetooth driver

[Amitkumar Karwar]

On Wed, Nov 15, 2017 at 9:07 PM, Marcel Holtmann  wrote:
> Hi Amitkumar,
>
>> Redpine bluetooth driver is a thin driver which depends on
>> 'rsi_91x' driver for transmitting and receiving packets
>> to/from device. It creates hci interface when attach() is
>> called from 'rsi_91x' module.
>>
>> Signed-off-by: Prameela Rani Garnepudi 
>> Signed-off-by: Siva Rebbagondla 
>> Signed-off-by: Amitkumar Karwar 
>> ---
>> drivers/bluetooth/Kconfig   |  12 ++
>> drivers/bluetooth/Makefile  |   2 +
>> drivers/bluetooth/btrsi.c   | 268 
>> 
>> drivers/bluetooth/rsi_hci.h |  51 +
>> 4 files changed, 333 insertions(+)
>> create mode 100644 drivers/bluetooth/btrsi.c
>> create mode 100644 drivers/bluetooth/rsi_hci.h
>>
>> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
>> index 60e1c7d..ca58d74 100644
>> --- a/drivers/bluetooth/Kconfig
>> +++ b/drivers/bluetooth/Kconfig
>> @@ -378,4 +378,16 @@ config BT_QCOMSMD
>> Say Y here to compile support for HCI over Qualcomm SMD into the
>> kernel or say M to compile as a module.
>>
>> +config BT_RSI
>> + tristate "Redpine HCI support"
>> + depends on BT && BT_RFCOMM
>
> the depends on BT_RFCOMM is wrong. And even the depends on BT is unneeded 
> since that is already covered with the above menu clause.
>
>
>> + default m
>
> No. New drivers are by default off.

We made it by default on, as RSI core module depends on this.

>
>> + help
>> +   Redpine BT driver.
>> +   This driver handles BT traffic from upper layers and pass
>> +   to the RSI_91x coex module for further scheduling to device
>> +
>> +   Say Y here to compile support for HCI over Qualcomm SMD into the
>> +   kernel or say M to compile as a module.
>> +
>
> What I am missing is the depends on the RSI core piece that is needed.

Actually RSI core module depends on RSI BT module as per our design.
As soon as firmware is downloaded by RSI core module, we get WLAN and
BT card ready frames from firmware. BT initialization has to happen at
this point. So btrsi module should be loaded first.

>
>> endmenu
>> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
>> index 4e4e44d..712af83a 100644
>> --- a/drivers/bluetooth/Makefile
>> +++ b/drivers/bluetooth/Makefile
>> @@ -28,6 +28,8 @@ obj-$(CONFIG_BT_QCA)+= btqca.o
>>
>> obj-$(CONFIG_BT_HCIUART_NOKIA)+= hci_nokia.o
>>
>> +obj-$(CONFIG_BT_RSI) += btrsi.o
>> +
>> btmrvl-y  := btmrvl_main.o
>> btmrvl-$(CONFIG_DEBUG_FS) += btmrvl_debugfs.o
>>
>> diff --git a/drivers/bluetooth/btrsi.c b/drivers/bluetooth/btrsi.c
>> new file mode 100644
>> index 000..c52f418
>> --- /dev/null
>> +++ b/drivers/bluetooth/btrsi.c
>> @@ -0,0 +1,268 @@
>> +/**
>> + * Copyright (c) 2017 Redpine Signals Inc.
>> + *
>> + * Permission to use, copy, modify, and/or distribute this software for any
>> + * purpose with or without fee is hereby granted, provided that the above
>> + * copyright notice and this permission notice appear in all copies.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
>> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
>> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
>> +e* ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
>> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
>> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
>> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>> + */
>> +#include 
>> +#include 
>> +#include 
>> +#include "rsi_hci.h"
>> +
>> +static struct rsi_mod_ops rsi_bt_ops = {
>> + .attach = rsi_hci_attach,
>> + .detach = rsi_hci_detach,
>> + .recv_pkt = rsi_hci_recv_pkt,
>> +};
>> +
>> +static int rsi_hci_open(struct hci_dev *hdev)
>> +{
>> + BT_INFO("%s open\n", hdev->name);
>
> No BT_INFO here. We are not spamming dmesg with pointless information. And 
> this applies to all of these.
>
>> +
>> + return 0;
>> +}
>> +
>> +static int rsi_hci_close(struct hci_dev *hdev)
>> +{
>> + BT_INFO("%s closed\n", hdev->name);
>> +
>> + return 0;
>> +}
>> +
>> +static int rsi_hci_flush(struct hci_dev *hdev)
>> +{
>> + BT_ERR("%s flush\n", hdev->name);
>> +
>> + return 0;
>> +}
>> +
>> +static int rsi_hci_send_pkt(struct hci_dev *hdev, struct sk_buff *skb)
>> +{
>> + struct rsi_hci_adapter *h_adapter;
>
> = hci_get_drvdata(dev);
>
> There is no need to do that separate.
>
>> + struct sk_buff *new_skb = NULL;
>> +
>> + int status = 0;
>> +
>> + h_adapter = hci_get_drvdata(hdev);
>> + if (!h_adapter) {
>> + status = -EFAULT;
>> + goto fail;
>> + }
>
> And this error 

Re: [PATCH 5/8] Bluetooth: btrsi: add new rsi bluetooth driver

[Amitkumar Karwar]

On Wed, Nov 15, 2017 at 9:07 PM, Marcel Holtmann  wrote:
> Hi Amitkumar,
>
>> Redpine bluetooth driver is a thin driver which depends on
>> 'rsi_91x' driver for transmitting and receiving packets
>> to/from device. It creates hci interface when attach() is
>> called from 'rsi_91x' module.
>>
>> Signed-off-by: Prameela Rani Garnepudi 
>> Signed-off-by: Siva Rebbagondla 
>> Signed-off-by: Amitkumar Karwar 
>> ---
>> drivers/bluetooth/Kconfig   |  12 ++
>> drivers/bluetooth/Makefile  |   2 +
>> drivers/bluetooth/btrsi.c   | 268 
>> 
>> drivers/bluetooth/rsi_hci.h |  51 +
>> 4 files changed, 333 insertions(+)
>> create mode 100644 drivers/bluetooth/btrsi.c
>> create mode 100644 drivers/bluetooth/rsi_hci.h
>>
>> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
>> index 60e1c7d..ca58d74 100644
>> --- a/drivers/bluetooth/Kconfig
>> +++ b/drivers/bluetooth/Kconfig
>> @@ -378,4 +378,16 @@ config BT_QCOMSMD
>> Say Y here to compile support for HCI over Qualcomm SMD into the
>> kernel or say M to compile as a module.
>>
>> +config BT_RSI
>> + tristate "Redpine HCI support"
>> + depends on BT && BT_RFCOMM
>
> the depends on BT_RFCOMM is wrong. And even the depends on BT is unneeded 
> since that is already covered with the above menu clause.
>
>
>> + default m
>
> No. New drivers are by default off.
>
>> + help
>> +   Redpine BT driver.
>> +   This driver handles BT traffic from upper layers and pass
>> +   to the RSI_91x coex module for further scheduling to device
>> +
>> +   Say Y here to compile support for HCI over Qualcomm SMD into the
>> +   kernel or say M to compile as a module.
>> +
>
> What I am missing is the depends on the RSI core piece that is needed.
>
>> endmenu
>> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
>> index 4e4e44d..712af83a 100644
>> --- a/drivers/bluetooth/Makefile
>> +++ b/drivers/bluetooth/Makefile
>> @@ -28,6 +28,8 @@ obj-$(CONFIG_BT_QCA)+= btqca.o
>>
>> obj-$(CONFIG_BT_HCIUART_NOKIA)+= hci_nokia.o
>>
>> +obj-$(CONFIG_BT_RSI) += btrsi.o
>> +
>> btmrvl-y  := btmrvl_main.o
>> btmrvl-$(CONFIG_DEBUG_FS) += btmrvl_debugfs.o
>>
>> diff --git a/drivers/bluetooth/btrsi.c b/drivers/bluetooth/btrsi.c
>> new file mode 100644
>> index 000..c52f418
>> --- /dev/null
>> +++ b/drivers/bluetooth/btrsi.c
>> @@ -0,0 +1,268 @@
>> +/**
>> + * Copyright (c) 2017 Redpine Signals Inc.
>> + *
>> + * Permission to use, copy, modify, and/or distribute this software for any
>> + * purpose with or without fee is hereby granted, provided that the above
>> + * copyright notice and this permission notice appear in all copies.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
>> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
>> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
>> +e* ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
>> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
>> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
>> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>> + */
>> +#include 
>> +#include 
>> +#include 
>> +#include "rsi_hci.h"
>> +
>> +static struct rsi_mod_ops rsi_bt_ops = {
>> + .attach = rsi_hci_attach,
>> + .detach = rsi_hci_detach,
>> + .recv_pkt = rsi_hci_recv_pkt,
>> +};
>> +
>> +static int rsi_hci_open(struct hci_dev *hdev)
>> +{
>> + BT_INFO("%s open\n", hdev->name);
>
> No BT_INFO here. We are not spamming dmesg with pointless information. And 
> this applies to all of these.
>
>> +
>> + return 0;
>> +}
>> +
>> +static int rsi_hci_close(struct hci_dev *hdev)
>> +{
>> + BT_INFO("%s closed\n", hdev->name);
>> +
>> + return 0;
>> +}
>> +
>> +static int rsi_hci_flush(struct hci_dev *hdev)
>> +{
>> + BT_ERR("%s flush\n", hdev->name);
>> +
>> + return 0;
>> +}
>> +
>> +static int rsi_hci_send_pkt(struct hci_dev *hdev, struct sk_buff *skb)
>> +{
>> + struct rsi_hci_adapter *h_adapter;
>
> = hci_get_drvdata(dev);
>
> There is no need to do that separate.
>
>> + struct sk_buff *new_skb = NULL;
>> +
>> + int status = 0;
>> +
>> + h_adapter = hci_get_drvdata(hdev);
>> + if (!h_adapter) {
>> + status = -EFAULT;
>> + goto fail;
>> + }
>
> And this error check seems unneeded since the Bluetooth core will never call 
> ->send unless it is all correctly set up.
>
>> +
>> + if (h_adapter->fsm_state != RSI_BT_FSM_DEVICE_READY) {
>> + BT_INFO("BT Device not ready\n");
>> + status = -ENODEV;
>> + goto fail;
>> + }
>
> Why are we registering 

Re: [PATCH 5/8] Bluetooth: btrsi: add new rsi bluetooth driver

[Marcel Holtmann]

Hi Amitkumar,

> Redpine bluetooth driver is a thin driver which depends on
> 'rsi_91x' driver for transmitting and receiving packets
> to/from device. It creates hci interface when attach() is
> called from 'rsi_91x' module.
> 
> Signed-off-by: Prameela Rani Garnepudi 
> Signed-off-by: Siva Rebbagondla 
> Signed-off-by: Amitkumar Karwar 
> ---
> drivers/bluetooth/Kconfig   |  12 ++
> drivers/bluetooth/Makefile  |   2 +
> drivers/bluetooth/btrsi.c   | 268 
> drivers/bluetooth/rsi_hci.h |  51 +
> 4 files changed, 333 insertions(+)
> create mode 100644 drivers/bluetooth/btrsi.c
> create mode 100644 drivers/bluetooth/rsi_hci.h
> 
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 60e1c7d..ca58d74 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -378,4 +378,16 @@ config BT_QCOMSMD
> Say Y here to compile support for HCI over Qualcomm SMD into the
> kernel or say M to compile as a module.
> 
> +config BT_RSI
> + tristate "Redpine HCI support"
> + depends on BT && BT_RFCOMM

the depends on BT_RFCOMM is wrong. And even the depends on BT is unneeded since 
that is already covered with the above menu clause.


> + default m

No. New drivers are by default off.

> + help
> +   Redpine BT driver.
> +   This driver handles BT traffic from upper layers and pass
> +   to the RSI_91x coex module for further scheduling to device
> +
> +   Say Y here to compile support for HCI over Qualcomm SMD into the
> +   kernel or say M to compile as a module.
> +

What I am missing is the depends on the RSI core piece that is needed.

> endmenu
> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> index 4e4e44d..712af83a 100644
> --- a/drivers/bluetooth/Makefile
> +++ b/drivers/bluetooth/Makefile
> @@ -28,6 +28,8 @@ obj-$(CONFIG_BT_QCA)+= btqca.o
> 
> obj-$(CONFIG_BT_HCIUART_NOKIA)+= hci_nokia.o
> 
> +obj-$(CONFIG_BT_RSI) += btrsi.o
> +
> btmrvl-y  := btmrvl_main.o
> btmrvl-$(CONFIG_DEBUG_FS) += btmrvl_debugfs.o
> 
> diff --git a/drivers/bluetooth/btrsi.c b/drivers/bluetooth/btrsi.c
> new file mode 100644
> index 000..c52f418
> --- /dev/null
> +++ b/drivers/bluetooth/btrsi.c
> @@ -0,0 +1,268 @@
> +/**
> + * Copyright (c) 2017 Redpine Signals Inc.
> + *
> + * Permission to use, copy, modify, and/or distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> +e* ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +#include 
> +#include 
> +#include 
> +#include "rsi_hci.h"
> +
> +static struct rsi_mod_ops rsi_bt_ops = {
> + .attach = rsi_hci_attach,
> + .detach = rsi_hci_detach,
> + .recv_pkt = rsi_hci_recv_pkt,
> +};
> +
> +static int rsi_hci_open(struct hci_dev *hdev)
> +{
> + BT_INFO("%s open\n", hdev->name);

No BT_INFO here. We are not spamming dmesg with pointless information. And this 
applies to all of these.

> +
> + return 0;
> +}
> +
> +static int rsi_hci_close(struct hci_dev *hdev)
> +{
> + BT_INFO("%s closed\n", hdev->name);
> +
> + return 0;
> +}
> +
> +static int rsi_hci_flush(struct hci_dev *hdev)
> +{
> + BT_ERR("%s flush\n", hdev->name);
> +
> + return 0;
> +}
> +
> +static int rsi_hci_send_pkt(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> + struct rsi_hci_adapter *h_adapter;

= hci_get_drvdata(dev);

There is no need to do that separate.

> + struct sk_buff *new_skb = NULL;
> +
> + int status = 0;
> +
> + h_adapter = hci_get_drvdata(hdev);
> + if (!h_adapter) {
> + status = -EFAULT;
> + goto fail;
> + }

And this error check seems unneeded since the Bluetooth core will never call 
->send unless it is all correctly set up.

> +
> + if (h_adapter->fsm_state != RSI_BT_FSM_DEVICE_READY) {
> + BT_INFO("BT Device not ready\n");
> + status = -ENODEV;
> + goto fail;
> + }

Why are we registering a HCI device that might not be ready. This will be a 
massive bug in the whole system. Also one that is not recoverable. If something 
goes wrong, then lets unregister the HCI device.

> +
> + if (!test_bit(HCI_RUNNING, >flags)) {
> + 

[PATCH 5/8] Bluetooth: btrsi: add new rsi bluetooth driver

[Amitkumar Karwar]

From: Prameela Rani Garnepudi 

Redpine bluetooth driver is a thin driver which depends on
'rsi_91x' driver for transmitting and receiving packets
to/from device. It creates hci interface when attach() is
called from 'rsi_91x' module.

Signed-off-by: Prameela Rani Garnepudi 
Signed-off-by: Siva Rebbagondla 
Signed-off-by: Amitkumar Karwar 
---
 drivers/bluetooth/Kconfig   |  12 ++
 drivers/bluetooth/Makefile  |   2 +
 drivers/bluetooth/btrsi.c   | 268 
 drivers/bluetooth/rsi_hci.h |  51 +
 4 files changed, 333 insertions(+)
 create mode 100644 drivers/bluetooth/btrsi.c
 create mode 100644 drivers/bluetooth/rsi_hci.h

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index 60e1c7d..ca58d74 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -378,4 +378,16 @@ config BT_QCOMSMD
  Say Y here to compile support for HCI over Qualcomm SMD into the
  kernel or say M to compile as a module.
 
+config BT_RSI
+   tristate "Redpine HCI support"
+   depends on BT && BT_RFCOMM
+   default m
+   help
+ Redpine BT driver.
+ This driver handles BT traffic from upper layers and pass
+ to the RSI_91x coex module for further scheduling to device
+
+ Say Y here to compile support for HCI over Qualcomm SMD into the
+ kernel or say M to compile as a module.
+
 endmenu
diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
index 4e4e44d..712af83a 100644
--- a/drivers/bluetooth/Makefile
+++ b/drivers/bluetooth/Makefile
@@ -28,6 +28,8 @@ obj-$(CONFIG_BT_QCA)  += btqca.o
 
 obj-$(CONFIG_BT_HCIUART_NOKIA) += hci_nokia.o
 
+obj-$(CONFIG_BT_RSI)   += btrsi.o
+
 btmrvl-y   := btmrvl_main.o
 btmrvl-$(CONFIG_DEBUG_FS)  += btmrvl_debugfs.o
 
diff --git a/drivers/bluetooth/btrsi.c b/drivers/bluetooth/btrsi.c
new file mode 100644
index 000..c52f418
--- /dev/null
+++ b/drivers/bluetooth/btrsi.c
@@ -0,0 +1,268 @@
+/**
+ * Copyright (c) 2017 Redpine Signals Inc.
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+e* ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+#include 
+#include 
+#include 
+#include "rsi_hci.h"
+
+static struct rsi_mod_ops rsi_bt_ops = {
+   .attach = rsi_hci_attach,
+   .detach = rsi_hci_detach,
+   .recv_pkt = rsi_hci_recv_pkt,
+};
+
+static int rsi_hci_open(struct hci_dev *hdev)
+{
+   BT_INFO("%s open\n", hdev->name);
+
+   return 0;
+}
+
+static int rsi_hci_close(struct hci_dev *hdev)
+{
+   BT_INFO("%s closed\n", hdev->name);
+
+   return 0;
+}
+
+static int rsi_hci_flush(struct hci_dev *hdev)
+{
+   BT_ERR("%s flush\n", hdev->name);
+
+   return 0;
+}
+
+static int rsi_hci_send_pkt(struct hci_dev *hdev, struct sk_buff *skb)
+{
+   struct rsi_hci_adapter *h_adapter;
+   struct sk_buff *new_skb = NULL;
+
+   int status = 0;
+
+   h_adapter = hci_get_drvdata(hdev);
+   if (!h_adapter) {
+   status = -EFAULT;
+   goto fail;
+   }
+
+   if (h_adapter->fsm_state != RSI_BT_FSM_DEVICE_READY) {
+   BT_INFO("BT Device not ready\n");
+   status = -ENODEV;
+   goto fail;
+   }
+
+   if (!test_bit(HCI_RUNNING, >flags)) {
+   status = -EBUSY;
+   goto fail;
+   }
+
+   switch (bt_cb(skb)->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;
+
+   default:
+   dev_kfree_skb(skb);
+   status = -EILSEQ;
+   goto fail;
+   }
+
+   if (skb_headroom(skb) < RSI_HEADROOM_FOR_BT_HAL) {
+   /* Re-allocate one more skb with sufficent headroom */
+   /* Space for Descriptor (16 bytes) is required in head room */
+   u16 new_len = skb->len + RSI_HEADROOM_FOR_BT_HAL;
+
+   new_skb = dev_alloc_skb(new_len);
+   if (!new_skb) {
+   dev_kfree_skb(skb);
+