Linux bluetooth development
 help / color / mirror / Atom feed
From: "Gustavo F. Padovan" <padovan@profusion.mobi>
To: Jaikumar Ganesh <jaikumarg@gmail.com>
Cc: linux-bluetooth@vger.kernel.org
Subject: Re: [PATCH] Bluetooth: Add BT_POWER L2CAP socket option.
Date: Wed, 1 Jun 2011 19:20:49 -0300	[thread overview]
Message-ID: <20110601222048.GJ2564@joana> (raw)
In-Reply-To: <BANLkTi=VyYixDBp33mANPwjiOtCwitTM_A@mail.gmail.com>

* Jaikumar Ganesh <jaikumarg@gmail.com> [2011-05-31 18:53:51 -0700]:

> Gustavo,
> 
> On Mon, May 30, 2011 at 3:11 PM, Gustavo F. Padovan
> <padovan@profusion.mobi> wrote:
> > Hi Jaikumar,
> >
> > * Jaikumar Ganesh <jaikumar@google.com> [2011-05-23 18:06:04 -0700]:
> >
> >> Add BT_POWER socket option used to control the power
> >> characteristics of the underlying ACL link. When the remote end
> >> has put the link in sniff mode and the host stack wants to send
> >> data we need need to explicitly exit sniff mode to work well with
> >> certain devices (For example, A2DP on Plantronics Voyager 855).
> >> However, this causes problems with HID devices.
> >>
> >> Hence, moving into active mode when sending data, irrespective
> >> of who set the sniff mode has been made as a socket option. By
> >> default, we will move into active mode. HID devices can set the
> >> L2CAP socket option to prevent this from happening.
> >>
> >> Currently, this has been implemented for L2CAP sockets. This has been
> >> tested with incoming and outgoing L2CAP sockets for HID and A2DP.
> >>
> >> Based on discussions on linux-bluetooth and patches submitted by
> >> Andrei Emeltchenko.
> >>
> >> Signed-off-by: Jaikumar Ganesh <jaikumar@google.com>
> >> ---
> >>  include/net/bluetooth/bluetooth.h |    8 ++++++++
> >>  include/net/bluetooth/hci_core.h  |    2 +-
> >>  include/net/bluetooth/l2cap.h     |    1 +
> >>  net/bluetooth/hci_conn.c          |    9 ++++++---
> >>  net/bluetooth/hci_core.c          |    4 ++--
> >>  net/bluetooth/l2cap_core.c        |    5 +++++
> >>  net/bluetooth/l2cap_sock.c        |   36 ++++++++++++++++++++++++++++++++++++
> >>  7 files changed, 59 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> >> index 4375043..af930a3 100644
> >> --- a/include/net/bluetooth/bluetooth.h
> >> +++ b/include/net/bluetooth/bluetooth.h
> >> @@ -69,6 +69,13 @@ struct bt_security {
> >>  #define BT_FLUSHABLE_OFF     0
> >>  #define BT_FLUSHABLE_ON              1
> >>
> >> +#define BT_POWER     9
> >> +struct bt_power {
> >> +     __u8 force_active;
> >> +};
> >> +#define BT_POWER_FORCE_ACTIVE_OFF 0
> >> +#define BT_POWER_FORCE_ACTIVE_ON  1
> >> +
> >>  #define BT_INFO(fmt, arg...) printk(KERN_INFO "Bluetooth: " fmt "\n" , ## arg)
> >>  #define BT_ERR(fmt, arg...)  printk(KERN_ERR "%s: " fmt "\n" , __func__ , ## arg)
> >>  #define BT_DBG(fmt, arg...)  pr_debug("%s: " fmt "\n" , __func__ , ## arg)
> >> @@ -150,6 +157,7 @@ struct bt_skb_cb {
> >>       __u8 retries;
> >>       __u8 sar;
> >>       unsigned short channel;
> >> +     __u8 force_active;
> >>  };
> >>  #define bt_cb(skb) ((struct bt_skb_cb *)((skb)->cb))
> >>
> >> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> >> index 6c994c0..0747841 100644
> >> --- a/include/net/bluetooth/hci_core.h
> >> +++ b/include/net/bluetooth/hci_core.h
> >> @@ -427,7 +427,7 @@ int hci_conn_security(struct hci_conn *conn, __u8 sec_level, __u8 auth_type);
> >>  int hci_conn_change_link_key(struct hci_conn *conn);
> >>  int hci_conn_switch_role(struct hci_conn *conn, __u8 role);
> >>
> >> -void hci_conn_enter_active_mode(struct hci_conn *conn);
> >> +void hci_conn_enter_active_mode(struct hci_conn *conn, __u8 force_active);
> >>  void hci_conn_enter_sniff_mode(struct hci_conn *conn);
> >>
> >>  void hci_conn_hold_device(struct hci_conn *conn);
> >> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> >> index d09c9b1..942970d 100644
> >> --- a/include/net/bluetooth/l2cap.h
> >> +++ b/include/net/bluetooth/l2cap.h
> >> @@ -302,6 +302,7 @@ struct l2cap_chan {
> >>       __u8            role_switch;
> >>       __u8            force_reliable;
> >>       __u8            flushable;
> >> +     __u8            force_active;
> >>
> >>       __u8            ident;
> >>
> >> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> >> index 3163330..5195715 100644
> >> --- a/net/bluetooth/hci_conn.c
> >> +++ b/net/bluetooth/hci_conn.c
> >> @@ -497,7 +497,7 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
> >>       if (acl->state == BT_CONNECTED &&
> >>                       (sco->state == BT_OPEN || sco->state == BT_CLOSED)) {
> >>               acl->power_save = 1;
> >> -             hci_conn_enter_active_mode(acl);
> >> +             hci_conn_enter_active_mode(acl, BT_POWER_FORCE_ACTIVE_ON);
> >>
> >>               if (test_bit(HCI_CONN_MODE_CHANGE_PEND, &acl->pend)) {
> >>                       /* defer SCO setup until mode change completed */
> >> @@ -676,7 +676,7 @@ int hci_conn_switch_role(struct hci_conn *conn, __u8 role)
> >>  EXPORT_SYMBOL(hci_conn_switch_role);
> >>
> >>  /* Enter active mode */
> >> -void hci_conn_enter_active_mode(struct hci_conn *conn)
> >> +void hci_conn_enter_active_mode(struct hci_conn *conn, __u8 force_active)
> >>  {
> >>       struct hci_dev *hdev = conn->hdev;
> >>
> >> @@ -685,7 +685,10 @@ void hci_conn_enter_active_mode(struct hci_conn *conn)
> >>       if (test_bit(HCI_RAW, &hdev->flags))
> >>               return;
> >>
> >> -     if (conn->mode != HCI_CM_SNIFF || !conn->power_save)
> >> +     if (conn->mode != HCI_CM_SNIFF)
> >> +             goto timer;
> >> +
> >> +     if (!conn->power_save && !force_active)
> >>               goto timer;
> >>
> >>       if (!test_and_set_bit(HCI_CONN_MODE_CHANGE_PEND, &conn->pend)) {
> >> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> >> index b6bda3f..be68e7d 100644
> >> --- a/net/bluetooth/hci_core.c
> >> +++ b/net/bluetooth/hci_core.c
> >> @@ -1893,7 +1893,7 @@ static inline void hci_sched_acl(struct hci_dev *hdev)
> >>               while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
> >>                       BT_DBG("skb %p len %d", skb, skb->len);
> >>
> >> -                     hci_conn_enter_active_mode(conn);
> >> +                     hci_conn_enter_active_mode(conn, bt_cb(skb)->force_active);
> >>
> >>                       hci_send_frame(skb);
> >>                       hdev->acl_last_tx = jiffies;
> >> @@ -2032,7 +2032,7 @@ static inline void hci_acldata_packet(struct hci_dev *hdev, struct sk_buff *skb)
> >>       if (conn) {
> >>               register struct hci_proto *hp;
> >>
> >> -             hci_conn_enter_active_mode(conn);
> >> +             hci_conn_enter_active_mode(conn, bt_cb(skb)->force_active);
> >>
> >>               /* Send to upper protocol */
> >>               hp = hci_proto[HCI_PROTO_L2CAP];
> >> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> >> index 7c0e571..2a80a91 100644
> >> --- a/net/bluetooth/l2cap_core.c
> >> +++ b/net/bluetooth/l2cap_core.c
> >> @@ -408,6 +408,8 @@ void l2cap_send_cmd(struct l2cap_conn *conn, u8 ident, u8 code, u16 len, void *d
> >>       else
> >>               flags = ACL_START;
> >>
> >> +     bt_cb(skb)->force_active = BT_POWER_FORCE_ACTIVE_ON;
> >> +
> >>       hci_send_acl(conn->hcon, skb, flags);
> >>  }
> >>
> >> @@ -461,6 +463,8 @@ static inline void l2cap_send_sframe(struct l2cap_chan *chan, u16 control)
> >>       else
> >>               flags = ACL_START;
> >>
> >> +     bt_cb(skb)->force_active = chan->force_active;
> >> +
> >>       hci_send_acl(chan->conn->hcon, skb, flags);
> >>  }
> >>
> >> @@ -1089,6 +1093,7 @@ void l2cap_do_send(struct l2cap_chan *chan, struct sk_buff *skb)
> >>       else
> >>               flags = ACL_START;
> >>
> >> +     bt_cb(skb)->force_active = chan->force_active;
> >>       hci_send_acl(hcon, skb, flags);
> >>  }
> >>
> >> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> >> index c98360d..9f99e1d 100644
> >> --- a/net/bluetooth/l2cap_sock.c
> >> +++ b/net/bluetooth/l2cap_sock.c
> >> @@ -436,6 +436,7 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname, ch
> >>       struct sock *sk = sock->sk;
> >>       struct l2cap_chan *chan = l2cap_pi(sk)->chan;
> >>       struct bt_security sec;
> >> +     struct bt_power pwr;
> >>       int len, err = 0;
> >>
> >>       BT_DBG("sk %p", sk);
> >> @@ -484,6 +485,21 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname, ch
> >>
> >>               break;
> >>
> >> +     case BT_POWER:
> >> +             if (sk->sk_type != SOCK_SEQPACKET && sk->sk_type != SOCK_STREAM
> >> +                             && sk->sk_type != SOCK_RAW) {
> >> +                     err = -EINVAL;
> >> +                     break;
> >> +             }
> >> +
> >> +             pwr.force_active = chan->force_active;
> >> +
> >> +             len = min_t(unsigned int, len, sizeof(pwr));
> >> +             if (copy_to_user(optval, (char *) &pwr, len))
> >> +                     err = -EFAULT;
> >> +
> >> +             break;
> >> +
> >>       default:
> >>               err = -ENOPROTOOPT;
> >>               break;
> >> @@ -584,6 +600,7 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
> >>       struct sock *sk = sock->sk;
> >>       struct l2cap_chan *chan = l2cap_pi(sk)->chan;
> >>       struct bt_security sec;
> >> +     struct bt_power pwr;
> >>       int len, err = 0;
> >>       u32 opt;
> >>
> >> @@ -660,6 +677,23 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
> >>               chan->flushable = opt;
> >>               break;
> >>
> >> +     case BT_POWER:
> >> +             if (sk->sk_type != SOCK_SEQPACKET && sk->sk_type != SOCK_STREAM
> >> +                             && sk->sk_type != SOCK_RAW) {
> >> +                     err = -EINVAL;
> >
> > This need to be rebased against bluetooth-next. Not sure, but this seems to be
> > the only place of conflict.
> 
> Rebased patch attached.
> 
> Thanks
> 
> >
> > --
> > Gustavo F. Padovan
> > http://profusion.mobi
> > --
> > 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
> >

> From 74e9a75ef48b633aa82e28ec775bb04b630d51ac Mon Sep 17 00:00:00 2001
> From: Jaikumar Ganesh <jaikumar@google.com>
> Date: Mon, 23 May 2011 18:06:04 -0700
> Subject: [PATCH] Bluetooth: Add BT_POWER L2CAP socket option.
> 
> Add BT_POWER socket option used to control the power
> characteristics of the underlying ACL link. When the remote end
> has put the link in sniff mode and the host stack wants to send
> data we need need to explicitly exit sniff mode to work well with
> certain devices (For example, A2DP on Plantronics Voyager 855).
> However, this causes problems with HID devices.
> 
> Hence, moving into active mode when sending data, irrespective
> of who set the sniff mode has been made as a socket option. By
> default, we will move into active mode. HID devices can set the
> L2CAP socket option to prevent this from happening.
> 
> Currently, this has been implemented for L2CAP sockets. This has been
> tested with incoming and outgoing L2CAP sockets for HID and A2DP.
> 
> Based on discussions on linux-bluetooth and patches submitted by
> Andrei Emeltchenko.
> 
> Signed-off-by: Jaikumar Ganesh <jaikumar@google.com>
> ---
>  include/net/bluetooth/bluetooth.h |    8 ++++++++
>  include/net/bluetooth/hci_core.h  |    2 +-
>  include/net/bluetooth/l2cap.h     |    1 +
>  net/bluetooth/hci_conn.c          |    9 ++++++---
>  net/bluetooth/hci_core.c          |    4 ++--
>  net/bluetooth/l2cap_core.c        |    5 +++++
>  net/bluetooth/l2cap_sock.c        |   36 ++++++++++++++++++++++++++++++++++++
>  7 files changed, 59 insertions(+), 6 deletions(-)

I applied it, but please next time send a inline patch. Thanks.

-- 
Gustavo F. Padovan
http://profusion.mobi

  reply	other threads:[~2011-06-01 22:20 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-24  1:06 [PATCH] Bluetooth: Add BT_POWER L2CAP socket option Jaikumar Ganesh
2011-05-24  8:37 ` Ville Tervo
2011-05-24 16:20   ` Luiz Augusto von Dentz
2011-05-24 16:44     ` Jaikumar Ganesh
2011-05-24 18:24       ` Luiz Augusto von Dentz
2011-05-25  1:07         ` Jaikumar Ganesh
2011-05-30 22:11 ` Gustavo F. Padovan
2011-06-01  1:53   ` Jaikumar Ganesh
2011-06-01 22:20     ` Gustavo F. Padovan [this message]
2011-06-02  1:18       ` Marcel Holtmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110601222048.GJ2564@joana \
    --to=padovan@profusion.mobi \
    --cc=jaikumarg@gmail.com \
    --cc=linux-bluetooth@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox