2023-10-10 18:50:17

by Anshul Dalal

[permalink] [raw]
Subject: [PATCH v4 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

Signed-off-by: Anshul Dalal <[email protected]>
---

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 | 59 +++++++++++++++++++
1 file changed, 59 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..e8e676006d2f
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
@@ -0,0 +1,59 @@
+# 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 <[email protected]>
+
+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
+
+ 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


2023-10-10 18:50:22

by Anshul Dalal

[permalink] [raw]
Subject: [PATCH v4 2/2] input: joystick: driver for Adafruit Seesaw Gamepad

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

Driver tested on RPi Zero 2W

Signed-off-by: Anshul Dalal <[email protected]>
---

Changes for v4:
- Changed `1UL << BUTTON_` to BIT(BUTTON_)
- Removed `hardware_id` field from `struct seesaw_gamepad`
- Removed redundant checks for the number of bytes written and received by
`i2c_master_send` and `i2c_master_recv`
- Used `get_unaligned_be32` to instantiate `u32 result` from `read_buf`
- Changed `result & (1UL << BUTTON_)` to
`test_bit(BUTTON_, (long *)&result)`
- Changed `KBUILD_MODNAME` in id-tables to `SEESAW_DEVICE_NAME`
- Fixed formatting issues
- Changed button reporting:
Since the gamepad had the action buttons in a non-standard layout:
(X)
(Y) (A)
(B)
Therefore moved to using generic directional action button event codes
instead of BTN_[ABXY].

Changes for v3:
- no updates

Changes for v2:
adafruit-seesaw.c:
- Renamed file from 'adafruit_seesaw.c'
- Changed device name from 'seesaw_gamepad' to 'seesaw-gamepad'
- Changed count parameter for receiving joystick x on line 118:
`2` to `sizeof(write_buf)`
- Fixed invalid buffer size on line 123 and 126:
`data->y` to `sizeof(data->y)`
- Added comment for the `mdelay(10)` on line 169
- Changed inconsistent indentation on line 271
Kconfig:
- Fixed indentation for the help text
- Updated module name
Makefile:
- Updated module object file name
MAINTAINERS:
- Updated file name for the driver and bindings

MAINTAINERS | 7 +
drivers/input/joystick/Kconfig | 9 +
drivers/input/joystick/Makefile | 1 +
drivers/input/joystick/adafruit-seesaw.c | 269 +++++++++++++++++++++++
4 files changed, 286 insertions(+)
create mode 100644 drivers/input/joystick/adafruit-seesaw.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 6c4cce45a09d..a314f9b48e21 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -441,6 +441,13 @@ W: http://wiki.analog.com/AD7879
W: https://ez.analog.com/linux-software-drivers
F: drivers/input/touchscreen/ad7879.c

+ADAFRUIT MINI I2C GAMEPAD
+M: Anshul Dalal <[email protected]>
+L: [email protected]
+S: Maintained
+F: Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
+F: drivers/input/joystick/adafruit-seesaw.c
+
ADDRESS SPACE LAYOUT RANDOMIZATION (ASLR)
M: Jiri Kosina <[email protected]>
S: Maintained
diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
index ac6925ce8366..df9cd1830b29 100644
--- a/drivers/input/joystick/Kconfig
+++ b/drivers/input/joystick/Kconfig
@@ -412,4 +412,13 @@ config JOYSTICK_SENSEHAT
To compile this driver as a module, choose M here: the
module will be called sensehat_joystick.

+config JOYSTICK_SEESAW
+ tristate "Adafruit Mini I2C Gamepad with Seesaw"
+ depends on I2C
+ help
+ Say Y here if you want to use the Adafruit Mini I2C Gamepad.
+
+ To compile this driver as a module, choose M here: the module will be
+ called adafruit-seesaw.
+
endif
diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
index 3937535f0098..9976f596a920 100644
--- a/drivers/input/joystick/Makefile
+++ b/drivers/input/joystick/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_JOYSTICK_N64) += n64joy.o
obj-$(CONFIG_JOYSTICK_PSXPAD_SPI) += psxpad-spi.o
obj-$(CONFIG_JOYSTICK_PXRC) += pxrc.o
obj-$(CONFIG_JOYSTICK_QWIIC) += qwiic-joystick.o
+obj-$(CONFIG_JOYSTICK_SEESAW) += adafruit-seesaw.o
obj-$(CONFIG_JOYSTICK_SENSEHAT) += sensehat-joystick.o
obj-$(CONFIG_JOYSTICK_SIDEWINDER) += sidewinder.o
obj-$(CONFIG_JOYSTICK_SPACEBALL) += spaceball.o
diff --git a/drivers/input/joystick/adafruit-seesaw.c b/drivers/input/joystick/adafruit-seesaw.c
new file mode 100644
index 000000000000..5cf10deb2372
--- /dev/null
+++ b/drivers/input/joystick/adafruit-seesaw.c
@@ -0,0 +1,269 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2023 Anshul Dalal <[email protected]>
+ *
+ * Driver for Adafruit Mini I2C Gamepad
+ *
+ * Based on the work of:
+ * Oleh Kravchenko (Sparkfun Qwiic Joystick driver)
+ *
+ * Product page: https://www.adafruit.com/product/5743
+ * Firmware and hardware sources: https://github.com/adafruit/Adafruit_Seesaw
+ */
+
+#include <asm-generic/unaligned.h>
+#include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+/* clang-format off */
+#define SEESAW_DEVICE_NAME "seesaw-gamepad"
+
+#define SEESAW_STATUS_BASE 0
+#define SEESAW_GPIO_BASE 1
+#define SEESAW_ADC_BASE 9
+
+#define SEESAW_GPIO_DIRCLR_BULK 3
+#define SEESAW_GPIO_BULK 4
+#define SEESAW_GPIO_BULK_SET 5
+#define SEESAW_GPIO_PULLENSET 11
+
+#define SEESAW_STATUS_HW_ID 1
+#define SEESAW_STATUS_SWRST 127
+
+#define SEESAW_ADC_OFFSET 7
+
+#define BUTTON_A 5
+#define BUTTON_B 1
+#define BUTTON_X 6
+#define BUTTON_Y 2
+#define BUTTON_START 16
+#define BUTTON_SELECT 0
+
+#define ANALOG_X 14
+#define ANALOG_Y 15
+
+#define SEESAW_JOYSTICK_MAX_AXIS 1023
+#define SEESAW_JOYSTICK_FUZZ 2
+#define SEESAW_JOYSTICK_FLAT 4
+
+#define SEESAW_GAMEPAD_POLL_INTERVAL 16
+#define SEESAW_GAMEPAD_POLL_MIN 8
+#define SEESAW_GAMEPAD_POLL_MAX 32
+/* clang-format on */
+
+u32 BUTTON_MASK = BIT(BUTTON_A) | BIT(BUTTON_B) | BIT(BUTTON_X) |
+ BIT(BUTTON_Y) | BIT(BUTTON_START) | BIT(BUTTON_SELECT);
+
+struct seesaw_gamepad {
+ char physical_path[32];
+ struct input_dev *input_dev;
+ struct i2c_client *i2c_client;
+};
+
+struct seesaw_data {
+ __be16 x;
+ __be16 y;
+ u8 button_a, button_b, button_x, button_y, button_start, button_select;
+};
+
+static int seesaw_read_data(struct i2c_client *client, struct seesaw_data *data)
+{
+ int err;
+ unsigned char write_buf[2] = { SEESAW_GPIO_BASE, SEESAW_GPIO_BULK };
+ unsigned char read_buf[4];
+
+ err = i2c_master_send(client, write_buf, sizeof(write_buf));
+ if (err < 0)
+ return err;
+ err = i2c_master_recv(client, read_buf, sizeof(read_buf));
+ if (err < 0)
+ return err;
+
+ u32 result = get_unaligned_be32(&read_buf);
+
+ data->button_a = !test_bit(BUTTON_A, (long *)&result);
+ data->button_b = !test_bit(BUTTON_B, (long *)&result);
+ data->button_x = !test_bit(BUTTON_X, (long *)&result);
+ data->button_y = !test_bit(BUTTON_Y, (long *)&result);
+ data->button_start = !test_bit(BUTTON_START, (long *)&result);
+ data->button_select = !test_bit(BUTTON_SELECT, (long *)&result);
+
+ write_buf[0] = SEESAW_ADC_BASE;
+ write_buf[1] = SEESAW_ADC_OFFSET + ANALOG_X;
+ err = i2c_master_send(client, write_buf, sizeof(write_buf));
+ if (err < 0)
+ return err;
+ err = i2c_master_recv(client, (char *)&data->x, sizeof(data->x));
+ if (err < 0)
+ return err;
+ /*
+ * ADC reads left as max and right as 0, must be reversed since kernel
+ * expects reports in opposite order.
+ */
+ data->x = SEESAW_JOYSTICK_MAX_AXIS - be16_to_cpu(data->x);
+
+ write_buf[1] = SEESAW_ADC_OFFSET + ANALOG_Y;
+ err = i2c_master_send(client, write_buf, sizeof(write_buf));
+ if (err < 0)
+ return err;
+ err = i2c_master_recv(client, (char *)&data->y, sizeof(data->y));
+ if (err < 0)
+ return err;
+ data->y = be16_to_cpu(data->y);
+
+ return 0;
+}
+
+static void seesaw_poll(struct input_dev *input)
+{
+ struct seesaw_gamepad *private = input_get_drvdata(input);
+ struct seesaw_data data;
+ int err;
+
+ err = seesaw_read_data(private->i2c_client, &data);
+ if (err != 0)
+ return;
+
+ input_report_abs(input, ABS_X, data.x);
+ input_report_abs(input, ABS_Y, data.y);
+ input_report_key(input, BTN_EAST, data.button_a);
+ input_report_key(input, BTN_SOUTH, data.button_b);
+ input_report_key(input, BTN_NORTH, data.button_x);
+ input_report_key(input, BTN_WEST, data.button_y);
+ input_report_key(input, BTN_START, data.button_start);
+ input_report_key(input, BTN_SELECT, data.button_select);
+ input_sync(input);
+}
+
+static int seesaw_probe(struct i2c_client *client)
+{
+ int err;
+ struct seesaw_gamepad *private;
+ unsigned char register_reset[] = { SEESAW_STATUS_BASE,
+ SEESAW_STATUS_SWRST, 0xFF };
+ unsigned char get_hw_id[] = { SEESAW_STATUS_BASE, SEESAW_STATUS_HW_ID };
+
+ err = i2c_master_send(client, register_reset, sizeof(register_reset));
+ if (err < 0)
+ return err;
+
+ /* Wait for the registers to reset before proceeding */
+ mdelay(10);
+
+ private = devm_kzalloc(&client->dev, sizeof(*private), GFP_KERNEL);
+ if (!private)
+ return -ENOMEM;
+
+ err = i2c_master_send(client, get_hw_id, sizeof(get_hw_id));
+ if (err < 0)
+ return err;
+
+ unsigned char hardware_id;
+
+ err = i2c_master_recv(client, &hardware_id, 1);
+ if (err < 0)
+ return err;
+
+ dev_dbg(&client->dev, "Adafruit Seesaw Gamepad, Hardware ID: %02x\n",
+ hardware_id);
+
+ private->i2c_client = client;
+ scnprintf(private->physical_path, sizeof(private->physical_path),
+ "i2c/%s", dev_name(&client->dev));
+ i2c_set_clientdata(client, private);
+
+ private->input_dev = devm_input_allocate_device(&client->dev);
+ if (!private->input_dev)
+ return -ENOMEM;
+
+ private->input_dev->id.bustype = BUS_I2C;
+ private->input_dev->name = "Adafruit Seesaw Gamepad";
+ private->input_dev->phys = private->physical_path;
+ input_set_drvdata(private->input_dev, private);
+ input_set_abs_params(private->input_dev, ABS_X, 0,
+ SEESAW_JOYSTICK_MAX_AXIS, SEESAW_JOYSTICK_FUZZ,
+ SEESAW_JOYSTICK_FLAT);
+ input_set_abs_params(private->input_dev, ABS_Y, 0,
+ SEESAW_JOYSTICK_MAX_AXIS, SEESAW_JOYSTICK_FUZZ,
+ SEESAW_JOYSTICK_FLAT);
+ input_set_capability(private->input_dev, EV_KEY, BTN_EAST);
+ input_set_capability(private->input_dev, EV_KEY, BTN_SOUTH);
+ input_set_capability(private->input_dev, EV_KEY, BTN_NORTH);
+ input_set_capability(private->input_dev, EV_KEY, BTN_WEST);
+ input_set_capability(private->input_dev, EV_KEY, BTN_START);
+ input_set_capability(private->input_dev, EV_KEY, BTN_SELECT);
+
+ err = input_setup_polling(private->input_dev, seesaw_poll);
+ if (err) {
+ dev_err(&client->dev, "failed to set up polling: %d\n", err);
+ return err;
+ }
+
+ input_set_poll_interval(private->input_dev,
+ SEESAW_GAMEPAD_POLL_INTERVAL);
+ input_set_max_poll_interval(private->input_dev,
+ SEESAW_GAMEPAD_POLL_MAX);
+ input_set_min_poll_interval(private->input_dev,
+ SEESAW_GAMEPAD_POLL_MIN);
+
+ err = input_register_device(private->input_dev);
+ if (err) {
+ dev_err(&client->dev, "failed to register joystick: %d\n", err);
+ return err;
+ }
+
+ /* Set Pin Mode to input and enable pull-up resistors */
+ unsigned char pin_mode[] = { SEESAW_GPIO_BASE, SEESAW_GPIO_DIRCLR_BULK,
+ BUTTON_MASK >> 24, BUTTON_MASK >> 16,
+ BUTTON_MASK >> 8, BUTTON_MASK };
+ err = i2c_master_send(client, pin_mode, sizeof(pin_mode));
+ if (err < 0)
+ return err;
+ pin_mode[1] = SEESAW_GPIO_PULLENSET;
+ err = i2c_master_send(client, pin_mode, sizeof(pin_mode));
+ if (err < 0)
+ return err;
+ pin_mode[1] = SEESAW_GPIO_BULK_SET;
+ err = i2c_master_send(client, pin_mode, sizeof(pin_mode));
+ if (err < 0)
+ return err;
+
+ return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id of_seesaw_match[] = {
+ {
+ .compatible = "adafruit,seesaw-gamepad",
+ },
+ { /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, of_seesaw_match);
+#endif /* CONFIG_OF */
+
+/* clang-format off */
+static const struct i2c_device_id seesaw_id_table[] = {
+ { SEESAW_DEVICE_NAME, 0 },
+ { /* Sentinel */ }
+};
+/* clang-format on */
+
+MODULE_DEVICE_TABLE(i2c, seesaw_id_table);
+
+static struct i2c_driver seesaw_driver = {
+ .driver = {
+ .name = SEESAW_DEVICE_NAME,
+ .of_match_table = of_match_ptr(of_seesaw_match),
+ },
+ .id_table = seesaw_id_table,
+ .probe = seesaw_probe,
+};
+module_i2c_driver(seesaw_driver);
+
+MODULE_AUTHOR("Anshul Dalal <[email protected]>");
+MODULE_DESCRIPTION("Adafruit Mini I2C Gamepad driver");
+MODULE_LICENSE("GPL");
--
2.42.0

2023-10-10 19:04:30

by Thomas Weißschuh

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] input: joystick: driver for Adafruit Seesaw Gamepad

On 2023-10-11 00:18:24+0530, Anshul Dalal wrote:
> Adds a 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
>
> Driver tested on RPi Zero 2W
>
> Signed-off-by: Anshul Dalal <[email protected]>

In general:

Reviewed-by: Thomas Weißschuh <[email protected]>

But one more nitpick below:

> [..]

> +static void seesaw_poll(struct input_dev *input)
> +{
> + struct seesaw_gamepad *private = input_get_drvdata(input);
> + struct seesaw_data data;
> + int err;
> +
> + err = seesaw_read_data(private->i2c_client, &data);
> + if (err != 0)
> + return;

This should be logged at debug level, so the user has some sort of
chance to figure out if something is broken.

> +
> + input_report_abs(input, ABS_X, data.x);
> + input_report_abs(input, ABS_Y, data.y);
> + input_report_key(input, BTN_EAST, data.button_a);
> + input_report_key(input, BTN_SOUTH, data.button_b);
> + input_report_key(input, BTN_NORTH, data.button_x);
> + input_report_key(input, BTN_WEST, data.button_y);
> + input_report_key(input, BTN_START, data.button_start);
> + input_report_key(input, BTN_SELECT, data.button_select);
> + input_sync(input);
> +}

> [..]

2023-10-11 16:15:29

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] dt-bindings: input: bindings for Adafruit Seesaw Gamepad

Hey,

On Wed, Oct 11, 2023 at 12:18:23AM +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
>
> Signed-off-by: Anshul Dalal <[email protected]>
> ---
>
> 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 | 59 +++++++++++++++++++
> 1 file changed, 59 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..e8e676006d2f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
> @@ -0,0 +1,59 @@
> +# 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

Binding mostly looks good to me. My main question is what is a seesaw?

> +
> +maintainers:
> + - Anshul Dalal <[email protected]>
> +
> +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
> +
> + Product page: https://www.adafruit.com/product/5743
> + Arduino Driver: https://github.com/adafruit/Adafruit_Seesaw

I'm not really sure what the arduino driver has to do with the binding.
Why is a link to it more relevant than the freebsd driver, or the linux
driver etc? Is there info about how the pad works in the arduino driver

Otherwise, this seems good to me.

Thanks,
Conor.

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


Attachments:
(No filename) (3.45 kB)
signature.asc (235.00 B)
Download all attachments

2023-10-11 19:01:20

by Anshul Dalal

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] dt-bindings: input: bindings for Adafruit Seesaw Gamepad

Hello,

On 10/11/23 21:45, Conor Dooley wrote:
> Hey,
>
> On Wed, Oct 11, 2023 at 12:18:23AM +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
>>
>> Signed-off-by: Anshul Dalal <[email protected]>
>> ---
>>
>> 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 | 59 +++++++++++++++++++
>> 1 file changed, 59 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..e8e676006d2f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
>> @@ -0,0 +1,59 @@
>> +# 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
>
> Binding mostly looks good to me. My main question is what is a seesaw?
>

Seesaw is a universal framework that enables extending I/O capabilities
of the i2c master devices with a compatible breakout board. As it
relates to the binding, this gamepad uses an AVR ATtiny816
microcontroller that reads the data from the buttons and the joystick
and sends the data to the master over i2c using the Seesaw framework.

>> +
>> +maintainers:
>> + - Anshul Dalal <[email protected]>
>> +
>> +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
>> +
>> + Product page: https://www.adafruit.com/product/5743
>> + Arduino Driver: https://github.com/adafruit/Adafruit_Seesaw
>
> I'm not really sure what the arduino driver has to do with the binding.
> Why is a link to it more relevant than the freebsd driver, or the linux
> driver etc? Is there info about how the pad works in the arduino driver
>
> Otherwise, this seems good to me.
>
> Thanks,
> Conor.

The Arduino driver I linked was the only resource that had the
implementation of the seesaw framework as well as the example code
specific to this device:
https://github.com/adafruit/Adafruit_Seesaw/tree/master/examples/Mini_I2C_Gamepad_QT
On further thought, a link to the accompanying document from the
manufacturer (https://cdn-learn.adafruit.com/downloads/pdf/gamepad-qt.pdf)
might be more relevant for the binding which includes the hardware
description as well as links to the above-mentioned Arduino driver.

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

Thanks for the review.

Best Regards,
Anshul

2023-10-12 08:39:33

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] dt-bindings: input: bindings for Adafruit Seesaw Gamepad

On 10/10/2023 20:48, 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
>
> Signed-off-by: Anshul Dalal <[email protected]>
> ---
>
> 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 | 59 +++++++++++++++++++
> 1 file changed, 59 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..e8e676006d2f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
> @@ -0,0 +1,59 @@
> +# 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 <[email protected]>
> +
> +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
> +
> + Product page: https://www.adafruit.com/product/5743
> + Arduino Driver: https://github.com/adafruit/Adafruit_Seesaw
> +
> +properties:
> + compatible:
> + const: adafruit,seesaw-gamepad

I thought seesaw is a name of a device, but it is not, thus it is quite
a generic compatible. Are you sure device does not have its name?
Looking at product page, it indeed might not have.


Best regards,
Krzysztof

2023-10-12 10:56:09

by Anshul Dalal

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] dt-bindings: input: bindings for Adafruit Seesaw Gamepad

On 10/12/23 14:09, Krzysztof Kozlowski wrote:
> On 10/10/2023 20:48, 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
>>
>> Signed-off-by: Anshul Dalal <[email protected]>
>> ---
>>
>> 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 | 59 +++++++++++++++++++
>> 1 file changed, 59 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..e8e676006d2f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
>> @@ -0,0 +1,59 @@
>> +# 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 <[email protected]>
>> +
>> +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
>> +
>> + Product page: https://www.adafruit.com/product/5743
>> + Arduino Driver: https://github.com/adafruit/Adafruit_Seesaw
>> +
>> +properties:
>> + compatible:
>> + const: adafruit,seesaw-gamepad
>
> I thought seesaw is a name of a device, but it is not, thus it is quite
> a generic compatible. Are you sure device does not have its name?
> Looking at product page, it indeed might not have.
>

I chose the title from the datasheet:
https://cdn-learn.adafruit.com/downloads/pdf/gamepad-qt.pdf
It lists the product name as "Adafruit Mini I2C STEMMA QT Gamepad with
seesaw" with STEMMA QT being the name of the 4-pin i2c connector that
Adafruit uses on their products. That's the most specific name for the
product I could find.

>
> Best regards,
> Krzysztof
>

Thanks,
Anshul

2023-10-12 12:36:47

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] dt-bindings: input: bindings for Adafruit Seesaw Gamepad

On 10/10/2023 20:48, 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
>
> Signed-off-by: Anshul Dalal <[email protected]>
> ---
>

Reviewed-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof

2023-10-12 15:47:44

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] dt-bindings: input: bindings for Adafruit Seesaw Gamepad

On Thu, Oct 12, 2023 at 12:29:58AM +0530, Anshul Dalal wrote:
> Hello,
>
> On 10/11/23 21:45, Conor Dooley wrote:
> > Hey,
> >
> > On Wed, Oct 11, 2023 at 12:18:23AM +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
> >>
> >> Signed-off-by: Anshul Dalal <[email protected]>
> >> ---
> >>
> >> 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 | 59 +++++++++++++++++++
> >> 1 file changed, 59 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..e8e676006d2f
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
> >> @@ -0,0 +1,59 @@
> >> +# 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
> >
> > Binding mostly looks good to me. My main question is what is a seesaw?
> >
>
> Seesaw is a universal framework that enables extending I/O capabilities
> of the i2c master devices with a compatible breakout board. As it
> relates to the binding, this gamepad uses an AVR ATtiny816
> microcontroller that reads the data from the buttons and the joystick
> and sends the data to the master over i2c using the Seesaw framework.

Right. Not a framework I was aware of, thanks for explaining.

>
> >> +
> >> +maintainers:
> >> + - Anshul Dalal <[email protected]>
> >> +
> >> +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
> >> +
> >> + Product page: https://www.adafruit.com/product/5743
> >> + Arduino Driver: https://github.com/adafruit/Adafruit_Seesaw
> >
> > I'm not really sure what the arduino driver has to do with the binding.
> > Why is a link to it more relevant than the freebsd driver, or the linux
> > driver etc? Is there info about how the pad works in the arduino driver
> >
> > Otherwise, this seems good to me.

> The Arduino driver I linked was the only resource that had the
> implementation of the seesaw framework as well as the example code
> specific to this device:
> https://github.com/adafruit/Adafruit_Seesaw/tree/master/examples/Mini_I2C_Gamepad_QT
> On further thought, a link to the accompanying document from the
> manufacturer (https://cdn-learn.adafruit.com/downloads/pdf/gamepad-qt.pdf)
> might be more relevant for the binding which includes the hardware
> description as well as links to the above-mentioned Arduino driver.

A link to the manufacturer docs would be nice & if, as you say, the
arduino driver was a useful resource for understanding the hardware then
I suppose it can stay too :)

Reviewed-by: Conor Dooley <[email protected]>

Thanks,
Conor.


Attachments:
(No filename) (4.37 kB)
signature.asc (235.00 B)
Download all attachments