Re: [PATCH v3] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.

quoted

@@ -147,11 +163,15 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,

 {
       struct mt_device *td = hid_get_drvdata(hdev);
       struct mt_class *cls = td->mtclass;
+       __s32 quirks = cls->quirks;
+
       switch (usage->hid & HID_USAGE_PAGE) {

       case HID_UP_GENDESK:
               switch (usage->hid) {
               case HID_GD_X:
+                       if (quirks & MT_QUIRK_EGALAX_XYZ_FIXUP)
+                               field->logical_maximum = 32760;

Please do not add numerical constants in a generic code. If I want to
override the values for another device, I'll have to rename your quirk
to MT_QUIRK_EGALAX_XYZ_FIXUP_WITH_0_32760_0_32760_0_x and add mine.
These values have to be moved in the MT_CLS.

It is a specific value used for a specific code path, no need to
over-generalize.  The quirk name could be specialized even more, i
suppose, but the point is that it makes the hid-multitouch driver do
exactly what is done in the hid-egalax driver, and it is easy to prove
that there are no side effects for other devices.

quoted

               case HID_DG_TIPPRESSURE:
+                       if (quirks & MT_QUIRK_EGALAX_XYZ_FIXUP)
+                               field->logical_minimum = 0;

No really need for the test and the quirk. In addition, here you do
not override logical_max. This shows that the quirk is not
MT_QUIRK_EGALAX_XYZ_FIXUP but the combination of
MT_QUIRK_EGALAX_XY_FIXUP and  MT_QUIRK_EGALAX_Z_MIN_FIXUP.
Just drop it here.

The added code path simply reflects what is in the hid-egalax
driver. It is true that the statement _probably_ works for other
devices as well, but it cannot be proven just by looking at the
code. Also, we have no known additional case where it is needed.

I am trying to think of a better name for the quirk, but frankly, I
think it already says what it needs to say.

Thanks,
Henrik
--
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