| Message ID | 20190311212041.GA25630@nova.chris.boyle.name (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | [1/3] drivers: hid: fix G940 axis/button mappings | expand |
[ CCing Gary Stein ] On Mon, 11 Mar 2019, Chris Boyle wrote: > Disable default auto-centering of the Logitech G940, which otherwise > makes the stick difficult to use. Also when autocenter is requested, obey > the magnitude. > > The auto-centering effect will only unlock (or become the requested > level) when an application starts using the joystick and the hand sensor > on the stick is covered. That appears to be a hardware limitation. > > To make the above and future work on spring/damper support simpler and > more type-safe, add a struct of the HID report. Many thanks to fred41 from > forums.x-plane.org for useful information towards that. > > It seems byte 0 does not need to be a "command byte" of 0x51; it can be > anything and is the least-significant byte of the constant force. > > Signed-off-by: Chris Boyle <chris@boyle.name> > --- > drivers/hid/hid-lg3ff.c | 148 +++++++++++++++++++++------------------- > 1 file changed, 78 insertions(+), 70 deletions(-) > > diff --git a/drivers/hid/hid-lg3ff.c b/drivers/hid/hid-lg3ff.c > index 8c2da183d3bc..6fcd449d5b46 100644 > --- a/drivers/hid/hid-lg3ff.c > +++ b/drivers/hid/hid-lg3ff.c > @@ -2,6 +2,7 @@ > * Force feedback support for Logitech Flight System G940 > * > * Copyright (c) 2009 Gary Stein <LordCnidarian@gmail.com> > + * Copyright (c) 2019 Chris Boyle > */ > > /* > @@ -26,51 +27,68 @@ > > #include "hid-lg.h" > > -/* > - * G940 Theory of Operation (from experimentation) > - * > - * There are 63 fields (only 3 of them currently used) > - * 0 - seems to be command field > - * 1 - 30 deal with the x axis > - * 31 -60 deal with the y axis > - * > - * Field 1 is x axis constant force > - * Field 31 is y axis constant force > - * > - * other interesting fields 1,2,3,4 on x axis > - * (same for 31,32,33,34 on y axis) > - * > - * 0 0 127 127 makes the joystick autocenter hard > - * > - * 127 0 127 127 makes the joystick loose on the right, > - * but stops all movemnt left > - * > - * -127 0 -127 -127 makes the joystick loose on the left, > - * but stops all movement right > - * > - * 0 0 -127 -127 makes the joystick rattle very hard > - * > - * I'm sure these are effects that I don't know enough about them > - */ > +/* Ensure we remember to swap bytes (there's no sle16) */ > +typedef __s16 __bitwise lg3_s16; > > -struct lg3ff_device { > - struct hid_report *report; > -}; > +static inline lg3_s16 lg3ff_cpu_to_sle16(s16 val) > +{ > + return (__force lg3_s16)cpu_to_le16(val); > +} > + > +struct hid_lg3ff_axis { > + lg3_s16 constant_force; /* can cancel autocenter on relevant side */ > + u8 _padding0; /* extra byte of strength? no apparent effect */ > + /* how far towards center does the effect keep pushing: > + * 0 = no autocenter, up to: > + * 127 = push immediately on any deflection > + * <0 = repel center > + */ > + s8 autocenter_strength; > + /* how hard does autocenter push */ > + s8 autocenter_force; > + /* damping with force of autocenter_force (see also damper_*) */ > + s8 autocenter_damping; > + lg3_s16 spring_deadzone_neg; /* for offset center, set these equal */ > + lg3_s16 spring_deadzone_pos; > + s8 spring_coeff_neg; /* <0 repels center */ > + s8 spring_coeff_pos; > + lg3_s16 spring_saturation; > + u8 _padding1[8]; /* [4-8]: a different way of autocentering? */ > + s8 damper_coeff_neg; > + s8 damper_coeff_pos; > + lg3_s16 damper_saturation; > + u8 _padding2[4]; /* seems to do the same as damper*? */ > +} __packed; > + > +struct hid_lg3ff_report { > + struct hid_lg3ff_axis x; > + struct hid_lg3ff_axis y; > + u8 _padding[3]; > +} __packed; > + > +#define FF_REPORT_ID 2 > + > +static void hig_lg3ff_send(struct input_dev *idev, > + struct hid_lg3ff_report *raw_rep) > +{ > + struct hid_device *hid = input_get_drvdata(idev); > + struct hid_report *hid_rep = hid->report_enum[HID_OUTPUT_REPORT] > + .report_id_hash[FF_REPORT_ID]; > + int i; > + > + /* We can be called while atomic (via hid_lg3ff_play) and must queue; > + * there's nowhere to enqueue a raw report, so populate a hid_report. > + */ > + for (i = 0; i < sizeof(*raw_rep); i++) > + hid_rep->field[0]->value[i] = ((u8 *)raw_rep)[i]; > + hid_hw_request(hid, hid_rep, HID_REQ_SET_REPORT); > +} > > static int hid_lg3ff_play(struct input_dev *dev, void *data, > struct ff_effect *effect) > { > - struct hid_device *hid = input_get_drvdata(dev); > - struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list; > - struct hid_report *report = list_entry(report_list->next, struct hid_report, list); > - int x, y; > - > -/* > - * Available values in the field should always be 63, but we only use up to > - * 35. Instead, clear the entire area, however big it is. > - */ > - memset(report->field[0]->value, 0, > - sizeof(__s32) * report->field[0]->report_count); > + struct hid_lg3ff_report report = {0}; > + s16 x, y; > > switch (effect->type) { > case FF_CONSTANT: > @@ -78,46 +96,32 @@ static int hid_lg3ff_play(struct input_dev *dev, void *data, > * Already clamped in ff_memless > * 0 is center (different then other logitech) > */ > - x = effect->u.ramp.start_level; > - y = effect->u.ramp.end_level; > - > - /* send command byte */ > - report->field[0]->value[0] = 0x51; > + x = -effect->u.ramp.start_level << 8; > + y = -effect->u.ramp.end_level << 8; > > /* > * Sign backwards from other Force3d pro > * which get recast here in two's complement 8 bits > */ > - report->field[0]->value[1] = (unsigned char)(-x); > - report->field[0]->value[31] = (unsigned char)(-y); > - > - hid_hw_request(hid, report, HID_REQ_SET_REPORT); > + report.x.constant_force = lg3ff_cpu_to_sle16(x); > + report.y.constant_force = lg3ff_cpu_to_sle16(y); > + hig_lg3ff_send(dev, &report); > break; > } > return 0; > } > static void hid_lg3ff_set_autocenter(struct input_dev *dev, u16 magnitude) > { > - struct hid_device *hid = input_get_drvdata(dev); > - struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list; > - struct hid_report *report = list_entry(report_list->next, struct hid_report, list); > + struct hid_lg3ff_report report = {0}; > > -/* > - * Auto Centering probed from device > - * NOTE: deadman's switch on G940 must be covered > - * for effects to work > - */ > - report->field[0]->value[0] = 0x51; > - report->field[0]->value[1] = 0x00; > - report->field[0]->value[2] = 0x00; > - report->field[0]->value[3] = 0x7F; > - report->field[0]->value[4] = 0x7F; > - report->field[0]->value[31] = 0x00; > - report->field[0]->value[32] = 0x00; > - report->field[0]->value[33] = 0x7F; > - report->field[0]->value[34] = 0x7F; > - > - hid_hw_request(hid, report, HID_REQ_SET_REPORT); > + /* negative means repel from center, so scale to 0-127 */ > + s8 mag_scaled = magnitude >> 9; > + > + report.x.autocenter_strength = 127; > + report.x.autocenter_force = mag_scaled; > + report.y.autocenter_strength = 127; > + report.y.autocenter_force = mag_scaled; > + hig_lg3ff_send(dev, &report); > } > > > @@ -136,7 +140,9 @@ int lg3ff_init(struct hid_device *hid) > int i; > > /* Check that the report looks ok */ > - if (!hid_validate_values(hid, HID_OUTPUT_REPORT, 0, 0, 35)) > + BUILD_BUG_ON(sizeof(struct hid_lg3ff_report) != 63); /* excl. id */ > + if (!hid_validate_values(hid, HID_OUTPUT_REPORT, FF_REPORT_ID, 0, > + sizeof(struct hid_lg3ff_report))) > return -ENODEV; > > /* Assume single fixed device G940 */ > @@ -147,8 +153,10 @@ int lg3ff_init(struct hid_device *hid) > if (error) > return error; > > - if (test_bit(FF_AUTOCENTER, dev->ffbit)) > + if (test_bit(FF_AUTOCENTER, dev->ffbit)) { > dev->ff->set_autocenter = hid_lg3ff_set_autocenter; > + hid_lg3ff_set_autocenter(dev, 0); > + } > > hid_info(hid, "Force feedback for Logitech Flight System G940 by Gary Stein <LordCnidarian@gmail.com>\n"); > return 0; > -- > 2.17.1 > > > -- > Chris Boyle > https://chris.boyle.name/ >
diff --git a/drivers/hid/hid-lg3ff.c b/drivers/hid/hid-lg3ff.c index 8c2da183d3bc..6fcd449d5b46 100644 --- a/drivers/hid/hid-lg3ff.c +++ b/drivers/hid/hid-lg3ff.c @@ -2,6 +2,7 @@ * Force feedback support for Logitech Flight System G940 * * Copyright (c) 2009 Gary Stein <LordCnidarian@gmail.com> + * Copyright (c) 2019 Chris Boyle */ /* @@ -26,51 +27,68 @@ #include "hid-lg.h" -/* - * G940 Theory of Operation (from experimentation) - * - * There are 63 fields (only 3 of them currently used) - * 0 - seems to be command field - * 1 - 30 deal with the x axis - * 31 -60 deal with the y axis - * - * Field 1 is x axis constant force - * Field 31 is y axis constant force - * - * other interesting fields 1,2,3,4 on x axis - * (same for 31,32,33,34 on y axis) - * - * 0 0 127 127 makes the joystick autocenter hard - * - * 127 0 127 127 makes the joystick loose on the right, - * but stops all movemnt left - * - * -127 0 -127 -127 makes the joystick loose on the left, - * but stops all movement right - * - * 0 0 -127 -127 makes the joystick rattle very hard - * - * I'm sure these are effects that I don't know enough about them - */ +/* Ensure we remember to swap bytes (there's no sle16) */ +typedef __s16 __bitwise lg3_s16; -struct lg3ff_device { - struct hid_report *report; -}; +static inline lg3_s16 lg3ff_cpu_to_sle16(s16 val) +{ + return (__force lg3_s16)cpu_to_le16(val); +} + +struct hid_lg3ff_axis { + lg3_s16 constant_force; /* can cancel autocenter on relevant side */ + u8 _padding0; /* extra byte of strength? no apparent effect */ + /* how far towards center does the effect keep pushing: + * 0 = no autocenter, up to: + * 127 = push immediately on any deflection + * <0 = repel center + */ + s8 autocenter_strength; + /* how hard does autocenter push */ + s8 autocenter_force; + /* damping with force of autocenter_force (see also damper_*) */ + s8 autocenter_damping; + lg3_s16 spring_deadzone_neg; /* for offset center, set these equal */ + lg3_s16 spring_deadzone_pos; + s8 spring_coeff_neg; /* <0 repels center */ + s8 spring_coeff_pos; + lg3_s16 spring_saturation; + u8 _padding1[8]; /* [4-8]: a different way of autocentering? */ + s8 damper_coeff_neg; + s8 damper_coeff_pos; + lg3_s16 damper_saturation; + u8 _padding2[4]; /* seems to do the same as damper*? */ +} __packed; + +struct hid_lg3ff_report { + struct hid_lg3ff_axis x; + struct hid_lg3ff_axis y; + u8 _padding[3]; +} __packed; + +#define FF_REPORT_ID 2 + +static void hig_lg3ff_send(struct input_dev *idev, + struct hid_lg3ff_report *raw_rep) +{ + struct hid_device *hid = input_get_drvdata(idev); + struct hid_report *hid_rep = hid->report_enum[HID_OUTPUT_REPORT] + .report_id_hash[FF_REPORT_ID]; + int i; + + /* We can be called while atomic (via hid_lg3ff_play) and must queue; + * there's nowhere to enqueue a raw report, so populate a hid_report. + */ + for (i = 0; i < sizeof(*raw_rep); i++) + hid_rep->field[0]->value[i] = ((u8 *)raw_rep)[i]; + hid_hw_request(hid, hid_rep, HID_REQ_SET_REPORT); +} static int hid_lg3ff_play(struct input_dev *dev, void *data, struct ff_effect *effect) { - struct hid_device *hid = input_get_drvdata(dev); - struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list; - struct hid_report *report = list_entry(report_list->next, struct hid_report, list); - int x, y; - -/* - * Available values in the field should always be 63, but we only use up to - * 35. Instead, clear the entire area, however big it is. - */ - memset(report->field[0]->value, 0, - sizeof(__s32) * report->field[0]->report_count); + struct hid_lg3ff_report report = {0}; + s16 x, y; switch (effect->type) { case FF_CONSTANT: @@ -78,46 +96,32 @@ static int hid_lg3ff_play(struct input_dev *dev, void *data, * Already clamped in ff_memless * 0 is center (different then other logitech) */ - x = effect->u.ramp.start_level; - y = effect->u.ramp.end_level; - - /* send command byte */ - report->field[0]->value[0] = 0x51; + x = -effect->u.ramp.start_level << 8; + y = -effect->u.ramp.end_level << 8; /* * Sign backwards from other Force3d pro * which get recast here in two's complement 8 bits */ - report->field[0]->value[1] = (unsigned char)(-x); - report->field[0]->value[31] = (unsigned char)(-y); - - hid_hw_request(hid, report, HID_REQ_SET_REPORT); + report.x.constant_force = lg3ff_cpu_to_sle16(x); + report.y.constant_force = lg3ff_cpu_to_sle16(y); + hig_lg3ff_send(dev, &report); break; } return 0; } static void hid_lg3ff_set_autocenter(struct input_dev *dev, u16 magnitude) { - struct hid_device *hid = input_get_drvdata(dev); - struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list; - struct hid_report *report = list_entry(report_list->next, struct hid_report, list); + struct hid_lg3ff_report report = {0}; -/* - * Auto Centering probed from device - * NOTE: deadman's switch on G940 must be covered - * for effects to work - */ - report->field[0]->value[0] = 0x51; - report->field[0]->value[1] = 0x00; - report->field[0]->value[2] = 0x00; - report->field[0]->value[3] = 0x7F; - report->field[0]->value[4] = 0x7F; - report->field[0]->value[31] = 0x00; - report->field[0]->value[32] = 0x00; - report->field[0]->value[33] = 0x7F; - report->field[0]->value[34] = 0x7F; - - hid_hw_request(hid, report, HID_REQ_SET_REPORT); + /* negative means repel from center, so scale to 0-127 */ + s8 mag_scaled = magnitude >> 9; + + report.x.autocenter_strength = 127; + report.x.autocenter_force = mag_scaled; + report.y.autocenter_strength = 127; + report.y.autocenter_force = mag_scaled; + hig_lg3ff_send(dev, &report); } @@ -136,7 +140,9 @@ int lg3ff_init(struct hid_device *hid) int i; /* Check that the report looks ok */ - if (!hid_validate_values(hid, HID_OUTPUT_REPORT, 0, 0, 35)) + BUILD_BUG_ON(sizeof(struct hid_lg3ff_report) != 63); /* excl. id */ + if (!hid_validate_values(hid, HID_OUTPUT_REPORT, FF_REPORT_ID, 0, + sizeof(struct hid_lg3ff_report))) return -ENODEV; /* Assume single fixed device G940 */ @@ -147,8 +153,10 @@ int lg3ff_init(struct hid_device *hid) if (error) return error; - if (test_bit(FF_AUTOCENTER, dev->ffbit)) + if (test_bit(FF_AUTOCENTER, dev->ffbit)) { dev->ff->set_autocenter = hid_lg3ff_set_autocenter; + hid_lg3ff_set_autocenter(dev, 0); + } hid_info(hid, "Force feedback for Logitech Flight System G940 by Gary Stein <LordCnidarian@gmail.com>\n"); return 0;
Disable default auto-centering of the Logitech G940, which otherwise makes the stick difficult to use. Also when autocenter is requested, obey the magnitude. The auto-centering effect will only unlock (or become the requested level) when an application starts using the joystick and the hand sensor on the stick is covered. That appears to be a hardware limitation. To make the above and future work on spring/damper support simpler and more type-safe, add a struct of the HID report. Many thanks to fred41 from forums.x-plane.org for useful information towards that. It seems byte 0 does not need to be a "command byte" of 0x51; it can be anything and is the least-significant byte of the constant force. Signed-off-by: Chris Boyle <chris@boyle.name> --- drivers/hid/hid-lg3ff.c | 148 +++++++++++++++++++++------------------- 1 file changed, 78 insertions(+), 70 deletions(-)