Hi Kalle,
On Thu, Dec 14, 2017 at 7:52 PM, Amitkumar Karwar <amitkarwar@xxxxxxxxx> wrote:
> On Wed, Dec 13, 2017 at 7:16 PM, Marcel Holtmann <marcel@xxxxxxxxxxxx> 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 <prameela.j04cs@xxxxxxxxx>
>>> Signed-off-by: Siva Rebbagondla <siva.rebbagondla@xxxxxxxxxxxxxxxxxx>
>>> Signed-off-by: Amitkumar Karwar <amit.karwar@xxxxxxxxxxxxxxxxxx>
>>> ---
>>> v5: Addressed review comments from Marcel.
>>> Removed reduntant switch case code from rsi_hci_recv_pkt()
>>> Changed bt_cb(skb)->pkt_type to hci_skb_pkt_type(skb)
>>> Removed reduntant '\n' from BT_ERR and redundant BT_INFO messages
>>> Changed u8 *pkt to const u8 *pkt in rsi_hci_recv_pkt()
>>> v4: Removed rsi_hci.h file. Made the functions static(Marcel)
>>> v3: Made BT_RSI module by default off(Marcel)
>>> Removed redundant exported function rsi_get_hci_ops()(Marcel)
>>> v2: Addressed review comments from Marcel
>>> Removed unnecessary 'depends on BT && BT_RFOMM' line in Kconfig
>>> Removed redundant BT_INFO messages
>>> h_adapter initialization and declaration in a single line.
>>> Removed unnecessary error checks for HCI_RUNNING and fsm_state
>>> Allocated new skb with skb_realloc_headroom() API if headroom is not sufficient
>>> Used get_unaligned_le16 helpers
>>> Moved a structure and union from header file to btrsi.c file
>>> ---
>>> drivers/bluetooth/Kconfig | 11 +++
>>> drivers/bluetooth/Makefile | 2 +
>>> drivers/bluetooth/btrsi.c | 188 +++++++++++++++++++++++++++++++++++++++++++++
>>> include/linux/rsi_header.h | 4 +-
>>> 4 files changed, 204 insertions(+), 1 deletion(-)
>>> create mode 100644 drivers/bluetooth/btrsi.c
>>>
>>> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
>>> index 60e1c7d..33d7514 100644
>>> --- a/drivers/bluetooth/Kconfig
>>> +++ b/drivers/bluetooth/Kconfig
>>> @@ -378,4 +378,15 @@ 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"
>>> + default n
>>> + 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 Redpine 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
>>> +
>>
>> actually I never caught this before. Since this driver is doing the HCI portion, for consistency CONFIG_BT_HCIRSI would be better.
>>
>>> 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 0000000..c9e92cc
>>> --- /dev/null
>>> +++ b/drivers/bluetooth/btrsi.c
>>> @@ -0,0 +1,188 @@
>>> +/**
>>> + * 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
>>> + * 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 <linux/version.h>
>>> +#include <linux/module.h>
>>> +#include <linux/kernel.h>
>>> +#include <net/bluetooth/bluetooth.h>
>>> +#include <net/bluetooth/hci_core.h>
>>> +#include <linux/unaligned/le_byteshift.h>
>>> +#include <linux/rsi_header.h>
>>> +#include <net/genetlink.h>
>>> +
>>> +#define RSI_HEADROOM_FOR_BT_HAL 16
>>> +#define RSI_FRAME_DESC_SIZE 16
>>> +
>>> +static struct rsi_hci_adapter {
>>> + void *priv;
>>> + struct rsi_proto_ops *proto_ops;
>>> + struct hci_dev *hdev;
>>> +};
>>> +
>>> +static int rsi_hci_open(struct hci_dev *hdev)
>>> +{
>>> + return 0;
>>> +}
>>> +
>>> +static int rsi_hci_close(struct hci_dev *hdev)
>>> +{
>>> + return 0;
>>> +}
>>> +
>>> +static int rsi_hci_flush(struct hci_dev *hdev)
>>> +{
>>> + 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(hdev);
>>> + struct sk_buff *new_skb = NULL;
>>> +
>>> + switch (hci_skb_pkt_type(skb)) {
>>> + 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;
>>> + }
>>> +
>>> + if (skb_headroom(skb) < RSI_HEADROOM_FOR_BT_HAL) {
>>> + /* Insufficient skb headroom - allocate a new skb */
>>> + new_skb = skb_realloc_headroom(skb, RSI_HEADROOM_FOR_BT_HAL);
>>> + if (unlikely(!new_skb))
>>> + return -ENOMEM;
>>> + bt_cb(new_skb)->pkt_type = hci_skb_pkt_type(skb);
>>> + kfree_skb(skb);
>>> + skb = new_skb;
>>> + }
>>> +
>>> + return h_adapter->proto_ops->coex_send_pkt(h_adapter->priv, skb,
>>> + RSI_BT_Q);
>>> +}
>>> +
>>> +static int rsi_hci_recv_pkt(void *priv, const u8 *pkt)
>>> +{
>>> + struct rsi_hci_adapter *h_adapter = priv;
>>> + struct hci_dev *hdev = h_adapter->hdev;
>>> + struct sk_buff *skb;
>>> + int pkt_len = get_unaligned_le16(pkt) & 0x0fff;
>>> +
>>> + skb = dev_alloc_skb(pkt_len);
>>> + if (!skb)
>>> + return -ENOMEM;
>>> +
>>> + memcpy(skb->data, pkt + RSI_FRAME_DESC_SIZE, pkt_len);
>>> + skb_put(skb, pkt_len);
>>> + h_adapter->hdev->stat.byte_rx += skb->len;
>>> +
>>> + hci_skb_pkt_type(skb) = pkt[14];
>>> +
>>> + return hci_recv_frame(hdev, skb);
>>> +}
>>> +
>>> +static int rsi_hci_attach(void *priv, struct rsi_proto_ops *ops)
>>> +{
>>> + struct rsi_hci_adapter *h_adapter = NULL;
>>> + struct hci_dev *hdev;
>>> + int err = 0;
>>> +
>>> + h_adapter = kzalloc(sizeof(*h_adapter), GFP_KERNEL);
>>> + if (!h_adapter)
>>> + return -ENOMEM;
>>> +
>>> + h_adapter->priv = priv;
>>> + ops->set_bt_context(priv, h_adapter);
>>> + h_adapter->proto_ops = ops;
>>> +
>>> + hdev = hci_alloc_dev();
>>> + if (!hdev) {
>>> + BT_ERR("Failed to alloc HCI device");
>>> + goto err;
>>> + }
>>> +
>>> + h_adapter->hdev = hdev;
>>> +
>>> + if (ops->get_host_intf(priv) == RSI_HOST_INTF_SDIO)
>>> + hdev->bus = HCI_SDIO;
>>> + else
>>> + hdev->bus = HCI_USB;
>>> +
>>> + hci_set_drvdata(hdev, h_adapter);
>>> + hdev->dev_type = HCI_PRIMARY;
>>> + hdev->open = rsi_hci_open;
>>> + hdev->close = rsi_hci_close;
>>> + hdev->flush = rsi_hci_flush;
>>> + hdev->send = rsi_hci_send_pkt;
>>
>> this doesn’t have to be in this patch, but if you support hdev->set_bdaddr, hdev->set_diag etc. that would be good to add in a follow up.
>>
>>> +
>>> + err = hci_register_dev(hdev);
>>> + if (err < 0) {
>>> + BT_ERR("HCI registration failed with errcode %d", err);
>>> + hci_free_dev(hdev);
>>> + goto err;
>>> + }
>>> +
>>> + return 0;
>>> +err:
>>> + h_adapter->hdev = NULL;
>>> + kfree(h_adapter);
>>> + return -EINVAL;
>>> +}
>>> +
>>> +static void rsi_hci_detach(void *priv)
>>> +{
>>> + struct rsi_hci_adapter *h_adapter = priv;
>>> + struct hci_dev *hdev;
>>> +
>>> + if (!h_adapter)
>>> + return;
>>> +
>>> + hdev = h_adapter->hdev;
>>> + if (hdev) {
>>> + hci_unregister_dev(hdev);
>>> + hci_free_dev(hdev);
>>> + h_adapter->hdev = NULL;
>>> + }
>>> +
>>> + kfree(h_adapter);
>>> +}
>>> +
>>> +const struct rsi_mod_ops rsi_bt_ops = {
>>> + .attach = rsi_hci_attach,
>>> + .detach = rsi_hci_detach,
>>> + .recv_pkt = rsi_hci_recv_pkt,
>>> +};
>>> +EXPORT_SYMBOL(rsi_bt_ops);
>>> +
>>> +static int rsi_91x_bt_module_init(void)
>>> +{
>>> + return 0;
>>> +}
>>> +
>>> +static void rsi_91x_bt_module_exit(void)
>>> +{
>>> + return;
>>> +}
>>> +
>>> +module_init(rsi_91x_bt_module_init);
>>> +module_exit(rsi_91x_bt_module_exit);
>>> +MODULE_AUTHOR("Redpine Signals Inc");
>>> +MODULE_DESCRIPTION("RSI BT driver");
>>> +MODULE_SUPPORTED_DEVICE("RSI-BT");
>>> +MODULE_LICENSE("Dual BSD/GPL”);
>>
>> Acked-by: Marcel Holtmann <marcel@xxxxxxxxxxxx>
>> Reviewed-by: Marcel Holtmann <marcel@xxxxxxxxxxxx>
>>
>
> Thanks Marcel.
> I will wait for Kalle's review and suggestions.
> I will address your minor comments while preparing updated version
> based on Kalle's feedback.
>
This patch series has been pending for a while.
Please review and provide your feedback/comments.
Regards,
Amitkumar
