Re: [PATCH v5 1/2] dt-bindings: input: bindings for Adafruit Seesaw Gamepad

Anshul Dalal
2023-10-23T11:59:22Z

Hello Jeff,

On 10/23/23 05:17, Jeff LaBundy wrote:
> Hi Anshul,
>
> On Tue, Oct 17, 2023 at 09:13:44AM +0530, Anshul Dalal wrote:
>> Adds bindings for the Adafruit Seesaw Gamepad.
>>
>> The gamepad functions as an i2c device with the default address of 0x50
>> and has an IRQ pin that can be enabled in the driver to allow for a rising
>> edge trigger on each button press or joystick movement.
>>
>> Product page:
>> https://www.adafruit.com/product/5743
>> Arduino driver:
>> https://github.com/adafruit/Adafruit_Seesaw
>>
>> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
>
> Perhaps this ship has sailed, but is there any reason this simple device
> cannot be added to Documentation/devicetree/bindings/trivial-devices.yaml
> as opposed to having its own binding?
>
> It has no vendor-specific properties, and the only properties are the
> standard properties already understood by the I2C core. In case I have
> misunderstood, please let me know.
>

The driver currently implements only a subset of the functionality in
the Adafruit Seesaw specification. I eventually plan on adding adding
full support for the Seesaw framework in the form of a driver for the
atsamd09 seesaw breakout board:
https://learn.adafruit.com/adafruit-seesaw-atsamd09-breakout

Then I think it would be better for this driver to use the newly exposed
seesaw APIs by the atsamd09 driver instead of relying on kernel's i2c APIs.

I would also like to add support for the provided interrupt pin later
down the line which is documented in the binding along with description
of the non-standard action button layout.

Above were my reasons for going for a standalone binding, please let me
know if you disagree.

>> ---
>>
>> Changes for v5:
>> - Added link to the datasheet
>>
>> Changes for v4:
>> - Fixed the URI for the id field
>> - Added `interrupts` property
>>
>> Changes for v3:
>> - Updated id field to reflect updated file name from previous version
>> - Added `reg` property
>>
>> Changes for v2:
>> - Renamed file to `adafruit,seesaw-gamepad.yaml`
>> - Removed quotes for `$id` and `$schema`
>> - Removed "Bindings for" from the description
>> - Changed node name to the generic name "joystick"
>> - Changed compatible to 'adafruit,seesaw-gamepad' instead of
>> 'adafruit,seesaw_gamepad'
>>
>> .../input/adafruit,seesaw-gamepad.yaml | 60 +++++++++++++++++++
>> 1 file changed, 60 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
>> new file mode 100644
>> index 000000000000..3f0d1c5a3b9b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
>> @@ -0,0 +1,60 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/input/adafruit,seesaw-gamepad.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Adafruit Mini I2C Gamepad with seesaw
>> +
>> +maintainers:
>> + - Anshul Dalal <anshulusr@gmail.com>
>> +
>> +description: |
>> + 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
>> +
>> + Datasheet: https://cdn-learn.adafruit.com/downloads/pdf/gamepad-qt.pdf
>> + Product page: https://www.adafruit.com/product/5743
>> + Arduino Driver: https://github.com/adafruit/Adafruit_Seesaw
>> +
>> +properties:
>> + compatible:
>> + const: adafruit,seesaw-gamepad
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + interrupts:
>> + maxItems: 1
>> + description:
>> + The gamepad's IRQ pin triggers a rising edge if interrupts are enabled.
>> +
>> +required:
>> + - compatible
>> + - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + i2c {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + joystick@50 {
>> + compatible = "adafruit,seesaw-gamepad";
>> + reg = <0x50>;
>> + };
>> + };
>> --
>> 2.42.0
>>
>
> Kind regards,
> Jeff LaBundy

Thank you,
Anshul Dalal