* Re: [PATCH 3/7] Bluetooth: Disallow LE scanning and connecting in peripheral mode
From: Johan Hedberg @ 2012-10-23 20:59 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <CAJdJm_PXwxZ1cyCd=2g170AYx7yjx24cHmksJCTbkxtxyOV4-g@mail.gmail.com>
Hi Lizardo,
On Tue, Oct 23, 2012, Anderson Lizardo wrote:
> This will conflict with an Observer role running on the same device.
> Multiple roles are allowed to run concurrently if supported by the
> controller. From Core spec page 1639:
>
> A device may operate in multiple GAP roles concurrently if supported
> by the Controller. The Host should read the supported Link Layer
> States and State combinations from the Controller before any
> procedures or modes are used.
>
> Do you propose SET_LE to become a bitfield to support concurrent
> roles? I.e. Broadcaster/Observer.
See the other email I sent about this. If peripheral mode is enabled I
don't see how we could simultaneously allow another LE GAP mode based on
the restrictions of the core spec. And yes, we could make set_le a bit
field, but peripheral will probably still remain mutually exclusive with
any other LE GAP mode.
Johan
^ permalink raw reply
* Re: [PATCH 2/7] Bluetooth: mgmt: Restrict BR/EDR settings to BR/EDR-only adapters
From: Johan Hedberg @ 2012-10-23 20:48 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: Marcel Holtmann, linux-bluetooth
In-Reply-To: <CAJdJm_OLBgRVBFLD4sEp+OK-qkF_-CXXzuZOCAyiaD+TVMOYKQ@mail.gmail.com>
Hi Lizardo,
On Tue, Oct 23, 2012, Anderson Lizardo wrote:
> Johan: do you have further plans on improving LE peripheral support
> after these patches? If yes, please keep in mind that we still have
> ongoing plans to push broadcaster/observer roles upstream, which also
> requires hability to enable/disable advertising and set advertising
> data/parameters.
Looking through the various "shall" and "shall not" clauses in the core
spec it seems to me that only Central, Observer and Broadcaster roles
are compatible with each other, and Peripheral is mutually exclusive
with anything else. E.g. section 9.2.4.2 (page 1700):
"While a device is in the Broadcaster, Observer or Central role the
device shall not support the general discoverable mode."
and section 9.3.4.2 (page 1709):
"While a device is in the Broadcaster, Observer, or the Central role
the device shall not support the undirected connectable mode."
Both general discoverable and undirected connectable are needed by
Peripheral role, i.e. essentially excluding the other roles.
Extending the other roles could indeed be a matter of interpreting the
mgmt_set_le parameter as a bit mask and then returning an error if an
incompatible combination of roles is attempted. Also, once support for
new roles is added it should be easy to extend the current state checks
to make a distinction on exactly what kind of scanning/advertising is
done instead of just looking at "advertising or not" on a general level.
Johan
^ permalink raw reply
* Re: [PATCH 1/7] Bluetooth: mgmt: Add support for switching to LE peripheral mode
From: Johan Hedberg @ 2012-10-23 20:26 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1351018879.1785.44.camel@aeonflux>
Hi Marcel,
On Tue, Oct 23, 2012, Marcel Holtmann wrote:
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -786,6 +786,9 @@ static void hci_set_le_support(struct hci_dev *hdev)
> > if (cp.le != !!(hdev->host_features[0] & LMP_HOST_LE))
> > 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);
>
> What is this doing and why it is correct? I am failing to see this. We
> need to enable LE host supported first anyway.
The first if-statement checks if the host feature bits are what we want
them to be, and if not send the WRITE_LE_HOST_SUPPORTED command. The
else if part (if the host features are already correct and don't need
updating) enables advertising if necessary. There's a similar check for
HCI_LE_PERIPHERAL in the command complete handler for
HCI_OP_WRITE_LE_HOST_SUPPORTED in case the first if statement was
triggered. FWIW, I have actually tested this both with hciconfig and
btmgmt and it works fine.
> > +static void hci_cc_le_set_adv_enable(struct hci_dev *hdev, struct sk_buff *skb)
> > +{
> > + __u8 *sent, status = *((__u8 *) skb->data);
> > +
> > + BT_DBG("%s status 0x%2.2x", hdev->name, status);
> > +
> > + sent = hci_sent_cmd_data(hdev, HCI_OP_LE_SET_ADV_ENABLE);
> > + if (!sent)
> > + return;
> > +
> > + hci_dev_lock(hdev);
> > +
> > + if (!status) {
> > + if (*sent)
> > + set_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
> > + else
> > + clear_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
> > + } else {
> > + if (*sent)
> > + clear_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
> > + else
> > + set_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
> > + }
>
> Are you sure we want to based LE peripheral enabling based on if we
> advertise or not. I can see cases where we are a peripheral, but might
> want to not always advertise.
These set_bit/clear_bit calls are actually all no-op in the case that
the mgmt interface is used (since the LE_PERIPHERAL flag is already
modified when receiving the mgmt command). The only reason I put them
here is so that the reported mgmt settings remain correct and we reject
connection initiation and scanning if "hciconfig hci0 leadv" or similar
is used.
The simplistic way I thought we'd initially keep peripheral mode is
that it always implies advertising (unless a connection exists).
Otherwise the user would need to switch to LE-off or Central mode.
> > @@ -1213,48 +1242,71 @@ static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
> > goto unlock;
> > }
> >
> > - val = !!cp->val;
> > - enabled = !!(hdev->host_features[0] & LMP_HOST_LE);
> > + if (!valid_le_mode(cp->val)) {
> > + err = cmd_status(sk, hdev->id, MGMT_OP_SET_LE,
> > + MGMT_STATUS_INVALID_PARAMS);
> > + goto unlock;
> > + }
> >
> > - if (!hdev_is_powered(hdev) || val == enabled) {
> > - bool changed = false;
> > + if (mgmt_pending_find(MGMT_OP_SET_LE, hdev)) {
> > + err = cmd_status(sk, hdev->id, MGMT_OP_SET_LE,
> > + MGMT_STATUS_BUSY);
> > + goto unlock;
> > + }
> > +
> > + new_mode = cp->val;
> > + old_mode = get_le_mode(hdev);
> > +
> > + BT_DBG("%s old_mode %u new_mode %u", hdev->name, old_mode, new_mode);
> > +
> > + if (new_mode == old_mode) {
> > + err = send_settings_rsp(sk, MGMT_OP_SET_LE, hdev);
> > + goto unlock;
> > + }
> > +
> > + if (old_mode == MGMT_LE_PERIPHERAL || new_mode == MGMT_LE_PERIPHERAL)
> > + change_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
> >
> > - if (val != test_bit(HCI_LE_ENABLED, &hdev->dev_flags)) {
> > + if (!hdev_is_powered(hdev)) {
> > + if (old_mode == MGMT_LE_OFF || new_mode == MGMT_LE_OFF)
> > change_bit(HCI_LE_ENABLED, &hdev->dev_flags);
> > - changed = true;
> > - }
> >
> > err = send_settings_rsp(sk, MGMT_OP_SET_LE, hdev);
> > if (err < 0)
> > goto unlock;
> >
> > - if (changed)
> > - err = new_settings(hdev, sk);
> > + err = new_settings(hdev, sk);
> >
> > goto unlock;
> > }
> >
> > - if (mgmt_pending_find(MGMT_OP_SET_LE, hdev)) {
> > - err = cmd_status(sk, hdev->id, MGMT_OP_SET_LE,
> > - MGMT_STATUS_BUSY);
> > - goto unlock;
> > - }
> > -
> > cmd = mgmt_pending_add(sk, MGMT_OP_SET_LE, hdev, data, len);
> > if (!cmd) {
> > err = -ENOMEM;
> > goto unlock;
> > }
> >
> > + if ((old_mode != MGMT_LE_OFF && new_mode == MGMT_LE_PERIPHERAL) ||
> > + old_mode == MGMT_LE_PERIPHERAL) {
> > + u8 adv_enable = (new_mode == MGMT_LE_PERIPHERAL);
> > +
> > + err = hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE,
> > + sizeof(adv_enable), &adv_enable);
> > + if (err < 0)
> > + mgmt_pending_remove(cmd);
> > +
> > + goto unlock;
> > + }
> > +
>
> this gets a bit complicated. We either need more comments or split this
> out in separate helper functions.
I actually spent over a half a day trying to figure out an easy way to
simplify this, but the best I got was those two simple helper functions.
So I think I'll opt for just adding good code comments.
Johan
^ permalink raw reply
* Re: [PATCH 2/7] Bluetooth: mgmt: Restrict BR/EDR settings to BR/EDR-only adapters
From: Anderson Lizardo @ 2012-10-23 19:31 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Johan Hedberg, linux-bluetooth
In-Reply-To: <1351018956.1785.45.camel@aeonflux>
Hi Marcel,
On Tue, Oct 23, 2012 at 3:02 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Johan,
>
>> This patch makes sure that settings which are specific for BR/EDR
>> capable adapters are not allowed for non-BR/EDR (e.g. LE-only) adapters.
>> Instead, a "not supported" error is returned of such a setting is
>> attempted to be set for a non-BR/EDR adapter.
>>
>> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
>> ---
>> net/bluetooth/mgmt.c | 22 +++++++++++++++++++---
>> 1 file changed, 19 insertions(+), 3 deletions(-)
>
> can we lead with this patch. It seems to be useful no matter what we do
> for LE peripheral or single-mode controllers.
I agree that current upstream will be more consistent if these
settings are made "not supported" for LE (as they indeed are not
implemented yet). I was just confused that the same patch series that
improve LE peripheral support does not include properly support for
Connectable/Discoverable modes on this role (which may restrict the
usefulness?).
Johan: do you have further plans on improving LE peripheral support
after these patches? If yes, please keep in mind that we still have
ongoing plans to push broadcaster/observer roles upstream, which also
requires hability to enable/disable advertising and set advertising
data/parameters.
Best Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCHv4 10/18] Bluetooth: Add logical link confirm
From: Mat Martineau @ 2012-10-23 19:27 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth, gustavo, sunnyk, andrei.emeltchenko.news
In-Reply-To: <1351018415.1785.39.camel@aeonflux>
Marcel -
On Tue, 23 Oct 2012, Marcel Holtmann wrote:
> Hi Mat,
>
>> The logical link confirm callback is executed when the AMP controller
>> completes its logical link setup. During a channel move, a newly
>> formed logical link allows a move responder to send a move channel
>> response. A move initiator will send a move channel confirm. A
>> failed logical link will end the channel move and send an appropriate
>> response or confirm command indicating a failure.
>>
>> If the channel is being created on an AMP controller, L2CAP
>> configuration is completed after the logical link is set up.
>>
>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
>> ---
>> net/bluetooth/l2cap_core.c | 124 ++++++++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 116 insertions(+), 8 deletions(-)
>>
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index 69d43c9..0edc955 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -3799,6 +3799,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn,
>> goto unlock;
>> }
>>
>> + chan->ident = cmd->ident;
>> l2cap_send_cmd(conn, cmd->ident, L2CAP_CONF_RSP, len, rsp);
>> chan->num_conf_rsp++;
>>
>> @@ -4198,17 +4199,17 @@ static int l2cap_create_channel_req(struct l2cap_conn *conn,
>> return 0;
>> }
>>
>> -static void l2cap_send_move_chan_rsp(struct l2cap_conn *conn, u8 ident,
>> - u16 icid, u16 result)
>> +static void l2cap_send_move_chan_rsp(struct l2cap_chan *chan, u16 result)
>> {
>> struct l2cap_move_chan_rsp rsp;
>>
>> - BT_DBG("icid 0x%4.4x, result 0x%4.4x", icid, result);
>> + BT_DBG("chan %p, result 0x%4.4x", chan, result);
>>
>> - rsp.icid = cpu_to_le16(icid);
>> + rsp.icid = cpu_to_le16(chan->dcid);
>> rsp.result = cpu_to_le16(result);
>>
>> - l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_RSP, sizeof(rsp), &rsp);
>> + l2cap_send_cmd(chan->conn, chan->ident, L2CAP_MOVE_CHAN_RSP,
>> + sizeof(rsp), &rsp);
>> }
>>
>> static void l2cap_send_move_chan_cfm(struct l2cap_chan *chan, u16 result)
>> @@ -4260,11 +4261,114 @@ static void __release_logical_link(struct l2cap_chan *chan)
>> /* Placeholder - release the logical link */
>> }
>>
>> +static void l2cap_logical_fail(struct l2cap_chan *chan)
>> +{
>> + /* Logical link setup failed */
>> + if (chan->state != BT_CONNECTED) {
>> + /* Create channel failure, disconnect */
>> + l2cap_send_disconn_req(chan->conn, chan, ECONNRESET);
>
> lets do this:
>
> if (chan->state != BT_CONNECTED) {
> ...
> return;
> }
>
Ok.
>> + } else if (chan->move_role == L2CAP_MOVE_ROLE_RESPONDER) {
>> + l2cap_move_revert(chan);
>> + chan->move_role = L2CAP_MOVE_ROLE_NONE;
>> + chan->move_state = L2CAP_MOVE_STABLE;
>> + l2cap_send_move_chan_rsp(chan, L2CAP_MR_NOT_SUPP);
>> + } else if (chan->move_role == L2CAP_MOVE_ROLE_INITIATOR) {
>> + if (chan->move_state == L2CAP_MOVE_WAIT_LOGICAL_COMP ||
>> + chan->move_state == L2CAP_MOVE_WAIT_LOGICAL_CFM) {
>> + /* Remote has only sent pending or
>> + * success responses, clean up
>> + */
>> + l2cap_move_revert(chan);
>> + chan->move_role = L2CAP_MOVE_ROLE_NONE;
>> + chan->move_state = L2CAP_MOVE_STABLE;
>> + }
>> +
>> + /* Other amp move states imply that the move
>> + * has already aborted
>> + */
>> + l2cap_send_move_chan_cfm(chan, L2CAP_MC_UNCONFIRMED);
>> + }
>
> And turn this into a switch statement.
>
>> +
>> + __release_logical_link(chan);
>
> And leave this to the caller.
>
Ok.
>> +}
>> +
>> +static void l2cap_logical_finish_create(struct l2cap_chan *chan,
>> + struct hci_chan *hchan)
>> +{
>> + struct l2cap_conf_rsp rsp;
>> + u8 code;
>> +
>> + chan->hs_hcon = hchan->conn;
>> + chan->hs_hcon->l2cap_data = chan->conn;
>> +
>> + code = l2cap_build_conf_rsp(chan, &rsp,
>> + L2CAP_CONF_SUCCESS, 0);
>> + l2cap_send_cmd(chan->conn, chan->ident, L2CAP_CONF_RSP, code,
>> + &rsp);
>> + set_bit(CONF_OUTPUT_DONE, &chan->conf_state);
>> +
>> + if (test_bit(CONF_INPUT_DONE, &chan->conf_state)) {
>> + int err = 0;
>> +
>> + set_default_fcs(chan);
>> +
>> + err = l2cap_ertm_init(chan);
>> + if (err < 0)
>> + l2cap_send_disconn_req(chan->conn, chan, -err);
>> + else
>> + l2cap_chan_ready(chan);
>> + }
>> +}
>> +
>> +static void l2cap_logical_finish_move(struct l2cap_chan *chan,
>> + struct hci_chan *hchan)
>> +{
>> + chan->hs_hcon = hchan->conn;
>> + chan->hs_hcon->l2cap_data = chan->conn;
>> +
>> + BT_DBG("move_state %d", chan->move_state);
>> +
>> + switch (chan->move_state) {
>> + case L2CAP_MOVE_WAIT_LOGICAL_COMP:
>> + /* Move confirm will be sent after a success
>> + * response is received
>> + */
>> + chan->move_state = L2CAP_MOVE_WAIT_RSP_SUCCESS;
>> + break;
>> + case L2CAP_MOVE_WAIT_LOGICAL_CFM:
>> + if (test_bit(CONN_LOCAL_BUSY, &chan->conn_state)) {
>> + chan->move_state = L2CAP_MOVE_WAIT_LOCAL_BUSY;
>
> My brain just hurts from these nested if-else. A nested two switch does
> not make it any better though. So we can leave it as this. Except the
> statement below is used multiple places and we have a function for it.
>
This version (v4) of the patch reflects some consolidation in these
statements already, where I put more code inside
l2cap_send_move_chan_cfm and l2cap_send_move_chan_rsp. The move_state
assignments don't fit well in those helper functions.
The next 7 lines of code are not duplicated anywhere else. The first
block (L2CAP_MOVE_WAIT_CONFIRM_RSP + send confirm) is used in one
other place. The second block (L2CAP_MOVE_WAIT_CONFIRM + send
response) is also used in just one other place -- but a different one.
The surrounding logic based on chan->move_role is not shared.
Do you want me to create 2-line helper functions for each case, or
were you thinking there was more duplicated code around? Adding new
functions is a net gain in lines of code and doesn't seem like a big
win for clarity.
>> + } else if (chan->move_role == L2CAP_MOVE_ROLE_INITIATOR) {
>> + chan->move_state = L2CAP_MOVE_WAIT_CONFIRM_RSP;
>> + l2cap_send_move_chan_cfm(chan, L2CAP_MC_CONFIRMED);
>> + } else if (chan->move_role == L2CAP_MOVE_ROLE_RESPONDER) {
>> + chan->move_state = L2CAP_MOVE_WAIT_CONFIRM;
>> + l2cap_send_move_chan_rsp(chan, L2CAP_MR_SUCCESS);
>> + }
>> + break;
>> + default:
>> + /* Move was not in expected state, free the channel */
>> + __release_logical_link(chan);
>> +
>> + chan->move_state = L2CAP_MOVE_STABLE;
>> + }
>> +}
>> +
>> +/* Call with chan locked */
>> static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
>> u8 status)
>> {
>> - /* Placeholder */
>> - return;
>> + BT_DBG("chan %p, hchan %p, status %d", chan, hchan, status);
>> +
>> + if (status) {
>> + l2cap_logical_fail(chan);
>
> I rather have a return here.
>
> if (status) {
> l2cap_logical_fail(chan);
> __release_logical_link(chan);
> return;
> }
>
Ok.
>> + } else if (chan->state != BT_CONNECTED) {
>> + /* Ignore logical link if channel is on BR/EDR */
>> + if (chan->local_amp_id)
>> + l2cap_logical_finish_create(chan, hchan);
>> + } else {
>> + l2cap_logical_finish_move(chan, hchan);
>> + }
>> }
>>
>> static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
>> @@ -4272,6 +4376,7 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
>> u16 cmd_len, void *data)
>> {
>> struct l2cap_move_chan_req *req = data;
>> + struct l2cap_move_chan_rsp rsp;
>> struct l2cap_chan *chan;
>> u16 icid = 0;
>> u16 result = L2CAP_MR_NOT_ALLOWED;
>> @@ -4348,7 +4453,10 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
>> }
>>
>> send_move_response:
>> - l2cap_send_move_chan_rsp(conn, cmd->ident, icid, result);
>> + rsp.icid = cpu_to_le16(icid);
>> + rsp.result = cpu_to_le16(result);
>> + l2cap_send_cmd(conn, cmd->ident, L2CAP_MOVE_CHAN_RSP,
>> + sizeof(rsp), &rsp);
>>
>> if (chan)
>> l2cap_chan_unlock(chan);
>
> While not part of this patch, I still dislike if (something) unlock
> style. Please have that fixed as well.
I'll fix it. This is the only "if (chan) / unlock" case left.
> Rest looks fine.
Thanks,
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply
* Re: [PATCH 7/7] Bluetooth: Make use feature test macros
From: Marcel Holtmann @ 2012-10-23 19:06 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1351011241-32515-8-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> For better code readability and avoiding simple bugs of checking the
> wrong byte of the features make use of feature test macros whenever
> possible.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> include/net/bluetooth/hci_core.h | 8 ++++++++
> net/bluetooth/hci_event.c | 26 +++++++++++++-------------
> net/bluetooth/mgmt.c | 8 ++++----
> 3 files changed, 25 insertions(+), 17 deletions(-)
nice cleanup we should do first as well.
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 6/7] Bluetooth: Sort feature test macros by bitmask location
From: Marcel Holtmann @ 2012-10-23 19:06 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1351011241-32515-7-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> This patch sorts the feature test marcos according to the location of
> each feature bit in the features bit mask. This helps easy lookup when
> adding new marcos and wanting to check if there already is a macro
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> include/net/bluetooth/hci_core.h | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
this could go first as well.
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 5/7] Bluetooth: Fix updating host feature bits for LE
From: Marcel Holtmann @ 2012-10-23 19:04 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1351011241-32515-6-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> When LE has been enabled with the simultaneous BR/EDR & LE parameter set
> to true we should also update the host features stored in struct hci_dev
> accordingly.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/hci_event.c | 5 +++++
> 1 file changed, 5 insertions(+)
should go first as well.
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 2/7] Bluetooth: mgmt: Restrict BR/EDR settings to BR/EDR-only adapters
From: Marcel Holtmann @ 2012-10-23 19:03 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: Johan Hedberg, linux-bluetooth
In-Reply-To: <CAJdJm_PoB=e_pi2gAN4AOn_ETyHz3nDihUy8yu2bthe0+p5cxw@mail.gmail.com>
Hi Anderson,
> > @@ -377,15 +377,15 @@ static u32 get_supported_settings(struct hci_dev *hdev)
> > u32 settings = 0;
> >
> > settings |= MGMT_SETTING_POWERED;
> > - settings |= MGMT_SETTING_CONNECTABLE;
> > - settings |= MGMT_SETTING_FAST_CONNECTABLE;
> > - settings |= MGMT_SETTING_DISCOVERABLE;
> > settings |= MGMT_SETTING_PAIRABLE;
> >
> > if (lmp_ssp_capable(hdev))
> > settings |= MGMT_SETTING_SSP;
> >
> > if (lmp_bredr_capable(hdev)) {
> > + settings |= MGMT_SETTING_CONNECTABLE;
> > + settings |= MGMT_SETTING_FAST_CONNECTABLE;
> > + settings |= MGMT_SETTING_DISCOVERABLE;
> > settings |= MGMT_SETTING_BREDR;
> > settings |= MGMT_SETTING_LINK_SECURITY;
>
> "Discoverable" (at least as a Discovery mode from GAP) is mandatory
> for LE Peripheral role (see page 1698). Same for "Connectable" (see
> page 1706).
>
> Unless you have other plans to implement this for single mode LE
> peripherals running bluez?
at this moment, we plan to keep LE a bit independent from BR/EDR.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 2/7] Bluetooth: mgmt: Restrict BR/EDR settings to BR/EDR-only adapters
From: Marcel Holtmann @ 2012-10-23 19:02 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1351011241-32515-3-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> This patch makes sure that settings which are specific for BR/EDR
> capable adapters are not allowed for non-BR/EDR (e.g. LE-only) adapters.
> Instead, a "not supported" error is returned of such a setting is
> attempted to be set for a non-BR/EDR adapter.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/mgmt.c | 22 +++++++++++++++++++---
> 1 file changed, 19 insertions(+), 3 deletions(-)
can we lead with this patch. It seems to be useful no matter what we do
for LE peripheral or single-mode controllers.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/7] Bluetooth: mgmt: Add support for switching to LE peripheral mode
From: Marcel Holtmann @ 2012-10-23 19:01 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1351011241-32515-2-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> This patch extends the mgmt_set_le command to allow for a new value
> (0x02) to mean that LE should be enabled together with advertising
> enabled. This paves the way for acting in a fully functional LE
> peripheral role. The change to the mgmt_set_le command is fully
> backwards compatible as the value 0x01 means LE central role (which was
> the old behavior).
>
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> include/net/bluetooth/hci.h | 3 +
> include/net/bluetooth/hci_core.h | 1 +
> include/net/bluetooth/mgmt.h | 6 ++
> net/bluetooth/hci_event.c | 44 +++++++++++
> net/bluetooth/mgmt.c | 159 +++++++++++++++++++++++++++++++-------
> 5 files changed, 185 insertions(+), 28 deletions(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 348f4bf..a633da0 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -115,6 +115,7 @@ enum {
> HCI_SSP_ENABLED,
> HCI_HS_ENABLED,
> HCI_LE_ENABLED,
> + HCI_LE_PERIPHERAL,
> HCI_CONNECTABLE,
> HCI_DISCOVERABLE,
> HCI_LINK_SECURITY,
> @@ -938,6 +939,8 @@ struct hci_rp_le_read_adv_tx_power {
> __s8 tx_power;
> } __packed;
>
> +#define HCI_OP_LE_SET_ADV_ENABLE 0x200a
> +
> #define HCI_OP_LE_SET_SCAN_PARAM 0x200b
> struct hci_cp_le_set_scan_param {
> __u8 type;
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 8260bf2..f288a8c 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1075,6 +1075,7 @@ int mgmt_set_local_name_complete(struct hci_dev *hdev, u8 *name, u8 status);
> int mgmt_read_local_oob_data_reply_complete(struct hci_dev *hdev, u8 *hash,
> u8 *randomizer, u8 status);
> int mgmt_le_enable_complete(struct hci_dev *hdev, u8 enable, u8 status);
> +int mgmt_le_adv_enable_complete(struct hci_dev *hdev, u8 enable, u8 status);
> int mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
> u8 addr_type, u8 *dev_class, s8 rssi, u8 cfm_name,
> u8 ssp, u8 *eir, u16 eir_len);
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index 22980a7..26c9a4d 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -92,6 +92,7 @@ struct mgmt_rp_read_index_list {
> #define MGMT_SETTING_BREDR 0x00000080
> #define MGMT_SETTING_HS 0x00000100
> #define MGMT_SETTING_LE 0x00000200
> +#define MGMT_SETTING_PERIPHERAL 0x00000400
>
> #define MGMT_OP_READ_INFO 0x0004
> #define MGMT_READ_INFO_SIZE 0
> @@ -134,6 +135,11 @@ struct mgmt_cp_set_discoverable {
> #define MGMT_OP_SET_HS 0x000C
>
> #define MGMT_OP_SET_LE 0x000D
> +
> +#define MGMT_LE_OFF 0x00
> +#define MGMT_LE_CENTRAL 0x01
> +#define MGMT_LE_PERIPHERAL 0x02
> +
> #define MGMT_OP_SET_DEV_CLASS 0x000E
> struct mgmt_cp_set_dev_class {
> __u8 major;
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index fd5a51c..0393b34 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -786,6 +786,9 @@ static void hci_set_le_support(struct hci_dev *hdev)
> if (cp.le != !!(hdev->host_features[0] & LMP_HOST_LE))
> 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);
What is this doing and why it is correct? I am failing to see this. We
need to enable LE host supported first anyway.
> }
>
> static void hci_cc_read_local_ext_features(struct hci_dev *hdev,
> @@ -1189,6 +1192,38 @@ static void hci_cc_le_set_scan_param(struct hci_dev *hdev, struct sk_buff *skb)
> }
> }
>
> +static void hci_cc_le_set_adv_enable(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> + __u8 *sent, status = *((__u8 *) skb->data);
> +
> + BT_DBG("%s status 0x%2.2x", hdev->name, status);
> +
> + sent = hci_sent_cmd_data(hdev, HCI_OP_LE_SET_ADV_ENABLE);
> + if (!sent)
> + return;
> +
> + hci_dev_lock(hdev);
> +
> + if (!status) {
> + if (*sent)
> + set_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
> + else
> + clear_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
> + } else {
> + if (*sent)
> + clear_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
> + else
> + set_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
> + }
Are you sure we want to based LE peripheral enabling based on if we
advertise or not. I can see cases where we are a peripheral, but might
want to not always advertise.
> +
> + if (!test_bit(HCI_INIT, &hdev->flags))
> + mgmt_le_adv_enable_complete(hdev, *sent, status);
> +
> + hci_dev_unlock(hdev);
> +
> + hci_req_complete(hdev, HCI_OP_LE_SET_ADV_ENABLE, status);
> +}
> +
> static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
> struct sk_buff *skb)
> {
> @@ -1287,6 +1322,11 @@ static void hci_cc_write_le_host_supported(struct hci_dev *hdev,
> hdev->host_features[0] |= LMP_HOST_LE;
> else
> hdev->host_features[0] &= ~LMP_HOST_LE;
> +
> + if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags) &&
> + test_bit(HCI_INIT, &hdev->flags))
> + hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE,
> + sizeof(sent->le), &sent->le);
> }
>
> if (test_bit(HCI_MGMT, &hdev->dev_flags) &&
> @@ -2549,6 +2589,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> hci_cc_le_set_scan_param(hdev, skb);
> break;
>
> + case HCI_OP_LE_SET_ADV_ENABLE:
> + hci_cc_le_set_adv_enable(hdev, skb);
> + break;
> +
> case HCI_OP_LE_SET_SCAN_ENABLE:
> hci_cc_le_set_scan_enable(hdev, skb);
> break;
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 3fe74f4..1d79979 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -393,8 +393,10 @@ static u32 get_supported_settings(struct hci_dev *hdev)
> if (enable_hs)
> settings |= MGMT_SETTING_HS;
>
> - if (lmp_le_capable(hdev))
> + if (lmp_le_capable(hdev)) {
> settings |= MGMT_SETTING_LE;
> + settings |= MGMT_SETTING_PERIPHERAL;
> + }
>
> return settings;
> }
> @@ -418,9 +420,13 @@ static u32 get_current_settings(struct hci_dev *hdev)
> if (lmp_bredr_capable(hdev))
> settings |= MGMT_SETTING_BREDR;
>
> - if (test_bit(HCI_LE_ENABLED, &hdev->dev_flags))
> + if (test_bit(HCI_LE_ENABLED, &hdev->dev_flags)) {
> settings |= MGMT_SETTING_LE;
>
> + if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
> + settings |= MGMT_SETTING_PERIPHERAL;
> + }
> +
> if (test_bit(HCI_LINK_SECURITY, &hdev->dev_flags))
> settings |= MGMT_SETTING_LINK_SECURITY;
>
> @@ -1195,13 +1201,36 @@ static int set_hs(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
> return send_settings_rsp(sk, MGMT_OP_SET_HS, hdev);
> }
>
> +static u8 get_le_mode(struct hci_dev *hdev)
> +{
> + if (hdev->host_features[0] & LMP_HOST_LE) {
> + if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
> + return MGMT_LE_PERIPHERAL;
> + return MGMT_LE_CENTRAL;
> + }
> +
> + return MGMT_LE_OFF;
> +}
> +
> +static bool valid_le_mode(u8 mode)
> +{
> + switch (mode) {
> + case MGMT_LE_OFF:
> + case MGMT_LE_CENTRAL:
> + case MGMT_LE_PERIPHERAL:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
I prefer to not use the default and just return false at the end of the
function.
> static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
> {
> - struct mgmt_mode *cp = data;
> struct hci_cp_write_le_host_supported hci_cp;
> + struct mgmt_mode *cp = data;
> struct pending_cmd *cmd;
> + u8 old_mode, new_mode;
> int err;
> - u8 val, enabled;
>
> BT_DBG("request for %s", hdev->name);
>
> @@ -1213,48 +1242,71 @@ static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
> goto unlock;
> }
>
> - val = !!cp->val;
> - enabled = !!(hdev->host_features[0] & LMP_HOST_LE);
> + if (!valid_le_mode(cp->val)) {
> + err = cmd_status(sk, hdev->id, MGMT_OP_SET_LE,
> + MGMT_STATUS_INVALID_PARAMS);
> + goto unlock;
> + }
>
> - if (!hdev_is_powered(hdev) || val == enabled) {
> - bool changed = false;
> + if (mgmt_pending_find(MGMT_OP_SET_LE, hdev)) {
> + err = cmd_status(sk, hdev->id, MGMT_OP_SET_LE,
> + MGMT_STATUS_BUSY);
> + goto unlock;
> + }
> +
> + new_mode = cp->val;
> + old_mode = get_le_mode(hdev);
> +
> + BT_DBG("%s old_mode %u new_mode %u", hdev->name, old_mode, new_mode);
> +
> + if (new_mode == old_mode) {
> + err = send_settings_rsp(sk, MGMT_OP_SET_LE, hdev);
> + goto unlock;
> + }
> +
> + if (old_mode == MGMT_LE_PERIPHERAL || new_mode == MGMT_LE_PERIPHERAL)
> + change_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
>
> - if (val != test_bit(HCI_LE_ENABLED, &hdev->dev_flags)) {
> + if (!hdev_is_powered(hdev)) {
> + if (old_mode == MGMT_LE_OFF || new_mode == MGMT_LE_OFF)
> change_bit(HCI_LE_ENABLED, &hdev->dev_flags);
> - changed = true;
> - }
>
> err = send_settings_rsp(sk, MGMT_OP_SET_LE, hdev);
> if (err < 0)
> goto unlock;
>
> - if (changed)
> - err = new_settings(hdev, sk);
> + err = new_settings(hdev, sk);
>
> goto unlock;
> }
>
> - if (mgmt_pending_find(MGMT_OP_SET_LE, hdev)) {
> - err = cmd_status(sk, hdev->id, MGMT_OP_SET_LE,
> - MGMT_STATUS_BUSY);
> - goto unlock;
> - }
> -
> cmd = mgmt_pending_add(sk, MGMT_OP_SET_LE, hdev, data, len);
> if (!cmd) {
> err = -ENOMEM;
> goto unlock;
> }
>
> + if ((old_mode != MGMT_LE_OFF && new_mode == MGMT_LE_PERIPHERAL) ||
> + old_mode == MGMT_LE_PERIPHERAL) {
> + u8 adv_enable = (new_mode == MGMT_LE_PERIPHERAL);
> +
> + err = hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE,
> + sizeof(adv_enable), &adv_enable);
> + if (err < 0)
> + mgmt_pending_remove(cmd);
> +
> + goto unlock;
> + }
> +
this gets a bit complicated. We either need more comments or split this
out in separate helper functions.
> memset(&hci_cp, 0, sizeof(hci_cp));
>
> - if (val) {
> - hci_cp.le = val;
> + if (old_mode == MGMT_LE_OFF) {
> + hci_cp.le = 1;
> hci_cp.simul = !!(hdev->features[6] & LMP_SIMUL_LE_BR);
> }
>
> - err = hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED, sizeof(hci_cp),
> - &hci_cp);
> + err = hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED,
> + sizeof(hci_cp), &hci_cp);
> if (err < 0)
> mgmt_pending_remove(cmd);
>
> @@ -3525,7 +3577,8 @@ int mgmt_read_local_oob_data_reply_complete(struct hci_dev *hdev, u8 *hash,
>
> int mgmt_le_enable_complete(struct hci_dev *hdev, u8 enable, u8 status)
> {
> - struct cmd_lookup match = { NULL, hdev };
> + struct pending_cmd *cmd;
> + struct mgmt_mode *cp;
> bool changed = false;
> int err = 0;
>
> @@ -3550,17 +3603,67 @@ int mgmt_le_enable_complete(struct hci_dev *hdev, u8 enable, u8 status)
> changed = true;
> }
>
> - mgmt_pending_foreach(MGMT_OP_SET_LE, hdev, settings_rsp, &match);
> + cmd = mgmt_pending_find(MGMT_OP_SET_LE, hdev);
> + if (!cmd)
> + goto done;
> +
> + cp = cmd->param;
> + if (enable && cp->val == MGMT_LE_PERIPHERAL)
> + return hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE,
> + sizeof(enable), &enable);
>
> + send_settings_rsp(cmd->sk, cmd->opcode, hdev);
> + list_del(&cmd->list);
> +
> +done:
> if (changed)
> - err = new_settings(hdev, match.sk);
> + err = new_settings(hdev, cmd ? cmd->sk : NULL);
>
> - if (match.sk)
> - sock_put(match.sk);
> + if (cmd)
> + mgmt_pending_free(cmd);
>
> return err;
> }
>
> +int mgmt_le_adv_enable_complete(struct hci_dev *hdev, u8 enable, u8 status)
> +{
> + struct pending_cmd *cmd;
> + struct mgmt_mode *cp;
> +
> + if (status) {
> + u8 mgmt_err = mgmt_status(status);
> +
> + mgmt_pending_foreach(MGMT_OP_SET_LE, hdev, cmd_status_rsp,
> + &mgmt_err);
> +
> + return 0;
> + }
> +
> + cmd = mgmt_pending_find(MGMT_OP_SET_LE, hdev);
> + if (!cmd) {
> + new_settings(hdev, NULL);
> + return 0;
> + }
> +
> + cp = cmd->param;
> + if (!enable && cp->val == MGMT_LE_OFF) {
> + struct hci_cp_write_le_host_supported hci_cp;
> +
> + memset(&hci_cp, 0, sizeof(hci_cp));
> +
> + return hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED,
> + sizeof(hci_cp), &hci_cp);
> + }
> +
> + new_settings(hdev, cmd->sk);
> + send_settings_rsp(cmd->sk, cmd->opcode, hdev);
> +
> + list_del(&cmd->list);
> + mgmt_pending_free(cmd);
> +
> + return 0;
> +}
> +
> int mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
> u8 addr_type, u8 *dev_class, s8 rssi, u8 cfm_name, u8
> ssp, u8 *eir, u16 eir_len)
Regards
Marcel
^ permalink raw reply
* Re: [PATCHv4 10/18] Bluetooth: Add logical link confirm
From: Marcel Holtmann @ 2012-10-23 18:53 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, sunnyk, andrei.emeltchenko.news
In-Reply-To: <1350682449-24818-11-git-send-email-mathewm@codeaurora.org>
Hi Mat,
> The logical link confirm callback is executed when the AMP controller
> completes its logical link setup. During a channel move, a newly
> formed logical link allows a move responder to send a move channel
> response. A move initiator will send a move channel confirm. A
> failed logical link will end the channel move and send an appropriate
> response or confirm command indicating a failure.
>
> If the channel is being created on an AMP controller, L2CAP
> configuration is completed after the logical link is set up.
>
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/l2cap_core.c | 124 ++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 116 insertions(+), 8 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 69d43c9..0edc955 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -3799,6 +3799,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn,
> goto unlock;
> }
>
> + chan->ident = cmd->ident;
> l2cap_send_cmd(conn, cmd->ident, L2CAP_CONF_RSP, len, rsp);
> chan->num_conf_rsp++;
>
> @@ -4198,17 +4199,17 @@ static int l2cap_create_channel_req(struct l2cap_conn *conn,
> return 0;
> }
>
> -static void l2cap_send_move_chan_rsp(struct l2cap_conn *conn, u8 ident,
> - u16 icid, u16 result)
> +static void l2cap_send_move_chan_rsp(struct l2cap_chan *chan, u16 result)
> {
> struct l2cap_move_chan_rsp rsp;
>
> - BT_DBG("icid 0x%4.4x, result 0x%4.4x", icid, result);
> + BT_DBG("chan %p, result 0x%4.4x", chan, result);
>
> - rsp.icid = cpu_to_le16(icid);
> + rsp.icid = cpu_to_le16(chan->dcid);
> rsp.result = cpu_to_le16(result);
>
> - l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_RSP, sizeof(rsp), &rsp);
> + l2cap_send_cmd(chan->conn, chan->ident, L2CAP_MOVE_CHAN_RSP,
> + sizeof(rsp), &rsp);
> }
>
> static void l2cap_send_move_chan_cfm(struct l2cap_chan *chan, u16 result)
> @@ -4260,11 +4261,114 @@ static void __release_logical_link(struct l2cap_chan *chan)
> /* Placeholder - release the logical link */
> }
>
> +static void l2cap_logical_fail(struct l2cap_chan *chan)
> +{
> + /* Logical link setup failed */
> + if (chan->state != BT_CONNECTED) {
> + /* Create channel failure, disconnect */
> + l2cap_send_disconn_req(chan->conn, chan, ECONNRESET);
lets do this:
if (chan->state != BT_CONNECTED) {
...
return;
}
> + } else if (chan->move_role == L2CAP_MOVE_ROLE_RESPONDER) {
> + l2cap_move_revert(chan);
> + chan->move_role = L2CAP_MOVE_ROLE_NONE;
> + chan->move_state = L2CAP_MOVE_STABLE;
> + l2cap_send_move_chan_rsp(chan, L2CAP_MR_NOT_SUPP);
> + } else if (chan->move_role == L2CAP_MOVE_ROLE_INITIATOR) {
> + if (chan->move_state == L2CAP_MOVE_WAIT_LOGICAL_COMP ||
> + chan->move_state == L2CAP_MOVE_WAIT_LOGICAL_CFM) {
> + /* Remote has only sent pending or
> + * success responses, clean up
> + */
> + l2cap_move_revert(chan);
> + chan->move_role = L2CAP_MOVE_ROLE_NONE;
> + chan->move_state = L2CAP_MOVE_STABLE;
> + }
> +
> + /* Other amp move states imply that the move
> + * has already aborted
> + */
> + l2cap_send_move_chan_cfm(chan, L2CAP_MC_UNCONFIRMED);
> + }
And turn this into a switch statement.
> +
> + __release_logical_link(chan);
And leave this to the caller.
> +}
> +
> +static void l2cap_logical_finish_create(struct l2cap_chan *chan,
> + struct hci_chan *hchan)
> +{
> + struct l2cap_conf_rsp rsp;
> + u8 code;
> +
> + chan->hs_hcon = hchan->conn;
> + chan->hs_hcon->l2cap_data = chan->conn;
> +
> + code = l2cap_build_conf_rsp(chan, &rsp,
> + L2CAP_CONF_SUCCESS, 0);
> + l2cap_send_cmd(chan->conn, chan->ident, L2CAP_CONF_RSP, code,
> + &rsp);
> + set_bit(CONF_OUTPUT_DONE, &chan->conf_state);
> +
> + if (test_bit(CONF_INPUT_DONE, &chan->conf_state)) {
> + int err = 0;
> +
> + set_default_fcs(chan);
> +
> + err = l2cap_ertm_init(chan);
> + if (err < 0)
> + l2cap_send_disconn_req(chan->conn, chan, -err);
> + else
> + l2cap_chan_ready(chan);
> + }
> +}
> +
> +static void l2cap_logical_finish_move(struct l2cap_chan *chan,
> + struct hci_chan *hchan)
> +{
> + chan->hs_hcon = hchan->conn;
> + chan->hs_hcon->l2cap_data = chan->conn;
> +
> + BT_DBG("move_state %d", chan->move_state);
> +
> + switch (chan->move_state) {
> + case L2CAP_MOVE_WAIT_LOGICAL_COMP:
> + /* Move confirm will be sent after a success
> + * response is received
> + */
> + chan->move_state = L2CAP_MOVE_WAIT_RSP_SUCCESS;
> + break;
> + case L2CAP_MOVE_WAIT_LOGICAL_CFM:
> + if (test_bit(CONN_LOCAL_BUSY, &chan->conn_state)) {
> + chan->move_state = L2CAP_MOVE_WAIT_LOCAL_BUSY;
My brain just hurts from these nested if-else. A nested two switch does
not make it any better though. So we can leave it as this. Except the
statement below is used multiple places and we have a function for it.
> + } else if (chan->move_role == L2CAP_MOVE_ROLE_INITIATOR) {
> + chan->move_state = L2CAP_MOVE_WAIT_CONFIRM_RSP;
> + l2cap_send_move_chan_cfm(chan, L2CAP_MC_CONFIRMED);
> + } else if (chan->move_role == L2CAP_MOVE_ROLE_RESPONDER) {
> + chan->move_state = L2CAP_MOVE_WAIT_CONFIRM;
> + l2cap_send_move_chan_rsp(chan, L2CAP_MR_SUCCESS);
> + }
> + break;
> + default:
> + /* Move was not in expected state, free the channel */
> + __release_logical_link(chan);
> +
> + chan->move_state = L2CAP_MOVE_STABLE;
> + }
> +}
> +
> +/* Call with chan locked */
> static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
> u8 status)
> {
> - /* Placeholder */
> - return;
> + BT_DBG("chan %p, hchan %p, status %d", chan, hchan, status);
> +
> + if (status) {
> + l2cap_logical_fail(chan);
I rather have a return here.
if (status) {
l2cap_logical_fail(chan);
__release_logical_link(chan);
return;
}
> + } else if (chan->state != BT_CONNECTED) {
> + /* Ignore logical link if channel is on BR/EDR */
> + if (chan->local_amp_id)
> + l2cap_logical_finish_create(chan, hchan);
> + } else {
> + l2cap_logical_finish_move(chan, hchan);
> + }
> }
>
> static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
> @@ -4272,6 +4376,7 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
> u16 cmd_len, void *data)
> {
> struct l2cap_move_chan_req *req = data;
> + struct l2cap_move_chan_rsp rsp;
> struct l2cap_chan *chan;
> u16 icid = 0;
> u16 result = L2CAP_MR_NOT_ALLOWED;
> @@ -4348,7 +4453,10 @@ static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
> }
>
> send_move_response:
> - l2cap_send_move_chan_rsp(conn, cmd->ident, icid, result);
> + rsp.icid = cpu_to_le16(icid);
> + rsp.result = cpu_to_le16(result);
> + l2cap_send_cmd(conn, cmd->ident, L2CAP_MOVE_CHAN_RSP,
> + sizeof(rsp), &rsp);
>
> if (chan)
> l2cap_chan_unlock(chan);
While not part of this patch, I still dislike if (something) unlock
style. Please have that fixed as well.
Rest looks fine.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 2/7] Bluetooth: mgmt: Restrict BR/EDR settings to BR/EDR-only adapters
From: Anderson Lizardo @ 2012-10-23 18:51 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1351011241-32515-3-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
On Tue, Oct 23, 2012 at 12:53 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> @@ -377,15 +377,15 @@ static u32 get_supported_settings(struct hci_dev *hdev)
> u32 settings = 0;
>
> settings |= MGMT_SETTING_POWERED;
> - settings |= MGMT_SETTING_CONNECTABLE;
> - settings |= MGMT_SETTING_FAST_CONNECTABLE;
> - settings |= MGMT_SETTING_DISCOVERABLE;
> settings |= MGMT_SETTING_PAIRABLE;
>
> if (lmp_ssp_capable(hdev))
> settings |= MGMT_SETTING_SSP;
>
> if (lmp_bredr_capable(hdev)) {
> + settings |= MGMT_SETTING_CONNECTABLE;
> + settings |= MGMT_SETTING_FAST_CONNECTABLE;
> + settings |= MGMT_SETTING_DISCOVERABLE;
> settings |= MGMT_SETTING_BREDR;
> settings |= MGMT_SETTING_LINK_SECURITY;
"Discoverable" (at least as a Discovery mode from GAP) is mandatory
for LE Peripheral role (see page 1698). Same for "Connectable" (see
page 1706).
Unless you have other plans to implement this for single mode LE
peripherals running bluez?
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCH 3/7] Bluetooth: Disallow LE scanning and connecting in peripheral mode
From: Anderson Lizardo @ 2012-10-23 18:42 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1351011241-32515-4-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
On Tue, Oct 23, 2012 at 12:53 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> From: Johan Hedberg <johan.hedberg@intel.com>
>
> When an adapter is in the LE peripheral mode scanning for other devices
> or initiating connections to them is not allowed. This patch makes sure
> that such attempts will result in appropriate error returns.
This will conflict with an Observer role running on the same device.
Multiple roles are allowed to run concurrently if supported by the
controller. From Core spec page 1639:
A device may operate in multiple GAP roles concurrently if supported by the
Controller. The Host should read the supported Link Layer States and State
combinations from the Controller before any procedures or modes are used.
Do you propose SET_LE to become a bitfield to support concurrent
roles? I.e. Broadcaster/Observer.
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCH 4/7] Bluetooth: Add support for setting LE advertising data
From: Anderson Lizardo @ 2012-10-23 18:30 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <1351011241-32515-5-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
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.
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?
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* [PATCH 7/7] Bluetooth: Make use feature test macros
From: Johan Hedberg @ 2012-10-23 16:54 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1351011241-32515-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
For better code readability and avoiding simple bugs of checking the
wrong byte of the features make use of feature test macros whenever
possible.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
include/net/bluetooth/hci_core.h | 8 ++++++++
net/bluetooth/hci_event.c | 26 +++++++++++++-------------
net/bluetooth/mgmt.c | 8 ++++----
3 files changed, 25 insertions(+), 17 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 006aca7..d447f45fa 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -753,14 +753,22 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
/* ----- LMP capabilities ----- */
#define lmp_encrypt_capable(dev) ((dev)->features[0] & LMP_ENCRYPT)
#define lmp_rswitch_capable(dev) ((dev)->features[0] & LMP_RSWITCH)
+#define lmp_hold_capable(dev) ((dev)->features[0] & LMP_HOLD)
#define lmp_sniff_capable(dev) ((dev)->features[0] & LMP_SNIFF)
+#define lmp_park_capable(dev) ((dev)->features[1] & LMP_PARK)
+#define lmp_inq_rssi_capable(dev) ((dev)->features[3] & LMP_RSSI_INQ)
#define lmp_esco_capable(dev) ((dev)->features[3] & LMP_ESCO)
#define lmp_bredr_capable(dev) (!((dev)->features[4] & LMP_NO_BREDR))
#define lmp_le_capable(dev) ((dev)->features[4] & LMP_LE)
#define lmp_sniffsubr_capable(dev) ((dev)->features[5] & LMP_SNIFF_SUBR)
+#define lmp_pause_enc_capable(dev) ((dev)->features[5] & LMP_PAUSE_ENC)
+#define lmp_ext_inq_capable(dev) ((dev)->features[6] & LMP_EXT_INQ)
#define lmp_le_br_capable(dev) ((dev)->features[6] & LMP_SIMUL_LE_BR)
#define lmp_ssp_capable(dev) ((dev)->features[6] & LMP_SIMPLE_PAIR)
#define lmp_no_flush_capable(dev) ((dev)->features[6] & LMP_NO_FLUSH)
+#define lmp_lsto_capable(dev) ((dev)->features[7] & LMP_LSTO)
+#define lmp_inq_tx_pwr_capable(dev) ((dev)->features[7] & LMP_INQ_TX_PWR)
+#define lmp_ext_feat_capable(dev) ((dev)->features[7] & LMP_EXTFEATURES)
/* ----- Extended LMP capabilities ----- */
#define lmp_host_le_capable(dev) ((dev)->host_features[0] & LMP_HOST_LE)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index b63a5f3..0dbdf9b 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -460,10 +460,10 @@ static void hci_cc_write_ssp_mode(struct hci_dev *hdev, struct sk_buff *skb)
static u8 hci_get_inquiry_mode(struct hci_dev *hdev)
{
- if (hdev->features[6] & LMP_EXT_INQ)
+ if (lmp_ext_inq_capable(hdev))
return 2;
- if (hdev->features[3] & LMP_RSSI_INQ)
+ if (lmp_inq_rssi_capable(hdev))
return 1;
if (hdev->manufacturer == 11 && hdev->hci_rev == 0x00 &&
@@ -515,22 +515,22 @@ static void hci_setup_event_mask(struct hci_dev *hdev)
events[5] |= 0x10; /* Synchronous Connection Changed */
}
- if (hdev->features[3] & LMP_RSSI_INQ)
+ if (lmp_inq_rssi_capable(hdev))
events[4] |= 0x02; /* Inquiry Result with RSSI */
if (lmp_sniffsubr_capable(hdev))
events[5] |= 0x20; /* Sniff Subrating */
- if (hdev->features[5] & LMP_PAUSE_ENC)
+ if (lmp_pause_enc_capable(hdev))
events[5] |= 0x80; /* Encryption Key Refresh Complete */
- if (hdev->features[6] & LMP_EXT_INQ)
+ if (lmp_ext_inq_capable(hdev))
events[5] |= 0x40; /* Extended Inquiry Result */
if (lmp_no_flush_capable(hdev))
events[7] |= 0x01; /* Enhanced Flush Complete */
- if (hdev->features[7] & LMP_LSTO)
+ if (lmp_lsto_capable(hdev))
events[6] |= 0x80; /* Link Supervision Timeout Changed */
if (lmp_ssp_capable(hdev)) {
@@ -633,13 +633,13 @@ static void hci_setup(struct hci_dev *hdev)
}
}
- if (hdev->features[3] & LMP_RSSI_INQ)
+ if (lmp_inq_rssi_capable(hdev))
hci_setup_inquiry_mode(hdev);
- if (hdev->features[7] & LMP_INQ_TX_PWR)
+ if (lmp_inq_tx_pwr_capable(hdev))
hci_send_cmd(hdev, HCI_OP_READ_INQ_RSP_TX_POWER, 0, NULL);
- if (hdev->features[7] & LMP_EXTFEATURES) {
+ if (lmp_ext_feat_capable(hdev)) {
struct hci_cp_read_local_ext_features cp;
cp.page = 0x01;
@@ -686,11 +686,11 @@ static void hci_setup_link_policy(struct hci_dev *hdev)
if (lmp_rswitch_capable(hdev))
link_policy |= HCI_LP_RSWITCH;
- if (hdev->features[0] & LMP_HOLD)
+ if (lmp_hold_capable(hdev))
link_policy |= HCI_LP_HOLD;
if (lmp_sniff_capable(hdev))
link_policy |= HCI_LP_SNIFF;
- if (hdev->features[1] & LMP_PARK)
+ if (lmp_park_capable(hdev))
link_policy |= HCI_LP_PARK;
cp.policy = cpu_to_le16(link_policy);
@@ -780,10 +780,10 @@ static void hci_set_le_support(struct hci_dev *hdev)
if (test_bit(HCI_LE_ENABLED, &hdev->dev_flags)) {
cp.le = 1;
- cp.simul = !!(hdev->features[6] & LMP_SIMUL_LE_BR);
+ cp.simul = !!lmp_le_br_capable(hdev);
}
- if (cp.le != !!(hdev->host_features[0] & LMP_HOST_LE))
+ 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))
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index fe77f2c..e024f91 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -572,7 +572,7 @@ static int update_eir(struct hci_dev *hdev)
if (!hdev_is_powered(hdev))
return 0;
- if (!(hdev->features[6] & LMP_EXT_INQ))
+ if (!lmp_ext_inq_capable(hdev))
return 0;
if (!test_bit(HCI_SSP_ENABLED, &hdev->dev_flags))
@@ -1302,7 +1302,7 @@ static int set_hs(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
static u8 get_le_mode(struct hci_dev *hdev)
{
- if (hdev->host_features[0] & LMP_HOST_LE) {
+ if (lmp_host_le_capable(hdev)) {
if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
return MGMT_LE_PERIPHERAL;
return MGMT_LE_CENTRAL;
@@ -1403,7 +1403,7 @@ static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
if (old_mode == MGMT_LE_OFF) {
hci_cp.le = 1;
- hci_cp.simul = !!(hdev->features[6] & LMP_SIMUL_LE_BR);
+ hci_cp.simul = !!lmp_le_br_capable(hdev);
}
err = hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED,
@@ -3510,7 +3510,7 @@ static int clear_eir(struct hci_dev *hdev)
{
struct hci_cp_write_eir cp;
- if (!(hdev->features[6] & LMP_EXT_INQ))
+ if (!lmp_ext_inq_capable(hdev))
return 0;
memset(hdev->eir, 0, sizeof(hdev->eir));
--
1.7.10.4
^ permalink raw reply related
* [PATCH 6/7] Bluetooth: Sort feature test macros by bitmask location
From: Johan Hedberg @ 2012-10-23 16:54 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1351011241-32515-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
This patch sorts the feature test marcos according to the location of
each feature bit in the features bit mask. This helps easy lookup when
adding new marcos and wanting to check if there already is a macro
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
include/net/bluetooth/hci_core.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 3ad3281..006aca7 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -751,16 +751,16 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
#define SET_HCIDEV_DEV(hdev, pdev) ((hdev)->dev.parent = (pdev))
/* ----- LMP capabilities ----- */
-#define lmp_rswitch_capable(dev) ((dev)->features[0] & LMP_RSWITCH)
#define lmp_encrypt_capable(dev) ((dev)->features[0] & LMP_ENCRYPT)
+#define lmp_rswitch_capable(dev) ((dev)->features[0] & LMP_RSWITCH)
#define lmp_sniff_capable(dev) ((dev)->features[0] & LMP_SNIFF)
-#define lmp_sniffsubr_capable(dev) ((dev)->features[5] & LMP_SNIFF_SUBR)
#define lmp_esco_capable(dev) ((dev)->features[3] & LMP_ESCO)
-#define lmp_ssp_capable(dev) ((dev)->features[6] & LMP_SIMPLE_PAIR)
-#define lmp_no_flush_capable(dev) ((dev)->features[6] & LMP_NO_FLUSH)
-#define lmp_le_capable(dev) ((dev)->features[4] & LMP_LE)
#define lmp_bredr_capable(dev) (!((dev)->features[4] & LMP_NO_BREDR))
+#define lmp_le_capable(dev) ((dev)->features[4] & LMP_LE)
+#define lmp_sniffsubr_capable(dev) ((dev)->features[5] & LMP_SNIFF_SUBR)
#define lmp_le_br_capable(dev) ((dev)->features[6] & LMP_SIMUL_LE_BR)
+#define lmp_ssp_capable(dev) ((dev)->features[6] & LMP_SIMPLE_PAIR)
+#define lmp_no_flush_capable(dev) ((dev)->features[6] & LMP_NO_FLUSH)
/* ----- Extended LMP capabilities ----- */
#define lmp_host_le_capable(dev) ((dev)->host_features[0] & LMP_HOST_LE)
--
1.7.10.4
^ permalink raw reply related
* [PATCH 5/7] Bluetooth: Fix updating host feature bits for LE
From: Johan Hedberg @ 2012-10-23 16:53 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1351011241-32515-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
When LE has been enabled with the simultaneous BR/EDR & LE parameter set
to true we should also update the host features stored in struct hci_dev
accordingly.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/hci_event.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index abad39c..b63a5f3 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1332,6 +1332,11 @@ static void hci_cc_write_le_host_supported(struct hci_dev *hdev,
else
hdev->host_features[0] &= ~LMP_HOST_LE;
+ if (sent->simul)
+ hdev->host_features[0] |= LMP_HOST_LE_BREDR;
+ else
+ hdev->host_features[0] &= ~LMP_HOST_LE_BREDR;
+
if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags) &&
test_bit(HCI_INIT, &hdev->flags))
hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE,
--
1.7.10.4
^ permalink raw reply related
* [PATCH 4/7] Bluetooth: Add support for setting LE advertising data
From: Johan Hedberg @ 2012-10-23 16:53 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1351011241-32515-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 | 4 ++
net/bluetooth/hci_event.c | 9 ++++
net/bluetooth/mgmt.c | 97 +++++++++++++++++++++++++++++++++++++-
4 files changed, 124 insertions(+), 1 deletion(-)
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 f288a8c..3ad3281 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);
@@ -758,9 +760,11 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
#define lmp_no_flush_capable(dev) ((dev)->features[6] & LMP_NO_FLUSH)
#define lmp_le_capable(dev) ((dev)->features[4] & LMP_LE)
#define lmp_bredr_capable(dev) (!((dev)->features[4] & LMP_NO_BREDR))
+#define lmp_le_br_capable(dev) ((dev)->features[6] & LMP_SIMUL_LE_BR)
/* ----- Extended LMP capabilities ----- */
#define lmp_host_le_capable(dev) ((dev)->host_features[0] & LMP_HOST_LE)
+#define lmp_host_le_br_capable(dev) ((dev)->host_features[0] & LMP_HOST_LE_BREDR)
/* ----- HCI protocols ----- */
static inline int hci_proto_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr,
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 0393b34..abad39c 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1089,6 +1089,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 eaa6fdc..fe77f2c 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;
@@ -1276,8 +1363,10 @@ static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
goto unlock;
}
- 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)
@@ -2972,6 +3061,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);
@@ -3553,6 +3643,7 @@ send_event:
sizeof(ev), cmd ? cmd->sk : NULL);
update_eir(hdev);
+ update_ad(hdev);
failed:
if (cmd)
@@ -3627,6 +3718,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);
@@ -3652,6 +3745,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
* [PATCH 3/7] Bluetooth: Disallow LE scanning and connecting in peripheral mode
From: Johan Hedberg @ 2012-10-23 16:53 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1351011241-32515-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
When an adapter is in the LE peripheral mode scanning for other devices
or initiating connections to them is not allowed. This patch makes sure
that such attempts will result in appropriate error returns.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/hci_conn.c | 3 +++
net/bluetooth/hci_core.c | 3 +++
2 files changed, 6 insertions(+)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index fe64621..a75b30e 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -502,6 +502,9 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
{
struct hci_conn *le;
+ if (test_bit(HCI_LE_PERIPHERAL, &hdev->flags))
+ return ERR_PTR(-ENOTSUPP);
+
le = hci_conn_hash_lookup_ba(hdev, LE_LINK, dst);
if (!le) {
le = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 487308b..b2adef4 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1575,6 +1575,9 @@ int hci_le_scan(struct hci_dev *hdev, u8 type, u16 interval, u16 window,
BT_DBG("%s", hdev->name);
+ if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
+ return -ENOTSUPP;
+
if (work_busy(&hdev->le_scan))
return -EINPROGRESS;
--
1.7.10.4
^ permalink raw reply related
* [PATCH 2/7] Bluetooth: mgmt: Restrict BR/EDR settings to BR/EDR-only adapters
From: Johan Hedberg @ 2012-10-23 16:53 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1351011241-32515-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
This patch makes sure that settings which are specific for BR/EDR
capable adapters are not allowed for non-BR/EDR (e.g. LE-only) adapters.
Instead, a "not supported" error is returned of such a setting is
attempted to be set for a non-BR/EDR adapter.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/mgmt.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 1d79979..eaa6fdc 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -377,15 +377,15 @@ static u32 get_supported_settings(struct hci_dev *hdev)
u32 settings = 0;
settings |= MGMT_SETTING_POWERED;
- settings |= MGMT_SETTING_CONNECTABLE;
- settings |= MGMT_SETTING_FAST_CONNECTABLE;
- settings |= MGMT_SETTING_DISCOVERABLE;
settings |= MGMT_SETTING_PAIRABLE;
if (lmp_ssp_capable(hdev))
settings |= MGMT_SETTING_SSP;
if (lmp_bredr_capable(hdev)) {
+ settings |= MGMT_SETTING_CONNECTABLE;
+ settings |= MGMT_SETTING_FAST_CONNECTABLE;
+ settings |= MGMT_SETTING_DISCOVERABLE;
settings |= MGMT_SETTING_BREDR;
settings |= MGMT_SETTING_LINK_SECURITY;
}
@@ -874,6 +874,10 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
BT_DBG("request for %s", hdev->name);
+ if (!lmp_bredr_capable(hdev))
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_DISCOVERABLE,
+ MGMT_STATUS_NOT_SUPPORTED);
+
timeout = __le16_to_cpu(cp->timeout);
if (!cp->val && timeout > 0)
return cmd_status(sk, hdev->id, MGMT_OP_SET_DISCOVERABLE,
@@ -969,6 +973,10 @@ static int set_connectable(struct sock *sk, struct hci_dev *hdev, void *data,
BT_DBG("request for %s", hdev->name);
+ if (!lmp_bredr_capable(hdev))
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_CONNECTABLE,
+ MGMT_STATUS_NOT_SUPPORTED);
+
hci_dev_lock(hdev);
if (!hdev_is_powered(hdev)) {
@@ -1067,6 +1075,10 @@ static int set_link_security(struct sock *sk, struct hci_dev *hdev, void *data,
BT_DBG("request for %s", hdev->name);
+ if (!lmp_bredr_capable(hdev))
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_LINK_SECURITY,
+ MGMT_STATUS_NOT_SUPPORTED);
+
hci_dev_lock(hdev);
if (!hdev_is_powered(hdev)) {
@@ -2647,6 +2659,10 @@ static int set_fast_connectable(struct sock *sk, struct hci_dev *hdev,
BT_DBG("%s", hdev->name);
+ if (!lmp_bredr_capable(hdev))
+ return cmd_status(sk, hdev->id, MGMT_OP_SET_FAST_CONNECTABLE,
+ MGMT_STATUS_NOT_SUPPORTED);
+
if (!hdev_is_powered(hdev))
return cmd_status(sk, hdev->id, MGMT_OP_SET_FAST_CONNECTABLE,
MGMT_STATUS_NOT_POWERED);
--
1.7.10.4
^ permalink raw reply related
* [PATCH 1/7] Bluetooth: mgmt: Add support for switching to LE peripheral mode
From: Johan Hedberg @ 2012-10-23 16:53 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1351011241-32515-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
This patch extends the mgmt_set_le command to allow for a new value
(0x02) to mean that LE should be enabled together with advertising
enabled. This paves the way for acting in a fully functional LE
peripheral role. The change to the mgmt_set_le command is fully
backwards compatible as the value 0x01 means LE central role (which was
the old behavior).
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
include/net/bluetooth/hci.h | 3 +
include/net/bluetooth/hci_core.h | 1 +
include/net/bluetooth/mgmt.h | 6 ++
net/bluetooth/hci_event.c | 44 +++++++++++
net/bluetooth/mgmt.c | 159 +++++++++++++++++++++++++++++++-------
5 files changed, 185 insertions(+), 28 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 348f4bf..a633da0 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -115,6 +115,7 @@ enum {
HCI_SSP_ENABLED,
HCI_HS_ENABLED,
HCI_LE_ENABLED,
+ HCI_LE_PERIPHERAL,
HCI_CONNECTABLE,
HCI_DISCOVERABLE,
HCI_LINK_SECURITY,
@@ -938,6 +939,8 @@ struct hci_rp_le_read_adv_tx_power {
__s8 tx_power;
} __packed;
+#define HCI_OP_LE_SET_ADV_ENABLE 0x200a
+
#define HCI_OP_LE_SET_SCAN_PARAM 0x200b
struct hci_cp_le_set_scan_param {
__u8 type;
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 8260bf2..f288a8c 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1075,6 +1075,7 @@ int mgmt_set_local_name_complete(struct hci_dev *hdev, u8 *name, u8 status);
int mgmt_read_local_oob_data_reply_complete(struct hci_dev *hdev, u8 *hash,
u8 *randomizer, u8 status);
int mgmt_le_enable_complete(struct hci_dev *hdev, u8 enable, u8 status);
+int mgmt_le_adv_enable_complete(struct hci_dev *hdev, u8 enable, u8 status);
int mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
u8 addr_type, u8 *dev_class, s8 rssi, u8 cfm_name,
u8 ssp, u8 *eir, u16 eir_len);
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 22980a7..26c9a4d 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -92,6 +92,7 @@ struct mgmt_rp_read_index_list {
#define MGMT_SETTING_BREDR 0x00000080
#define MGMT_SETTING_HS 0x00000100
#define MGMT_SETTING_LE 0x00000200
+#define MGMT_SETTING_PERIPHERAL 0x00000400
#define MGMT_OP_READ_INFO 0x0004
#define MGMT_READ_INFO_SIZE 0
@@ -134,6 +135,11 @@ struct mgmt_cp_set_discoverable {
#define MGMT_OP_SET_HS 0x000C
#define MGMT_OP_SET_LE 0x000D
+
+#define MGMT_LE_OFF 0x00
+#define MGMT_LE_CENTRAL 0x01
+#define MGMT_LE_PERIPHERAL 0x02
+
#define MGMT_OP_SET_DEV_CLASS 0x000E
struct mgmt_cp_set_dev_class {
__u8 major;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index fd5a51c..0393b34 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -786,6 +786,9 @@ static void hci_set_le_support(struct hci_dev *hdev)
if (cp.le != !!(hdev->host_features[0] & LMP_HOST_LE))
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);
}
static void hci_cc_read_local_ext_features(struct hci_dev *hdev,
@@ -1189,6 +1192,38 @@ static void hci_cc_le_set_scan_param(struct hci_dev *hdev, struct sk_buff *skb)
}
}
+static void hci_cc_le_set_adv_enable(struct hci_dev *hdev, struct sk_buff *skb)
+{
+ __u8 *sent, status = *((__u8 *) skb->data);
+
+ BT_DBG("%s status 0x%2.2x", hdev->name, status);
+
+ sent = hci_sent_cmd_data(hdev, HCI_OP_LE_SET_ADV_ENABLE);
+ if (!sent)
+ return;
+
+ hci_dev_lock(hdev);
+
+ if (!status) {
+ if (*sent)
+ set_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
+ else
+ clear_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
+ } else {
+ if (*sent)
+ clear_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
+ else
+ set_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
+ }
+
+ if (!test_bit(HCI_INIT, &hdev->flags))
+ mgmt_le_adv_enable_complete(hdev, *sent, status);
+
+ hci_dev_unlock(hdev);
+
+ hci_req_complete(hdev, HCI_OP_LE_SET_ADV_ENABLE, status);
+}
+
static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
struct sk_buff *skb)
{
@@ -1287,6 +1322,11 @@ static void hci_cc_write_le_host_supported(struct hci_dev *hdev,
hdev->host_features[0] |= LMP_HOST_LE;
else
hdev->host_features[0] &= ~LMP_HOST_LE;
+
+ if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags) &&
+ test_bit(HCI_INIT, &hdev->flags))
+ hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE,
+ sizeof(sent->le), &sent->le);
}
if (test_bit(HCI_MGMT, &hdev->dev_flags) &&
@@ -2549,6 +2589,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
hci_cc_le_set_scan_param(hdev, skb);
break;
+ case HCI_OP_LE_SET_ADV_ENABLE:
+ hci_cc_le_set_adv_enable(hdev, skb);
+ break;
+
case HCI_OP_LE_SET_SCAN_ENABLE:
hci_cc_le_set_scan_enable(hdev, skb);
break;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 3fe74f4..1d79979 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -393,8 +393,10 @@ static u32 get_supported_settings(struct hci_dev *hdev)
if (enable_hs)
settings |= MGMT_SETTING_HS;
- if (lmp_le_capable(hdev))
+ if (lmp_le_capable(hdev)) {
settings |= MGMT_SETTING_LE;
+ settings |= MGMT_SETTING_PERIPHERAL;
+ }
return settings;
}
@@ -418,9 +420,13 @@ static u32 get_current_settings(struct hci_dev *hdev)
if (lmp_bredr_capable(hdev))
settings |= MGMT_SETTING_BREDR;
- if (test_bit(HCI_LE_ENABLED, &hdev->dev_flags))
+ if (test_bit(HCI_LE_ENABLED, &hdev->dev_flags)) {
settings |= MGMT_SETTING_LE;
+ if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
+ settings |= MGMT_SETTING_PERIPHERAL;
+ }
+
if (test_bit(HCI_LINK_SECURITY, &hdev->dev_flags))
settings |= MGMT_SETTING_LINK_SECURITY;
@@ -1195,13 +1201,36 @@ static int set_hs(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
return send_settings_rsp(sk, MGMT_OP_SET_HS, hdev);
}
+static u8 get_le_mode(struct hci_dev *hdev)
+{
+ if (hdev->host_features[0] & LMP_HOST_LE) {
+ if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
+ return MGMT_LE_PERIPHERAL;
+ return MGMT_LE_CENTRAL;
+ }
+
+ return MGMT_LE_OFF;
+}
+
+static bool valid_le_mode(u8 mode)
+{
+ switch (mode) {
+ case MGMT_LE_OFF:
+ case MGMT_LE_CENTRAL:
+ case MGMT_LE_PERIPHERAL:
+ return true;
+ default:
+ return false;
+ }
+}
+
static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
{
- struct mgmt_mode *cp = data;
struct hci_cp_write_le_host_supported hci_cp;
+ struct mgmt_mode *cp = data;
struct pending_cmd *cmd;
+ u8 old_mode, new_mode;
int err;
- u8 val, enabled;
BT_DBG("request for %s", hdev->name);
@@ -1213,48 +1242,71 @@ static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
goto unlock;
}
- val = !!cp->val;
- enabled = !!(hdev->host_features[0] & LMP_HOST_LE);
+ if (!valid_le_mode(cp->val)) {
+ err = cmd_status(sk, hdev->id, MGMT_OP_SET_LE,
+ MGMT_STATUS_INVALID_PARAMS);
+ goto unlock;
+ }
- if (!hdev_is_powered(hdev) || val == enabled) {
- bool changed = false;
+ if (mgmt_pending_find(MGMT_OP_SET_LE, hdev)) {
+ err = cmd_status(sk, hdev->id, MGMT_OP_SET_LE,
+ MGMT_STATUS_BUSY);
+ goto unlock;
+ }
+
+ new_mode = cp->val;
+ old_mode = get_le_mode(hdev);
+
+ BT_DBG("%s old_mode %u new_mode %u", hdev->name, old_mode, new_mode);
+
+ if (new_mode == old_mode) {
+ err = send_settings_rsp(sk, MGMT_OP_SET_LE, hdev);
+ goto unlock;
+ }
+
+ if (old_mode == MGMT_LE_PERIPHERAL || new_mode == MGMT_LE_PERIPHERAL)
+ change_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags);
- if (val != test_bit(HCI_LE_ENABLED, &hdev->dev_flags)) {
+ if (!hdev_is_powered(hdev)) {
+ if (old_mode == MGMT_LE_OFF || new_mode == MGMT_LE_OFF)
change_bit(HCI_LE_ENABLED, &hdev->dev_flags);
- changed = true;
- }
err = send_settings_rsp(sk, MGMT_OP_SET_LE, hdev);
if (err < 0)
goto unlock;
- if (changed)
- err = new_settings(hdev, sk);
+ err = new_settings(hdev, sk);
goto unlock;
}
- if (mgmt_pending_find(MGMT_OP_SET_LE, hdev)) {
- err = cmd_status(sk, hdev->id, MGMT_OP_SET_LE,
- MGMT_STATUS_BUSY);
- goto unlock;
- }
-
cmd = mgmt_pending_add(sk, MGMT_OP_SET_LE, hdev, data, len);
if (!cmd) {
err = -ENOMEM;
goto unlock;
}
+ if ((old_mode != MGMT_LE_OFF && new_mode == MGMT_LE_PERIPHERAL) ||
+ old_mode == MGMT_LE_PERIPHERAL) {
+ u8 adv_enable = (new_mode == MGMT_LE_PERIPHERAL);
+
+ err = hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE,
+ sizeof(adv_enable), &adv_enable);
+ if (err < 0)
+ mgmt_pending_remove(cmd);
+
+ goto unlock;
+ }
+
memset(&hci_cp, 0, sizeof(hci_cp));
- if (val) {
- hci_cp.le = val;
+ if (old_mode == MGMT_LE_OFF) {
+ hci_cp.le = 1;
hci_cp.simul = !!(hdev->features[6] & LMP_SIMUL_LE_BR);
}
- err = hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED, sizeof(hci_cp),
- &hci_cp);
+ err = hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED,
+ sizeof(hci_cp), &hci_cp);
if (err < 0)
mgmt_pending_remove(cmd);
@@ -3525,7 +3577,8 @@ int mgmt_read_local_oob_data_reply_complete(struct hci_dev *hdev, u8 *hash,
int mgmt_le_enable_complete(struct hci_dev *hdev, u8 enable, u8 status)
{
- struct cmd_lookup match = { NULL, hdev };
+ struct pending_cmd *cmd;
+ struct mgmt_mode *cp;
bool changed = false;
int err = 0;
@@ -3550,17 +3603,67 @@ int mgmt_le_enable_complete(struct hci_dev *hdev, u8 enable, u8 status)
changed = true;
}
- mgmt_pending_foreach(MGMT_OP_SET_LE, hdev, settings_rsp, &match);
+ cmd = mgmt_pending_find(MGMT_OP_SET_LE, hdev);
+ if (!cmd)
+ goto done;
+
+ cp = cmd->param;
+ if (enable && cp->val == MGMT_LE_PERIPHERAL)
+ return hci_send_cmd(hdev, HCI_OP_LE_SET_ADV_ENABLE,
+ sizeof(enable), &enable);
+ send_settings_rsp(cmd->sk, cmd->opcode, hdev);
+ list_del(&cmd->list);
+
+done:
if (changed)
- err = new_settings(hdev, match.sk);
+ err = new_settings(hdev, cmd ? cmd->sk : NULL);
- if (match.sk)
- sock_put(match.sk);
+ if (cmd)
+ mgmt_pending_free(cmd);
return err;
}
+int mgmt_le_adv_enable_complete(struct hci_dev *hdev, u8 enable, u8 status)
+{
+ struct pending_cmd *cmd;
+ struct mgmt_mode *cp;
+
+ if (status) {
+ u8 mgmt_err = mgmt_status(status);
+
+ mgmt_pending_foreach(MGMT_OP_SET_LE, hdev, cmd_status_rsp,
+ &mgmt_err);
+
+ return 0;
+ }
+
+ cmd = mgmt_pending_find(MGMT_OP_SET_LE, hdev);
+ if (!cmd) {
+ new_settings(hdev, NULL);
+ return 0;
+ }
+
+ cp = cmd->param;
+ if (!enable && cp->val == MGMT_LE_OFF) {
+ struct hci_cp_write_le_host_supported hci_cp;
+
+ memset(&hci_cp, 0, sizeof(hci_cp));
+
+ return hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED,
+ sizeof(hci_cp), &hci_cp);
+ }
+
+ new_settings(hdev, cmd->sk);
+ send_settings_rsp(cmd->sk, cmd->opcode, hdev);
+
+ list_del(&cmd->list);
+ mgmt_pending_free(cmd);
+
+ return 0;
+}
+
int mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
u8 addr_type, u8 *dev_class, s8 rssi, u8 cfm_name, u8
ssp, u8 *eir, u16 eir_len)
--
1.7.10.4
^ permalink raw reply related
* [PATCH 0/7] Bluetooth: Improved single-mode and LE peripheral support
From: Johan Hedberg @ 2012-10-23 16:53 UTC (permalink / raw)
To: linux-bluetooth
Hi,
This set of patches builds on top of my previous one. It introduces a
few more fixes for single-mode controllers as well as adds a new feature
to the management interface to switch a controller to LE peripheral
mode. User space already has code to support this and you can enable
the new mode using "tools/btmgmt le peripheral".
Johan
^ permalink raw reply
* Re: [PATCH] Bluetooth: Add support for Atheros [04ca:3004]
From: Dwaine Garden VE3GIF @ 2012-10-23 16:05 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1351003333.1785.26.camel@aeonflux>
Thanks. =A0I will get the right output and fix the tabbing probleem; resend=
ing the patch again.=0A=0ADwaine=0A=0A=0A=0A----- Original Message -----=0A=
From: Marcel Holtmann <marcel@holtmann.org>=0ATo: Dwaine Garden VE3GIF <dwa=
inegarden@rogers.com>=0ACc: "linux-bluetooth@vger.kernel.org" <linux-blueto=
oth@vger.kernel.org>=0ASent: Tuesday, October 23, 2012 10:42:13 AM=0ASubjec=
t: Re: [PATCH] Bluetooth: Add support for Atheros [04ca:3004]=0A=0AHi Dwain=
e,=0A=0A> Add another vendor specific ID for Atheros AR3012 device.=0A> Thi=
s chip is wrapped by Lite-On Technology Corp.=0A> =0A> output of usb-device=
s:=0A> Bus 001 Device 008: ID 04ca:3004 Lite-On Technology Corp. =0A> Devic=
e Descriptor:=0A>=A0 bLength=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 18=0A> =0A> bD=
escriptorType=A0 =A0 =A0 =A0 1=0A>=A0 bcdUSB=A0 =A0 =A0 =A0 =A0 =A0 =A0 =
1.10=0A>=A0 bDeviceClass=A0 =A0 =A0 =A0 =A0 224 Wireless=0A>=A0 bDeviceSu=
bClass=A0 =A0 =A0 =A0 1 Radio Frequency=0A>=A0 bDeviceProtocol=A0 =A0 =A0=
=A0 1 Bluetooth=0A>=A0 bMaxPacketSize0=A0 =A0 =A0 =A0 64=0A>=A0 idVendo=
r=A0 =A0 =A0 =A0 =A0 0x04ca Lite-On Technology Corp.=0A>=A0 idProduct=A0 =
=A0 =A0 =A0 =A0 0x3004 =0A>=A0 bcdDevice=A0 =A0 =A0 =A0 =A0 =A0 0.02=0A>=
=A0 iManufacturer=A0 =A0 =A0 =A0 =A0 1 Atheros Communications=0A> =0A> iP=
roduct=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 2 Bluetooth USB Host Controller=0A>=
=A0 iSerial=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 3 Alaska Day 2006=0A>=A0 bNum=
Configurations=A0 =A0 =A0 1=0A=0AI prefer /sys/kernel/debug/usb/devices out=
put here.=0A=0A> =0A> =0A> Signed-off-by: Dwaine Garden <DwaineGarden@roger=
s.com>=A0 ---=0A> =0A> diff --git a/drivers/bluetooth/ath3k.c b/drivers/blu=
etooth/ath3k.c=0A> index fc2de55..1486f15 100644=0A> --- a/drivers/bluetoot=
h/ath3k.c=0A> +++ b/drivers/bluetooth/ath3k.c=0A> @@ -75,6 +75,7 @@ static =
struct usb_device_id ath3k_table[] =3D {=0A> { USB_DEVICE(0x0CF3, 0x3004) }=
,=0A> { USB_DEVICE(0x0CF3, 0x311D) },=0A> { USB_DEVICE(0x13d3, 0x3375) },=
=0A> +=A0 =A0 { USB_DEVICE(0x04CA, 0x3004) },=0A=0AAnd you need to use an e=
mail client that does not mess up the tabs vs=0Awhite spaces.=0A=0ARegards=
=0A=0AMarcel
^ permalink raw reply
* Re: [PATCH v1 5/5] Bluetooth: Fix L2CAP dynamic PSM bind issue
From: Marcel Holtmann @ 2012-10-23 14:52 UTC (permalink / raw)
To: Syam Sidhardhan; +Cc: linux-bluetooth
In-Reply-To: <1350999140-7481-5-git-send-email-s.syam@samsung.com>
Hi Syam,
> Dynamic PSM choosen by the kernel should bound to either BDADDR_ANY
> or the same adapter address(not both) for a particular adapter.
>
> The problem here is by giving PSM 0, the kernel should return the first
> available PSM in the non-reserverd space, but since we reuse the same
> code to do the matching it end up given both Obexd and HDP the same
> PSM.
>
> Provid a helper function to handle the dymanic PSM auto selection.
>
> Signed-off-by: Syam Sidhardhan <s.syam@samsung.com>
> ---
> net/bluetooth/l2cap_core.c | 55 ++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 46 insertions(+), 9 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index d42cdb1..33b5d34 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -119,6 +119,43 @@ static struct l2cap_chan *__l2cap_global_chan_by_addr(__le16 psm, bdaddr_t *src)
> return NULL;
> }
>
> +/* Returns free dynamic PSM */
> +static u16 __l2cap_global_get_dyna_chan_by_addr(bdaddr_t *src)
> +{
this is a bad name. No idea what kind of meaning "dyna" has.
And btw. there is no such thing as dynamic PSM. It has nothing dynamic
about it. It is an auto-selected PSM. And in the end, it just selects
the next free one.
> + struct l2cap_chan *c;
> + u16 p;
> + bool found;
> +
> + for (p = 0x1001; p < 0x1100; p += 2) {
> + found = false;
> +
> + list_for_each_entry(c, &chan_list, global_l) {
> + if (c->sport != p)
> + continue;
> +
> + /* PSM match found */
> + found = true;
> +
> + /* Exact match */
> + if (!bacmp(&bt_sk(c->sk)->src, src))
> + break;
> +
> + /* BDADDR_ANY match */
> + if (!bacmp(&bt_sk(c->sk)->src, BDADDR_ANY) ||
> + !bacmp(src, BDADDR_ANY))
> + break;
> +
> + /* Match found only for other adapter address */
> + return p;
> + }
> +
> + if (!found)
> + return p;
> + }
> +
> + return 0;
> +}
> +
This code needs a comment on how it is suppose to work and why it is
correct.
> int l2cap_add_psm(struct l2cap_chan *chan, bdaddr_t *src, __le16 psm)
> {
> int err;
> @@ -135,16 +172,16 @@ int l2cap_add_psm(struct l2cap_chan *chan, bdaddr_t *src, __le16 psm)
> chan->sport = psm;
> err = 0;
> } else {
> - u16 p;
> -
> + /* No PSM given by user space */
> + u16 dpsm;
> err = -EINVAL;
> - for (p = 0x1001; p < 0x1100; p += 2)
> - if (!__l2cap_global_chan_by_addr(cpu_to_le16(p), src)) {
> - chan->psm = cpu_to_le16(p);
> - chan->sport = cpu_to_le16(p);
> - err = 0;
> - break;
> - }
> +
> + dpsm = __l2cap_global_get_dyna_chan_by_addr(src);
> + if (dpsm) {
> + chan->psm = dpsm;
> + chan->sport = dpsm;
> + err = 0;
> + }
> }
>
> done:
Regards
Marcel
^ 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