| Message ID | 3580124.nFkHgCoyEf@geidi-prime (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
Hi, I have simply taken what we have right now and ported it over to ff-mlnx. I believe that any improvements based on the previous investigation should go into another series because we most certainly don't want to introduce any regressions. Michal -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday 05 of February 2014 00:54:45 Michal Malý wrote: > Hi, > > I have simply taken what we have right now and ported it over to ff-mlnx. > I believe that any improvements based on the previous investigation should > go into another series because we most certainly don't want to introduce any > regressions. > > Michal Ahh... please disregard this. 2/5 should have the same fix for zero force as we have right now - it should effectively stop the effect when the force is zero. Michal -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Ahh... please disregard this. 2/5 should have the same fix for zero force > as > we have right now - it should effectively stop the effect when the force > is zero. Hi, We need to think carefully on this as a '+10' combined with '-10' force will result in '0'; however we do not want to deactivate the wheel (no-resistance mode) as this will result in 'ticking' on the wheel as force transitions through 0.... ideally we'd need to know that there are no forces set. I'll attempt to build these patches this week and give them a test, Simon. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday 05 of February 2014 09:41:16 you wrote: > On Wed, Feb 5, 2014 at 1:08 AM, <simon@mungewell.org> wrote: > > > Ahh... please disregard this. 2/5 should have the same fix for zero > > > force > > > as > > > we have right now - it should effectively stop the effect when the force > > > is zero. > > > > Hi, > > We need to think carefully on this as a '+10' combined with '-10' force > > will result in '0'; however we do not want to deactivate the wheel > > (no-resistance mode) as this will result in 'ticking' on the wheel as > > force transitions through 0.... ideally we'd need to know that there are > > no forces set. > > No problem Simon, this is thought about: > If forces are active, and they cancel out, hid_lg4ff_start_combined is > called with 0 force (scaled_x = 0x80), instead of hid_lg4ff_stop_combined > as you assumed. Okay, so to set things straight: I should modify to patch to send 0x13 when the effect is fully stopped and use 0x11 0x80 to only zeroize the force when the overall force is zero? My DFP doesn't differentiate between these states so I cannot tell what is the right thing to do. Michal -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 5 Feb 2014, Michal Malý wrote: > > > We need to think carefully on this as a '+10' combined with '-10' force > > > will result in '0'; however we do not want to deactivate the wheel > > > (no-resistance mode) as this will result in 'ticking' on the wheel as > > > force transitions through 0.... ideally we'd need to know that there are > > > no forces set. > > > > No problem Simon, this is thought about: > > If forces are active, and they cancel out, hid_lg4ff_start_combined is > > called with 0 force (scaled_x = 0x80), instead of hid_lg4ff_stop_combined > > as you assumed. > > Okay, so to set things straight: I should modify to patch to send 0x13 when > the effect is fully stopped and use 0x11 0x80 to only zeroize the force when > the overall force is zero? My DFP doesn't differentiate between these states > so I cannot tell what is the right thing to do. Has there been any conclusion to this, please?
On Thursday 20 of February 2014 16:16:52 Jiri Kosina wrote: > On Wed, 5 Feb 2014, Michal Malý wrote: > > > > We need to think carefully on this as a '+10' combined with '-10' > > > > force > > > > will result in '0'; however we do not want to deactivate the wheel > > > > (no-resistance mode) as this will result in 'ticking' on the wheel as > > > > force transitions through 0.... ideally we'd need to know that there > > > > are > > > > no forces set. > > > > > > No problem Simon, this is thought about: > > > If forces are active, and they cancel out, hid_lg4ff_start_combined is > > > called with 0 force (scaled_x = 0x80), instead of > > > hid_lg4ff_stop_combined > > > as you assumed. > > > > Okay, so to set things straight: I should modify to patch to send 0x13 > > when > > the effect is fully stopped and use 0x11 0x80 to only zeroize the force > > when the overall force is zero? My DFP doesn't differentiate between > > these states so I cannot tell what is the right thing to do. > > Has there been any conclusion to this, please? Yes the has and the necessary fix is already in place in my working copy along with another few fixed to the ff-memless-next. Do you consider our approach sane and viable for mainlining? I've already explained to Joe Perches why I propose ff-memless-next as a separate module rather than a patch to ff-memless we have now. Unless anybody has any objections I'll check in with the rest of the guys and submit a hopefully final version within a day or two. Michal -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig index 34e2d39..31c4b43 100644 --- a/drivers/hid/Kconfig +++ b/drivers/hid/Kconfig @@ -389,7 +389,7 @@ config LOGIG940_FF config LOGIWHEELS_FF bool "Logitech wheels configuration and force feedback support" depends on HID_LOGITECH - select INPUT_FF_MEMLESS + select INPUT_FF_MEMLESS_NEXT default LOGITECH_FF help Say Y here if you want to enable force feedback and range setting diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c index befe0e3..f236b9f 100644 --- a/drivers/hid/hid-lg4ff.c +++ b/drivers/hid/hid-lg4ff.c @@ -25,6 +25,7 @@ #include <linux/input.h> +#include <linux/input/ff-memless-next.h> #include <linux/usb.h> #include <linux/hid.h> @@ -44,6 +45,8 @@ #define G27_REV_MAJ 0x12 #define G27_REV_MIN 0x38 +#define FF_UPDATE_RATE 8 + #define to_hid_device(pdev) container_of(pdev, struct hid_device, dev) static void hid_lg4ff_set_range_dfp(struct hid_device *hid, u16 range); @@ -182,47 +185,60 @@ int lg4ff_adjust_input_event(struct hid_device *hid, struct hid_field *field, } } -static int hid_lg4ff_play(struct input_dev *dev, void *data, struct ff_effect *effect) +static int hid_lg4ff_start_combined(struct hid_device *hid, struct hid_report *report, + const struct mlnx_simple_force *force) +{ + __s32 *value = report->field[0]->value; + int scaled_x; + + /* Scale down from MLNX range */ + scaled_x = 0x80 - (force->x * 0xff / 0xffff); + + value[0] = 0x11; /* Slot 1 */ + value[1] = 0x08; + value[2] = scaled_x; + value[3] = 0x80; + value[4] = 0x00; + value[5] = 0x00; + value[6] = 0x00; + + hid_hw_request(hid, report, HID_REQ_SET_REPORT); + return 0; +} + +static int hid_lg4ff_stop_combined(struct hid_device *hid, struct hid_report *report) +{ + __s32 *value = report->field[0]->value; + + value[0] = 0x11; /* Slot 1 */ + value[1] = 0x08; + value[2] = 0x80; + value[3] = 0x80; + value[4] = 0x00; + value[5] = 0x00; + value[6] = 0x00; + + hid_hw_request(hid, report, HID_REQ_SET_REPORT); + return 0; +} + +static int hid_lg4ff_control(struct input_dev *dev, void *data, const struct mlnx_effect_command *command) { 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); - __s32 *value = report->field[0]->value; - int x; - -#define CLAMP(x) do { if (x < 0) x = 0; else if (x > 0xff) x = 0xff; } while (0) - - switch (effect->type) { - case FF_CONSTANT: - x = effect->u.ramp.start_level + 0x80; /* 0x80 is no force */ - CLAMP(x); - - if (x == 0x80) { - /* De-activate force in slot-1*/ - value[0] = 0x13; - value[1] = 0x00; - value[2] = 0x00; - value[3] = 0x00; - value[4] = 0x00; - value[5] = 0x00; - value[6] = 0x00; - - hid_hw_request(hid, report, HID_REQ_SET_REPORT); - return 0; - } - value[0] = 0x11; /* Slot 1 */ - value[1] = 0x08; - value[2] = x; - value[3] = 0x80; - value[4] = 0x00; - value[5] = 0x00; - value[6] = 0x00; - - hid_hw_request(hid, report, HID_REQ_SET_REPORT); + switch (command->cmd) { + case MLNX_START_COMBINED: + return hid_lg4ff_start_combined(hid, report, &command->u.simple_force); + break; + case MLNX_STOP_COMBINED: + return hid_lg4ff_stop_combined(hid, report); break; + default: + dbg_hid("Unsupported effect command"); + return -EINVAL; } - return 0; } /* Sends default autocentering command compatible with @@ -608,7 +624,7 @@ int lg4ff_init(struct hid_device *hid) for (j = 0; lg4ff_devices[i].ff_effects[j] >= 0; j++) set_bit(lg4ff_devices[i].ff_effects[j], dev->ffbit); - error = input_ff_create_memless(dev, NULL, hid_lg4ff_play); + error = input_ff_create_mlnx(dev, (void *)NULL, hid_lg4ff_control, FF_UPDATE_RATE); if (error) return error;