Message ID | 20231017034356.1436677-2-anshulusr@gmail.com |
---|---|
State | New |
Headers | show |
Series | [v5,1/2] dt-bindings: input: bindings for Adafruit Seesaw Gamepad | expand |
Hi Anshul, kernel test robot noticed the following build warnings: [auto build test WARNING on dtor-input/next] [also build test WARNING on dtor-input/for-linus hid/for-next linus/master v6.6-rc6 next-20231018] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Anshul-Dalal/input-joystick-driver-for-Adafruit-Seesaw-Gamepad/20231017-160635 base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next patch link: https://lore.kernel.org/r/20231017034356.1436677-2-anshulusr%40gmail.com patch subject: [PATCH v5 2/2] input: joystick: driver for Adafruit Seesaw Gamepad config: sparc-allyesconfig (https://download.01.org/0day-ci/archive/20231019/202310190852.BCw4Ry7D-lkp@intel.com/config) compiler: sparc64-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231019/202310190852.BCw4Ry7D-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202310190852.BCw4Ry7D-lkp@intel.com/ All warnings (new ones prefixed by >>): In file included from include/linux/thread_info.h:27, from arch/sparc/include/asm/current.h:15, from include/linux/sched.h:12, from include/linux/delay.h:23, from drivers/input/joystick/adafruit-seesaw.c:17: drivers/input/joystick/adafruit-seesaw.c: In function 'seesaw_read_data': >> include/linux/bitops.h:52:11: warning: array subscript 'long unsigned int[0]' is partly outside array bounds of 'u32[1]' {aka 'unsigned int[1]'} [-Warray-bounds=] 52 | __builtin_constant_p(*(const unsigned long *)(addr))) ? \ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/bitops.h:61:41: note: in expansion of macro 'bitop' 61 | #define test_bit(nr, addr) bitop(_test_bit, nr, addr) | ^~~~~ drivers/input/joystick/adafruit-seesaw.c:89:27: note: in expansion of macro 'test_bit' 89 | data->button_a = !test_bit(BUTTON_A, (long *)&result); | ^~~~~~~~ drivers/input/joystick/adafruit-seesaw.c:87:13: note: object 'result' of size 4 87 | u32 result = get_unaligned_be32(&read_buf); | ^~~~~~ In file included from include/linux/bitops.h:34: In function 'generic_test_bit', inlined from 'seesaw_read_data' at drivers/input/joystick/adafruit-seesaw.c:89:20: >> include/asm-generic/bitops/generic-non-atomic.h:128:27: warning: array subscript 'long unsigned int[0]' is partly outside array bounds of 'u32[1]' {aka 'unsigned int[1]'} [-Warray-bounds=] 128 | return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1))); | ~~~~^~~~~~~~~~~~~~ drivers/input/joystick/adafruit-seesaw.c: In function 'seesaw_read_data': drivers/input/joystick/adafruit-seesaw.c:87:13: note: object 'result' of size 4 87 | u32 result = get_unaligned_be32(&read_buf); | ^~~~~~ In function 'generic_test_bit', inlined from 'seesaw_read_data' at drivers/input/joystick/adafruit-seesaw.c:90:20: >> include/asm-generic/bitops/generic-non-atomic.h:128:27: warning: array subscript 'long unsigned int[0]' is partly outside array bounds of 'u32[1]' {aka 'unsigned int[1]'} [-Warray-bounds=] 128 | return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1))); | ~~~~^~~~~~~~~~~~~~ drivers/input/joystick/adafruit-seesaw.c: In function 'seesaw_read_data': drivers/input/joystick/adafruit-seesaw.c:87:13: note: object 'result' of size 4 87 | u32 result = get_unaligned_be32(&read_buf); | ^~~~~~ In function 'generic_test_bit', inlined from 'seesaw_read_data' at drivers/input/joystick/adafruit-seesaw.c:91:20: >> include/asm-generic/bitops/generic-non-atomic.h:128:27: warning: array subscript 'long unsigned int[0]' is partly outside array bounds of 'u32[1]' {aka 'unsigned int[1]'} [-Warray-bounds=] 128 | return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1))); | ~~~~^~~~~~~~~~~~~~ drivers/input/joystick/adafruit-seesaw.c: In function 'seesaw_read_data': drivers/input/joystick/adafruit-seesaw.c:87:13: note: object 'result' of size 4 87 | u32 result = get_unaligned_be32(&read_buf); | ^~~~~~ In function 'generic_test_bit', inlined from 'seesaw_read_data' at drivers/input/joystick/adafruit-seesaw.c:92:20: >> include/asm-generic/bitops/generic-non-atomic.h:128:27: warning: array subscript 'long unsigned int[0]' is partly outside array bounds of 'u32[1]' {aka 'unsigned int[1]'} [-Warray-bounds=] 128 | return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1))); | ~~~~^~~~~~~~~~~~~~ drivers/input/joystick/adafruit-seesaw.c: In function 'seesaw_read_data': drivers/input/joystick/adafruit-seesaw.c:87:13: note: object 'result' of size 4 87 | u32 result = get_unaligned_be32(&read_buf); | ^~~~~~ In function 'generic_test_bit', inlined from 'seesaw_read_data' at drivers/input/joystick/adafruit-seesaw.c:93:24: >> include/asm-generic/bitops/generic-non-atomic.h:128:27: warning: array subscript 'long unsigned int[0]' is partly outside array bounds of 'u32[1]' {aka 'unsigned int[1]'} [-Warray-bounds=] 128 | return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1))); | ~~~~^~~~~~~~~~~~~~ drivers/input/joystick/adafruit-seesaw.c: In function 'seesaw_read_data': drivers/input/joystick/adafruit-seesaw.c:87:13: note: object 'result' of size 4 87 | u32 result = get_unaligned_be32(&read_buf); | ^~~~~~ In function 'generic_test_bit', inlined from 'seesaw_read_data' at drivers/input/joystick/adafruit-seesaw.c:94:25: >> include/asm-generic/bitops/generic-non-atomic.h:128:27: warning: array subscript 'long unsigned int[0]' is partly outside array bounds of 'u32[1]' {aka 'unsigned int[1]'} [-Warray-bounds=] 128 | return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1))); | ~~~~^~~~~~~~~~~~~~ drivers/input/joystick/adafruit-seesaw.c: In function 'seesaw_read_data': drivers/input/joystick/adafruit-seesaw.c:87:13: note: object 'result' of size 4 87 | u32 result = get_unaligned_be32(&read_buf); | ^~~~~~ vim +52 include/linux/bitops.h 0e862838f29014 Alexander Lobakin 2022-06-24 35 b03fc1173c0c2b Alexander Lobakin 2022-06-24 36 /* b03fc1173c0c2b Alexander Lobakin 2022-06-24 37 * Many architecture-specific non-atomic bitops contain inline asm code and due b03fc1173c0c2b Alexander Lobakin 2022-06-24 38 * to that the compiler can't optimize them to compile-time expressions or b03fc1173c0c2b Alexander Lobakin 2022-06-24 39 * constants. In contrary, generic_*() helpers are defined in pure C and b03fc1173c0c2b Alexander Lobakin 2022-06-24 40 * compilers optimize them just well. b03fc1173c0c2b Alexander Lobakin 2022-06-24 41 * Therefore, to make `unsigned long foo = 0; __set_bit(BAR, &foo)` effectively b03fc1173c0c2b Alexander Lobakin 2022-06-24 42 * equal to `unsigned long foo = BIT(BAR)`, pick the generic C alternative when b03fc1173c0c2b Alexander Lobakin 2022-06-24 43 * the arguments can be resolved at compile time. That expression itself is a b03fc1173c0c2b Alexander Lobakin 2022-06-24 44 * constant and doesn't bring any functional changes to the rest of cases. b03fc1173c0c2b Alexander Lobakin 2022-06-24 45 * The casts to `uintptr_t` are needed to mitigate `-Waddress` warnings when b03fc1173c0c2b Alexander Lobakin 2022-06-24 46 * passing a bitmap from .bss or .data (-> `!!addr` is always true). b03fc1173c0c2b Alexander Lobakin 2022-06-24 47 */ e69eb9c460f128 Alexander Lobakin 2022-06-24 48 #define bitop(op, nr, addr) \ b03fc1173c0c2b Alexander Lobakin 2022-06-24 49 ((__builtin_constant_p(nr) && \ b03fc1173c0c2b Alexander Lobakin 2022-06-24 50 __builtin_constant_p((uintptr_t)(addr) != (uintptr_t)NULL) && \ b03fc1173c0c2b Alexander Lobakin 2022-06-24 51 (uintptr_t)(addr) != (uintptr_t)NULL && \ b03fc1173c0c2b Alexander Lobakin 2022-06-24 @52 __builtin_constant_p(*(const unsigned long *)(addr))) ? \ b03fc1173c0c2b Alexander Lobakin 2022-06-24 53 const##op(nr, addr) : op(nr, addr)) e69eb9c460f128 Alexander Lobakin 2022-06-24 54
Hi Jeff, Oct 23, 2023 23:24:55 Jeff LaBundy <jeff@labundy.com>: > On Mon, Oct 23, 2023 at 07:55:52AM +0200, Thomas Weißschuh wrote: > > [...] > >>>> + err = i2c_master_send(client, write_buf, sizeof(write_buf)); >>>> + if (err < 0) >>>> + return err; >>> >>> You correctly return err (or rather, ret) for negative values, but you should also >>> check that ret matches the size of the data sent. For 0 <= ret < sizeof(writebuf), >>> return -EIO. >> >> The driver did this originally. >> I then requested it to be removed as this case >> can never happen. >> i2c_master_send will either return size of(writebuf) or an error. > > Great catch; indeed you are correct. Apologies for having missed this > in the change log; this is good to know in the future. I guess it would make sense to also adapt the function documentation to be more explicit about this invariant. No need to complicate every caller unnecessarily. I can send a patch somewhere next week, but if you want to send one I'll be happy to review it. > That being said, it's a moot point IMO; this driver seems like a good > candidate for regmap. If regmap cannot be made to work here for some > reason, then I'd like to at least see some wrapper functions to avoid > duplicate code and manual assignments to a buffer. Ack. Thomas
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 <anshulusr@gmail.com> +L: linux-input@vger.kernel.org +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 <jikos@kernel.org> 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..2a1eae8d2861 --- /dev/null +++ b/drivers/input/joystick/adafruit-seesaw.c @@ -0,0 +1,273 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2023 Anshul Dalal <anshulusr@gmail.com> + * + * Driver for Adafruit Mini I2C Gamepad + * + * Based on the work of: + * Oleh Kravchenko (Sparkfun Qwiic Joystick driver) + * + * Datasheet: https://cdn-learn.adafruit.com/downloads/pdf/gamepad-qt.pdf + * 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) { + dev_dbg(&input->dev, "failed to read joystick state: %d\n", + err); + 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 <anshulusr@gmail.com>"); +MODULE_DESCRIPTION("Adafruit Mini I2C Gamepad driver"); +MODULE_LICENSE("GPL");