Re: [PATCH v6] Input: psxpad-spi - Add PlayStation 1/2 joypads via SPI interface Driver
From: Tomohiro Yoshidomi
Date: Sat Apr 29 2017 - 23:37:02 EST
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 <dmitry.torokhov@xxxxxxxxx>:
> 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 <sylph23k@xxxxxxxxx>
>
> 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 <sylph23k@xxxxxxxxx>
>> + * 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 <sylph23k@xxxxxxxxx>");
>> +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