[PATCH 1/2 v4] dt-bindings: Add vendor prefix and bindings for Qwiic Joystick

Subsystems: open firmware and flattened device tree bindings, the rest

STALE1910d LANDED

Landed in mainline as 9b2336965a4c on 2021-06-19.

8 messages, 3 authors, 2021-06-15 · open the first message on its own page

[PATCH 1/2 v4] dt-bindings: Add vendor prefix and bindings for Qwiic Joystick

From: Oleh Kravchenko <hidden>
Date: 2021-06-08 22:32:47 

Add vendor prefix for SparkFun Electronics.
Update trivial-devices.yaml with SparkFun Qwiic Joystick description.

Cc: Benjamin Tissoires <redacted>
Cc: Device Tree mailing list <redacted>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Jiri Kosina <redacted>
Cc: Patchwork Bot <patchwork-bot@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Oleh Kravchenko <redacted>
---

Changes for v4:
- no updates.

Changes for v3:
- update patch after code review.

Changes for v2:
- update code after code review


 Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index a327130d1faa..706aa102d96d 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -261,6 +261,8 @@ properties:
             # Socionext SynQuacer TPM MMIO module
           - socionext,synquacer-tpm-mmio
             # i2c serial eeprom  (24cxx)
+          - sparkfun,qwiic-joystick
+            # SparkFun Qwiic Joystick (COM-15168) with i2c interface
           - st,24c256
             # Ambient Light Sensor with SMBUS/Two Wire Serial Interface
           - taos,tsl2550
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,.*":
-- 
2.26.3

[PATCH 2/2 v4] input: add SparkFun Qwiic Joystick driver

From: Oleh Kravchenko <hidden>
Date: 2021-06-08 22:33:18 

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 <redacted>
Cc: Device Tree mailing list <redacted>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Jiri Kosina <redacted>
Cc: Patchwork Bot <patchwork-bot@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Oleh Kravchenko <redacted>
Reviewed-by: Jeff LaBundy <redacted>
---

Changes for v4:
- alphabetize include headers.

Changes for v3:
- rebase patch for device tree before code.

Changes for v2:
- Separate patch for device tree bindings.

 drivers/input/joystick/Kconfig          |   9 ++
 drivers/input/joystick/Makefile         |   1 +
 drivers/input/joystick/qwiic-joystick.c | 147 ++++++++++++++++++++++++
 3 files changed, 157 insertions(+)
 create mode 100644 drivers/input/joystick/qwiic-joystick.c

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..557708e9e756
--- /dev/null
+++ b/drivers/input/joystick/qwiic-joystick.c
@@ -0,0 +1,147 @@
+// 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/kernel.h>
+#include <linux/module.h>
+
+#define DRV_NAME "qwiic-joystick"
+
+#define QWIIC_JSK_REG_VERS	1
+#define QWIIC_JSK_REG_DATA	3
+
+#define QWIIC_JSK_MAX_AXIS	GENMASK(9, 0)
+#define QWIIC_JSK_FUZZ		2
+#define QWIIC_JSK_FLAT		2
+#define QWIIC_JSK_POLL_INTERVAL	16
+#define QWIIC_JSK_POLL_MIN	8
+#define QWIIC_JSK_POLL_MAX	32
+
+struct qwiic_jsk {
+	char phys[32];
+	struct input_dev *dev;
+	struct i2c_client *client;
+};
+
+struct qwiic_ver {
+	u8 major;
+	u8 minor;
+};
+
+struct qwiic_data {
+	__be16 x;
+	__be16 y;
+	u8 thumb;
+} __packed;
+
+static void qwiic_poll(struct input_dev *input)
+{
+	struct qwiic_jsk *priv;
+	struct qwiic_data data;
+	int err;
+
+	priv = input_get_drvdata(input);
+
+	err = i2c_smbus_read_i2c_block_data(priv->client, QWIIC_JSK_REG_DATA,
+					    sizeof(data), (u8 *)&data);
+	if (err != sizeof(data))
+		return;
+
+	input_report_abs(input, ABS_X, be16_to_cpu(data.x) >> 6);
+	input_report_abs(input, ABS_Y, be16_to_cpu(data.y) >> 6);
+	input_report_key(input, BTN_THUMBL, !data.thumb);
+	input_sync(input);
+}
+
+static int qwiic_probe(struct i2c_client *client,
+		       const struct i2c_device_id *id)
+{
+	struct qwiic_jsk *priv;
+	struct qwiic_ver vers;
+	int err;
+
+	err = i2c_smbus_read_i2c_block_data(client, QWIIC_JSK_REG_VERS,
+					    sizeof(vers), (u8 *)&vers);
+	if (err < 0)
+		return err;
+	if (err != sizeof(vers))
+		return -EIO;
+
+	dev_dbg(&client->dev, "SparkFun Qwiic Joystick, FW: %u.%u\n",
+		vers.major, vers.minor);
+
+	priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->client = client;
+	snprintf(priv->phys, sizeof(priv->phys),
+		 "i2c/%s", dev_name(&client->dev));
+	i2c_set_clientdata(client, priv);
+
+	priv->dev = devm_input_allocate_device(&client->dev);
+	if (!priv->dev)
+		return -ENOMEM;
+
+	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);
+
+	err = input_setup_polling(priv->dev, qwiic_poll);
+	if (err) {
+		dev_err(&client->dev, "failed to set up polling: %d\n", err);
+		return err;
+	}
+	input_set_poll_interval(priv->dev, QWIIC_JSK_POLL_INTERVAL);
+	input_set_min_poll_interval(priv->dev, QWIIC_JSK_POLL_MIN);
+	input_set_max_poll_interval(priv->dev, QWIIC_JSK_POLL_MAX);
+
+	err = input_register_device(priv->dev);
+	if (err)
+		dev_err(&client->dev, "failed to register joystick: %d\n", err);
+
+	return err;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id of_qwiic_match[] = {
+	{ .compatible = "sparkfun,qwiic-joystick", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, of_qwiic_match);
+#endif /* CONFIG_OF */
+
+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		= DRV_NAME,
+		.of_match_table	= of_match_ptr(of_qwiic_match),
+	},
+	.id_table	= qwiic_id_table,
+	.probe		= qwiic_probe,
+};
+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

Re: [PATCH 1/2 v4] dt-bindings: Add vendor prefix and bindings for Qwiic Joystick

From: Oleh Kravchenko <hidden>
Date: 2021-06-10 15:28:03 

Hello Rob,
could you please review this patch?

09.06.21 01:31, Oleh Kravchenko пише:
quoted hunk
Add vendor prefix for SparkFun Electronics.
Update trivial-devices.yaml with SparkFun Qwiic Joystick description.

Cc: Benjamin Tissoires <redacted>
Cc: Device Tree mailing list <redacted>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Jiri Kosina <redacted>
Cc: Patchwork Bot <patchwork-bot@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Oleh Kravchenko <redacted>
---

Changes for v4:
- no updates.

Changes for v3:
- update patch after code review.

Changes for v2:
- update code after code review


 Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index a327130d1faa..706aa102d96d 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -261,6 +261,8 @@ properties:
             # Socionext SynQuacer TPM MMIO module
           - socionext,synquacer-tpm-mmio
             # i2c serial eeprom  (24cxx)
+          - sparkfun,qwiic-joystick
+            # SparkFun Qwiic Joystick (COM-15168) with i2c interface
           - st,24c256
             # Ambient Light Sensor with SMBUS/Two Wire Serial Interface
           - taos,tsl2550
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,.*":

Re: [PATCH 1/2 v4] dt-bindings: Add vendor prefix and bindings for Qwiic Joystick

From: Rob Herring <robh@kernel.org>
Date: 2021-06-10 16:38:59 

On Wed, 09 Jun 2021 01:31:30 +0300, Oleh Kravchenko wrote:
Add vendor prefix for SparkFun Electronics.
Update trivial-devices.yaml with SparkFun Qwiic Joystick description.

Cc: Benjamin Tissoires <redacted>
Cc: Device Tree mailing list <redacted>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Jiri Kosina <redacted>
Cc: Patchwork Bot <patchwork-bot@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Oleh Kravchenko <redacted>
---

Changes for v4:
- no updates.

Changes for v3:
- update patch after code review.

Changes for v2:
- update code after code review


 Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 2 files changed, 4 insertions(+)
Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 1/2 v4] dt-bindings: Add vendor prefix and bindings for Qwiic Joystick

From: Oleh Kravchenko <hidden>
Date: 2021-06-13 09:04:33 

Hello Dmitry,
Could you please review these patches?

09.06.21 01:31, Oleh Kravchenko пише:
quoted hunk
Add vendor prefix for SparkFun Electronics.
Update trivial-devices.yaml with SparkFun Qwiic Joystick description.

Cc: Benjamin Tissoires <redacted>
Cc: Device Tree mailing list <redacted>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Jiri Kosina <redacted>
Cc: Patchwork Bot <patchwork-bot@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Oleh Kravchenko <redacted>
---

Changes for v4:
- no updates.

Changes for v3:
- update patch after code review.

Changes for v2:
- update code after code review


 Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index a327130d1faa..706aa102d96d 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -261,6 +261,8 @@ properties:
             # Socionext SynQuacer TPM MMIO module
           - socionext,synquacer-tpm-mmio
             # i2c serial eeprom  (24cxx)
+          - sparkfun,qwiic-joystick
+            # SparkFun Qwiic Joystick (COM-15168) with i2c interface
           - st,24c256
             # Ambient Light Sensor with SMBUS/Two Wire Serial Interface
           - taos,tsl2550
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,.*":
-- 
Best regards,
Oleh Kravchenko

Re: [PATCH 2/2 v4] input: add SparkFun Qwiic Joystick driver

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2021-06-15 04:57:54 

Hi Oleh,

On Wed, Jun 09, 2021 at 01:31:32AM +0300, Oleh Kravchenko wrote:
quoted hunk
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 <redacted>
Cc: Device Tree mailing list <redacted>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Jiri Kosina <redacted>
Cc: Patchwork Bot <patchwork-bot@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Oleh Kravchenko <redacted>
Reviewed-by: Jeff LaBundy <redacted>
---

Changes for v4:
- alphabetize include headers.

Changes for v3:
- rebase patch for device tree before code.

Changes for v2:
- Separate patch for device tree bindings.

 drivers/input/joystick/Kconfig          |   9 ++
 drivers/input/joystick/Makefile         |   1 +
 drivers/input/joystick/qwiic-joystick.c | 147 ++++++++++++++++++++++++
 3 files changed, 157 insertions(+)
 create mode 100644 drivers/input/joystick/qwiic-joystick.c

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..557708e9e756
--- /dev/null
+++ b/drivers/input/joystick/qwiic-joystick.c
@@ -0,0 +1,147 @@
+// 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/kernel.h>
+#include <linux/module.h>
+
+#define DRV_NAME "qwiic-joystick"
+
+#define QWIIC_JSK_REG_VERS	1
+#define QWIIC_JSK_REG_DATA	3
+
+#define QWIIC_JSK_MAX_AXIS	GENMASK(9, 0)
+#define QWIIC_JSK_FUZZ		2
+#define QWIIC_JSK_FLAT		2
+#define QWIIC_JSK_POLL_INTERVAL	16
+#define QWIIC_JSK_POLL_MIN	8
+#define QWIIC_JSK_POLL_MAX	32
+
+struct qwiic_jsk {
+	char phys[32];
+	struct input_dev *dev;
+	struct i2c_client *client;
+};
+
+struct qwiic_ver {
+	u8 major;
+	u8 minor;
+};
+
+struct qwiic_data {
+	__be16 x;
+	__be16 y;
+	u8 thumb;
+} __packed;
The members of this structure are naturally aligned, so there is no need
to declare it as __packed.

+
+static void qwiic_poll(struct input_dev *input)
+{
+	struct qwiic_jsk *priv;
+	struct qwiic_data data;
+	int err;
+
+	priv = input_get_drvdata(input);
+
+	err = i2c_smbus_read_i2c_block_data(priv->client, QWIIC_JSK_REG_DATA,
+					    sizeof(data), (u8 *)&data);
+	if (err != sizeof(data))
+		return;
+
+	input_report_abs(input, ABS_X, be16_to_cpu(data.x) >> 6);
+	input_report_abs(input, ABS_Y, be16_to_cpu(data.y) >> 6);
+	input_report_key(input, BTN_THUMBL, !data.thumb);
+	input_sync(input);
+}
+
+static int qwiic_probe(struct i2c_client *client,
+		       const struct i2c_device_id *id)
The probe() does not use the i2c_device_id parameter, so I will switch
it to probe_new() to avoid the temptation of using it in the future.

Please let me know if you disagree, otherwise I will go and apply (no
need to resubmit).

Thanks.

-- 
Dmitry

Re: [PATCH 2/2 v4] input: add SparkFun Qwiic Joystick driver

From: Oleh Kravchenko <hidden>
Date: 2021-06-15 07:50:05 

Hello Dmitry,
Thank you for your code-review

15 черв. 2021 р. о 07:57 Dmitry Torokhov [off-list ref] пише:

Hi Oleh,
quoted
+struct qwiic_data {
+    __be16 x;
+    __be16 y;
+    u8 thumb;
+} __packed;
The members of this structure are naturally aligned, so there is no need
to declare it as __packed.
Sorry, I forgot to fix this nitpick :-/
quoted
+
+static void qwiic_poll(struct input_dev *input)
+{
+    struct qwiic_jsk *priv;
+    struct qwiic_data data;
+    int err;
+
+    priv = input_get_drvdata(input);
+
+    err = i2c_smbus_read_i2c_block_data(priv->client, QWIIC_JSK_REG_DATA,
+                        sizeof(data), (u8 *)&data);
+    if (err != sizeof(data))
+        return;
+
+    input_report_abs(input, ABS_X, be16_to_cpu(data.x) >> 6);
+    input_report_abs(input, ABS_Y, be16_to_cpu(data.y) >> 6);
+    input_report_key(input, BTN_THUMBL, !data.thumb);
+    input_sync(input);
+}
+
+static int qwiic_probe(struct i2c_client *client,
+               const struct i2c_device_id *id)
The probe() does not use the i2c_device_id parameter, so I will switch
it to probe_new() to avoid the temptation of using it in the future.
Sounds awesome!

Please let me know if you disagree, otherwise I will go and apply (no
need to resubmit).
I agree,
please merge.

Re: [PATCH 2/2 v4] input: add SparkFun Qwiic Joystick driver

From: Oleh Kravchenko <hidden>
Date: 2021-06-15 14:32:15 

15.06.21 07:57, Dmitry Torokhov пише:
quoted
+struct qwiic_ver {
+	u8 major;
+	u8 minor;
+};
+
+struct qwiic_data {
+	__be16 x;
+	__be16 y;
+	u8 thumb;
+} __packed;
The members of this structure are naturally aligned, so there is no need
to declare it as __packed.

But struct qwiic_data require packed attribute, because without that it will be 6 bytes not 5.
quoted
+
+static void qwiic_poll(struct input_dev *input)
+{
+	struct qwiic_jsk *priv;
+	struct qwiic_data data;
+	int err;
+
+	priv = input_get_drvdata(input);
+
+	err = i2c_smbus_read_i2c_block_data(priv->client, QWIIC_JSK_REG_DATA,
+					    sizeof(data), (u8 *)&data);
+	if (err != sizeof(data))
+		return;
+
+	input_report_abs(input, ABS_X, be16_to_cpu(data.x) >> 6);
+	input_report_abs(input, ABS_Y, be16_to_cpu(data.y) >> 6);
+	input_report_key(input, BTN_THUMBL, !data.thumb);
+	input_sync(input);
+}
+
+static int qwiic_probe(struct i2c_client *client,
+		       const struct i2c_device_id *id)
The probe() does not use the i2c_device_id parameter, so I will switch
it to probe_new() to avoid the temptation of using it in the future.

Please let me know if you disagree, otherwise I will go and apply (no
need to resubmit).

Thanks.
-- 
Best regards,
Oleh Kravchenko
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help