| Message ID | 20231007144052.1535417-1-anshulusr@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] dt-bindings: Add bindings for Adafruit Seesaw Gamepad | expand |
On 07/10/2023 16:40, Anshul Dalal wrote: > A simple driver for a mini gamepad that communicates over i2c, the gamepad > has bidirectional thumb stick input and six buttons. Thank you for your patch. There is something to discuss/improve. Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching. > > The gamepad chip utilizes the open framework from Adafruit called 'Seesaw' > to transmit the ADC data for the joystick and digital pin state for the > buttons. I have only implemented the functionality required to receive the > thumb stick and button state. > > Steps in reading the gamepad state over i2c: > 1. Reset the registers > 2. Set the pin mode of the pins specified by the `BUTTON_MASK` to input > `BUTTON_MASK`: A bit-map for the six digital pins internally > connected to the joystick buttons. > 3. Enable internal pullup resistors for the `BUTTON_MASK` > 4. Bulk set the pin state HIGH for `BUTTON_MASK` > 5. Poll the device for button and joystick state done by: > `seesaw_read_data(struct i2c_client *client, struct seesaw_data *data)` This describes driver, not bindings or hardware Please instead describe hardware. > > Product page: > https://www.adafruit.com/product/5743 > Arduino driver: > https://github.com/adafruit/Adafruit_Seesaw > > Tested on RPi Zero 2W How can you test bindings on RPi Zero 2W? Somehow I don't believe you, see below. > > Signed-off-by: Anshul Dalal <anshulusr@gmail.com> > --- > .../bindings/input/adafruit_seesaw.yaml | 45 +++++++++++++++++++ > 1 file changed, 45 insertions(+) > create mode 100644 Documentation/devicetree/bindings/input/adafruit_seesaw.yaml > > diff --git a/Documentation/devicetree/bindings/input/adafruit_seesaw.yaml b/Documentation/devicetree/bindings/input/adafruit_seesaw.yaml > new file mode 100644 > index 000000000000..1d00d9da637a > --- /dev/null > +++ b/Documentation/devicetree/bindings/input/adafruit_seesaw.yaml Use compatible syntax, so vendor-prefix,device-name.yaml > @@ -0,0 +1,45 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/input/adafruit_seesaw.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" Drop quotes. It does not look like you tested the bindings, at least after quick look. Please run `make dt_binding_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions). Maybe you need to update your dtschema and yamllint. > + > +title: Adafruit Mini I2C Gamepad with seesaw > + > +maintainers: > + - Anshul Dalal <anshulusr@gmail.com> > + > +description: | > + Bindings for Adafruit Mini I2C Gamepad Drop "Bindings for" > + > + +-----------------------------+ > + | ___ | > + | / \ (X) | > + | | S | __ __ (Y) (A) | > + | \___/ |ST| |SE| (B) | > + | | > + +-----------------------------+ > + > + S -> 10-bit percision bidirectional analog joystick > + ST -> Start > + SE -> Select > + X, A, B, Y -> Digital action buttons > + > + Product page: https://www.adafruit.com/product/5743 > + Arduino Driver: https://github.com/adafruit/Adafruit_Seesaw > + > +properties: > + compatible: > + const: adafruit,seesaw_gamepad > + > +required: > + - compatible > + > +additionalProperties: false > + > +examples: > + - | > + seesaw_gamepad@50 { No underscores, generic node names. Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + compatible = "adafruit,seesaw_gamepad"; > + reg = <0x50>; Second hint that you did not test it... > + }; Best regards, Krzysztof
On Sat, 07 Oct 2023 20:10:49 +0530, Anshul Dalal wrote: > A simple driver for a mini gamepad that communicates over i2c, the gamepad > has bidirectional thumb stick input and six buttons. > > The gamepad chip utilizes the open framework from Adafruit called 'Seesaw' > to transmit the ADC data for the joystick and digital pin state for the > buttons. I have only implemented the functionality required to receive the > thumb stick and button state. > > Steps in reading the gamepad state over i2c: > 1. Reset the registers > 2. Set the pin mode of the pins specified by the `BUTTON_MASK` to input > `BUTTON_MASK`: A bit-map for the six digital pins internally > connected to the joystick buttons. > 3. Enable internal pullup resistors for the `BUTTON_MASK` > 4. Bulk set the pin state HIGH for `BUTTON_MASK` > 5. Poll the device for button and joystick state done by: > `seesaw_read_data(struct i2c_client *client, struct seesaw_data *data)` > > Product page: > https://www.adafruit.com/product/5743 > Arduino driver: > https://github.com/adafruit/Adafruit_Seesaw > > Tested on RPi Zero 2W > > Signed-off-by: Anshul Dalal <anshulusr@gmail.com> > --- > .../bindings/input/adafruit_seesaw.yaml | 45 +++++++++++++++++++ > 1 file changed, 45 insertions(+) > create mode 100644 Documentation/devicetree/bindings/input/adafruit_seesaw.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: ./Documentation/devicetree/bindings/input/adafruit_seesaw.yaml:4:6: [error] string value is redundantly quoted with any quotes (quoted-strings) ./Documentation/devicetree/bindings/input/adafruit_seesaw.yaml:5:10: [error] string value is redundantly quoted with any quotes (quoted-strings) dtschema/dtc warnings/errors: Documentation/devicetree/bindings/input/adafruit_seesaw.example.dts:20.13-26: Warning (reg_format): /example-0/seesaw_gamepad@50:reg: property has invalid length (4 bytes) (#address-cells == 1, #size-cells == 1) Documentation/devicetree/bindings/input/adafruit_seesaw.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/input/adafruit_seesaw.example.dtb: Warning (pci_device_bus_num): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/input/adafruit_seesaw.example.dtb: Warning (simple_bus_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/input/adafruit_seesaw.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/input/adafruit_seesaw.example.dtb: Warning (spi_bus_reg): Failed prerequisite 'reg_format' /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/adafruit_seesaw.example.dtb: seesaw_gamepad@50: 'reg' does not match any of the regexes: 'pinctrl-[0-9]+' from schema $id: http://devicetree.org/schemas/input/adafruit_seesaw.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231007144052.1535417-1-anshulusr@gmail.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. 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 after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
diff --git a/Documentation/devicetree/bindings/input/adafruit_seesaw.yaml b/Documentation/devicetree/bindings/input/adafruit_seesaw.yaml new file mode 100644 index 000000000000..1d00d9da637a --- /dev/null +++ b/Documentation/devicetree/bindings/input/adafruit_seesaw.yaml @@ -0,0 +1,45 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/input/adafruit_seesaw.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Adafruit Mini I2C Gamepad with seesaw + +maintainers: + - Anshul Dalal <anshulusr@gmail.com> + +description: | + Bindings for Adafruit Mini I2C Gamepad + + +-----------------------------+ + | ___ | + | / \ (X) | + | | S | __ __ (Y) (A) | + | \___/ |ST| |SE| (B) | + | | + +-----------------------------+ + + S -> 10-bit percision bidirectional analog joystick + ST -> Start + SE -> Select + X, A, B, Y -> Digital action buttons + + Product page: https://www.adafruit.com/product/5743 + Arduino Driver: https://github.com/adafruit/Adafruit_Seesaw + +properties: + compatible: + const: adafruit,seesaw_gamepad + +required: + - compatible + +additionalProperties: false + +examples: + - | + seesaw_gamepad@50 { + compatible = "adafruit,seesaw_gamepad"; + reg = <0x50>; + };
A simple driver for a mini gamepad that communicates over i2c, the gamepad has bidirectional thumb stick input and six buttons. The gamepad chip utilizes the open framework from Adafruit called 'Seesaw' to transmit the ADC data for the joystick and digital pin state for the buttons. I have only implemented the functionality required to receive the thumb stick and button state. Steps in reading the gamepad state over i2c: 1. Reset the registers 2. Set the pin mode of the pins specified by the `BUTTON_MASK` to input `BUTTON_MASK`: A bit-map for the six digital pins internally connected to the joystick buttons. 3. Enable internal pullup resistors for the `BUTTON_MASK` 4. Bulk set the pin state HIGH for `BUTTON_MASK` 5. Poll the device for button and joystick state done by: `seesaw_read_data(struct i2c_client *client, struct seesaw_data *data)` Product page: https://www.adafruit.com/product/5743 Arduino driver: https://github.com/adafruit/Adafruit_Seesaw Tested on RPi Zero 2W Signed-off-by: Anshul Dalal <anshulusr@gmail.com> --- .../bindings/input/adafruit_seesaw.yaml | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 Documentation/devicetree/bindings/input/adafruit_seesaw.yaml