mceusb and related fixups
Subsystems (3)
cover mceusb and related fixups
Just a few mceusb-centric patches to fix some minor issues.
Jarod Wilson (4):
mceusb: fix inverted mask inversion logic
mceusb: set a default rx timeout
rc: conversion is to microseconds, not nanoseconds
mceusb: add another Fintek device ID
drivers/media/rc/ene_ir.c | 16 ++++++++-------- drivers/media/rc/ene_ir.h | 2 -- drivers/media/rc/mceusb.c | 30 ++++++++++++++++-------------- include/media/rc-core.h | 1 + 4 files changed, 25 insertions(+), 24 deletions(-)
PATCH 1/4 mceusb: fix inverted mask inversion logic
As it turns out, somewhere along the way, we managed to invert the meaning of the tx_mask_inverted flag. Looking back over the old lirc driver, tx_mask_inverted was set to 0 if the device was in tx_mask_list. Now we have a tx_mask_inverted flag set to 1 for all the devices that were in the list, and set tx_mask_inverted to that flag value, which is actually the opposite of what we used to set, causing set_tx_mask to use the wrong mask setting option. Since there seem to be more devices with inverted masks than not (using the original device as the baseline for inverted vs. normal), lets just call the ones currently marked as inverted normal instead, and flip the if/else actions that key off of the inverted flag.
Note: the problem only cropped up if a call to set_tx_mask was made, if no mask was set, the device would work just fine, which is why this managed to slip though w/o getting noticed until now.
Tested successfully by myself and Dennis Gilmore.
--- drivers/media/rc/mceusb.c | 22 +++++++++++----------- 1 files changed, 11 insertions(+), 11 deletions(-)
drivers/media/rc/mceusb.c +11 -11
@@ -155,7 +155,7 @@ struct mceusb_model { u32 mce_gen1:1; u32 mce_gen2:1; u32 mce_gen3:1; - u32 tx_mask_inverted:1; + u32 tx_mask_normal:1; u32 is_polaris:1; u32 no_tx:1;
@@ -166,18 +166,18 @@ struct mceusb_model {static const struct mceusb_model mceusb_model[] = { [MCE_GEN1 ] = { .mce_gen1 =1 , - .tx_mask_inverted =1 , + .tx_mask_normal =1 , }, [MCE_GEN2 ] = { .mce_gen2 =1 , }, [MCE_GEN2_TX_INV ] = { .mce_gen2 =1 , - .tx_mask_inverted =1 , + .tx_mask_normal =1 , }, [MCE_GEN3 ] = { .mce_gen3 =1 , - .tx_mask_inverted =1 , + .tx_mask_normal =1 , }, [POLARIS_EVK ] = { .is_polaris =1 ,
@@ -346,7 +346,7 @@ struct mceusb_dev {struct {u32 connected:1 ; -u32 tx_mask_inverted:1 ; +u32 tx_mask_normal:1 ;u32 microsoft_gen1:1 ;u32 no_tx:1 ; } flags;
@@ -749,11 +749,11 @@ static int mceusb_set_tx_mask(struct rc_dev *dev, u32 mask) {struct mceusb_dev *ir = dev->priv; -if (ir->flags.tx_mask_inverted) +if (ir->flags.tx_mask_normal) + ir->tx_mask = mask; +else ir->tx_mask = (mask !=MCE_DEFAULT_TX_MASK ? mask ^MCE_DEFAULT_TX_MASK : mask) <<1 ; -else - ir->tx_mask = mask;return 0 ; }
@@ -1095,7 +1095,7 @@ static int __devinit mceusb_dev_probe(struct usb_interface *intf,enum mceusb_model_type model = id->driver_info;bool is_gen3;bool is_microsoft_gen1; -bool tx_mask_inverted; +bool tx_mask_normal;bool is_polaris;dev_dbg (&intf->dev,"%s called\n" , __func__);
@@ -1104,7 +1104,7 @@ static int __devinit mceusb_dev_probe(struct usb_interface *intf, is_gen3 = mceusb_model[model].mce_gen3; is_microsoft_gen1 = mceusb_model[model].mce_gen1; - tx_mask_inverted = mceusb_model[model].tx_mask_inverted; + tx_mask_normal = mceusb_model[model].tx_mask_normal; is_polaris = mceusb_model[model].is_polaris;if (is_polaris) {
@@ -1171,7 +1171,7 @@ static int __devinit mceusb_dev_probe(struct usb_interface *intf, ir->dev = &intf->dev; ir->len_in = maxp; ir->flags.microsoft_gen1 = is_microsoft_gen1; - ir->flags.tx_mask_inverted = tx_mask_inverted; + ir->flags.tx_mask_normal = tx_mask_normal; ir->flags.no_tx = mceusb_model[model].no_tx; ir->model = model;
Signature · 1 line
1.7.1
PATCH 2/4 mceusb: set a default rx timeout
In reply to Jarod WilsonIts possible for the call to read rx timeout from the hardware to fail, in which case we end up with a bogus rx timeout value. Set a default one when filling in the rc struct, and we'll just overwrite it later w/the value from hardware, but if that read fails, we've at least got a sane rx timeout value to work with (1000ms is the default value I've seen returned on most if not all mceusb hardware).
--- drivers/media/rc/mceusb.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/media/rc/mceusb.c +1 -0
@@ -1059,6 +1059,7 @@ static struct rc_dev *mceusb_init_rc_dev(struct mceusb_dev *ir) rc->priv = ir; rc->driver_type =RC_DRIVER_IR_RAW ; rc->allowed_protos =RC_TYPE_ALL ; + rc->timeout =MS_TO_NS (1000 );if (!ir->flags.no_tx) { rc->s_tx_mask = mceusb_set_tx_mask; rc->s_tx_carrier = mceusb_set_tx_carrier;
Signature · 1 line
1.7.1
PATCH 3/4 rc: conversion is to microseconds, not nanoseconds
In reply to Jarod WilsonFix a thinko, and move macro definition to a common header so it can be shared amongst all drivers, as ms to us conversion is something that multiple drivers need to do. We probably ought to have its inverse available as well.
--- drivers/media/rc/ene_ir.c | 16 ++++++++-------- drivers/media/rc/ene_ir.h | 2 -- drivers/media/rc/mceusb.c | 7 +++---- include/media/rc-core.h | 1 + 4 files changed, 12 insertions(+), 14 deletions(-)
drivers/media/rc/ene_ir.c +8 -8
@@ -446,27 +446,27 @@ static void ene_rx_setup(struct ene_device *dev) select_timeout:if (dev->rx_fan_input_inuse) { - dev->rdev->rx_resolution =MS_TO_NS (ENE_FW_SAMPLE_PERIOD_FAN ); + dev->rdev->rx_resolution =MS_TO_US (ENE_FW_SAMPLE_PERIOD_FAN );/* Fan input doesn't support timeouts, it just ends the input with a maximum sample */ dev->rdev->min_timeout = dev->rdev->max_timeout = -MS_TO_NS (ENE_FW_SMPL_BUF_FAN_MSK * +MS_TO_US (ENE_FW_SMPL_BUF_FAN_MSK *ENE_FW_SAMPLE_PERIOD_FAN ); }else { - dev->rdev->rx_resolution =MS_TO_NS (sample_period); + dev->rdev->rx_resolution =MS_TO_US (sample_period);/* Theoreticly timeout is unlimited, but we cap it * because it was seen that on one device, it * would stop sending spaces after around 250 msec. * Besides, this is close to 2^32 anyway and timeout is u32. */ - dev->rdev->min_timeout =MS_TO_NS (127 * sample_period); - dev->rdev->max_timeout =MS_TO_NS (200000 ); + dev->rdev->min_timeout =MS_TO_US (127 * sample_period); + dev->rdev->max_timeout =MS_TO_US (200000 ); }if (dev->hw_learning_and_tx_capable) - dev->rdev->tx_resolution =MS_TO_NS (sample_period); + dev->rdev->tx_resolution =MS_TO_US (sample_period);if (dev->rdev->timeout > dev->rdev->max_timeout) dev->rdev->timeout = dev->rdev->max_timeout;
@@ -801,7 +801,7 @@ static irqreturn_t ene_isr(int irq, void *data)dbg ("RX: %d (%s)" , hw_sample, pulse ?"pulse" :"space" ); - ev.duration =MS_TO_NS (hw_sample); + ev.duration =MS_TO_US (hw_sample); ev.pulse = pulse;ir_raw_event_store_with_filter (dev->rdev, &ev); }
@@ -821,7 +821,7 @@ static void ene_setup_default_settings(struct ene_device *dev) dev->learning_mode_enabled = learning_mode_force;/* Set reasonable default timeout */ - dev->rdev->timeout =MS_TO_NS (150000 ); + dev->rdev->timeout =MS_TO_US (150000 ); }/* Upload all hardware settings at once. Used at load and resume time */
drivers/media/rc/ene_ir.h +0 -2
@@ -201,8 +201,6 @@#define dbg_verbose (format, ...) __dbg(2, format, ## __VA_ARGS__)#define dbg_regs (format, ...) __dbg(3, format, ## __VA_ARGS__) -#define MS_TO_NS (msec) ((msec) * 1000) -struct ene_device {struct pnp_dev *pnp_dev;struct rc_dev *rdev;
drivers/media/rc/mceusb.c +3 -4
@@ -48,7 +48,6 @@#define USB_BUFLEN 32/* USB reception buffer length */ #define USB_CTRL_MSG_SZ 2/* Size of usb ctrl msg on gen1 hw */ #define MCE_G1_INIT_MSGS 40/* Init messages on gen1 hw to throw out */ -#define MS_TO_NS (msec) ((msec) * 1000)/* MCE constants */ #define MCE_CMDBUF_SIZE 384/* MCE Command buffer length */
@@ -815,7 +814,7 @@ static void mceusb_handle_command(struct mceusb_dev *ir, int index)switch (ir->buf_in[index]) {/* 2-byte return value commands */ case MCE_CMD_S_TIMEOUT : - ir->rc->timeout =MS_TO_NS ((hi <<8 | lo) /2 ); + ir->rc->timeout =MS_TO_US ((hi <<8 | lo) /2 );break ;/* 1-byte return value commands */
@@ -856,7 +855,7 @@ static void mceusb_process_ir_data(struct mceusb_dev *ir, int buf_len) ir->rem--; rawir.pulse = ((ir->buf_in[i] &MCE_PULSE_BIT ) !=0 ); rawir.duration = (ir->buf_in[i] &MCE_PULSE_MASK ) - *MS_TO_NS (MCE_TIME_UNIT ); + *MS_TO_US (MCE_TIME_UNIT ); dev_dbg(ir->dev, "Storing %s with duration %d\n", rawir.pulse ? "pulse" : "space",
@@ -1059,7 +1058,7 @@ static struct rc_dev *mceusb_init_rc_dev(struct mceusb_dev *ir) rc->priv = ir; rc->driver_type =RC_DRIVER_IR_RAW ; rc->allowed_protos =RC_TYPE_ALL ; - rc->timeout =MS_TO_NS (1000 ); + rc->timeout =MS_TO_US (1000 );if (!ir->flags.no_tx) { rc->s_tx_mask = mceusb_set_tx_mask; rc->s_tx_carrier = mceusb_set_tx_carrier;
include/media/rc-core.h +1 -0
@@ -185,6 +185,7 @@ static inline void init_ir_raw_event(struct ir_raw_event *ev) }#define IR_MAX_DURATION 0xFFFFFFFF/* a bit more than 4 seconds */ +#define MS_TO_US (msec) ((msec) * 1000)void ir_raw_event_handle (struct rc_dev *dev);int ir_raw_event_store (struct rc_dev *dev,struct ir_raw_event *ev);
Signature · 1 line
1.7.1
Maxim Levitsky
maintainerRe: [PATCH 3/4] rc: conversion is to microseconds, not nanoseconds
Quoted 4 lines · Jarod Wilson
Fix a thinko, and move macro definition to a common header so it can be shared amongst all drivers, as ms to us conversion is something that multiple drivers need to do. We probably ought to have its inverse available as well.
Nope, at least ENE gets 'us' samples, that is 10^-6 seconds, and I multiply that by 1000, and that gives nanoseconds (10^-9). I have nothing against moving MS_TO_NS to common code of course.
Best regards,
Maxim Levitsky
Quoted 131 lines
Reported-by David Härdeman <[email protected]>CC Maxim Levitsky <[email protected]>Signed-off-by Jarod Wilson <[email protected]>--- drivers/media/rc/ene_ir.c | 16 ++++++++-------- drivers/media/rc/ene_ir.h | 2 -- drivers/media/rc/mceusb.c | 7 +++---- include/media/rc-core.h | 1 + 4 files changed, 12 insertions(+), 14 deletions(-)drivers/media/rc/ene_ir.c +8
-8@@ -446,27 +446,27 @@ static void ene_rx_setup(struct ene_device *dev) select_timeout:if (dev->rx_fan_input_inuse) { - dev->rdev->rx_resolution =MS_TO_NS (ENE_FW_SAMPLE_PERIOD_FAN ); + dev->rdev->rx_resolution =MS_TO_US (ENE_FW_SAMPLE_PERIOD_FAN );/* Fan input doesn't support timeouts, it just ends the input with a maximum sample */ dev->rdev->min_timeout = dev->rdev->max_timeout = -MS_TO_NS (ENE_FW_SMPL_BUF_FAN_MSK * +MS_TO_US (ENE_FW_SMPL_BUF_FAN_MSK *ENE_FW_SAMPLE_PERIOD_FAN ); }else { - dev->rdev->rx_resolution =MS_TO_NS (sample_period); + dev->rdev->rx_resolution =MS_TO_US (sample_period);/* Theoreticly timeout is unlimited, but we cap it * because it was seen that on one device, it * would stop sending spaces after around 250 msec. * Besides, this is close to 2^32 anyway and timeout is u32. */ - dev->rdev->min_timeout =MS_TO_NS (127 * sample_period); - dev->rdev->max_timeout =MS_TO_NS (200000 ); + dev->rdev->min_timeout =MS_TO_US (127 * sample_period); + dev->rdev->max_timeout =MS_TO_US (200000 ); }if (dev->hw_learning_and_tx_capable) - dev->rdev->tx_resolution =MS_TO_NS (sample_period); + dev->rdev->tx_resolution =MS_TO_US (sample_period);if (dev->rdev->timeout > dev->rdev->max_timeout) dev->rdev->timeout = dev->rdev->max_timeout;@@ -801,7 +801,7 @@ static irqreturn_t ene_isr(int irq, void *data)dbg ("RX: %d (%s)" , hw_sample, pulse ?"pulse" :"space" ); - ev.duration =MS_TO_NS (hw_sample); + ev.duration =MS_TO_US (hw_sample); ev.pulse = pulse;ir_raw_event_store_with_filter (dev->rdev, &ev); }@@ -821,7 +821,7 @@ static void ene_setup_default_settings(struct ene_device *dev) dev->learning_mode_enabled = learning_mode_force;/* Set reasonable default timeout */ - dev->rdev->timeout =MS_TO_NS (150000 ); + dev->rdev->timeout =MS_TO_US (150000 ); }/* Upload all hardware settings at once. Used at load and resume time */ drivers/media/rc/ene_ir.h +0
-2@@ -201,8 +201,6 @@#define dbg_verbose (format, ...) __dbg(2, format, ## __VA_ARGS__)#define dbg_regs (format, ...) __dbg(3, format, ## __VA_ARGS__) -#define MS_TO_NS (msec) ((msec) * 1000) -struct ene_device {struct pnp_dev *pnp_dev;struct rc_dev *rdev;drivers/media/rc/mceusb.c +3
-4@@ -48,7 +48,6 @@#define USB_BUFLEN 32/* USB reception buffer length */ #define USB_CTRL_MSG_SZ 2/* Size of usb ctrl msg on gen1 hw */ #define MCE_G1_INIT_MSGS 40/* Init messages on gen1 hw to throw out */ -#define MS_TO_NS (msec) ((msec) * 1000)/* MCE constants */ #define MCE_CMDBUF_SIZE 384/* MCE Command buffer length */ @@ -815,7 +814,7 @@ static void mceusb_handle_command(struct mceusb_dev *ir, int index)switch (ir->buf_in[index]) {/* 2-byte return value commands */ case MCE_CMD_S_TIMEOUT : - ir->rc->timeout =MS_TO_NS ((hi <<8 | lo) /2 ); + ir->rc->timeout =MS_TO_US ((hi <<8 | lo) /2 );break ;/* 1-byte return value commands */ @@ -856,7 +855,7 @@ static void mceusb_process_ir_data(struct mceusb_dev *ir, int buf_len) ir->rem--; rawir.pulse = ((ir->buf_in[i] &MCE_PULSE_BIT ) !=0 ); rawir.duration = (ir->buf_in[i] &MCE_PULSE_MASK ) - *MS_TO_NS (MCE_TIME_UNIT ); + *MS_TO_US (MCE_TIME_UNIT ); dev_dbg(ir->dev, "Storing %s with duration %d\n", rawir.pulse ? "pulse" : "space",@@ -1059,7 +1058,7 @@ static struct rc_dev *mceusb_init_rc_dev(struct mceusb_dev *ir) rc->priv = ir; rc->driver_type =RC_DRIVER_IR_RAW ; rc->allowed_protos =RC_TYPE_ALL ; - rc->timeout =MS_TO_NS (1000 ); + rc->timeout =MS_TO_US (1000 );if (!ir->flags.no_tx) { rc->s_tx_mask = mceusb_set_tx_mask; rc->s_tx_carrier = mceusb_set_tx_carrier;include/media/rc-core.h +1
-0@@ -185,6 +185,7 @@ static inline void init_ir_raw_event(struct ir_raw_event *ev) }#define IR_MAX_DURATION 0xFFFFFFFF/* a bit more than 4 seconds */ +#define MS_TO_US (msec) ((msec) * 1000)void ir_raw_event_handle (struct rc_dev *dev);int ir_raw_event_store (struct rc_dev *dev,struct ir_raw_event *ev);
Re: [PATCH 3/4] rc: conversion is to microseconds, not nanoseconds
Quoted 9 lines · Maxim Levitsky
Quoted 4 lines · Jarod Wilson
Fix a thinko, and move macro definition to a common header so it can be shared amongst all drivers, as ms to us conversion is something that multiple drivers need to do. We probably ought to have its inverse available as well.
Nope, at least ENE gets 'us' samples, that is 10^-6 seconds, and I multiply that by 1000, and that gives nanoseconds (10^-9).
Huh, okay. I'd have expected it to be US_TO_NS then. I basically hijacked a copy over to mceusb and was using it for ms to us conversions.
Quoted 1 line
I have nothing against moving
MS_TO_NSto common code of course.
I'll throw together something new in the relatively near future, will probably just add macros for both conversions and both directions.
Signature · 2 lines
Jarod Wilson [email protected]
PATCH 4/4 mceusb: add another Fintek device ID
In reply to Jarod Wilson--- drivers/media/rc/mceusb.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
drivers/media/rc/mceusb.c +2 -0
@@ -281,6 +281,8 @@ static struct usb_device_id mceusb_dev_table[] = { { USB_DEVICE(VENDOR_FORMOSA, 0xe03c) },/* Formosa Industrial Computing */ { USB_DEVICE(VENDOR_FORMOSA, 0xe03e) }, +/* Fintek eHome Infrared Transceiver (HP branded) */ + { USB_DEVICE(VENDOR_FINTEK, 0x5168) },/* Fintek eHome Infrared Transceiver */ { USB_DEVICE(VENDOR_FINTEK, 0x0602) },/* Fintek eHome Infrared Transceiver (in the AOpen MP45) */
Signature · 1 line
1.7.1