| Message ID | 20211103114830.62711-3-alistair@alistair23.me (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for the Cypress cyttsp5 | expand |
On Wed, 03 Nov 2021 21:48:28 +1000, Alistair Francis wrote: > From: Mylène Josserand <mylene.josserand@free-electrons.com> > > Add the Cypress TrueTouch Generation 5 touchscreen device tree bindings > documentation. It can use I2C or SPI bus. > This touchscreen can handle some defined zone that are designed and > sent as button. To be able to customize the keycode sent, the > "linux,code" property in a "button" sub-node can be used. > > Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com> > Message-Id: <20170529144538.29187-3-mylene.josserand@free-electrons.com> > Signed-off-by: Alistair Francis <alistair@alistair23.me> > --- > .../input/touchscreen/cypress,tt21000.yaml | 92 +++++++++++++++++++ > 1 file changed, 92 insertions(+) > create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cypress,tt21000.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: ./Documentation/devicetree/bindings/input/touchscreen/cypress,tt21000.yaml: $id: relative path/filename doesn't match actual path or filename expected: http://devicetree.org/schemas/input/touchscreen/cypress,tt21000.yaml# Documentation/devicetree/bindings/input/touchscreen/cypress,tt21000.example.dts:37.26-39.19: Warning (unit_address_vs_reg): /example-0/i2c/touchscreen@24/button@0: node has a unit name, but no reg or ranges property Documentation/devicetree/bindings/input/touchscreen/cypress,tt21000.example.dts:41.26-43.19: Warning (unit_address_vs_reg): /example-0/i2c/touchscreen@24/button@1: node has a unit name, but no reg or ranges property Documentation/devicetree/bindings/input/touchscreen/cypress,tt21000.example.dts:45.26-47.19: Warning (unit_address_vs_reg): /example-0/i2c/touchscreen@24/button@2: node has a unit name, but no reg or ranges property Documentation/devicetree/bindings/input/touchscreen/cypress,tt21000.example.dt.yaml:0:0: /example-0/i2c/touchscreen@24: failed to match any schema with compatible: ['cypress,tt2100'] doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/1550218 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
Hi, I finally found time to test this. On Wed, 3 Nov 2021 21:48:28 +1000 Alistair Francis <alistair@alistair23.me> wrote: [...] > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + touchscreen@24 { > + compatible = "cypress,tt2100"; > + reg = <0x24>; > + pinctrl-names = "default"; > + pinctrl-0 = <&tp_reset_ds203>; > + interrupt-parent = <&pio>; > + interrupts = <1 5 IRQ_TYPE_LEVEL_LOW>; hmm, in the code is IRQ_TRIGGER_FALLING but here is LEVEL_LOW, hmm what it is really? > + reset-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; hmm, if the reset gpio at the chip is active low (I guess it is) that would indicate an inverter between SoC and gpio. So a nonstandard setup as an example, probably not a good idea. Regards, Andreas
On Sat, Nov 6, 2021 at 12:22 AM Andreas Kemnade <andreas@kemnade.info> wrote: > > Hi, > > I finally found time to test this. > > On Wed, 3 Nov 2021 21:48:28 +1000 > Alistair Francis <alistair@alistair23.me> wrote: > > [...] > > + i2c { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + touchscreen@24 { > > + compatible = "cypress,tt2100"; > > + reg = <0x24>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&tp_reset_ds203>; > > + interrupt-parent = <&pio>; > > + interrupts = <1 5 IRQ_TYPE_LEVEL_LOW>; > hmm, in the code is IRQ_TRIGGER_FALLING but here is LEVEL_LOW, hmm what > it is really? The reMarkable uses IRQ_TYPE_EDGE_FALLING, but this example isn't based on that. It' based on the original documentation from the patch series. > > > + reset-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; > > hmm, if the reset gpio at the chip is active low (I guess it is) that > would indicate an inverter between SoC and gpio. So a nonstandard setup > as an example, probably not a good idea. It seems to be common for the cypress,tt2100, as the original documentation and the rM2 both do this. Does the Kobo not do this? Alistair > > Regards, > Andreas
On Wed, 10 Nov 2021 22:59:50 +1000 Alistair Francis <alistair23@gmail.com> wrote: [...] > > > > > + reset-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; > > > > hmm, if the reset gpio at the chip is active low (I guess it is) that > > would indicate an inverter between SoC and gpio. So a nonstandard setup > > as an example, probably not a good idea. > > It seems to be common for the cypress,tt2100, as the original > documentation and the rM2 both do this. Does the Kobo not do this? > You have a kind of double inversion here, so things are automagically fixed. IMHO to describe it correctly would be to set GPIO_ACTIVE_LOW here and in the driver /* Reset the gpio to be in a reset state */ ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); if (IS_ERR(ts->reset_gpio)) { rc = PTR_ERR(ts->reset_gpio); dev_err(dev, "Failed to request reset gpio, error %d\n", rc); return rc; } gpiod_set_value(ts->reset_gpio, 0); That is the way how other active-low reset lines are handled. Regards, Andreas
On Wed, Nov 10, 2021 at 6:37 PM Andreas Kemnade <andreas@kemnade.info> wrote: > Alistair Francis <alistair23@gmail.com> wrote: > You have a kind of double inversion here, so things are automagically fixed. > IMHO to describe it correctly would be to set GPIO_ACTIVE_LOW here > and in the driver > > /* Reset the gpio to be in a reset state */ > ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > if (IS_ERR(ts->reset_gpio)) { > rc = PTR_ERR(ts->reset_gpio); > dev_err(dev, "Failed to request reset gpio, error %d\n", rc); > return rc; > } > gpiod_set_value(ts->reset_gpio, 0); > > That is the way how other active-low reset lines are handled. Correct. This is a source of confusion, I contemplated just changing the name of GPIOD_OUT_HIGH to GPIOD_OUT_ASSERTED etc to indicate what is going on. gpiod_set_value(ts->reset_gpio, 0) should similarly be interpreted as "de-assert this line" no matter the polarity. Yours, Linus Walleij
On Fri, Nov 12, 2021 at 1:16 AM Linus Walleij <linus.walleij@linaro.org> wrote: > > On Wed, Nov 10, 2021 at 6:37 PM Andreas Kemnade <andreas@kemnade.info> wrote: > > Alistair Francis <alistair23@gmail.com> wrote: > > > You have a kind of double inversion here, so things are automagically fixed. > > IMHO to describe it correctly would be to set GPIO_ACTIVE_LOW here > > and in the driver > > > > /* Reset the gpio to be in a reset state */ > > ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > > if (IS_ERR(ts->reset_gpio)) { > > rc = PTR_ERR(ts->reset_gpio); > > dev_err(dev, "Failed to request reset gpio, error %d\n", rc); > > return rc; > > } > > gpiod_set_value(ts->reset_gpio, 0); > > > > That is the way how other active-low reset lines are handled. > > Correct. > > This is a source of confusion, I contemplated just changing the name > of GPIOD_OUT_HIGH to GPIOD_OUT_ASSERTED etc to indicate > what is going on. > > gpiod_set_value(ts->reset_gpio, 0) should similarly be interpreted > as "de-assert this line" no matter the polarity. Thanks! I have fixed this Alistair > > Yours, > Linus Walleij
diff --git a/Documentation/devicetree/bindings/input/touchscreen/cypress,tt21000.yaml b/Documentation/devicetree/bindings/input/touchscreen/cypress,tt21000.yaml new file mode 100644 index 000000000000..ff7eca412440 --- /dev/null +++ b/Documentation/devicetree/bindings/input/touchscreen/cypress,tt21000.yaml @@ -0,0 +1,92 @@ +# SPDX-License-Identifier: GPL-2.0-only +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/input/touchscreen/cypress,cyttsp5.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Cypress TT2100 touchscreen controller + +description: The Cypress TT2100 series (also known as "CYTTSP5" after + the marketing name Cypress TrueTouch Standard Product series 5). + +maintainers: + - Alistair Francis <alistair@alistair23.me> + +allOf: + - $ref: touchscreen.yaml# + +properties: + compatible: + const: cypress,tt21000 + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + vdd-supply: + description: Regulator for voltage. + + reset-gpios: + maxItems: 1 + + linux,code: + $ref: /schemas/types.yaml#/definitions/uint32 + description: EV_ABS specific event code generated by the axis. + +patternProperties: + "^button-[0-9]+$": + type: object + properties: + linux,code: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Keycode to emit + + required: + - linux,code + + additionalProperties: false + +required: + - compatible + - reg + - interrupts + - vdd-supply + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/input/linux-event-codes.h> + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + touchscreen@24 { + compatible = "cypress,tt2100"; + reg = <0x24>; + pinctrl-names = "default"; + pinctrl-0 = <&tp_reset_ds203>; + interrupt-parent = <&pio>; + interrupts = <1 5 IRQ_TYPE_LEVEL_LOW>; + reset-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; + vdd-supply = <®_touch>; + + button@0 { + linux,code = <KEY_HOMEPAGE>; + }; + + button@1 { + linux,code = <KEY_MENU>; + }; + + button@2 { + linux,code = <KEY_BACK>; + }; + }; + }; +...