* [PATCH 6/6] Bluetooth: Add support for setting LE advertising data
From: Johan Hedberg @ 2012-10-24 21:09 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1351112996-9597-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
This patch adds support for setting basing LE advertising data. The
three elements supported for now are the advertising flags, the TX power
and the friendly name.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
include/net/bluetooth/hci.h | 15 ++++++
include/net/bluetooth/hci_core.h | 2 +
net/bluetooth/hci_event.c | 9 ++++
net/bluetooth/mgmt.c | 102 ++++++++++++++++++++++++++++++++++++--
4 files changed, 125 insertions(+), 3 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index a633da0..f850e94 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -335,6 +335,13 @@ enum {
#define EIR_SSP_RAND_R 0x0F /* Simple Pairing Randomizer R */
#define EIR_DEVICE_ID 0x10 /* device ID */
+/* Low Energy Advertising Flags */
+#define LE_AD_LIMITED 0x01 /* Limited Discoverable */
+#define LE_AD_GENERAL 0x02 /* General Discoverable */
+#define LE_AD_NO_BREDR 0x04 /* BR/EDR not supported */
+#define LE_AD_SIM_LE_BREDR_CTRL 0x08 /* Simultaneous LE & BR/EDR Controller */
+#define LE_AD_SIM_LE_BREDR_HOST 0x10 /* Simultaneous LE & BR/EDR Host */
+
/* ----- HCI Commands ---- */
#define HCI_OP_NOP 0x0000
@@ -939,6 +946,14 @@ struct hci_rp_le_read_adv_tx_power {
__s8 tx_power;
} __packed;
+#define HCI_MAX_AD_LENGTH 31
+
+#define HCI_OP_LE_SET_ADV_DATA 0x2008
+struct hci_cp_le_set_adv_data {
+ __u8 length;
+ __u8 data[HCI_MAX_AD_LENGTH];
+} __packed;
+
#define HCI_OP_LE_SET_ADV_ENABLE 0x200a
#define HCI_OP_LE_SET_SCAN_PARAM 0x200b
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 9d7e94e..0981d3c 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -279,6 +279,8 @@ struct hci_dev {
struct le_scan_params le_scan_params;
__s8 adv_tx_power;
+ __u8 adv_data[HCI_MAX_AD_LENGTH];
+ __u8 adv_data_len;
int (*open)(struct hci_dev *hdev);
int (*close)(struct hci_dev *hdev);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index ed6b1e2..3691251 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1108,6 +1108,15 @@ static void hci_cc_le_read_adv_tx_power(struct hci_dev *hdev,
if (!rp->status)
hdev->adv_tx_power = rp->tx_power;
+ if (hdev->adv_data_len > 0) {
+ struct hci_cp_le_set_adv_data cp;
+
+ memcpy(cp.data, hdev->adv_data, sizeof(cp.data));
+ cp.length = cpu_to_le16(hdev->adv_data_len);
+
+ hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_DATA, sizeof(cp), &cp);
+ }
+
hci_req_complete(hdev, HCI_OP_LE_READ_ADV_TX_POWER, rp->status);
}
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 6770835..98c776a 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -593,6 +593,93 @@ static int update_eir(struct hci_dev *hdev)
return hci_send_cmd(hdev, HCI_OP_WRITE_EIR, sizeof(cp), &cp);
}
+static u16 create_ad(struct hci_dev *hdev, u8 *data)
+{
+ u8 *ptr = data;
+ u16 ad_len = 0;
+ size_t name_len;
+ u8 flags = 0;
+
+ if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
+ flags |= LE_AD_GENERAL;
+
+ if (!lmp_bredr_capable(hdev))
+ flags |= LE_AD_NO_BREDR;
+
+ if (lmp_le_br_capable(hdev))
+ flags |= LE_AD_SIM_LE_BREDR_CTRL;
+
+ if (lmp_host_le_br_capable(hdev))
+ flags |= LE_AD_SIM_LE_BREDR_HOST;
+
+ if (flags) {
+ BT_DBG("adv flags 0x%02x", flags);
+
+ ptr[0] = 2;
+ ptr[1] = EIR_FLAGS;
+ ptr[2] = flags;
+
+ ad_len += 3;
+ ptr += 3;
+ }
+
+ if (hdev->adv_tx_power) {
+ ptr[0] = 2;
+ ptr[1] = EIR_TX_POWER;
+ ptr[2] = (u8) hdev->adv_tx_power;
+
+ ad_len += 3;
+ ptr += 3;
+ }
+
+ name_len = strlen(hdev->dev_name);
+ if (name_len > 0) {
+ size_t max_len = HCI_MAX_AD_LENGTH - ad_len - 2;
+
+ if (name_len > max_len) {
+ name_len = max_len;
+ ptr[1] = EIR_NAME_SHORT;
+ } else
+ ptr[1] = EIR_NAME_COMPLETE;
+
+ ptr[0] = name_len + 1;
+
+ memcpy(ptr + 2, hdev->dev_name, name_len);
+
+ ad_len += (name_len + 2);
+ ptr += (name_len + 2);
+ }
+
+ return ad_len;
+}
+
+static int update_ad(struct hci_dev *hdev)
+{
+ struct hci_cp_le_set_adv_data cp;
+ u16 len;
+
+ if (!hdev_is_powered(hdev))
+ return 0;
+
+ if (!lmp_le_capable(hdev))
+ return 0;
+
+ memset(&cp, 0, sizeof(cp));
+
+ len = create_ad(hdev, cp.data);
+
+ if (hdev->adv_data_len == len &&
+ memcmp(cp.data, hdev->adv_data, len) == 0)
+ return 0;
+
+ memcpy(hdev->adv_data, cp.data, sizeof(cp.data));
+ hdev->adv_data_len = len;
+
+ cp.length = cpu_to_le16(len);
+
+ return hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_DATA, sizeof(cp), &cp);
+}
+
static u8 get_service_classes(struct hci_dev *hdev)
{
struct bt_uuid *uuid;
@@ -1279,8 +1366,10 @@ static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
/* If we're switching away from or into peripheral role toggle
* the corresponding flag
*/
- if (old_mode == MGMT_LE_PERIPHERAL || new_mode == MGMT_LE_PERIPHERAL)
+ if (old_mode == MGMT_LE_PERIPHERAL || new_mode == MGMT_LE_PERIPHERAL) {
change_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
+ update_ad(hdev);
+ }
if (!hdev_is_powered(hdev)) {
if (old_mode == MGMT_LE_OFF || new_mode == MGMT_LE_OFF)
@@ -3002,6 +3091,7 @@ int mgmt_powered(struct hci_dev *hdev, u8 powered)
update_name(hdev, hdev->dev_name);
update_eir(hdev);
}
+ update_ad(hdev);
} else {
u8 status = MGMT_STATUS_NOT_POWERED;
mgmt_pending_foreach(0, hdev, cmd_status_rsp, &status);
@@ -3582,12 +3672,14 @@ send_event:
err = mgmt_event(MGMT_EV_LOCAL_NAME_CHANGED, hdev, &ev,
sizeof(ev), cmd ? cmd->sk : NULL);
- /* EIR is taken care of separately when powering on the
+ /* EIR and AD are taken care of separately when powering on the
* adapter so only update them here if this is a name change
* unrelated to power on.
*/
- if (!test_bit(HCI_INIT, &hdev->flags))
+ if (!test_bit(HCI_INIT, &hdev->flags)) {
update_eir(hdev);
+ update_ad(hdev);
+ }
failed:
if (cmd)
@@ -3662,6 +3754,8 @@ int mgmt_le_enable_complete(struct hci_dev *hdev, u8 enable, u8 status)
if (enable && cp->val == MGMT_LE_PERIPHERAL)
return hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE,
sizeof(enable), &enable);
+ else
+ update_ad(hdev);
send_settings_rsp(cmd->sk, cmd->opcode, hdev);
list_del(&cmd->list);
@@ -3687,6 +3781,8 @@ int mgmt_le_adv_enable_complete(struct hci_dev *hdev, u8 enable, u8 status)
mgmt_pending_foreach(MGMT_OP_SET_LE, hdev, cmd_status_rsp,
&mgmt_err);
+ update_ad(hdev);
+
return 0;
}
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH 1/6] Bluetooth: Fix setting host feature bits for SSP
From: Marcel Holtmann @ 2012-10-24 21:45 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1351112996-9597-2-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> When we get a successful command complete for HCI_Write_SSP_Mode we need
> to update the host feature bits for the hdev struct accordingly.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/hci_event.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 2/6] Bluetooth: Fix sending unnecessary HCI_Write_SSP_Mode command
From: Marcel Holtmann @ 2012-10-24 21:48 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1351112996-9597-3-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> This patch fixes sending an unnecessary HCI_Write_SSP_Mode command if
> the command has already been sent as part of the default HCI init
> sequence.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/mgmt.c | 3 ++-
> 2 files changed, 3 insertions(+), 1 deletion(-)
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 3/6] Bluetooth: Fix sending unnecessary HCI_LE_Host_Enable
From: Marcel Holtmann @ 2012-10-24 21:48 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1351112996-9597-4-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> This patch fixes sending an unnecessary HCI_LE_Host_Enable command if
> the command has already been sent as part of the default HCI init
> sequence.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/mgmt.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 4/6] Bluetooth: Fix unnecessary EIR update during powering on
From: Marcel Holtmann @ 2012-10-24 21:49 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1351112996-9597-5-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> When powered on the EIR data gets updated as the last step by mgmt.
> Therefore avoid an update when getting a local name update as that's
> part of the normal HCI init sequence.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/mgmt.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 0/6] Bluetooth: Some fixes and full peripheral role support
From: Marcel Holtmann @ 2012-10-24 21:53 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1351112996-9597-1-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> This set of patches contains some basic fixes to SSP and LE enablement
> HCI commands as well as the two missing patches from my previous set,
> i.e. implementation of mgmt_set_le for peripheral mode as well as the
> writing of advertising data.
patches 1-4 look good to me. With 5-6, I would hold off a little bit
until we get this all figured out.
However we might should merge some of the basic HCI constants and
structs you need for advertisement handling. So that Aloisio's and your
patches do not conflict that much.
Regards
Marcel
^ permalink raw reply
* [bluetooth:master 12/33] net/bluetooth/l2cap_core.c:4495:6: sparse: symbol 'l2cap_physical_cfm' was not declared. Should it be static?
From: Fengguang Wu @ 2012-10-24 21:55 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, Gustavo Padovan
[-- Attachment #1: Type: text/plain, Size: 606 bytes --]
Hi Mat,
FYI, there are new sparse warnings show up in
tree: git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
head: 77220c6582477e696e3f3a53634fb1bd73128e95
commit: 8eb200bd2f1c772dcb7f108f690ef03b054be04e [12/33] Bluetooth: Handle physical link completion
+ net/bluetooth/l2cap_core.c:4495:6: sparse: symbol 'l2cap_physical_cfm' was not declared. Should it be static?
Please consider folding the attached diff :-)
---
0-DAY kernel build testing backend Open Source Technology Center
Fengguang Wu, Yuanhan Liu Intel Corporation
[-- Attachment #2: make-it-static-8eb200b.diff --]
[-- Type: text/x-diff, Size: 543 bytes --]
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 898529d..24285e5 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4492,7 +4492,7 @@ static void l2cap_do_move_cancel(struct l2cap_chan *chan, int result)
l2cap_ertm_send(chan);
}
-void l2cap_physical_cfm(struct l2cap_chan *chan, int result, u8 local_amp_id,
+static void l2cap_physical_cfm(struct l2cap_chan *chan, int result, u8 local_amp_id,
u8 remote_amp_id)
{
BT_DBG("chan %p, result %d, local_amp_id %d, remote_amp_id %d",
^ permalink raw reply related
* Re: [bluetooth:master 12/33] net/bluetooth/l2cap_core.c:4495:6: sparse: symbol 'l2cap_physical_cfm' was not declared. Should it be static?
From: Gustavo Padovan @ 2012-10-24 22:01 UTC (permalink / raw)
To: Fengguang Wu; +Cc: Mat Martineau, linux-bluetooth, Gustavo Padovan
In-Reply-To: <508863c1.QTbySMnQ9kj6jDZz%fengguang.wu@intel.com>
Hi Fengguang,
* Fengguang Wu <fengguang.wu@intel.com> [2012-10-25 05:55:13 +0800]:
> Hi Mat,
>
> FYI, there are new sparse warnings show up in
>
> tree: git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
> head: 77220c6582477e696e3f3a53634fb1bd73128e95
> commit: 8eb200bd2f1c772dcb7f108f690ef03b054be04e [12/33] Bluetooth: Handle physical link completion
>
> + net/bluetooth/l2cap_core.c:4495:6: sparse: symbol 'l2cap_physical_cfm' was not declared. Should it be static?
>
> Please consider folding the attached diff :-)
Thanks a lot for finding this, but please can you send me a proper git
formatted patch with Signed-off-by message so I can add your patch to
bluetooth-next.
Gustavo
^ permalink raw reply
* Re: [PATCH 5/6] Bluetooth: mgmt: Add support for switching to LE peripheral mode
From: Vinicius Costa Gomes @ 2012-10-24 22:13 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1351112996-9597-6-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
On 00:09 Thu 25 Oct, Johan Hedberg wrote:
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index dc60d31..ed6b1e2 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -790,9 +790,24 @@ static void hci_set_le_support(struct hci_dev *hdev)
> cp.simul = !!lmp_le_br_capable(hdev);
> }
>
> + /* If the host features don't reflect the desired state for LE
> + * then send the write_le_host_supported command. The command
> + * complete handler for it will take care of any necessary
> + * subsequent commands like set_adv_enable.
> + *
> + * If the host features for LE are already correct and
> + * peripheral mode is enabled directly send the le_set_adv
> + * command. The value of &cp.le is used so that advertising will
> + * not be enabled in the exceptional case that LE for some
> + * reason isn't enabled - something that should only be possible
> + * if someone is doing direct raw access to HCI.
> + */
> if (cp.le != !!lmp_host_le_capable(hdev))
> hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED, sizeof(cp),
> &cp);
> + else if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
> + hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE, sizeof(cp.le),
> + &cp.le);
> }
I agree with Marcel, and one point that worried me was this
unconditional advertising, I feel that we should be smarter about when
to start advertising, for example, here we are not taking into account
the user's visiblity settings.
Cheers,
--
Vinicius
^ permalink raw reply
* Re: [PATCH 5/6] Bluetooth: mgmt: Add support for switching to LE peripheral mode
From: Johan Hedberg @ 2012-10-24 22:36 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <20121024221341.GA22764@samus>
Hi Vinicius,
On Wed, Oct 24, 2012, Vinicius Costa Gomes wrote:
> On 00:09 Thu 25 Oct, Johan Hedberg wrote:
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index dc60d31..ed6b1e2 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -790,9 +790,24 @@ static void hci_set_le_support(struct hci_dev *hdev)
> > cp.simul = !!lmp_le_br_capable(hdev);
> > }
> >
> > + /* If the host features don't reflect the desired state for LE
> > + * then send the write_le_host_supported command. The command
> > + * complete handler for it will take care of any necessary
> > + * subsequent commands like set_adv_enable.
> > + *
> > + * If the host features for LE are already correct and
> > + * peripheral mode is enabled directly send the le_set_adv
> > + * command. The value of &cp.le is used so that advertising will
> > + * not be enabled in the exceptional case that LE for some
> > + * reason isn't enabled - something that should only be possible
> > + * if someone is doing direct raw access to HCI.
> > + */
> > if (cp.le != !!lmp_host_le_capable(hdev))
> > hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED, sizeof(cp),
> > &cp);
> > + else if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
> > + hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE, sizeof(cp.le),
> > + &cp.le);
> > }
>
> I agree with Marcel, and one point that worried me was this
> unconditional advertising, I feel that we should be smarter about when
> to start advertising, for example, here we are not taking into account
> the user's visiblity settings.
If you're a peripheral and you're not connectable/discoverable then what
good does it do to have Bluetooth enabled at all? Since a peripheral
can't scan or initiate connections you can't really do anything. So if a
higher layer doesn't want us advertising it could either set LE to off
or central role or even power off the adapter completely.
Johan
^ permalink raw reply
* Re: [PATCH 4/7] Bluetooth: Add support for setting LE advertising data
From: Anderson Lizardo @ 2012-10-25 0:00 UTC (permalink / raw)
To: Anderson Lizardo, linux-bluetooth
In-Reply-To: <20121023212637.GA17404@x220.ger.corp.intel.com>
Hi Johan,
On Tue, Oct 23, 2012 at 5:26 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Lizardo,
>
> On Tue, Oct 23, 2012, Anderson Lizardo wrote:
>> On Tue, Oct 23, 2012 at 12:53 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>> > + if (hdev->adv_tx_power) {
>> > + ptr[0] = 2;
>> > + ptr[1] = EIR_TX_POWER;
>> > + ptr[2] = (u8) hdev->adv_tx_power;
>> > +
>> > + ad_len += 3;
>> > + ptr += 3;
>> > + }
>>
>> 0dBm is a valid TX power. Not sure the if() clause is valid here.
>
> If it's not, we'd need to extend the storage size of hdev->adv_tx_power
> (to have some invalid value) or add another variable to hdev to indicate
> that we know the tx_power. FWIW, create_eir also uses the same test for
> including the inq_tx_power, so either both or neither should be fixed.
> For simplicity I'd just keep the 0-test unless 0 is a really common
> value.
0dBm is right in the middle of the valid range for BR/EDR. From "7.5.4
Read RSSI Command" (page 934):
BR/EDR
Range: -128 <= N <= 127 (signed integer)
Units: dBm
...
LE:
Range: -127 to 20, 127 (signed integer)
Units: dBm
>From "7.8.6 LE Read Advertising Channel Tx Power Command" (page 1061):
Range: -20 <= N <= 10
and from "7.3.35 Read Transmit Power Level Command" (page 857):
Range: -30 <= N <= 20
Units: dBm
IIRC, TI based LE controllers I tested for Proximity used to have 0dBm
TX power for active connections (as measured using Read Transmit Power
Level Command or TX Power GATT characteristic). It's not uncommon,
IMHO.
My suggestion would be to have a separate boolean for checking if
Advertising/Inquiry TX Power was read. I suppose this is not critical
as this information is not mandatory on Advertising Data anyway (at
least LE Proximity profile uses the connection specific TX power, not
advertising TX Power).
>> Also, I'm worried how we are going to put other advertising data here,
>> i.e. Manufacturer Specific data or Service Data. On last BlueZ meeting
>> we proposed (and have been implementing) the Set Controller Data mgmt
>> command to set them. Is this still an acceptable approach?
>
> Yes, though we only have 31 bytes to play with here which is quite
> little. Also, we might want to make this dependent on the LE GAP role.
> E.g. we might be more interested in manufacturer specific data for
> broadcaster role than for peripheral role (maybe a user space
> application *only* wants its data for broadcaster role).
I agree, but If I understand correctly, it will be BlueZ's
responsibility to enable/disable Broadcaster/Observer modes depending
on whether there is any active application registered over D-Bus.
>
> Johan
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* [PATCH] Bluetooth: l2cap_physical_cfm() can be static
From: Fengguang Wu @ 2012-10-25 0:26 UTC (permalink / raw)
To: Gustavo Padovan; +Cc: Mat Martineau, linux-bluetooth
In-Reply-To: <20121024220131.GB3030@joana>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---
net/bluetooth/l2cap_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-next.orig/net/bluetooth/l2cap_core.c 2012-10-25 08:23:52.456742699 +0800
+++ linux-next/net/bluetooth/l2cap_core.c 2012-10-25 08:23:55.056742760 +0800
@@ -4569,7 +4569,7 @@ static void l2cap_do_move_cancel(struct
l2cap_ertm_send(chan);
}
-void l2cap_physical_cfm(struct l2cap_chan *chan, int result, u8 local_amp_id,
+static void l2cap_physical_cfm(struct l2cap_chan *chan, int result, u8 local_amp_id,
u8 remote_amp_id)
{
BT_DBG("chan %p, result %d, local_amp_id %d, remote_amp_id %d",
^ permalink raw reply
* Re: [PATCH] add support bluetooth usb 0489:e046 Foxconn / Hon Hai
From: koko Aborigines @ 2012-10-25 0:57 UTC (permalink / raw)
To: Gustavo Padovan, koko Aborigines, marcel, padovan,
linux-bluetooth, linux-kernel
In-Reply-To: <20120816051056.GA25290@joana>
[-- Attachment #1: Type: text/plain, Size: 868 bytes --]
Hi,
On ubuntu 12.10 Base Kernel 3.5.5
Your Patch is work
Thank You.
ref https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1030233
On Thu, Aug 16, 2012 at 12:10 PM, Gustavo Padovan <gustavo@padovan.org>wrote:
> Hi,
>
> * koko Aborigines <7aborigines7@gmail.com> [2012-08-16 10:12:44 +0700]:
>
> > Hi, Gustavo
> >
> > i have error with your patch
> >
> > CC [M] drivers/bluetooth/btusb.o
> > drivers/bluetooth/btusb.c:113:2: error: implicit declaration of function
> > ‘USB_VENDOR_AND_INTERFACE_INFO’ [-Werror=implicit-function-declaration]
> > drivers/bluetooth/btusb.c:113:2: error: initializer element is not
> constant
> > drivers/bluetooth/btusb.c:113:2: error: (near initialization for
> > ‘btusb_table[22].match_flags’)
>
> You need to apply it against the bluetooth-next tree (or linux-next).
>
> Gustavo
>
[-- Attachment #2: Type: text/html, Size: 1406 bytes --]
^ permalink raw reply
* Re: [PATCH 5/6] Bluetooth: mgmt: Add support for switching to LE peripheral mode
From: Vinicius Costa Gomes @ 2012-10-25 0:58 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <20121024223635.GA11976@x220.P-661HNU-F1>
Hi Johan,
On 01:36 Thu 25 Oct, Johan Hedberg wrote:
> Hi Vinicius,
>
> On Wed, Oct 24, 2012, Vinicius Costa Gomes wrote:
> > On 00:09 Thu 25 Oct, Johan Hedberg wrote:
> > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > > index dc60d31..ed6b1e2 100644
> > > --- a/net/bluetooth/hci_event.c
> > > +++ b/net/bluetooth/hci_event.c
> > > @@ -790,9 +790,24 @@ static void hci_set_le_support(struct hci_dev *hdev)
> > > cp.simul = !!lmp_le_br_capable(hdev);
> > > }
> > >
> > > + /* If the host features don't reflect the desired state for LE
> > > + * then send the write_le_host_supported command. The command
> > > + * complete handler for it will take care of any necessary
> > > + * subsequent commands like set_adv_enable.
> > > + *
> > > + * If the host features for LE are already correct and
> > > + * peripheral mode is enabled directly send the le_set_adv
> > > + * command. The value of &cp.le is used so that advertising will
> > > + * not be enabled in the exceptional case that LE for some
> > > + * reason isn't enabled - something that should only be possible
> > > + * if someone is doing direct raw access to HCI.
> > > + */
> > > if (cp.le != !!lmp_host_le_capable(hdev))
> > > hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED, sizeof(cp),
> > > &cp);
> > > + else if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
> > > + hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE, sizeof(cp.le),
> > > + &cp.le);
> > > }
> >
> > I agree with Marcel, and one point that worried me was this
> > unconditional advertising, I feel that we should be smarter about when
> > to start advertising, for example, here we are not taking into account
> > the user's visiblity settings.
>
> If you're a peripheral and you're not connectable/discoverable then what
> good does it do to have Bluetooth enabled at all? Since a peripheral
> can't scan or initiate connections you can't really do anything. So if a
> higher layer doesn't want us advertising it could either set LE to off
> or central role or even power off the adapter completely.
And what about the device that receive these advertising events? It
won't be able to do anything with them. And we would be increasing the
risk of interfering with the communication of others.
For me, the LE peripheral mode setting has a longer lifetime than (and is
orthogonal to) the Discoverable/Connectable settings. So, I would enable
Peripheral mode once during the lifetime of my session, and control the
type and whether I am advertising using the Connectable/Discoverable
settings (perhaps even from the Apps that are listening for connections).
Having to change the role of the device to the Central role or turning
the controller off (which would mean to re-do a lot of the
initialization when I turn it on again) to stop advertising seems, at
least, not intuitive.
Then, the only way to send not Discoverable nor Connectable advertising
events would be with the Broadcaster role.
>
> Johan
Cheers,
--
Vinicius
^ permalink raw reply
* [PATCH] Bluetooth: Add support for Atheros [04ca:3004]
From: Dwaine Garden VE3GIF @ 2012-10-25 1:46 UTC (permalink / raw)
To: linux-bluetooth@vger.kernel.org; +Cc: Dwaine Garden
Add another vendor specific ID for Atheros AR3012 device.
This chip is wrapped by Lite-On Technology Corp.
output of usb-devices:
Bus 001 Device 008: ID 04ca:3004 Lite-On Technology Corp.
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 1.10
bDeviceClass 224 Wireless
bDeviceSubClass 1 Radio Frequency
bDeviceProtocol 1 Bluetooth
bMaxPacketSize0 64
idVendor 0x04ca Lite-On Technology Corp.
idProduct 0x3004
bcdDevice 0.02
iManufacturer 1 Atheros Communications
iProduct 2 Bluetooth USB Host Controller
iSerial 3 Alaska Day 2006
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 177
bNumInterfaces 2
bConfigurationValue 1
iConfiguration 4 BT HCI
bmAttributes 0xe0
Self Powered
Remote Wakeup
MaxPower 100mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 0
bAlternateSetting 0
bNumEndpoints 3
bInterfaceClass 224 Wireless
bInterfaceSubClass 1 Radio Frequency
bInterfaceProtocol 1 Bluetooth
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81 EP 1 IN
bmAttributes 3
Transfer Type Interrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0010 1x 16 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x82 EP 2 IN
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x02 EP 2 OUT
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 0
bNumEndpoints 2
bInterfaceClass 224 Wireless
bInterfaceSubClass 1 Radio Frequency
bInterfaceProtocol 1 Bluetooth
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0000 1x 0 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x03 EP 3 OUT
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0000 1x 0 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 1
bNumEndpoints 2
bInterfaceClass 224 Wireless
bInterfaceSubClass 1 Radio Frequency
bInterfaceProtocol 1 Bluetooth
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0009 1x 9 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x03 EP 3 OUT
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0009 1x 9 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 2
bNumEndpoints 2
bInterfaceClass 224 Wireless
bInterfaceSubClass 1 Radio Frequency
bInterfaceProtocol 1 Bluetooth
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0011 1x 17 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x03 EP 3 OUT
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0011 1x 17 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 3
bNumEndpoints 2
bInterfaceClass 224 Wireless
bInterfaceSubClass 1 Radio Frequency
bInterfaceProtocol 1 Bluetooth
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0019 1x 25 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x03 EP 3 OUT
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0019 1x 25 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 4
bNumEndpoints 2
bInterfaceClass 224 Wireless
bInterfaceSubClass 1 Radio Frequency
bInterfaceProtocol 1 Bluetooth
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0021 1x 33 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x03 EP 3 OUT
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0021 1x 33 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 5
bNumEndpoints 2
bInterfaceClass 224 Wireless
bInterfaceSubClass 1 Radio Frequency
bInterfaceProtocol 1 Bluetooth
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0031 1x 49 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x03 EP 3 OUT
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0031 1x 49 bytes
bInterval 1
Device Status: 0x0003
Self Powered
Remote Wakeup Enabled
Signed-off-by: Dwaine Garden <DwaineGarden@rogers.com>
---
diff --git a/drivers/bluetooth/ath3k.c b/drivers/bluetooth/ath3k.c
index fc2de55..1486f15 100644
--- a/drivers/bluetooth/ath3k.c
+++ b/drivers/bluetooth/ath3k.c
@@ -75,6 +75,7 @@ static struct usb_device_id ath3k_table[] = {
{ USB_DEVICE(0x0CF3, 0x3004) },
{ USB_DEVICE(0x0CF3, 0x311D) },
{ USB_DEVICE(0x13d3, 0x3375) },
+ { USB_DEVICE(0x04CA, 0x3004) },
{ USB_DEVICE(0x04CA, 0x3005) },
{ USB_DEVICE(0x13d3, 0x3362) },
{ USB_DEVICE(0x0CF3, 0xE004) },
@@ -102,6 +103,7 @@ static struct usb_device_id ath3k_blist_tbl[] = {
{ USB_DEVICE(0x0cf3, 0x3004), .driver_info = BTUSB_ATH3012 },
{ USB_DEVICE(0x0cf3, 0x311D), .driver_info = BTUSB_ATH3012 },
{ USB_DEVICE(0x13d3, 0x3375), .driver_info = BTUSB_ATH3012 },
+ { USB_DEVICE(0x04ca, 0x3004), .driver_info = BTUSB_ATH3012 },
{ USB_DEVICE(0x04ca, 0x3005), .driver_info = BTUSB_ATH3012 },
{ USB_DEVICE(0x13d3, 0x3362), .driver_info = BTUSB_ATH3012 },
{ USB_DEVICE(0x0cf3, 0xe004), .driver_info = BTUSB_ATH3012 },
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index debda27..0529cee 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -132,6 +132,7 @@ static struct usb_device_id blacklist_table[] = {
{ USB_DEVICE(0x0cf3, 0x3004), .driver_info = BTUSB_ATH3012 },
{ USB_DEVICE(0x0cf3, 0x311d), .driver_info = BTUSB_ATH3012 },
{ USB_DEVICE(0x13d3, 0x3375), .driver_info = BTUSB_ATH3012 },
+ { USB_DEVICE(0x04ca, 0x3004), .driver_info = BTUSB_ATH3012 },
{ USB_DEVICE(0x04ca, 0x3005), .driver_info = BTUSB_ATH3012 },
{ USB_DEVICE(0x13d3, 0x3362), .driver_info = BTUSB_ATH3012 },
{ USB_DEVICE(0x0cf3, 0xe004), .driver_info = BTUSB_ATH3012 },
^ permalink raw reply related
* Re: [PATCH 5/6] Bluetooth: mgmt: Add support for switching to LE peripheral mode
From: Marcel Holtmann @ 2012-10-25 1:54 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <20121025005842.GB22764@samus>
Hi Vinicius,
> > > On 00:09 Thu 25 Oct, Johan Hedberg wrote:
> > > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > > > index dc60d31..ed6b1e2 100644
> > > > --- a/net/bluetooth/hci_event.c
> > > > +++ b/net/bluetooth/hci_event.c
> > > > @@ -790,9 +790,24 @@ static void hci_set_le_support(struct hci_dev *hdev)
> > > > cp.simul = !!lmp_le_br_capable(hdev);
> > > > }
> > > >
> > > > + /* If the host features don't reflect the desired state for LE
> > > > + * then send the write_le_host_supported command. The command
> > > > + * complete handler for it will take care of any necessary
> > > > + * subsequent commands like set_adv_enable.
> > > > + *
> > > > + * If the host features for LE are already correct and
> > > > + * peripheral mode is enabled directly send the le_set_adv
> > > > + * command. The value of &cp.le is used so that advertising will
> > > > + * not be enabled in the exceptional case that LE for some
> > > > + * reason isn't enabled - something that should only be possible
> > > > + * if someone is doing direct raw access to HCI.
> > > > + */
> > > > if (cp.le != !!lmp_host_le_capable(hdev))
> > > > hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED, sizeof(cp),
> > > > &cp);
> > > > + else if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
> > > > + hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE, sizeof(cp.le),
> > > > + &cp.le);
> > > > }
> > >
> > > I agree with Marcel, and one point that worried me was this
> > > unconditional advertising, I feel that we should be smarter about when
> > > to start advertising, for example, here we are not taking into account
> > > the user's visiblity settings.
> >
> > If you're a peripheral and you're not connectable/discoverable then what
> > good does it do to have Bluetooth enabled at all? Since a peripheral
> > can't scan or initiate connections you can't really do anything. So if a
> > higher layer doesn't want us advertising it could either set LE to off
> > or central role or even power off the adapter completely.
>
> And what about the device that receive these advertising events? It
> won't be able to do anything with them. And we would be increasing the
> risk of interfering with the communication of others.
>
> For me, the LE peripheral mode setting has a longer lifetime than (and is
> orthogonal to) the Discoverable/Connectable settings. So, I would enable
> Peripheral mode once during the lifetime of my session, and control the
> type and whether I am advertising using the Connectable/Discoverable
> settings (perhaps even from the Apps that are listening for connections).
>
> Having to change the role of the device to the Central role or turning
> the controller off (which would mean to re-do a lot of the
> initialization when I turn it on again) to stop advertising seems, at
> least, not intuitive.
>
> Then, the only way to send not Discoverable nor Connectable advertising
> events would be with the Broadcaster role.
that is actually not my worries. That is something that bluetoothd will
handle for the applications. And I do not want to intermix connectable
and discoverable from BR/EDR. These are two different things.
The way I see it currently is that peripheral mode is tight to an
application that is current providing a service. Or even a plugin inside
bluetoothd.
I am currently almost leading with Johan here that once peripheral mode
is enabled, we should just advertise. If you do not want to advertise,
then just disable peripheral mode.
The one thing I want to have figured out before we go ahead with
peripheral support is of course broadcaster and observer support.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH] Bluetooth: l2cap_physical_cfm() can be static
From: Marcel Holtmann @ 2012-10-25 1:55 UTC (permalink / raw)
To: Fengguang Wu; +Cc: Gustavo Padovan, Mat Martineau, linux-bluetooth
In-Reply-To: <20121025002608.GA10418@localhost>
Hi Fengguang,
> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
> ---
> net/bluetooth/l2cap_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- linux-next.orig/net/bluetooth/l2cap_core.c 2012-10-25 08:23:52.456742699 +0800
> +++ linux-next/net/bluetooth/l2cap_core.c 2012-10-25 08:23:55.056742760 +0800
> @@ -4569,7 +4569,7 @@ static void l2cap_do_move_cancel(struct
> l2cap_ertm_send(chan);
> }
>
> -void l2cap_physical_cfm(struct l2cap_chan *chan, int result, u8 local_amp_id,
> +static void l2cap_physical_cfm(struct l2cap_chan *chan, int result, u8 local_amp_id,
> u8 remote_amp_id)
I rather wait for Mat to ACK or NACK this one. Maybe it is an oversight
or we need that later on to be actually public.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH] Bluetooth: Add support for Atheros [04ca:3004]
From: Marcel Holtmann @ 2012-10-25 1:56 UTC (permalink / raw)
To: Dwaine Garden VE3GIF; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1351129589.50995.YahooMailNeo@web125405.mail.ne1.yahoo.com>
Hi Dwaine,
> Add another vendor specific ID for Atheros AR3012 device.
> This chip is wrapped by Lite-On Technology Corp.
>
> output of usb-devices:
>
> Bus 001 Device 008: ID 04ca:3004 Lite-On Technology Corp.
> Device Descriptor:
> bLength 18
> bDescriptorType 1
> bcdUSB 1.10
> bDeviceClass 224 Wireless
> bDeviceSubClass 1 Radio Frequency
> bDeviceProtocol 1 Bluetooth
I want /sys/kernel/debug/usb/devices for this device and nothing else.
> diff --git a/drivers/bluetooth/ath3k.c b/drivers/bluetooth/ath3k.c
> index fc2de55..1486f15 100644
> --- a/drivers/bluetooth/ath3k.c
> +++ b/drivers/bluetooth/ath3k.c
> @@ -75,6 +75,7 @@ static struct usb_device_id ath3k_table[] = {
> { USB_DEVICE(0x0CF3, 0x3004) },
> { USB_DEVICE(0x0CF3, 0x311D) },
> { USB_DEVICE(0x13d3, 0x3375) },
> + { USB_DEVICE(0x04CA, 0x3004) },
> { USB_DEVICE(0x04CA, 0x3005) },
This is still broken.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH] systemd: prevent duplicate logging messages in journal
From: Vinicius Costa Gomes @ 2012-10-25 2:12 UTC (permalink / raw)
To: Marti Raudsepp; +Cc: linux-bluetooth
In-Reply-To: <1349884769-5157-1-git-send-email-marti@juffo.org>
Hi Marti,
On 18:59 Wed 10 Oct, Marti Raudsepp wrote:
> By default, both stdout and syslog messages go to the systemd journal,
> which results in duplicate messages being logged.
> ---
Patch looks good.
Ack.
On a side note: seems that connman and ofono have the same problem, would you
be interested on sending the respective fixes?
> src/bluetooth.service.in | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/src/bluetooth.service.in b/src/bluetooth.service.in
> index 2a576a3..a8442a9 100644
> --- a/src/bluetooth.service.in
> +++ b/src/bluetooth.service.in
> @@ -5,6 +5,7 @@ Description=Bluetooth service
> Type=dbus
> BusName=org.bluez
> ExecStart=@prefix@/sbin/bluetoothd -n
> +StandardOutput=null
>
> [Install]
> WantedBy=bluetooth.target
> --
> 1.7.12.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Cheers,
--
Vinicius
^ permalink raw reply
* Re: [PATCH 5/6] Bluetooth: mgmt: Add support for switching to LE peripheral mode
From: Vinicius Costa Gomes @ 2012-10-25 4:56 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1351130085.1785.84.camel@aeonflux>
Hi Marcel,
On 18:54 Wed 24 Oct, Marcel Holtmann wrote:
> Hi Vinicius,
>
> > > > On 00:09 Thu 25 Oct, Johan Hedberg wrote:
> > > > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > > > > index dc60d31..ed6b1e2 100644
> > > > > --- a/net/bluetooth/hci_event.c
> > > > > +++ b/net/bluetooth/hci_event.c
> > > > > @@ -790,9 +790,24 @@ static void hci_set_le_support(struct hci_dev *hdev)
> > > > > cp.simul = !!lmp_le_br_capable(hdev);
> > > > > }
> > > > >
> > > > > + /* If the host features don't reflect the desired state for LE
> > > > > + * then send the write_le_host_supported command. The command
> > > > > + * complete handler for it will take care of any necessary
> > > > > + * subsequent commands like set_adv_enable.
> > > > > + *
> > > > > + * If the host features for LE are already correct and
> > > > > + * peripheral mode is enabled directly send the le_set_adv
> > > > > + * command. The value of &cp.le is used so that advertising will
> > > > > + * not be enabled in the exceptional case that LE for some
> > > > > + * reason isn't enabled - something that should only be possible
> > > > > + * if someone is doing direct raw access to HCI.
> > > > > + */
> > > > > if (cp.le != !!lmp_host_le_capable(hdev))
> > > > > hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED, sizeof(cp),
> > > > > &cp);
> > > > > + else if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
> > > > > + hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE, sizeof(cp.le),
> > > > > + &cp.le);
> > > > > }
> > > >
> > > > I agree with Marcel, and one point that worried me was this
> > > > unconditional advertising, I feel that we should be smarter about when
> > > > to start advertising, for example, here we are not taking into account
> > > > the user's visiblity settings.
> > >
> > > If you're a peripheral and you're not connectable/discoverable then what
> > > good does it do to have Bluetooth enabled at all? Since a peripheral
> > > can't scan or initiate connections you can't really do anything. So if a
> > > higher layer doesn't want us advertising it could either set LE to off
> > > or central role or even power off the adapter completely.
> >
> > And what about the device that receive these advertising events? It
> > won't be able to do anything with them. And we would be increasing the
> > risk of interfering with the communication of others.
> >
> > For me, the LE peripheral mode setting has a longer lifetime than (and is
> > orthogonal to) the Discoverable/Connectable settings. So, I would enable
> > Peripheral mode once during the lifetime of my session, and control the
> > type and whether I am advertising using the Connectable/Discoverable
> > settings (perhaps even from the Apps that are listening for connections).
> >
> > Having to change the role of the device to the Central role or turning
> > the controller off (which would mean to re-do a lot of the
> > initialization when I turn it on again) to stop advertising seems, at
> > least, not intuitive.
> >
> > Then, the only way to send not Discoverable nor Connectable advertising
> > events would be with the Broadcaster role.
>
> that is actually not my worries. That is something that bluetoothd will
> handle for the applications. And I do not want to intermix connectable
> and discoverable from BR/EDR. These are two different things.
Ok. This is new to me. So, what information will we use to set the
advertising type and the Flags field in the advertising event (think
general/limited discoverable modes)?
>
> The way I see it currently is that peripheral mode is tight to an
> application that is current providing a service. Or even a plugin inside
> bluetoothd.
Makes total sense.
>
> I am currently almost leading with Johan here that once peripheral mode
> is enabled, we should just advertise. If you do not want to advertise,
> then just disable peripheral mode.
Ok. I think I got it. I was with the impression that the Peripheral bit was
something like a capability, something that I would set and forget. In
reality, it is something meant to be more transient. I would argue that
this confusion alone is a point in favor of having a separate command for it.
But I guess it is too late now.
So, my worry of not sending useless advertising events would be addressed by
bluetoothd. Sounds good enough.
>
> The one thing I want to have figured out before we go ahead with
> peripheral support is of course broadcaster and observer support.
>
>
> Regards
>
> Marcel
>
>
Cheers,
--
Vinicius
^ permalink raw reply
* RE: [RFC 1/2] Bluetooth: Add driver extension for vendor specific init
From: Ho, Albert O @ 2012-10-25 6:38 UTC (permalink / raw)
To: Marcel Holtmann
Cc: linux-bluetooth, Hedberg, Johan, tedd.hj.an@gmail.com, An, Tedd
In-Reply-To: <1351091899.1785.69.camel@aeonflux>
QWNrbm93bGVkZ2VkLg0KDQpSZWdhcmRzLA0KQWxiZXJ0DQoNCg0KLS0tLS1PcmlnaW5hbCBNZXNz
YWdlLS0tLS0NCkZyb206IE1hcmNlbCBIb2x0bWFubiBbbWFpbHRvOm1hcmNlbEBob2x0bWFubi5v
cmddIA0KU2VudDogV2VkbmVzZGF5LCBPY3RvYmVyIDI0LCAyMDEyIDg6MTggQU0NClRvOiBIbywg
QWxiZXJ0IE8NCkNjOiBsaW51eC1ibHVldG9vdGg7IEhlZGJlcmcsIEpvaGFuOyB0ZWRkLmhqLmFu
QGdtYWlsLmNvbTsgQW4sIFRlZGQNClN1YmplY3Q6IFJlOiBbUkZDIDEvMl0gQmx1ZXRvb3RoOiBB
ZGQgZHJpdmVyIGV4dGVuc2lvbiBmb3IgdmVuZG9yIHNwZWNpZmljIGluaXQNCg0KSGkgQWxiZXJ0
LA0KDQo+ID4gVGhpcyBwYXRjaCBwcm92aWRlcyBhbiBleHRlbnNpb24gb2YgYnR1c2IgdG8gc3Vw
cG9ydCBkZXZpY2UgdmVuZG9yIA0KPiA+IGNhbiBpbXBsZW1lbnQgdGhlaXIgb3duIG1vZHVsZSB0
byBleGVjdXRlIHRoZSB2ZW5kb3Igc3BlY2lmaWMgZGV2aWNlIA0KPiA+IGluaXRpYWxpemF0aW9u
IGJlZm9yZSB0aGUgc3RhY2sgc2VuZHMgZ2VuZXJpYyBCVCBkZXZpY2UgDQo+ID4gaW5pdGlhbGl6
YXRpb24uDQo+ID4gDQo+ID4gU2lnbmVkLW9mZi1ieTogVGVkZCBIby1KZW9uZyBBbiA8dGVkZC5h
bkBpbnRlbC5jb20+DQo+ID4gLS0tDQo+ID4gIGRyaXZlcnMvYmx1ZXRvb3RoL2J0dXNiLmMgICAg
ICAgIHwgIDE5MSArKysrKysrKysrKysrKysrKysrKysrKysrLS0tLS0tLS0tLS0tLQ0KPiA+ICBk
cml2ZXJzL2JsdWV0b290aC9idHVzYi5oICAgICAgICB8ICAgNTMgKysrKysrKysrKysNCj4gPiAg
aW5jbHVkZS9uZXQvYmx1ZXRvb3RoL2hjaV9jb3JlLmggfCAgIDEwICsrDQo+ID4gIG5ldC9ibHVl
dG9vdGgvaGNpX2NvcmUuYyAgICAgICAgIHwgICAxNSArKy0NCj4gPiAgNCBmaWxlcyBjaGFuZ2Vk
LCAyMDQgaW5zZXJ0aW9ucygrKSwgNjUgZGVsZXRpb25zKC0pICBjcmVhdGUgbW9kZQ0KPiA+IDEw
MDY0NCBkcml2ZXJzL2JsdWV0b290aC9idHVzYi5oDQo+ID4gDQo+ID4gZGlmZiAtLWdpdCBhL2Ry
aXZlcnMvYmx1ZXRvb3RoL2J0dXNiLmMgYi9kcml2ZXJzL2JsdWV0b290aC9idHVzYi5jIA0KPiA+
IGluZGV4IGY2MzdjMjUuLmFmYTE1NTggMTAwNjQ0DQo+ID4gLS0tIGEvZHJpdmVycy9ibHVldG9v
dGgvYnR1c2IuYw0KPiA+ICsrKyBiL2RyaXZlcnMvYmx1ZXRvb3RoL2J0dXNiLmMNCj4gPiBAQCAt
MjYsNiArMjYsNyBAQA0KPiA+ICANCj4gPiAgI2luY2x1ZGUgPG5ldC9ibHVldG9vdGgvYmx1ZXRv
b3RoLmg+ICAjaW5jbHVkZSANCj4gPiA8bmV0L2JsdWV0b290aC9oY2lfY29yZS5oPg0KPiA+ICsj
aW5jbHVkZSAiYnR1c2IuaCINCj4gPiAgDQo+ID4gICNkZWZpbmUgVkVSU0lPTiAiMC42Ig0KPiA+
ICANCj4gPiBAQCAtMzksMTQgKzQwLDU5IEBAIHN0YXRpYyBib29sIHJlc2V0ID0gMTsNCj4gPiAg
DQo+ID4gIHN0YXRpYyBzdHJ1Y3QgdXNiX2RyaXZlciBidHVzYl9kcml2ZXI7DQo+ID4gIA0KPiA+
IC0jZGVmaW5lIEJUVVNCX0lHTk9SRQkJMHgwMQ0KPiA+IC0jZGVmaW5lIEJUVVNCX0RJR0lBTlNX
RVIJMHgwMg0KPiA+IC0jZGVmaW5lIEJUVVNCX0NTUgkJMHgwNA0KPiA+IC0jZGVmaW5lIEJUVVNC
X1NOSUZGRVIJCTB4MDgNCj4gPiAtI2RlZmluZSBCVFVTQl9CQ005MjAzNQkJMHgxMA0KPiA+IC0j
ZGVmaW5lIEJUVVNCX0JST0tFTl9JU09DCTB4MjANCj4gPiAtI2RlZmluZSBCVFVTQl9XUk9OR19T
Q09fTVRVCTB4NDANCj4gPiAtI2RlZmluZSBCVFVTQl9BVEgzMDEyCQkweDgwDQo+ID4gKy8qDQo+
ID4gKyAqIENyZWF0ZSBidHVzYl9kcml2ZXJfaW5mbyBzdHJ1Y3QgZm9yIGVhY2ggZHJpdmVyX2lu
Zm8gZmxhZ3MgdXNlZCANCj4gPiArYnkNCj4gPiArICogYmxhY2tsaXN0IHNpbmNlIHZlbmRvcidz
IGJ0dXNiIGRyaXZlciB3aWxsIHJldHVybiBidHVzYl9kcml2ZXJfaW5mbyBzdHJ1Y3QuDQo+ID4g
KyAqLw0KPiA+ICsNCj4gPiArLyoNCj4gPiArICogaWYgdGhlIGRldmljZSBpcyBzZXQgdG8gdGhp
cywgdGhpcyBtZW5hcyB0aGF0IHRoZSBkZXZpY2UgaXMgDQo+ID4gK2dlbmVyaWMgYW5kDQo+ID4g
KyAqIGRvZXNuJ3QgcmVxdWlyZSBhbnkgdmVuZG9yIHNwZWNpZmljIGhhbmRsaW5nICAqLyBzdGF0
aWMgY29uc3QgDQo+ID4gK3N0cnVjdCBidHVzYl9kcml2ZXJfaW5mbyBnZW5lcmljID0gew0KPiA+
ICsJLmRlc2NyaXB0aW9uCT0gIkJUVVNCIEdlbmVyaWMiLA0KPiA+ICsJLmZsYWdzCQk9IEJUVVNC
X0dFTkVSSUMsDQo+ID4gK307DQo+ID4gKw0KPiA+ICtzdGF0aWMgY29uc3Qgc3RydWN0IGJ0dXNi
X2RyaXZlcl9pbmZvIGlnbm9yZSA9IHsNCj4gPiArCS5kZXNjcmlwdGlvbgk9ICJCVFVTQiBJZ25v
cmUiLA0KPiA+ICsJLmZsYWdzCQk9IEJUVVNCX0lHTk9SRSwNCj4gPiArfTsNCj4gDQo+ID4+IEkg
bGlrZSB0aGUgZWZmb3J0LCBidXQgSSB0aGluayB5b3Ugd2VudCBhIGxpdHRsZSBiaXQgdG9vIGZh
ciBoZXJlLiBGb3IgdGhlc2Ugc2ltcGxlIG9uZXMsIHdlIGNhbiBlYXNpbHkga2VlcCBvdXIgc2lt
cGxlIGJsYWNrbGlzdC4gSXQga2VlcHMgdGhlIGNvZGUgbW9yZSByZWFkYWJsZSB0aGFuIHRoaXMg
cGFydC4gQnV0IEkgZG8gYXBwcmVjaWF0ZSB0aGUgYXR0ZW1wdCBpbiB1bmlmeWluZyB0aGlzLg0K
PiANCj4gDQo+ID4gKw0KPiA+ICtzdGF0aWMgY29uc3Qgc3RydWN0IGJ0dXNiX2RyaXZlcl9pbmZv
IGRpZ2lhbnN3ZXIgPSB7DQo+ID4gKwkuZGVzY3JpcHRpb24JPSAiQlRVU0IgRElHSUFOU1dFUiIs
DQo+ID4gKwkuZmxhZ3MJCT0gQlRVU0JfRElHSUFOU1dFUiwNCj4gPiArfTsNCj4gPiArDQo+ID4g
K3N0YXRpYyBjb25zdCBzdHJ1Y3QgYnR1c2JfZHJpdmVyX2luZm8gY3NyID0gew0KPiA+ICsJLmRl
c2NyaXB0aW9uCT0gIkJUVVNCIENTUiIsDQo+ID4gKwkuZmxhZ3MJCT0gQlRVU0JfQ1NSLA0KPiA+
ICt9Ow0KPiA+ICsNCj4gPiArc3RhdGljIGNvbnN0IHN0cnVjdCBidHVzYl9kcml2ZXJfaW5mbyBz
bmlmZmVyID0gew0KPiA+ICsJLmRlc2NyaXB0aW9uCT0gIkJUVVNCIFNuaWZmZXIiLA0KPiA+ICsJ
LmZsYWdzCQk9IEJUVVNCX1NOSUZGRVIsDQo+ID4gK307DQo+ID4gKw0KPiA+ICtzdGF0aWMgY29u
c3Qgc3RydWN0IGJ0dXNiX2RyaXZlcl9pbmZvIGJjbTkyMDM1ID0gew0KPiA+ICsJLmRlc2NyaXB0
aW9uCT0gIkJUVVNCIEJDTTkyMDM1IiwNCj4gPiArCS5mbGFncwkJPSBCVFVTQl9CQ005MjAzNSwN
Cj4gPiArfTsNCj4gPiArDQo+ID4gK3N0YXRpYyBjb25zdCBzdHJ1Y3QgYnR1c2JfZHJpdmVyX2lu
Zm8gYnJva2VuX2lzb2MgPSB7DQo+ID4gKwkuZGVzY3JpcHRpb24JPSAiQlRVU0IgQnJva2VuIElT
T0MiLA0KPiA+ICsJLmZsYWdzCQk9IEJUVVNCX0JST0tFTl9JU09DLA0KPiA+ICt9Ow0KPiA+ICsN
Cj4gPiArc3RhdGljIGNvbnN0IHN0cnVjdCBidHVzYl9kcml2ZXJfaW5mbyB3cm9uZ19zY29fbXR1
ID0gew0KPiA+ICsJLmRlc2NyaXB0aW9uCT0gIkJUVVNCIFdyb25nIFNDTyBNVFUiLA0KPiA+ICsJ
LmZsYWdzCQk9IEJUVVNCX1dST05HX1NDT19NVFUsDQo+ID4gK307DQo+ID4gKw0KPiA+ICtzdGF0
aWMgY29uc3Qgc3RydWN0IGJ0dXNiX2RyaXZlcl9pbmZvIGF0aDMwMTIgPSB7DQo+ID4gKwkuZGVz
Y3JpcHRpb24JPSAiQlRVU0IgQXRoMzAxMiIsDQo+ID4gKwkuZmxhZ3MJCT0gQlRVU0JfQVRIMzAx
MiwNCj4gPiArfTsNCj4gPiAgDQo+ID4gIHN0YXRpYyBzdHJ1Y3QgdXNiX2RldmljZV9pZCBidHVz
Yl90YWJsZVtdID0gew0KPiA+ICAJLyogR2VuZXJpYyBCbHVldG9vdGggVVNCIGRldmljZSAqLyBA
QCAtMTA1LDkwICsxNTEsODkgQEAgc3RhdGljIA0KPiA+IHN0cnVjdCB1c2JfZGV2aWNlX2lkIGJ0
dXNiX3RhYmxlW10gPSB7DQo+ID4gIA0KPiA+ICAJeyB9CS8qIFRlcm1pbmF0aW5nIGVudHJ5ICov
DQo+ID4gIH07DQo+ID4gLQ0KPiA+ICBNT0RVTEVfREVWSUNFX1RBQkxFKHVzYiwgYnR1c2JfdGFi
bGUpOw0KPiA+ICANCj4gPiAgc3RhdGljIHN0cnVjdCB1c2JfZGV2aWNlX2lkIGJsYWNrbGlzdF90
YWJsZVtdID0gew0KPiA+ICAJLyogQ1NSIEJsdWVDb3JlIGRldmljZXMgKi8NCj4gPiAtCXsgVVNC
X0RFVklDRSgweDBhMTIsIDB4MDAwMSksIC5kcml2ZXJfaW5mbyA9IEJUVVNCX0NTUiB9LA0KPiA+
ICsJeyBVU0JfREVWSUNFKDB4MGExMiwgMHgwMDAxKSwgLmRyaXZlcl9pbmZvID0gKHVuc2lnbmVk
IGxvbmcpICZjc3IgDQo+ID4gK30sDQo+IA0KPiA+PiBLZWVwIHRoZSBibGFja2xpc3RfdGFibGUg
YXMgaXQgaXMuIFRoZSBpbXBvcnRhbnQgdGFibGUgdG8gbW9kaWZ5IGlzIGJ0dXNiX3RhYmxlIGFu
ZCB1c2UgYSBjb21tb24gZHJpdmVyX2luZm8gdGhhdCB3aWxsIGJlIHNoYXJlZCBiZXR3ZWVuIGRy
aXZlcnMuDQo+IA0KPiA+PiBUaGF0IHdvdWxkIGFsc28gbWFrZSBpdCBzaW1wbGUgdG8ganVzdCBh
ZGQgQlRVU0JfSUdOT1JFIG9yIGEgbmV3IEJUVVNCX1ZFTkRPUiBmbGFnIHRvIGNhbGwgb3V0IHRo
ZSBkcml2ZXJzIHRoYXQgaGF2ZSBhIHNlcGFyYXRlIGRyaXZlciB3aXRoIGEgdmVuZG9yIGluaXQg
ZnVuY3Rpb24sIGJ1dCB3b3VsZCBtYXRjaCA+PiB0aGUgQmx1ZXRvb3RoIGdlbmVyYWwgVVNCIGRl
c2NyaXB0b3JzLg0KPiANCj4gSSBsb29rZWQgYXQgdXNibmV0IG1pbmlkcml2ZXIgYXMgeW91IHN1
Z2dlc3RlZC4gIElmIHdlIGRvIGEgc2ltaWxhciBzY2hlbWUgc3VjaCB0aGF0IGJ0dXNiX3Byb2Jl
KCkgaW52b2tlcyB0aGUgbWluaV9kcml2ZXIncyBiaW5kKCkvdW5iaW5kKCkgdGhlbiBidHVzYidz
IHVzYWdlIG9mIGRyaXZlcl9pbmZvIHdpbGwgbmVlZCB0byBjaGFuZ2UgZnJvbSBzdG9yaW5nIGZs
YWdzIChleDogQlRVU0JfSUdOT1JFKSBhbmQgY2hhbmdlIHRvIHN0b3JlICJzdGF0aWMgY29uc3Qg
c3RydWN0IiB3aGVyZSBpdCBob2xkcyBhIHN0cnVjdCBjb250YWluaW5nIHRoZSBtaW5pLWRyaXZl
cidzIGJpbmQvdW5iaW5kIGZ1bmN0aW9ucyAoanVzdCBsaWtlIHVzYm5ldCBtaW5pZHJpdmVyKS4g
ICBBcmUgeW91IGdvb2Qgd2l0aCB0aGlzIGNoYW5nZT8gIEkgYWxzbyB3YW50IHRvIG1lbnRpb24g
dGhhdCBpbiB0aGUgcGF0Y2ggY29udGFpbmluZyB0aGUgQlQgVVNCIG1pbmkgZHJpdmVyIHRlbXBs
YXRlLCBpdCBhbHJlYWR5IGhhcyBpdHMgb3duIG1vZHVsZV91c2JfZHJpdmVyKCkgc2VjdGlvbiB3
aXRoIGl0cyBWSUQvUElEIGxpc3RlZC4NCg0KeW91IGtub3cgd2hhdC4gSnVzdCBzZW5kIGFyb3Vu
ZCB3aGF0IHlvdSBoYXZlIHJpZ2h0IG5vdyBhZnRlciB0aGUgY2xlYW51cCBhbmQgdGhlbiBJIGNh
biB0YWtlIGFub3RoZXIgbG9vayBhdCBpdC4NCg0KUmVnYXJkcw0KDQpNYXJjZWwNCg0KDQo=
^ permalink raw reply
* Re: [PATCH] Bluetooth: l2cap_physical_cfm() can be static
From: Andrei Emeltchenko @ 2012-10-25 7:13 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Fengguang Wu, Gustavo Padovan, Mat Martineau, linux-bluetooth
In-Reply-To: <1351130134.1785.85.camel@aeonflux>
Hi,
On Wed, Oct 24, 2012 at 06:55:34PM -0700, Marcel Holtmann wrote:
> Hi Fengguang,
>
> > Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
> > ---
> > net/bluetooth/l2cap_core.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > --- linux-next.orig/net/bluetooth/l2cap_core.c 2012-10-25 08:23:52.456742699 +0800
> > +++ linux-next/net/bluetooth/l2cap_core.c 2012-10-25 08:23:55.056742760 +0800
> > @@ -4569,7 +4569,7 @@ static void l2cap_do_move_cancel(struct
> > l2cap_ertm_send(chan);
> > }
> >
> > -void l2cap_physical_cfm(struct l2cap_chan *chan, int result, u8 local_amp_id,
> > +static void l2cap_physical_cfm(struct l2cap_chan *chan, int result, u8 local_amp_id,
> > u8 remote_amp_id)
>
> I rather wait for Mat to ACK or NACK this one. Maybe it is an oversight
> or we need that later on to be actually public.
Agree with Marcel here. This shall be public.
Best regards
Andrei Emeltchenko
^ permalink raw reply
* Re: [PATCH 5/6] Bluetooth: mgmt: Add support for switching to LE peripheral mode
From: Johan Hedberg @ 2012-10-25 7:50 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: Marcel Holtmann, linux-bluetooth
In-Reply-To: <20121025045605.GB12303@echo>
Hi Vinicius,
On Thu, Oct 25, 2012, Vinicius Costa Gomes wrote:
> So, what information will we use to set the advertising type and the
> Flags field in the advertising event (think general/limited
> discoverable modes)?
Take a look at the create_ad() function in the last patch of this set.
Setting the right flags value is really quite simple. The advertising
type could be selected in a similar way, i.e. through hdev->dev_flags.
Johan
^ permalink raw reply
* Re: [PATCH v7 13/16] device: Retrieve name from cache directory
From: Johan Hedberg @ 2012-10-25 9:52 UTC (permalink / raw)
To: Frédéric Danis; +Cc: linux-bluetooth
In-Reply-To: <1351089258-25179-14-git-send-email-frederic.danis@linux.intel.com>
Hi Frédéric,
On Wed, Oct 24, 2012, Frédéric Danis wrote:
> ---
> src/device.c | 25 ++++++++++++++++++++++++-
> 1 file changed, 24 insertions(+), 1 deletion(-)
I've applied patches 1-12 since they seemed fine to me but there are
some things to consider with 13-16:
> --- a/src/device.c
> +++ b/src/device.c
> @@ -1573,6 +1573,8 @@ struct btd_device *device_create(struct btd_adapter *adapter,
> const bdaddr_t *src;
> char srcaddr[18], alias[MAX_NAME_LENGTH + 1];
> uint16_t vendor, product, version;
> + char filename[PATH_MAX + 1];
> + GKeyFile *key_file;
>
> device = g_try_malloc0(sizeof(struct btd_device));
> if (device == NULL)
> @@ -1600,7 +1602,28 @@ struct btd_device *device_create(struct btd_adapter *adapter,
> src = adapter_get_address(adapter);
> ba2str(src, srcaddr);
>
> - read_device_name(srcaddr, address, bdaddr_type, device->name);
> + snprintf(filename, PATH_MAX, STORAGEDIR "/%s/cache/%s", srcaddr,
> + address);
> + filename[PATH_MAX] = '\0';
> + key_file = g_key_file_new();
> +
> + if (g_key_file_load_from_file(key_file, filename, 0, NULL)) {
> + char *str;
> + int len;
> +
> + str = g_key_file_get_string(key_file, "General", "Name", NULL);
> + if (str) {
> + len = strlen(str);
> + if (len > HCI_MAX_NAME_LENGTH)
> + str[HCI_MAX_NAME_LENGTH] = '\0';
> +
> + strcpy(device->name, str);
> + g_free(str);
> + }
> + }
> +
> + g_key_file_free(key_file);
> +
I'd rather have you split off a separate function to load the config
like you have for adapters.
Regarding the device name, I think the idea is that removing the cache
shouldn't cause any bad behavior with the persistent part of the storage
(i.e. configured devices). So we should probably also have the name in
the per-device storage file and try to read it from there first and if
that fails fall back to the cache.
Johan
^ permalink raw reply
* Re: [PATCH v7 14/16] dbusoob: Store device name in cache directory
From: Johan Hedberg @ 2012-10-25 9:54 UTC (permalink / raw)
To: Frédéric Danis; +Cc: linux-bluetooth
In-Reply-To: <1351089258-25179-15-git-send-email-frederic.danis@linux.intel.com>
Hi Frédéric,
On Wed, Oct 24, 2012, Frédéric Danis wrote:
> ---
> plugins/dbusoob.c | 26 +++++++++++++++++++++++---
> 1 file changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/plugins/dbusoob.c b/plugins/dbusoob.c
> index 5c5b6ef..82d512c 100644
> --- a/plugins/dbusoob.c
> +++ b/plugins/dbusoob.c
> @@ -31,6 +31,7 @@
>
> #include <errno.h>
> #include <gdbus.h>
> +#include <sys/stat.h>
>
> #include <bluetooth/bluetooth.h>
> #include <bluetooth/hci.h>
> @@ -194,6 +195,11 @@ static gboolean parse_data(DBusMessageIter *data, struct oob_data *remote_data)
> static gboolean store_data(struct btd_adapter *adapter, struct oob_data *data)
> {
> bdaddr_t bdaddr;
> + char filename[PATH_MAX + 1];
> + char s_addr[18];
> + GKeyFile *key_file;
> + char *str;
> + gsize length = 0;
>
> str2ba(data->addr, &bdaddr);
>
> @@ -207,9 +213,23 @@ static gboolean store_data(struct btd_adapter *adapter, struct oob_data *data)
> write_remote_class(adapter_get_address(adapter), &bdaddr,
> data->class);
>
> - if (data->name)
> - write_device_name(adapter_get_address(adapter), &bdaddr, 0,
> - data->name);
> + if (data->name) {
> + ba2str(adapter_get_address(adapter), s_addr);
> + snprintf(filename, PATH_MAX, STORAGEDIR "/%s/cache/%s",
> + s_addr, data->addr);
> + filename[PATH_MAX] = '\0';
> + create_file(filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
> +
> + key_file = g_key_file_new();
> + g_key_file_load_from_file(key_file, filename, 0, NULL);
> + g_key_file_set_string(key_file, "General", "Name", data->name);
> +
> + str = g_key_file_to_data(key_file, &length, NULL);
> + g_file_set_contents(filename, str, length, NULL);
> + g_free(str);
> +
> + g_key_file_free(key_file);
> + }
There shouldn't this kind of core-daemon keyfile access from plugins, In
this case you should get a btd_device object and simply do a
btd_device_set_name (and if that function doesn't yet write to storage
update it so that it does).
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