linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] IR/mceusb: kill pinnacle-device-specific nonsense
@ 2010-06-16 20:10 Jarod Wilson
  2010-07-03  4:02 ` [PATCH v3] " Jarod Wilson
  0 siblings, 1 reply; 5+ messages in thread
From: Jarod Wilson @ 2010-06-16 20:10 UTC (permalink / raw)
  To: linux-media

I have pinnacle hardware now. None of this pinnacle-specific crap is at
all necessary (in fact, some of it needed to be removed to actually make
it work). The only thing unique about this device is that it often
transfers inbound data w/a header of 0x90, meaning 16 bytes of IR data
following it, so I had to make adjustments for that, and now its working
perfectly fine.

Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/media/IR/mceusb.c |   63 ++++++++++-----------------------------------
 1 files changed, 14 insertions(+), 49 deletions(-)

diff --git a/drivers/media/IR/mceusb.c b/drivers/media/IR/mceusb.c
index 756f718..708a71a 100644
--- a/drivers/media/IR/mceusb.c
+++ b/drivers/media/IR/mceusb.c
@@ -68,7 +68,7 @@
 #define MCE_PULSE_BIT	0x80 /* Pulse bit, MSB set == PULSE else SPACE */
 #define MCE_PULSE_MASK	0x7F /* Pulse mask */
 #define MCE_MAX_PULSE_LENGTH 0x7F /* Longest transmittable pulse symbol */
-#define MCE_PACKET_LENGTH_MASK  0xF /* Packet length mask */
+#define MCE_PACKET_LENGTH_MASK  0x1F /* Packet length mask */
 
 
 /* module parameters */
@@ -209,11 +209,6 @@ static struct usb_device_id gen3_list[] = {
 	{}
 };
 
-static struct usb_device_id pinnacle_list[] = {
-	{ USB_DEVICE(VENDOR_PINNACLE, 0x0225) },
-	{}
-};
-
 static struct usb_device_id microsoft_gen1_list[] = {
 	{ USB_DEVICE(VENDOR_MICROSOFT, 0x006d) },
 	{}
@@ -542,6 +537,7 @@ static void mceusb_process_ir_data(struct mceusb_dev *ir, int buf_len)
 {
 	struct ir_raw_event rawir = { .pulse = false, .duration = 0 };
 	int i, start_index = 0;
+	u8 hdr = MCE_CONTROL_HEADER;
 
 	/* skip meaningless 0xb1 0x60 header bytes on orig receiver */
 	if (ir->flags.microsoft_gen1)
@@ -551,15 +547,16 @@ static void mceusb_process_ir_data(struct mceusb_dev *ir, int buf_len)
 		if (ir->rem == 0) {
 			/* decode mce packets of the form (84),AA,BB,CC,DD */
 			/* IR data packets can span USB messages - rem */
-			ir->rem = (ir->buf_in[i] & MCE_PACKET_LENGTH_MASK);
-			ir->cmd = (ir->buf_in[i] & ~MCE_PACKET_LENGTH_MASK);
+			hdr = ir->buf_in[i];
+			ir->rem = (hdr & MCE_PACKET_LENGTH_MASK);
+			ir->cmd = (hdr & ~MCE_PACKET_LENGTH_MASK);
 			dev_dbg(ir->dev, "New data. rem: 0x%02x, cmd: 0x%02x\n",
 				ir->rem, ir->cmd);
 			i++;
 		}
 
-		/* Only cmd 0x8<bytes> is IR data, don't process MCE commands */
-		if (ir->cmd != 0x80) {
+		/* don't process MCE commands */
+		if (hdr == MCE_CONTROL_HEADER || hdr == 0xff) {
 			ir->rem = 0;
 			return;
 		}
@@ -841,12 +838,10 @@ static int __devinit mceusb_dev_probe(struct usb_interface *intf,
 	struct usb_endpoint_descriptor *ep_out = NULL;
 	struct usb_host_config *config;
 	struct mceusb_dev *ir = NULL;
-	int pipe, maxp;
-	int i, ret;
+	int pipe, maxp, i;
 	char buf[63], name[128] = "";
 	bool is_gen3;
 	bool is_microsoft_gen1;
-	bool is_pinnacle;
 	bool tx_mask_inverted;
 
 	dev_dbg(&intf->dev, ": %s called\n", __func__);
@@ -858,7 +853,6 @@ static int __devinit mceusb_dev_probe(struct usb_interface *intf,
 
 	is_gen3 = usb_match_id(intf, gen3_list) ? 1 : 0;
 	is_microsoft_gen1 = usb_match_id(intf, microsoft_gen1_list) ? 1 : 0;
-	is_pinnacle = usb_match_id(intf, pinnacle_list) ? 1 : 0;
 	tx_mask_inverted = usb_match_id(intf, std_tx_mask_list) ? 0 : 1;
 
 	/* step through the endpoints to find first bulk in and out endpoint */
@@ -873,19 +867,11 @@ static int __devinit mceusb_dev_probe(struct usb_interface *intf,
 			|| ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
 			    == USB_ENDPOINT_XFER_INT))) {
 
-			dev_dbg(&intf->dev, ": acceptable inbound endpoint "
-				"found\n");
 			ep_in = ep;
 			ep_in->bmAttributes = USB_ENDPOINT_XFER_INT;
-			if (!is_pinnacle)
-				/*
-				 * Ideally, we'd use what the device offers up,
-				 * but that leads to non-functioning first and
-				 * second-gen devices, and many devices have an
-				 * invalid bInterval of 0. Pinnacle devices
-				 * don't work witha  bInterval of 1 though.
-				 */
-				ep_in->bInterval = 1;
+			ep_in->bInterval = 1;
+			dev_dbg(&intf->dev, ": acceptable inbound endpoint "
+				"found\n");
 		}
 
 		if ((ep_out == NULL)
@@ -896,19 +882,11 @@ static int __devinit mceusb_dev_probe(struct usb_interface *intf,
 			|| ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
 			    == USB_ENDPOINT_XFER_INT))) {
 
-			dev_dbg(&intf->dev, ": acceptable outbound endpoint "
-				"found\n");
 			ep_out = ep;
 			ep_out->bmAttributes = USB_ENDPOINT_XFER_INT;
-			if (!is_pinnacle)
-				/*
-				 * Ideally, we'd use what the device offers up,
-				 * but that leads to non-functioning first and
-				 * second-gen devices, and many devices have an
-				 * invalid bInterval of 0. Pinnacle devices
-				 * don't work witha  bInterval of 1 though.
-				 */
-				ep_out->bInterval = 1;
+			ep_out->bInterval = 1;
+			dev_dbg(&intf->dev, ": acceptable outbound endpoint "
+				"found\n");
 		}
 	}
 	if (ep_in == NULL) {
@@ -962,19 +940,6 @@ static int __devinit mceusb_dev_probe(struct usb_interface *intf,
 	ir->urb_in->transfer_dma = ir->dma_in;
 	ir->urb_in->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
 
-	if (is_pinnacle) {
-		/*
-		 * I have no idea why but this reset seems to be crucial to
-		 * getting the device to do outbound IO correctly - without
-		 * this the device seems to hang, ignoring all input - although
-		 * IR signals are correctly sent from the device, no input is
-		 * interpreted by the device and the host never does the
-		 * completion routine
-		 */
-		ret = usb_reset_configuration(dev);
-		dev_info(&intf->dev, "usb reset config ret %x\n", ret);
-	}
-
 	/* initialize device */
 	if (ir->flags.gen3)
 		mceusb_gen3_init(ir);
-- 
1.6.5.2

-- 
Jarod Wilson
jarod@redhat.com


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

* [PATCH v3] IR/mceusb: kill pinnacle-device-specific nonsense
  2010-06-16 20:10 [PATCH] IR/mceusb: kill pinnacle-device-specific nonsense Jarod Wilson
@ 2010-07-03  4:02 ` Jarod Wilson
  2010-07-03 22:17   ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 5+ messages in thread
From: Jarod Wilson @ 2010-07-03  4:02 UTC (permalink / raw)
  To: linux-media

I have pinnacle hardware now. None of this pinnacle-specific crap is at
all necessary (in fact, some of it needed to be removed to actually make
it work). The only thing unique about this device is that it often
transfers inbound data w/a header of 0x90, meaning 16 bytes of IR data
following it, so I had to make adjustments for that, and now its working
perfectly fine.

v2: stillborn

v3: remove completely unnecessary usb_reset_device() call that only served
to piss off the pinnacle device regularly and unify/simplify some of the
generation-specific device initialization code.

post-mortem: it seems the pinnacle hardware actually still gets pissed off
from time to time, but I can (try) to fix that later (if possible). The
patch is still quite helpful from a code reduction standpoint.

Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 Makefile                  |    2 +-
 drivers/media/IR/mceusb.c |  110 +++++++++------------------------------------
 2 files changed, 22 insertions(+), 90 deletions(-)

diff --git a/Makefile b/Makefile
index 6e39ec7..0417c74 100644
--- a/Makefile
+++ b/Makefile
@@ -1,7 +1,7 @@
 VERSION = 2
 PATCHLEVEL = 6
 SUBLEVEL = 35
-EXTRAVERSION = -rc1
+EXTRAVERSION = -rc1-ir
 NAME = Sheep on Meth
 
 # *DOCUMENTATION*
diff --git a/drivers/media/IR/mceusb.c b/drivers/media/IR/mceusb.c
index 756f718..640e2e6 100644
--- a/drivers/media/IR/mceusb.c
+++ b/drivers/media/IR/mceusb.c
@@ -68,7 +68,7 @@
 #define MCE_PULSE_BIT	0x80 /* Pulse bit, MSB set == PULSE else SPACE */
 #define MCE_PULSE_MASK	0x7F /* Pulse mask */
 #define MCE_MAX_PULSE_LENGTH 0x7F /* Longest transmittable pulse symbol */
-#define MCE_PACKET_LENGTH_MASK  0xF /* Packet length mask */
+#define MCE_PACKET_LENGTH_MASK  0x1F /* Packet length mask */
 
 
 /* module parameters */
@@ -209,11 +209,6 @@ static struct usb_device_id gen3_list[] = {
 	{}
 };
 
-static struct usb_device_id pinnacle_list[] = {
-	{ USB_DEVICE(VENDOR_PINNACLE, 0x0225) },
-	{}
-};
-
 static struct usb_device_id microsoft_gen1_list[] = {
 	{ USB_DEVICE(VENDOR_MICROSOFT, 0x006d) },
 	{}
@@ -542,6 +537,7 @@ static void mceusb_process_ir_data(struct mceusb_dev *ir, int buf_len)
 {
 	struct ir_raw_event rawir = { .pulse = false, .duration = 0 };
 	int i, start_index = 0;
+	u8 hdr = MCE_CONTROL_HEADER;
 
 	/* skip meaningless 0xb1 0x60 header bytes on orig receiver */
 	if (ir->flags.microsoft_gen1)
@@ -551,15 +547,16 @@ static void mceusb_process_ir_data(struct mceusb_dev *ir, int buf_len)
 		if (ir->rem == 0) {
 			/* decode mce packets of the form (84),AA,BB,CC,DD */
 			/* IR data packets can span USB messages - rem */
-			ir->rem = (ir->buf_in[i] & MCE_PACKET_LENGTH_MASK);
-			ir->cmd = (ir->buf_in[i] & ~MCE_PACKET_LENGTH_MASK);
+			hdr = ir->buf_in[i];
+			ir->rem = (hdr & MCE_PACKET_LENGTH_MASK);
+			ir->cmd = (hdr & ~MCE_PACKET_LENGTH_MASK);
 			dev_dbg(ir->dev, "New data. rem: 0x%02x, cmd: 0x%02x\n",
 				ir->rem, ir->cmd);
 			i++;
 		}
 
-		/* Only cmd 0x8<bytes> is IR data, don't process MCE commands */
-		if (ir->cmd != 0x80) {
+		/* don't process MCE commands */
+		if (hdr == MCE_CONTROL_HEADER || hdr == 0xff) {
 			ir->rem = 0;
 			return;
 		}
@@ -649,47 +646,18 @@ static void mceusb_gen1_init(struct mceusb_dev *ir)
 	int i, ret;
 	int partial = 0;
 	struct device *dev = ir->dev;
-	char *junk, *data;
-
-	junk = kmalloc(2 * USB_BUFLEN, GFP_KERNEL);
-	if (!junk) {
-		dev_err(dev, "%s: memory allocation failed!\n", __func__);
-		return;
-	}
+	char *data;
 
 	data = kzalloc(USB_CTRL_MSG_SZ, GFP_KERNEL);
 	if (!data) {
 		dev_err(dev, "%s: memory allocation failed!\n", __func__);
-		kfree(junk);
 		return;
 	}
 
 	/*
-	 * Clear off the first few messages. These look like calibration
-	 * or test data, I can't really tell. This also flushes in case
-	 * we have random ir data queued up.
-	 */
-	for (i = 0; i < MCE_G1_INIT_MSGS; i++)
-		usb_bulk_msg(ir->usbdev,
-			usb_rcvbulkpipe(ir->usbdev,
-				ir->usb_ep_in->bEndpointAddress),
-			junk, sizeof(junk), &partial, HZ * 10);
-
-	/* Get Status */
-	ret = usb_control_msg(ir->usbdev, usb_rcvctrlpipe(ir->usbdev, 0),
-			      USB_REQ_GET_STATUS, USB_DIR_IN,
-			      0, 0, data, USB_CTRL_MSG_SZ, HZ * 3);
-
-	/*    ret = usb_get_status( ir->usbdev, 0, 0, data ); */
-	dev_dbg(dev, "%s - ret = %d status = 0x%x 0x%x\n", __func__,
-		ret, data[0], data[1]);
-
-	/*
-	 * This is a strange one. They issue a set address to the device
+	 * This is a strange one. Windows issues a set address to the device
 	 * on the receive control pipe and expect a certain value pair back
 	 */
-	memset(data, 0, sizeof(data));
-
 	ret = usb_control_msg(ir->usbdev, usb_rcvctrlpipe(ir->usbdev, 0),
 			      USB_REQ_SET_ADDRESS, USB_TYPE_VENDOR, 0, 0,
 			      data, USB_CTRL_MSG_SZ, HZ * 3);
@@ -717,16 +685,12 @@ static void mceusb_gen1_init(struct mceusb_dev *ir)
 	dev_dbg(dev, "%s - retC = %d\n", __func__, ret);
 
 	kfree(data);
-	kfree(junk);
 };
 
 static void mceusb_gen2_init(struct mceusb_dev *ir)
 {
 	int maxp = ir->len_in;
 
-	mce_sync_in(ir, NULL, maxp);
-	mce_sync_in(ir, NULL, maxp);
-
 	/* device reset */
 	mce_async_out(ir, DEVICE_RESET, sizeof(DEVICE_RESET));
 	mce_sync_in(ir, NULL, maxp);
@@ -744,8 +708,6 @@ static void mceusb_gen3_init(struct mceusb_dev *ir)
 {
 	int maxp = ir->len_in;
 
-	mce_sync_in(ir, NULL, maxp);
-
 	/* device reset */
 	mce_async_out(ir, DEVICE_RESET, sizeof(DEVICE_RESET));
 	mce_sync_in(ir, NULL, maxp);
@@ -841,24 +803,19 @@ static int __devinit mceusb_dev_probe(struct usb_interface *intf,
 	struct usb_endpoint_descriptor *ep_out = NULL;
 	struct usb_host_config *config;
 	struct mceusb_dev *ir = NULL;
-	int pipe, maxp;
-	int i, ret;
+	int pipe, maxp, i;
 	char buf[63], name[128] = "";
 	bool is_gen3;
 	bool is_microsoft_gen1;
-	bool is_pinnacle;
 	bool tx_mask_inverted;
 
 	dev_dbg(&intf->dev, ": %s called\n", __func__);
 
-	usb_reset_device(dev);
-
 	config = dev->actconfig;
 	idesc  = intf->cur_altsetting;
 
 	is_gen3 = usb_match_id(intf, gen3_list) ? 1 : 0;
 	is_microsoft_gen1 = usb_match_id(intf, microsoft_gen1_list) ? 1 : 0;
-	is_pinnacle = usb_match_id(intf, pinnacle_list) ? 1 : 0;
 	tx_mask_inverted = usb_match_id(intf, std_tx_mask_list) ? 0 : 1;
 
 	/* step through the endpoints to find first bulk in and out endpoint */
@@ -873,19 +830,11 @@ static int __devinit mceusb_dev_probe(struct usb_interface *intf,
 			|| ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
 			    == USB_ENDPOINT_XFER_INT))) {
 
-			dev_dbg(&intf->dev, ": acceptable inbound endpoint "
-				"found\n");
 			ep_in = ep;
 			ep_in->bmAttributes = USB_ENDPOINT_XFER_INT;
-			if (!is_pinnacle)
-				/*
-				 * Ideally, we'd use what the device offers up,
-				 * but that leads to non-functioning first and
-				 * second-gen devices, and many devices have an
-				 * invalid bInterval of 0. Pinnacle devices
-				 * don't work witha  bInterval of 1 though.
-				 */
-				ep_in->bInterval = 1;
+			ep_in->bInterval = 1;
+			dev_dbg(&intf->dev, ": acceptable inbound endpoint "
+				"found\n");
 		}
 
 		if ((ep_out == NULL)
@@ -896,19 +845,11 @@ static int __devinit mceusb_dev_probe(struct usb_interface *intf,
 			|| ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
 			    == USB_ENDPOINT_XFER_INT))) {
 
-			dev_dbg(&intf->dev, ": acceptable outbound endpoint "
-				"found\n");
 			ep_out = ep;
 			ep_out->bmAttributes = USB_ENDPOINT_XFER_INT;
-			if (!is_pinnacle)
-				/*
-				 * Ideally, we'd use what the device offers up,
-				 * but that leads to non-functioning first and
-				 * second-gen devices, and many devices have an
-				 * invalid bInterval of 0. Pinnacle devices
-				 * don't work witha  bInterval of 1 though.
-				 */
-				ep_out->bInterval = 1;
+			ep_out->bInterval = 1;
+			dev_dbg(&intf->dev, ": acceptable outbound endpoint "
+				"found\n");
 		}
 	}
 	if (ep_in == NULL) {
@@ -956,25 +897,16 @@ static int __devinit mceusb_dev_probe(struct usb_interface *intf,
 	if (!ir->idev)
 		goto input_dev_fail;
 
-	/* inbound data */
+	/* flush buffers on the device */
+	mce_sync_in(ir, NULL, maxp);
+	mce_sync_in(ir, NULL, maxp);
+
+	/* wire up inbound data handler */
 	usb_fill_int_urb(ir->urb_in, dev, pipe, ir->buf_in,
 		maxp, (usb_complete_t) mceusb_dev_recv, ir, ep_in->bInterval);
 	ir->urb_in->transfer_dma = ir->dma_in;
 	ir->urb_in->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
 
-	if (is_pinnacle) {
-		/*
-		 * I have no idea why but this reset seems to be crucial to
-		 * getting the device to do outbound IO correctly - without
-		 * this the device seems to hang, ignoring all input - although
-		 * IR signals are correctly sent from the device, no input is
-		 * interpreted by the device and the host never does the
-		 * completion routine
-		 */
-		ret = usb_reset_configuration(dev);
-		dev_info(&intf->dev, "usb reset config ret %x\n", ret);
-	}
-
 	/* initialize device */
 	if (ir->flags.gen3)
 		mceusb_gen3_init(ir);
-- 
1.7.1


-- 
Jarod Wilson
jarod@redhat.com


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

* Re: [PATCH v3] IR/mceusb: kill pinnacle-device-specific nonsense
  2010-07-03  4:02 ` [PATCH v3] " Jarod Wilson
@ 2010-07-03 22:17   ` Mauro Carvalho Chehab
  2010-07-04  0:41     ` [PATCH] " Jarod Wilson
  0 siblings, 1 reply; 5+ messages in thread
From: Mauro Carvalho Chehab @ 2010-07-03 22:17 UTC (permalink / raw)
  To: Jarod Wilson; +Cc: linux-media

Em 03-07-2010 01:02, Jarod Wilson escreveu:
> I have pinnacle hardware now. None of this pinnacle-specific crap is at
> all necessary (in fact, some of it needed to be removed to actually make
> it work). The only thing unique about this device is that it often
> transfers inbound data w/a header of 0x90, meaning 16 bytes of IR data
> following it, so I had to make adjustments for that, and now its working
> perfectly fine.
> 
> v2: stillborn
> 
> v3: remove completely unnecessary usb_reset_device() call that only served
> to piss off the pinnacle device regularly and unify/simplify some of the
> generation-specific device initialization code.
> 
> post-mortem: it seems the pinnacle hardware actually still gets pissed off
> from time to time, but I can (try) to fix that later (if possible). The
> patch is still quite helpful from a code reduction standpoint.
> 
> Signed-off-by: Jarod Wilson <jarod@redhat.com>

I've already applied a previous version of this patch:

http://git.linuxtv.org/v4l-dvb.git?a=commit;h=afd46362e573276e3fb0a44834ad320c97947233

Could you please rebase it against my tree?

> ---
>  Makefile                  |    2 +-
>  drivers/media/IR/mceusb.c |  110 +++++++++------------------------------------
>  2 files changed, 22 insertions(+), 90 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 6e39ec7..0417c74 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1,7 +1,7 @@
>  VERSION = 2
>  PATCHLEVEL = 6
>  SUBLEVEL = 35
> -EXTRAVERSION = -rc1
> +EXTRAVERSION = -rc1-ir

Please, don't patch the upstream Makefile ;)

Cheers,
Mauro.

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

* Re: [PATCH] IR/mceusb: kill pinnacle-device-specific nonsense
  2010-07-03 22:17   ` Mauro Carvalho Chehab
@ 2010-07-04  0:41     ` Jarod Wilson
  2010-07-04  1:42       ` [PATCH] IR/mceusb: unify and simplify different gen device init Jarod Wilson
  0 siblings, 1 reply; 5+ messages in thread
From: Jarod Wilson @ 2010-07-04  0:41 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: Jarod Wilson, linux-media@vger.kernel.org

On Saturday, July 3, 2010, Mauro Carvalho Chehab <mchehab@redhat.com> wrote:
> Em 03-07-2010 01:02, Jarod Wilson escreveu:
>> I have pinnacle hardware now. None of this pinnacle-specific crap is at
>> all necessary (in fact, some of it needed to be removed to actually make
>> it work). The only thing unique about this device is that it often
>> transfers inbound data w/a header of 0x90, meaning 16 bytes of IR data
>> following it, so I had to make adjustments for that, and now its working
>> perfectly fine.
>>
>> v2: stillborn
>>
>> v3: remove completely unnecessary usb_reset_device() call that only served
>> to piss off the pinnacle device regularly and unify/simplify some of the
>> generation-specific device initialization code.
>>
>> post-mortem: it seems the pinnacle hardware actually still gets pissed off
>> from time to time, but I can (try) to fix that later (if possible). The
>> patch is still quite helpful from a code reduction standpoint.
>>
>> Signed-off-by: Jarod Wilson <jarod@redhat.com>
>
> I've already applied a previous version of this patch:
>
> http://git.linuxtv.org/v4l-dvb.git?a=commit;h=afd46362e573276e3fb0a44834ad320c97947233
>
> Could you please rebase it against my tree?

D'oh, sure, will do, should be able to knock that out tonight.

>> ---
>>  Makefile                  |    2 +-
>>  drivers/media/IR/mceusb.c |  110 +++++++++------------------------------------
>>  2 files changed, 22 insertions(+), 90 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 6e39ec7..0417c74 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1,7 +1,7 @@
>>  VERSION = 2
>>  PATCHLEVEL = 6
>>  SUBLEVEL = 35
>> -EXTRAVERSION = -rc1
>> +EXTRAVERSION = -rc1-ir
>
> Please, don't patch the upstream Makefile ;)

Gah, I saw that and meant to remove it. The perils of rushing things
the night before going on vacation...

--jarod



-- 
Jarod Wilson
jarod@wilsonet.com

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

* [PATCH] IR/mceusb: unify and simplify different gen device init
  2010-07-04  0:41     ` [PATCH] " Jarod Wilson
@ 2010-07-04  1:42       ` Jarod Wilson
  0 siblings, 0 replies; 5+ messages in thread
From: Jarod Wilson @ 2010-07-04  1:42 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-media@vger.kernel.org

Started out as an effort to try to tackle the last remaining issue I'm
having with this damned pinnacle device getting wedged the first time
its plugged in after an indeterminate length of not being plugged in.
Didn't get that solved yet, but did streamline the init code a bit more
and remove some superfluous gunk. Nukes a completely unneeded call to
usb_device_init() and several lines of overly complex crap in the gen1
device init path.

Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/media/IR/mceusb.c |   47 ++++++--------------------------------------
 1 files changed, 7 insertions(+), 40 deletions(-)

diff --git a/drivers/media/IR/mceusb.c b/drivers/media/IR/mceusb.c
index aaa40d8..46de9bc 100644
--- a/drivers/media/IR/mceusb.c
+++ b/drivers/media/IR/mceusb.c
@@ -766,47 +766,18 @@ static void mceusb_gen1_init(struct mceusb_dev *ir)
 	int i, ret;
 	int partial = 0;
 	struct device *dev = ir->dev;
-	char *junk, *data;
-
-	junk = kmalloc(2 * USB_BUFLEN, GFP_KERNEL);
-	if (!junk) {
-		dev_err(dev, "%s: memory allocation failed!\n", __func__);
-		return;
-	}
+	char *data;
 
 	data = kzalloc(USB_CTRL_MSG_SZ, GFP_KERNEL);
 	if (!data) {
 		dev_err(dev, "%s: memory allocation failed!\n", __func__);
-		kfree(junk);
 		return;
 	}
 
 	/*
-	 * Clear off the first few messages. These look like calibration
-	 * or test data, I can't really tell. This also flushes in case
-	 * we have random ir data queued up.
-	 */
-	for (i = 0; i < MCE_G1_INIT_MSGS; i++)
-		usb_bulk_msg(ir->usbdev,
-			usb_rcvbulkpipe(ir->usbdev,
-				ir->usb_ep_in->bEndpointAddress),
-			junk, sizeof(junk), &partial, HZ * 10);
-
-	/* Get Status */
-	ret = usb_control_msg(ir->usbdev, usb_rcvctrlpipe(ir->usbdev, 0),
-			      USB_REQ_GET_STATUS, USB_DIR_IN,
-			      0, 0, data, USB_CTRL_MSG_SZ, HZ * 3);
-
-	/*    ret = usb_get_status( ir->usbdev, 0, 0, data ); */
-	dev_dbg(dev, "%s - ret = %d status = 0x%x 0x%x\n", __func__,
-		ret, data[0], data[1]);
-
-	/*
-	 * This is a strange one. They issue a set address to the device
+	 * This is a strange one. Windows issues a set address to the device
 	 * on the receive control pipe and expect a certain value pair back
 	 */
-	memset(data, 0, sizeof(data));
-
 	ret = usb_control_msg(ir->usbdev, usb_rcvctrlpipe(ir->usbdev, 0),
 			      USB_REQ_SET_ADDRESS, USB_TYPE_VENDOR, 0, 0,
 			      data, USB_CTRL_MSG_SZ, HZ * 3);
@@ -834,16 +805,12 @@ static void mceusb_gen1_init(struct mceusb_dev *ir)
 	dev_dbg(dev, "%s - retC = %d\n", __func__, ret);
 
 	kfree(data);
-	kfree(junk);
 };
 
 static void mceusb_gen2_init(struct mceusb_dev *ir)
 {
 	int maxp = ir->len_in;
 
-	mce_sync_in(ir, NULL, maxp);
-	mce_sync_in(ir, NULL, maxp);
-
 	/* device reset */
 	mce_async_out(ir, DEVICE_RESET, sizeof(DEVICE_RESET));
 	mce_sync_in(ir, NULL, maxp);
@@ -861,8 +828,6 @@ static void mceusb_gen3_init(struct mceusb_dev *ir)
 {
 	int maxp = ir->len_in;
 
-	mce_sync_in(ir, NULL, maxp);
-
 	/* device reset */
 	mce_async_out(ir, DEVICE_RESET, sizeof(DEVICE_RESET));
 	mce_sync_in(ir, NULL, maxp);
@@ -969,8 +934,6 @@ static int __devinit mceusb_dev_probe(struct usb_interface *intf,
 
 	dev_dbg(&intf->dev, ": %s called\n", __func__);
 
-	usb_reset_device(dev);
-
 	config = dev->actconfig;
 	idesc  = intf->cur_altsetting;
 
@@ -1057,7 +1020,11 @@ static int __devinit mceusb_dev_probe(struct usb_interface *intf,
 	if (!ir->idev)
 		goto input_dev_fail;
 
-	/* inbound data */
+	/* flush buffers on the device */
+	mce_sync_in(ir, NULL, maxp);
+	mce_sync_in(ir, NULL, maxp);
+
+	/* wire up inbound data handler */
 	usb_fill_int_urb(ir->urb_in, dev, pipe, ir->buf_in,
 		maxp, (usb_complete_t) mceusb_dev_recv, ir, ep_in->bInterval);
 	ir->urb_in->transfer_dma = ir->dma_in;
-- 
1.7.1


-- 
Jarod Wilson
jarod@redhat.com


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

end of thread, other threads:[~2010-07-04  1:42 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-16 20:10 [PATCH] IR/mceusb: kill pinnacle-device-specific nonsense Jarod Wilson
2010-07-03  4:02 ` [PATCH v3] " Jarod Wilson
2010-07-03 22:17   ` Mauro Carvalho Chehab
2010-07-04  0:41     ` [PATCH] " Jarod Wilson
2010-07-04  1:42       ` [PATCH] IR/mceusb: unify and simplify different gen device init Jarod Wilson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).