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

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>
---

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

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.

> ---
> 
> 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

Hi Anshul,

Thank you for this additional information.

On Mon, Oct 23, 2023 at 05:28:10PM +0530, Anshul Dalal wrote:
> 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.

The underlying functions used to implement I2C communication are orthogonal
to the binding. Whether you use the kernel's core I2C support, regmap, or
your own wrappers built on top of either have no bearing on whether or not
a binding is necessary.

The binding is used to define device tree properties that describe the
hardware and its constraints. Classic examples are things such as clock
frequency, regulator voltage, etc. Drivers often translate device tree
properties into register settings.

In the case of this device, the only thing the driver needs to know about
the hardware are its compatible string and I2C client address, both of
which are already supported in the common trivial devices binding [1].

> 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.

The trivial devices binding includes interrupts as well; please see [1].
My opinion is that the device's own documentation is responsible for
describing the product and anything unique about its physical layout.

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

I don't see any need for a binding for this device because it has no vendor-
specific properties, and the only properties it does have are covered by
existing infrastructure.

My feedback is that this patch can be replaced with at most a two-line patch
to [1]. This is just my $.02; it is ultimately up to the maintainers. The
existing binding, albeit unnecessary, is nicely written either way :)

Kind regards,
Jeff LaBundy

[1] Documentation/devicetree/bindings/trivial-devices.yaml