linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] input: add SparkFun Qwiic Joystick driver
@ 2021-05-31  9:54 Oleh Kravchenko
  2021-05-31 13:16 ` kernel test robot
  0 siblings, 1 reply; 9+ messages in thread
From: Oleh Kravchenko @ 2021-05-31  9:54 UTC (permalink / raw)
  To: linux-input
  Cc: Oleh Kravchenko, Benjamin Tissoires, Device Tree mailing list,
	Dmitry Torokhov, Jiri Kosina, Patchwork Bot

A simple analog joystick built on Low Power ATtiny85 Microcontroller.
Directional movements are measured with two 10 kΩ potentiometers
connected with a gimbal mechanism that separates the horizontal and
vertical movements. This joystick also has a select button that is actuated
when the joystick is pressed down.

Input events polled over the I2C bus.

Product page:
https://www.sparkfun.com/products/15168
Firmware and hardware sources:
https://github.com/sparkfun/Qwiic_Joystick

Tested on RPi4B and O4-iMX-NANO boards.

Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: Device Tree mailing list <devicetree@vger.kernel.org>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Jiri Kosina <jikos@jikos.cz>
Cc: Patchwork Bot <patchwork-bot@kernel.org>
Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
---
 .../bindings/input/qwiic-joystick.yaml        |  31 ++++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 drivers/input/joystick/Kconfig                |   9 +
 drivers/input/joystick/Makefile               |   1 +
 drivers/input/joystick/qwiic-joystick.c       | 170 ++++++++++++++++++
 5 files changed, 213 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/qwiic-joystick.yaml
 create mode 100644 drivers/input/joystick/qwiic-joystick.c

diff --git a/Documentation/devicetree/bindings/input/qwiic-joystick.yaml b/Documentation/devicetree/bindings/input/qwiic-joystick.yaml
new file mode 100644
index 000000000000..51cadeb350f2
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/qwiic-joystick.yaml
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2021 Oleh Kravchenko
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/input/qwiic-joystick.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: SparkFun Qwiic Joystick
+
+maintainers:
+  - Oleh Kravchenko <oleg@kaa.org.ua>
+
+description: |
+  Bindings for SparkFun Qwiic Joystick (COM-15168).
+  https://www.sparkfun.com/products/15168
+
+properties:
+  compatible:
+    const: sparkfun,qwiic-joystick
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    qwiic@20 {
+        compatible = "sparkfun,qwiic-joystick";
+        reg = <0x20>;
+    };
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 944b02bb96d7..a5631e68f5d1 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1060,6 +1060,8 @@ patternProperties:
     description: Sony Corporation
   "^spansion,.*":
     description: Spansion Inc.
+  "^sparkfun,.*":
+    description: SparkFun Electronics
   "^sprd,.*":
     description: Spreadtrum Communications Inc.
   "^sst,.*":
diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
index 5e38899058c1..7dfe8ea90923 100644
--- a/drivers/input/joystick/Kconfig
+++ b/drivers/input/joystick/Kconfig
@@ -372,6 +372,15 @@ config JOYSTICK_PXRC
 	  To compile this driver as a module, choose M here: the
 	  module will be called pxrc.
 
+config JOYSTICK_QWIIC
+	tristate "SparkFun Qwiic Joystick"
+	depends on I2C
+	help
+	  Say Y here if you want to use the SparkFun Qwiic Joystick.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called qwiic-joystick.
+
 config JOYSTICK_FSIA6B
 	tristate "FlySky FS-iA6B RC Receiver"
 	select SERIO
diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
index 31d720c9e493..5174b8aba2dd 100644
--- a/drivers/input/joystick/Makefile
+++ b/drivers/input/joystick/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_JOYSTICK_MAPLE)		+= maplecontrol.o
 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_SIDEWINDER)	+= sidewinder.o
 obj-$(CONFIG_JOYSTICK_SPACEBALL)	+= spaceball.o
 obj-$(CONFIG_JOYSTICK_SPACEORB)		+= spaceorb.o
diff --git a/drivers/input/joystick/qwiic-joystick.c b/drivers/input/joystick/qwiic-joystick.c
new file mode 100644
index 000000000000..a20793f7f2a3
--- /dev/null
+++ b/drivers/input/joystick/qwiic-joystick.c
@@ -0,0 +1,170 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2021 Oleh Kravchenko <oleg@kaa.org.ua>
+
+/*
+ * SparkFun Qwiic Joystick
+ * Product page:https://www.sparkfun.com/products/15168
+ * Firmware and hardware sources:https://github.com/sparkfun/Qwiic_Joystick
+ */
+
+#include <linux/bits.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input-polldev.h>
+#include <linux/module.h>
+
+#define QWIIC_JSK_REG_VERS	0
+#define QWIIC_JSK_REG_DATA	3
+
+#define QWIIC_JSK_MAX_AXIS	GENMASK(10, 0)
+#define QWIIC_JSK_FUZZ		2
+#define QWIIC_JSK_FLAT		2
+
+struct qwiic_jsk {
+	char			phys[32];
+	struct input_dev	*dev;
+	struct i2c_client	*i2c;
+};
+
+struct qwiic_ver {
+	u8 addr;
+	u8 major;
+	u8 minor;
+} __packed;
+
+struct qwiic_data {
+	u8 hx;
+	u8 lx;
+	u8 hy;
+	u8 ly;
+	u8 thumb;
+} __packed;
+
+static void qwiic_poll(struct input_dev *input)
+{
+	struct qwiic_jsk	*priv;
+	struct qwiic_data	data;
+	int			ret;
+	int			x, y, btn;
+
+	priv = input_get_drvdata(input);
+
+	ret = i2c_smbus_read_i2c_block_data(priv->i2c, QWIIC_JSK_REG_DATA,
+					    sizeof(data), (u8 *)&data);
+	if (ret == sizeof(data)) {
+		x = (data.hx << 8 | data.lx) >> 6;
+		y = (data.hy << 8 | data.ly) >> 6;
+		btn = !!!data.thumb;
+
+		input_report_abs(input, ABS_X, x);
+		input_report_abs(input, ABS_Y, y);
+		input_report_key(input, BTN_THUMBL, btn);
+
+		input_sync(input);
+	}
+}
+
+static int qwiic_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
+{
+	struct qwiic_jsk	*priv;
+	struct qwiic_ver	vers;
+	int			ret;
+
+	ret = i2c_smbus_read_i2c_block_data(i2c, QWIIC_JSK_REG_VERS,
+					    sizeof(vers), (u8 *)&vers);
+	if (ret != sizeof(vers)) {
+		ret = -EIO;
+		goto err;
+	}
+
+	if (i2c->addr != vers.addr) {
+		dev_err(&i2c->dev, "address doesn't match!\n");
+		ret = -ENODEV;
+		goto err;
+	}
+
+	dev_info(&i2c->dev, "SparkFun Qwiic Joystick, FW: %d.%d\n",
+		 vers.major, vers.minor);
+
+	priv = devm_kzalloc(&i2c->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	priv->i2c = i2c;
+	snprintf(priv->phys, sizeof(priv->phys), "i2c/%s", dev_name(&i2c->dev));
+	i2c_set_clientdata(i2c, priv);
+
+	priv->dev = devm_input_allocate_device(&i2c->dev);
+	if (!priv->dev) {
+		dev_err(&i2c->dev, "failed to allocate input device\n");
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	priv->dev->dev.parent = &i2c->dev;
+	priv->dev->id.bustype = BUS_I2C;
+	priv->dev->name = "SparkFun Qwiic Joystick";
+	priv->dev->phys = priv->phys;
+	input_set_drvdata(priv->dev, priv);
+
+	input_set_abs_params(priv->dev, ABS_X, 0, QWIIC_JSK_MAX_AXIS,
+			     QWIIC_JSK_FUZZ, QWIIC_JSK_FLAT);
+	input_set_abs_params(priv->dev, ABS_Y, 0, QWIIC_JSK_MAX_AXIS,
+			     QWIIC_JSK_FUZZ, QWIIC_JSK_FLAT);
+	input_set_capability(priv->dev, EV_KEY, BTN_THUMBL);
+
+	ret = input_setup_polling(priv->dev, qwiic_poll);
+	if (ret) {
+		dev_err(&i2c->dev, "failed to set up polling: %d\n", ret);
+		goto err;
+	}
+	input_set_poll_interval(priv->dev, 16);
+	input_set_min_poll_interval(priv->dev, 8);
+	input_set_max_poll_interval(priv->dev, 32);
+
+	ret = input_register_device(priv->dev);
+	if (ret)
+		dev_err(&i2c->dev, "failed to register joystick: %d\n", ret);
+
+err:
+	return ret;
+}
+
+static int qwiic_remove(struct i2c_client *i2c)
+{
+	struct qwiic_jsk *priv;
+
+	priv = i2c_get_clientdata(i2c);
+	input_unregister_device(priv->dev);
+
+	return 0;
+}
+
+static const struct of_device_id of_qwiic_match[] = {
+	{ .compatible = "sparkfun,qwiic-joystick", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_qwiic_match);
+
+static const struct i2c_device_id qwiic_id_table[] = {
+	{ KBUILD_MODNAME, 0 },
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, qwiic_id_table);
+
+static struct i2c_driver qwiic_driver = {
+	.driver = {
+		.name		= KBUILD_MODNAME,
+		.of_match_table	= of_match_ptr(of_qwiic_match),
+	},
+	.id_table	= qwiic_id_table,
+	.probe		= qwiic_probe,
+	.remove		= qwiic_remove,
+};
+module_i2c_driver(qwiic_driver);
+
+MODULE_AUTHOR("Oleh Kravchenko <oleg@kaa.org.ua>");
+MODULE_DESCRIPTION("SparkFun Qwiic Joystick driver");
+MODULE_LICENSE("GPL v2");
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] input: add SparkFun Qwiic Joystick driver
  2021-05-31  9:54 [PATCH] input: add SparkFun Qwiic Joystick driver Oleh Kravchenko
@ 2021-05-31 13:16 ` kernel test robot
  0 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2021-05-31 13:16 UTC (permalink / raw)
  To: Oleh Kravchenko, linux-input
  Cc: kbuild-all, Oleh Kravchenko, Benjamin Tissoires,
	Device Tree mailing list, Dmitry Torokhov, Jiri Kosina,
	Patchwork Bot

[-- Attachment #1: Type: text/plain, Size: 2228 bytes --]

Hi Oleh,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on input/next]
[also build test ERROR on hid/for-next robh/for-next v5.13-rc4 next-20210528]
[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]

url:    https://github.com/0day-ci/linux/commits/Oleh-Kravchenko/input-add-SparkFun-Qwiic-Joystick-driver/20210531-180329
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) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/9329c9d6369fa740480c97b2e7102e3d24ac8ae7
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Oleh-Kravchenko/input-add-SparkFun-Qwiic-Joystick-driver/20210531-180329
        git checkout 9329c9d6369fa740480c97b2e7102e3d24ac8ae7
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=ia64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/input/joystick/qwiic-joystick.c:13:10: fatal error: linux/input-polldev.h: No such file or directory
      13 | #include <linux/input-polldev.h>
         |          ^~~~~~~~~~~~~~~~~~~~~~~
   compilation terminated.


vim +13 drivers/input/joystick/qwiic-joystick.c

     3	
     4	/*
     5	 * SparkFun Qwiic Joystick
     6	 * Product page:https://www.sparkfun.com/products/15168
     7	 * Firmware and hardware sources:https://github.com/sparkfun/Qwiic_Joystick
     8	 */
     9	
    10	#include <linux/bits.h>
    11	#include <linux/i2c.h>
    12	#include <linux/input.h>
  > 13	#include <linux/input-polldev.h>
    14	#include <linux/module.h>
    15	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 63902 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] input: add SparkFun Qwiic Joystick driver
  2021-06-01  6:18 ` Dmitry Torokhov
@ 2021-06-01 20:55   ` Rob Herring
  0 siblings, 0 replies; 9+ messages in thread
From: Rob Herring @ 2021-06-01 20:55 UTC (permalink / raw)
  To: Dmitry Torokhov, Oleh Kravchenko
  Cc: linux-input, Benjamin Tissoires, Device Tree mailing list,
	Jiri Kosina, Patchwork Bot

On Mon, May 31, 2021 at 11:18:29PM -0700, Dmitry Torokhov wrote:
> Hi Oleh,
> 
> On Mon, May 31, 2021 at 04:32:43PM +0300, Oleh Kravchenko wrote:
> > A simple analog joystick built on Low Power ATtiny85 Microcontroller.
> > Directional movements are measured with two 10 kΩ potentiometers
> > connected with a gimbal mechanism that separates the horizontal and
> > vertical movements. This joystick also has a select button that is actuated
> > when the joystick is pressed down.
> > 
> > Input events polled over the I2C bus.
> 
> Thank you for the driver.
> 
> > 
> > Product page:
> > https://www.sparkfun.com/products/15168
> > Firmware and hardware sources:
> > https://github.com/sparkfun/Qwiic_Joystick
> > 
> > Tested on RPi4B and O4-iMX-NANO boards.
> > 
> > Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > Cc: Device Tree mailing list <devicetree@vger.kernel.org>
> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Cc: Jiri Kosina <jikos@jikos.cz>
> > Cc: Patchwork Bot <patchwork-bot@kernel.org>
> > Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
> > ---
> >  .../bindings/input/qwiic-joystick.yaml        |  31 ++++
> >  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
> >  drivers/input/joystick/Kconfig                |   9 +
> >  drivers/input/joystick/Makefile               |   1 +
> >  drivers/input/joystick/qwiic-joystick.c       | 169 ++++++++++++++++++
> >  5 files changed, 212 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/input/qwiic-joystick.yaml
> >  create mode 100644 drivers/input/joystick/qwiic-joystick.c
> > 
> > diff --git a/Documentation/devicetree/bindings/input/qwiic-joystick.yaml b/Documentation/devicetree/bindings/input/qwiic-joystick.yaml
> > new file mode 100644
> > index 000000000000..51cadeb350f2
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/qwiic-joystick.yaml
> > @@ -0,0 +1,31 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +# Copyright 2021 Oleh Kravchenko
> > +%YAML 1.2
> > +---
> > +$id: "http://devicetree.org/schemas/input/qwiic-joystick.yaml#"
> > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > +
> > +title: SparkFun Qwiic Joystick
> > +
> > +maintainers:
> > +  - Oleh Kravchenko <oleg@kaa.org.ua>
> > +
> > +description: |
> > +  Bindings for SparkFun Qwiic Joystick (COM-15168).
> > +  https://www.sparkfun.com/products/15168
> > +
> > +properties:
> > +  compatible:
> > +    const: sparkfun,qwiic-joystick
> 
> Rob, is this compatible acceptable?

Yes.

> 
> > +
> > +required:
> > +  - compatible
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    qwiic@20 {
> > +        compatible = "sparkfun,qwiic-joystick";
> > +        reg = <0x20>;
> > +    };
> 
> I think this device would be covered by
> Documentation/devicetree/bindings/trivial-devices.yaml so please add its
> compatible there.

That's fine. Or not.

Please make bindings a separate patch.

Rob

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] input: add SparkFun Qwiic Joystick driver
  2021-05-31 13:32 Oleh Kravchenko
  2021-05-31 22:58 ` Jeff LaBundy
  2021-06-01  6:18 ` Dmitry Torokhov
@ 2021-06-01 13:32 ` Rob Herring
  2 siblings, 0 replies; 9+ messages in thread
From: Rob Herring @ 2021-06-01 13:32 UTC (permalink / raw)
  To: Oleh Kravchenko
  Cc: linux-input, Benjamin Tissoires, Device Tree mailing list,
	Dmitry Torokhov, Patchwork Bot, Jiri Kosina

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 3333 bytes --]

On Mon, 31 May 2021 16:32:43 +0300, Oleh Kravchenko wrote:
> A simple analog joystick built on Low Power ATtiny85 Microcontroller.
> Directional movements are measured with two 10 kΩ potentiometers
> connected with a gimbal mechanism that separates the horizontal and
> vertical movements. This joystick also has a select button that is actuated
> when the joystick is pressed down.
> 
> Input events polled over the I2C bus.
> 
> Product page:
> https://www.sparkfun.com/products/15168
> Firmware and hardware sources:
> https://github.com/sparkfun/Qwiic_Joystick
> 
> Tested on RPi4B and O4-iMX-NANO boards.
> 
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Cc: Device Tree mailing list <devicetree@vger.kernel.org>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Jiri Kosina <jikos@jikos.cz>
> Cc: Patchwork Bot <patchwork-bot@kernel.org>
> Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
> ---
>  .../bindings/input/qwiic-joystick.yaml        |  31 ++++
>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
>  drivers/input/joystick/Kconfig                |   9 +
>  drivers/input/joystick/Makefile               |   1 +
>  drivers/input/joystick/qwiic-joystick.c       | 169 ++++++++++++++++++
>  5 files changed, 212 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/qwiic-joystick.yaml
>  create mode 100644 drivers/input/joystick/qwiic-joystick.c
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/input/qwiic-joystick.example.dts:21.13-26: Warning (reg_format): /example-0/qwiic@20:reg: property has invalid length (4 bytes) (#address-cells == 1, #size-cells == 1)
Documentation/devicetree/bindings/input/qwiic-joystick.example.dt.yaml: Warning (pci_device_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/input/qwiic-joystick.example.dt.yaml: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/input/qwiic-joystick.example.dt.yaml: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/input/qwiic-joystick.example.dt.yaml: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/input/qwiic-joystick.example.dt.yaml: Warning (spi_bus_reg): Failed prerequisite 'reg_format'
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/input/qwiic-joystick.example.dt.yaml: example-0: qwiic@20:reg:0: [32] is too short
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/input/qwiic-joystick.example.dt.yaml: qwiic@20: 'reg' does not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/input/qwiic-joystick.yaml

See https://patchwork.ozlabs.org/patch/1485677

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] input: add SparkFun Qwiic Joystick driver
  2021-05-31 13:32 Oleh Kravchenko
  2021-05-31 22:58 ` Jeff LaBundy
@ 2021-06-01  6:18 ` Dmitry Torokhov
  2021-06-01 20:55   ` Rob Herring
  2021-06-01 13:32 ` Rob Herring
  2 siblings, 1 reply; 9+ messages in thread
From: Dmitry Torokhov @ 2021-06-01  6:18 UTC (permalink / raw)
  To: Oleh Kravchenko, Rob Herring
  Cc: linux-input, Benjamin Tissoires, Device Tree mailing list,
	Jiri Kosina, Patchwork Bot

Hi Oleh,

On Mon, May 31, 2021 at 04:32:43PM +0300, Oleh Kravchenko wrote:
> A simple analog joystick built on Low Power ATtiny85 Microcontroller.
> Directional movements are measured with two 10 kΩ potentiometers
> connected with a gimbal mechanism that separates the horizontal and
> vertical movements. This joystick also has a select button that is actuated
> when the joystick is pressed down.
> 
> Input events polled over the I2C bus.

Thank you for the driver.

> 
> Product page:
> https://www.sparkfun.com/products/15168
> Firmware and hardware sources:
> https://github.com/sparkfun/Qwiic_Joystick
> 
> Tested on RPi4B and O4-iMX-NANO boards.
> 
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Cc: Device Tree mailing list <devicetree@vger.kernel.org>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Jiri Kosina <jikos@jikos.cz>
> Cc: Patchwork Bot <patchwork-bot@kernel.org>
> Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
> ---
>  .../bindings/input/qwiic-joystick.yaml        |  31 ++++
>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
>  drivers/input/joystick/Kconfig                |   9 +
>  drivers/input/joystick/Makefile               |   1 +
>  drivers/input/joystick/qwiic-joystick.c       | 169 ++++++++++++++++++
>  5 files changed, 212 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/qwiic-joystick.yaml
>  create mode 100644 drivers/input/joystick/qwiic-joystick.c
> 
> diff --git a/Documentation/devicetree/bindings/input/qwiic-joystick.yaml b/Documentation/devicetree/bindings/input/qwiic-joystick.yaml
> new file mode 100644
> index 000000000000..51cadeb350f2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/qwiic-joystick.yaml
> @@ -0,0 +1,31 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2021 Oleh Kravchenko
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/input/qwiic-joystick.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: SparkFun Qwiic Joystick
> +
> +maintainers:
> +  - Oleh Kravchenko <oleg@kaa.org.ua>
> +
> +description: |
> +  Bindings for SparkFun Qwiic Joystick (COM-15168).
> +  https://www.sparkfun.com/products/15168
> +
> +properties:
> +  compatible:
> +    const: sparkfun,qwiic-joystick

Rob, is this compatible acceptable?

> +
> +required:
> +  - compatible
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    qwiic@20 {
> +        compatible = "sparkfun,qwiic-joystick";
> +        reg = <0x20>;
> +    };

I think this device would be covered by
Documentation/devicetree/bindings/trivial-devices.yaml so please add its
compatible there.

> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index 944b02bb96d7..a5631e68f5d1 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -1060,6 +1060,8 @@ patternProperties:
>      description: Sony Corporation
>    "^spansion,.*":
>      description: Spansion Inc.
> +  "^sparkfun,.*":
> +    description: SparkFun Electronics

This should be a separate patch, please also send to Rob Herring and
device tree list.

>    "^sprd,.*":
>      description: Spreadtrum Communications Inc.
>    "^sst,.*":
> diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
> index 5e38899058c1..7dfe8ea90923 100644
> --- a/drivers/input/joystick/Kconfig
> +++ b/drivers/input/joystick/Kconfig
> @@ -372,6 +372,15 @@ config JOYSTICK_PXRC
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called pxrc.
>  
> +config JOYSTICK_QWIIC
> +	tristate "SparkFun Qwiic Joystick"
> +	depends on I2C
> +	help
> +	  Say Y here if you want to use the SparkFun Qwiic Joystick.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called qwiic-joystick.
> +
>  config JOYSTICK_FSIA6B
>  	tristate "FlySky FS-iA6B RC Receiver"
>  	select SERIO
> diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
> index 31d720c9e493..5174b8aba2dd 100644
> --- a/drivers/input/joystick/Makefile
> +++ b/drivers/input/joystick/Makefile
> @@ -27,6 +27,7 @@ obj-$(CONFIG_JOYSTICK_MAPLE)		+= maplecontrol.o
>  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_SIDEWINDER)	+= sidewinder.o
>  obj-$(CONFIG_JOYSTICK_SPACEBALL)	+= spaceball.o
>  obj-$(CONFIG_JOYSTICK_SPACEORB)		+= spaceorb.o
> diff --git a/drivers/input/joystick/qwiic-joystick.c b/drivers/input/joystick/qwiic-joystick.c
> new file mode 100644
> index 000000000000..9b7ab0dd1218
> --- /dev/null
> +++ b/drivers/input/joystick/qwiic-joystick.c
> @@ -0,0 +1,169 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2021 Oleh Kravchenko <oleg@kaa.org.ua>
> +
> +/*
> + * SparkFun Qwiic Joystick
> + * Product page:https://www.sparkfun.com/products/15168
> + * Firmware and hardware sources:https://github.com/sparkfun/Qwiic_Joystick
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/module.h>
> +
> +#define QWIIC_JSK_REG_VERS	0
> +#define QWIIC_JSK_REG_DATA	3
> +
> +#define QWIIC_JSK_MAX_AXIS	GENMASK(10, 0)
> +#define QWIIC_JSK_FUZZ		2
> +#define QWIIC_JSK_FLAT		2
> +
> +struct qwiic_jsk {
> +	char			phys[32];
> +	struct input_dev	*dev;
> +	struct i2c_client	*i2c;
> +};
> +
> +struct qwiic_ver {
> +	u8 addr;
> +	u8 major;
> +	u8 minor;
> +} __packed;

There is no reason to declare structure containing only bytes as packed.

> +
> +struct qwiic_data {
> +	u8 hx;
> +	u8 lx;
> +	u8 hy;
> +	u8 ly;
> +	u8 thumb;
> +} __packed;
> +
> +static void qwiic_poll(struct input_dev *input)
> +{
> +	struct qwiic_jsk	*priv;
> +	struct qwiic_data	data;
> +	int			ret;
> +	int			x, y, btn;
> +
> +	priv = input_get_drvdata(input);
> +
> +	ret = i2c_smbus_read_i2c_block_data(priv->i2c, QWIIC_JSK_REG_DATA,
> +					    sizeof(data), (u8 *)&data);
> +	if (ret == sizeof(data)) {
> +		x = (data.hx << 8 | data.lx) >> 6;

Why not define data as a u8 array, and then do

		u16 x = be16_to_cpup(data) >> 6;
		u16 y = be16_to_cpup(data + 2) >> 6;

Or declare coordinates as __be16 in qwiic_data (and use be16_to_cpu when
accessing).

> +		y = (data.hy << 8 | data.ly) >> 6;
> +		btn = !!!data.thumb;

Triple negation is too much, one should be enough.

> +
> +		input_report_abs(input, ABS_X, x);
> +		input_report_abs(input, ABS_Y, y);
> +		input_report_key(input, BTN_THUMBL, btn);
> +
> +		input_sync(input);
> +	}
> +}
> +
> +static int qwiic_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
> +{
> +	struct qwiic_jsk	*priv;
> +	struct qwiic_ver	vers;
> +	int			ret;
> +
> +	ret = i2c_smbus_read_i2c_block_data(i2c, QWIIC_JSK_REG_VERS,
> +					    sizeof(vers), (u8 *)&vers);
> +	if (ret != sizeof(vers)) {
> +		ret = -EIO;
> +		goto err;
> +	}
> +
> +	if (i2c->addr != vers.addr) {
> +		dev_err(&i2c->dev, "address doesn't match!\n");
> +		ret = -ENODEV;
> +		goto err;
> +	}
> +
> +	dev_info(&i2c->dev, "SparkFun Qwiic Joystick, FW: %d.%d\n",
> +		 vers.major, vers.minor);

Preference is not to be too noisy, please change to dev_dbg().

> +
> +	priv = devm_kzalloc(&i2c->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv) {
> +		ret = -ENOMEM;

Return -ENOMEM directly, no need to use labels when you do not need to
do any cleanup by hand.

> +		goto err;
> +	}
> +
> +	priv->i2c = i2c;
> +	snprintf(priv->phys, sizeof(priv->phys), "i2c/%s", dev_name(&i2c->dev));
> +	i2c_set_clientdata(i2c, priv);
> +
> +	priv->dev = devm_input_allocate_device(&i2c->dev);
> +	if (!priv->dev) {
> +		dev_err(&i2c->dev, "failed to allocate input device\n");
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +
> +	priv->dev->dev.parent = &i2c->dev;

Done by devm_input_allocate_device(), please drop.

> +	priv->dev->id.bustype = BUS_I2C;
> +	priv->dev->name = "SparkFun Qwiic Joystick";
> +	priv->dev->phys = priv->phys;
> +	input_set_drvdata(priv->dev, priv);
> +
> +	input_set_abs_params(priv->dev, ABS_X, 0, QWIIC_JSK_MAX_AXIS,
> +			     QWIIC_JSK_FUZZ, QWIIC_JSK_FLAT);
> +	input_set_abs_params(priv->dev, ABS_Y, 0, QWIIC_JSK_MAX_AXIS,
> +			     QWIIC_JSK_FUZZ, QWIIC_JSK_FLAT);
> +	input_set_capability(priv->dev, EV_KEY, BTN_THUMBL);
> +
> +	ret = input_setup_polling(priv->dev, qwiic_poll);

Please call variables that carry error code or 0 "error" and not "ret".

> +	if (ret) {
> +		dev_err(&i2c->dev, "failed to set up polling: %d\n", ret);
> +		goto err;
> +	}
> +	input_set_poll_interval(priv->dev, 16);
> +	input_set_min_poll_interval(priv->dev, 8);
> +	input_set_max_poll_interval(priv->dev, 32);
> +
> +	ret = input_register_device(priv->dev);
> +	if (ret)
> +		dev_err(&i2c->dev, "failed to register joystick: %d\n", ret);
> +
> +err:
> +	return ret;
> +}
> +
> +static int qwiic_remove(struct i2c_client *i2c)
> +{
> +	struct qwiic_jsk *priv;
> +
> +	priv = i2c_get_clientdata(i2c);
> +	input_unregister_device(priv->dev);

Since you are using devm_input_allocate_device() you do not need to
manually unregister it, it will be done automatically. Drop this and you
can remove qwiic_remove() altogether.

> +
> +	return 0;
> +}
> +
> +static const struct of_device_id of_qwiic_match[] = {
> +	{ .compatible = "sparkfun,qwiic-joystick", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, of_qwiic_match);
> +
> +static const struct i2c_device_id qwiic_id_table[] = {
> +	{ KBUILD_MODNAME, 0 },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(i2c, qwiic_id_table);
> +
> +static struct i2c_driver qwiic_driver = {
> +	.driver = {
> +		.name		= KBUILD_MODNAME,
> +		.of_match_table	= of_match_ptr(of_qwiic_match),

You either need to guard of_qwiic_match with #ifdef CONFIG_OF, or drop
of_match_ptr() and assign of_match_table directly, otherwise for certain
configs you may get "defined but not used" warning.

> +	},
> +	.id_table	= qwiic_id_table,
> +	.probe		= qwiic_probe,
> +	.remove		= qwiic_remove,
> +};
> +module_i2c_driver(qwiic_driver);
> +
> +MODULE_AUTHOR("Oleh Kravchenko <oleg@kaa.org.ua>");
> +MODULE_DESCRIPTION("SparkFun Qwiic Joystick driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.26.3
> 

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] input: add SparkFun Qwiic Joystick driver
  2021-05-31 13:32 Oleh Kravchenko
@ 2021-05-31 22:58 ` Jeff LaBundy
  2021-06-01  6:18 ` Dmitry Torokhov
  2021-06-01 13:32 ` Rob Herring
  2 siblings, 0 replies; 9+ messages in thread
From: Jeff LaBundy @ 2021-05-31 22:58 UTC (permalink / raw)
  To: Oleh Kravchenko
  Cc: linux-input, Benjamin Tissoires, Device Tree mailing list,
	Dmitry Torokhov, Jiri Kosina, Patchwork Bot

Hi Oleh,

Neat little device; some comments below.

On Mon, May 31, 2021 at 04:32:43PM +0300, Oleh Kravchenko wrote:
> A simple analog joystick built on Low Power ATtiny85 Microcontroller.
> Directional movements are measured with two 10 kΩ potentiometers
> connected with a gimbal mechanism that separates the horizontal and
> vertical movements. This joystick also has a select button that is actuated
> when the joystick is pressed down.
> 
> Input events polled over the I2C bus.
> 
> Product page:
> https://www.sparkfun.com/products/15168
> Firmware and hardware sources:
> https://github.com/sparkfun/Qwiic_Joystick
> 
> Tested on RPi4B and O4-iMX-NANO boards.
> 
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Cc: Device Tree mailing list <devicetree@vger.kernel.org>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Jiri Kosina <jikos@jikos.cz>
> Cc: Patchwork Bot <patchwork-bot@kernel.org>
> Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
> ---

Please use a version number (e.g. v2, v3) and add a change log under
the '---' when sending new versions.

>  .../bindings/input/qwiic-joystick.yaml        |  31 ++++
>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
>  drivers/input/joystick/Kconfig                |   9 +
>  drivers/input/joystick/Makefile               |   1 +
>  drivers/input/joystick/qwiic-joystick.c       | 169 ++++++++++++++++++
>  5 files changed, 212 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/qwiic-joystick.yaml
>  create mode 100644 drivers/input/joystick/qwiic-joystick.c

Bindings must be a separate patch with Rob Herring (see MAINTAINERS)
on the To: list.

> 
> diff --git a/Documentation/devicetree/bindings/input/qwiic-joystick.yaml b/Documentation/devicetree/bindings/input/qwiic-joystick.yaml
> new file mode 100644
> index 000000000000..51cadeb350f2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/qwiic-joystick.yaml
> @@ -0,0 +1,31 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2021 Oleh Kravchenko
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/input/qwiic-joystick.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: SparkFun Qwiic Joystick
> +
> +maintainers:
> +  - Oleh Kravchenko <oleg@kaa.org.ua>
> +
> +description: |
> +  Bindings for SparkFun Qwiic Joystick (COM-15168).
> +  https://www.sparkfun.com/products/15168
> +
> +properties:
> +  compatible:
> +    const: sparkfun,qwiic-joystick
> +
> +required:
> +  - compatible

'reg' is required as well.

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    qwiic@20 {
> +        compatible = "sparkfun,qwiic-joystick";
> +        reg = <0x20>;
> +    };
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index 944b02bb96d7..a5631e68f5d1 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -1060,6 +1060,8 @@ patternProperties:
>      description: Sony Corporation
>    "^spansion,.*":
>      description: Spansion Inc.
> +  "^sparkfun,.*":
> +    description: SparkFun Electronics
>    "^sprd,.*":
>      description: Spreadtrum Communications Inc.
>    "^sst,.*":
> diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
> index 5e38899058c1..7dfe8ea90923 100644
> --- a/drivers/input/joystick/Kconfig
> +++ b/drivers/input/joystick/Kconfig
> @@ -372,6 +372,15 @@ config JOYSTICK_PXRC
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called pxrc.
>  
> +config JOYSTICK_QWIIC
> +	tristate "SparkFun Qwiic Joystick"
> +	depends on I2C
> +	help
> +	  Say Y here if you want to use the SparkFun Qwiic Joystick.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called qwiic-joystick.
> +
>  config JOYSTICK_FSIA6B
>  	tristate "FlySky FS-iA6B RC Receiver"
>  	select SERIO
> diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
> index 31d720c9e493..5174b8aba2dd 100644
> --- a/drivers/input/joystick/Makefile
> +++ b/drivers/input/joystick/Makefile
> @@ -27,6 +27,7 @@ obj-$(CONFIG_JOYSTICK_MAPLE)		+= maplecontrol.o
>  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_SIDEWINDER)	+= sidewinder.o
>  obj-$(CONFIG_JOYSTICK_SPACEBALL)	+= spaceball.o
>  obj-$(CONFIG_JOYSTICK_SPACEORB)		+= spaceorb.o
> diff --git a/drivers/input/joystick/qwiic-joystick.c b/drivers/input/joystick/qwiic-joystick.c
> new file mode 100644
> index 000000000000..9b7ab0dd1218
> --- /dev/null
> +++ b/drivers/input/joystick/qwiic-joystick.c
> @@ -0,0 +1,169 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2021 Oleh Kravchenko <oleg@kaa.org.ua>
> +
> +/*
> + * SparkFun Qwiic Joystick
> + * Product page:https://www.sparkfun.com/products/15168
> + * Firmware and hardware sources:https://github.com/sparkfun/Qwiic_Joystick
> + */

Nit: typically the copyright information goes in the multi-line introductory
comments; there is no need for a newline after the SPDX identifier either.

> +
> +#include <linux/bits.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/module.h>
> +
> +#define QWIIC_JSK_REG_VERS	0
> +#define QWIIC_JSK_REG_DATA	3
> +
> +#define QWIIC_JSK_MAX_AXIS	GENMASK(10, 0)
> +#define QWIIC_JSK_FUZZ		2
> +#define QWIIC_JSK_FLAT		2
> +
> +struct qwiic_jsk {
> +	char			phys[32];
> +	struct input_dev	*dev;
> +	struct i2c_client	*i2c;
> +};
> +
> +struct qwiic_ver {
> +	u8 addr;
> +	u8 major;
> +	u8 minor;
> +} __packed;
> +
> +struct qwiic_data {
> +	u8 hx;
> +	u8 lx;
> +	u8 hy;
> +	u8 ly;
> +	u8 thumb;
> +} __packed;

Let's use __be16 for 16-bit big-endian horizontal and vertical position.

> +
> +static void qwiic_poll(struct input_dev *input)
> +{
> +	struct qwiic_jsk	*priv;
> +	struct qwiic_data	data;
> +	int			ret;
> +	int			x, y, btn;

No need to align these declarations.

> +
> +	priv = input_get_drvdata(input);
> +
> +	ret = i2c_smbus_read_i2c_block_data(priv->i2c, QWIIC_JSK_REG_DATA,
> +					    sizeof(data), (u8 *)&data);
> +	if (ret == sizeof(data)) {

This is personal preference, but returning early on error avoids having
to indent the rest of the function.

> +		x = (data.hx << 8 | data.lx) >> 6;
> +		y = (data.hy << 8 | data.ly) >> 6;

With hx/lx and hy/ly defined using __be16, you can use be16_to_cpu() to
resolve the 16-bit value. A comment to explain the 6-bit shift from the
documentation would be useful too.

> +		btn = !!!data.thumb;

I don't think the triple-not is necessary as input_report_key() already
squashes non-zero values to 1 with '!!'. If register 0x07's polarity is
inverted, just pass !data.thumb to input_report_key().

> +
> +		input_report_abs(input, ABS_X, x);
> +		input_report_abs(input, ABS_Y, y);
> +		input_report_key(input, BTN_THUMBL, btn);
> +
> +		input_sync(input);
> +	}
> +}
> +
> +static int qwiic_probe(struct i2c_client *i2c, const struct i2c_device_id *id)

It's much more common (and easier to read) if the i2c_client struct is
passed as *client.

> +{
> +	struct qwiic_jsk	*priv;
> +	struct qwiic_ver	vers;
> +	int			ret;

No need to align these declarations.

> +
> +	ret = i2c_smbus_read_i2c_block_data(i2c, QWIIC_JSK_REG_VERS,
> +					    sizeof(vers), (u8 *)&vers);
> +	if (ret != sizeof(vers)) {
> +		ret = -EIO;
> +		goto err;
> +	}

If i2c_smbus_read_i2c_block_data() returned a negative value, the original
return value should be preserved. Only return -EIO if 0 <= ret < sizeof().

You can also simply return directly; there is nothing to be undone at this
stage.

> +
> +	if (i2c->addr != vers.addr) {
> +		dev_err(&i2c->dev, "address doesn't match!\n");
> +		ret = -ENODEV;
> +		goto err;
> +	}

This error should be impossible. If the address in dts does not match what
the hardware is expecting, i2c_smbus_read_i2c_block_data() would have been
NAK'd in the first place.

> +
> +	dev_info(&i2c->dev, "SparkFun Qwiic Joystick, FW: %d.%d\n",
> +		 vers.major, vers.minor);

This should be dev_dbg() at most.

> +
> +	priv = devm_kzalloc(&i2c->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv) {
> +		ret = -ENOMEM;
> +		goto err;
> +	}

Just return -ENOMEM here.

> +
> +	priv->i2c = i2c;
> +	snprintf(priv->phys, sizeof(priv->phys), "i2c/%s", dev_name(&i2c->dev));
> +	i2c_set_clientdata(i2c, priv);
> +
> +	priv->dev = devm_input_allocate_device(&i2c->dev);
> +	if (!priv->dev) {
> +		dev_err(&i2c->dev, "failed to allocate input device\n");

No need for a print here.

> +		ret = -ENOMEM;
> +		goto err;

Just return -ENOMEM here.

> +	}
> +
> +	priv->dev->dev.parent = &i2c->dev;

This is not necessary; devm_input_allocate_device() takes care of it.

> +	priv->dev->id.bustype = BUS_I2C;
> +	priv->dev->name = "SparkFun Qwiic Joystick";
> +	priv->dev->phys = priv->phys;
> +	input_set_drvdata(priv->dev, priv);
> +
> +	input_set_abs_params(priv->dev, ABS_X, 0, QWIIC_JSK_MAX_AXIS,
> +			     QWIIC_JSK_FUZZ, QWIIC_JSK_FLAT);
> +	input_set_abs_params(priv->dev, ABS_Y, 0, QWIIC_JSK_MAX_AXIS,
> +			     QWIIC_JSK_FUZZ, QWIIC_JSK_FLAT);
> +	input_set_capability(priv->dev, EV_KEY, BTN_THUMBL);
> +
> +	ret = input_setup_polling(priv->dev, qwiic_poll);
> +	if (ret) {
> +		dev_err(&i2c->dev, "failed to set up polling: %d\n", ret);
> +		goto err;
> +	}

Again, you can just return 'ret' here.

> +	input_set_poll_interval(priv->dev, 16);
> +	input_set_min_poll_interval(priv->dev, 8);
> +	input_set_max_poll_interval(priv->dev, 32);

Is there any reason not to #define these intervals?

> +
> +	ret = input_register_device(priv->dev);
> +	if (ret)
> +		dev_err(&i2c->dev, "failed to register joystick: %d\n", ret);
> +
> +err:

This goto label is unnecessary.

> +	return ret;
> +}
> +
> +static int qwiic_remove(struct i2c_client *i2c)
> +{
> +	struct qwiic_jsk *priv;
> +
> +	priv = i2c_get_clientdata(i2c);
> +	input_unregister_device(priv->dev);
> +
> +	return 0;
> +}

Because the input device was created with devm_input_allocate_device(),
input_register_device() is managed as well. Therefore you can drop the
remove callback altogether.

> +
> +static const struct of_device_id of_qwiic_match[] = {
> +	{ .compatible = "sparkfun,qwiic-joystick", },
> +	{},

Nit: { }

> +};
> +MODULE_DEVICE_TABLE(of, of_qwiic_match);
> +
> +static const struct i2c_device_id qwiic_id_table[] = {
> +	{ KBUILD_MODNAME, 0 },
> +	{},

And here (see other drivers).

> +};
> +MODULE_DEVICE_TABLE(i2c, qwiic_id_table);
> +
> +static struct i2c_driver qwiic_driver = {
> +	.driver = {
> +		.name		= KBUILD_MODNAME,
> +		.of_match_table	= of_match_ptr(of_qwiic_match),
> +	},
> +	.id_table	= qwiic_id_table,
> +	.probe		= qwiic_probe,
> +	.remove		= qwiic_remove,
> +};
> +module_i2c_driver(qwiic_driver);

No need for alignment here either.

> +
> +MODULE_AUTHOR("Oleh Kravchenko <oleg@kaa.org.ua>");
> +MODULE_DESCRIPTION("SparkFun Qwiic Joystick driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.26.3
> 

Kind regards,
Jeff LaBundy

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] input: add SparkFun Qwiic Joystick driver
@ 2021-05-31 13:32 Oleh Kravchenko
  2021-05-31 22:58 ` Jeff LaBundy
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Oleh Kravchenko @ 2021-05-31 13:32 UTC (permalink / raw)
  To: linux-input
  Cc: Oleh Kravchenko, Benjamin Tissoires, Device Tree mailing list,
	Dmitry Torokhov, Jiri Kosina, Patchwork Bot

A simple analog joystick built on Low Power ATtiny85 Microcontroller.
Directional movements are measured with two 10 kΩ potentiometers
connected with a gimbal mechanism that separates the horizontal and
vertical movements. This joystick also has a select button that is actuated
when the joystick is pressed down.

Input events polled over the I2C bus.

Product page:
https://www.sparkfun.com/products/15168
Firmware and hardware sources:
https://github.com/sparkfun/Qwiic_Joystick

Tested on RPi4B and O4-iMX-NANO boards.

Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: Device Tree mailing list <devicetree@vger.kernel.org>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Jiri Kosina <jikos@jikos.cz>
Cc: Patchwork Bot <patchwork-bot@kernel.org>
Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
---
 .../bindings/input/qwiic-joystick.yaml        |  31 ++++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 drivers/input/joystick/Kconfig                |   9 +
 drivers/input/joystick/Makefile               |   1 +
 drivers/input/joystick/qwiic-joystick.c       | 169 ++++++++++++++++++
 5 files changed, 212 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/qwiic-joystick.yaml
 create mode 100644 drivers/input/joystick/qwiic-joystick.c

diff --git a/Documentation/devicetree/bindings/input/qwiic-joystick.yaml b/Documentation/devicetree/bindings/input/qwiic-joystick.yaml
new file mode 100644
index 000000000000..51cadeb350f2
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/qwiic-joystick.yaml
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2021 Oleh Kravchenko
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/input/qwiic-joystick.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: SparkFun Qwiic Joystick
+
+maintainers:
+  - Oleh Kravchenko <oleg@kaa.org.ua>
+
+description: |
+  Bindings for SparkFun Qwiic Joystick (COM-15168).
+  https://www.sparkfun.com/products/15168
+
+properties:
+  compatible:
+    const: sparkfun,qwiic-joystick
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    qwiic@20 {
+        compatible = "sparkfun,qwiic-joystick";
+        reg = <0x20>;
+    };
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 944b02bb96d7..a5631e68f5d1 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1060,6 +1060,8 @@ patternProperties:
     description: Sony Corporation
   "^spansion,.*":
     description: Spansion Inc.
+  "^sparkfun,.*":
+    description: SparkFun Electronics
   "^sprd,.*":
     description: Spreadtrum Communications Inc.
   "^sst,.*":
diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
index 5e38899058c1..7dfe8ea90923 100644
--- a/drivers/input/joystick/Kconfig
+++ b/drivers/input/joystick/Kconfig
@@ -372,6 +372,15 @@ config JOYSTICK_PXRC
 	  To compile this driver as a module, choose M here: the
 	  module will be called pxrc.
 
+config JOYSTICK_QWIIC
+	tristate "SparkFun Qwiic Joystick"
+	depends on I2C
+	help
+	  Say Y here if you want to use the SparkFun Qwiic Joystick.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called qwiic-joystick.
+
 config JOYSTICK_FSIA6B
 	tristate "FlySky FS-iA6B RC Receiver"
 	select SERIO
diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
index 31d720c9e493..5174b8aba2dd 100644
--- a/drivers/input/joystick/Makefile
+++ b/drivers/input/joystick/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_JOYSTICK_MAPLE)		+= maplecontrol.o
 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_SIDEWINDER)	+= sidewinder.o
 obj-$(CONFIG_JOYSTICK_SPACEBALL)	+= spaceball.o
 obj-$(CONFIG_JOYSTICK_SPACEORB)		+= spaceorb.o
diff --git a/drivers/input/joystick/qwiic-joystick.c b/drivers/input/joystick/qwiic-joystick.c
new file mode 100644
index 000000000000..9b7ab0dd1218
--- /dev/null
+++ b/drivers/input/joystick/qwiic-joystick.c
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2021 Oleh Kravchenko <oleg@kaa.org.ua>
+
+/*
+ * SparkFun Qwiic Joystick
+ * Product page:https://www.sparkfun.com/products/15168
+ * Firmware and hardware sources:https://github.com/sparkfun/Qwiic_Joystick
+ */
+
+#include <linux/bits.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/module.h>
+
+#define QWIIC_JSK_REG_VERS	0
+#define QWIIC_JSK_REG_DATA	3
+
+#define QWIIC_JSK_MAX_AXIS	GENMASK(10, 0)
+#define QWIIC_JSK_FUZZ		2
+#define QWIIC_JSK_FLAT		2
+
+struct qwiic_jsk {
+	char			phys[32];
+	struct input_dev	*dev;
+	struct i2c_client	*i2c;
+};
+
+struct qwiic_ver {
+	u8 addr;
+	u8 major;
+	u8 minor;
+} __packed;
+
+struct qwiic_data {
+	u8 hx;
+	u8 lx;
+	u8 hy;
+	u8 ly;
+	u8 thumb;
+} __packed;
+
+static void qwiic_poll(struct input_dev *input)
+{
+	struct qwiic_jsk	*priv;
+	struct qwiic_data	data;
+	int			ret;
+	int			x, y, btn;
+
+	priv = input_get_drvdata(input);
+
+	ret = i2c_smbus_read_i2c_block_data(priv->i2c, QWIIC_JSK_REG_DATA,
+					    sizeof(data), (u8 *)&data);
+	if (ret == sizeof(data)) {
+		x = (data.hx << 8 | data.lx) >> 6;
+		y = (data.hy << 8 | data.ly) >> 6;
+		btn = !!!data.thumb;
+
+		input_report_abs(input, ABS_X, x);
+		input_report_abs(input, ABS_Y, y);
+		input_report_key(input, BTN_THUMBL, btn);
+
+		input_sync(input);
+	}
+}
+
+static int qwiic_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
+{
+	struct qwiic_jsk	*priv;
+	struct qwiic_ver	vers;
+	int			ret;
+
+	ret = i2c_smbus_read_i2c_block_data(i2c, QWIIC_JSK_REG_VERS,
+					    sizeof(vers), (u8 *)&vers);
+	if (ret != sizeof(vers)) {
+		ret = -EIO;
+		goto err;
+	}
+
+	if (i2c->addr != vers.addr) {
+		dev_err(&i2c->dev, "address doesn't match!\n");
+		ret = -ENODEV;
+		goto err;
+	}
+
+	dev_info(&i2c->dev, "SparkFun Qwiic Joystick, FW: %d.%d\n",
+		 vers.major, vers.minor);
+
+	priv = devm_kzalloc(&i2c->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	priv->i2c = i2c;
+	snprintf(priv->phys, sizeof(priv->phys), "i2c/%s", dev_name(&i2c->dev));
+	i2c_set_clientdata(i2c, priv);
+
+	priv->dev = devm_input_allocate_device(&i2c->dev);
+	if (!priv->dev) {
+		dev_err(&i2c->dev, "failed to allocate input device\n");
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	priv->dev->dev.parent = &i2c->dev;
+	priv->dev->id.bustype = BUS_I2C;
+	priv->dev->name = "SparkFun Qwiic Joystick";
+	priv->dev->phys = priv->phys;
+	input_set_drvdata(priv->dev, priv);
+
+	input_set_abs_params(priv->dev, ABS_X, 0, QWIIC_JSK_MAX_AXIS,
+			     QWIIC_JSK_FUZZ, QWIIC_JSK_FLAT);
+	input_set_abs_params(priv->dev, ABS_Y, 0, QWIIC_JSK_MAX_AXIS,
+			     QWIIC_JSK_FUZZ, QWIIC_JSK_FLAT);
+	input_set_capability(priv->dev, EV_KEY, BTN_THUMBL);
+
+	ret = input_setup_polling(priv->dev, qwiic_poll);
+	if (ret) {
+		dev_err(&i2c->dev, "failed to set up polling: %d\n", ret);
+		goto err;
+	}
+	input_set_poll_interval(priv->dev, 16);
+	input_set_min_poll_interval(priv->dev, 8);
+	input_set_max_poll_interval(priv->dev, 32);
+
+	ret = input_register_device(priv->dev);
+	if (ret)
+		dev_err(&i2c->dev, "failed to register joystick: %d\n", ret);
+
+err:
+	return ret;
+}
+
+static int qwiic_remove(struct i2c_client *i2c)
+{
+	struct qwiic_jsk *priv;
+
+	priv = i2c_get_clientdata(i2c);
+	input_unregister_device(priv->dev);
+
+	return 0;
+}
+
+static const struct of_device_id of_qwiic_match[] = {
+	{ .compatible = "sparkfun,qwiic-joystick", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_qwiic_match);
+
+static const struct i2c_device_id qwiic_id_table[] = {
+	{ KBUILD_MODNAME, 0 },
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, qwiic_id_table);
+
+static struct i2c_driver qwiic_driver = {
+	.driver = {
+		.name		= KBUILD_MODNAME,
+		.of_match_table	= of_match_ptr(of_qwiic_match),
+	},
+	.id_table	= qwiic_id_table,
+	.probe		= qwiic_probe,
+	.remove		= qwiic_remove,
+};
+module_i2c_driver(qwiic_driver);
+
+MODULE_AUTHOR("Oleh Kravchenko <oleg@kaa.org.ua>");
+MODULE_DESCRIPTION("SparkFun Qwiic Joystick driver");
+MODULE_LICENSE("GPL v2");
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] input: add SparkFun Qwiic Joystick driver
  2021-05-28  8:44 Oleh Kravchenko
@ 2021-05-28 15:11 ` Oleh Kravchenko
  0 siblings, 0 replies; 9+ messages in thread
From: Oleh Kravchenko @ 2021-05-28 15:11 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring, Matthias Brugger; +Cc: linux-input

Hello guys!
Could you please review this patch?

28.05.21 11:44, Oleh Kravchenko пише:
> Directional movements are simply measured with two 10 kΩ potentiometers,
> connected with a gimbal mechanism that separates the horizontal
> and vertical movements. This joystick also has a select button
> that is actuated when the joystick is pressed down.
> 
> This chip is plugged on an I2C bus.
> Tested on RPi4B and O4-iMX-NANO boards.
> 
> Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
> ---
> 
> Link to product page:
>   https://www.sparkfun.com/products/15168
> 
>  .../bindings/input/qwiic-joystick.yaml        |  30 ++++
>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
>  drivers/input/joystick/Kconfig                |   9 +
>  drivers/input/joystick/Makefile               |   1 +
>  drivers/input/joystick/qwiic-joystick.c       | 164 ++++++++++++++++++
>  5 files changed, 206 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/qwiic-joystick.yaml
>  create mode 100644 drivers/input/joystick/qwiic-joystick.c
> 
> diff --git a/Documentation/devicetree/bindings/input/qwiic-joystick.yaml b/Documentation/devicetree/bindings/input/qwiic-joystick.yaml
> new file mode 100644
> index 000000000000..03549689e288
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/qwiic-joystick.yaml
> @@ -0,0 +1,30 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2021 Oleh Kravchenko
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/input/qwiic-joystick.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: SparkFun Qwiic Joystick
> +
> +maintainers:
> +  - Oleh Kravchenko <oleg@kaa.org.ua>
> +
> +description: |
> +  Bindings for SparkFun Qwiic Joystick (COM-15168).
> +
> +properties:
> +  compatible:
> +    const: sparkfun,qwiic-joystick
> +
> +required:
> +  - compatible
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    qwiic@20 {
> +        compatible = "sparkfun,qwiic-joystick";
> +        reg = <0x20>;
> +    };
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index b868cefc7c55..30e9a0817e21 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -1076,6 +1076,8 @@ patternProperties:
>      description: Sony Corporation
>    "^spansion,.*":
>      description: Spansion Inc.
> +  "^sparkfun,.*":
> +    description: SparkFun Electronics
>    "^sprd,.*":
>      description: Spreadtrum Communications Inc.
>    "^sst,.*":
> diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
> index 5e38899058c1..7dfe8ea90923 100644
> --- a/drivers/input/joystick/Kconfig
> +++ b/drivers/input/joystick/Kconfig
> @@ -372,6 +372,15 @@ config JOYSTICK_PXRC
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called pxrc.
>  
> +config JOYSTICK_QWIIC
> +	tristate "SparkFun Qwiic Joystick"
> +	depends on I2C
> +	help
> +	  Say Y here if you want to use the SparkFun Qwiic Joystick.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called qwiic-joystick.
> +
>  config JOYSTICK_FSIA6B
>  	tristate "FlySky FS-iA6B RC Receiver"
>  	select SERIO
> diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
> index 31d720c9e493..5174b8aba2dd 100644
> --- a/drivers/input/joystick/Makefile
> +++ b/drivers/input/joystick/Makefile
> @@ -27,6 +27,7 @@ obj-$(CONFIG_JOYSTICK_MAPLE)		+= maplecontrol.o
>  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_SIDEWINDER)	+= sidewinder.o
>  obj-$(CONFIG_JOYSTICK_SPACEBALL)	+= spaceball.o
>  obj-$(CONFIG_JOYSTICK_SPACEORB)		+= spaceorb.o
> diff --git a/drivers/input/joystick/qwiic-joystick.c b/drivers/input/joystick/qwiic-joystick.c
> new file mode 100644
> index 000000000000..8e0efeeb1f8a
> --- /dev/null
> +++ b/drivers/input/joystick/qwiic-joystick.c
> @@ -0,0 +1,164 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2021 Oleh Kravchenko <oleg@kaa.org.ua>
> +
> +#include <linux/bits.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/input-polldev.h>
> +#include <linux/module.h>
> +
> +#define QWIIC_JSK_REG_VERS	0
> +#define QWIIC_JSK_REG_DATA	3
> +
> +#define QWIIC_JSK_MAX_AXIS	GENMASK(10, 0)
> +#define QWIIC_JSK_FUZZ		2
> +#define QWIIC_JSK_FLAT		2
> +
> +struct qwiic_jsk {
> +	char			phys[32];
> +	struct input_dev	*dev;
> +	struct i2c_client	*i2c;
> +};
> +
> +struct qwiic_ver {
> +	u8 addr;
> +	u8 major;
> +	u8 minor;
> +} __packed;
> +
> +struct qwiic_data {
> +	u8 hx;
> +	u8 lx;
> +	u8 hy;
> +	u8 ly;
> +	u8 thumb;
> +} __packed;
> +
> +static void qwiic_poll(struct input_dev *input)
> +{
> +	struct qwiic_jsk	*priv;
> +	struct qwiic_data	data;
> +	int			ret;
> +	int			x, y, btn;
> +
> +	priv = input_get_drvdata(input);
> +
> +	ret = i2c_smbus_read_i2c_block_data(priv->i2c, QWIIC_JSK_REG_DATA,
> +					    sizeof(data), (u8 *)&data);
> +	if (ret == sizeof(data)) {
> +		x = (data.hx << 8 | data.lx) >> 6;
> +		y = (data.hy << 8 | data.ly) >> 6;
> +		btn = !!!data.thumb;
> +
> +		input_report_abs(input, ABS_X, x);
> +		input_report_abs(input, ABS_Y, y);
> +		input_report_key(input, BTN_THUMBL, btn);
> +
> +		input_sync(input);
> +	}
> +}
> +
> +static int qwiic_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
> +{
> +	struct qwiic_jsk	*priv;
> +	struct qwiic_ver	vers;
> +	int			ret;
> +
> +	ret = i2c_smbus_read_i2c_block_data(i2c, QWIIC_JSK_REG_VERS,
> +					    sizeof(vers), (u8 *)&vers);
> +	if (ret != sizeof(vers)) {
> +		ret = -EIO;
> +		goto err;
> +	}
> +
> +	if (i2c->addr != vers.addr) {
> +		dev_err(&i2c->dev, "address doesn't match!\n");
> +		ret = -ENODEV;
> +		goto err;
> +	}
> +
> +	dev_info(&i2c->dev, "SparkFun Qwiic Joystick, FW: %d.%d\n",
> +		 vers.major, vers.minor);
> +
> +	priv = devm_kzalloc(&i2c->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv) {
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +
> +	priv->i2c = i2c;
> +	snprintf(priv->phys, sizeof(priv->phys), "i2c/%s", dev_name(&i2c->dev));
> +	i2c_set_clientdata(i2c, priv);
> +
> +	priv->dev = devm_input_allocate_device(&i2c->dev);
> +	if (!priv->dev) {
> +		dev_err(&i2c->dev, "failed to allocate input device\n");
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +
> +	priv->dev->dev.parent = &i2c->dev;
> +	priv->dev->id.bustype = BUS_I2C;
> +	priv->dev->name = "SparkFun Qwiic Joystick";
> +	priv->dev->phys = priv->phys;
> +	input_set_drvdata(priv->dev, priv);
> +
> +	input_set_abs_params(priv->dev, ABS_X, 0, QWIIC_JSK_MAX_AXIS,
> +			     QWIIC_JSK_FUZZ, QWIIC_JSK_FLAT);
> +	input_set_abs_params(priv->dev, ABS_Y, 0, QWIIC_JSK_MAX_AXIS,
> +			     QWIIC_JSK_FUZZ, QWIIC_JSK_FLAT);
> +	input_set_capability(priv->dev, EV_KEY, BTN_THUMBL);
> +
> +	ret = input_setup_polling(priv->dev, qwiic_poll);
> +	if (ret) {
> +		dev_err(&i2c->dev, "failed to set up polling: %d\n", ret);
> +		goto err;
> +	}
> +	input_set_poll_interval(priv->dev, 16);
> +	input_set_min_poll_interval(priv->dev, 8);
> +	input_set_max_poll_interval(priv->dev, 32);
> +
> +	ret = input_register_device(priv->dev);
> +	if (ret)
> +		dev_err(&i2c->dev, "failed to register joystick: %d\n", ret);
> +
> +err:
> +	return ret;
> +}
> +
> +static int qwiic_remove(struct i2c_client *i2c)
> +{
> +	struct qwiic_jsk *priv;
> +
> +	priv = i2c_get_clientdata(i2c);
> +	input_unregister_device(priv->dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id of_qwiic_match[] = {
> +	{ .compatible = "sparkfun,qwiic-joystick", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, of_qwiic_match);
> +
> +static const struct i2c_device_id qwiic_id_table[] = {
> +	{ KBUILD_MODNAME, 0 },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(i2c, qwiic_id_table);
> +
> +static struct i2c_driver qwiic_driver = {
> +	.driver = {
> +		.name		= KBUILD_MODNAME,
> +		.of_match_table	= of_match_ptr(of_qwiic_match),
> +	},
> +	.id_table	= qwiic_id_table,
> +	.probe		= qwiic_probe,
> +	.remove		= qwiic_remove,
> +};
> +module_i2c_driver(qwiic_driver);
> +
> +MODULE_AUTHOR("Oleh Kravchenko <oleg@kaa.org.ua>");
> +MODULE_DESCRIPTION("SparkFun Qwiic Joystick driver");
> +MODULE_LICENSE("GPL v2");
> 

-- 
Best regards,
Oleh Kravchenko


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] input: add SparkFun Qwiic Joystick driver
@ 2021-05-28  8:44 Oleh Kravchenko
  2021-05-28 15:11 ` Oleh Kravchenko
  0 siblings, 1 reply; 9+ messages in thread
From: Oleh Kravchenko @ 2021-05-28  8:44 UTC (permalink / raw)
  To: linux-input; +Cc: Oleh Kravchenko

Directional movements are simply measured with two 10 kΩ potentiometers,
connected with a gimbal mechanism that separates the horizontal
and vertical movements. This joystick also has a select button
that is actuated when the joystick is pressed down.

This chip is plugged on an I2C bus.
Tested on RPi4B and O4-iMX-NANO boards.

Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
---

Link to product page:
  https://www.sparkfun.com/products/15168

 .../bindings/input/qwiic-joystick.yaml        |  30 ++++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 drivers/input/joystick/Kconfig                |   9 +
 drivers/input/joystick/Makefile               |   1 +
 drivers/input/joystick/qwiic-joystick.c       | 164 ++++++++++++++++++
 5 files changed, 206 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/qwiic-joystick.yaml
 create mode 100644 drivers/input/joystick/qwiic-joystick.c

diff --git a/Documentation/devicetree/bindings/input/qwiic-joystick.yaml b/Documentation/devicetree/bindings/input/qwiic-joystick.yaml
new file mode 100644
index 000000000000..03549689e288
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/qwiic-joystick.yaml
@@ -0,0 +1,30 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2021 Oleh Kravchenko
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/input/qwiic-joystick.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: SparkFun Qwiic Joystick
+
+maintainers:
+  - Oleh Kravchenko <oleg@kaa.org.ua>
+
+description: |
+  Bindings for SparkFun Qwiic Joystick (COM-15168).
+
+properties:
+  compatible:
+    const: sparkfun,qwiic-joystick
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    qwiic@20 {
+        compatible = "sparkfun,qwiic-joystick";
+        reg = <0x20>;
+    };
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index b868cefc7c55..30e9a0817e21 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1076,6 +1076,8 @@ patternProperties:
     description: Sony Corporation
   "^spansion,.*":
     description: Spansion Inc.
+  "^sparkfun,.*":
+    description: SparkFun Electronics
   "^sprd,.*":
     description: Spreadtrum Communications Inc.
   "^sst,.*":
diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
index 5e38899058c1..7dfe8ea90923 100644
--- a/drivers/input/joystick/Kconfig
+++ b/drivers/input/joystick/Kconfig
@@ -372,6 +372,15 @@ config JOYSTICK_PXRC
 	  To compile this driver as a module, choose M here: the
 	  module will be called pxrc.
 
+config JOYSTICK_QWIIC
+	tristate "SparkFun Qwiic Joystick"
+	depends on I2C
+	help
+	  Say Y here if you want to use the SparkFun Qwiic Joystick.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called qwiic-joystick.
+
 config JOYSTICK_FSIA6B
 	tristate "FlySky FS-iA6B RC Receiver"
 	select SERIO
diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
index 31d720c9e493..5174b8aba2dd 100644
--- a/drivers/input/joystick/Makefile
+++ b/drivers/input/joystick/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_JOYSTICK_MAPLE)		+= maplecontrol.o
 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_SIDEWINDER)	+= sidewinder.o
 obj-$(CONFIG_JOYSTICK_SPACEBALL)	+= spaceball.o
 obj-$(CONFIG_JOYSTICK_SPACEORB)		+= spaceorb.o
diff --git a/drivers/input/joystick/qwiic-joystick.c b/drivers/input/joystick/qwiic-joystick.c
new file mode 100644
index 000000000000..8e0efeeb1f8a
--- /dev/null
+++ b/drivers/input/joystick/qwiic-joystick.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2021 Oleh Kravchenko <oleg@kaa.org.ua>
+
+#include <linux/bits.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input-polldev.h>
+#include <linux/module.h>
+
+#define QWIIC_JSK_REG_VERS	0
+#define QWIIC_JSK_REG_DATA	3
+
+#define QWIIC_JSK_MAX_AXIS	GENMASK(10, 0)
+#define QWIIC_JSK_FUZZ		2
+#define QWIIC_JSK_FLAT		2
+
+struct qwiic_jsk {
+	char			phys[32];
+	struct input_dev	*dev;
+	struct i2c_client	*i2c;
+};
+
+struct qwiic_ver {
+	u8 addr;
+	u8 major;
+	u8 minor;
+} __packed;
+
+struct qwiic_data {
+	u8 hx;
+	u8 lx;
+	u8 hy;
+	u8 ly;
+	u8 thumb;
+} __packed;
+
+static void qwiic_poll(struct input_dev *input)
+{
+	struct qwiic_jsk	*priv;
+	struct qwiic_data	data;
+	int			ret;
+	int			x, y, btn;
+
+	priv = input_get_drvdata(input);
+
+	ret = i2c_smbus_read_i2c_block_data(priv->i2c, QWIIC_JSK_REG_DATA,
+					    sizeof(data), (u8 *)&data);
+	if (ret == sizeof(data)) {
+		x = (data.hx << 8 | data.lx) >> 6;
+		y = (data.hy << 8 | data.ly) >> 6;
+		btn = !!!data.thumb;
+
+		input_report_abs(input, ABS_X, x);
+		input_report_abs(input, ABS_Y, y);
+		input_report_key(input, BTN_THUMBL, btn);
+
+		input_sync(input);
+	}
+}
+
+static int qwiic_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
+{
+	struct qwiic_jsk	*priv;
+	struct qwiic_ver	vers;
+	int			ret;
+
+	ret = i2c_smbus_read_i2c_block_data(i2c, QWIIC_JSK_REG_VERS,
+					    sizeof(vers), (u8 *)&vers);
+	if (ret != sizeof(vers)) {
+		ret = -EIO;
+		goto err;
+	}
+
+	if (i2c->addr != vers.addr) {
+		dev_err(&i2c->dev, "address doesn't match!\n");
+		ret = -ENODEV;
+		goto err;
+	}
+
+	dev_info(&i2c->dev, "SparkFun Qwiic Joystick, FW: %d.%d\n",
+		 vers.major, vers.minor);
+
+	priv = devm_kzalloc(&i2c->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	priv->i2c = i2c;
+	snprintf(priv->phys, sizeof(priv->phys), "i2c/%s", dev_name(&i2c->dev));
+	i2c_set_clientdata(i2c, priv);
+
+	priv->dev = devm_input_allocate_device(&i2c->dev);
+	if (!priv->dev) {
+		dev_err(&i2c->dev, "failed to allocate input device\n");
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	priv->dev->dev.parent = &i2c->dev;
+	priv->dev->id.bustype = BUS_I2C;
+	priv->dev->name = "SparkFun Qwiic Joystick";
+	priv->dev->phys = priv->phys;
+	input_set_drvdata(priv->dev, priv);
+
+	input_set_abs_params(priv->dev, ABS_X, 0, QWIIC_JSK_MAX_AXIS,
+			     QWIIC_JSK_FUZZ, QWIIC_JSK_FLAT);
+	input_set_abs_params(priv->dev, ABS_Y, 0, QWIIC_JSK_MAX_AXIS,
+			     QWIIC_JSK_FUZZ, QWIIC_JSK_FLAT);
+	input_set_capability(priv->dev, EV_KEY, BTN_THUMBL);
+
+	ret = input_setup_polling(priv->dev, qwiic_poll);
+	if (ret) {
+		dev_err(&i2c->dev, "failed to set up polling: %d\n", ret);
+		goto err;
+	}
+	input_set_poll_interval(priv->dev, 16);
+	input_set_min_poll_interval(priv->dev, 8);
+	input_set_max_poll_interval(priv->dev, 32);
+
+	ret = input_register_device(priv->dev);
+	if (ret)
+		dev_err(&i2c->dev, "failed to register joystick: %d\n", ret);
+
+err:
+	return ret;
+}
+
+static int qwiic_remove(struct i2c_client *i2c)
+{
+	struct qwiic_jsk *priv;
+
+	priv = i2c_get_clientdata(i2c);
+	input_unregister_device(priv->dev);
+
+	return 0;
+}
+
+static const struct of_device_id of_qwiic_match[] = {
+	{ .compatible = "sparkfun,qwiic-joystick", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_qwiic_match);
+
+static const struct i2c_device_id qwiic_id_table[] = {
+	{ KBUILD_MODNAME, 0 },
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, qwiic_id_table);
+
+static struct i2c_driver qwiic_driver = {
+	.driver = {
+		.name		= KBUILD_MODNAME,
+		.of_match_table	= of_match_ptr(of_qwiic_match),
+	},
+	.id_table	= qwiic_id_table,
+	.probe		= qwiic_probe,
+	.remove		= qwiic_remove,
+};
+module_i2c_driver(qwiic_driver);
+
+MODULE_AUTHOR("Oleh Kravchenko <oleg@kaa.org.ua>");
+MODULE_DESCRIPTION("SparkFun Qwiic Joystick driver");
+MODULE_LICENSE("GPL v2");
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-06-01 20:55 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-31  9:54 [PATCH] input: add SparkFun Qwiic Joystick driver Oleh Kravchenko
2021-05-31 13:16 ` kernel test robot
  -- strict thread matches above, loose matches on Subject: below --
2021-05-31 13:32 Oleh Kravchenko
2021-05-31 22:58 ` Jeff LaBundy
2021-06-01  6:18 ` Dmitry Torokhov
2021-06-01 20:55   ` Rob Herring
2021-06-01 13:32 ` Rob Herring
2021-05-28  8:44 Oleh Kravchenko
2021-05-28 15:11 ` Oleh Kravchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).