Hi Henrik, Thanks for the great feedback. On Sat, Aug 13, 2011 at 5:09 AM, Henrik Rydberg <rydberg@xxxxxxxxxxx> wrote: > Hi Daniel, > > looks good, some details below. > >> +static void synaptics_report_slot_empty(struct input_dev *dev, int slot) >> +{ >> + input_mt_slot(dev, slot); >> + input_mt_report_slot_state(dev, MT_TOOL_FINGER, false); >> +} >> + >> +static void synaptics_report_slot_sgm(struct input_dev *dev, int slot, >> + const struct synaptics_hw_state *sgm) >> +{ >> + input_mt_slot(dev, slot); >> + input_mt_report_slot_state(dev, MT_TOOL_FINGER, true); >> + input_report_abs(dev, ABS_MT_POSITION_X, sgm->x); >> + input_report_abs(dev, ABS_MT_POSITION_Y, invert_y(sgm->y)); >> + input_report_abs(dev, ABS_MT_PRESSURE, sgm->z); >> + /* SGM can sometimes contain valid width */ >> + if (sgm->w >= 4) >> + input_report_abs(dev, ABS_MT_TOUCH_MAJOR, sgm->w); >> +} > > The ABS_MT_TOUCH_MAJOR is supposed to have zero intercept, to remain > compatible with user space handling of type A devices. Also, the scale > should match the screen/pad size, such that the actual size of the > touch area can be deduced. In addition, based on the current sensor > technologies, ABS_MT_PRESSURE and ABS_MT_TOUCH_MAJOR are normally > mutually exclusive. > > All in all, I would prefer to only report width via (the single-finger > axis) ABS_TOOL_WIDTH, and only for compatibility reasons. > > + > +static void synaptics_report_slot_agm(struct input_dev *dev, int slot, > + const struct synaptics_hw_state *agm) > +{ > + input_mt_slot(dev, slot); > + input_mt_report_slot_state(dev, MT_TOOL_FINGER, true); > + input_report_abs(dev, ABS_MT_POSITION_X, agm->x); > + input_report_abs(dev, ABS_MT_POSITION_Y, invert_y(agm->y)); > + input_report_abs(dev, ABS_MT_PRESSURE, agm->z); > +} With ABS_MT_TOUCH_MAJOR dropped, sgm and agm seems to coincide... .... > > Looks good otherwise. > > Thanks, > Henrik > Ahh.... ok, this makes sense. I missed this detail previously. Basically: * Only report synaptics "w" as ABS_TOOL_WIDTH (for backwards compatibility) * Report synaptics "z" as ABS_MT_TOUCH_MAJOR (assuming it is really 'width of finger in surface units'), and ABS_PRESSURE (for backwards compatibility). * Don't use ABS_MT_PRESSURE. * This will simplify agm/sgm reporting by making them both reoprt the same axes. I will rework these patches to do this. Thanks, -Dan -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html