2017-04-30 03:36:00

by Tomohiro Yoshidomi

[permalink] [raw]
Subject: [PATCH v6] Input: psxpad-spi - Add PlayStation 1/2 joypads via SPI interface Driver

PlayStation 1/2 joypads can be connected directly to the SPI interface.

Signed-off-by: Tomohiro Yoshidomi <[email protected]>
---
drivers/input/joystick/Kconfig | 22 ++
drivers/input/joystick/Makefile | 1 +
drivers/input/joystick/psxpad-spi.c | 433 ++++++++++++++++++++++++++++++++++++
3 files changed, 456 insertions(+)
create mode 100644 drivers/input/joystick/psxpad-spi.c

diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
index 4215b538..cbcb9a6e 100644
--- a/drivers/input/joystick/Kconfig
+++ b/drivers/input/joystick/Kconfig
@@ -330,4 +330,26 @@ config JOYSTICK_MAPLE
To compile this as a module choose M here: the module will be called
maplecontrol.

+config JOYSTICK_PSXPAD_SPI
+ tristate "PlayStation 1/2 joypads via SPI interface"
+ depends on SPI
+ select INPUT_POLLDEV
+ help
+ Say Y here if you wish to connect PlayStation 1/2 joypads
+ via SPI interface.
+
+ To compile this driver as a module, choose M here: the
+ module will be called psxpad-spi.
+
+config JOYSTICK_PSXPAD_SPI_FF
+ bool "PlayStation 1/2 joypads force feedback (rumble) support"
+ depends on JOYSTICK_PSXPAD_SPI
+ select INPUT_FF_MEMLESS
+ help
+ Say Y here if you want to take advantage of
+ PlayStation 1/2 joypads rumble features.
+
+ To drive rumble motor,
+ a dedicated power supply is required.
+
endif
diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
index 92dc0de9..496fd56b 100644
--- a/drivers/input/joystick/Makefile
+++ b/drivers/input/joystick/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_JOYSTICK_INTERACT) += interact.o
obj-$(CONFIG_JOYSTICK_JOYDUMP) += joydump.o
obj-$(CONFIG_JOYSTICK_MAGELLAN) += magellan.o
obj-$(CONFIG_JOYSTICK_MAPLE) += maplecontrol.o
+obj-$(CONFIG_JOYSTICK_PSXPAD_SPI) += psxpad-spi.o
obj-$(CONFIG_JOYSTICK_SIDEWINDER) += sidewinder.o
obj-$(CONFIG_JOYSTICK_SPACEBALL) += spaceball.o
obj-$(CONFIG_JOYSTICK_SPACEORB) += spaceorb.o
diff --git a/drivers/input/joystick/psxpad-spi.c b/drivers/input/joystick/psxpad-spi.c
new file mode 100644
index 00000000..d3eb0854
--- /dev/null
+++ b/drivers/input/joystick/psxpad-spi.c
@@ -0,0 +1,433 @@
+/*
+ * PlayStation 1/2 joypads via SPI interface Driver
+ *
+ * Copyright (C) 2017 Tomohiro Yoshidomi <[email protected]>
+ * Licensed under the GPL-2 or later.
+ *
+ * PlayStation 1/2 joypad's plug (not socket)
+ * 123 456 789
+ * (...|...|...)
+ *
+ * 1: DAT -> MISO (pullup with 1k owm to 3.3V)
+ * 2: CMD -> MOSI
+ * 3: 9V (for motor, if not use N.C.)
+ * 4: GND
+ * 5: 3.3V
+ * 6: Attention -> CS(SS)
+ * 7: SCK -> SCK
+ * 8: N.C.
+ * 9: ACK -> N.C.
+ */
+
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/input.h>
+#include <linux/input-polldev.h>
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+#include <linux/types.h>
+#include <linux/pm.h>
+#include <linux/pm_runtime.h>
+
+#define REVERSE_BIT(x) ((((x) & 0x80) >> 7) | (((x) & 0x40) >> 5) | \
+ (((x) & 0x20) >> 3) | (((x) & 0x10) >> 1) | (((x) & 0x08) << 1) | \
+ (((x) & 0x04) << 3) | (((x) & 0x02) << 5) | (((x) & 0x01) << 7))
+
+/* PlayStation 1/2 joypad command and response are LSBFIRST. */
+
+/* 0x01, 0x42, 0x00, 0x00, 0x00,
+ * 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ * 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+ */
+static const u8 PSX_CMD_POLL[] = {
+ 0x80, 0x42, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+/* 0x01, 0x43, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00 */
+static const u8 PSX_CMD_ENTER_CFG[] = {
+ 0x80, 0xC2, 0x00, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+/* 0x01, 0x43, 0x00, 0x00, 0x5A, 0x5A, 0x5A, 0x5A, 0x5A */
+static const u8 PSX_CMD_EXIT_CFG[] = {
+ 0x80, 0xC2, 0x00, 0x00, 0x5A, 0x5A, 0x5A, 0x5A, 0x5A
+};
+/* 0x01, 0x4D, 0x00, 0x00, 0x01, 0xFF, 0xFF, 0xFF, 0xFF */
+static const u8 PSX_CMD_ENABLE_MOTOR[] = {
+ 0x80, 0xB2, 0x00, 0x00, 0x80, 0xFF, 0xFF, 0xFF, 0xFF
+};
+
+struct psxpad {
+ struct spi_device *spi;
+ struct input_polled_dev *pdev;
+ struct input_dev *idev;
+ char phys[0x20];
+ bool motor1enable;
+ bool motor2enable;
+ u8 motor1level;
+ u8 motor2level;
+ u8 sendbuf[0x20] ____cacheline_aligned;
+ u8 response[sizeof(PSX_CMD_POLL)] ____cacheline_aligned;
+};
+
+static int psxpad_command(struct psxpad *pad, const u8 sendcmdlen)
+{
+ struct spi_transfer xfers = {
+ .tx_buf = pad->sendbuf,
+ .rx_buf = pad->response,
+ .len = sendcmdlen,
+ };
+ int err;
+
+ err = spi_sync_transfer(pad->spi, &xfers, 1);
+ if (err) {
+ dev_err(&pad->spi->dev,
+ "%s: failed to SPI xfers mode: %d\n",
+ __func__, err);
+ return err;
+ }
+
+ return 0;
+}
+
+#ifdef CONFIG_JOYSTICK_PSXPAD_SPI_FF
+static void psxpad_setenablemotor(
+ struct psxpad *pad, bool motor1enable, bool motor2enable)
+{
+ int err;
+
+ pad->motor1enable = motor1enable;
+ pad->motor2enable = motor2enable;
+
+ memcpy(pad->sendbuf, PSX_CMD_ENTER_CFG, sizeof(PSX_CMD_ENTER_CFG));
+ err = psxpad_command(pad, sizeof(PSX_CMD_ENTER_CFG));
+ if (err) {
+ dev_err(&pad->spi->dev,
+ "%s: failed to enter config mode: %d\n",
+ __func__, err);
+ return;
+ }
+ memcpy(pad->sendbuf,
+ PSX_CMD_ENABLE_MOTOR, sizeof(PSX_CMD_ENABLE_MOTOR));
+ pad->sendbuf[3] = pad->motor1enable ? 0x00 : 0xFF;
+ pad->sendbuf[4] = pad->motor2enable ? 0x80 : 0xFF;
+ err = psxpad_command(pad, sizeof(PSX_CMD_ENABLE_MOTOR));
+ if (err) {
+ dev_err(&pad->spi->dev,
+ "%s: failed to enable motor mode: %d\n",
+ __func__, err);
+ return;
+ }
+ memcpy(pad->sendbuf, PSX_CMD_EXIT_CFG, sizeof(PSX_CMD_EXIT_CFG));
+ err = psxpad_command(pad, sizeof(PSX_CMD_EXIT_CFG));
+ if (err) {
+ dev_err(&pad->spi->dev,
+ "%s: failed to exit config mode: %d\n",
+ __func__, err);
+ return;
+ }
+}
+
+static void psxpad_set_motor_level(
+ struct psxpad *pad, u8 motor1level, u8 motor2level)
+{
+ pad->motor1level = motor1level ? 0xFF : 0x00;
+ pad->motor2level = REVERSE_BIT(motor2level);
+}
+#else /* CONFIG_JOYSTICK_PSXPAD_SPI_FF */
+static void psxpad_setenablemotor(
+ struct psxpad *pad, bool motor1enable, bool motor2enable) { }
+
+static void psxpad_set_motor_level(
+ struct psxpad *pad, u8 motor1level, u8 motor2level) { }
+#endif /* CONFIG_JOYSTICK_PSXPAD_SPI_FF */
+
+static void psxpad_spi_poll_open(struct input_polled_dev *pdev)
+{
+ struct psxpad *pad = pdev->private;
+
+ pm_runtime_get_sync(&pad->spi->dev);
+}
+
+static void psxpad_spi_poll_close(struct input_polled_dev *pdev)
+{
+ struct psxpad *pad = pdev->private;
+
+ pm_runtime_put_sync(&pad->spi->dev);
+}
+
+static void psxpad_spi_poll(struct input_polled_dev *pdev)
+{
+ struct psxpad *pad = pdev->private;
+ int err;
+
+ psxpad_setenablemotor(pad, true, true);
+
+ memcpy(pad->sendbuf, PSX_CMD_POLL, sizeof(PSX_CMD_POLL));
+ pad->sendbuf[3] = pad->motor1enable ? pad->motor1level : 0x00;
+ pad->sendbuf[4] = pad->motor2enable ? pad->motor2level : 0x00;
+ err = psxpad_command(pad, sizeof(PSX_CMD_POLL));
+ if (err) {
+ dev_err(&pad->spi->dev,
+ "%s: failed to poll cmd mode: %d\n",
+ __func__, err);
+ return;
+ }
+
+ switch (pad->response[1]) {
+ case 0xCE: /* 0x73 : analog 1 */
+ input_report_abs(pad->idev,
+ ABS_X, REVERSE_BIT(pad->response[7]));
+ input_report_abs(pad->idev,
+ ABS_Y, REVERSE_BIT(pad->response[8]));
+ input_report_abs(pad->idev,
+ ABS_RX, REVERSE_BIT(pad->response[5]));
+ input_report_abs(pad->idev,
+ ABS_RY, REVERSE_BIT(pad->response[6]));
+ input_report_key(pad->idev,
+ BTN_DPAD_UP, !(pad->response[3] & BIT(3)));
+ input_report_key(pad->idev,
+ BTN_DPAD_DOWN, !(pad->response[3] & BIT(1)));
+ input_report_key(pad->idev,
+ BTN_DPAD_LEFT, !(pad->response[3] & BIT(0)));
+ input_report_key(pad->idev,
+ BTN_DPAD_RIGHT, !(pad->response[3] & BIT(2)));
+ input_report_key(pad->idev,
+ BTN_X, !(pad->response[4] & BIT(3)));
+ input_report_key(pad->idev,
+ BTN_A, !(pad->response[4] & BIT(2)));
+ input_report_key(pad->idev,
+ BTN_B, !(pad->response[4] & BIT(1)));
+ input_report_key(pad->idev,
+ BTN_Y, !(pad->response[4] & BIT(0)));
+ input_report_key(pad->idev,
+ BTN_TL, !(pad->response[4] & BIT(5)));
+ input_report_key(pad->idev,
+ BTN_TR, !(pad->response[4] & BIT(4)));
+ input_report_key(pad->idev,
+ BTN_TL2, !(pad->response[4] & BIT(7)));
+ input_report_key(pad->idev,
+ BTN_TR2, !(pad->response[4] & BIT(6)));
+ input_report_key(pad->idev,
+ BTN_THUMBL, !(pad->response[3] & BIT(6)));
+ input_report_key(pad->idev,
+ BTN_THUMBR, !(pad->response[3] & BIT(5)));
+ input_report_key(pad->idev,
+ BTN_SELECT, !(pad->response[3] & BIT(7)));
+ input_report_key(pad->idev,
+ BTN_START, !(pad->response[3] & BIT(4)));
+ break;
+
+ case 0x82: /* 0x41 : digital */
+ input_report_abs(pad->idev,
+ ABS_X, 0x80);
+ input_report_abs(pad->idev,
+ ABS_Y, 0x80);
+ input_report_abs(pad->idev,
+ ABS_RX, 0x80);
+ input_report_abs(pad->idev,
+ ABS_RY, 0x80);
+ input_report_key(pad->idev,
+ BTN_DPAD_UP, !(pad->response[3] & BIT(3)));
+ input_report_key(pad->idev,
+ BTN_DPAD_DOWN, !(pad->response[3] & BIT(1)));
+ input_report_key(pad->idev,
+ BTN_DPAD_LEFT, !(pad->response[3] & BIT(0)));
+ input_report_key(pad->idev,
+ BTN_DPAD_RIGHT, !(pad->response[3] & BIT(2)));
+ input_report_key(pad->idev,
+ BTN_X, !(pad->response[4] & BIT(3)));
+ input_report_key(pad->idev,
+ BTN_A, !(pad->response[4] & BIT(2)));
+ input_report_key(pad->idev,
+ BTN_B, !(pad->response[4] & BIT(1)));
+ input_report_key(pad->idev,
+ BTN_Y, !(pad->response[4] & BIT(0)));
+ input_report_key(pad->idev,
+ BTN_TL, !(pad->response[4] & BIT(5)));
+ input_report_key(pad->idev,
+ BTN_TR, !(pad->response[4] & BIT(4)));
+ input_report_key(pad->idev,
+ BTN_TL2, !(pad->response[4] & BIT(7)));
+ input_report_key(pad->idev,
+ BTN_TR2, !(pad->response[4] & BIT(6)));
+ input_report_key(pad->idev,
+ BTN_THUMBL, false);
+ input_report_key(pad->idev,
+ BTN_THUMBR, false);
+ input_report_key(pad->idev,
+ BTN_SELECT, !(pad->response[3] & BIT(7)));
+ input_report_key(pad->idev,
+ BTN_START, !(pad->response[3] & BIT(4)));
+ break;
+ }
+
+ input_sync(pad->idev);
+}
+
+#ifdef CONFIG_JOYSTICK_PSXPAD_SPI_FF
+static int psxpad_spi_ff(
+ struct input_dev *idev, void *data, struct ff_effect *effect)
+{
+ struct input_polled_dev *pdev = input_get_drvdata(idev);
+ struct psxpad *pad = pdev->private;
+
+ switch (effect->type) {
+ case FF_RUMBLE:
+ psxpad_set_motor_level(pad,
+ (effect->u.rumble.weak_magnitude >> 8) & 0xFFU,
+ (effect->u.rumble.strong_magnitude >> 8) & 0xFFU);
+ break;
+ }
+
+ return 0;
+}
+
+static int psxpad_spi_init_ff(struct psxpad *pad)
+{
+ int err;
+
+ input_set_capability(pad->idev, EV_FF, FF_RUMBLE);
+ err = input_ff_create_memless(pad->idev, NULL, psxpad_spi_ff);
+ if (err) {
+ dev_err(&pad->spi->dev,
+ "%s: failed to ff alloc mode: %d\n",
+ __func__, err);
+ }
+
+ return err;
+}
+#else /* CONFIG_JOYSTICK_PSXPAD_SPI_FF */
+static inline int psxpad_spi_init_ff(struct psxpad *pad)
+{
+ return 0;
+}
+#endif /* CONFIG_JOYSTICK_PSXPAD_SPI_FF */
+
+static int psxpad_spi_probe(struct spi_device *spi)
+{
+ struct psxpad *pad;
+ struct input_polled_dev *pdev;
+ struct input_dev *idev;
+ int err;
+
+ pad = devm_kzalloc(&spi->dev, sizeof(struct psxpad), GFP_KERNEL);
+ if (!pad)
+ return -ENOMEM;
+ pdev = input_allocate_polled_device();
+ if (!pdev) {
+ dev_err(&spi->dev,
+ "%s: failed to pdev alloc mode: %d\n",
+ __func__, err);
+ return -ENOMEM;
+ }
+
+ /* input poll device settings */
+ pad->pdev = pdev;
+ pad->spi = spi;
+ pad->motor1enable = false;
+ pad->motor2enable = false;
+ pdev->private = pad;
+ pdev->open = psxpad_spi_poll_open;
+ pdev->close = psxpad_spi_poll_close;
+ pdev->poll = psxpad_spi_poll;
+ /* poll interval is about 60fps */
+ pdev->poll_interval = 16;
+ pdev->poll_interval_min = 8;
+ pdev->poll_interval_max = 32;
+
+ /* input device settings */
+ idev = pdev->input;
+ pad->idev = idev;
+ idev->name = "PlayStation 1/2 joypad";
+ snprintf(pad->phys, sizeof(pad->phys), "%s/input", dev_name(&spi->dev));
+ idev->id.bustype = BUS_SPI;
+
+ /* key/value map settings */
+ input_set_abs_params(idev, ABS_X, 0, 255, 0, 0);
+ input_set_abs_params(idev, ABS_Y, 0, 255, 0, 0);
+ input_set_abs_params(idev, ABS_RX, 0, 255, 0, 0);
+ input_set_abs_params(idev, ABS_RY, 0, 255, 0, 0);
+ input_set_capability(idev, EV_KEY, BTN_DPAD_UP);
+ input_set_capability(idev, EV_KEY, BTN_DPAD_DOWN);
+ input_set_capability(idev, EV_KEY, BTN_DPAD_LEFT);
+ input_set_capability(idev, EV_KEY, BTN_DPAD_RIGHT);
+ input_set_capability(idev, EV_KEY, BTN_A);
+ input_set_capability(idev, EV_KEY, BTN_B);
+ input_set_capability(idev, EV_KEY, BTN_X);
+ input_set_capability(idev, EV_KEY, BTN_Y);
+ input_set_capability(idev, EV_KEY, BTN_TL);
+ input_set_capability(idev, EV_KEY, BTN_TR);
+ input_set_capability(idev, EV_KEY, BTN_TL2);
+ input_set_capability(idev, EV_KEY, BTN_TR2);
+ input_set_capability(idev, EV_KEY, BTN_THUMBL);
+ input_set_capability(idev, EV_KEY, BTN_THUMBR);
+ input_set_capability(idev, EV_KEY, BTN_SELECT);
+ input_set_capability(idev, EV_KEY, BTN_START);
+
+ /* force feedback */
+ err = psxpad_spi_init_ff(pad);
+ if (err) {
+ dev_err(&spi->dev,
+ "%s: failed to init ff mode: %d\n",
+ __func__, err);
+ return err;
+ }
+
+ /* SPI settings */
+ spi->mode = SPI_MODE_3;
+ spi->bits_per_word = 8;
+ /* (PlayStation 1/2 joypad might be possible works 250kHz/500kHz) */
+ spi->master->min_speed_hz = 125000;
+ spi->master->max_speed_hz = 125000;
+ spi_setup(spi);
+
+ /* pad settings */
+ psxpad_set_motor_level(pad, 0, 0);
+
+ /* register input poll device */
+ err = input_register_polled_device(pdev);
+ if (err) {
+ dev_err(&spi->dev,
+ "%s: failed to register input poll device: %d\n",
+ __func__, err);
+ return err;
+ }
+
+ pm_runtime_enable(&spi->dev);
+
+ return 0;
+}
+
+static int __maybe_unused psxpad_spi_suspend(struct device *dev)
+{
+ struct spi_device *spi = to_spi_device(dev);
+ struct psxpad *pad = spi_get_drvdata(spi);
+
+ psxpad_set_motor_level(pad, 0, 0);
+
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(psxpad_spi_pm, psxpad_spi_suspend, psxpad_spi_resume);
+
+static const struct spi_device_id psxpad_spi_id[] = {
+ { "psxpad-spi", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(spi, psxpad_spi_id);
+
+static struct spi_driver psxpad_spi_driver = {
+ .driver = {
+ .name = "psxpad-spi",
+ .pm = &psxpad_spi_pm,
+ },
+ .id_table = psxpad_spi_id,
+ .probe = psxpad_spi_probe,
+};
+
+module_spi_driver(psxpad_spi_driver);
+
+MODULE_AUTHOR("Tomohiro Yoshidomi <[email protected]>");
+MODULE_DESCRIPTION("PlayStation 1/2 joypads via SPI interface Driver");
+MODULE_LICENSE("GPL");
--
2.11.0


2017-04-30 03:36:55

by Tomohiro Yoshidomi

[permalink] [raw]
Subject: Re: [PATCH v6] Input: psxpad-spi - Add PlayStation 1/2 joypads via SPI interface Driver

Mr.Torokhov

I fixed source, and sent PATCH v6 to you.

As Mr.Nocera said, I fixed souces does not use 'PSX' in sentences.
Also, I wrote that a separate power supply is required for the motor.

2017-04-30 3:16 GMT+09:00 Dmitry Torokhov <[email protected]>:
> Hi,
>
> On Sat, Apr 29, 2017 at 06:40:53PM +0900, Tomohiro Yoshidomi wrote:
>> PSX pads can be connected directly to the SPI bus.
>>
>> Signed-off-by: Tomohiro Yoshidomi <[email protected]>
>
> Thank you very much for making requested changes and your patience. I
> think we just need a few finishing touches and we can commit this
> driver.
>
>> ---
>> drivers/input/joystick/Kconfig | 17 ++
>> drivers/input/joystick/Makefile | 1 +
>> drivers/input/joystick/psxpad-spi.c | 387 ++++++++++++++++++++++++++++++++++++
>> 3 files changed, 405 insertions(+)
>> create mode 100644 drivers/input/joystick/psxpad-spi.c
>>
>> diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
>> index 4215b538..bfbda643 100644
>> --- a/drivers/input/joystick/Kconfig
>> +++ b/drivers/input/joystick/Kconfig
>> @@ -330,4 +330,21 @@ config JOYSTICK_MAPLE
>> To compile this as a module choose M here: the module will be called
>> maplecontrol.
>>
>> +config JOYSTICK_PSXPAD_SPI
>> + tristate "PSX (Play Station 1/2) pad with SPI Bus Driver"
>
> Let's call this "PSX (Play Station 1/2) gamepad on SPI bus"

Fixed.
"PlayStation 1/2 joypads via SPI interface"

>> + depends on SPI
>> + select INPUT_POLLDEV
>> + help
>> + Say Y here if you connect PSX (PS1/2) pad with SPI Interface.
>
> "Say Y here if you wish to connect PSX (PS1/2) pad via SPI interface."

Fixed.
"Say Y here if you wish to connect PlayStation 1/2 joypads via SPI interface."

>> +
>> + To compile this driver as a module, choose M here: the
>> + module will be called psxpad-spi.
>> +
>> +config JOYSTICK_PSXPAD_SPI_FF
>> + bool "PSX pad with SPI Bus rumble support"
>
> Let's call it "PSX gamepad force feedback (rumble) support"

Fixed.
"PlayStation 1/2 joypads force feedback (rumble) support"

>> + depends on JOYSTICK_PSXPAD_SPI
>> + select INPUT_FF_MEMLESS
>> + help
>> + Say Y here if you want to take advantage of PSX pad rumble features.
>> +
>> endif
>> diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
>> index 92dc0de9..496fd56b 100644
>> --- a/drivers/input/joystick/Makefile
>> +++ b/drivers/input/joystick/Makefile
>> @@ -21,6 +21,7 @@ obj-$(CONFIG_JOYSTICK_INTERACT) += interact.o
>> obj-$(CONFIG_JOYSTICK_JOYDUMP) += joydump.o
>> obj-$(CONFIG_JOYSTICK_MAGELLAN) += magellan.o
>> obj-$(CONFIG_JOYSTICK_MAPLE) += maplecontrol.o
>> +obj-$(CONFIG_JOYSTICK_PSXPAD_SPI) += psxpad-spi.o
>> obj-$(CONFIG_JOYSTICK_SIDEWINDER) += sidewinder.o
>> obj-$(CONFIG_JOYSTICK_SPACEBALL) += spaceball.o
>> obj-$(CONFIG_JOYSTICK_SPACEORB) += spaceorb.o
>> diff --git a/drivers/input/joystick/psxpad-spi.c b/drivers/input/joystick/psxpad-spi.c
>> new file mode 100644
>> index 00000000..9684839c
>> --- /dev/null
>> +++ b/drivers/input/joystick/psxpad-spi.c
>> @@ -0,0 +1,387 @@
>> +/*
>> + * PSX (Play Station 1/2) pad (SPI Interface)
>> + *
>> + * Copyright (C) 2017 Tomohiro Yoshidomi <[email protected]>
>> + * Licensed under the GPL-2 or later.
>> + *
>> + * PSX pad plug (not socket)
>> + * 123 456 789
>> + * (...|...|...)
>> + *
>> + * 1: DAT -> MISO (pullup with 1k owm to 3.3V)
>> + * 2: CMD -> MOSI
>> + * 3: 9V (for motor, if not use N.C.)
>> + * 4: GND
>> + * 5: 3.3V
>> + * 6: Attention -> CS(SS)
>> + * 7: SCK -> SCK
>> + * 8: N.C.
>> + * 9: ACK -> N.C.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/device.h>
>> +#include <linux/input.h>
>> +#include <linux/input-polldev.h>
>> +#include <linux/module.h>
>> +#include <linux/spi/spi.h>
>> +#include <linux/types.h>
>> +#include <linux/pm.h>
>> +#include <linux/pm_runtime.h>
>> +
>> +#define REVERSE_BIT(x) ((((x) & 0x80) >> 7) | (((x) & 0x40) >> 5) | (((x) & 0x20) >> 3) | (((x) & 0x10) >> 1) | (((x) & 0x08) << 1) | (((x) & 0x04) << 3) | (((x) & 0x02) << 5) | (((x) & 0x01) << 7))
>> +
>> +static const u8 PSX_CMD_POLL[] = {0x01, 0x42, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
>> +static const u8 PSX_CMD_ENTER_CFG[] = {0x01, 0x43, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00};
>> +static const u8 PSX_CMD_EXIT_CFG[] = {0x01, 0x43, 0x00, 0x00, 0x5A, 0x5A, 0x5A, 0x5A, 0x5A};
>> +static const u8 PSX_CMD_ENABLE_MOTOR[] = {0x01, 0x4D, 0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF};
>> +
>> +struct psxpad {
>> + struct spi_device *spi;
>> + struct input_polled_dev *pdev;
>> + struct input_dev *idev;
>> + char phys[0x20];
>> + bool motor1enable;
>> + bool motor2enable;
>> + u8 motor1level;
>> + u8 motor2level;
>> + u8 pollcmd[sizeof(PSX_CMD_POLL)];
>> + u8 enablemotor[sizeof(PSX_CMD_ENABLE_MOTOR)];
>> + u8 sendbuf[0x20] ____cacheline_aligned;
>> + u8 response[sizeof(PSX_CMD_POLL)] ____cacheline_aligned;
>> +};
>> +
>> +static int psxpad_command(struct psxpad *pad, const u8 sendcmd[], u8 response[], const u8 sendcmdlen)
>
> I wonder if we really need to pass "response" as a parameter here.

Fixed.

>> +{
>> + struct spi_transfer xfers = {
>> + .tx_buf = pad->sendbuf,
>> + .rx_buf = response,
>> + .len = sendcmdlen,
>> + };
>> + struct spi_message msg;
>> + int err;
>> + u8 loc;
>
> Let's call it "int i" - the standard loop variable.
>
>> +
>> + spi_message_init(&msg);
>
> I do not think we need "msg" anymore.

Fixed.

>> +
>> + for (loc = 0; loc < sendcmdlen; loc++)
>> + pad->sendbuf[loc] = REVERSE_BIT(sendcmd[loc]);
>
> I think I already asked this, but I do not recall the answer: can we
> supply commands with bits pre-reversed (i.e. specify PSX_CMD_POLL as
> 0x80, 0x42, 0x00, ... and PSX_CMD_ENTER_CFG as 0x80, 0xc2, 0x00, 0x80,
> 0x00, ... and so on, and reverse arguments when you supply them? See
> more in psxpad_setenablemotor().

Fixed.
I use MSBFIRST (not LSBFIRST) values.

>> +
>> + err = spi_sync_transfer(pad->spi, &xfers, 1);
>> + if (err) {
>> + dev_err(&pad->idev->dev, "psxpad-spi: failed SPI xfers!!\n");
>> + goto err_end;
>
> Just return directly here:
>
> return err;

Fixed.

>> + }
>> +
>> + for (loc = 0; loc < sendcmdlen; loc++)
>> + response[loc] = REVERSE_BIT(response[loc]);
>
> We only use returned data when we poll the device. Maybe we should do
> this bit-reversing outsize of psxpad_command()?

Fixed.

>> +
>> + return 0;
>> +
>> + err_end:
>> +
>> + return err;
>> +}
>> +
>> +#ifdef CONFIG_JOYSTICK_PSXPAD_SPI_FF
>> +static void psxpad_setenablemotor(struct psxpad *pad, const bool motor1enable, const bool motor2enable)
>
> psxpad_control_motor() and there is really no need for "const" on scalar
> arguments.

Fixed.
psxpad_set_motor_level() was fixed too.

>> +{
>> + int err;
>> +
>> + pad->motor1enable = motor1enable;
>> + pad->motor2enable = motor2enable;
>> +
>> + pad->enablemotor[3] = pad->motor1enable ? 0x00 : 0xFF;
>> + pad->enablemotor[4] = pad->motor2enable ? 0x01 : 0xFF;
>
> Assuming that the commands are "pre-revrsed":
>
> memcpy(pad->sendbuf, PSX_CMD_ENTER_CFG, sizeof(PSX_CMD_ENTER_CFG);
> err = psxpad_command(pad, sizeof(PSX_CMD_ENTER_CFG));
> if (err) {
> dev_err(&pad->spi->dev,
> "%s: failed to enter config mode: %d\n",
> __func__, err);
> return err;
> }
>
> memcpy(pad->sendbuf, PSX_CMD_ENABLE_MOTOR, sizeof(PSX_CMD_ENABLE_MOTOR);
> pad->sendbuf[3] = motor1enable ? 0x00 : 0xff;
> pad->sendbuf[4] = motor2enable ? 0x80 : 0xff;
> err = psxpad_command(pad, sizeof(PSX_CMD_ENABLE_MOTOR));
> if (err) {
> dev_err(&pad->spi->dev,
> "%s: ENABLE_MOTOR command failed: %d\n",
> __func__, err);
> return err;
> }
>
> ...

Fixed.

I wrote this code with reading otherone's PSX pad's analysis informaton.
The LSBFIRST values are listed in them.
I implemented the value so that it is easy to read.

I commented LSBFIRST value, and change to use MSBFIRST value.

>> +
>> + err = psxpad_command(pad, PSX_CMD_ENTER_CFG, pad->response, sizeof(PSX_CMD_ENTER_CFG));
>> + if (err) {
>> + dev_err(&pad->idev->dev, "psxpad-spi: setenablemotor: enter cfg failed!!\n");
>> + return;
>> + }
>> + err = psxpad_command(pad, pad->enablemotor, pad->response, sizeof(PSX_CMD_ENABLE_MOTOR));
>> + if (err) {
>> + dev_err(&pad->idev->dev, "psxpad-spi: setenablemotor: enable motor failed!!\n");
>> + return;
>> + }
>> + err = psxpad_command(pad, PSX_CMD_EXIT_CFG, pad->response, sizeof(PSX_CMD_EXIT_CFG));
>> + if (err) {
>> + dev_err(&pad->idev->dev, "psxpad-spi: setenablemotor: exit config failed!!\n");
>> + return;
>> + }
>> +}
>> +
>> +static void psxpad_setmotorlevel(struct psxpad *pad, const u8 motor1level, const u8 motor2level)
>
> psxpad_set_motor_level.

Fixed.

>> +{
>> + pad->motor1level = motor1level ? 0xFF : 0x00;
>> + pad->motor2level = motor2level;
>> +
>> + pad->pollcmd[3] = pad->motor1level;
>> + pad->pollcmd[4] = pad->motor2level;
>
> So we need to control motors both while polling and explicitly at end of
> poll?

If the polling is interrupted at regular intervals, the pad will
forget to set it.
So I'm enabling the motor in the polling.

And, I changed setting motors at first of polling.

>> +}
>> +#else /* CONFIG_JOYSTICK_PSXPAD_SPI_FF */
>> +static void psxpad_setenablemotor(struct psxpad *pad, const bool motor1enable, const bool motor2enable) { }
>> +
>> +static void psxpad_setmotorlevel(struct psxpad *pad, const u8 motor1level, const u8 motor2level) { }
>
> Please try keeping within 80 columns.
>

Fixed.
I got it.

>> +#endif /* CONFIG_JOYSTICK_PSXPAD_SPI_FF */
>> +
>> +static void psxpad_spi_poll_open(struct input_polled_dev *pdev)
>> +{
>> + struct psxpad *pad = pdev->private;
>> +
>> + pm_runtime_get_sync(&pad->spi->dev);
>> +}
>> +
>> +static void psxpad_spi_poll_close(struct input_polled_dev *pdev)
>> +{
>> + struct psxpad *pad = pdev->private;
>> +
>> + pm_runtime_put_sync(&pad->spi->dev);
>> +}
>> +
>> +static void psxpad_spi_poll(struct input_polled_dev *pdev)
>> +{
>> + struct psxpad *pad = pdev->private;
>> + int err;
>> +
>> + err = psxpad_command(pad, pad->pollcmd, pad->response, sizeof(PSX_CMD_POLL));
>> + if (err) {
>> + dev_err(&pad->idev->dev, "psxpad-spi: poll: poll cmd failed!!\n");
>
> Please use pad->spi->dev in error messages, so errors are associated
> with a physical device and not abstract one (inputNNN).

Fixed.

>> + return;
>> + }
>> +
>> + switch (pad->response[1]) {
>> + case 0x73: /* analog 1 */
>> + input_report_abs(pad->idev, ABS_X, pad->response[7]);
>> + input_report_abs(pad->idev, ABS_Y, pad->response[8]);
>> + input_report_abs(pad->idev, ABS_RX, pad->response[5]);
>> + input_report_abs(pad->idev, ABS_RY, pad->response[6]);
>> + input_report_key(pad->idev, BTN_DPAD_UP, (pad->response[3] & 0x10U) ? false : true);
>> + input_report_key(pad->idev, BTN_DPAD_DOWN, (pad->response[3] & 0x40U) ? false : true);
>
> Please use BIT() macro; there is also no need to convert to true/false
> as input_report_key also does it.
>
> input_report_key(pad->idev, BTN_DPAD_UP,
> pad->response[3] & BIT(4));
> input_report_key(pad->idev, BTN_DPAD_DOWN,
> pad->response[3] & BIT(7));
>
> and so on.

Fixed.

>> + input_report_key(pad->idev, BTN_DPAD_LEFT, (pad->response[3] & 0x80U) ? false : true);
>> + input_report_key(pad->idev, BTN_DPAD_RIGHT, (pad->response[3] & 0x20U) ? false : true);
>> + input_report_key(pad->idev, BTN_X, (pad->response[4] & 0x10U) ? false : true);
>> + input_report_key(pad->idev, BTN_A, (pad->response[4] & 0x20U) ? false : true);
>> + input_report_key(pad->idev, BTN_B, (pad->response[4] & 0x40U) ? false : true);
>> + input_report_key(pad->idev, BTN_Y, (pad->response[4] & 0x80U) ? false : true);
>> + input_report_key(pad->idev, BTN_TL, (pad->response[4] & 0x04U) ? false : true);
>> + input_report_key(pad->idev, BTN_TR, (pad->response[4] & 0x08U) ? false : true);
>> + input_report_key(pad->idev, BTN_TL2, (pad->response[4] & 0x01U) ? false : true);
>> + input_report_key(pad->idev, BTN_TR2, (pad->response[4] & 0x02U) ? false : true);
>> + input_report_key(pad->idev, BTN_THUMBL, (pad->response[3] & 0x02U) ? false : true);
>> + input_report_key(pad->idev, BTN_THUMBR, (pad->response[3] & 0x04U) ? false : true);
>> + input_report_key(pad->idev, BTN_SELECT, (pad->response[3] & 0x01U) ? false : true);
>> + input_report_key(pad->idev, BTN_START, (pad->response[3] & 0x08U) ? false : true);
>> + break;
>> +
>> + case 0x41: /* digital */
>> + input_report_abs(pad->idev, ABS_X, 0x80);
>> + input_report_abs(pad->idev, ABS_Y, 0x80);
>> + input_report_abs(pad->idev, ABS_RX, 0x80);
>> + input_report_abs(pad->idev, ABS_RY, 0x80);
>> + input_report_key(pad->idev, BTN_DPAD_UP, (pad->response[3] & 0x10U) ? false : true);
>> + input_report_key(pad->idev, BTN_DPAD_DOWN, (pad->response[3] & 0x40U) ? false : true);
>> + input_report_key(pad->idev, BTN_DPAD_LEFT, (pad->response[3] & 0x80U) ? false : true);
>> + input_report_key(pad->idev, BTN_DPAD_RIGHT, (pad->response[3] & 0x20U) ? false : true);
>> + input_report_key(pad->idev, BTN_X, (pad->response[4] & 0x10U) ? false : true);
>> + input_report_key(pad->idev, BTN_A, (pad->response[4] & 0x20U) ? false : true);
>> + input_report_key(pad->idev, BTN_B, (pad->response[4] & 0x40U) ? false : true);
>> + input_report_key(pad->idev, BTN_Y, (pad->response[4] & 0x80U) ? false : true);
>> + input_report_key(pad->idev, BTN_TL, (pad->response[4] & 0x04U) ? false : true);
>> + input_report_key(pad->idev, BTN_TR, (pad->response[4] & 0x08U) ? false : true);
>> + input_report_key(pad->idev, BTN_TL2, (pad->response[4] & 0x01U) ? false : true);
>> + input_report_key(pad->idev, BTN_TR2, (pad->response[4] & 0x02U) ? false : true);
>> + input_report_key(pad->idev, BTN_THUMBL, false);
>> + input_report_key(pad->idev, BTN_THUMBR, false);
>> + input_report_key(pad->idev, BTN_SELECT, (pad->response[3] & 0x01U) ? false : true);
>> + input_report_key(pad->idev, BTN_START, (pad->response[3] & 0x08U) ? false : true);
>> + break;
>> + }
>> +
>> + input_sync(pad->idev);
>> + psxpad_setenablemotor(pad, true, true);
>> +}
>> +
>> +#ifdef CONFIG_JOYSTICK_PSXPAD_SPI_FF
>> +static int psxpad_spi_ff(struct input_dev *idev, void *data, struct ff_effect *effect)
>> +{
>> + struct psxpad *pad = idev->dev.platform_data;
>
> platform_data usually reserved for platform (board) code, not driver
> data.
>
> struct input_polled_dev *pdev = input_get_drvdata(input);
> struct psxpad *pad = pdev->private;

Fixed.

>> +
>> + switch (effect->type) {
>> + case FF_RUMBLE:
>> + psxpad_setmotorlevel(pad, (effect->u.rumble.weak_magnitude >> 8) & 0xFFU, (effect->u.rumble.strong_magnitude >> 8) & 0xFFU);
>> + break;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int psxpad_spi_init_ff(struct psxpad *pad)
>> +{
>> + int err;
>> +
>> + input_set_capability(pad->idev, EV_FF, FF_RUMBLE);
>> + err = input_ff_create_memless(pad->idev, NULL, psxpad_spi_ff);
>> + if (err) {
>> + dev_err(&pad->idev->dev, "psxpad-spi: init_ff: ff alloc failed!!\n");
>> + err = -ENOMEM;
>
> Please do not clobber error value returned by input_ff_create_memless(),
> pass it on as is.

Fixed.

>> + }
>> +
>> + return err;
>> +}
>> +#else /* CONFIG_JOYSTICK_PSXPAD_SPI_FF */
>> +static inline int psxpad_spi_init_ff(struct psxpad *pad)
>> +{
>> + return 0;
>> +}
>> +#endif /* CONFIG_JOYSTICK_PSXPAD_SPI_FF */
>> +
>> +static int psxpad_spi_probe(struct spi_device *spi)
>> +{
>> + struct psxpad *pad;
>> + struct input_polled_dev *pdev;
>> + struct input_dev *idev;
>> + int err, i;
>> +
>> + pad = devm_kzalloc(&spi->dev, sizeof(struct psxpad), GFP_KERNEL);
>> + if (!pad) {
>> + err = -ENOMEM;
>> + goto err_free_mem1;
>
> Just
> return -ENOMEM;

Fixed.

>> + }
>> + pdev = input_allocate_polled_device();
>> + if (!pdev) {
>> + dev_err(&spi->dev, "psxpad-spi: probe: pdev alloc failed!!\n");
>
> No need for "psxpad-spi: " prefix - dev_err adds driver name to error
> messages.
>
>>
>> + err = -ENOMEM;
>> + goto err_free_mem1;
>
> Just
> return -ENOMEM;
>
> devm will take care of freeing "pad" memory. That is is point of devm_*
> API.
>

Fixed.

>> + }
>> + for (i = 0; i < sizeof(PSX_CMD_POLL); i++)
>> + pad->pollcmd[i] = PSX_CMD_POLL[i];
>> + for (i = 0; i < sizeof(PSX_CMD_ENABLE_MOTOR); i++)
>> + pad->enablemotor[i] = PSX_CMD_ENABLE_MOTOR[i];
>
> I do not think you'll need this anymore if you do what I proposed with
> regard to command handling.
>

Fixed.

>> +
>> + /* input poll device settings */
>> + pad->pdev = pdev;
>> + pad->spi = spi;
>> + pdev->private = pad;
>> + pdev->open = psxpad_spi_poll_open;
>> + pdev->close = psxpad_spi_poll_close;
>> + pdev->poll = psxpad_spi_poll;
>> + /* poll interval is about 60fps */
>> + pdev->poll_interval = 16;
>> + pdev->poll_interval_min = 8;
>> + pdev->poll_interval_max = 32;
>> +
>> + /* input device settings */
>> + idev = pdev->input;
>> + pad->idev = idev;
>> + idev->name = "PSX (PS1/2) pad";
>> + snprintf(pad->phys, sizeof(pad->phys), "%s/input", dev_name(&spi->dev));
>> + idev->id.bustype = BUS_SPI;
>> + idev->dev.parent = &spi->dev;
>
> Please drop - devm_input_allocate_polled_device should set it up for
> you.

Fixed.

>> + idev->dev.platform_data = pad;
>
> Please drop.

Fixed.

>> +
>> + /* key/value map settings */
>> + input_set_abs_params(idev, ABS_X, 0, 255, 0, 0);
>> + input_set_abs_params(idev, ABS_Y, 0, 255, 0, 0);
>> + input_set_abs_params(idev, ABS_RX, 0, 255, 0, 0);
>> + input_set_abs_params(idev, ABS_RY, 0, 255, 0, 0);
>> + input_set_capability(idev, EV_KEY, BTN_DPAD_UP);
>> + input_set_capability(idev, EV_KEY, BTN_DPAD_DOWN);
>> + input_set_capability(idev, EV_KEY, BTN_DPAD_LEFT);
>> + input_set_capability(idev, EV_KEY, BTN_DPAD_RIGHT);
>> + input_set_capability(idev, EV_KEY, BTN_A);
>> + input_set_capability(idev, EV_KEY, BTN_B);
>> + input_set_capability(idev, EV_KEY, BTN_X);
>> + input_set_capability(idev, EV_KEY, BTN_Y);
>> + input_set_capability(idev, EV_KEY, BTN_TL);
>> + input_set_capability(idev, EV_KEY, BTN_TR);
>> + input_set_capability(idev, EV_KEY, BTN_TL2);
>> + input_set_capability(idev, EV_KEY, BTN_TR2);
>> + input_set_capability(idev, EV_KEY, BTN_THUMBL);
>> + input_set_capability(idev, EV_KEY, BTN_THUMBR);
>> + input_set_capability(idev, EV_KEY, BTN_SELECT);
>> + input_set_capability(idev, EV_KEY, BTN_START);
>> +
>> + /* force feedback */
>> + err = psxpad_spi_init_ff(pad);
>> + if (err) {
>> + dev_err(&spi->dev, "psxpad-spi: probe: failed init ff!!\n");
>> + err = -ENOMEM;
>> + goto err_free_mem2;
>
> return err;

Fixed.

>> + }
>> +
>> + /* SPI settings */
>> + spi->mode = SPI_MODE_3;
>> + spi->bits_per_word = 8;
>> + /* (PSX pad might be possible works 250kHz/500kHz) */
>> + spi->master->min_speed_hz = 125000;
>> + spi->master->max_speed_hz = 125000;
>> + spi_setup(spi);
>> +
>> + /* pad settings */
>> + psxpad_setmotorlevel(pad, 0, 0);
>> +
>> + /* register input poll device */
>> + err = input_register_polled_device(pdev);
>> + if (err) {
>> + dev_err(&spi->dev, "psxpad-spi: probe: failed register!!\n");
>> + goto err_free_mem2;
>
> return err;

Fixed.

>> + }
>> +
>> + pm_runtime_enable(&spi->dev);
>> +
>> + return 0;
>> +
>
> Code below is not needed.
>

Fixed.

>> + err_free_mem2:
>> + input_free_polled_device(pdev);
>> +
>> + err_free_mem1:
>> + devm_kfree(&spi->dev, pad);
>> +
>> + return err;
>> +}
>> +
>> +
>> +static int psxpad_spi_remove(struct spi_device *spi)
>> +{
>> + return 0;
>> +}
>
> You can remove this now empty stub.

Fixed.

>> +
>> +static int __maybe_unused psxpad_spi_suspend(struct device *dev)
>> +{
>> + struct spi_device *spi = to_spi_device(dev);
>> + struct psxpad *pad = spi_get_drvdata(spi);
>> +
>> + psxpad_setmotorlevel(pad, 0, 0);
>> +
>> + return 0;
>> +}
>> +
>> +static int __maybe_unused psxpad_spi_resume(struct device *dev)
>> +{
>> + return 0;
>> +}
>
> You can remove this empty stub.

Fixed.

>> +
>> +static SIMPLE_DEV_PM_OPS(psxpad_spi_pm, psxpad_spi_suspend, psxpad_spi_resume);
>> +
>> +static const struct spi_device_id psxpad_spi_id[] = {
>> + { "psxpad-spi", 0 },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(spi, psxpad_spi_id);
>> +
>> +static struct spi_driver psxpad_spi_driver = {
>> + .driver = {
>> + .name = "psxpad-spi",
>> + .pm = &psxpad_spi_pm,
>> + },
>> + .id_table = psxpad_spi_id,
>> + .probe = psxpad_spi_probe,
>> + .remove = psxpad_spi_remove,
>> +};
>> +
>> +module_spi_driver(psxpad_spi_driver);
>> +
>> +MODULE_AUTHOR("Tomohiro Yoshidomi <[email protected]>");
>> +MODULE_DESCRIPTION("PSX (Play Station 1/2) pad with SPI Bus Driver");
>> +MODULE_LICENSE("GPL");
>> --
>> 2.11.0
>>
>
> Thanks!
>
> --
> Dmitry

Thank you!
If you have any problem, please mail me.

--
Tomohiro

2017-04-30 04:09:04

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v6] Input: psxpad-spi - Add PlayStation 1/2 joypads via SPI interface Driver

Hi Tomohiro,

[auto build test WARNING on input/next]
[also build test WARNING on v4.11-rc8 next-20170428]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Tomohiro-Yoshidomi/Input-psxpad-spi-Add-PlayStation-1-2-joypads-via-SPI-interface-Driver/20170430-114033
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=ia64

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

drivers/input/joystick/psxpad-spi.c: In function 'psxpad_spi_probe':
>> drivers/input/joystick/psxpad-spi.c:319:3: warning: 'err' may be used uninitialized in this function [-Wmaybe-uninitialized]
dev_err(&spi->dev,
^~~~~~~~~~~~~~~~~~
"%s: failed to pdev alloc mode: %d\n",
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
__func__, err);
~~~~~~~~~~~~~~

vim +/err +319 drivers/input/joystick/psxpad-spi.c

303 return 0;
304 }
305 #endif /* CONFIG_JOYSTICK_PSXPAD_SPI_FF */
306
307 static int psxpad_spi_probe(struct spi_device *spi)
308 {
309 struct psxpad *pad;
310 struct input_polled_dev *pdev;
311 struct input_dev *idev;
312 int err;
313
314 pad = devm_kzalloc(&spi->dev, sizeof(struct psxpad), GFP_KERNEL);
315 if (!pad)
316 return -ENOMEM;
317 pdev = input_allocate_polled_device();
318 if (!pdev) {
> 319 dev_err(&spi->dev,
320 "%s: failed to pdev alloc mode: %d\n",
321 __func__, err);
322 return -ENOMEM;
323 }
324
325 /* input poll device settings */
326 pad->pdev = pdev;
327 pad->spi = spi;

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (2.18 kB)
.config.gz (45.69 kB)
Download all attachments

2017-04-30 04:23:04

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v6] Input: psxpad-spi - Add PlayStation 1/2 joypads via SPI interface Driver

Hi Tomohiro,

[auto build test ERROR on input/next]
[also build test ERROR on v4.11-rc8 next-20170428]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Tomohiro-Yoshidomi/Input-psxpad-spi-Add-PlayStation-1-2-joypads-via-SPI-interface-Driver/20170430-114033
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: blackfin-allmodconfig (attached as .config)
compiler: bfin-uclinux-gcc (GCC) 6.2.0
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=blackfin

All error/warnings (new ones prefixed by >>):

In file included from include/linux/device.h:25:0,
from drivers/input/joystick/psxpad-spi.c:23:
>> drivers/input/joystick/psxpad-spi.c:412:61: error: 'psxpad_spi_resume' undeclared here (not in a function)
static SIMPLE_DEV_PM_OPS(psxpad_spi_pm, psxpad_spi_suspend, psxpad_spi_resume);
^
include/linux/pm.h:319:12: note: in definition of macro 'SET_SYSTEM_SLEEP_PM_OPS'
.resume = resume_fn, \
^~~~~~~~~
>> drivers/input/joystick/psxpad-spi.c:412:8: note: in expansion of macro 'SIMPLE_DEV_PM_OPS'
static SIMPLE_DEV_PM_OPS(psxpad_spi_pm, psxpad_spi_suspend, psxpad_spi_resume);
^~~~~~~~~~~~~~~~~

vim +/psxpad_spi_resume +412 drivers/input/joystick/psxpad-spi.c

406
407 psxpad_set_motor_level(pad, 0, 0);
408
409 return 0;
410 }
411
> 412 static SIMPLE_DEV_PM_OPS(psxpad_spi_pm, psxpad_spi_suspend, psxpad_spi_resume);
413
414 static const struct spi_device_id psxpad_spi_id[] = {
415 { "psxpad-spi", 0 },

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (2.00 kB)
.config.gz (42.88 kB)
Download all attachments