Re: [PATCH 4/8 v3] Input: synaptics - add image sensor support

[Daniel Kurtz]

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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/