[PATCH] Bluetooth: Create empty l2cap ops function

[PATCH] Bluetooth: Create empty l2cap ops function

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

A2MP doesn't use part of the L2CAP chan ops API so we just create general
empty function instead of the A2MP specific one.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 include/net/bluetooth/l2cap.h |   12 ++++++++++++
 net/bluetooth/a2mp.c          |   23 +++--------------------
 2 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index a00b43e..b939e90 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -740,6 +740,18 @@ static inline __u16 __next_seq(struct l2cap_chan *chan, __u16 seq)
 	return (seq + 1) % (chan->tx_win_max + 1);
 }
 
+static inline struct l2cap_chan *l2cap_chan_no_new_connection(struct l2cap_chan *chan)
+{
+	return NULL;
+}
+
+static inline void l2cap_chan_no_teardown(struct l2cap_chan *chan, int err)
+{
+}
+
+static inline void l2cap_chan_no_ready(struct l2cap_chan *chan)
+{
+}
 
 extern bool disable_ertm;
 
diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
index e08ca2a..ec1ef2e 100644
--- a/net/bluetooth/a2mp.c
+++ b/net/bluetooth/a2mp.c
@@ -440,23 +440,6 @@ static struct sk_buff *a2mp_chan_alloc_skb_cb(struct l2cap_chan *chan,
 	return bt_skb_alloc(len, GFP_KERNEL);
 }
 
-static struct l2cap_chan *a2mp_chan_no_new_conn_cb(struct l2cap_chan *chan)
-{
-	BT_ERR("new_connection for chan %p not implemented", chan);
-
-	return NULL;
-}
-
-static void a2mp_chan_no_teardown_cb(struct l2cap_chan *chan, int err)
-{
-	BT_ERR("teardown for chan %p not implemented", chan);
-}
-
-static void a2mp_chan_no_ready(struct l2cap_chan *chan)
-{
-	BT_ERR("ready for chan %p not implemented", chan);
-}
-
 static struct l2cap_ops a2mp_chan_ops = {
 	.name = "L2CAP A2MP channel",
 	.recv = a2mp_chan_recv_cb,
@@ -465,9 +448,9 @@ static struct l2cap_ops a2mp_chan_ops = {
 	.alloc_skb = a2mp_chan_alloc_skb_cb,
 
 	/* Not implemented for A2MP */
-	.new_connection = a2mp_chan_no_new_conn_cb,
-	.teardown = a2mp_chan_no_teardown_cb,
-	.ready = a2mp_chan_no_ready,
+	.new_connection = l2cap_chan_no_new_connection,
+	.teardown = l2cap_chan_no_teardown,
+	.ready = l2cap_chan_no_ready,
 };
 
 static struct l2cap_chan *a2mp_chan_open(struct l2cap_conn *conn)
-- 
1.7.10.2

Re: [PATCH] Bluetooth: Create empty l2cap ops function

[Mat Martineau]

Gustavo -

On Tue, 29 May 2012, Gustavo Padovan wrote:

> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>
> A2MP doesn't use part of the L2CAP chan ops API so we just create general
> empty function instead of the A2MP specific one.
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
> include/net/bluetooth/l2cap.h |   12 ++++++++++++
> net/bluetooth/a2mp.c          |   23 +++--------------------
> 2 files changed, 15 insertions(+), 20 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index a00b43e..b939e90 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -740,6 +740,18 @@ static inline __u16 __next_seq(struct l2cap_chan *chan, __u16 seq)
> 	return (seq + 1) % (chan->tx_win_max + 1);
> }
>
> +static inline struct l2cap_chan *l2cap_chan_no_new_connection(struct l2cap_chan *chan)
> +{
> +	return NULL;
> +}
> +
> +static inline void l2cap_chan_no_teardown(struct l2cap_chan *chan, int err)
> +{
> +}
> +
> +static inline void l2cap_chan_no_ready(struct l2cap_chan *chan)
> +{
> +}
>
> extern bool disable_ertm;
>
> diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
> index e08ca2a..ec1ef2e 100644
> --- a/net/bluetooth/a2mp.c
> +++ b/net/bluetooth/a2mp.c
> @@ -440,23 +440,6 @@ static struct sk_buff *a2mp_chan_alloc_skb_cb(struct l2cap_chan *chan,
> 	return bt_skb_alloc(len, GFP_KERNEL);
> }
>
> -static struct l2cap_chan *a2mp_chan_no_new_conn_cb(struct l2cap_chan *chan)
> -{
> -	BT_ERR("new_connection for chan %p not implemented", chan);
> -
> -	return NULL;
> -}
> -
> -static void a2mp_chan_no_teardown_cb(struct l2cap_chan *chan, int err)
> -{
> -	BT_ERR("teardown for chan %p not implemented", chan);
> -}
> -
> -static void a2mp_chan_no_ready(struct l2cap_chan *chan)
> -{
> -	BT_ERR("ready for chan %p not implemented", chan);
> -}
> -
> static struct l2cap_ops a2mp_chan_ops = {
> 	.name = "L2CAP A2MP channel",
> 	.recv = a2mp_chan_recv_cb,
> @@ -465,9 +448,9 @@ static struct l2cap_ops a2mp_chan_ops = {
> 	.alloc_skb = a2mp_chan_alloc_skb_cb,
>
> 	/* Not implemented for A2MP */
> -	.new_connection = a2mp_chan_no_new_conn_cb,
> -	.teardown = a2mp_chan_no_teardown_cb,
> -	.ready = a2mp_chan_no_ready,
> +	.new_connection = l2cap_chan_no_new_connection,
> +	.teardown = l2cap_chan_no_teardown,
> +	.ready = l2cap_chan_no_ready,
> };
>
> static struct l2cap_chan *a2mp_chan_open(struct l2cap_conn *conn)
> -- 
> 1.7.10.2

If these pointers all need to be populated, will you also remove the 
NULL checks for the callbacks in l2cap_core.c?  Maybe the callbacks 
could be validated once when the l2cap_chan is set up.

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [PATCH] Bluetooth: Create empty l2cap ops function

[Mat Martineau]

Gustavo -

On Tue, 29 May 2012, Mat Martineau wrote:

>
> Gustavo -
>
> On Tue, 29 May 2012, Gustavo Padovan wrote:
>
>> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>> 
>> A2MP doesn't use part of the L2CAP chan ops API so we just create general
>> empty function instead of the A2MP specific one.
>> 
>> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>> ---
>> include/net/bluetooth/l2cap.h |   12 ++++++++++++
>> net/bluetooth/a2mp.c          |   23 +++--------------------
>> 2 files changed, 15 insertions(+), 20 deletions(-)
>> 
>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>> index a00b43e..b939e90 100644
>> --- a/include/net/bluetooth/l2cap.h
>> +++ b/include/net/bluetooth/l2cap.h
>> @@ -740,6 +740,18 @@ static inline __u16 __next_seq(struct l2cap_chan 
>> *chan, __u16 seq)
>> 	return (seq + 1) % (chan->tx_win_max + 1);
>> }
>> 
>> +static inline struct l2cap_chan *l2cap_chan_no_new_connection(struct 
>> l2cap_chan *chan)
>> +{
>> +	return NULL;
>> +}
>> +
>> +static inline void l2cap_chan_no_teardown(struct l2cap_chan *chan, int 
>> err)
>> +{
>> +}
>> +
>> +static inline void l2cap_chan_no_ready(struct l2cap_chan *chan)
>> +{
>> +}
>> 
>> extern bool disable_ertm;
>> 
>> diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
>> index e08ca2a..ec1ef2e 100644
>> --- a/net/bluetooth/a2mp.c
>> +++ b/net/bluetooth/a2mp.c
>> @@ -440,23 +440,6 @@ static struct sk_buff *a2mp_chan_alloc_skb_cb(struct 
>> l2cap_chan *chan,
>> 	return bt_skb_alloc(len, GFP_KERNEL);
>> }
>> 
>> -static struct l2cap_chan *a2mp_chan_no_new_conn_cb(struct l2cap_chan 
>> *chan)
>> -{
>> -	BT_ERR("new_connection for chan %p not implemented", chan);
>> -
>> -	return NULL;
>> -}
>> -
>> -static void a2mp_chan_no_teardown_cb(struct l2cap_chan *chan, int err)
>> -{
>> -	BT_ERR("teardown for chan %p not implemented", chan);
>> -}
>> -
>> -static void a2mp_chan_no_ready(struct l2cap_chan *chan)
>> -{
>> -	BT_ERR("ready for chan %p not implemented", chan);
>> -}
>> -
>> static struct l2cap_ops a2mp_chan_ops = {
>> 	.name = "L2CAP A2MP channel",
>> 	.recv = a2mp_chan_recv_cb,
>> @@ -465,9 +448,9 @@ static struct l2cap_ops a2mp_chan_ops = {
>> 	.alloc_skb = a2mp_chan_alloc_skb_cb,
>>
>> 	/* Not implemented for A2MP */
>> -	.new_connection = a2mp_chan_no_new_conn_cb,
>> -	.teardown = a2mp_chan_no_teardown_cb,
>> -	.ready = a2mp_chan_no_ready,
>> +	.new_connection = l2cap_chan_no_new_connection,
>> +	.teardown = l2cap_chan_no_teardown,
>> +	.ready = l2cap_chan_no_ready,
>> };
>> 
>> static struct l2cap_chan *a2mp_chan_open(struct l2cap_conn *conn)
>> -- 
>> 1.7.10.2
>
> If these pointers all need to be populated, will you also remove the NULL 
> checks for the callbacks in l2cap_core.c?  Maybe the callbacks could be 
> validated once when the l2cap_chan is set up.

Nevermind, you already have a patch for this.  I should have caught up 
on the rest of the mailing list first...


--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [PATCH] Bluetooth: Create empty l2cap ops function

[Gustavo Padovan]

Hi Mat,

* Mat Martineau <mathewm@codeaurora.org> [2012-05-29 09:41:45 -0700]:

> 
> Gustavo -
> 
> On Tue, 29 May 2012, Gustavo Padovan wrote:
> 
> >From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> >
> >A2MP doesn't use part of the L2CAP chan ops API so we just create general
> >empty function instead of the A2MP specific one.
> >
> >Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> >---
> >include/net/bluetooth/l2cap.h |   12 ++++++++++++
> >net/bluetooth/a2mp.c          |   23 +++--------------------
> >2 files changed, 15 insertions(+), 20 deletions(-)
> >
> >diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> >index a00b43e..b939e90 100644
> >--- a/include/net/bluetooth/l2cap.h
> >+++ b/include/net/bluetooth/l2cap.h
> >@@ -740,6 +740,18 @@ static inline __u16 __next_seq(struct l2cap_chan *chan, __u16 seq)
> >	return (seq + 1) % (chan->tx_win_max + 1);
> >}
> >
> >+static inline struct l2cap_chan *l2cap_chan_no_new_connection(struct l2cap_chan *chan)
> >+{
> >+	return NULL;
> >+}
> >+
> >+static inline void l2cap_chan_no_teardown(struct l2cap_chan *chan, int err)
> >+{
> >+}
> >+
> >+static inline void l2cap_chan_no_ready(struct l2cap_chan *chan)
> >+{
> >+}
> >
> >extern bool disable_ertm;
> >
> >diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
> >index e08ca2a..ec1ef2e 100644
> >--- a/net/bluetooth/a2mp.c
> >+++ b/net/bluetooth/a2mp.c
> >@@ -440,23 +440,6 @@ static struct sk_buff *a2mp_chan_alloc_skb_cb(struct l2cap_chan *chan,
> >	return bt_skb_alloc(len, GFP_KERNEL);
> >}
> >
> >-static struct l2cap_chan *a2mp_chan_no_new_conn_cb(struct l2cap_chan *chan)
> >-{
> >-	BT_ERR("new_connection for chan %p not implemented", chan);
> >-
> >-	return NULL;
> >-}
> >-
> >-static void a2mp_chan_no_teardown_cb(struct l2cap_chan *chan, int err)
> >-{
> >-	BT_ERR("teardown for chan %p not implemented", chan);
> >-}
> >-
> >-static void a2mp_chan_no_ready(struct l2cap_chan *chan)
> >-{
> >-	BT_ERR("ready for chan %p not implemented", chan);
> >-}
> >-
> >static struct l2cap_ops a2mp_chan_ops = {
> >	.name = "L2CAP A2MP channel",
> >	.recv = a2mp_chan_recv_cb,
> >@@ -465,9 +448,9 @@ static struct l2cap_ops a2mp_chan_ops = {
> >	.alloc_skb = a2mp_chan_alloc_skb_cb,
> >
> >	/* Not implemented for A2MP */
> >-	.new_connection = a2mp_chan_no_new_conn_cb,
> >-	.teardown = a2mp_chan_no_teardown_cb,
> >-	.ready = a2mp_chan_no_ready,
> >+	.new_connection = l2cap_chan_no_new_connection,
> >+	.teardown = l2cap_chan_no_teardown,
> >+	.ready = l2cap_chan_no_ready,
> >};
> >
> >static struct l2cap_chan *a2mp_chan_open(struct l2cap_conn *conn)
> >-- 
> >1.7.10.2
> 
> If these pointers all need to be populated, will you also remove the
> NULL checks for the callbacks in l2cap_core.c?

Yes, one of my patchset that is on the mailing list do this.

>Maybe the callbacks
> could be validated once when the l2cap_chan is set up.

I think it's pointless, we are the only user of our API, so we can make sure
we won't have NULL pointer floating around.

	Gustavo

Re: [PATCH] Bluetooth: Create empty l2cap ops function

[Andrei Emeltchenko]

Hi Gustavo,

On Tue, May 29, 2012 at 01:19:26PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> A2MP doesn't use part of the L2CAP chan ops API so we just create general
> empty function instead of the A2MP specific one.
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  include/net/bluetooth/l2cap.h |   12 ++++++++++++
>  net/bluetooth/a2mp.c          |   23 +++--------------------
>  2 files changed, 15 insertions(+), 20 deletions(-)
> 
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index a00b43e..b939e90 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -740,6 +740,18 @@ static inline __u16 __next_seq(struct l2cap_chan *chan, __u16 seq)
>  	return (seq + 1) % (chan->tx_win_max + 1);
>  }
>  
> +static inline struct l2cap_chan *l2cap_chan_no_new_connection(struct l2cap_chan *chan)
> +{
> +	return NULL;
> +}
> +
> +static inline void l2cap_chan_no_teardown(struct l2cap_chan *chan, int err)
> +{
> +}
> +
> +static inline void l2cap_chan_no_ready(struct l2cap_chan *chan)
> +{
> +}

One note here. Shall we have debug or error message to indicate that
function is not implemented?

Best regards 
Andrei Emeltchenko 

Re: [PATCH] Bluetooth: Create empty l2cap ops function

[Gustavo Padovan]

Hi Andrei,

* Andrei Emeltchenko <andrei.emeltchenko.news@gmail.com> [2012-05-30 10:02:00 +0300]:

> Hi Gustavo,
> 
> On Tue, May 29, 2012 at 01:19:26PM -0300, Gustavo Padovan wrote:
> > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > 
> > A2MP doesn't use part of the L2CAP chan ops API so we just create general
> > empty function instead of the A2MP specific one.
> > 
> > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > ---
> >  include/net/bluetooth/l2cap.h |   12 ++++++++++++
> >  net/bluetooth/a2mp.c          |   23 +++--------------------
> >  2 files changed, 15 insertions(+), 20 deletions(-)
> > 
> > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> > index a00b43e..b939e90 100644
> > --- a/include/net/bluetooth/l2cap.h
> > +++ b/include/net/bluetooth/l2cap.h
> > @@ -740,6 +740,18 @@ static inline __u16 __next_seq(struct l2cap_chan *chan, __u16 seq)
> >  	return (seq + 1) % (chan->tx_win_max + 1);
> >  }
> >  
> > +static inline struct l2cap_chan *l2cap_chan_no_new_connection(struct l2cap_chan *chan)
> > +{
> > +	return NULL;
> > +}
> > +
> > +static inline void l2cap_chan_no_teardown(struct l2cap_chan *chan, int err)
> > +{
> > +}
> > +
> > +static inline void l2cap_chan_no_ready(struct l2cap_chan *chan)
> > +{
> > +}
> 
> One note here. Shall we have debug or error message to indicate that
> function is not implemented?

No, these are like NOPs. Shouldn't be noticed by anyone, if you decided to use
the you are saying that you don't wanna know about it.

	Gustavo

Re: [PATCH] Bluetooth: Create empty l2cap ops function

[Johan Hedberg]

Hi Gustavo,

On Tue, May 29, 2012, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> A2MP doesn't use part of the L2CAP chan ops API so we just create general
> empty function instead of the A2MP specific one.
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  include/net/bluetooth/l2cap.h |   12 ++++++++++++
>  net/bluetooth/a2mp.c          |   23 +++--------------------
>  2 files changed, 15 insertions(+), 20 deletions(-)

Applied to bluetooth-next. Thanks.

Johan