On Mon, Jun 26, 2017 at 8:22 AM, Wang Yafei <wangyafei@xxxxxxxxxx> wrote:
> Hi Rob,
>
> Thank you for the review.
>
> On Fri, Jun 23, 2017 at 03:37:06PM -0500, Rob Herring wrote:
>> On Mon, Jun 19, 2017 at 06:40:32PM +0800, Wang Yafei wrote:
>> > Signed-off-by: Wang Yafei <wangyafei@xxxxxxxxxx>
>> > ---
>> > .../bindings/input/touchscreen/goodix-gtx5.txt | 75 ++++++++++++++++++++++
>> > 1 file changed, 75 insertions(+)
>> > create mode 100755 Documentation/devicetree/bindings/input/touchscreen/goodix-gtx5.txt
[...]
>> > +- touchscreen-key-map: keycode value map /*KEY_HOMEPAGE, KEY_BACK*/
>>
>> This hardly seems sufficient to map screen areas to keys. There's been
>> some discussion about doing that in the past.
>
> Sorry haven't notice that before, I will change it to the flowing
> property format:
> linux,keycodes = <KEY_HOMEPAGE>, <KEY_MENU>, <KEY_BACK>;
That's better, but not really what I meant. There's still some
assumption about what region each key is located in. Is every design 3
keys? How do you know what region of the touchscreen is a given key?
iOW, what are the valid ranges of coordinates for each key?
>> > + compatible = "goodix,gtx5";
>> > + reg = <0x14>;
>> > + interrupt-parent = <&msm_gpio>;
>> > + interrupts = <13 0x2800>;
>> > + vtouch-supply = <&pm8916_l15>;
>> > + reset-gpios = <&msm_gpio 12 0x0>;
>> > + irq-gpios = <&msm_gpio 13 0x2800>;
>> > + irq-flags = <1>; /* 1:trigger rising, 2:trigger falling;*/
>> > + touchscreen-max-id = <10>;
>> > + touchscreen-size-x = <400>;
>> > + touchscreen-size-y = <400>;
>> > + touchscreen-max-w = <400>;
>> > + touchscreen-max-pressure = <255>;
>> > + touchscreen-swapped-x-y;
>> > + touchscreen-key-map = <172 158>; /*KEY_HOMEPAGE, KEY_BACK*/
>> > + sensor0 {
>>
>> How many sensors can there be? That needs to be documented.
>>
>> Why do you need a sub-node?
>
> sensor is an internal call, we use senesorX to represent the touchpanel
> is produced by which factory. A panel factory will have it corresponding
> config data to get the best touch results. It will delete the sub-node
> and change to the following format:
> panel0,normal-cfg = [...] /* panelX,cfg : X is no more than 10*/
panel0 is not a vendor and normal doesn't tell us anything. How about
just "goodix,panel-cfg-X"?
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
