All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFCv0] Bluetooth: General HCI callback implementation
@ 2011-12-09 13:10 Emeltchenko Andrei
  2011-12-10 12:47 ` Anderson Lizardo
  2011-12-14 22:20 ` Marcel Holtmann
  0 siblings, 2 replies; 6+ messages in thread
From: Emeltchenko Andrei @ 2011-12-09 13:10 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Add general HCI callback implementation. Can be used for executing
HCI commands from A2MP protocol.
---
 include/net/bluetooth/hci_core.h |   14 +++++++++++
 net/bluetooth/hci_core.c         |   47 ++++++++++++++++++++++++++++++++++++++
 net/bluetooth/hci_event.c        |    4 +++
 3 files changed, 65 insertions(+), 0 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index cc5481d..e473cd2 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -121,6 +121,15 @@ struct adv_entry {
 	u8 bdaddr_type;
 };
 
+struct hci_dev;
+
+struct cb_cmd {
+	struct list_head list;
+	u16 opcode;
+	void *opt;
+	void (*cb)(struct hci_dev *hdev, void *opt);
+};
+
 #define NUM_REASSEMBLY 4
 struct hci_dev {
 	struct list_head list;
@@ -228,6 +237,7 @@ struct hci_dev {
 	__u16			init_last_cmd;
 
 	struct list_head	mgmt_pending;
+	struct list_head	cb_list;
 
 	struct inquiry_cache	inq_cache;
 	struct hci_conn_hash	conn_hash;
@@ -1037,4 +1047,8 @@ void hci_le_ltk_neg_reply(struct hci_conn *conn);
 int hci_do_inquiry(struct hci_dev *hdev, u8 length);
 int hci_cancel_inquiry(struct hci_dev *hdev);
 
+struct cb_cmd *hci_find_cb(struct hci_dev *hdev, __u16 opcode);
+int hci_cmd_cb(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param,
+		void (*cb)(struct hci_dev *hdev, void *opt), void *opt);
+
 #endif /* __HCI_CORE_H */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index ff56cd3..8416593 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1560,6 +1560,8 @@ int hci_register_dev(struct hci_dev *hdev)
 
 	INIT_LIST_HEAD(&hdev->mgmt_pending);
 
+	INIT_LIST_HEAD(&hdev->cb_list);
+
 	INIT_LIST_HEAD(&hdev->blacklist);
 
 	INIT_LIST_HEAD(&hdev->uuids);
@@ -2033,6 +2035,51 @@ int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param)
 	return 0;
 }
 
+void hci_add_cb(struct hci_dev *hdev, __u16 opcode,
+		void (*cb)(struct hci_dev *hdev, void *opt), void *opt)
+{
+	struct cb_cmd *cmd;
+
+	cmd = kmalloc(sizeof(*cmd), GFP_ATOMIC);
+	if (!cmd)
+		return;
+
+	cmd->cb = cb;
+	cmd->opcode = opcode;
+	cmd->opt = opt;
+
+	list_add(&cmd->list, &hdev->cb_list);
+}
+
+struct cb_cmd *hci_find_cb(struct hci_dev *hdev, __u16 opcode)
+{
+	struct cb_cmd *cmd;
+
+	list_for_each_entry(cmd, &hdev->cb_list, list)
+		if (cmd->opcode == opcode)
+			return cmd;
+
+	return NULL;
+}
+
+static void hci_remove_cb(struct cb_cmd *cmd)
+{
+	list_del(&cmd->list);
+
+	kfree(cmd->opt);
+	kfree(cmd);
+}
+
+/* Send HCI command with callback */
+int hci_cmd_cb(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param,
+		void (*cb)(struct hci_dev *hdev, void *opt), void *opt)
+{
+	if (cb)
+		hci_add_cb(hdev, opcode, cb, opt);
+
+	return hci_send_cmd(hdev, opcode, plen, param);
+}
+
 /* Get data from the previously sent command */
 void *hci_sent_cmd_data(struct hci_dev *hdev, __u16 opcode)
 {
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 8fb1a54..03d1b25 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1945,6 +1945,7 @@ static inline void hci_qos_setup_complete_evt(struct hci_dev *hdev, struct sk_bu
 static inline void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_ev_cmd_complete *ev = (void *) skb->data;
+	struct cb_cmd *cmd;
 	__u16 opcode;
 
 	skb_pull(skb, sizeof(*ev));
@@ -2074,6 +2075,9 @@ static inline void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *sk
 
 	case HCI_OP_READ_LOCAL_AMP_INFO:
 		hci_cc_read_local_amp_info(hdev, skb);
+		cmd = hci_find_cb(hdev, opcode);
+		if (cmd)
+			cmd->cb(hdev, cmd->opt);
 		break;
 
 	case HCI_OP_DELETE_STORED_LINK_KEY:
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [RFCv0] Bluetooth: General HCI callback implementation
  2011-12-09 13:10 [RFCv0] Bluetooth: General HCI callback implementation Emeltchenko Andrei
@ 2011-12-10 12:47 ` Anderson Lizardo
  2011-12-12  9:15   ` Emeltchenko Andrei
  2011-12-14 22:20 ` Marcel Holtmann
  1 sibling, 1 reply; 6+ messages in thread
From: Anderson Lizardo @ 2011-12-10 12:47 UTC (permalink / raw)
  To: Emeltchenko Andrei; +Cc: linux-bluetooth

Hi Andrei,

On Fri, Dec 9, 2011 at 9:10 AM, Emeltchenko Andrei
<Andrei.Emeltchenko.news@gmail.com> wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> Add general HCI callback implementation. Can be used for executing
> HCI commands from A2MP protocol.
> ---
>  include/net/bluetooth/hci_core.h |   14 +++++++++++
>  net/bluetooth/hci_core.c         |   47 ++++++++++++++++++++++++++++++++++++++
>  net/bluetooth/hci_event.c        |    4 +++
>  3 files changed, 65 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index cc5481d..e473cd2 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -121,6 +121,15 @@ struct adv_entry {
>        u8 bdaddr_type;
>  };
>
> +struct hci_dev;
> +
> +struct cb_cmd {
> +       struct list_head list;
> +       u16 opcode;
> +       void *opt;
> +       void (*cb)(struct hci_dev *hdev, void *opt);
> +};
> +

Isn't this a too generic name for an exported struct, which may cause clashes?

Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFCv0] Bluetooth: General HCI callback implementation
  2011-12-10 12:47 ` Anderson Lizardo
@ 2011-12-12  9:15   ` Emeltchenko Andrei
  0 siblings, 0 replies; 6+ messages in thread
From: Emeltchenko Andrei @ 2011-12-12  9:15 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: linux-bluetooth

Hi Anderson,

On Sat, Dec 10, 2011 at 08:47:38AM -0400, Anderson Lizardo wrote:
> Hi Andrei,
> 
> On Fri, Dec 9, 2011 at 9:10 AM, Emeltchenko Andrei
> <Andrei.Emeltchenko.news@gmail.com> wrote:
> > From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> >
> > Add general HCI callback implementation. Can be used for executing
> > HCI commands from A2MP protocol.
> > ---
> >  include/net/bluetooth/hci_core.h |   14 +++++++++++
> >  net/bluetooth/hci_core.c         |   47 ++++++++++++++++++++++++++++++++++++++
> >  net/bluetooth/hci_event.c        |    4 +++
> >  3 files changed, 65 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > index cc5481d..e473cd2 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -121,6 +121,15 @@ struct adv_entry {
> >        u8 bdaddr_type;
> >  };
> >
> > +struct hci_dev;
> > +
> > +struct cb_cmd {
> > +       struct list_head list;
> > +       u16 opcode;
> > +       void *opt;
> > +       void (*cb)(struct hci_dev *hdev, void *opt);
> > +};
> > +
> 
> Isn't this a too generic name for an exported struct, which may cause clashes?

At least my cscope cannot find other struct with the same name in Linux
kernel.

But I agree that name might be better. Maybe "hci_req_cb" ?

Best regards 
Andrei Emeltchenko 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFCv0] Bluetooth: General HCI callback implementation
  2011-12-09 13:10 [RFCv0] Bluetooth: General HCI callback implementation Emeltchenko Andrei
  2011-12-10 12:47 ` Anderson Lizardo
@ 2011-12-14 22:20 ` Marcel Holtmann
  2011-12-15  9:19   ` Emeltchenko Andrei
  1 sibling, 1 reply; 6+ messages in thread
From: Marcel Holtmann @ 2011-12-14 22:20 UTC (permalink / raw)
  To: Emeltchenko Andrei; +Cc: linux-bluetooth

Hi Andrei,

> Add general HCI callback implementation. Can be used for executing
> HCI commands from A2MP protocol.
> ---
>  include/net/bluetooth/hci_core.h |   14 +++++++++++
>  net/bluetooth/hci_core.c         |   47 ++++++++++++++++++++++++++++++++++++++
>  net/bluetooth/hci_event.c        |    4 +++
>  3 files changed, 65 insertions(+), 0 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index cc5481d..e473cd2 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -121,6 +121,15 @@ struct adv_entry {
>  	u8 bdaddr_type;
>  };
>  
> +struct hci_dev;
> +
> +struct cb_cmd {
> +	struct list_head list;
> +	u16 opcode;
> +	void *opt;
> +	void (*cb)(struct hci_dev *hdev, void *opt);
> +};
> +

so I am actually thinking that we might just update hci_request into a
bit more flexible schema.

Gustavo is working on moving everything into a process context, so we
can actually sleep. And we could just make a simple premise to only
allow one request at a time. That would also then allow us to utilize
hci_request. We just need a clean way to report back the result.

Regards

Marcel



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFCv0] Bluetooth: General HCI callback implementation
  2011-12-14 22:20 ` Marcel Holtmann
@ 2011-12-15  9:19   ` Emeltchenko Andrei
  2011-12-15  9:40     ` Marcel Holtmann
  0 siblings, 1 reply; 6+ messages in thread
From: Emeltchenko Andrei @ 2011-12-15  9:19 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth

Hi Marcel,

On Wed, Dec 14, 2011 at 11:20:23PM +0100, Marcel Holtmann wrote:
> Hi Andrei,
> 
> > Add general HCI callback implementation. Can be used for executing
> > HCI commands from A2MP protocol.
> > ---
> >  include/net/bluetooth/hci_core.h |   14 +++++++++++
> >  net/bluetooth/hci_core.c         |   47 ++++++++++++++++++++++++++++++++++++++
> >  net/bluetooth/hci_event.c        |    4 +++
> >  3 files changed, 65 insertions(+), 0 deletions(-)
> > 
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > index cc5481d..e473cd2 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -121,6 +121,15 @@ struct adv_entry {
> >  	u8 bdaddr_type;
> >  };
> >  
> > +struct hci_dev;
> > +
> > +struct cb_cmd {
> > +	struct list_head list;
> > +	u16 opcode;
> > +	void *opt;
> > +	void (*cb)(struct hci_dev *hdev, void *opt);
> > +};
> > +
> 
> so I am actually thinking that we might just update hci_request into a
> bit more flexible schema.

Do you mean that we can use blocking hci_request? This may work but some
commands requires special logic (like executing several Read AMP Assoc
data requests). I don't know would it be the best solution especially when
executing from A2MP/L2CAP code.

> Gustavo is working on moving everything into a process context, so we
> can actually sleep. And we could just make a simple premise to only

This would be good, meanwhile I have implemented callback execution in a
workqueue. I will send second RFC version later today.

Best regards 
Andrei Emeltchenko 

> allow one request at a time. That would also then allow us to utilize
> hci_request. We just need a clean way to report back the result.
> 
> Regards
> 
> Marcel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFCv0] Bluetooth: General HCI callback implementation
  2011-12-15  9:19   ` Emeltchenko Andrei
@ 2011-12-15  9:40     ` Marcel Holtmann
  0 siblings, 0 replies; 6+ messages in thread
From: Marcel Holtmann @ 2011-12-15  9:40 UTC (permalink / raw)
  To: Emeltchenko Andrei; +Cc: linux-bluetooth

Hi Andrei,

> > > Add general HCI callback implementation. Can be used for executing
> > > HCI commands from A2MP protocol.
> > > ---
> > >  include/net/bluetooth/hci_core.h |   14 +++++++++++
> > >  net/bluetooth/hci_core.c         |   47 ++++++++++++++++++++++++++++++++++++++
> > >  net/bluetooth/hci_event.c        |    4 +++
> > >  3 files changed, 65 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > > index cc5481d..e473cd2 100644
> > > --- a/include/net/bluetooth/hci_core.h
> > > +++ b/include/net/bluetooth/hci_core.h
> > > @@ -121,6 +121,15 @@ struct adv_entry {
> > >  	u8 bdaddr_type;
> > >  };
> > >  
> > > +struct hci_dev;
> > > +
> > > +struct cb_cmd {
> > > +	struct list_head list;
> > > +	u16 opcode;
> > > +	void *opt;
> > > +	void (*cb)(struct hci_dev *hdev, void *opt);
> > > +};
> > > +
> > 
> > so I am actually thinking that we might just update hci_request into a
> > bit more flexible schema.
> 
> Do you mean that we can use blocking hci_request? This may work but some
> commands requires special logic (like executing several Read AMP Assoc
> data requests). I don't know would it be the best solution especially when
> executing from A2MP/L2CAP code.

that is my point exactly. Once we do HCI processing in process context,
we will also have L2CAP running fully in process context. Hence it could
just wait for the result.

> > Gustavo is working on moving everything into a process context, so we
> > can actually sleep. And we could just make a simple premise to only
> 
> This would be good, meanwhile I have implemented callback execution in a
> workqueue. I will send second RFC version later today.

Not sure how that helps us right now. I rather wait a bit for Gustavo to
submit his patchset and then we figure this out. It seems to be a more
general problem anyway and we keep hitting it quite of often right now.

Regards

Marcel



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-12-15  9:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-09 13:10 [RFCv0] Bluetooth: General HCI callback implementation Emeltchenko Andrei
2011-12-10 12:47 ` Anderson Lizardo
2011-12-12  9:15   ` Emeltchenko Andrei
2011-12-14 22:20 ` Marcel Holtmann
2011-12-15  9:19   ` Emeltchenko Andrei
2011-12-15  9:40     ` Marcel Holtmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.