[PATCH] - for BT HID fixes + beginning of ctrl handling

[PATCH] - for BT HID fixes + beginning of ctrl handling

[Matthew Grant]

[-- Attachment #1.1: Type: text/plain, Size: 759 bytes --]

Marcel,

Please find 2 patches attached.  My original against 2.6.9-mh4, and one
for 2.6.10-rc2-bk.  These are the patches I can easily produce. Is this
OK with you? Please let me know what else you need and I will see what
can be done.

I have not one yet between straight 2.6.10-rc2-bk+base-bthid+-mh4 and my
trunk as the source tree manipulations are getting quite convoluted.
Making this patching easier is something I would really like to sort out
for both our work flows.

My question is, how do you manage the patch creation of -mh4 patches
etc? Looks like this just a simple diff of your trunk net/bluetooth and
drivers/bluetooth against current mainstream kernel.

Thanks for your suggestions here.

Best Regards,

Matthew Grant

[-- Attachment #1.2: linux-2.6.10-rc2-bk-maghidp2004120201.patch --]
[-- Type: text/x-patch, Size: 12006 bytes --]

diff -uNr linux-2.6.10-rc2-bk13/net/bluetooth/hidp/core.c linux-2.6.10-rc2-bk13-bthid/net/bluetooth/hidp/core.c
--- linux-2.6.10-rc2-bk13/net/bluetooth/hidp/core.c	2004-10-19 10:54:55.000000000 +1300
+++ linux-2.6.10-rc2-bk13-bthid/net/bluetooth/hidp/core.c	2004-12-02 08:27:41.045629872 +1300
@@ -45,7 +45,7 @@
 
 #include "hidp.h"
 
-#ifndef CONFIG_BT_HIDP_DEBUG
+#ifndef CONFIG_BT_HIDP_DEBUG_BT
 #undef  BT_DBG
 #define BT_DBG(D...)
 #endif
@@ -130,8 +130,8 @@
 	struct sk_buff *skb;
 	unsigned char newleds;
 
-	BT_DBG("session %p hid %p data %p size %d", session, device, data, size);
-
+	BT_DBG("");
+	 
 	if (type != EV_LED)
 		return -1;
 
@@ -151,7 +151,7 @@
 		return -ENOMEM;
 	}
 
-	*skb_put(skb, 1) = 0xa2;
+	*skb_put(skb, 1) = HIDP_TRANS_DATA|HIDP_DATA_RTYPE_OUPUT;
 	*skb_put(skb, 1) = 0x01;
 	*skb_put(skb, 1) = newleds;
 
@@ -232,7 +232,36 @@
 		del_timer(&session->timer);
 }
 
-static inline void hidp_send_message(struct hidp_session *session, unsigned char hdr)
+static inline int hidp_send_ctrl_message(struct hidp_session *session,
+			unsigned char hdr,
+			unsigned char *data, int size)
+{
+	struct sk_buff *skb;
+
+	BT_DBG("session %p data %p size %d", session, data, size);
+
+	if (!(skb = alloc_skb(size + 1, GFP_ATOMIC))) {
+		BT_ERR("Can't allocate memory for new frame");
+		return -ENOMEM;
+	}
+
+	*skb_put(skb, 1) = hdr;
+	if (size > 0)
+		memcpy(skb_put(skb, size), data, size);
+
+	skb_queue_tail(&session->ctrl_transmit, skb);
+
+	hidp_schedule(session);
+
+	return 0;
+}
+
+/* Send a 1 byte control message.
+ *   Schedule flag has to be set if calling from outside HID session process 
+ */
+static inline void hidp_send_ctrl_byte(struct hidp_session *session,
+		 			unsigned char hdr,
+					int schedule)
 {
 	struct sk_buff *skb;
 
@@ -247,29 +276,199 @@
 
 	skb_queue_tail(&session->ctrl_transmit, skb);
 
-	hidp_schedule(session);
+	if (schedule)
+		hidp_schedule(session);
+}
+
+
+static inline void hidp_process_handshake(struct hidp_session *session, __u8 param){
+	switch (param) {
+	case HIDP_HSHK_SUCCESSFUL:
+		/* Call into SET_ GET_ handlers here */
+		break;
+	case HIDP_HSHK_NOT_READY:
+	case HIDP_HSHK_ERR_INVALID_REPORT_ID:
+	case HIDP_HSHK_ERR_UNSUPPORTED_REQUEST:
+	case HIDP_HSHK_ERR_INVALID_PARAMETER:
+		/* Call into SET_ GET_ handlers here */
+		break;
+	case HIDP_HSHK_ERR_UNKNOWN:
+		BT_INFO("HANDSHAKE parameter ERR_UNKNOWN seen.");
+		break;
+	case HIDP_HSHK_ERR_FATAL:
+		/* Device requests a reboot, as this is the only way this error
+		 * can be recovered.
+		 */
+		hidp_send_ctrl_byte(session,
+			HIDP_TRANS_HID_CONTROL|HIDP_CTRL_SOFT_RESET,
+			0);
+		break;
+	default:
+		hidp_send_ctrl_byte(session,
+			HIDP_TRANS_HANDSHAKE|HIDP_HSHK_ERR_INVALID_PARAMETER,
+			0);
+		break;
+	}
+
+}
+
+static inline void hidp_process_hid_control (struct hidp_session *session, __u8 param)
+{
+	switch (param) {
+	case HIDP_CTRL_NOP:
+		break;
+	case HIDP_CTRL_VIRTUAL_CABLE_UNPLUG:
+		/* Flush the transmit queues */
+		skb_queue_purge(&session->ctrl_transmit);
+		skb_queue_purge(&session->intr_transmit);
+
+		/* Kill session thread */
+		atomic_inc(&session->terminate);
+
+		/* Do some funky HCI stuff here to delete pairing on dongle? */
+		break;
+	case HIDP_CTRL_HARD_RESET:
+	case HIDP_CTRL_SOFT_RESET:
+	case HIDP_CTRL_SUSPEND:
+	case HIDP_CTRL_EXIT_SUSPEND:
+		/* We have to parse these and return no error */
+		break;
+	default:
+		hidp_send_ctrl_byte(session,
+			HIDP_TRANS_HANDSHAKE|HIDP_HSHK_ERR_INVALID_PARAMETER,
+			0);
+		break;
+	}
+}
+
+static inline int hidp_process_data(struct hidp_session *session, struct sk_buff *skb, __u8 param)
+{
+	int result = 0;
+
+	BT_INFO("DATA packet on control channel, GET_, SET_ not implemented");
+
+	switch (param) {
+	case HIDP_DATA_RTYPE_INPUT:
+		hidp_set_timer(session);
+
+		if (session->input)
+			hidp_input_report(session, skb);
+
+		if (session->hid) {
+			result = hid_recv_report(session->hid, 
+					HID_INPUT_REPORT, 
+					skb->data, 
+					skb->len);
+			switch (result) {
+			case -EPROTOTYPE:
+				hidp_send_ctrl_byte(session,
+                        		HIDP_TRANS_SET_PROTOCOL|HIDP_PROTO_REPORT,
+                        		0);
+				break;
+			case -EBADF:
+				hidp_send_ctrl_byte(session,
+                        		HIDP_TRANS_HANDSHAKE|HIDP_HSHK_ERR_INVALID_REPORT_ID,
+                        		0);
+				break;
+			default:
+				break;
+			}
+		}
+		break;
+	case HIDP_DATA_RTYPE_OTHER:
+	case HIDP_DATA_RTYPE_OUPUT:
+	case HIDP_DATA_RTYPE_FEATURE:
+		BT_DBG("Unimplemented DATA parameter 0x%01x", param);
+		break;
+	default:
+		BT_DBG("Invalid DATA parameter 0x%01x", param);
+		hidp_send_ctrl_byte(session,
+			HIDP_TRANS_HANDSHAKE|HIDP_HSHK_ERR_INVALID_PARAMETER,
+			0);
+	}
+	
+	return 0;
 }
 
-static inline int hidp_recv_frame(struct hidp_session *session, struct sk_buff *skb)
+
+static inline int hidp_recv_ctrl_frame(struct hidp_session *session, struct sk_buff *skb)
 {
 	__u8 hdr;
+	__u8 type;
+	__u8 param;
+	int retval = 0;
 
 	BT_DBG("session %p skb %p len %d", session, skb, skb->len);
 
 	hdr = skb->data[0];
 	skb_pull(skb, 1);
 
-	if (hdr == 0xa1) {
-		hidp_set_timer(session);
+	type = hdr & HIDP_THDR_TRANS_MASK;
+	param = hdr & HIDP_THDR_PARAM_MASK;
 
-		if (session->input)
-			hidp_input_report(session, skb);
-	} else {
+	switch (type) {
+	case HIDP_TRANS_HANDSHAKE:
+		hidp_process_handshake(session, param);
+		break;
+	case HIDP_TRANS_HID_CONTROL:
+		hidp_process_hid_control(session, param);
+		break;
+	case HIDP_TRANS_DATA:
+		retval = hidp_process_data(session, skb, param);
+		break;
+	default:
 		BT_DBG("Unsupported protocol header 0x%02x", hdr);
+		hidp_send_ctrl_byte(session,
+			HIDP_TRANS_HANDSHAKE|HIDP_HSHK_ERR_UNSUPPORTED_REQUEST,
+			0);
+		break;
 	}
+		
+	kfree_skb(skb);
+	return retval;
+}
+
+/* This needs to be separate as there are only DATA and DATC packets on the 
+ * interrupt channel. MAG
+ */ 
+static inline int hidp_recv_intr_frame(struct hidp_session *session, struct sk_buff *skb)
+{
+	__u8 hdr;
+	int retval = 0;
+	int result = 0;
+
+	BT_DBG("session %p skb %p len %d", session, skb, skb->len);
+
+	hdr = skb->data[0];
+	skb_pull(skb, 1);
+
+	if ( hdr == (HIDP_TRANS_DATA|HIDP_DATA_RTYPE_INPUT) ) {
+		hidp_set_timer(session);
+		if (session->input)
+			hidp_input_report(session, skb);
 
+		if (session->hid)
+			result = hid_recv_report(session->hid, 
+					HID_INPUT_REPORT, 
+					skb->data, 
+					skb->len);
+			switch (result) {
+			case -EPROTOTYPE:
+				hidp_send_ctrl_byte(session,
+                        		HIDP_TRANS_SET_PROTOCOL|HIDP_PROTO_REPORT,
+                        		0);
+				break;
+			default:
+				break;
+			}
+			retval = result;
+	}
+	else {
+		BT_INFO("Unsupported protocol header 0x%02x", hdr);
+	}
+		
 	kfree_skb(skb);
-	return 0;
+	return retval;
 }
 
 static int hidp_send_frame(struct socket *sock, unsigned char *data, int len)
@@ -350,12 +549,12 @@
 
 		while ((skb = skb_dequeue(&ctrl_sk->sk_receive_queue))) {
 			skb_orphan(skb);
-			hidp_recv_frame(session, skb);
+			hidp_recv_ctrl_frame(session, skb);
 		}
 
 		while ((skb = skb_dequeue(&intr_sk->sk_receive_queue))) {
 			skb_orphan(skb);
-			hidp_recv_frame(session, skb);
+			hidp_recv_intr_frame(session, skb);
 		}
 
 		hidp_process_transmit(session);
@@ -514,7 +713,9 @@
 		goto unlink;
 
 	if (session->input) {
-		hidp_send_message(session, 0x70);
+		hidp_send_ctrl_byte(session, 
+			HIDP_TRANS_SET_PROTOCOL|HIDP_PROTO_BOOT,
+			1);
 		session->flags |= (1 << HIDP_BOOT_PROTOCOL_MODE);
 
 		session->leds = 0xff;
@@ -554,14 +755,16 @@
 	session = __hidp_get_session(&req->bdaddr);
 	if (session) {
 		if (req->flags & (1 << HIDP_VIRTUAL_CABLE_UNPLUG)) {
-			hidp_send_message(session, 0x15);
+			hidp_send_ctrl_byte(session, 
+				HIDP_TRANS_HID_CONTROL|HIDP_CTRL_VIRTUAL_CABLE_UNPLUG, 
+				1);
 		} else {
 			/* Flush the transmit queues */
 			skb_queue_purge(&session->ctrl_transmit);
 			skb_queue_purge(&session->intr_transmit);
 
 			/* Kill session thread */
-			atomic_inc(&session->terminate);
+			atomic_inc(&session->terminate);	
 			hidp_schedule(session);
 		}
 	} else
diff -uNr linux-2.6.10-rc2-bk13/net/bluetooth/hidp/hidp.h linux-2.6.10-rc2-bk13-bthid/net/bluetooth/hidp/hidp.h
--- linux-2.6.10-rc2-bk13/net/bluetooth/hidp/hidp.h	2004-10-19 10:53:50.000000000 +1300
+++ linux-2.6.10-rc2-bk13-bthid/net/bluetooth/hidp/hidp.h	2004-12-02 08:03:57.857987280 +1300
@@ -26,6 +26,71 @@
 #include <linux/types.h>
 #include <net/bluetooth/bluetooth.h>
 
+/*
+ * Blue Tooth HID packet defines
+ */
+
+/*
+ * HID Transaction Types, and Transaction header stuff
+ */
+#define HIDP_THDR_TRANS_MASK		0xF0
+#define HIDP_THDR_PARAM_MASK		0x0F
+
+#define HIDP_TRANS_HANDSHAKE		0x00
+#define HIDP_TRANS_HID_CONTROL		0x10
+#define HIDP_TRANS_RSRVD_2		0x20
+#define HIDP_TRANS_RSRVD_3		0x30
+#define HIDP_TRANS_GET_REPORT		0x40
+#define HIDP_TRANS_SET_REPORT		0x50
+#define HIDP_TRANS_GET_PROTOCOL		0x60
+#define HIDP_TRANS_SET_PROTOCOL		0x70
+#define HIDP_TRANS_GET_IDLE		0x80
+#define HIDP_TRANS_SET_IDLE		0x90
+#define HIDP_TRANS_DATA			0xA0
+#define HIDP_TRANS_DATC			0xB0
+#define HIDP_TRANS_RSRVD_C		0xC0
+#define HIDP_TRANS_RSRVD_D		0xD0
+#define HIDP_TRANS_RSVRD_E		0xE0
+#define HIDP_TRANS_RSVRD_F		0xF0
+
+/*
+ * HID Handshake results returned in the result parameter of the handshake 
+ * transaction HID packet 
+ */
+#define HIDP_HSHK_SUCCESSFUL			0x00
+#define HIDP_HSHK_NOT_READY			0x01
+#define HIDP_HSHK_ERR_INVALID_REPORT_ID		0x02
+#define HIDP_HSHK_ERR_UNSUPPORTED_REQUEST	0x03
+#define HIDP_HSHK_ERR_INVALID_PARAMETER		0x04
+#define HIDP_HSHK_ERR_UNKNOWN			0x0E
+#define HIDP_HSHK_ERR_FATAL			0x0F
+
+/*
+ * HID HID_CONTROL operation parameter
+ */
+#define HIDP_CTRL_NOP			0x00	/* No operation */
+#define HIDP_CTRL_HARD_RESET		0x01	/* Request hard reset */
+#define HIDP_CTRL_SOFT_RESET		0x02	/* Request soft reset */
+#define HIDP_CTRL_SUSPEND		0x03	/* Request device to suspend */
+#define HIDP_CTRL_EXIT_SUSPEND		0x04	/* request exit suspend */
+#define HIDP_CTRL_VIRTUAL_CABLE_UNPLUG	0x05	/* only one Mouse/kbd can send */
+
+/*
+ * HID DATA Transaction header parameter nibble
+ */
+#define HIDP_DATA_RTYPE_MASK		0x03
+#define HIDP_DATA_RSRVD_MASK		0x0C
+#define HIDP_DATA_RTYPE_OTHER		0x00
+#define HIDP_DATA_RTYPE_INPUT		0x01
+#define HIDP_DATA_RTYPE_OUPUT		0x02
+#define HIDP_DATA_RTYPE_FEATURE		0x03
+
+/*
+ * HID SET_PROTOCOL header parameter nibble
+ */
+#define HIDP_PROTO_BOOT			0x00
+#define HIDP_PROTO_REPORT		0x01
+
 /* HIDP ioctl defines */
 #define HIDPCONNADD	_IOW('H', 200, int)
 #define HIDPCONNDEL	_IOW('H', 201, int)
diff -uNr linux-2.6.10-rc2-bk13/net/bluetooth/hidp/Kconfig linux-2.6.10-rc2-bk13-bthid/net/bluetooth/hidp/Kconfig
--- linux-2.6.10-rc2-bk13/net/bluetooth/hidp/Kconfig	2004-10-19 10:54:38.000000000 +1300
+++ linux-2.6.10-rc2-bk13-bthid/net/bluetooth/hidp/Kconfig	2004-12-02 08:03:55.595331256 +1300
@@ -10,3 +10,27 @@
 	  Say Y here to compile HIDP support into the kernel or say M to
 	  compile it as module (hidp).
 
+config BT_HIDP_DEBUG_BT
+	bool "HIDP BT Debug code"
+	depends on BT_HIDP
+	select INPUT
+	help
+	  Turn this on if you have problems and want to see what the Blue Tooth 
+          side of the code is doing.
+
+config BT_HIDP_DEBUG_REPORT
+	bool "HIDP Report mode Debug code"
+	depends on BT_HIDP
+	select INPUT
+	help
+	  Turn this on if you have problems and want to see what the HID Report 
+          part of the code is doing.
+
+
+config BT_HIDP_DEBUG_DATA
+	bool "HIDP Debug of Data"
+	depends on BT_HIDP
+	select INPUT
+	help
+	  Turn this on for debug printout of BT HIDP data.
+

[-- Attachment #1.3: linux-2.6.9-mh4-maghidp2004112801.patch --]
[-- Type: text/x-patch, Size: 14106 bytes --]

--- kernel-source-2.6.9-mag/net/bluetooth/hidp/Kconfig.mh3	2004-10-19 10:54:38.000000000 +1300
+++ kernel-source-2.6.9-mag/net/bluetooth/hidp/Kconfig	2004-11-16 08:15:02.000000000 +1300
@@ -10,3 +10,27 @@
 	  Say Y here to compile HIDP support into the kernel or say M to
 	  compile it as module (hidp).
 
+config BT_HIDP_DEBUG_BT
+	bool "HIDP BT Debug code"
+	depends on BT_HIDP
+	select INPUT
+	help
+	  Turn this on if you have problems and want to see what the Blue Tooth 
+          side of the code is doing.
+
+config BT_HIDP_DEBUG_REPORT
+	bool "HIDP Report mode Debug code"
+	depends on BT_HIDP
+	select INPUT
+	help
+	  Turn this on if you have problems and want to see what the HID Report 
+          part of the code is doing.
+
+
+config BT_HIDP_DEBUG_DATA
+	bool "HIDP Debug of Data"
+	depends on BT_HIDP
+	select INPUT
+	help
+	  Turn this on for debug printout of BT HIDP data.
+
--- kernel-source-2.6.9-mag/net/bluetooth/hidp/core.c.mh3	2004-11-28 12:57:06.536278608 +1300
+++ kernel-source-2.6.9-mag/net/bluetooth/hidp/core.c	2004-11-28 11:09:50.836653664 +1300
@@ -46,7 +46,7 @@
 
 #include "hidp.h"
 
-#ifndef CONFIG_BT_HIDP_DEBUG
+#ifndef CONFIG_BT_HIDP_DEBUG_BT
 #undef  BT_DBG
 #define BT_DBG(D...)
 #endif
@@ -138,8 +138,8 @@
 	struct sk_buff *skb;
 	unsigned char newleds;
 
-	BT_DBG("session %p hid %p data %p size %d", session, device, data, size);
-
+	BT_DBG("");
+	 
 	if (type != EV_LED)
 		return -1;
 
@@ -159,7 +159,7 @@
 		return -ENOMEM;
 	}
 
-	*skb_put(skb, 1) = 0xa2;
+	*skb_put(skb, 1) = HIDP_TRANS_DATA|HIDP_DATA_RTYPE_OUPUT;
 	*skb_put(skb, 1) = 0x01;
 	*skb_put(skb, 1) = newleds;
 
@@ -263,7 +263,36 @@
 		del_timer(&session->timer);
 }
 
-static inline void hidp_send_message(struct hidp_session *session, unsigned char hdr)
+static inline int hidp_send_ctrl_message(struct hidp_session *session,
+			unsigned char hdr,
+			unsigned char *data, int size)
+{
+	struct sk_buff *skb;
+
+	BT_DBG("session %p data %p size %d", session, data, size);
+
+	if (!(skb = alloc_skb(size + 1, GFP_ATOMIC))) {
+		BT_ERR("Can't allocate memory for new frame");
+		return -ENOMEM;
+	}
+
+	*skb_put(skb, 1) = hdr;
+	if (size > 0)
+		memcpy(skb_put(skb, size), data, size);
+
+	skb_queue_tail(&session->ctrl_transmit, skb);
+
+	hidp_schedule(session);
+
+	return 0;
+}
+
+/* Send a 1 byte control message.
+ *   Schedule flag has to be set if calling from outside HID session process 
+ */
+static inline void hidp_send_ctrl_byte(struct hidp_session *session,
+		 			unsigned char hdr,
+					int schedule)
 {
 	struct sk_buff *skb;
 
@@ -278,32 +307,199 @@
 
 	skb_queue_tail(&session->ctrl_transmit, skb);
 
-	hidp_schedule(session);
+	if (schedule)
+		hidp_schedule(session);
+}
+
+static inline void hidp_process_handshake(struct hidp_session *session, __u8 param)
+{
+	switch (param) {
+	case HIDP_HSHK_SUCCESSFUL:
+		/* Call into SET_ GET_ handlers here */
+		break;
+	case HIDP_HSHK_NOT_READY:
+	case HIDP_HSHK_ERR_INVALID_REPORT_ID:
+	case HIDP_HSHK_ERR_UNSUPPORTED_REQUEST:
+	case HIDP_HSHK_ERR_INVALID_PARAMETER:
+		/* Call into SET_ GET_ handlers here */
+		break;
+	case HIDP_HSHK_ERR_UNKNOWN:
+		BT_INFO("HANDSHAKE parameter ERR_UNKNOWN seen.");
+		break;
+	case HIDP_HSHK_ERR_FATAL:
+		/* Device requests a reboot, as this is the only way this error
+ 		 * can be recovered.
+		 */
+		hidp_send_ctrl_byte(session,
+			HIDP_TRANS_HID_CONTROL|HIDP_CTRL_SOFT_RESET,
+			0);
+		break;
+	default:
+		hidp_send_ctrl_byte(session,
+			HIDP_TRANS_HANDSHAKE|HIDP_HSHK_ERR_INVALID_PARAMETER,
+			0);
+		break;
+	}
+
+}
+
+static inline void hidp_process_hid_control (struct hidp_session *session, __u8 param)
+{
+	switch (param) {
+	case HIDP_CTRL_NOP:
+		break;
+	case HIDP_CTRL_VIRTUAL_CABLE_UNPLUG:
+		/* Flush the transmit queues */
+		skb_queue_purge(&session->ctrl_transmit);
+		skb_queue_purge(&session->intr_transmit);
+
+		/* Kill session thread */
+		atomic_inc(&session->terminate);	
+
+		/* Do some funky HCI stuff here to delete pairing on dongle? */
+		break;
+	case HIDP_CTRL_HARD_RESET:
+	case HIDP_CTRL_SOFT_RESET:
+	case HIDP_CTRL_SUSPEND:
+	case HIDP_CTRL_EXIT_SUSPEND:
+		/* We have to parse these and return no error */
+		break;
+	default:
+		hidp_send_ctrl_byte(session,
+			HIDP_TRANS_HANDSHAKE|HIDP_HSHK_ERR_INVALID_PARAMETER,
+			0);
+		break;
+	}
+}
+
+static inline int hidp_process_data(struct hidp_session *session, struct sk_buff *skb, __u8 param)
+{
+	int result = 0;
+
+	BT_INFO("DATA packet on control channel, GET_, SET_ not implemented");
+
+	switch (param) {
+	case HIDP_DATA_RTYPE_INPUT:
+		hidp_set_timer(session);
+
+		if (session->input)
+			hidp_input_report(session, skb);
+
+		if (session->hid) {
+			result = hid_recv_report(session->hid, 
+					HID_INPUT_REPORT, 
+					skb->data, 
+					skb->len);
+			switch (result) {
+			case -EPROTOTYPE:
+				hidp_send_ctrl_byte(session,
+                        		HIDP_TRANS_SET_PROTOCOL|HIDP_PROTO_REPORT,
+                        		0);
+				break;
+			case -EBADF:
+				hidp_send_ctrl_byte(session,
+                        		HIDP_TRANS_HANDSHAKE|HIDP_HSHK_ERR_INVALID_REPORT_ID,
+                        		0);
+				break;
+			default:
+				break;
+			}
+		}
+		break;
+	case HIDP_DATA_RTYPE_OTHER:
+	case HIDP_DATA_RTYPE_OUPUT:
+	case HIDP_DATA_RTYPE_FEATURE:
+		BT_DBG("Unimplemented DATA parameter 0x%01x", param);
+		break;
+	default:
+		BT_DBG("Invalid DATA parameter 0x%01x", param);
+		hidp_send_ctrl_byte(session,
+			HIDP_TRANS_HANDSHAKE|HIDP_HSHK_ERR_INVALID_PARAMETER,
+			0);
+	}
+	
+	return 0;
 }
 
-static inline int hidp_recv_frame(struct hidp_session *session, struct sk_buff *skb)
+
+static inline int hidp_recv_ctrl_frame(struct hidp_session *session, struct sk_buff *skb)
 {
 	__u8 hdr;
+	__u8 type;
+	__u8 param;
+	int retval = 0;
 
 	BT_DBG("session %p skb %p len %d", session, skb, skb->len);
 
 	hdr = skb->data[0];
 	skb_pull(skb, 1);
 
-	if (hdr == 0xa1) {
-		hidp_set_timer(session);
+	type = hdr & HIDP_THDR_TRANS_MASK;
+	param = hdr & HIDP_THDR_PARAM_MASK;
 
+	switch (type) {
+	case HIDP_TRANS_HANDSHAKE:
+		hidp_process_handshake(session, param);
+		break;
+	case HIDP_TRANS_HID_CONTROL:
+		hidp_process_hid_control(session, param);
+		break;
+	case HIDP_TRANS_DATA:
+		retval = hidp_process_data(session, skb, param);
+		break;
+	default:
+		BT_DBG("Unsupported protocol header 0x%02x", hdr);
+		hidp_send_ctrl_byte(session,
+			HIDP_TRANS_HANDSHAKE|HIDP_HSHK_ERR_UNSUPPORTED_REQUEST,
+			0);
+		break;
+	}
+		
+	kfree_skb(skb);
+	return retval;
+}
+
+/* This needs to be separate as there are only DATA and DATC packets on the 
+ * interrupt channel. MAG
+ */ 
+static inline int hidp_recv_intr_frame(struct hidp_session *session, struct sk_buff *skb)
+{
+	__u8 hdr;
+	int retval = 0;
+	int result = 0;
+
+	BT_DBG("session %p skb %p len %d", session, skb, skb->len);
+
+	hdr = skb->data[0];
+	skb_pull(skb, 1);
+
+	if ( hdr == (HIDP_TRANS_DATA|HIDP_DATA_RTYPE_INPUT) ) {
+		hidp_set_timer(session);
 		if (session->input)
 			hidp_input_report(session, skb);
 
 		if (session->hid)
-			hid_recv_report(session->hid, HID_INPUT_REPORT, skb->data, skb->len);
-	} else {
-		BT_DBG("Unsupported protocol header 0x%02x", hdr);
+			result = hid_recv_report(session->hid, 
+					HID_INPUT_REPORT, 
+					skb->data, 
+					skb->len);
+			switch (result) {
+			case -EPROTOTYPE:
+				hidp_send_ctrl_byte(session,
+                        		HIDP_TRANS_SET_PROTOCOL|HIDP_PROTO_REPORT,
+                        		0);
+				break;
+			default:
+				break;
+			}
+			retval = result;
 	}
-
+	else {
+		BT_INFO("Unsupported protocol header 0x%02x", hdr);
+	}
+		
 	kfree_skb(skb);
-	return 0;
+	return retval;
 }
 
 static int hidp_send_frame(struct socket *sock, unsigned char *data, int len)
@@ -389,12 +585,12 @@
 
 		while ((skb = skb_dequeue(&ctrl_sk->sk_receive_queue))) {
 			skb_orphan(skb);
-			hidp_recv_frame(session, skb);
+			hidp_recv_ctrl_frame(session, skb);
 		}
 
 		while ((skb = skb_dequeue(&intr_sk->sk_receive_queue))) {
 			skb_orphan(skb);
-			hidp_recv_frame(session, skb);
+			hidp_recv_intr_frame(session, skb);
 		}
 
 		hidp_process_transmit(session);
@@ -608,7 +804,9 @@
 		goto unlink;
 
 	if (session->input) {
-		hidp_send_message(session, 0x70);
+		hidp_send_ctrl_byte(session, 
+			HIDP_TRANS_SET_PROTOCOL|HIDP_PROTO_BOOT,
+			1);
 		session->flags |= (1 << HIDP_BOOT_PROTOCOL_MODE);
 
 		session->leds = 0xff;
@@ -654,14 +852,16 @@
 	session = __hidp_get_session(&req->bdaddr);
 	if (session) {
 		if (req->flags & (1 << HIDP_VIRTUAL_CABLE_UNPLUG)) {
-			hidp_send_message(session, 0x15);
+			hidp_send_ctrl_byte(session, 
+				HIDP_TRANS_HID_CONTROL|HIDP_CTRL_VIRTUAL_CABLE_UNPLUG, 
+				1);
 		} else {
 			/* Flush the transmit queues */
 			skb_queue_purge(&session->ctrl_transmit);
 			skb_queue_purge(&session->intr_transmit);
 
 			/* Kill session thread */
-			atomic_inc(&session->terminate);
+			atomic_inc(&session->terminate);	
 			hidp_schedule(session);
 		}
 	} else
--- kernel-source-2.6.9-mag/net/bluetooth/hidp/hid.c.mh3	2004-11-07 15:50:20.000000000 +1300
+++ kernel-source-2.6.9-mag/net/bluetooth/hidp/hid.c	2004-11-27 17:50:19.000000000 +1300
@@ -858,11 +858,11 @@
 
 	if (!size) {
 		dbg("empty report");
-		return -1;
+		return -EINVAL;
 	}
 
-#ifdef DEBUG_DATA
-	printk(KERN_DEBUG __FILE__ ": report (size %u) (%snumbered)\n", len, report_enum->numbered ? "" : "un");
+#ifdef CONFIG_BT_HIDP_DEBUG_DATA
+	printk(KERN_DEBUG __FILE__ ": report (size %u) (%snumbered)\n", size, report_enum->numbered ? "" : "un");
 #endif
 
 	n = 0;				/* Normally report number is 0 */
@@ -871,7 +871,7 @@
 		size--;
 	}
 
-#ifdef DEBUG_DATA
+#ifdef CONFIG_BT_HIDP_DEBUG_DATA
 	{
 		int i;
 		printk(KERN_DEBUG __FILE__ ": report %d (size %u) = ", n, size);
@@ -883,14 +883,33 @@
 
 	if (!(report = report_enum->report_id_hash[n])) {
 		dbg("undefined report_id %d received", n);
-		return -1;
+		return -EBADF;
 	}
 
 	rsize = ((report->size - 1) >> 3) + 1;
 
+#ifdef CONFIG_BT_HIDP_DEBUG_DATA
+	dbg("report %d, expected %d bits, %d maxfield", report->id, report->size, report->maxfield);
+	for (n = 0; n < report->maxfield; n++)
+		dbg("report:field %d:%d, count %d , offset %d, size(bits) %d",
+			report->id, n,
+			report->field[n]->report_count,
+			report->field[n]->report_offset,
+			report->field[n]->report_size);
+#endif
+
 	if (size < rsize) {
-		dbg("report %d is too short, (%d < %d)", report->id, size, rsize);
-		return -1;
+		dbg("report %d is too short, (%d < %d) %d bits expected, %d maxfield", report->id, size, rsize, report->size, report->maxfield);
+		if (size == 8) {
+			/* 
+			 * FIXME - need to check if device is a keyboard!!
+			 */
+			dbg("tell upper layer wrong protocol - switch to report"); 
+			return -EPROTOTYPE;
+		}
+		else {
+			return -EMSGSIZE;
+		}
 	}
 
 	for (n = 0; n < report->maxfield; n++)
--- kernel-source-2.6.9-mag/net/bluetooth/hidp/hidp.h.mh3	2004-11-07 15:50:20.000000000 +1300
+++ kernel-source-2.6.9-mag/net/bluetooth/hidp/hidp.h	2004-11-28 10:34:27.331475520 +1300
@@ -26,6 +26,71 @@
 #include <linux/types.h>
 #include <net/bluetooth/bluetooth.h>
 
+/*
+ * Blue Tooth HID packet defines
+ */
+
+/*
+ * HID Transaction Types, and Transaction header stuff
+ */
+#define HIDP_THDR_TRANS_MASK		0xF0
+#define HIDP_THDR_PARAM_MASK		0x0F
+
+#define HIDP_TRANS_HANDSHAKE		0x00
+#define HIDP_TRANS_HID_CONTROL		0x10
+#define HIDP_TRANS_RSRVD_2		0x20
+#define HIDP_TRANS_RSRVD_3		0x30
+#define HIDP_TRANS_GET_REPORT		0x40
+#define HIDP_TRANS_SET_REPORT		0x50
+#define HIDP_TRANS_GET_PROTOCOL		0x60
+#define HIDP_TRANS_SET_PROTOCOL		0x70
+#define HIDP_TRANS_GET_IDLE		0x80
+#define HIDP_TRANS_SET_IDLE		0x90
+#define HIDP_TRANS_DATA			0xA0
+#define HIDP_TRANS_DATC			0xB0
+#define HIDP_TRANS_RSRVD_C		0xC0
+#define HIDP_TRANS_RSRVD_D		0xD0
+#define HIDP_TRANS_RSVRD_E		0xE0
+#define HIDP_TRANS_RSVRD_F		0xF0
+
+/*
+ * HID Handshake results returned in the result parameter of the handshake 
+ * transaction HID packet 
+ */
+#define HIDP_HSHK_SUCCESSFUL			0x00
+#define HIDP_HSHK_NOT_READY			0x01
+#define HIDP_HSHK_ERR_INVALID_REPORT_ID		0x02
+#define HIDP_HSHK_ERR_UNSUPPORTED_REQUEST	0x03
+#define HIDP_HSHK_ERR_INVALID_PARAMETER		0x04
+#define HIDP_HSHK_ERR_UNKNOWN			0x0E
+#define HIDP_HSHK_ERR_FATAL			0x0F
+
+/*
+ * HID HID_CONTROL operation parameter
+ */
+#define HIDP_CTRL_NOP			0x00	/* No operation */
+#define HIDP_CTRL_HARD_RESET		0x01	/* Request hard reset */
+#define HIDP_CTRL_SOFT_RESET		0x02	/* Request soft reset */
+#define HIDP_CTRL_SUSPEND		0x03	/* Request device to suspend */
+#define HIDP_CTRL_EXIT_SUSPEND		0x04	/* request exit suspend */
+#define HIDP_CTRL_VIRTUAL_CABLE_UNPLUG	0x05	/* only one Mouse/kbd can send */
+
+/*
+ * HID DATA Transaction header parameter nibble
+ */
+#define HIDP_DATA_RTYPE_MASK		0x03
+#define HIDP_DATA_RSRVD_MASK		0x0C
+#define HIDP_DATA_RTYPE_OTHER		0x00
+#define HIDP_DATA_RTYPE_INPUT		0x01
+#define HIDP_DATA_RTYPE_OUPUT		0x02
+#define HIDP_DATA_RTYPE_FEATURE		0x03
+
+/*
+ * HID SET_PROTOCOL header parameter nibble
+ */
+#define HIDP_PROTO_BOOT			0x00
+#define HIDP_PROTO_REPORT		0x01
+
 /* HIDP ioctl defines */
 #define HIDPCONNADD	_IOW('H', 200, int)
 #define HIDPCONNDEL	_IOW('H', 201, int)
--- kernel-source-2.6.9-mag/net/bluetooth/hidp/hid.h.mh3	2004-11-28 12:48:49.800793880 +1300
+++ kernel-source-2.6.9-mag/net/bluetooth/hidp/hid.h	2004-11-16 08:13:45.000000000 +1300
@@ -26,7 +26,7 @@
 #ifndef __HID_H
 #define __HID_H
 
-#ifdef DEBUG
+#ifdef CONFIG_BT_HIDP_DEBUG_REPORT
 #define dbg(format, arg...) printk(KERN_DEBUG "%s: " format "\n" , __FILE__ , ## arg)
 #else
 #define dbg(format, arg...) do {} while (0)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[Bluez-devel] Re: [PATCH] - for BT HID fixes + beginning of ctrl handling

[Marcel Holtmann]

Hi Matthew,

don't do any cross postings and there is no need to address these mails
to me directly. I read the mailing lists ;)

> Please find 2 patches attached.  My original against 2.6.9-mh4, and one
> for 2.6.10-rc2-bk.  These are the patches I can easily produce. Is this
> OK with you? Please let me know what else you need and I will see what
> can be done.

from a first review, this looks good so far, but please don't change any
debug stuff. Leave it as it is. I am not going to apply any debug config
options.

It is "Bluetooth" and not "Blue Tooth" or "BlueTooth" and you should
follow more the coding style:

	"hidp_process_hid_control (" ==> "hidp_process_hid_control("
	"if ( hdr" == > "if (hdr"
	"HIDP_TRANS_DATA|HIDP_D..." ==> "HIDP_TRANS_DATA | HIDP_D..."

I think you see what I mean and don't introduce extra whitespaces:

-                 atomic_inc(&session->terminate);<== correct
+                 atomic_inc(&session->terminate);        <== wrong


There is no need to use "retval" and "result". Use one of them and I
actually prefer using "err".

And you must explain the extra need of the "schedule" parameter to me.

> I have not one yet between straight 2.6.10-rc2-bk+base-bthid+-mh4 and my
> trunk as the source tree manipulations are getting quite convoluted.
> Making this patching easier is something I would really like to sort out
> for both our work flows.

Fix the stuff I mentioned above and don't forget the Signed-off-by line
and I will start testing it on my machines.

> My question is, how do you manage the patch creation of -mh4 patches
> etc? Looks like this just a simple diff of your trunk net/bluetooth and
> drivers/bluetooth against current mainstream kernel.

The -mh patches are created by a bunch of scripts. Actually every patch
is part of a collection and these scripts will apply them in the correct
order and then rediff against vanilla. If all my work is merged mainline
then this a diff of mainline versus trunk. But sometimes there are also
testing patches in there, like the HIDP report mode support and the
RFCOMM service level security stuff.

Regards

Marcel




-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now. 
http://productguide.itmanagersjournal.com/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

[Bluez-devel] Re: [PATCH] - for BT HID fixes + beginning of ctrl handling

[Matthew Grant]

[-- Attachment #1: Type: text/plain, Size: 2913 bytes --]

Hi Marcel,

Busy weekend.  Remotely upgrading a Debian box, and upgrading its
hardware, having to take it one step at a time. I have started editing
the code to put in the style changes you recommended. Should be able to
get something to you tomorrow.

On Thu, 2004-12-02 at 12:35 +0100, Marcel Holtmann wrote:
> Hi Matthew,
> 
> as a side note: Finally decide if you are going to discuss this in
> private with me or on the mailing list. Don't switch back and forth.
> 

Probably bets to discuss this matter on bluez-devel.

> > At times the hidp_send_message gets called from within the context of
> > the running kernel thread when responding to received messages with
> > error handshakes, and you want to keep processing if possible until the
> > original hidp_schedule() after the transmission of messages.  The aim is
> > to keep the execution flow as you planned it originally.
> > 
> > Do you think a separate function is needed, without the schedule call at
> > the end?
> 
> Actually a schedule should not harm in any way. If you think we really
> need it then I would prefer the same as a looked() versus __unlocked()
> functions.

Like you I want to keep things simple here, but I want to also avoid the
effects of giving up the processor to readily.  Also I want to avoid the
complications of more locks.  The reply messages are being sent inside
the global? Bluetooth HID lock.  We have to be careful about the context
that this is all happening within.  If you are that concerned about the
third function schedule parameter, I will create 2 separate control
message sending functions as they are only 3-4 lines long.

> > Thinking about how best to look after all the code and patching.
> > Putting all the BT code into a version control system here is looking to
> > be quite sensible to make the patching easier to handle.  If we were not
> > patching core.c for the report mode support, this would be a lot easier
> > and cleaner.
> 
> I don't see any need for it at the moment. Once we get your HIDP core
> stuff ready, I will try to merge it back mainline as soon as possible.
> Every change after that should be minimal.
> 

This first patch is only phase 1 in a 3 stage process.  Next 2 steps
will be just as big and invasive.  After this probably half of the BT
HID may be changed around quite significantly.  Please accept Debug code
code changes I make, as I need these to get debug code to compile for my
own use.  The version control system is so that I can more easily keep
track of what I am doing.

As I write this, I am making changes you suggested to my code so that we
can get it merged.  I still have a lot of this work on the machine in
Bangladesh to deal with for the next couple of nights till I can put it
on hold for a week or two before their final upgrade from woody to
sarge.

Best Regards,

Matthew Grant

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Bluez-devel] Re: [PATCH] - for BT HID fixes + beginning of ctrl handling

[Marcel Holtmann]

Hi Matthew,

> > Actually a schedule should not harm in any way. If you think we really
> > need it then I would prefer the same as a looked() versus __unlocked()
> > functions.
> 
> Like you I want to keep things simple here, but I want to also avoid the
> effects of giving up the processor to readily.  Also I want to avoid the
> complications of more locks.  The reply messages are being sent inside
> the global? Bluetooth HID lock.  We have to be careful about the context
> that this is all happening within.  If you are that concerned about the
> third function schedule parameter, I will create 2 separate control
> message sending functions as they are only 3-4 lines long.

maybe we have to move the hidp_schedule() call around. Don't duplicate
code even if they are only 3-4 lines long.

> > > Thinking about how best to look after all the code and patching.
> > > Putting all the BT code into a version control system here is looking to
> > > be quite sensible to make the patching easier to handle.  If we were not
> > > patching core.c for the report mode support, this would be a lot easier
> > > and cleaner.
> > 
> > I don't see any need for it at the moment. Once we get your HIDP core
> > stuff ready, I will try to merge it back mainline as soon as possible.
> > Every change after that should be minimal.
> > 
> 
> This first patch is only phase 1 in a 3 stage process.  Next 2 steps
> will be just as big and invasive.  After this probably half of the BT
> HID may be changed around quite significantly.  Please accept Debug code
> code changes I make, as I need these to get debug code to compile for my
> own use.  The version control system is so that I can more easily keep
> track of what I am doing.

The debug code is for the development stage. After the merge back we
remove it and leave only a minimal amount of debug code in there. That
is also the reason why no config option for it exists even if the define
looks like one.

Regards

Marcel




-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now. 
http://productguide.itmanagersjournal.com/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel