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@xxxxxxxxx> > --- > .../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@xxxxxxxxx> > + > +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
