* Re: [PATCH] bluetooth: fix unaligned access to l2cap conf data
From: Gustavo F. Padovan @ 2010-10-18 16:32 UTC (permalink / raw)
To: Harvey Harrison
Cc: Mike Frysinger, linux-bluetooth, Marcel Holtmann,
uclinux-dist-devel, linux-kernel, Andrew Morton, steven miao
In-Reply-To: <AANLkTinMSbhTkH0zQn_YO7hFF7zkuv-Gj-B0BUFWy5nP@mail.gmail.com>
Hi Harvey,
* Harvey Harrison <harvey.harrison@gmail.com> [2010-10-18 11:17:28 -0700]:
> On Sat, Oct 16, 2010 at 3:29 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> > From: steven miao <realmz6@gmail.com>
> >
> >
> > =A0 =A0 =A0 =A0case 2:
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 *((__le16 *) opt->val) =3D cpu_to_le16(va=
l);
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 put_unaligned_le16(cpu_to_le16(val), opt-=
>val);
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
> >
>=20
> I think you wanted:
> put_unaligned_le16(val, opt->val);
I fixed that in the tree. Thanks for the report.=20
--=20
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Setting Inquiry Transmit Power Level
From: Roel Huybrechts @ 2010-10-18 16:54 UTC (permalink / raw)
To: linux-bluetooth
[-- Attachment #1: Type: text/plain, Size: 1727 bytes --]
Dear list,
I'm facing the same problem that Natale Vinto mentioned nearly a
year ago [1] on this list. I'm trying to set the Inquiry Transmit
Power Level of a Belkin USB Bluetooth adapter, unfortunately
without much of success.
It is a Bluetooth 2.1 capable device as mentioned by hciconfig:
# hciconfig hci0 version
hci0: Type: BR/EDR Bus: USB
BD Address: 00:19:0E:08:62:EC ACL MTU: 1021:8 SCO MTU: 64:1
HCI Version: 2.1 (0x4) Revision: 0x5184
LMP Version: 2.1 (0x4) Subversion: 0x420e
Manufacturer: Broadcom Corporation (15)
# hciconfig hci0 revision
hci0: Type: BR/EDR Bus: USB
BD Address: 00:19:0E:08:62:EC ACL MTU: 1021:8 SCO MTU: 64:1
Firmware 132.66 / 14
Writing the inqtpl is supported, as shown by 'hciconfig hci0
commands'.
However:
# hciconfig hci0 inqtpl
hci0: Type: BR/EDR Bus: USB
BD Address: 00:19:0E:08:62:EC ACL MTU: 1021:8 SCO MTU: 64:1
Inquiry transmit power level: 0
# hciconfig hci0 inqtpl 4
# hciconfig hci0 inqtpl
hci0: Type: BR/EDR Bus: USB
BD Address: 00:19:0E:08:62:EC ACL MTU: 1021:8 SCO
MTU: 64:1
Inquiry transmit power level: 0
All this happens with Bluez 4.70 (the one from Debian unstable)
on a (pretty much plain upstream) 2.6.32.22 kernel.
I'd like to hear from you if this is a hardware or firmware issue
of the adapter/chip (in which case I suspect it can't be fixed
and I need to find another adapter which isn't broken), or
whether it can be a Bluez issue.
[1] http://www.spinics.net/lists/linux-bluetooth/msg03785.html
Thanks very much for your time.
Have a nice day,
Roel Huybrechts
--
GPG | 4096R/5E2DE374
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* RE: [PATCH] Bluetooth: btwilink driver
From: Savoy, Pavan @ 2010-10-18 17:32 UTC (permalink / raw)
To: Savoy, Pavan, padovan@profusion.mobi, marcel@holtmann.org
Cc: linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1287176299-19313-1-git-send-email-pavan_savoy@ti.com>
> -----Original Message-----
> From: Savoy, Pavan
> Sent: Friday, October 15, 2010 3:58 PM
> To: padovan@profusion.mobi; marcel@holtmann.org
> Cc: linux-bluetooth@vger.kernel.org; linux-kernel@vger.kernel.org; Savoy,
> Pavan
> Subject: [PATCH] Bluetooth: btwilink driver
>
> From: Pavan Savoy <pavan_savoy@ti.com>
>
> Gustavo, Marcel,
Hope you had a nice weekend :)
Any comments on this patch?
As you might notice, I no longer register to HCI from module_init and have =
it
as a platform device driver and register to HCI inside the probe.
> Renaming the patch, since the driver is renamed to btwilink.
> Thanks for the comments,
> Please review and provide your comments on this version of patch,
>
> -- patch description --
>
> This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> Texas Instrument's WiLink chipsets combine wireless technologies
> like BT, FM, GPS and WLAN onto a single chip.
>
> This Bluetooth driver works on top of the TI_ST shared transport
> line discipline driver which also allows other drivers like
> FM V4L2 and GPS character driver to make use of the same UART interface.
>
> Kconfig and Makefile modifications to enable the Bluetooth
> driver for Texas Instrument's WiLink 7 chipset.
>
> Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> ---
> drivers/bluetooth/Kconfig | 10 +
> drivers/bluetooth/Makefile | 1 +
> drivers/bluetooth/btwilink.c | 424
> ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 435 insertions(+), 0 deletions(-)
> create mode 100644 drivers/bluetooth/btwilink.c
>
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 02deef4..e0d67dd 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -219,4 +219,14 @@ config BT_ATH3K
> Say Y here to compile support for "Atheros firmware download driv=
er"
> into the kernel or say M to compile it as module (ath3k).
>
> +config BT_WILINK
> + tristate "BlueZ bluetooth driver for TI ST"
> + depends on TI_ST
> + help
> + This enables the Bluetooth driver for Texas Instrument's BT/FM/GP=
S
> + 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 driv=
er
> + into the kernel or say M to compile it as module.
> endmenu
> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> index 71bdf13..f4460f4 100644
> --- a/drivers/bluetooth/Makefile
> +++ b/drivers/bluetooth/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_BT_HCIBTSDIO) +=3D btsdio.o
> obj-$(CONFIG_BT_ATH3K) +=3D ath3k.o
> obj-$(CONFIG_BT_MRVL) +=3D btmrvl.o
> obj-$(CONFIG_BT_MRVL_SDIO) +=3D btmrvl_sdio.o
> +obj-$(CONFIG_BT_WILINK) +=3D btwilink.o
>
> btmrvl-y :=3D btmrvl_main.o
> btmrvl-$(CONFIG_DEBUG_FS) +=3D btmrvl_debugfs.o
> diff --git a/drivers/bluetooth/btwilink.c b/drivers/bluetooth/btwilink.c
> new file mode 100644
> index 0000000..f67791f
> --- /dev/null
> +++ b/drivers/bluetooth/btwilink.c
> @@ -0,0 +1,424 @@
> +/*
> + * 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 <raja_mani@ti.com>
> + * Pavan Savoy <pavan_savoy@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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307=
USA
> + *
> + */
> +
> +#include <linux/platform_device.h>
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +
> +#include <linux/ti_wilink_st.h>
> +
> +/* Bluetooth Driver Version */
> +#define VERSION "1.0"
> +
> +/* Defines number of seconds to wait for reg completion
> + * callback getting called from ST (in case,registration
> + * with ST returns PENDING status)
> + */
> +#define BT_REGISTER_TIMEOUT 6000 /* 6 sec */
> +
> +/**
> + * struct ti_st - BT driver operation structure
> + * @hdev: hci device pointer which binds to bt driver
> + * @flags: used locally,to maintain various BT driver status
> + * @streg_cbdata: to hold ST registration callback status
> + * @st_write: write function pointer of ST driver
> + * @wait_reg_completion - completion sync between ti_st_open
> + * and ti_st_registration_completion_cb.
> + */
> +struct ti_st {
> + struct hci_dev *hdev;
> + char streg_cbdata;
> + long (*st_write) (struct sk_buff *);
> + struct completion wait_reg_completion;
> +};
> +
> +static int reset;
> +
> +/* 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;
> + hdev =3D 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_registration_completion_cb(void *priv_data, char data)
> +{
> + struct ti_st *lhst =3D (struct ti_st *)priv_data;
> + /* ti_st_open() function needs value of 'data' to know
> + * the registration status(success/fail),So have a back
> + * up of it.
> + */
> + lhst->streg_cbdata =3D data;
> +
> + /* Got a feedback from ST for BT driver registration
> + * request.Wackup ti_st_open() function to continue
> + * it's open operation.
> + */
> + 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)
> +{
> + int err;
> + struct ti_st *lhst =3D (struct ti_st *)priv_data;
> +
> + if (!skb)
> + return -EFAULT;
> +
> + if (!lhst) {
> + kfree_skb(skb);
> + return -EFAULT;
> + }
> +
> + skb->dev =3D (struct net_device *)lhst->hdev;
> +
> + /* Forward skb to HCI CORE layer */
> + err =3D hci_recv_frame(skb);
> + if (err) {
> + kfree_skb(skb);
> + BT_ERR("Unable to push skb to HCI CORE(%d)", err);
> + return err;
> + }
> +
> + lhst->hdev->stat.byte_rx +=3D skb->len;
> + return 0;
> +}
> +
> +/* ------- Interfaces to HCI layer ------ */
> +/* protocol structure registered with shared transport */
> +static struct st_proto_s ti_st_proto =3D {
> + .type =3D ST_BT,
> + .recv =3D st_receive,
> + .reg_complete_cb =3D st_registration_completion_cb,
> + .priv_data =3D NULL,
> +};
> +
> +/* 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;
> +
> + BT_DBG("%s %p", hdev->name, hdev);
> +
> + /* provide contexts for callbacks from ST */
> + hst =3D hdev->driver_data;
> + ti_st_proto.priv_data =3D hst;
> +
> + err =3D st_register(&ti_st_proto);
> + if (err =3D=3D -EINPROGRESS) {
> + /* Prepare wait-for-completion handler data structures.
> + * Needed to syncronize this and st_registration_completion=
_cb()
> + * functions.
> + */
> + init_completion(&hst->wait_reg_completion);
> +
> + /* Reset ST registration callback status flag , this value
> + * will be updated in ti_st_registration_completion_cb()
> + * function whenever it called from ST driver.
> + */
> + hst->streg_cbdata =3D -EINPROGRESS;
> +
> + /* ST is busy with other protocol registration(may be busy =
with
> + * firmware download).So,Wait till the registration callbac=
k
> + * (passed as a argument to st_register() function) getting
> + * called from ST.
> + */
> + BT_DBG(" waiting for reg completion signal from ST");
> +
> + timeleft =3D 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 value? */
> + if (hst->streg_cbdata !=3D 0) {
> + BT_ERR("ST reg completion CB called with invalid"
> + "status %d", hst->streg_cbdata);
> + return -EAGAIN;
> + }
> + err =3D 0;
> + } else if (err =3D=3D -EPERM) {
> + BT_ERR("st_register failed %d", err);
> + return -EAGAIN;
> + }
> +
> + /* Do we have proper ST write function? */
> + if (ti_st_proto.write !=3D NULL) {
> + /* We need this pointer for sending any Bluetooth pkts */
> + hst->st_write =3D ti_st_proto.write;
> + } else {
> + BT_ERR("failed to get ST write func pointer");
> +
> + /* Undo registration with ST */
> + err =3D st_unregister(ST_BT);
> + if (err)
> + BT_ERR("st_unregister failed %d", err);
> +
> + hst->st_write =3D NULL;
> + return -EAGAIN;
> + }
> +
> + /* Registration with ST layer is completed successfully,
> + * now chip is ready to accept commands from HCI CORE.
> + * Mark HCI Device flag as RUNNING
> + */
> + set_bit(HCI_RUNNING, &hdev->flags);
> + return err;
> +}
> +
> +/* Close device */
> +static int ti_st_close(struct hci_dev *hdev)
> +{
> + int err =3D 0;
> + struct ti_st *hst;
> +
> + hst =3D hdev->driver_data;
> +
> + /* Clear HCI device RUNNING flag */
> + if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
> + BT_ERR("HCI not RUNNING?");
> +
> + /* continue to unregister from transport */
> + err =3D st_unregister(ST_BT);
> + if (err)
> + BT_ERR("st_unregister failed %d", err);
> +
> + hst->st_write =3D NULL;
> + return err;
> +}
> +
> +/* Called from HCI CORE , Sends frames to Shared Transport */
> +static int ti_st_send_frame(struct sk_buff *skb)
> +{
> + struct hci_dev *hdev;
> + struct ti_st *hst;
> + long len;
> +
> + if (!skb)
> + return -ENOMEM;
> +
> + hdev =3D (struct hci_dev *)skb->dev;
> + if (!hdev)
> + return -ENODEV;
> +
> + if (!test_bit(HCI_RUNNING, &hdev->flags))
> + return -EBUSY;
> +
> + hst =3D (struct ti_st *)hdev->driver_data;
> +
> + /* Prepend skb with frame type */
> + memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
> +
> + BT_DBG(" %s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type,
> + 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.
> + */
> + if (!hst->st_write) {
> + kfree_skb(skb);
> + BT_ERR(" Can't write to ST, st_write null?");
> + return -EAGAIN;
> + }
> +
> + len =3D hst->st_write(skb);
> + if (len < 0) {
> + kfree_skb(skb);
> + BT_ERR(" ST write failed (%ld)", len);
> + return -EAGAIN;
> + }
> +
> + /* ST accepted our skb. So, Go ahead and do rest */
> + hdev->stat.byte_tx +=3D len;
> + ti_st_tx_complete(hst, bt_cb(skb)->pkt_type);
> +
> + return 0;
> +}
> +
> +static void ti_st_destruct(struct hci_dev *hdev)
> +{
> + if (!hdev)
> + BT_ERR("Destruct called with invalid HCI Device"
> + "(hdev=3DNULL)");
> +
> + BT_DBG("%s", hdev->name);
> +
> + /* free ti_st memory */
> + kfree(hdev->driver_data);
> + return;
> +}
> +
> +/* Creates new HCI device */
> +static int ti_st_register_dev(struct ti_st *hst)
> +{
> + struct hci_dev *hdev;
> +
> + /* Initialize and register HCI device */
> + hdev =3D hci_alloc_dev();
> + if (!hdev)
> + return -ENOMEM;
> +
> + BT_DBG(" HCI device allocated. hdev=3D %p", hdev);
> +
> + hst->hdev =3D hdev;
> + hdev->bus =3D HCI_UART;
> + hdev->driver_data =3D hst;
> + hdev->open =3D ti_st_open;
> + hdev->close =3D ti_st_close;
> + hdev->flush =3D NULL;
> + hdev->send =3D ti_st_send_frame;
> + hdev->destruct =3D ti_st_destruct;
> + hdev->owner =3D THIS_MODULE;
> +
> + if (reset)
> + set_bit(HCI_QUIRK_NO_RESET, &hdev->quirks);
> +
> + if (hci_register_dev(hdev) < 0) {
> + BT_ERR("Can't register HCI device");
> + hci_free_dev(hdev);
> + return -ENODEV;
> + }
> +
> + BT_DBG(" HCI device registered. hdev=3D %p", hdev);
> + return 0;
> +}
> +
> +
> +static int bt_ti_probe(struct platform_device *pdev)
> +{
> + int err;
> + static struct ti_st *hst;
> + err =3D 0;
> +
> + BT_DBG(" Bluetooth Driver Version %s", VERSION);
> +
> + hst =3D kzalloc(sizeof(struct ti_st), GFP_KERNEL);
> + if (!hst) {
> + BT_ERR("Can't allocate control structure");
> + return -ENOMEM;
> + }
> +
> + /* Expose "hciX" device to user space */
> + err =3D ti_st_register_dev(hst);
> + if (err) {
> + kfree(hst);
> + BT_ERR("Unable to expose hciX device(%d)", err);
> + return err;
> + }
> +
> + dev_set_drvdata(&pdev->dev, hst);
> + return err;
> +}
> +
> +static int bt_ti_remove(struct platform_device *pdev)
> +{
> + struct ti_st *hst;
> +
> + hst =3D dev_get_drvdata(&pdev->dev);
> + /* Deallocate local resource's memory */
> + if (hst) {
> + struct hci_dev *hdev =3D hst->hdev;
> + if (!hdev) {
> + BT_ERR("Invalid hdev memory");
> + kfree(hst);
> + } else {
> + ti_st_close(hdev);
> + hci_unregister_dev(hdev);
> + /* Free HCI device memory */
> + hci_free_dev(hdev);
> + }
> + }
> + return 0;
> +}
> +
> +static struct platform_driver btwilink_driver =3D {
> + .probe =3D bt_ti_probe,
> + .remove =3D bt_ti_remove,
> + .driver =3D {
> + .name =3D "btwilink",
> + .owner =3D THIS_MODULE,
> + },
> +};
> +
> +/* ------- Module Init/Exit interfaces ------ */
> +static int __init bt_drv_init(void)
> +{
> + long ret;
> +
> + ret =3D platform_driver_register(&btwilink_driver);
> + if (ret !=3D 0) {
> + BT_ERR("btwilink platform drv registration failed");
> + return -EPERM;
> + }
> + return 0;
> +}
> +
> +static void __exit bt_drv_exit(void)
> +{
> + platform_driver_unregister(&btwilink_driver);
> +}
> +
> +module_init(bt_drv_init);
> +module_exit(bt_drv_exit);
> +
> +/* ------ Module Info ------ */
> +
> +module_param(reset, bool, 0644);
> +MODULE_PARM_DESC(reset, "Send HCI reset command on initialization");
> +MODULE_AUTHOR("Raja Mani <raja_mani@ti.com>");
> +MODULE_DESCRIPTION("Bluetooth Driver for TI Shared Transport" VERSION);
> +MODULE_VERSION(VERSION);
> +MODULE_LICENSE("GPL");
> --
> 1.6.5
^ permalink raw reply
* Re: [PATCH] bluetooth: fix unaligned access to l2cap conf data
From: Harvey Harrison @ 2010-10-18 18:17 UTC (permalink / raw)
To: Mike Frysinger
Cc: linux-bluetooth, Marcel Holtmann, uclinux-dist-devel,
linux-kernel, Andrew Morton, steven miao
In-Reply-To: <1287268187-9628-1-git-send-email-vapier@gentoo.org>
On Sat, Oct 16, 2010 at 3:29 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> From: steven miao <realmz6@gmail.com>
>
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0case 2:
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *((__le16 *) opt->val)=
=3D cpu_to_le16(val);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 put_unaligned_le16(cpu=
_to_le16(val), opt->val);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0break;
>
I think you wanted:
put_unaligned_le16(val, opt->val);
Cheers,
Harvey
^ permalink raw reply
* Re: >net-wireless/bluez-4.63 unable to connect audio streams due commit
From: Pacho Ramos @ 2010-10-18 18:24 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Luiz Augusto von Dentz, Johan Hedberg, linux-bluetooth
In-Reply-To: <20101004123552.GC11737@pengutronix.de>
[-- Attachment #1.1: Type: text/plain, Size: 899 bytes --]
El lun, 04-10-2010 a las 14:35 +0200, Uwe Kleine-König escribió:
> Hello Pacho,
>
> On Mon, Oct 04, 2010 at 12:25:46PM +0200, Pacho Ramos wrote:
> > > I would say this was because of double authentication request, but it
> > > seems it is not the case, actually ssp doesn't seems to be used at all
> > > here so this must be something else, maybe you should try this:
> > >
> > > http://thread.gmane.org/gmane.linux.bluez.kernel/7256
> > >
> >
> > Thanks but, how should I try to apply that patch? Looks like
> > net/bluetooth/rfcomm/core.c is not present on bluez-4.72 sources
> I guess this is a patch to apply to your kernel, not bluez.
>
> Best regards
> Uwe
>
Downstream affected reported told me it's still failing even with the
patch:
http://bugs.gentoo.org/show_bug.cgi?id=327705#c19
Attached is the new hcidump output
Thanks a lot for your help :-)
[-- Attachment #1.2: hcidump.out --]
[-- Type: text/plain, Size: 5614 bytes --]
HCI sniffer - Bluetooth packet analyzer ver 1.42
device: hci0 snap_len: 1028 filter: 0xffffffffffffffff
< HCI Command: Create Connection (0x01|0x0005) plen 13
> HCI Event: Command Status (0x0f) plen 4
> HCI Event: Link Key Request (0x17) plen 6
< HCI Command: Link Key Request Reply (0x01|0x000b) plen 22
> HCI Event: Command Complete (0x0e) plen 10
> HCI Event: Connect Complete (0x03) plen 11
< HCI Command: Read Remote Supported Features (0x01|0x001b) plen 2
> HCI Event: Command Status (0x0f) plen 4
< HCI Command: Remote Name Request (0x01|0x0019) plen 10
> HCI Event: Max Slots Change (0x1b) plen 3
> HCI Event: Read Remote Supported Features (0x0b) plen 11
< ACL data: handle 12 flags 0x02 dlen 10
L2CAP(s): Info req: type 2
> HCI Event: Command Status (0x0f) plen 4
> ACL data: handle 12 flags 0x02 dlen 16
L2CAP(s): Info rsp: type 2 result 0
Extended feature mask 0x0000
< ACL data: handle 12 flags 0x02 dlen 12
L2CAP(s): Connect req: psm 1 scid 0x0040
> ACL data: handle 12 flags 0x02 dlen 16
L2CAP(s): Connect rsp: dcid 0x005a scid 0x0040 result 1 status 2
Connection pending - Authorization pending
> ACL data: handle 12 flags 0x02 dlen 16
L2CAP(s): Connect rsp: dcid 0x005a scid 0x0040 result 0 status 0
Connection successful
< ACL data: handle 12 flags 0x02 dlen 12
L2CAP(s): Config req: dcid 0x005a flags 0x00 clen 0
> HCI Event: Remote Name Req Complete (0x07) plen 255
> ACL data: handle 12 flags 0x02 dlen 14
L2CAP(s): Config rsp: scid 0x0040 flags 0x00 result 0 clen 0
Success
> ACL data: handle 12 flags 0x02 dlen 16
L2CAP(s): Config req: dcid 0x0040 flags 0x00 clen 4
MTU 48
< ACL data: handle 12 flags 0x02 dlen 18
L2CAP(s): Config rsp: scid 0x005a flags 0x00 result 0 clen 4
MTU 48
< ACL data: handle 12 flags 0x02 dlen 24
L2CAP(d): cid 0x005a len 20 [psm 1]
SDP SSA Req: tid 0x0 len 0xf
pat uuid-16 0x111e (Handsfree)
max 65535
aid(s) 0x0000 - 0xffff
cont 00
> HCI Event: Number of Completed Packets (0x13) plen 5
> ACL data: handle 12 flags 0x02 dlen 52
L2CAP(d): cid 0x0040 len 48 [psm 1]
SDP SSA Rsp: tid 0x0 len 0x2b
count 38
cont 02 00 39
< ACL data: handle 12 flags 0x02 dlen 26
L2CAP(d): cid 0x005a len 22 [psm 1]
SDP SSA Req: tid 0x1 len 0x11
pat uuid-16 0x111e (Handsfree)
max 65535
aid(s) 0x0000 - 0xffff
cont 02 00 39
> ACL data: handle 12 flags 0x02 dlen 52
L2CAP(d): cid 0x0040 len 48 [psm 1]
SDP SSA Rsp: tid 0x1 len 0x2b
count 38
cont 02 00 13
< ACL data: handle 12 flags 0x02 dlen 26
L2CAP(d): cid 0x005a len 22 [psm 1]
SDP SSA Req: tid 0x2 len 0x11
pat uuid-16 0x111e (Handsfree)
max 65535
aid(s) 0x0000 - 0xffff
cont 02 00 13
> ACL data: handle 12 flags 0x02 dlen 31
L2CAP(d): cid 0x0040 len 27 [psm 1]
SDP SSA Rsp: tid 0x2 len 0x16
count 19
record #0
aid 0x0000 (SrvRecHndl)
uint 0x10004
aid 0x0001 (SrvClassIDList)
< uuid-16 0x111e (Handsfree) uuid-16 0x1203 (Audio) >
aid 0x0004 (ProtocolDescList)
< < uuid-16 0x0100 (L2CAP) > <
uuid-16 0x0003 (RFCOMM) uint 0x1 > >
aid 0x0006 (LangBaseAttrIDList)
< uint 0x656e uint 0x6a uint 0x100 >
aid 0x0009 (BTProfileDescList)
< < uuid-16 0x111e (Handsfree) uint 0x101 > >
aid 0x0100 (SrvName)
str "Hands-Free unit"
aid 0x0311 (SuppFeatures)
uint 0x18
cont 00
< HCI Command: Authentication Requested (0x01|0x0011) plen 2
> HCI Event: Command Status (0x0f) plen 4
> HCI Event: Auth Complete (0x06) plen 3
< HCI Command: Set Connection Encryption (0x01|0x0013) plen 3
> HCI Event: Command Status (0x0f) plen 4
> HCI Event: Number of Completed Packets (0x13) plen 5
< ACL data: handle 12 flags 0x02 dlen 12
L2CAP(s): Disconn req: dcid 0x005a scid 0x0040
> HCI Event: Number of Completed Packets (0x13) plen 5
> HCI Event: Disconn Complete (0x05) plen 4
< HCI Command: Create Connection (0x01|0x0005) plen 13
> HCI Event: Command Status (0x0f) plen 4
> HCI Event: Link Key Request (0x17) plen 6
< HCI Command: Link Key Request Reply (0x01|0x000b) plen 22
> HCI Event: Command Complete (0x0e) plen 10
> HCI Event: Connect Complete (0x03) plen 11
< HCI Command: Read Remote Supported Features (0x01|0x001b) plen 2
> HCI Event: Max Slots Change (0x1b) plen 3
> HCI Event: Command Status (0x0f) plen 4
< HCI Command: Remote Name Request (0x01|0x0019) plen 10
> HCI Event: Read Remote Supported Features (0x0b) plen 11
< ACL data: handle 11 flags 0x02 dlen 10
L2CAP(s): Info req: type 2
> HCI Event: Command Status (0x0f) plen 4
> ACL data: handle 11 flags 0x02 dlen 16
L2CAP(s): Info rsp: type 2 result 0
Extended feature mask 0x0000
< HCI Command: Authentication Requested (0x01|0x0011) plen 2
> HCI Event: Remote Name Req Complete (0x07) plen 255
> HCI Event: Command Status (0x0f) plen 4
> HCI Event: Auth Complete (0x06) plen 3
< HCI Command: Set Connection Encryption (0x01|0x0013) plen 3
> HCI Event: Command Status (0x0f) plen 4
> HCI Event: Number of Completed Packets (0x13) plen 5
> HCI Event: Disconn Complete (0x05) plen 4
< HCI Command: Reset (0x03|0x0003) plen 0
> HCI Event: Command Complete (0x0e) plen 4
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: pull request: bluetooth-2.6 2010-10-18
From: Gustavo F. Padovan @ 2010-10-18 19:02 UTC (permalink / raw)
To: John W. Linville; +Cc: marcel, linux-wireless, linux-bluetooth, davem
In-Reply-To: <20101018200830.GA2443@tuxdriver.com>
* John W. Linville <linville@tuxdriver.com> [2010-10-18 16:08:31 -0400]:
> On Mon, Oct 18, 2010 at 03:55:40AM -0200, Gustavo F. Padovan wrote:
> > Hi John,
> >
> > Not sure if we still have time for a last minute fix, but here goes a
> > fix for a NULL dereference in L2CAP layer for 2.6.36-rc8. Please also let
> > me know if this is the right way to handle such last minute fix. Maybe you
> > should want this directly through Dave.
> >
> > Thanks.
>
> The fix seems fine to me. Given the short schedule, it might be best
> to skip the middleman and have Dave pull directly from you?
>
> Dave, be forewarned that Gustavo has based his tree off 2.6.36-rc8 --
> dunno if you are ready to pull that into net-2.6 or not.
If you guys think that rebase against Linus' tree could be a problem, I
can start to base bluetooth-2.6 against net-2.6 for the next pull
requests.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH] bluetooth: fix unaligned access to l2cap conf data
From: Mike Frysinger @ 2010-10-18 19:10 UTC (permalink / raw)
To: Gustavo F. Padovan
Cc: Harvey Harrison, linux-bluetooth, Marcel Holtmann,
uclinux-dist-devel, linux-kernel, Andrew Morton, steven miao
In-Reply-To: <20101018163223.GC2468@vigoh>
On Mon, Oct 18, 2010 at 12:32, Gustavo F. Padovan wrote:
> * Harvey Harrison <harvey.harrison@gmail.com> [2010-10-18 11:17:28 -0700]=
:
>> On Sat, Oct 16, 2010 at 3:29 PM, Mike Frysinger <vapier@gentoo.org> wrot=
e:
>> > From: steven miao <realmz6@gmail.com>
>> >
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0case 2:
>> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *((__le16 *) opt->v=
al) =3D cpu_to_le16(val);
>> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 put_unaligned_le16(=
cpu_to_le16(val), opt->val);
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0break;
>>
>> I think you wanted:
>> put_unaligned_le16(val, opt->val);
>
> I fixed that in the tree. Thanks for the report.
i guess you fixed the 32bit one too ?
put_unaligned_le32(cpu_to_le32(val), opt->val);
-mike
^ permalink raw reply
* Re: [PATCH] bluetooth: fix unaligned access to l2cap conf data
From: Gustavo F. Padovan @ 2010-10-18 19:12 UTC (permalink / raw)
To: Mike Frysinger
Cc: Harvey Harrison, linux-bluetooth, Marcel Holtmann,
uclinux-dist-devel, linux-kernel, Andrew Morton, steven miao
In-Reply-To: <AANLkTin3Tx8jyR47G3SNbP_zFJ+tGdR6FtraHmM2T79j@mail.gmail.com>
* Mike Frysinger <vapier@gentoo.org> [2010-10-18 15:10:36 -0400]:
> On Mon, Oct 18, 2010 at 12:32, Gustavo F. Padovan wrote:
> > * Harvey Harrison <harvey.harrison@gmail.com> [2010-10-18 11:17:28 -070=
0]:
> >> On Sat, Oct 16, 2010 at 3:29 PM, Mike Frysinger <vapier@gentoo.org> wr=
ote:
> >> > From: steven miao <realmz6@gmail.com>
> >> >
> >> > =A0 =A0 =A0 =A0case 2:
> >> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 *((__le16 *) opt->val) =3D cpu_to_le16=
(val);
> >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 put_unaligned_le16(cpu_to_le16(val), o=
pt->val);
> >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
> >>
> >> I think you wanted:
> >> put_unaligned_le16(val, opt->val);
> >
> > I fixed that in the tree. Thanks for the report.
>=20
> i guess you fixed the 32bit one too ?
> put_unaligned_le32(cpu_to_le32(val), opt->val);
Yes, I did.
--=20
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: [PATCH] bluetooth: fix unaligned access to l2cap conf data
From: Mike Frysinger @ 2010-10-18 19:39 UTC (permalink / raw)
To: Gustavo F. Padovan
Cc: Harvey Harrison, linux-bluetooth, Marcel Holtmann,
uclinux-dist-devel, linux-kernel, Andrew Morton, steven miao
In-Reply-To: <20101018191209.GE2468@vigoh>
On Mon, Oct 18, 2010 at 15:12, Gustavo F. Padovan wrote:
> * Mike Frysinger <vapier@gentoo.org> [2010-10-18 15:10:36 -0400]:
>> On Mon, Oct 18, 2010 at 12:32, Gustavo F. Padovan wrote:
>> > * Harvey Harrison <harvey.harrison@gmail.com> [2010-10-18 11:17:28 -07=
00]:
>> >> On Sat, Oct 16, 2010 at 3:29 PM, Mike Frysinger <vapier@gentoo.org> w=
rote:
>> >> > From: steven miao <realmz6@gmail.com>
>> >> >
>> >> > =C2=A0 =C2=A0 =C2=A0 =C2=A0case 2:
>> >> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *((__le16 *) opt=
->val) =3D cpu_to_le16(val);
>> >> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 put_unaligned_le=
16(cpu_to_le16(val), opt->val);
>> >> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0break;
>> >>
>> >> I think you wanted:
>> >> put_unaligned_le16(val, opt->val);
>> >
>> > I fixed that in the tree. Thanks for the report.
>>
>> i guess you fixed the 32bit one too ?
>> =C2=A0 put_unaligned_le32(cpu_to_le32(val), opt->val);
>
> Yes, I did.
cool. thanks guys !
-mike
^ permalink raw reply
* Re: [PATCH] Bluetooth: btwilink driver
From: Gustavo F. Padovan @ 2010-10-18 19:48 UTC (permalink / raw)
To: pavan_savoy; +Cc: marcel, linux-bluetooth, linux-kernel
In-Reply-To: <1287176299-19313-1-git-send-email-pavan_savoy@ti.com>
Hi Pavan,
* pavan_savoy@ti.com <pavan_savoy@ti.com> [2010-10-15 16:58:19 -0400]:
> From: Pavan Savoy <pavan_savoy@ti.com>
>
> Gustavo, Marcel,
>
> Renaming the patch, since the driver is renamed to btwilink.
> Thanks for the comments,
> Please review and provide your comments on this version of patch,
>
> -- patch description --
>
> This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> Texas Instrument's WiLink chipsets combine wireless technologies
> like BT, FM, GPS and WLAN onto a single chip.
>
> This Bluetooth driver works on top of the TI_ST shared transport
> line discipline driver which also allows other drivers like
> FM V4L2 and GPS character driver to make use of the same UART interface.
>
> Kconfig and Makefile modifications to enable the Bluetooth
> driver for Texas Instrument's WiLink 7 chipset.
>
> Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> ---
> drivers/bluetooth/Kconfig | 10 +
> drivers/bluetooth/Makefile | 1 +
> drivers/bluetooth/btwilink.c | 424 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 435 insertions(+), 0 deletions(-)
> create mode 100644 drivers/bluetooth/btwilink.c
>
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 02deef4..e0d67dd 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -219,4 +219,14 @@ 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 "BlueZ bluetooth driver for TI ST"
> + 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.
> endmenu
> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> index 71bdf13..f4460f4 100644
> --- a/drivers/bluetooth/Makefile
> +++ b/drivers/bluetooth/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_BT_HCIBTSDIO) += btsdio.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
>
> btmrvl-y := btmrvl_main.o
> btmrvl-$(CONFIG_DEBUG_FS) += btmrvl_debugfs.o
> diff --git a/drivers/bluetooth/btwilink.c b/drivers/bluetooth/btwilink.c
> new file mode 100644
> index 0000000..f67791f
> --- /dev/null
> +++ b/drivers/bluetooth/btwilink.c
> @@ -0,0 +1,424 @@
> +/*
> + * 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 <raja_mani@ti.com>
> + * Pavan Savoy <pavan_savoy@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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + *
> + */
> +
> +#include <linux/platform_device.h>
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +
> +#include <linux/ti_wilink_st.h>
> +
> +/* Bluetooth Driver Version */
> +#define VERSION "1.0"
> +
> +/* Defines number of seconds to wait for reg completion
> + * callback getting called from ST (in case,registration
> + * with ST returns PENDING status)
> + */
> +#define BT_REGISTER_TIMEOUT 6000 /* 6 sec */
> +
> +/**
> + * struct ti_st - BT driver operation structure
> + * @hdev: hci device pointer which binds to bt driver
> + * @flags: used locally,to maintain various BT driver status
> + * @streg_cbdata: to hold ST registration callback status
> + * @st_write: write function pointer of ST driver
> + * @wait_reg_completion - completion sync between ti_st_open
> + * and ti_st_registration_completion_cb.
> + */
> +struct ti_st {
> + struct hci_dev *hdev;
> + char streg_cbdata;
> + long (*st_write) (struct sk_buff *);
> + struct completion wait_reg_completion;
> +};
> +
> +static int reset;
> +
> +/* 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;
> + 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_registration_completion_cb(void *priv_data, char data)
> +{
> + struct ti_st *lhst = (struct ti_st *)priv_data;
Blank line here.
> + /* ti_st_open() function needs value of 'data' to know
> + * the registration status(success/fail),So have a back
> + * up of it.
> + */
> + lhst->streg_cbdata = data;
> +
> + /* Got a feedback from ST for BT driver registration
> + * request.Wackup ti_st_open() function to continue
> + * it's open operation.
> + */
> + 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)
> +{
> + int err;
> + struct ti_st *lhst = (struct ti_st *)priv_data;
> +
> + if (!skb)
> + return -EFAULT;
> +
> + if (!lhst) {
> + kfree_skb(skb);
> + return -EFAULT;
> + }
> +
> + skb->dev = (struct net_device *)lhst->hdev;
> +
> + /* Forward skb to HCI CORE layer */
> + err = hci_recv_frame(skb);
> + if (err) {
> + kfree_skb(skb);
> + BT_ERR("Unable to push skb to HCI CORE(%d)", err);
s/CORE/core/
> + 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 = {
> + .type = ST_BT,
> + .recv = st_receive,
> + .reg_complete_cb = st_registration_completion_cb,
> + .priv_data = NULL,
> +};
> +
> +/* 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;
> +
> + BT_DBG("%s %p", hdev->name, hdev);
> +
> + /* provide contexts for callbacks from ST */
> + hst = hdev->driver_data;
> + ti_st_proto.priv_data = hst;
> +
> + err = st_register(&ti_st_proto);
> + if (err == -EINPROGRESS) {
> + /* Prepare wait-for-completion handler data structures.
> + * Needed to syncronize this and st_registration_completion_cb()
> + * functions.
> + */
> + init_completion(&hst->wait_reg_completion);
> +
> + /* Reset ST registration callback status flag , this value
> + * will be updated in ti_st_registration_completion_cb()
> + * function whenever it called from ST driver.
> + */
> + hst->streg_cbdata = -EINPROGRESS;
> +
> + /* ST is busy with other protocol registration(may be busy with
> + * firmware download).So,Wait till the registration callback
> + * (passed as a argument to st_register() function) getting
> + * called from ST.
> + */
> + BT_DBG(" waiting for reg 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);
How does this get printed. "...regcompletion..." ?
> + return -ETIMEDOUT;
> + }
> +
> + /* Is ST registration callback called with ERROR value? */
> + if (hst->streg_cbdata != 0) {
> + BT_ERR("ST reg completion CB called with invalid"
> + "status %d", hst->streg_cbdata);
> + return -EAGAIN;
> + }
> + err = 0;
> + } else if (err == -EPERM) {
> + BT_ERR("st_register failed %d", err);
> + return -EAGAIN;
Why? if -EPERM return -EAGAIN?
> + }
> +
> + /* Do we have proper ST write function? */
> + if (ti_st_proto.write != NULL) {
> + /* We need this pointer for sending any Bluetooth pkts */
> + hst->st_write = ti_st_proto.write;
I asked in the other e-mail: Who sets ti_st_proto.write()? I didn't get
this.
> + } else {
> + BT_ERR("failed to get ST write func pointer");
> +
> + /* Undo registration with ST */
> + err = st_unregister(ST_BT);
> + if (err)
> + BT_ERR("st_unregister failed %d", err);
> +
> + hst->st_write = NULL;
> + return -EAGAIN;
return err; instead
> + }
> +
> + /* Registration with ST layer is completed successfully,
> + * now chip is ready to accept commands from HCI CORE.
> + * Mark HCI Device flag as RUNNING
> + */
> + set_bit(HCI_RUNNING, &hdev->flags);
> + return err;
> +}
> +
> +/* Close device */
> +static int ti_st_close(struct hci_dev *hdev)
> +{
> + int err = 0;
do not set err to 0 here.
> + struct ti_st *hst;
you can set hst to hdev->.... here.
> +
> + hst = hdev->driver_data;
> +
> + /* Clear HCI device RUNNING flag */
> + if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
> + BT_ERR("HCI not RUNNING?");
You didn't fix the things I told you. This is not a error. Just return 0
here.
> +
> + /* continue to unregister from transport */
> + err = st_unregister(ST_BT);
> + if (err)
> + BT_ERR("st_unregister failed %d", err);
> +
> + hst->st_write = NULL;
> + return err;
> +}
> +
> +/* Called from HCI CORE , Sends frames to Shared Transport */
> +static int ti_st_send_frame(struct sk_buff *skb)
> +{
> + struct hci_dev *hdev;
> + struct ti_st *hst;
> + long len;
> +
> + if (!skb)
> + return -ENOMEM;
> +
> + hdev = (struct hci_dev *)skb->dev;
> + if (!hdev)
> + return -ENODEV;
> +
> + if (!test_bit(HCI_RUNNING, &hdev->flags))
> + return -EBUSY;
> +
> + hst = (struct ti_st *)hdev->driver_data;
> +
> + /* Prepend skb with frame type */
> + memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
> +
> + BT_DBG(" %s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type,
> + 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.
> + */
> + if (!hst->st_write) {
> + kfree_skb(skb);
> + BT_ERR(" Can't write to ST, st_write null?");
> + return -EAGAIN;
> + }
> +
> + len = hst->st_write(skb);
> + if (len < 0) {
> + kfree_skb(skb);
> + BT_ERR(" ST write failed (%ld)", len);
> + return -EAGAIN;
> + }
Explain to me why it is worth return -EAGAIN in both cases.
> +
> + /* ST accepted our skb. So, Go ahead and do rest */
> + hdev->stat.byte_tx += len;
> + ti_st_tx_complete(hst, bt_cb(skb)->pkt_type);
> +
> + return 0;
> +}
> +
> +static void ti_st_destruct(struct hci_dev *hdev)
> +{
> + if (!hdev)
> + BT_ERR("Destruct called with invalid HCI Device"
> + "(hdev=NULL)");
> +
> + BT_DBG("%s", hdev->name);
> +
> + /* free ti_st memory */
> + kfree(hdev->driver_data);
> + return;
> +}
> +
> +/* Creates new HCI device */
> +static int ti_st_register_dev(struct ti_st *hst)
> +{
> + struct hci_dev *hdev;
> +
> + /* Initialize and register HCI device */
> + hdev = hci_alloc_dev();
> + if (!hdev)
> + return -ENOMEM;
> +
> + BT_DBG(" HCI device allocated. hdev= %p", hdev);
just BT_DBG("hdev= %p", hdev);
> +
> + hst->hdev = hdev;
> + hdev->bus = HCI_UART;
> + hdev->driver_data = hst;
> + hdev->open = ti_st_open;
> + hdev->close = ti_st_close;
> + hdev->flush = NULL;
> + hdev->send = ti_st_send_frame;
> + hdev->destruct = ti_st_destruct;
> + hdev->owner = THIS_MODULE;
> +
> + if (reset)
> + set_bit(HCI_QUIRK_NO_RESET, &hdev->quirks);
> +
> + if (hci_register_dev(hdev) < 0) {
err = hci_register_dev()
if (err < 0)
...
...
return err;
> + BT_ERR("Can't register HCI device");
> + hci_free_dev(hdev);
> + return -ENODEV;
> + }
> +
> + BT_DBG(" HCI device registered. hdev= %p", hdev);
> + return 0;
> +}
> +
> +
> +static int bt_ti_probe(struct platform_device *pdev)
> +{
> + int err;
> + static struct ti_st *hst;
> + err = 0;
> +
> + BT_DBG(" Bluetooth Driver Version %s", VERSION);
> +
> + hst = kzalloc(sizeof(struct ti_st), GFP_KERNEL);
> + if (!hst) {
> + BT_ERR("Can't allocate control structure");
remove this BT_ERR.
> + return -ENOMEM;
> + }
> +
> + /* Expose "hciX" device to user space */
> + err = ti_st_register_dev(hst);
> + if (err) {
> + kfree(hst);
> + BT_ERR("Unable to expose hciX device(%d)", err);
Ok, if you don't know how to extract the hci id, remove the whole error
message then.
> + return err;
> + }
> +
> + dev_set_drvdata(&pdev->dev, hst);
> + return err;
> +}
> +
> +static int bt_ti_remove(struct platform_device *pdev)
> +{
> + struct ti_st *hst;
> +
> + hst = dev_get_drvdata(&pdev->dev);
> + /* Deallocate local resource's memory */
> + if (hst) {
> + struct hci_dev *hdev = hst->hdev;
> + if (!hdev) {
> + BT_ERR("Invalid hdev memory");
> + kfree(hst);
> + } else {
> + ti_st_close(hdev);
> + hci_unregister_dev(hdev);
> + /* Free HCI device memory */
> + hci_free_dev(hdev);
> + }
> + }
> + return 0;
> +}
> +
> +static struct platform_driver btwilink_driver = {
> + .probe = bt_ti_probe,
> + .remove = bt_ti_remove,
> + .driver = {
> + .name = "btwilink",
> + .owner = THIS_MODULE,
> + },
> +};
> +
> +/* ------- Module Init/Exit interfaces ------ */
> +static int __init bt_drv_init(void)
> +{
> + long ret;
> +
> + ret = platform_driver_register(&btwilink_driver);
> + if (ret != 0) {
> + BT_ERR("btwilink platform drv registration failed");
> + return -EPERM;
> + }
> + return 0;
if (ret)
BT_ERR("btwilink platform drv registration failed");
return ret;
and s/drv/driver/
Please fix all the issues, then we go to another round of review. ;)
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* RE: [PATCH] Bluetooth: btwilink driver
From: Savoy, Pavan @ 2010-10-18 19:53 UTC (permalink / raw)
To: Gustavo F. Padovan
Cc: marcel@holtmann.org, linux-bluetooth@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20101018194811.GG2468@vigoh>
> -----Original Message-----
> From: Gustavo F. Padovan [mailto:pao@profusion.mobi] On Behalf Of Gustavo=
F.
> Padovan
> Sent: Monday, October 18, 2010 2:48 PM
> To: Savoy, Pavan
> Cc: marcel@holtmann.org; linux-bluetooth@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH] Bluetooth: btwilink driver
>
> Hi Pavan,
>
> * pavan_savoy@ti.com <pavan_savoy@ti.com> [2010-10-15 16:58:19 -0400]:
>
> > From: Pavan Savoy <pavan_savoy@ti.com>
> >
> > Gustavo, Marcel,
> >
> > Renaming the patch, since the driver is renamed to btwilink.
> > Thanks for the comments,
> > Please review and provide your comments on this version of patch,
> >
> > -- patch description --
> >
> > This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> > Texas Instrument's WiLink chipsets combine wireless technologies
> > like BT, FM, GPS and WLAN onto a single chip.
> >
> > This Bluetooth driver works on top of the TI_ST shared transport
> > line discipline driver which also allows other drivers like
> > FM V4L2 and GPS character driver to make use of the same UART interface=
.
> >
> > Kconfig and Makefile modifications to enable the Bluetooth
> > driver for Texas Instrument's WiLink 7 chipset.
> >
> > Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> > ---
> > drivers/bluetooth/Kconfig | 10 +
> > drivers/bluetooth/Makefile | 1 +
> > drivers/bluetooth/btwilink.c | 424
> ++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 435 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/bluetooth/btwilink.c
> >
> > diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> > index 02deef4..e0d67dd 100644
> > --- a/drivers/bluetooth/Kconfig
> > +++ b/drivers/bluetooth/Kconfig
> > @@ -219,4 +219,14 @@ config BT_ATH3K
> > Say Y here to compile support for "Atheros firmware download driv=
er"
> > into the kernel or say M to compile it as module (ath3k).
> >
> > +config BT_WILINK
> > + tristate "BlueZ bluetooth driver for TI ST"
> > + depends on TI_ST
> > + help
> > + This enables the Bluetooth driver for Texas Instrument's BT/FM/GP=
S
> > + 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 driv=
er
> > + into the kernel or say M to compile it as module.
> > endmenu
> > diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> > index 71bdf13..f4460f4 100644
> > --- a/drivers/bluetooth/Makefile
> > +++ b/drivers/bluetooth/Makefile
> > @@ -18,6 +18,7 @@ obj-$(CONFIG_BT_HCIBTSDIO) +=3D btsdio.o
> > obj-$(CONFIG_BT_ATH3K) +=3D ath3k.o
> > obj-$(CONFIG_BT_MRVL) +=3D btmrvl.o
> > obj-$(CONFIG_BT_MRVL_SDIO) +=3D btmrvl_sdio.o
> > +obj-$(CONFIG_BT_WILINK) +=3D btwilink.o
> >
> > btmrvl-y :=3D btmrvl_main.o
> > btmrvl-$(CONFIG_DEBUG_FS) +=3D btmrvl_debugfs.o
> > diff --git a/drivers/bluetooth/btwilink.c b/drivers/bluetooth/btwilink.=
c
> > new file mode 100644
> > index 0000000..f67791f
> > --- /dev/null
> > +++ b/drivers/bluetooth/btwilink.c
> > @@ -0,0 +1,424 @@
> > +/*
> > + * 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 <raja_mani@ti.com>
> > + * Pavan Savoy <pavan_savoy@ti.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modi=
fy
> > + * 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.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-13=
07
> USA
> > + *
> > + */
> > +
> > +#include <linux/platform_device.h>
> > +#include <net/bluetooth/bluetooth.h>
> > +#include <net/bluetooth/hci_core.h>
> > +
> > +#include <linux/ti_wilink_st.h>
> > +
> > +/* Bluetooth Driver Version */
> > +#define VERSION "1.0"
> > +
> > +/* Defines number of seconds to wait for reg completion
> > + * callback getting called from ST (in case,registration
> > + * with ST returns PENDING status)
> > + */
> > +#define BT_REGISTER_TIMEOUT 6000 /* 6 sec */
> > +
> > +/**
> > + * struct ti_st - BT driver operation structure
> > + * @hdev: hci device pointer which binds to bt driver
> > + * @flags: used locally,to maintain various BT driver status
> > + * @streg_cbdata: to hold ST registration callback status
> > + * @st_write: write function pointer of ST driver
> > + * @wait_reg_completion - completion sync between ti_st_open
> > + * and ti_st_registration_completion_cb.
> > + */
> > +struct ti_st {
> > + struct hci_dev *hdev;
> > + char streg_cbdata;
> > + long (*st_write) (struct sk_buff *);
> > + struct completion wait_reg_completion;
> > +};
> > +
> > +static int reset;
> > +
> > +/* 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;
> > + hdev =3D 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_registration_completion_cb(void *priv_data, char data)
> > +{
> > + struct ti_st *lhst =3D (struct ti_st *)priv_data;
>
> Blank line here.
>
> > + /* ti_st_open() function needs value of 'data' to know
> > + * the registration status(success/fail),So have a back
> > + * up of it.
> > + */
> > + lhst->streg_cbdata =3D data;
> > +
> > + /* Got a feedback from ST for BT driver registration
> > + * request.Wackup ti_st_open() function to continue
> > + * it's open operation.
> > + */
> > + 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)
> > +{
> > + int err;
> > + struct ti_st *lhst =3D (struct ti_st *)priv_data;
> > +
> > + if (!skb)
> > + return -EFAULT;
> > +
> > + if (!lhst) {
> > + kfree_skb(skb);
> > + return -EFAULT;
> > + }
> > +
> > + skb->dev =3D (struct net_device *)lhst->hdev;
> > +
> > + /* Forward skb to HCI CORE layer */
> > + err =3D hci_recv_frame(skb);
> > + if (err) {
> > + kfree_skb(skb);
> > + BT_ERR("Unable to push skb to HCI CORE(%d)", err);
>
> s/CORE/core/
>
> > + return err;
> > + }
> > +
> > + lhst->hdev->stat.byte_rx +=3D skb->len;
> > + return 0;
> > +}
> > +
> > +/* ------- Interfaces to HCI layer ------ */
> > +/* protocol structure registered with shared transport */
> > +static struct st_proto_s ti_st_proto =3D {
> > + .type =3D ST_BT,
> > + .recv =3D st_receive,
> > + .reg_complete_cb =3D st_registration_completion_cb,
> > + .priv_data =3D NULL,
> > +};
> > +
> > +/* 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;
> > +
> > + BT_DBG("%s %p", hdev->name, hdev);
> > +
> > + /* provide contexts for callbacks from ST */
> > + hst =3D hdev->driver_data;
> > + ti_st_proto.priv_data =3D hst;
> > +
> > + err =3D st_register(&ti_st_proto);
> > + if (err =3D=3D -EINPROGRESS) {
> > + /* Prepare wait-for-completion handler data structures.
> > + * Needed to syncronize this and st_registration_completion=
_cb()
> > + * functions.
> > + */
> > + init_completion(&hst->wait_reg_completion);
> > +
> > + /* Reset ST registration callback status flag , this value
> > + * will be updated in ti_st_registration_completion_cb()
> > + * function whenever it called from ST driver.
> > + */
> > + hst->streg_cbdata =3D -EINPROGRESS;
> > +
> > + /* ST is busy with other protocol registration(may be busy =
with
> > + * firmware download).So,Wait till the registration callbac=
k
> > + * (passed as a argument to st_register() function) getting
> > + * called from ST.
> > + */
> > + BT_DBG(" waiting for reg completion signal from ST");
> > +
> > + timeleft =3D 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);
>
> How does this get printed. "...regcompletion..." ?
>
> > + return -ETIMEDOUT;
> > + }
> > +
> > + /* Is ST registration callback called with ERROR value? */
> > + if (hst->streg_cbdata !=3D 0) {
> > + BT_ERR("ST reg completion CB called with invalid"
> > + "status %d", hst->streg_cbdata);
> > + return -EAGAIN;
> > + }
> > + err =3D 0;
> > + } else if (err =3D=3D -EPERM) {
> > + BT_ERR("st_register failed %d", err);
> > + return -EAGAIN;
>
> Why? if -EPERM return -EAGAIN?
>
> > + }
> > +
> > + /* Do we have proper ST write function? */
> > + if (ti_st_proto.write !=3D NULL) {
> > + /* We need this pointer for sending any Bluetooth pkts */
> > + hst->st_write =3D ti_st_proto.write;
>
> I asked in the other e-mail: Who sets ti_st_proto.write()? I didn't get
> this.
The write function is set by the TI-ST driver.
This was in order to avoid another EXPORT_SYMBOL(st_write) or so.
So, as soon as some protocol driver registers to shared transport driver, t=
he
registration function inside the driver will set it's write function to thi=
s
function pointer.
Yes, I will fix up the rest and post another version,
Thanks.
> > + } else {
> > + BT_ERR("failed to get ST write func pointer");
> > +
> > + /* Undo registration with ST */
> > + err =3D st_unregister(ST_BT);
> > + if (err)
> > + BT_ERR("st_unregister failed %d", err);
> > +
> > + hst->st_write =3D NULL;
> > + return -EAGAIN;
>
> return err; instead
>
> > + }
> > +
> > + /* Registration with ST layer is completed successfully,
> > + * now chip is ready to accept commands from HCI CORE.
> > + * Mark HCI Device flag as RUNNING
> > + */
> > + set_bit(HCI_RUNNING, &hdev->flags);
> > + return err;
> > +}
> > +
> > +/* Close device */
> > +static int ti_st_close(struct hci_dev *hdev)
> > +{
> > + int err =3D 0;
>
> do not set err to 0 here.
>
> > + struct ti_st *hst;
>
> you can set hst to hdev->.... here.
>
> > +
> > + hst =3D hdev->driver_data;
> > +
> > + /* Clear HCI device RUNNING flag */
> > + if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
> > + BT_ERR("HCI not RUNNING?");
>
>
> You didn't fix the things I told you. This is not a error. Just return 0
> here.
>
> > +
> > + /* continue to unregister from transport */
> > + err =3D st_unregister(ST_BT);
> > + if (err)
> > + BT_ERR("st_unregister failed %d", err);
> > +
> > + hst->st_write =3D NULL;
> > + return err;
> > +}
> > +
> > +/* Called from HCI CORE , Sends frames to Shared Transport */
> > +static int ti_st_send_frame(struct sk_buff *skb)
> > +{
> > + struct hci_dev *hdev;
> > + struct ti_st *hst;
> > + long len;
> > +
> > + if (!skb)
> > + return -ENOMEM;
> > +
> > + hdev =3D (struct hci_dev *)skb->dev;
> > + if (!hdev)
> > + return -ENODEV;
> > +
> > + if (!test_bit(HCI_RUNNING, &hdev->flags))
> > + return -EBUSY;
> > +
> > + hst =3D (struct ti_st *)hdev->driver_data;
> > +
> > + /* Prepend skb with frame type */
> > + memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
> > +
> > + BT_DBG(" %s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type,
> > + 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.
> > + */
> > + if (!hst->st_write) {
> > + kfree_skb(skb);
> > + BT_ERR(" Can't write to ST, st_write null?");
> > + return -EAGAIN;
> > + }
> > +
> > + len =3D hst->st_write(skb);
> > + if (len < 0) {
> > + kfree_skb(skb);
> > + BT_ERR(" ST write failed (%ld)", len);
> > + return -EAGAIN;
> > + }
>
> Explain to me why it is worth return -EAGAIN in both cases.
>
> > +
> > + /* ST accepted our skb. So, Go ahead and do rest */
> > + hdev->stat.byte_tx +=3D len;
> > + ti_st_tx_complete(hst, bt_cb(skb)->pkt_type);
> > +
> > + return 0;
> > +}
> > +
> > +static void ti_st_destruct(struct hci_dev *hdev)
> > +{
> > + if (!hdev)
> > + BT_ERR("Destruct called with invalid HCI Device"
> > + "(hdev=3DNULL)");
> > +
> > + BT_DBG("%s", hdev->name);
> > +
> > + /* free ti_st memory */
> > + kfree(hdev->driver_data);
> > + return;
> > +}
> > +
> > +/* Creates new HCI device */
> > +static int ti_st_register_dev(struct ti_st *hst)
> > +{
> > + struct hci_dev *hdev;
> > +
> > + /* Initialize and register HCI device */
> > + hdev =3D hci_alloc_dev();
> > + if (!hdev)
> > + return -ENOMEM;
> > +
> > + BT_DBG(" HCI device allocated. hdev=3D %p", hdev);
>
> just BT_DBG("hdev=3D %p", hdev);
>
> > +
> > + hst->hdev =3D hdev;
> > + hdev->bus =3D HCI_UART;
> > + hdev->driver_data =3D hst;
> > + hdev->open =3D ti_st_open;
> > + hdev->close =3D ti_st_close;
> > + hdev->flush =3D NULL;
> > + hdev->send =3D ti_st_send_frame;
> > + hdev->destruct =3D ti_st_destruct;
> > + hdev->owner =3D THIS_MODULE;
> > +
> > + if (reset)
> > + set_bit(HCI_QUIRK_NO_RESET, &hdev->quirks);
> > +
> > + if (hci_register_dev(hdev) < 0) {
>
> err =3D hci_register_dev()
> if (err < 0)
> ...
> ...
> return err;
>
> > + BT_ERR("Can't register HCI device");
> > + hci_free_dev(hdev);
> > + return -ENODEV;
> > + }
> > +
> > + BT_DBG(" HCI device registered. hdev=3D %p", hdev);
> > + return 0;
> > +}
> > +
> > +
> > +static int bt_ti_probe(struct platform_device *pdev)
> > +{
> > + int err;
> > + static struct ti_st *hst;
> > + err =3D 0;
> > +
> > + BT_DBG(" Bluetooth Driver Version %s", VERSION);
> > +
> > + hst =3D kzalloc(sizeof(struct ti_st), GFP_KERNEL);
> > + if (!hst) {
> > + BT_ERR("Can't allocate control structure");
>
> remove this BT_ERR.
>
> > + return -ENOMEM;
> > + }
> > +
> > + /* Expose "hciX" device to user space */
> > + err =3D ti_st_register_dev(hst);
> > + if (err) {
> > + kfree(hst);
> > + BT_ERR("Unable to expose hciX device(%d)", err);
>
> Ok, if you don't know how to extract the hci id, remove the whole error
> message then.
>
> > + return err;
> > + }
> > +
> > + dev_set_drvdata(&pdev->dev, hst);
> > + return err;
> > +}
> > +
> > +static int bt_ti_remove(struct platform_device *pdev)
> > +{
> > + struct ti_st *hst;
> > +
> > + hst =3D dev_get_drvdata(&pdev->dev);
> > + /* Deallocate local resource's memory */
> > + if (hst) {
> > + struct hci_dev *hdev =3D hst->hdev;
> > + if (!hdev) {
> > + BT_ERR("Invalid hdev memory");
> > + kfree(hst);
> > + } else {
> > + ti_st_close(hdev);
> > + hci_unregister_dev(hdev);
> > + /* Free HCI device memory */
> > + hci_free_dev(hdev);
> > + }
> > + }
> > + return 0;
> > +}
> > +
> > +static struct platform_driver btwilink_driver =3D {
> > + .probe =3D bt_ti_probe,
> > + .remove =3D bt_ti_remove,
> > + .driver =3D {
> > + .name =3D "btwilink",
> > + .owner =3D THIS_MODULE,
> > + },
> > +};
> > +
> > +/* ------- Module Init/Exit interfaces ------ */
> > +static int __init bt_drv_init(void)
> > +{
> > + long ret;
> > +
> > + ret =3D platform_driver_register(&btwilink_driver);
> > + if (ret !=3D 0) {
> > + BT_ERR("btwilink platform drv registration failed");
> > + return -EPERM;
> > + }
> > + return 0;
>
> if (ret)
> BT_ERR("btwilink platform drv registration failed");
>
> return ret;
>
> and s/drv/driver/
>
>
> Please fix all the issues, then we go to another round of review. ;)
>
>
> --
> Gustavo F. Padovan
> ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* Re: pull request: bluetooth-2.6 2010-10-18
From: John W. Linville @ 2010-10-18 20:08 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: marcel, linux-wireless, linux-bluetooth, davem
In-Reply-To: <20101018055540.GA13511@vigoh>
On Mon, Oct 18, 2010 at 03:55:40AM -0200, Gustavo F. Padovan wrote:
> Hi John,
>
> Not sure if we still have time for a last minute fix, but here goes a
> fix for a NULL dereference in L2CAP layer for 2.6.36-rc8. Please also let
> me know if this is the right way to handle such last minute fix. Maybe you
> should want this directly through Dave.
>
> Thanks.
The fix seems fine to me. Given the short schedule, it might be best
to skip the middleman and have Dave pull directly from you?
Dave, be forewarned that Gustavo has based his tree off 2.6.36-rc8 --
dunno if you are ready to pull that into net-2.6 or not.
John
> Linux 2.6.36-rc8 (2010-10-14 16:26:43 -0700)
>
> are available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/padovan/bluetooth-2.6.git master
>
> Nathan Holstein (1):
> Bluetooth: fix oops in l2cap_connect_req
>
> net/bluetooth/l2cap.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> --
> Gustavo F. Padovan
> ProFUSION embedded systems - http://profusion.mobi
>
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply
* Re: [PATCH] Bluetooth: btwilink driver
From: Gustavo F. Padovan @ 2010-10-18 20:10 UTC (permalink / raw)
To: Savoy, Pavan
Cc: marcel@holtmann.org, linux-bluetooth@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <19F8576C6E063C45BE387C64729E739404AA4E759A@dbde02.ent.ti.com>
* Savoy, Pavan <pavan_savoy@ti.com> [2010-10-19 01:23:54 +0530]:
>
> > -----Original Message-----
> > From: Gustavo F. Padovan [mailto:pao@profusion.mobi] On Behalf Of Gustavo F.
> > Padovan
> > Sent: Monday, October 18, 2010 2:48 PM
> > To: Savoy, Pavan
> > Cc: marcel@holtmann.org; linux-bluetooth@vger.kernel.org; linux-
> > kernel@vger.kernel.org
> > Subject: Re: [PATCH] Bluetooth: btwilink driver
> >
> > Hi Pavan,
> >
> > * pavan_savoy@ti.com <pavan_savoy@ti.com> [2010-10-15 16:58:19 -0400]:
> >
> > > From: Pavan Savoy <pavan_savoy@ti.com>
> > >
> > > Gustavo, Marcel,
> > >
> > > Renaming the patch, since the driver is renamed to btwilink.
> > > Thanks for the comments,
> > > Please review and provide your comments on this version of patch,
> > >
> > > -- patch description --
> > >
> > > This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> > > Texas Instrument's WiLink chipsets combine wireless technologies
> > > like BT, FM, GPS and WLAN onto a single chip.
> > >
> > > This Bluetooth driver works on top of the TI_ST shared transport
> > > line discipline driver which also allows other drivers like
> > > FM V4L2 and GPS character driver to make use of the same UART interface.
> > >
> > > Kconfig and Makefile modifications to enable the Bluetooth
> > > driver for Texas Instrument's WiLink 7 chipset.
> > >
> > > Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> > > ---
> > > drivers/bluetooth/Kconfig | 10 +
> > > drivers/bluetooth/Makefile | 1 +
> > > drivers/bluetooth/btwilink.c | 424
> > ++++++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 435 insertions(+), 0 deletions(-)
> > > create mode 100644 drivers/bluetooth/btwilink.c
> > >
> > > diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> > > index 02deef4..e0d67dd 100644
> > > --- a/drivers/bluetooth/Kconfig
> > > +++ b/drivers/bluetooth/Kconfig
> > > @@ -219,4 +219,14 @@ 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 "BlueZ bluetooth driver for TI ST"
> > > + 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.
> > > endmenu
> > > diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> > > index 71bdf13..f4460f4 100644
> > > --- a/drivers/bluetooth/Makefile
> > > +++ b/drivers/bluetooth/Makefile
> > > @@ -18,6 +18,7 @@ obj-$(CONFIG_BT_HCIBTSDIO) += btsdio.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
> > >
> > > btmrvl-y := btmrvl_main.o
> > > btmrvl-$(CONFIG_DEBUG_FS) += btmrvl_debugfs.o
> > > diff --git a/drivers/bluetooth/btwilink.c b/drivers/bluetooth/btwilink.c
> > > new file mode 100644
> > > index 0000000..f67791f
> > > --- /dev/null
> > > +++ b/drivers/bluetooth/btwilink.c
> > > @@ -0,0 +1,424 @@
> > > +/*
> > > + * 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 <raja_mani@ti.com>
> > > + * Pavan Savoy <pavan_savoy@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.
> > > + *
> > > + * You should have received a copy of the GNU General Public License
> > > + * along with this program; if not, write to the Free Software
> > > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
> > USA
> > > + *
> > > + */
> > > +
> > > +#include <linux/platform_device.h>
> > > +#include <net/bluetooth/bluetooth.h>
> > > +#include <net/bluetooth/hci_core.h>
> > > +
> > > +#include <linux/ti_wilink_st.h>
> > > +
> > > +/* Bluetooth Driver Version */
> > > +#define VERSION "1.0"
> > > +
> > > +/* Defines number of seconds to wait for reg completion
> > > + * callback getting called from ST (in case,registration
> > > + * with ST returns PENDING status)
> > > + */
> > > +#define BT_REGISTER_TIMEOUT 6000 /* 6 sec */
> > > +
> > > +/**
> > > + * struct ti_st - BT driver operation structure
> > > + * @hdev: hci device pointer which binds to bt driver
> > > + * @flags: used locally,to maintain various BT driver status
> > > + * @streg_cbdata: to hold ST registration callback status
> > > + * @st_write: write function pointer of ST driver
> > > + * @wait_reg_completion - completion sync between ti_st_open
> > > + * and ti_st_registration_completion_cb.
> > > + */
> > > +struct ti_st {
> > > + struct hci_dev *hdev;
> > > + char streg_cbdata;
> > > + long (*st_write) (struct sk_buff *);
> > > + struct completion wait_reg_completion;
> > > +};
> > > +
> > > +static int reset;
> > > +
> > > +/* 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;
> > > + 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_registration_completion_cb(void *priv_data, char data)
> > > +{
> > > + struct ti_st *lhst = (struct ti_st *)priv_data;
> >
> > Blank line here.
> >
> > > + /* ti_st_open() function needs value of 'data' to know
> > > + * the registration status(success/fail),So have a back
> > > + * up of it.
> > > + */
> > > + lhst->streg_cbdata = data;
> > > +
> > > + /* Got a feedback from ST for BT driver registration
> > > + * request.Wackup ti_st_open() function to continue
> > > + * it's open operation.
> > > + */
> > > + 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)
> > > +{
> > > + int err;
> > > + struct ti_st *lhst = (struct ti_st *)priv_data;
> > > +
> > > + if (!skb)
> > > + return -EFAULT;
> > > +
> > > + if (!lhst) {
> > > + kfree_skb(skb);
> > > + return -EFAULT;
> > > + }
> > > +
> > > + skb->dev = (struct net_device *)lhst->hdev;
> > > +
> > > + /* Forward skb to HCI CORE layer */
> > > + err = hci_recv_frame(skb);
> > > + if (err) {
> > > + kfree_skb(skb);
> > > + BT_ERR("Unable to push skb to HCI CORE(%d)", err);
> >
> > s/CORE/core/
> >
> > > + 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 = {
> > > + .type = ST_BT,
> > > + .recv = st_receive,
> > > + .reg_complete_cb = st_registration_completion_cb,
> > > + .priv_data = NULL,
> > > +};
> > > +
> > > +/* 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;
> > > +
> > > + BT_DBG("%s %p", hdev->name, hdev);
> > > +
> > > + /* provide contexts for callbacks from ST */
> > > + hst = hdev->driver_data;
> > > + ti_st_proto.priv_data = hst;
> > > +
> > > + err = st_register(&ti_st_proto);
> > > + if (err == -EINPROGRESS) {
> > > + /* Prepare wait-for-completion handler data structures.
> > > + * Needed to syncronize this and st_registration_completion_cb()
> > > + * functions.
> > > + */
> > > + init_completion(&hst->wait_reg_completion);
> > > +
> > > + /* Reset ST registration callback status flag , this value
> > > + * will be updated in ti_st_registration_completion_cb()
> > > + * function whenever it called from ST driver.
> > > + */
> > > + hst->streg_cbdata = -EINPROGRESS;
> > > +
> > > + /* ST is busy with other protocol registration(may be busy with
> > > + * firmware download).So,Wait till the registration callback
> > > + * (passed as a argument to st_register() function) getting
> > > + * called from ST.
> > > + */
> > > + BT_DBG(" waiting for reg 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);
> >
> > How does this get printed. "...regcompletion..." ?
> >
> > > + return -ETIMEDOUT;
> > > + }
> > > +
> > > + /* Is ST registration callback called with ERROR value? */
> > > + if (hst->streg_cbdata != 0) {
> > > + BT_ERR("ST reg completion CB called with invalid"
> > > + "status %d", hst->streg_cbdata);
> > > + return -EAGAIN;
> > > + }
> > > + err = 0;
> > > + } else if (err == -EPERM) {
> > > + BT_ERR("st_register failed %d", err);
> > > + return -EAGAIN;
> >
> > Why? if -EPERM return -EAGAIN?
> >
> > > + }
> > > +
> > > + /* Do we have proper ST write function? */
> > > + if (ti_st_proto.write != NULL) {
> > > + /* We need this pointer for sending any Bluetooth pkts */
> > > + hst->st_write = ti_st_proto.write;
> >
> > I asked in the other e-mail: Who sets ti_st_proto.write()? I didn't get
> > this.
>
> The write function is set by the TI-ST driver.
> This was in order to avoid another EXPORT_SYMBOL(st_write) or so.
> So, as soon as some protocol driver registers to shared transport driver, the
> registration function inside the driver will set it's write function to this
> function pointer.
The usual approach is to export the symbol and use it inside your
driver. That is what most of our drivers do (if not all of them).
So I recommend you to change that in your driver.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* RE: [PATCH] Bluetooth: btwilink driver
From: Savoy, Pavan @ 2010-10-18 20:14 UTC (permalink / raw)
To: Gustavo F. Padovan
Cc: marcel@holtmann.org, linux-bluetooth@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20101018201014.GH2468@vigoh>
Gustavo,
> -----Original Message-----
> From: Gustavo F. Padovan [mailto:pao@profusion.mobi] On Behalf Of Gustavo=
F.
> Padovan
> Sent: Monday, October 18, 2010 3:10 PM
> To: Savoy, Pavan
> Cc: marcel@holtmann.org; linux-bluetooth@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH] Bluetooth: btwilink driver
>=20
> * Savoy, Pavan <pavan_savoy@ti.com> [2010-10-19 01:23:54 +0530]:
>=20
> >
> > > -----Original Message-----
> > > From: Gustavo F. Padovan [mailto:pao@profusion.mobi] On Behalf Of Gus=
tavo
> F.
> > > Padovan
> > > Sent: Monday, October 18, 2010 2:48 PM
> > > To: Savoy, Pavan
> > > Cc: marcel@holtmann.org; linux-bluetooth@vger.kernel.org; linux-
> > > kernel@vger.kernel.org
> > > Subject: Re: [PATCH] Bluetooth: btwilink driver
> > >
> > > Hi Pavan,
> > >
> > > * pavan_savoy@ti.com <pavan_savoy@ti.com> [2010-10-15 16:58:19 -0400]=
:
> > >
> > > > From: Pavan Savoy <pavan_savoy@ti.com>
> > > >
> > > > Gustavo, Marcel,
> > > >
> > > > Renaming the patch, since the driver is renamed to btwilink.
> > > > Thanks for the comments,
> > > > Please review and provide your comments on this version of patch,
> > > >
> > > > -- patch description --
> > > >
> > > > This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> > > > Texas Instrument's WiLink chipsets combine wireless technologies
> > > > like BT, FM, GPS and WLAN onto a single chip.
> > > >
> > > > This Bluetooth driver works on top of the TI_ST shared transport
> > > > line discipline driver which also allows other drivers like
> > > > FM V4L2 and GPS character driver to make use of the same UART inter=
face.
> > > >
> > > > Kconfig and Makefile modifications to enable the Bluetooth
> > > > driver for Texas Instrument's WiLink 7 chipset.
> > > >
> > > > Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> > > > ---
> > > > drivers/bluetooth/Kconfig | 10 +
> > > > drivers/bluetooth/Makefile | 1 +
> > > > drivers/bluetooth/btwilink.c | 424
> > > ++++++++++++++++++++++++++++++++++++++++++
> > > > 3 files changed, 435 insertions(+), 0 deletions(-)
> > > > create mode 100644 drivers/bluetooth/btwilink.c
> > > >
> > > > diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> > > > index 02deef4..e0d67dd 100644
> > > > --- a/drivers/bluetooth/Kconfig
> > > > +++ b/drivers/bluetooth/Kconfig
> > > > @@ -219,4 +219,14 @@ 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 "BlueZ bluetooth driver for TI ST"
> > > > + depends on TI_ST
> > > > + help
> > > > + This enables the Bluetooth driver for Texas Instrument's BT/F=
M/GPS
> > > > + combo devices. This makes use of shared transport line discip=
line
> > > > + 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.
> > > > endmenu
> > > > diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefil=
e
> > > > index 71bdf13..f4460f4 100644
> > > > --- a/drivers/bluetooth/Makefile
> > > > +++ b/drivers/bluetooth/Makefile
> > > > @@ -18,6 +18,7 @@ obj-$(CONFIG_BT_HCIBTSDIO) +=3D btsdio.o
> > > > obj-$(CONFIG_BT_ATH3K) +=3D ath3k.o
> > > > obj-$(CONFIG_BT_MRVL) +=3D btmrvl.o
> > > > obj-$(CONFIG_BT_MRVL_SDIO) +=3D btmrvl_sdio.o
> > > > +obj-$(CONFIG_BT_WILINK) +=3D btwilink.o
> > > >
> > > > btmrvl-y :=3D btmrvl_main.o
> > > > btmrvl-$(CONFIG_DEBUG_FS) +=3D btmrvl_debugfs.o
> > > > diff --git a/drivers/bluetooth/btwilink.c b/drivers/bluetooth/btwil=
ink.c
> > > > new file mode 100644
> > > > index 0000000..f67791f
> > > > --- /dev/null
> > > > +++ b/drivers/bluetooth/btwilink.c
> > > > @@ -0,0 +1,424 @@
> > > > +/*
> > > > + * 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 <raja_mani@ti.com>
> > > > + * Pavan Savoy <pavan_savoy@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.
> > > > + *
> > > > + * You should have received a copy of the GNU General Public Lice=
nse
> > > > + * along with this program; if not, write to the Free Software
> > > > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 0211=
1-
> 1307
> > > USA
> > > > + *
> > > > + */
> > > > +
> > > > +#include <linux/platform_device.h>
> > > > +#include <net/bluetooth/bluetooth.h>
> > > > +#include <net/bluetooth/hci_core.h>
> > > > +
> > > > +#include <linux/ti_wilink_st.h>
> > > > +
> > > > +/* Bluetooth Driver Version */
> > > > +#define VERSION "1.0"
> > > > +
> > > > +/* Defines number of seconds to wait for reg completion
> > > > + * callback getting called from ST (in case,registration
> > > > + * with ST returns PENDING status)
> > > > + */
> > > > +#define BT_REGISTER_TIMEOUT 6000 /* 6 sec */
> > > > +
> > > > +/**
> > > > + * struct ti_st - BT driver operation structure
> > > > + * @hdev: hci device pointer which binds to bt driver
> > > > + * @flags: used locally,to maintain various BT driver status
> > > > + * @streg_cbdata: to hold ST registration callback status
> > > > + * @st_write: write function pointer of ST driver
> > > > + * @wait_reg_completion - completion sync between ti_st_open
> > > > + * and ti_st_registration_completion_cb.
> > > > + */
> > > > +struct ti_st {
> > > > + struct hci_dev *hdev;
> > > > + char streg_cbdata;
> > > > + long (*st_write) (struct sk_buff *);
> > > > + struct completion wait_reg_completion;
> > > > +};
> > > > +
> > > > +static int reset;
> > > > +
> > > > +/* Increments HCI counters based on pocket ID (cmd,acl,sco) */
> > > > +static inline void ti_st_tx_complete(struct ti_st *hst, int pkt_ty=
pe)
> > > > +{
> > > > + struct hci_dev *hdev;
> > > > + hdev =3D 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_registration_completion_cb(void *priv_data, char da=
ta)
> > > > +{
> > > > + struct ti_st *lhst =3D (struct ti_st *)priv_data;
> > >
> > > Blank line here.
> > >
> > > > + /* ti_st_open() function needs value of 'data' to know
> > > > + * the registration status(success/fail),So have a back
> > > > + * up of it.
> > > > + */
> > > > + lhst->streg_cbdata =3D data;
> > > > +
> > > > + /* Got a feedback from ST for BT driver registration
> > > > + * request.Wackup ti_st_open() function to continue
> > > > + * it's open operation.
> > > > + */
> > > > + 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)
> > > > +{
> > > > + int err;
> > > > + struct ti_st *lhst =3D (struct ti_st *)priv_data;
> > > > +
> > > > + if (!skb)
> > > > + return -EFAULT;
> > > > +
> > > > + if (!lhst) {
> > > > + kfree_skb(skb);
> > > > + return -EFAULT;
> > > > + }
> > > > +
> > > > + skb->dev =3D (struct net_device *)lhst->hdev;
> > > > +
> > > > + /* Forward skb to HCI CORE layer */
> > > > + err =3D hci_recv_frame(skb);
> > > > + if (err) {
> > > > + kfree_skb(skb);
> > > > + BT_ERR("Unable to push skb to HCI CORE(%d)", err);
> > >
> > > s/CORE/core/
> > >
> > > > + return err;
> > > > + }
> > > > +
> > > > + lhst->hdev->stat.byte_rx +=3D skb->len;
> > > > + return 0;
> > > > +}
> > > > +
> > > > +/* ------- Interfaces to HCI layer ------ */
> > > > +/* protocol structure registered with shared transport */
> > > > +static struct st_proto_s ti_st_proto =3D {
> > > > + .type =3D ST_BT,
> > > > + .recv =3D st_receive,
> > > > + .reg_complete_cb =3D st_registration_completion_cb,
> > > > + .priv_data =3D NULL,
> > > > +};
> > > > +
> > > > +/* 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;
> > > > +
> > > > + BT_DBG("%s %p", hdev->name, hdev);
> > > > +
> > > > + /* provide contexts for callbacks from ST */
> > > > + hst =3D hdev->driver_data;
> > > > + ti_st_proto.priv_data =3D hst;
> > > > +
> > > > + err =3D st_register(&ti_st_proto);
> > > > + if (err =3D=3D -EINPROGRESS) {
> > > > + /* Prepare wait-for-completion handler data structures.
> > > > + * Needed to syncronize this and
> st_registration_completion_cb()
> > > > + * functions.
> > > > + */
> > > > + init_completion(&hst->wait_reg_completion);
> > > > +
> > > > + /* Reset ST registration callback status flag , this va=
lue
> > > > + * will be updated in ti_st_registration_completion_cb(=
)
> > > > + * function whenever it called from ST driver.
> > > > + */
> > > > + hst->streg_cbdata =3D -EINPROGRESS;
> > > > +
> > > > + /* ST is busy with other protocol registration(may be b=
usy
> with
> > > > + * firmware download).So,Wait till the registration cal=
lback
> > > > + * (passed as a argument to st_register() function) get=
ting
> > > > + * called from ST.
> > > > + */
> > > > + BT_DBG(" waiting for reg completion signal from ST");
> > > > +
> > > > + timeleft =3D 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);
> > >
> > > How does this get printed. "...regcompletion..." ?
> > >
> > > > + return -ETIMEDOUT;
> > > > + }
> > > > +
> > > > + /* Is ST registration callback called with ERROR value?=
*/
> > > > + if (hst->streg_cbdata !=3D 0) {
> > > > + BT_ERR("ST reg completion CB called with invali=
d"
> > > > + "status %d", hst->streg_cbdata)=
;
> > > > + return -EAGAIN;
> > > > + }
> > > > + err =3D 0;
> > > > + } else if (err =3D=3D -EPERM) {
> > > > + BT_ERR("st_register failed %d", err);
> > > > + return -EAGAIN;
> > >
> > > Why? if -EPERM return -EAGAIN?
> > >
> > > > + }
> > > > +
> > > > + /* Do we have proper ST write function? */
> > > > + if (ti_st_proto.write !=3D NULL) {
> > > > + /* We need this pointer for sending any Bluetooth pkts =
*/
> > > > + hst->st_write =3D ti_st_proto.write;
> > >
> > > I asked in the other e-mail: Who sets ti_st_proto.write()? I didn't =
get
> > > this.
> >
> > The write function is set by the TI-ST driver.
> > This was in order to avoid another EXPORT_SYMBOL(st_write) or so.
> > So, as soon as some protocol driver registers to shared transport drive=
r,
> the
> > registration function inside the driver will set it's write function to=
this
> > function pointer.
>=20
> The usual approach is to export the symbol and use it inside your
> driver. That is what most of our drivers do (if not all of them).
> So I recommend you to change that in your driver.
Yes, I had it like that.
However only the register/unregister functions + write function had the=20
EXPORT_SYMBOL and "receive" function didn't, since each driver had to send =
its own receive function.
So make things similar based on review comments, changed this to having
pointers for the write and read, keep the exported symbols for register/unr=
egister.
> --
> Gustavo F. Padovan
> ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* [PATCH v2] Bluetooth: btwilink driver
From: pavan_savoy @ 2010-10-18 22:34 UTC (permalink / raw)
To: padovan, marcel; +Cc: linux-bluetooth, linux-kernel, Pavan Savoy
From: Pavan Savoy <pavan_savoy@ti.com>
v2 comments
Review comments about blank lines, error message, CORE->core,
drv->driver, returning err, not initializing err have been taken care.
a. The HCI not running error during ti_st_close is seen
when I try and rmmod when hci0 is up, and hence thought of
keeping it in prevous version - But removed now.
b. The error codes returned in the send_frame don't seem to matter
since they aren't considered in hci_core.
So returning err as we receive it from TI-ST driver now.
c. Even if we return different error code in hci's open device, EIO
is returned from the hci_core, we have different error codes
in TI-ST driver because we want to,
1. prevent same protocol from registering twice - EAGAIN
2. it is not really an error if we return EINPROGRESS since it means
firmware download is in progress because other protocol requested it
and the transport will soon be available.
Please review & Thanks,
Pavan
-- patch description --
This is the bluetooth protocol driver for the TI WiLink7 chipsets.
Texas Instrument's WiLink chipsets combine wireless technologies
like BT, FM, GPS and WLAN onto a single chip.
This Bluetooth driver works on top of the TI_ST shared transport
line discipline driver which also allows other drivers like
FM V4L2 and GPS character driver to make use of the same UART interface.
Kconfig and Makefile modifications to enable the Bluetooth
driver for Texas Instrument's WiLink 7 chipset.
Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
---
drivers/bluetooth/Kconfig | 10 +
drivers/bluetooth/Makefile | 1 +
drivers/bluetooth/btwilink.c | 417 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 428 insertions(+), 0 deletions(-)
create mode 100644 drivers/bluetooth/btwilink.c
diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index 02deef4..e0d67dd 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -219,4 +219,14 @@ 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 "BlueZ bluetooth driver for TI ST"
+ 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.
endmenu
diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
index 71bdf13..f4460f4 100644
--- a/drivers/bluetooth/Makefile
+++ b/drivers/bluetooth/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_BT_HCIBTSDIO) += btsdio.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
btmrvl-y := btmrvl_main.o
btmrvl-$(CONFIG_DEBUG_FS) += btmrvl_debugfs.o
diff --git a/drivers/bluetooth/btwilink.c b/drivers/bluetooth/btwilink.c
new file mode 100644
index 0000000..e6e2e64
--- /dev/null
+++ b/drivers/bluetooth/btwilink.c
@@ -0,0 +1,417 @@
+/*
+ * 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 <raja_mani@ti.com>
+ * Pavan Savoy <pavan_savoy@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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#include <linux/platform_device.h>
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci_core.h>
+
+#include <linux/ti_wilink_st.h>
+
+/* Bluetooth Driver Version */
+#define VERSION "1.0"
+
+/* Defines number of seconds to wait for reg completion
+ * callback getting called from ST (in case,registration
+ * with ST returns PENDING status)
+ */
+#define BT_REGISTER_TIMEOUT 6000 /* 6 sec */
+
+/**
+ * struct ti_st - BT driver operation structure
+ * @hdev: hci device pointer which binds to bt driver
+ * @flags: used locally,to maintain various BT driver status
+ * @streg_cbdata: to hold ST registration callback status
+ * @st_write: write function pointer of ST driver
+ * @wait_reg_completion - completion sync between ti_st_open
+ * and ti_st_registration_completion_cb.
+ */
+struct ti_st {
+ struct hci_dev *hdev;
+ char streg_cbdata;
+ long (*st_write) (struct sk_buff *);
+ struct completion wait_reg_completion;
+};
+
+static int reset;
+
+/* 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;
+ 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_registration_completion_cb(void *priv_data, char data)
+{
+ struct ti_st *lhst = (struct ti_st *)priv_data;
+
+ /* ti_st_open() function needs value of 'data' to know
+ * the registration status(success/fail),So have a back
+ * up of it.
+ */
+ lhst->streg_cbdata = data;
+
+ /* Got a feedback from ST for BT driver registration
+ * request.Wackup ti_st_open() function to continue
+ * it's open operation.
+ */
+ 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)
+{
+ int err;
+ struct ti_st *lhst = (struct ti_st *)priv_data;
+
+ if (!skb)
+ return -EFAULT;
+
+ if (!lhst) {
+ kfree_skb(skb);
+ return -EFAULT;
+ }
+
+ skb->dev = (struct net_device *)lhst->hdev;
+
+ /* Forward skb to HCI core layer */
+ err = hci_recv_frame(skb);
+ if (err) {
+ kfree_skb(skb);
+ 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 = {
+ .type = ST_BT,
+ .recv = st_receive,
+ .reg_complete_cb = st_registration_completion_cb,
+ .priv_data = NULL,
+};
+
+/* 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;
+
+ BT_DBG("%s %p", hdev->name, hdev);
+
+ /* provide contexts for callbacks from ST */
+ hst = hdev->driver_data;
+ ti_st_proto.priv_data = hst;
+
+ err = st_register(&ti_st_proto);
+ if (err == -EINPROGRESS) {
+ /* Prepare wait-for-completion handler data structures.
+ * Needed to syncronize this and st_registration_completion_cb()
+ * functions.
+ */
+ init_completion(&hst->wait_reg_completion);
+
+ /* Reset ST registration callback status flag , this value
+ * will be updated in ti_st_registration_completion_cb()
+ * function whenever it called from ST driver.
+ */
+ hst->streg_cbdata = -EINPROGRESS;
+
+ /* ST is busy with other protocol registration(may be busy with
+ * firmware download).So,Wait till the registration callback
+ * (passed as a argument to st_register() function) getting
+ * called from ST.
+ */
+ BT_DBG(" waiting for reg 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 value? */
+ if (hst->streg_cbdata != 0) {
+ BT_ERR("ST reg completion CB called with invalid"
+ "status %d", hst->streg_cbdata);
+ return -EAGAIN;
+ }
+ err = 0;
+ } else if (err == -EPERM) {
+ BT_ERR("st_register failed %d", err);
+ return err;
+ }
+
+ /* Do we have proper ST write function? */
+ if (ti_st_proto.write != NULL) {
+ /* We need this pointer for sending any Bluetooth pkts */
+ hst->st_write = ti_st_proto.write;
+ } else {
+ BT_ERR("failed to get ST write func pointer");
+
+ /* Undo registration with ST */
+ err = st_unregister(ST_BT);
+ if (err)
+ BT_ERR("st_unregister failed %d", err);
+
+ hst->st_write = NULL;
+ return err;
+ }
+
+ /* Registration with ST layer is completed successfully,
+ * now chip is ready to accept commands from HCI core.
+ * Mark HCI Device flag as RUNNING
+ */
+ set_bit(HCI_RUNNING, &hdev->flags);
+ return err;
+}
+
+/* Close device */
+static int ti_st_close(struct hci_dev *hdev)
+{
+ int err;
+ struct ti_st *hst = hdev->driver_data;
+
+ /* continue to unregister from transport */
+ err = st_unregister(ST_BT);
+ if (err)
+ BT_ERR("st_unregister failed %d", err);
+
+ hst->st_write = NULL;
+ return err;
+}
+
+/* Called from HCI core, Sends frames to Shared Transport */
+static int ti_st_send_frame(struct sk_buff *skb)
+{
+ struct hci_dev *hdev;
+ struct ti_st *hst;
+ long len;
+
+ if (!skb)
+ return -ENOMEM;
+
+ hdev = (struct hci_dev *)skb->dev;
+ if (!hdev)
+ return -ENODEV;
+
+ if (!test_bit(HCI_RUNNING, &hdev->flags))
+ return -EBUSY;
+
+ hst = (struct ti_st *)hdev->driver_data;
+
+ /* Prepend skb with frame type */
+ memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
+
+ BT_DBG(" %s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type,
+ 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.
+ */
+ if (!hst->st_write) {
+ kfree_skb(skb);
+ BT_ERR(" Can't write to ST, st_write null?");
+ return -EAGAIN;
+ }
+
+ len = hst->st_write(skb);
+ if (len < 0) {
+ kfree_skb(skb);
+ BT_ERR(" ST write failed (%ld)", len);
+ return -EAGAIN;
+ }
+
+ /* ST accepted our skb. So, Go ahead and do rest */
+ hdev->stat.byte_tx += len;
+ ti_st_tx_complete(hst, bt_cb(skb)->pkt_type);
+
+ return 0;
+}
+
+static void ti_st_destruct(struct hci_dev *hdev)
+{
+ if (!hdev)
+ BT_ERR("Destruct called with invalid HCI Device"
+ "(hdev=NULL)");
+
+ BT_DBG("%s", hdev->name);
+
+ /* free ti_st memory */
+ kfree(hdev->driver_data);
+ return;
+}
+
+/* Creates new HCI device */
+static int ti_st_register_dev(struct ti_st *hst)
+{
+ int err;
+ struct hci_dev *hdev;
+
+ /* Initialize and register HCI device */
+ hdev = hci_alloc_dev();
+ if (!hdev)
+ return -ENOMEM;
+
+ BT_DBG("hdev= %p", hdev);
+
+ hst->hdev = hdev;
+ hdev->bus = HCI_UART;
+ hdev->driver_data = hst;
+ hdev->open = ti_st_open;
+ hdev->close = ti_st_close;
+ hdev->flush = NULL;
+ hdev->send = ti_st_send_frame;
+ hdev->destruct = ti_st_destruct;
+ hdev->owner = THIS_MODULE;
+
+ if (reset)
+ set_bit(HCI_QUIRK_NO_RESET, &hdev->quirks);
+
+ err = hci_register_dev(hdev);
+ if (err < 0) {
+ BT_ERR("Can't register HCI device");
+ hci_free_dev(hdev);
+ return err;
+ }
+
+ BT_DBG(" HCI device registered. hdev= %p", hdev);
+ return 0;
+}
+
+
+static int bt_ti_probe(struct platform_device *pdev)
+{
+ int err;
+ static struct ti_st *hst;
+
+ BT_DBG(" Bluetooth Driver Version %s", VERSION);
+
+ hst = kzalloc(sizeof(struct ti_st), GFP_KERNEL);
+ if (!hst)
+ return -ENOMEM;
+
+ /* Expose "hciX" device to user space */
+ err = ti_st_register_dev(hst);
+ if (err) {
+ kfree(hst);
+ return err;
+ }
+
+ dev_set_drvdata(&pdev->dev, hst);
+ return err;
+}
+
+static int bt_ti_remove(struct platform_device *pdev)
+{
+ struct ti_st *hst;
+
+ hst = dev_get_drvdata(&pdev->dev);
+ /* Deallocate local resource's memory */
+ if (hst) {
+ struct hci_dev *hdev = hst->hdev;
+ if (!hdev) {
+ BT_ERR("Invalid hdev memory");
+ kfree(hst);
+ } else {
+ ti_st_close(hdev);
+ hci_unregister_dev(hdev);
+ /* Free HCI device memory */
+ hci_free_dev(hdev);
+ }
+ }
+ return 0;
+}
+
+static struct platform_driver btwilink_driver = {
+ .probe = bt_ti_probe,
+ .remove = bt_ti_remove,
+ .driver = {
+ .name = "btwilink",
+ .owner = THIS_MODULE,
+ },
+};
+
+/* ------- Module Init/Exit interfaces ------ */
+static int __init bt_drv_init(void)
+{
+ long ret;
+
+ ret = platform_driver_register(&btwilink_driver);
+ if (ret != 0) {
+ BT_ERR("btwilink platform driver registration failed");
+ return -EPERM;
+ }
+ return 0;
+}
+
+static void __exit bt_drv_exit(void)
+{
+ platform_driver_unregister(&btwilink_driver);
+}
+
+module_init(bt_drv_init);
+module_exit(bt_drv_exit);
+
+/* ------ Module Info ------ */
+
+module_param(reset, bool, 0644);
+MODULE_PARM_DESC(reset, "Send HCI reset command on initialization");
+MODULE_AUTHOR("Raja Mani <raja_mani@ti.com>");
+MODULE_DESCRIPTION("Bluetooth Driver for TI Shared Transport" VERSION);
+MODULE_VERSION(VERSION);
+MODULE_LICENSE("GPL");
--
1.6.5
^ permalink raw reply related
* AVRCP 1.4 : Future on Target Role
From: Shivendra Agrawal @ 2010-10-19 6:16 UTC (permalink / raw)
To: linux-bluetooth
Hi All,
I have been looking at AVRCP 1.4 in BlueZ and intending to
enhance/develop this profile for the target role. I have been
following the discussion initiated by Sander van Grieken earlier last
month, and as I understand, they are premitive and has scope for
further enhancements.
In the current BlueZ implementation, control-api.txt mentions few
methods e.g. SendVendoeDependent, ChangePlayback..., that are not
referred/implemented in the code, or I may be unable to find at right
place.
Further, there are some more AVRCP 1.4 TG specific Notify and Browsing
commands that can be added.
I am willing to define some preliminary interface APIs for Target
Media Applications to register itself with BlueZ, and would come back
with some idea proposal for your suggestions on improvements.
I am keen to receive your feedback with some ideas and thoughts to put
my effort in right direction.
Question:
Is anyone working on AVRCP 1.4 Target role profile development?
Regards,
Shivendra
^ permalink raw reply
* Re: AVRCP 1.4 : Future on Target Role
From: Luiz Augusto von Dentz @ 2010-10-19 8:31 UTC (permalink / raw)
To: Shivendra Agrawal; +Cc: linux-bluetooth
In-Reply-To: <AANLkTi=jnPSWAiYWWE6Z_oPswA-uXSOWWMfZKfcfgegH@mail.gmail.com>
Hi,
On Tue, Oct 19, 2010 at 9:16 AM, Shivendra Agrawal
<ag.shivendra@gmail.com> wrote:
> Hi All,
>
> I have been looking at AVRCP 1.4 in BlueZ and intending to
> enhance/develop this profile for the target role. I have been
> following the discussion initiated by Sander van Grieken earlier last
> month, and as I understand, they are premitive and has scope for
> further enhancements.
Joao Paulo also did some work in this area using MPRIS, I guess he
even publish his tree somewhere.
> In the current BlueZ implementation, control-api.txt mentions few
> methods e.g. SendVendoeDependent, ChangePlayback..., that are not
> referred/implemented in the code, or I may be unable to find at right
> place.
> Further, there are some more AVRCP 1.4 TG specific Notify and Browsing
> commands that can be added.
I would suggest you to take a look at media-api.txt, this is what we
area planning to use for streams and we should probably add support
for metadata and browsing (those 2 seems to be the most asked features
from avrcp nowadays). Actually metadata seems to fit nicely there, we
just need another interface to receive stream metadata, now browsing
is probably not so easy.
> I am willing to define some preliminary interface APIs for Target
> Media Applications to register itself with BlueZ, and would come back
> with some idea proposal for your suggestions on improvements.
If media players all agree on using MPRIS than we probably don't need
to have them registering to us, in the other hand Im not sure if MPRIS
API do cover everything in terms of avrcp, maybe you can figure this
out.
> I am keen to receive your feedback with some ideas and thoughts to put
> my effort in right direction.
>
> Question:
> Is anyone working on AVRCP 1.4 Target role profile development?
Joao Paulo (http://jprvita.wordpress.com/2010/07/22/avrcp-metadata/)
^ permalink raw reply
* Re: AVRCP 1.4 : Future on Target Role
From: Sander van Grieken @ 2010-10-19 9:47 UTC (permalink / raw)
To: linux-bluetooth
On Tuesday 19 October 2010 10:31:33 Luiz Augusto von Dentz wrote:
> Hi,
>
> On Tue, Oct 19, 2010 at 9:16 AM, Shivendra Agrawal
> <ag.shivendra@gmail.com> wrote:
> > Hi All,
> >
> > I have been looking at AVRCP 1.4 in BlueZ and intending to
> > enhance/develop this profile for the target role. I have been
> > following the discussion initiated by Sander van Grieken earlier last
> > month, and as I understand, they are premitive and has scope for
> > further enhancements.
>
> Joao Paulo also did some work in this area using MPRIS, I guess he
> even publish his tree somewhere.
Yes, it's at git://git.profusion.mobi/users/jprvita/bluez.git
> > In the current BlueZ implementation, control-api.txt mentions few
> > methods e.g. SendVendoeDependent, ChangePlayback..., that are not
> > referred/implemented in the code, or I may be unable to find at right
> > place.
Yes, the document is more like a proposal, than a description of the actual
implementation.
> > Further, there are some more AVRCP 1.4 TG specific Notify and Browsing
> > commands that can be added.
>
> I would suggest you to take a look at media-api.txt, this is what we
> area planning to use for streams and we should probably add support
> for metadata and browsing (those 2 seems to be the most asked features
> from avrcp nowadays). Actually metadata seems to fit nicely there, we
> just need another interface to receive stream metadata, now browsing
> is probably not so easy.
>
> > I am willing to define some preliminary interface APIs for Target
> > Media Applications to register itself with BlueZ, and would come back
> > with some idea proposal for your suggestions on improvements.
>
> If media players all agree on using MPRIS than we probably don't need
> to have them registering to us, in the other hand Im not sure if MPRIS
> API do cover everything in terms of avrcp, maybe you can figure this
> out.
I am very much in favor of not directly depending on MPRIS, but instead letting
applications registering themselves as a target. For two reasons:
- AVRCP seems to be a superset of MPRIS (which is very limited IMO), and might have
different semantics, especially w.r.t. event notifications. So we would limit ourselves to
the intersecting subset of both technologies.
- A separate AVRCP/TG <-> MPRIS bridge agent would still allow controlling all MPRIS-
enabled players, so we can have both full implementation of the AVRCP spec, AND generic
MPRIS support.
> > I am keen to receive your feedback with some ideas and thoughts to put
> > my effort in right direction.
> >
> > Question:
> > Is anyone working on AVRCP 1.4 Target role profile development?
>
> Joao Paulo (http://jprvita.wordpress.com/2010/07/22/avrcp-metadata/)
Actually, the metadata work is not part of v1.4 of the spec, but 1.3
Second, I have already added some boilerplate stuff (like a DBUS Connect method for RCP and
some fixes for CT commands), but I've based on Joao's branch, so I have to wait until his
stuff gets merged. Alternatively, I could rebase that stuff on HEAD, but that would overlap
and conflict with Joao's stuff, so I'm hesitant to go there.
Shivendra, before you start, let's sync so we don't duplicate efforts
grtz,
Sander
^ permalink raw reply
* [PATCH] Fix GUI crash while connecting Interlink Bluetooth Mouse in android platform.
From: Balamurugan Mahalingam @ 2010-10-19 9:54 UTC (permalink / raw)
To: marcel; +Cc: linux-bluetooth, charubala, Balamurugan Mahalingam
Crash is due to some junk characters in the device name.
The device name is copied from a string variable after freeing the memory it points to, so there were some junk characters in it which was the reason for the crash.
Fixing the issue by freeing the memory in string variable after device name is copied.
Signed-off-by: Balamurugan Mahalingam <mbalamurugan@atheros.com>
---
compat/sdp.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/compat/sdp.c b/compat/sdp.c
index 8898136..ff2e39f 100644
--- a/compat/sdp.c
+++ b/compat/sdp.c
@@ -152,7 +152,6 @@ int get_stored_device_info(const bdaddr_t *src, const bdaddr_t *dst, struct hidp
&vendor, &product, &version, &subclass, &country,
&parser, desc, &req->flags, &pos);
- free(str);
req->vendor = vendor;
req->product = product;
@@ -163,6 +162,7 @@ int get_stored_device_info(const bdaddr_t *src, const bdaddr_t *dst, struct hidp
snprintf(req->name, 128, "%s", str + pos);
+ free(str);
req->rd_size = strlen(desc) / 2;
req->rd_data = malloc(req->rd_size);
if (!req->rd_data) {
--
1.6.3.3
^ permalink raw reply related
* Re: [PATCH] Fix GUI crash while connecting Interlink Bluetooth Mouse in android platform.
From: Johan Hedberg @ 2010-10-19 11:23 UTC (permalink / raw)
To: Balamurugan Mahalingam; +Cc: marcel, linux-bluetooth, charubala
In-Reply-To: <1287482080-10093-1-git-send-email-mbalamurugan@atheros.com>
Hi,
On Tue, Oct 19, 2010, Balamurugan Mahalingam wrote:
> Crash is due to some junk characters in the device name.
> The device name is copied from a string variable after freeing the memory it points to, so there were some junk characters in it which was the reason for the crash.
> Fixing the issue by freeing the memory in string variable after device name is copied.
>
> Signed-off-by: Balamurugan Mahalingam <mbalamurugan@atheros.com>
> ---
> compat/sdp.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
Looks like a good fix, but could you please fix your commit message and
resend:
1. The commit message doesn't so much describe the change as it does
some symptom in a downstream system (android). E.g. the summary line
should be something like "Fix accessing freed memory". You can still
talk about the background to this issue wrt. android but the main point
should be in a bluez-only context.
2. Keep the commit message lines below 74 characters so that it's
properly viewable on a 80-character wide terminal when running "git log"
3. Remove the signed-off-by line. We don't use that in userspace.
Johan
^ permalink raw reply
* [PATCH] Fix regression when formatting timestamps from tracker
From: Luiz Augusto von Dentz @ 2010-10-19 11:53 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
Tracker format user delimiters like '-' and ':' which need to be keep in
order to sscanf to work.
---
plugins/phonebook-tracker.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index 2da825b..f0e00b8 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -705,18 +705,21 @@ static char *iso8601_utc_to_localtime(const char *datetime)
memset(&tm, 0, sizeof(tm));
- nr = sscanf(datetime, "%04u%02u%02uT%02u%02u%02u%c",
+ nr = sscanf(datetime, "%04u-%02u-%02uT%02u:%02u:%02u%c",
&tm.tm_year, &tm.tm_mon, &tm.tm_mday,
&tm.tm_hour, &tm.tm_min, &tm.tm_sec,
&tz);
if (nr < 6) {
/* Invalid time format */
+ error("sscanf(): %s (%d)", strerror(errno), errno);
return g_strdup("");
}
/* Time already in localtime */
- if (nr == 6)
- return g_strdup(datetime);
+ if (nr == 6) {
+ strftime(localdate, sizeof(localdate), "%Y%m%dT%H%M%S", &tm);
+ return g_strdup(localdate);
+ }
tm.tm_year -= 1900; /* Year since 1900 */
tm.tm_mon--; /* Months since January, values 0-11 */
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] Fix GUI crash while connecting Interlink Bluetooth Mouse in android platform.
From: Balamurugan Mahalingam @ 2010-10-19 12:01 UTC (permalink / raw)
To: Balamurugan Mahalingam, marcel, linux-bluetooth, charubala
In-Reply-To: <20101019112328.GA12554@jh-x301>
Johan Hedberg wrote:
> Hi,
>
> On Tue, Oct 19, 2010, Balamurugan Mahalingam wrote:
>
>> Crash is due to some junk characters in the device name.
>> The device name is copied from a string variable after freeing the memory it points to, so there were some junk characters in it which was the reason for the crash.
>> Fixing the issue by freeing the memory in string variable after device name is copied.
>>
>> Signed-off-by: Balamurugan Mahalingam <mbalamurugan@atheros.com>
>> ---
>> compat/sdp.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>
> Looks like a good fix, but could you please fix your commit message and
> resend:
>
> 1. The commit message doesn't so much describe the change as it does
> some symptom in a downstream system (android). E.g. the summary line
> should be something like "Fix accessing freed memory". You can still
> talk about the background to this issue wrt. android but the main point
> should be in a bluez-only context.
>
> 2. Keep the commit message lines below 74 characters so that it's
> properly viewable on a 80-character wide terminal when running "git log"
>
> 3. Remove the signed-off-by line. We don't use that in userspace.
>
> Johan
> .
>
>
Hi Johan,
Thanks for your comments. I will do the changes and send the patch once
again
Thanks
Balamurugan Mahalingam
^ permalink raw reply
* [PATCH] Fix accessing freed memory
From: Balamurugan Mahalingam @ 2010-10-19 12:30 UTC (permalink / raw)
To: marcel; +Cc: linux-bluetooth, charubala, Balamurugan Mahalingam
Crash is due to some junk characters in the device name.
The device name is copied from a string variable after freeing the memory
it points to, so there were some junk characters in it which was the reason
for the crash.
Fixing the issue by freeing the memory in string variable after
device name is copied.
---
compat/sdp.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/compat/sdp.c b/compat/sdp.c
index 8898136..ff2e39f 100644
--- a/compat/sdp.c
+++ b/compat/sdp.c
@@ -152,7 +152,6 @@ int get_stored_device_info(const bdaddr_t *src, const bdaddr_t *dst, struct hidp
&vendor, &product, &version, &subclass, &country,
&parser, desc, &req->flags, &pos);
- free(str);
req->vendor = vendor;
req->product = product;
@@ -163,6 +162,7 @@ int get_stored_device_info(const bdaddr_t *src, const bdaddr_t *dst, struct hidp
snprintf(req->name, 128, "%s", str + pos);
+ free(str);
req->rd_size = strlen(desc) / 2;
req->rd_data = malloc(req->rd_size);
if (!req->rd_data) {
--
1.6.3.3
^ permalink raw reply related
* [PATCH] Fix problem with handling CHLD=0 command
From: Lukasz Pawlik @ 2010-10-19 12:32 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Lukasz Pawlik
This patch change handling of CHLD=0 command. Previously both calls
(held and waiting) were terminated when HS sent CHLD=0. Now all held
calls will be released or only incoming call will be rejected without
affecting any held calls.
---
audio/telephony-maemo6.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/audio/telephony-maemo6.c b/audio/telephony-maemo6.c
index f4dfbde..33fd13f 100644
--- a/audio/telephony-maemo6.c
+++ b/audio/telephony-maemo6.c
@@ -784,8 +784,11 @@ void telephony_call_hold_req(void *telephony_device, const char *cmd)
switch (cmd[0]) {
case '0':
- foreach_call_with_status(CSD_CALL_STATUS_HOLD, release_call);
- foreach_call_with_status(CSD_CALL_STATUS_WAITING,
+ if (find_call_with_status(CSD_CALL_STATUS_WAITING))
+ foreach_call_with_status(CSD_CALL_STATUS_WAITING,
+ release_call);
+ else
+ foreach_call_with_status(CSD_CALL_STATUS_HOLD,
release_call);
break;
case '1':
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] Fix regression when formatting timestamps from tracker
From: Johan Hedberg @ 2010-10-19 13:42 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1287489236-4558-1-git-send-email-luiz.dentz@gmail.com>
Hi Luiz,
On Tue, Oct 19, 2010, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
>
> Tracker format user delimiters like '-' and ':' which need to be keep in
> order to sscanf to work.
> ---
> plugins/phonebook-tracker.c | 9 ++++++---
> 1 files changed, 6 insertions(+), 3 deletions(-)
Pushed upstream. Thanks.
Johan
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox