Re: [PATCH] [media] mceusb: add IR learning support features (IR carrier frequency measurement and wide-band/short range receiver)

[Sean Young]

Hi,

On Sun, Mar 11, 2018 at 05:40:28AM -0400, A Sun wrote:
> 
> Windows Media Center IR transceivers include two IR receivers;
> wide-band/short-range and narrow-band/long-range. The short-range
> (5cm distance) receiver is for IR learning and has IR carrier
> frequency measuring ability.
> 
> Add mceusb driver support to select the short range IR receiver
> and enable pass through of its IR carrier frequency measurements.
> 
> RC and LIRC already support these mceusb driver additions.

That's great, this feature has been missing for a long time. 

I've tested it with my four mceusb devices, and I get carrier reports with
all of them. Please see the notes below.

> Test platform:
> 
> Linux raspberrypi 4.9.59-v7+ #1047 SMP Sun Oct 29 12:19:23 GMT 2017 armv7l 
> GNU/Linux
> mceusb 1-1.2:1.0: Registered Pinnacle Systems PCTV Remote USB with mce 
> emulator interface version 1
> mceusb 1-1.2:1.0: 2 tx ports (0x0 cabled) and 2 rx sensors (0x1 active)
> 
> Sony TV remote control
> 
> ir-ctl from v4l-utils
> 
> pi@raspberrypi:~ $ ir-ctl -V
> IR raw version 1.12.3
> pi@raspberrypi:~ $ ir-ctl -w -m -d /dev/lirc0 -r
> ...
> pulse 600
> space 600
> pulse 1250
> space 550
> pulse 650
> space 600
> pulse 550
> space 600
> pulse 600
> space 600
> pulse 650
> carrier 38803

Sony protocol remotes have a 4Hz carrier, and I am getting lower values
for the carrier with other carrier frequencies as well. Any idea why?

> space 16777215
> ^C
> pi@raspberrypi:~ $ exit
> 
> Signed-off-by: A Sun 
> ---
>  drivers/media/rc/mceusb.c | 90 
> ++-
>  1 file changed, 82 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
> index a9187b0b4..8bbb0f2da 100644
> --- a/drivers/media/rc/mceusb.c
> +++ b/drivers/media/rc/mceusb.c
> @@ -42,7 +42,7 @@
>  #include 
>  #include 
>  
> -#define DRIVER_VERSION   "1.93"
> +#define DRIVER_VERSION   "1.94"
>  #define DRIVER_AUTHOR"Jarod Wilson "
>  #define DRIVER_DESC  "Windows Media Center Ed. eHome Infrared Transceiver " \
>   "device driver"
> @@ -427,6 +427,7 @@ struct mceusb_dev {
>   struct rc_dev *rc;
>  
>   /* optional features we can enable */
> + bool carrier_report_enabled;
>   bool learning_enabled;
>  
>   /* core device bits */
> @@ -475,6 +476,9 @@ struct mceusb_dev {
>   u8 txports_cabled;  /* bitmask of transmitters with cable */
>   u8 rxports_active;  /* bitmask of active receive sensors */
>  
> + /* receiver carrier frequency detection support */
> + u32 pulse_tunit;/* IR pulse "on" cumulative time units */
> +
>   /*
>* support for async error handler mceusb_deferred_kevent()
>* where usb_clear_halt(), usb_reset_configuration(),
> @@ -956,12 +960,60 @@ static int mceusb_set_tx_carrier(struct rc_dev *dev, 
> u32 carrier)
>  }
>  
>  /*
> + * Select or deselect the 2nd receiver port.
> + * Second receiver is learning mode, wide-band, short-range receiver.
> + * Only one receiver (long or short range) may be active at a time.
> + */
> +static int mceusb_set_rx_wideband(struct rc_dev *dev, int enable)
> +{
> + struct mceusb_dev *ir = dev->priv;
> + unsigned char cmdbuf[3] = { MCE_CMD_PORT_IR,
> + MCE_CMD_SETIRRXPORTEN, 0x00 };
> +
> + if (enable != 0 && enable != 1)
> + return -EINVAL;
> +
> + /*
> +  * cmdbuf[2] is receiver port number
> +  * port 1 is long range receiver
> +  * port 2 is short range receiver
> +  */
> + cmdbuf[2] = enable + 1;

You could do enable ? 2 : 1 here and do away with the check above. Enable
always is 0 or 1 anyway.

> + dev_dbg(ir->dev, "select %s-range receive sensor",
> + enable ? "short" : "long");
> + mce_async_out(ir, cmdbuf, sizeof(cmdbuf));
> +
> + return 0;
> +}
> +
> +/*
> + * Enable/disable receiver carrier frequency pass through reporting.
> + * Frequency measurement only works with the short-range receiver.
> + * The long-range receiver always reports no carrier frequency
> + * (MCE_RSP_EQIRRXCFCNT, 0, 0) so we always ignore its report.
> + */
> +static int mceusb_set_rx_carrier_report(struct rc_dev *dev, int enable)
> +{
> + struct mceusb_dev *ir = dev->priv;
> +
> + if (enable != 0 && enable != 1)
> + return -EINVAL;

This is only called from lirc_dev.c, where the expression is:

ret = dev->s_carrier_report(dev, !!val);

There is no need to check for enable being not 0 or 1.

> +
> + dev_dbg(ir->dev, "%s short-range receiver carrier reporting",
> + enable ? "enable" : "disable");
> + ir->carrier_report_enabled = (enable == 1);

Since enable is 0 or 1, there is no need for the enable == 1 expression.

Note that the other drivers that support carrier reports (winbond-cir,
redrat3, ene-cir) all enable the wideband receiver when carrier reports
are 

[PATCH] [media] mceusb: add IR learning support features (IR carrier frequency measurement and wide-band/short range receiver)

[A Sun]

Windows Media Center IR transceivers include two IR receivers;
wide-band/short-range and narrow-band/long-range. The short-range
(5cm distance) receiver is for IR learning and has IR carrier
frequency measuring ability.

Add mceusb driver support to select the short range IR receiver
and enable pass through of its IR carrier frequency measurements.

RC and LIRC already support these mceusb driver additions.

Test platform:

Linux raspberrypi 4.9.59-v7+ #1047 SMP Sun Oct 29 12:19:23 GMT 2017 armv7l 
GNU/Linux
mceusb 1-1.2:1.0: Registered Pinnacle Systems PCTV Remote USB with mce emulator 
interface version 1
mceusb 1-1.2:1.0: 2 tx ports (0x0 cabled) and 2 rx sensors (0x1 active)

Sony TV remote control

ir-ctl from v4l-utils

pi@raspberrypi:~ $ ir-ctl -V
IR raw version 1.12.3
pi@raspberrypi:~ $ ir-ctl -w -m -d /dev/lirc0 -r
...
pulse 600
space 600
pulse 1250
space 550
pulse 650
space 600
pulse 550
space 600
pulse 600
space 600
pulse 650
carrier 38803
space 16777215
^C
pi@raspberrypi:~ $ exit

Signed-off-by: A Sun 
---
 drivers/media/rc/mceusb.c | 90 ++-
 1 file changed, 82 insertions(+), 8 deletions(-)

diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index a9187b0b4..8bbb0f2da 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -42,7 +42,7 @@
 #include 
 #include 
 
-#define DRIVER_VERSION "1.93"
+#define DRIVER_VERSION "1.94"
 #define DRIVER_AUTHOR  "Jarod Wilson "
 #define DRIVER_DESC"Windows Media Center Ed. eHome Infrared Transceiver " \
"device driver"
@@ -427,6 +427,7 @@ struct mceusb_dev {
struct rc_dev *rc;
 
/* optional features we can enable */
+   bool carrier_report_enabled;
bool learning_enabled;
 
/* core device bits */
@@ -475,6 +476,9 @@ struct mceusb_dev {
u8 txports_cabled;  /* bitmask of transmitters with cable */
u8 rxports_active;  /* bitmask of active receive sensors */
 
+   /* receiver carrier frequency detection support */
+   u32 pulse_tunit;/* IR pulse "on" cumulative time units */
+
/*
 * support for async error handler mceusb_deferred_kevent()
 * where usb_clear_halt(), usb_reset_configuration(),
@@ -956,12 +960,60 @@ static int mceusb_set_tx_carrier(struct rc_dev *dev, u32 
carrier)
 }
 
 /*
+ * Select or deselect the 2nd receiver port.
+ * Second receiver is learning mode, wide-band, short-range receiver.
+ * Only one receiver (long or short range) may be active at a time.
+ */
+static int mceusb_set_rx_wideband(struct rc_dev *dev, int enable)
+{
+   struct mceusb_dev *ir = dev->priv;
+   unsigned char cmdbuf[3] = { MCE_CMD_PORT_IR,
+   MCE_CMD_SETIRRXPORTEN, 0x00 };
+
+   if (enable != 0 && enable != 1)
+   return -EINVAL;
+
+   /*
+* cmdbuf[2] is receiver port number
+* port 1 is long range receiver
+* port 2 is short range receiver
+*/
+   cmdbuf[2] = enable + 1;
+   dev_dbg(ir->dev, "select %s-range receive sensor",
+   enable ? "short" : "long");
+   mce_async_out(ir, cmdbuf, sizeof(cmdbuf));
+
+   return 0;
+}
+
+/*
+ * Enable/disable receiver carrier frequency pass through reporting.
+ * Frequency measurement only works with the short-range receiver.
+ * The long-range receiver always reports no carrier frequency
+ * (MCE_RSP_EQIRRXCFCNT, 0, 0) so we always ignore its report.
+ */
+static int mceusb_set_rx_carrier_report(struct rc_dev *dev, int enable)
+{
+   struct mceusb_dev *ir = dev->priv;
+
+   if (enable != 0 && enable != 1)
+   return -EINVAL;
+
+   dev_dbg(ir->dev, "%s short-range receiver carrier reporting",
+   enable ? "enable" : "disable");
+   ir->carrier_report_enabled = (enable == 1);
+
+   return 0;
+}
+
+/*
  * We don't do anything but print debug spew for many of the command bits
  * we receive from the hardware, but some of them are useful information
  * we want to store so that we can use them.
  */
 static void mceusb_handle_command(struct mceusb_dev *ir, int index)
 {
+   DEFINE_IR_RAW_EVENT(rawir);
u8 hi = ir->buf_in[index + 1] & 0xff;
u8 lo = ir->buf_in[index + 2] & 0xff;
 
@@ -980,6 +1032,18 @@ static void mceusb_handle_command(struct mceusb_dev *ir, 
int index)
ir->num_txports = hi;
ir->num_rxports = lo;
break;
+   case MCE_RSP_EQIRRXCFCNT:
+   if (ir->carrier_report_enabled && ir->learning_enabled
+   && ir->pulse_tunit > 0) {
+   init_ir_raw_event();
+   rawir.carrier_report = 1;
+   rawir.carrier = (100u / MCE_TIME_UNIT) *
+   (hi << 8 | lo) / ir->pulse_tunit;
+   dev_dbg(ir->dev, "RX carrier frequency %u Hz (pulse