Add driver for Maxim Integrated MAX7360 rotary encoder controller,
supporting a single rotary switch.
Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
---
drivers/input/misc/Kconfig | 11 +++
drivers/input/misc/Makefile | 1 +
drivers/input/misc/max7360-rotary.c | 161 ++++++++++++++++++++++++++++++++++++
3 files changed, 173 insertions(+)
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 13d135257e06..77b07e053265 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -230,6 +230,17 @@ config INPUT_M68K_BEEP
tristate "M68k Beeper support"
depends on M68K
+config INPUT_MAX7360_ROTARY
+ tristate "Maxim MAX7360 Rotary Encoder"
+ depends on MFD_MAX7360
+ select PINCTRL_MAX7360
+ help
+ If you say yes here you get support for the rotary encoder on the
+ Maxim MAX7360 I/O Expander.
+
+ To compile this driver as a module, choose M here: the
+ module will be called max7360_rotary.
+
config INPUT_MAX77650_ONKEY
tristate "Maxim MAX77650 ONKEY support"
depends on MFD_MAX77650
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 6d91804d0a6f..c454fba3a3ae 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -51,6 +51,7 @@ obj-$(CONFIG_INPUT_IQS7222) += iqs7222.o
obj-$(CONFIG_INPUT_KEYSPAN_REMOTE) += keyspan_remote.o
obj-$(CONFIG_INPUT_KXTJ9) += kxtj9.o
obj-$(CONFIG_INPUT_M68K_BEEP) += m68kspkr.o
+obj-$(CONFIG_INPUT_MAX7360_ROTARY) += max7360-rotary.o
obj-$(CONFIG_INPUT_MAX77650_ONKEY) += max77650-onkey.o
obj-$(CONFIG_INPUT_MAX77693_HAPTIC) += max77693-haptic.o
obj-$(CONFIG_INPUT_MAX8925_ONKEY) += max8925_onkey.o
diff --git a/drivers/input/misc/max7360-rotary.c b/drivers/input/misc/max7360-rotary.c
new file mode 100644
index 000000000000..3046ef64dd56
--- /dev/null
+++ b/drivers/input/misc/max7360-rotary.c
@@ -0,0 +1,161 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2025 Bootlin
+ *
+ * Author: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
+ */
+
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/max7360.h>
+#include <linux/property.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_wakeirq.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+struct max7360_rotary {
+ u32 axis;
+ struct input_dev *input;
+ unsigned int debounce_ms;
+ struct regmap *regmap;
+};
+
+static irqreturn_t max7360_rotary_irq(int irq, void *data)
+{
+ struct max7360_rotary *max7360_rotary = data;
+ int val;
+ int ret;
+
+ ret = regmap_read(max7360_rotary->regmap, MAX7360_REG_RTR_CNT, &val);
+ if (ret < 0) {
+ dev_err(&max7360_rotary->input->dev,
+ "Failed to read rotary counter\n");
+ return IRQ_NONE;
+ }
+
+ if (val == 0) {
+ dev_dbg(&max7360_rotary->input->dev,
+ "Got a spurious interrupt\n");
+
+ return IRQ_NONE;
+ }
+
+ input_report_rel(max7360_rotary->input, max7360_rotary->axis,
+ (int8_t)val);
+ input_sync(max7360_rotary->input);
+
+ return IRQ_HANDLED;
+}
+
+static int max7360_rotary_hw_init(struct max7360_rotary *max7360_rotary)
+{
+ int val;
+ int ret;
+
+ val = FIELD_PREP(MAX7360_ROT_DEBOUNCE, max7360_rotary->debounce_ms) |
+ FIELD_PREP(MAX7360_ROT_INTCNT, 1) | MAX7360_ROT_INTCNT_DLY;
+ ret = regmap_write(max7360_rotary->regmap, MAX7360_REG_RTRCFG, val);
+ if (ret) {
+ dev_err(&max7360_rotary->input->dev,
+ "Failed to set max7360 rotary encoder configuration\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int max7360_rotary_probe(struct platform_device *pdev)
+{
+ struct max7360_rotary *max7360_rotary;
+ struct input_dev *input;
+ int irq;
+ int ret;
+
+ if (!pdev->dev.parent)
+ return dev_err_probe(&pdev->dev, -ENODEV, "No parent device\n");
+
+ irq = platform_get_irq_byname(to_platform_device(pdev->dev.parent),
+ "inti");
+ if (irq < 0)
+ return irq;
+
+ max7360_rotary = devm_kzalloc(&pdev->dev, sizeof(*max7360_rotary),
+ GFP_KERNEL);
+ if (!max7360_rotary)
+ return -ENOMEM;
+
+ max7360_rotary->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+ if (!max7360_rotary->regmap)
+ dev_err_probe(&pdev->dev, -ENODEV,
+ "Could not get parent regmap\n");
+
+ device_property_read_u32(pdev->dev.parent, "linux,axis",
+ &max7360_rotary->axis);
+ device_property_read_u32(pdev->dev.parent, "rotary-debounce-delay-ms",
+ &max7360_rotary->debounce_ms);
+ if (max7360_rotary->debounce_ms > MAX7360_ROT_DEBOUNCE_MAX)
+ return dev_err_probe(&pdev->dev, -EINVAL,
+ "Invalid debounce timing: %u\n",
+ max7360_rotary->debounce_ms);
+
+ input = devm_input_allocate_device(&pdev->dev);
+ if (!input)
+ return -ENOMEM;
+
+ max7360_rotary->input = input;
+
+ input->id.bustype = BUS_I2C;
+ input->name = pdev->name;
+ input->dev.parent = &pdev->dev;
+
+ input_set_capability(input, EV_REL, max7360_rotary->axis);
+ input_set_drvdata(input, max7360_rotary);
+
+ ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+ max7360_rotary_irq,
+ IRQF_TRIGGER_LOW | IRQF_ONESHOT | IRQF_SHARED,
+ "max7360-rotary", max7360_rotary);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "Failed to register interrupt\n");
+
+ ret = input_register_device(input);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "Could not register input device\n");
+
+ platform_set_drvdata(pdev, max7360_rotary);
+
+ device_init_wakeup(&pdev->dev, true);
+ ret = dev_pm_set_wake_irq(&pdev->dev, irq);
+ if (ret)
+ dev_warn(&pdev->dev, "Failed to set up wakeup irq: %d\n", ret);
+
+ ret = max7360_rotary_hw_init(max7360_rotary);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "Failed to initialize max7360 rotary\n");
+
+ return 0;
+}
+
+static void max7360_rotary_remove(struct platform_device *pdev)
+{
+ dev_pm_clear_wake_irq(&pdev->dev);
+}
+
+static struct platform_driver max7360_rotary_driver = {
+ .driver = {
+ .name = "max7360-rotary",
+ },
+ .probe = max7360_rotary_probe,
+ .remove = max7360_rotary_remove,
+};
+module_platform_driver(max7360_rotary_driver);
+
+MODULE_DESCRIPTION("MAX7360 Rotary driver");
+MODULE_AUTHOR("Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>");
+MODULE_LICENSE("GPL");
--
2.39.5Hi Mathieu,
kernel test robot noticed the following build warnings:
[auto build test WARNING on a64dcfb451e254085a7daee5fe51bf22959d52d3]
url: https://github.com/intel-lab-lkp/linux/commits/Mathieu-Dubois-Briand/dt-bindings-mfd-gpio-Add-MAX7360/20250319-003750
base: a64dcfb451e254085a7daee5fe51bf22959d52d3
patch link: https://lore.kernel.org/r/20250318-mdb-max7360-support-v5-10-fb20baf97da0%40bootlin.com
patch subject: [PATCH v5 10/11] input: misc: Add support for MAX7360 rotary
config: nios2-kismet-CONFIG_PINCTRL_MAX7360-CONFIG_INPUT_MAX7360_ROTARY-0-0 (https://download.01.org/0day-ci/archive/20250320/202503200825.rmtamLkh-lkp@intel.com/config)
reproduce: (https://download.01.org/0day-ci/archive/20250320/202503200825.rmtamLkh-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503200825.rmtamLkh-lkp@intel.com/
kismet warnings: (new ones prefixed by >>)
>> kismet: WARNING: unmet direct dependencies detected for PINCTRL_MAX7360 when selected by INPUT_MAX7360_ROTARY
WARNING: unmet direct dependencies detected for PINCTRL_MAX7360
Depends on [n]: PINCTRL [=n] && MFD_MAX7360 [=y]
Selected by [y]:
- INPUT_MAX7360_ROTARY [=y] && INPUT [=y] && INPUT_MISC [=y] && MFD_MAX7360 [=y]
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Mathieu,
kernel test robot noticed the following build errors:
[auto build test ERROR on a64dcfb451e254085a7daee5fe51bf22959d52d3]
url: https://github.com/intel-lab-lkp/linux/commits/Mathieu-Dubois-Briand/dt-bindings-mfd-gpio-Add-MAX7360/20250319-003750
base: a64dcfb451e254085a7daee5fe51bf22959d52d3
patch link: https://lore.kernel.org/r/20250318-mdb-max7360-support-v5-10-fb20baf97da0%40bootlin.com
patch subject: [PATCH v5 10/11] input: misc: Add support for MAX7360 rotary
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20250319/202503192326.KOCnEkdj-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250319/202503192326.KOCnEkdj-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503192326.KOCnEkdj-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/input/misc/max7360-rotary.c: In function 'max7360_rotary_hw_init':
>> drivers/input/misc/max7360-rotary.c:58:8: error: implicit declaration of function 'FIELD_PREP'; did you mean 'EV_REP'? [-Werror=implicit-function-declaration]
val = FIELD_PREP(MAX7360_ROT_DEBOUNCE, max7360_rotary->debounce_ms) |
^~~~~~~~~~
EV_REP
cc1: some warnings being treated as errors
vim +58 drivers/input/misc/max7360-rotary.c
52
53 static int max7360_rotary_hw_init(struct max7360_rotary *max7360_rotary)
54 {
55 int val;
56 int ret;
57
> 58 val = FIELD_PREP(MAX7360_ROT_DEBOUNCE, max7360_rotary->debounce_ms) |
59 FIELD_PREP(MAX7360_ROT_INTCNT, 1) | MAX7360_ROT_INTCNT_DLY;
60 ret = regmap_write(max7360_rotary->regmap, MAX7360_REG_RTRCFG, val);
61 if (ret) {
62 dev_err(&max7360_rotary->input->dev,
63 "Failed to set max7360 rotary encoder configuration\n");
64 return ret;
65 }
66
67 return 0;
68 }
69
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Tue, Mar 18, 2025 at 05:26:26PM +0100, Mathieu Dubois-Briand wrote:
> Add driver for Maxim Integrated MAX7360 rotary encoder controller,
> supporting a single rotary switch.
...
> + help
> + If you say yes here you get support for the rotary encoder on the
> + Maxim MAX7360 I/O Expander.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called max7360_rotary.
Seems all your patches have unaligned line widths in the help texts.
...
+ bitfield.h
+ device/devres.h
+ dev_printk.h
> +#include <linux/init.h>
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/max7360.h>
> +#include <linux/property.h>
> +#include <linux/of.h>
Why?
> +#include <linux/platform_device.h>
> +#include <linux/pm_wakeirq.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
Not needed.
+ types.h
...
> +static irqreturn_t max7360_rotary_irq(int irq, void *data)
> +{
> + struct max7360_rotary *max7360_rotary = data;
> + int val;
> + int ret;
> +
> + ret = regmap_read(max7360_rotary->regmap, MAX7360_REG_RTR_CNT, &val);
> + if (ret < 0) {
> + dev_err(&max7360_rotary->input->dev,
> + "Failed to read rotary counter\n");
> + return IRQ_NONE;
> + }
> +
> + if (val == 0) {
> + dev_dbg(&max7360_rotary->input->dev,
> + "Got a spurious interrupt\n");
> +
> + return IRQ_NONE;
> + }
> +
> + input_report_rel(max7360_rotary->input, max7360_rotary->axis,
> + (int8_t)val);
This is strange:
1) why casting to begin with?
2) why to C type and not kernel (s8) type?
> + input_sync(max7360_rotary->input);
> +
> + return IRQ_HANDLED;
> +}
...
> +static int max7360_rotary_hw_init(struct max7360_rotary *max7360_rotary)
> +{
> + int val;
> + int ret;
> +
> + val = FIELD_PREP(MAX7360_ROT_DEBOUNCE, max7360_rotary->debounce_ms) |
> + FIELD_PREP(MAX7360_ROT_INTCNT, 1) | MAX7360_ROT_INTCNT_DLY;
> + ret = regmap_write(max7360_rotary->regmap, MAX7360_REG_RTRCFG, val);
> + if (ret) {
> + dev_err(&max7360_rotary->input->dev,
> + "Failed to set max7360 rotary encoder configuration\n");
> + return ret;
> + }
> +
> + return 0;
Just
return ret;
?
> +}
...
> +static int max7360_rotary_probe(struct platform_device *pdev)
> +{
> + struct max7360_rotary *max7360_rotary;
struct device *dev = &pdev->dev;
> + struct input_dev *input;
> + int irq;
> + int ret;
> + if (!pdev->dev.parent)
> + return dev_err_probe(&pdev->dev, -ENODEV, "No parent device\n");
Just check for regmap.
> + irq = platform_get_irq_byname(to_platform_device(pdev->dev.parent),
> + "inti");
One line (and instead of that dance with container_of(), just use
fwnode_irq_get_byname().
> + if (irq < 0)
> + return irq;
> +
> + max7360_rotary = devm_kzalloc(&pdev->dev, sizeof(*max7360_rotary),
> + GFP_KERNEL);
One line.
> + if (!max7360_rotary)
> + return -ENOMEM;
> +
> + max7360_rotary->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> + if (!max7360_rotary->regmap)
> + dev_err_probe(&pdev->dev, -ENODEV,
> + "Could not get parent regmap\n");
One line.
> + device_property_read_u32(pdev->dev.parent, "linux,axis",
> + &max7360_rotary->axis);
> + device_property_read_u32(pdev->dev.parent, "rotary-debounce-delay-ms",
> + &max7360_rotary->debounce_ms);
No, instead of parent, make sure that fwnode is propagated to the children (and
tadaam, it's done in MFD already).
> + if (max7360_rotary->debounce_ms > MAX7360_ROT_DEBOUNCE_MAX)
> + return dev_err_probe(&pdev->dev, -EINVAL,
> + "Invalid debounce timing: %u\n",
> + max7360_rotary->debounce_ms);
> +
> + input = devm_input_allocate_device(&pdev->dev);
> + if (!input)
> + return -ENOMEM;
> +
> + max7360_rotary->input = input;
> +
> + input->id.bustype = BUS_I2C;
> + input->name = pdev->name;
> + input->dev.parent = &pdev->dev;
> +
> + input_set_capability(input, EV_REL, max7360_rotary->axis);
> + input_set_drvdata(input, max7360_rotary);
> +
> + ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> + max7360_rotary_irq,
> + IRQF_TRIGGER_LOW | IRQF_ONESHOT | IRQF_SHARED,
What's wrong with the firmware description of the interrupt?
> + "max7360-rotary", max7360_rotary);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "Failed to register interrupt\n");
> +
> + ret = input_register_device(input);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "Could not register input device\n");
One line.
> + platform_set_drvdata(pdev, max7360_rotary);
Is it used?
> + device_init_wakeup(&pdev->dev, true);
> + ret = dev_pm_set_wake_irq(&pdev->dev, irq);
> + if (ret)
> + dev_warn(&pdev->dev, "Failed to set up wakeup irq: %d\n", ret);
> +
> + ret = max7360_rotary_hw_init(max7360_rotary);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "Failed to initialize max7360 rotary\n");
In the other driver the HW initialisation and wake setup were swapped,
can you choose one way? Otherwise both cases need a comment to explain
the order of calls.
> + return 0;
> +}
--
With Best Regards,
Andy Shevchenko
On Wed Mar 19, 2025 at 1:11 PM CET, Andy Shevchenko wrote:
> On Tue, Mar 18, 2025 at 05:26:26PM +0100, Mathieu Dubois-Briand wrote:
> > Add driver for Maxim Integrated MAX7360 rotary encoder controller,
> > supporting a single rotary switch.
>
> ...
>
> > +static irqreturn_t max7360_rotary_irq(int irq, void *data)
> > +{
> > + struct max7360_rotary *max7360_rotary = data;
> > + int val;
> > + int ret;
> > +
> > + ret = regmap_read(max7360_rotary->regmap, MAX7360_REG_RTR_CNT, &val);
> > + if (ret < 0) {
> > + dev_err(&max7360_rotary->input->dev,
> > + "Failed to read rotary counter\n");
> > + return IRQ_NONE;
> > + }
> > +
> > + if (val == 0) {
> > + dev_dbg(&max7360_rotary->input->dev,
> > + "Got a spurious interrupt\n");
> > +
> > + return IRQ_NONE;
> > + }
> > +
> > + input_report_rel(max7360_rotary->input, max7360_rotary->axis,
> > + (int8_t)val);
>
> This is strange:
> 1) why casting to begin with?
> 2) why to C type and not kernel (s8) type?
>
I believe the cast is needed, as, while the value read with
regmap_read() is stored in an int, the underlying value is indeed a
signed 8 bits integer.
Without cast negative values will not be correct: -1 (0xFF) -> will be
interpreted as 255 (0x000000FF).
Ok for s8.
Thanks for your review
Mathieu
--
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On Tue, Mar 25, 2025 at 04:56:20PM +0100, Mathieu Dubois-Briand wrote:
> On Wed Mar 19, 2025 at 1:11 PM CET, Andy Shevchenko wrote:
> > On Tue, Mar 18, 2025 at 05:26:26PM +0100, Mathieu Dubois-Briand wrote:
...
> > > + int val;
Btw, this has to be unsigned to match the API.
> > > + int ret;
> > > +
> > > + ret = regmap_read(max7360_rotary->regmap, MAX7360_REG_RTR_CNT, &val);
> > > + if (ret < 0) {
> > > + dev_err(&max7360_rotary->input->dev,
> > > + "Failed to read rotary counter\n");
> > > + return IRQ_NONE;
> > > + }
> > > + input_report_rel(max7360_rotary->input, max7360_rotary->axis,
> > > + (int8_t)val);
> >
> > This is strange:
> > 1) why casting to begin with?
> > 2) why to C type and not kernel (s8) type?
>
> I believe the cast is needed, as, while the value read with
> regmap_read() is stored in an int, the underlying value is indeed a
> signed 8 bits integer.
>
> Without cast negative values will not be correct: -1 (0xFF) -> will be
> interpreted as 255 (0x000000FF).
With the above fix it makes sense, but it's not clear for the reader still.
What you want is most likely to call sign_extend32().
--
With Best Regards,
Andy Shevchenko
© 2016 - 2025 Red Hat, Inc.