[PATCH 1/5] [media] redrat3: ensure whole packet is read

[PATCH 1/5] [media] redrat3: ensure whole packet is read

[Sean Young]

The length in the header excludes the header itself, so we're getting
spurious readings.

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/rc/redrat3.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/rc/redrat3.c b/drivers/media/rc/redrat3.c
index 0042367..ccd267f 100644
--- a/drivers/media/rc/redrat3.c
+++ b/drivers/media/rc/redrat3.c
@@ -663,7 +663,8 @@ static int redrat3_get_ir_data(struct redrat3_dev *rr3, unsigned len)
 		goto out;
 	}
 
-	if (rr3->bytes_read < be16_to_cpu(rr3->irdata.header.length))
+	if (rr3->bytes_read < be16_to_cpu(rr3->irdata.header.length) +
+						sizeof(struct redrat3_header))
 		/* we're still accumulating data */
 		return 0;
 
-- 
1.8.3.1

[PATCH 2/5] [media] rc: add feedback led trigger for rc keypresses

[Sean Young]

Many devices with an ir receiver also have a feedback led. Add the
led trigger to support this.

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/rc/rc-main.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 1cf382a..628ac2a 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -16,6 +16,7 @@
 #include <linux/spinlock.h>
 #include <linux/delay.h>
 #include <linux/input.h>
+#include <linux/leds.h>
 #include <linux/slab.h>
 #include <linux/device.h>
 #include <linux/module.h>
@@ -31,6 +32,7 @@
 /* Used to keep track of known keymaps */
 static LIST_HEAD(rc_map_list);
 static DEFINE_SPINLOCK(rc_map_lock);
+static struct led_trigger *led_feedback;
 
 static struct rc_map_list *seek_rc_map(const char *name)
 {
@@ -535,6 +537,7 @@ static void ir_do_keyup(struct rc_dev *dev, bool sync)
 
 	IR_dprintk(1, "keyup key 0x%04x\n", dev->last_keycode);
 	input_report_key(dev->input_dev, dev->last_keycode, 0);
+	led_trigger_event(led_feedback, LED_OFF);
 	if (sync)
 		input_sync(dev->input_dev);
 	dev->keypressed = false;
@@ -648,6 +651,7 @@ static void ir_do_keydown(struct rc_dev *dev, int scancode,
 		input_report_key(dev->input_dev, keycode, 1);
 	}
 
+	led_trigger_event(led_feedback, LED_FULL);
 	input_sync(dev->input_dev);
 }
 
@@ -1184,6 +1188,7 @@ static int __init rc_core_init(void)
 		return rc;
 	}
 
+	led_trigger_register_simple("rc-feedback", &led_feedback);
 	rc_map_register(&empty_map);
 
 	return 0;
@@ -1192,6 +1197,7 @@ static int __init rc_core_init(void)
 static void __exit rc_core_exit(void)
 {
 	class_unregister(&rc_class);
+	led_trigger_unregister_simple(led_feedback);
 	rc_map_unregister(&empty_map);
 }
 
-- 
1.8.3.1

[PATCH 3/5] [media] ttusbir: wire up rc feedback led

[Sean Young]

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/rc/ttusbir.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/rc/ttusbir.c b/drivers/media/rc/ttusbir.c
index 891762d..d8de205 100644
--- a/drivers/media/rc/ttusbir.c
+++ b/drivers/media/rc/ttusbir.c
@@ -302,6 +302,7 @@ static int ttusbir_probe(struct usb_interface *intf,
 						ttusbir_bulk_complete, tt);
 
 	tt->led.name = "ttusbir:green:power";
+	tt->led.default_trigger = "rc-feedback";
 	tt->led.brightness_set = ttusbir_brightness_set;
 	tt->led.brightness_get = ttusbir_brightness_get;
 	tt->is_led_on = tt->led_on = true;
-- 
1.8.3.1

[PATCH 4/5] [media] winbond: wire up rc feedback led

[Sean Young]

Note that with the rc-feedback trigger, the cir-rx trigger is now
redundant. The cir-tx trigger is not used by default; if this
functionality is desired then it should exist in rc-core, not in
a driver.

Also make sure that the led is suspended on suspend.

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/rc/Kconfig       |  1 -
 drivers/media/rc/winbond-cir.c | 38 ++++++--------------------------------
 2 files changed, 6 insertions(+), 33 deletions(-)

diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index 5a79c33..7fa6b22 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -248,7 +248,6 @@ config IR_WINBOND_CIR
 	depends on RC_CORE
 	select NEW_LEDS
 	select LEDS_CLASS
-	select LEDS_TRIGGERS
 	select BITREVERSE
 	---help---
 	   Say Y here if you want to use the IR remote functionality found
diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
index 87af2d3..98bd496 100644
--- a/drivers/media/rc/winbond-cir.c
+++ b/drivers/media/rc/winbond-cir.c
@@ -213,13 +213,11 @@ struct wbcir_data {
 
 	/* RX state */
 	enum wbcir_rxstate rxstate;
-	struct led_trigger *rxtrigger;
 	int carrier_report_enabled;
 	u32 pulse_duration;
 
 	/* TX state */
 	enum wbcir_txstate txstate;
-	struct led_trigger *txtrigger;
 	u32 txlen;
 	u32 txoff;
 	u32 *txbuf;
@@ -366,14 +364,11 @@ wbcir_idle_rx(struct rc_dev *dev, bool idle)
 {
 	struct wbcir_data *data = dev->priv;
 
-	if (!idle && data->rxstate == WBCIR_RXSTATE_INACTIVE) {
+	if (!idle && data->rxstate == WBCIR_RXSTATE_INACTIVE)
 		data->rxstate = WBCIR_RXSTATE_ACTIVE;
-		led_trigger_event(data->rxtrigger, LED_FULL);
-	}
 
 	if (idle && data->rxstate != WBCIR_RXSTATE_INACTIVE) {
 		data->rxstate = WBCIR_RXSTATE_INACTIVE;
-		led_trigger_event(data->rxtrigger, LED_OFF);
 
 		if (data->carrier_report_enabled)
 			wbcir_carrier_report(data);
@@ -425,7 +420,6 @@ wbcir_irq_tx(struct wbcir_data *data)
 	case WBCIR_TXSTATE_INACTIVE:
 		/* TX FIFO empty */
 		space = 16;
-		led_trigger_event(data->txtrigger, LED_FULL);
 		break;
 	case WBCIR_TXSTATE_ACTIVE:
 		/* TX FIFO low (3 bytes or less) */
@@ -464,7 +458,6 @@ wbcir_irq_tx(struct wbcir_data *data)
 			/* Clear TX underrun bit */
 			outb(WBCIR_TX_UNDERRUN, data->sbase + WBCIR_REG_SP3_ASCR);
 		wbcir_set_irqmask(data, WBCIR_IRQ_RX | WBCIR_IRQ_ERR);
-		led_trigger_event(data->txtrigger, LED_OFF);
 		kfree(data->txbuf);
 		data->txbuf = NULL;
 		data->txstate = WBCIR_TXSTATE_INACTIVE;
@@ -878,15 +871,13 @@ finish:
 	 */
 	wbcir_set_irqmask(data, WBCIR_IRQ_NONE);
 	disable_irq(data->irq);
-
-	/* Disable LED */
-	led_trigger_event(data->rxtrigger, LED_OFF);
-	led_trigger_event(data->txtrigger, LED_OFF);
 }
 
 static int
 wbcir_suspend(struct pnp_dev *device, pm_message_t state)
 {
+	struct wbcir_data *data = pnp_get_drvdata(device);
+	led_classdev_suspend(&data->led);
 	wbcir_shutdown(device);
 	return 0;
 }
@@ -1015,6 +1006,7 @@ wbcir_resume(struct pnp_dev *device)
 
 	wbcir_init_hw(data);
 	enable_irq(data->irq);
+	led_classdev_resume(&data->led);
 
 	return 0;
 }
@@ -1058,25 +1050,13 @@ wbcir_probe(struct pnp_dev *device, const struct pnp_device_id *dev_id)
 		"(w: 0x%lX, e: 0x%lX, s: 0x%lX, i: %u)\n",
 		data->wbase, data->ebase, data->sbase, data->irq);
 
-	led_trigger_register_simple("cir-tx", &data->txtrigger);
-	if (!data->txtrigger) {
-		err = -ENOMEM;
-		goto exit_free_data;
-	}
-
-	led_trigger_register_simple("cir-rx", &data->rxtrigger);
-	if (!data->rxtrigger) {
-		err = -ENOMEM;
-		goto exit_unregister_txtrigger;
-	}
-
 	data->led.name = "cir::activity";
-	data->led.default_trigger = "cir-rx";
+	data->led.default_trigger = "rc-feedback";
 	data->led.brightness_set = wbcir_led_brightness_set;
 	data->led.brightness_get = wbcir_led_brightness_get;
 	err = led_classdev_register(&device->dev, &data->led);
 	if (err)
-		goto exit_unregister_rxtrigger;
+		goto exit_free_data;
 
 	data->dev = rc_allocate_device();
 	if (!data->dev) {
@@ -1156,10 +1136,6 @@ exit_free_rc:
 	rc_free_device(data->dev);
 exit_unregister_led:
 	led_classdev_unregister(&data->led);
-exit_unregister_rxtrigger:
-	led_trigger_unregister_simple(data->rxtrigger);
-exit_unregister_txtrigger:
-	led_trigger_unregister_simple(data->txtrigger);
 exit_free_data:
 	kfree(data);
 	pnp_set_drvdata(device, NULL);
@@ -1187,8 +1163,6 @@ wbcir_remove(struct pnp_dev *device)
 
 	rc_unregister_device(data->dev);
 
-	led_trigger_unregister_simple(data->rxtrigger);
-	led_trigger_unregister_simple(data->txtrigger);
 	led_classdev_unregister(&data->led);
 
 	/* This is ok since &data->led isn't actually used */
-- 
1.8.3.1

[PATCH 5/5] [media] redrat3: wire up rc feedback led

[Sean Young]

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/rc/Kconfig   |  2 ++
 drivers/media/rc/redrat3.c | 83 ++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 82 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index 7fa6b22..11e84bc 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -223,6 +223,8 @@ config IR_REDRAT3
 	tristate "RedRat3 IR Transceiver"
 	depends on USB_ARCH_HAS_HCD
 	depends on RC_CORE
+	select NEW_LEDS
+	select LEDS_CLASS
 	select USB
 	---help---
 	   Say Y here if you want to use a RedRat3 Infrared Transceiver.
diff --git a/drivers/media/rc/redrat3.c b/drivers/media/rc/redrat3.c
index ccd267f..094484f 100644
--- a/drivers/media/rc/redrat3.c
+++ b/drivers/media/rc/redrat3.c
@@ -47,6 +47,7 @@
 
 #include <asm/unaligned.h>
 #include <linux/device.h>
+#include <linux/leds.h>
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/usb.h>
@@ -186,6 +187,13 @@ struct redrat3_dev {
 	struct rc_dev *rc;
 	struct device *dev;
 
+	/* led control */
+	struct led_classdev led;
+	atomic_t flash;
+	struct usb_ctrlrequest flash_control;
+	struct urb *flash_urb;
+	u8 flash_in_buf;
+
 	/* save off the usb device pointer */
 	struct usb_device *udev;
 
@@ -480,9 +488,9 @@ static inline void redrat3_delete(struct redrat3_dev *rr3,
 {
 	rr3_ftr(rr3->dev, "%s cleaning up\n", __func__);
 	usb_kill_urb(rr3->read_urb);
-
+	usb_kill_urb(rr3->flash_urb);
 	usb_free_urb(rr3->read_urb);
-
+	usb_free_urb(rr3->flash_urb);
 	usb_free_coherent(udev, le16_to_cpu(rr3->ep_in->wMaxPacketSize),
 			  rr3->bulk_in_buf, rr3->dma_in);
 
@@ -850,6 +858,44 @@ out:
 	return ret;
 }
 
+static void redrat3_brightness_set(struct led_classdev *led_dev, enum
+						led_brightness brightness)
+{
+	struct redrat3_dev *rr3 = container_of(led_dev, struct redrat3_dev,
+									led);
+
+	if (brightness != LED_OFF && atomic_cmpxchg(&rr3->flash, 0, 1) == 0) {
+		int ret = usb_submit_urb(rr3->flash_urb, GFP_ATOMIC);
+		if (ret != 0) {
+			dev_dbg(rr3->dev, "%s: unexpected ret of %d\n",
+				__func__, ret);
+			atomic_set(&rr3->flash, 0);
+		}
+	}
+}
+
+static void redrat3_led_complete(struct urb *urb)
+{
+	struct redrat3_dev *rr3 = urb->context;
+
+	switch (urb->status) {
+	case 0:
+		break;
+	case -ECONNRESET:
+	case -ENOENT:
+	case -ESHUTDOWN:
+		usb_unlink_urb(urb);
+		return;
+	case -EPIPE:
+	default:
+		dev_dbg(rr3->dev, "Error: urb status = %d\n", urb->status);
+		break;
+	}
+
+	rr3->led.brightness = LED_OFF;
+	atomic_dec(&rr3->flash);
+}
+
 static struct rc_dev *redrat3_init_rc_dev(struct redrat3_dev *rr3)
 {
 	struct device *dev = rr3->dev;
@@ -993,10 +1039,35 @@ static int redrat3_dev_probe(struct usb_interface *intf,
 	/* default.. will get overridden by any sends with a freq defined */
 	rr3->carrier = 38000;
 
+	/* led control */
+	rr3->led.name = "redrat3:red:feedback";
+	rr3->led.default_trigger = "rc-feedback";
+	rr3->led.brightness_set = redrat3_brightness_set;
+	retval = led_classdev_register(&intf->dev, &rr3->led);
+	if (retval)
+		goto error;
+
+	atomic_set(&rr3->flash, 0);
+	rr3->flash_urb = usb_alloc_urb(0, GFP_KERNEL);
+	if (!rr3->flash_urb) {
+		retval = -ENOMEM;
+		goto led_free_error;
+	}
+
+	/* setup packet is 'c0 b9 0000 0000 0001' */
+	rr3->flash_control.bRequestType = 0xc0;
+	rr3->flash_control.bRequest = RR3_BLINK_LED;
+	rr3->flash_control.wLength = cpu_to_le16(1);
+
+	usb_fill_control_urb(rr3->flash_urb, udev, usb_rcvctrlpipe(udev, 0),
+			(unsigned char *)&rr3->flash_control,
+			&rr3->flash_in_buf, sizeof(rr3->flash_in_buf),
+			redrat3_led_complete, rr3);
+
 	rr3->rc = redrat3_init_rc_dev(rr3);
 	if (!rr3->rc) {
 		retval = -ENOMEM;
-		goto error;
+		goto led_free_error;
 	}
 	setup_timer(&rr3->rx_timeout, redrat3_rx_timeout, (unsigned long)rr3);
 
@@ -1006,6 +1077,8 @@ static int redrat3_dev_probe(struct usb_interface *intf,
 	rr3_ftr(dev, "Exiting %s\n", __func__);
 	return 0;
 
+led_free_error:
+	led_classdev_unregister(&rr3->led);
 error:
 	redrat3_delete(rr3, rr3->udev);
 
@@ -1027,6 +1100,7 @@ static void redrat3_dev_disconnect(struct usb_interface *intf)
 
 	usb_set_intfdata(intf, NULL);
 	rc_unregister_device(rr3->rc);
+	led_classdev_unregister(&rr3->led);
 	del_timer_sync(&rr3->rx_timeout);
 	redrat3_delete(rr3, udev);
 
@@ -1037,7 +1111,9 @@ static int redrat3_dev_suspend(struct usb_interface *intf, pm_message_t message)
 {
 	struct redrat3_dev *rr3 = usb_get_intfdata(intf);
 	rr3_ftr(rr3->dev, "suspend\n");
+	led_classdev_suspend(&rr3->led);
 	usb_kill_urb(rr3->read_urb);
+	usb_kill_urb(rr3->flash_urb);
 	return 0;
 }
 
@@ -1047,6 +1123,7 @@ static int redrat3_dev_resume(struct usb_interface *intf)
 	rr3_ftr(rr3->dev, "resume\n");
 	if (usb_submit_urb(rr3->read_urb, GFP_ATOMIC))
 		return -EIO;
+	led_classdev_resume(&rr3->led);
 	return 0;
 }
 
-- 
1.8.3.1

Re: [PATCH 4/5] [media] winbond: wire up rc feedback led

[David Härdeman]

On Wed, Jul 31, 2013 at 12:00:03AM +0100, Sean Young wrote:
>Note that with the rc-feedback trigger, the cir-rx trigger is now
>redundant. The cir-tx trigger is not used by default; if this
>functionality is desired then it should exist in rc-core, not in
>a driver.
>
>Also make sure that the led is suspended on suspend.
>
>Signed-off-by: Sean Young <sean@mess.org>
Signed-off-by: David Härdeman <david@hardeman.nu>

>---
> drivers/media/rc/Kconfig       |  1 -
> drivers/media/rc/winbond-cir.c | 38 ++++++--------------------------------
> 2 files changed, 6 insertions(+), 33 deletions(-)
>
>diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
>index 5a79c33..7fa6b22 100644
>--- a/drivers/media/rc/Kconfig
>+++ b/drivers/media/rc/Kconfig
>@@ -248,7 +248,6 @@ config IR_WINBOND_CIR
> 	depends on RC_CORE
> 	select NEW_LEDS
> 	select LEDS_CLASS
>-	select LEDS_TRIGGERS
> 	select BITREVERSE
> 	---help---
> 	   Say Y here if you want to use the IR remote functionality found
>diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
>index 87af2d3..98bd496 100644
>--- a/drivers/media/rc/winbond-cir.c
>+++ b/drivers/media/rc/winbond-cir.c
>@@ -213,13 +213,11 @@ struct wbcir_data {
> 
> 	/* RX state */
> 	enum wbcir_rxstate rxstate;
>-	struct led_trigger *rxtrigger;
> 	int carrier_report_enabled;
> 	u32 pulse_duration;
> 
> 	/* TX state */
> 	enum wbcir_txstate txstate;
>-	struct led_trigger *txtrigger;
> 	u32 txlen;
> 	u32 txoff;
> 	u32 *txbuf;
>@@ -366,14 +364,11 @@ wbcir_idle_rx(struct rc_dev *dev, bool idle)
> {
> 	struct wbcir_data *data = dev->priv;
> 
>-	if (!idle && data->rxstate == WBCIR_RXSTATE_INACTIVE) {
>+	if (!idle && data->rxstate == WBCIR_RXSTATE_INACTIVE)
> 		data->rxstate = WBCIR_RXSTATE_ACTIVE;
>-		led_trigger_event(data->rxtrigger, LED_FULL);
>-	}
> 
> 	if (idle && data->rxstate != WBCIR_RXSTATE_INACTIVE) {
> 		data->rxstate = WBCIR_RXSTATE_INACTIVE;
>-		led_trigger_event(data->rxtrigger, LED_OFF);
> 
> 		if (data->carrier_report_enabled)
> 			wbcir_carrier_report(data);
>@@ -425,7 +420,6 @@ wbcir_irq_tx(struct wbcir_data *data)
> 	case WBCIR_TXSTATE_INACTIVE:
> 		/* TX FIFO empty */
> 		space = 16;
>-		led_trigger_event(data->txtrigger, LED_FULL);
> 		break;
> 	case WBCIR_TXSTATE_ACTIVE:
> 		/* TX FIFO low (3 bytes or less) */
>@@ -464,7 +458,6 @@ wbcir_irq_tx(struct wbcir_data *data)
> 			/* Clear TX underrun bit */
> 			outb(WBCIR_TX_UNDERRUN, data->sbase + WBCIR_REG_SP3_ASCR);
> 		wbcir_set_irqmask(data, WBCIR_IRQ_RX | WBCIR_IRQ_ERR);
>-		led_trigger_event(data->txtrigger, LED_OFF);
> 		kfree(data->txbuf);
> 		data->txbuf = NULL;
> 		data->txstate = WBCIR_TXSTATE_INACTIVE;
>@@ -878,15 +871,13 @@ finish:
> 	 */
> 	wbcir_set_irqmask(data, WBCIR_IRQ_NONE);
> 	disable_irq(data->irq);
>-
>-	/* Disable LED */
>-	led_trigger_event(data->rxtrigger, LED_OFF);
>-	led_trigger_event(data->txtrigger, LED_OFF);
> }
> 
> static int
> wbcir_suspend(struct pnp_dev *device, pm_message_t state)
> {
>+	struct wbcir_data *data = pnp_get_drvdata(device);
>+	led_classdev_suspend(&data->led);
> 	wbcir_shutdown(device);
> 	return 0;
> }
>@@ -1015,6 +1006,7 @@ wbcir_resume(struct pnp_dev *device)
> 
> 	wbcir_init_hw(data);
> 	enable_irq(data->irq);
>+	led_classdev_resume(&data->led);
> 
> 	return 0;
> }
>@@ -1058,25 +1050,13 @@ wbcir_probe(struct pnp_dev *device, const struct pnp_device_id *dev_id)
> 		"(w: 0x%lX, e: 0x%lX, s: 0x%lX, i: %u)\n",
> 		data->wbase, data->ebase, data->sbase, data->irq);
> 
>-	led_trigger_register_simple("cir-tx", &data->txtrigger);
>-	if (!data->txtrigger) {
>-		err = -ENOMEM;
>-		goto exit_free_data;
>-	}
>-
>-	led_trigger_register_simple("cir-rx", &data->rxtrigger);
>-	if (!data->rxtrigger) {
>-		err = -ENOMEM;
>-		goto exit_unregister_txtrigger;
>-	}
>-
> 	data->led.name = "cir::activity";
>-	data->led.default_trigger = "cir-rx";
>+	data->led.default_trigger = "rc-feedback";
> 	data->led.brightness_set = wbcir_led_brightness_set;
> 	data->led.brightness_get = wbcir_led_brightness_get;
> 	err = led_classdev_register(&device->dev, &data->led);
> 	if (err)
>-		goto exit_unregister_rxtrigger;
>+		goto exit_free_data;
> 
> 	data->dev = rc_allocate_device();
> 	if (!data->dev) {
>@@ -1156,10 +1136,6 @@ exit_free_rc:
> 	rc_free_device(data->dev);
> exit_unregister_led:
> 	led_classdev_unregister(&data->led);
>-exit_unregister_rxtrigger:
>-	led_trigger_unregister_simple(data->rxtrigger);
>-exit_unregister_txtrigger:
>-	led_trigger_unregister_simple(data->txtrigger);
> exit_free_data:
> 	kfree(data);
> 	pnp_set_drvdata(device, NULL);
>@@ -1187,8 +1163,6 @@ wbcir_remove(struct pnp_dev *device)
> 
> 	rc_unregister_device(data->dev);
> 
>-	led_trigger_unregister_simple(data->rxtrigger);
>-	led_trigger_unregister_simple(data->txtrigger);
> 	led_classdev_unregister(&data->led);
> 
> 	/* This is ok since &data->led isn't actually used */
>-- 
>1.8.3.1
>

-- 
David Härdeman