[PATCH v6 00/12] Add support for MAX7360

[PATCH v6 00/12] Add support for MAX7360

[Mathieu Dubois-Briand]

This series implements a set of drivers allowing to support the Maxim
Integrated MAX7360 device.

The MAX7360 is an I2C key-switch and led controller, with following
functionalities:
- Keypad controller for a key matrix of up to 8 rows and 8 columns.
- Rotary encoder support, for a single rotary encoder.
- Up to 8 PWM outputs.
- Up to 8 GPIOs with support for interrupts and 6 GPOs.

Chipset pins are shared between all functionalities, so all cannot be
used at the same time.

Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
---
Changes in v6:
- Rebased on v6.15-rc1.
- Use device_set_of_node_from_dev() instead of creating PWM and Pinctrl
  on parent device.
- Various small fixes in all drivers.
- Fix pins property pattern in pinctrl dt bindings.
- Link to v5: https://lore.kernel.org/r/20250318-mdb-max7360-support-v5-0-fb20baf97da0@bootlin.com

Changes in v5:
- Add pinctrl driver to replace the previous use of request()/free()
  callbacks for PORT pins.
- Remove ngpios property from GPIO device tree bindings.
- Use GPIO valid_mask to mark unusable keypad columns GPOs, instead of
  changing ngpios.
- Drop patches adding support for request()/free() callbacks in GPIO
  regmap and gpio_regmap_get_ngpio().
- Allow gpio_regmap_register() to create the associated regmap IRQ.
- Various fixes in MFD, PWM, GPIO and KEYPAD drivers.
- Link to v4: https://lore.kernel.org/r/20250214-mdb-max7360-support-v4-0-8a35c6dbb966@bootlin.com

Changes in v4:
- Modified the GPIO driver to use gpio-regmap and regmap-irq.
- Add support for request()/free() callbacks in gpio-regmap.
- Add support for status_is_level in regmap-irq.
- Switched the PWM driver to waveform callbacks.
- Various small fixes in MFD, PWM, GPIO drivers and dt bindings.
- Rebased on v6.14-rc2.
- Link to v3: https://lore.kernel.org/r/20250113-mdb-max7360-support-v3-0-9519b4acb0b1@bootlin.com

Changes in v3:
- Fix MFD device tree binding to add gpio child nodes.
- Fix various small issues in device tree bindings.
- Add missing line returns in error messages.
- Use dev_err_probe() when possible.
- Link to v2: https://lore.kernel.org/r/20241223-mdb-max7360-support-v2-0-37a8d22c36ed@bootlin.com

Changes in v2:
- Removing device tree subnodes for keypad, rotary encoder and pwm
  functionalities.
- Fixed dt-bindings syntax and naming.
- Fixed missing handling of requested period in PWM driver.
- Cleanup of the code
- Link to v1: https://lore.kernel.org/r/20241219-mdb-max7360-support-v1-0-8e8317584121@bootlin.com

---
Kamel Bouhara (2):
      mfd: Add max7360 support
      pwm: max7360: Add MAX7360 PWM support

Mathieu Dubois-Briand (10):
      dt-bindings: mfd: gpio: Add MAX7360
      pinctrl: Add MAX7360 pinctrl driver
      regmap: irq: Remove unreachable goto
      regmap: irq: Add support for chips without separate IRQ status
      gpio: regmap: Allow to allocate regmap-irq device
      gpio: regmap: Allow to provide init_valid_mask callback
      gpio: max7360: Add MAX7360 gpio support
      input: keyboard: Add support for MAX7360 keypad
      input: misc: Add support for MAX7360 rotary
      MAINTAINERS: Add entry on MAX7360 driver

 .../bindings/gpio/maxim,max7360-gpio.yaml          |  83 ++++++
 .../devicetree/bindings/mfd/maxim,max7360.yaml     | 171 ++++++++++++
 MAINTAINERS                                        |  13 +
 drivers/base/regmap/regmap-irq.c                   |  97 ++++---
 drivers/gpio/Kconfig                               |  12 +
 drivers/gpio/Makefile                              |   1 +
 drivers/gpio/gpio-max7360.c                        | 250 +++++++++++++++++
 drivers/gpio/gpio-regmap.c                         |  22 +-
 drivers/input/keyboard/Kconfig                     |  12 +
 drivers/input/keyboard/Makefile                    |   1 +
 drivers/input/keyboard/max7360-keypad.c            | 299 +++++++++++++++++++++
 drivers/input/misc/Kconfig                         |  10 +
 drivers/input/misc/Makefile                        |   1 +
 drivers/input/misc/max7360-rotary.c                | 148 ++++++++++
 drivers/mfd/Kconfig                                |  14 +
 drivers/mfd/Makefile                               |   1 +
 drivers/mfd/max7360.c                              | 186 +++++++++++++
 drivers/pinctrl/Kconfig                            |  11 +
 drivers/pinctrl/Makefile                           |   1 +
 drivers/pinctrl/pinctrl-max7360.c                  | 208 ++++++++++++++
 drivers/pwm/Kconfig                                |  10 +
 drivers/pwm/Makefile                               |   1 +
 drivers/pwm/pwm-max7360.c                          | 190 +++++++++++++
 include/linux/gpio/regmap.h                        |  21 ++
 include/linux/mfd/max7360.h                        | 109 ++++++++
 include/linux/regmap.h                             |   3 +
 26 files changed, 1842 insertions(+), 33 deletions(-)
---
base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
change-id: 20241219-mdb-max7360-support-223a8ce45ba3

Best regards,
-- 
Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>

[PATCH v6 01/12] dt-bindings: mfd: gpio: Add MAX7360

[Mathieu Dubois-Briand]

Add device tree bindings for Maxim Integrated MAX7360 device with
support for keypad, rotary, gpios and pwm functionalities.

Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
---
 .../bindings/gpio/maxim,max7360-gpio.yaml          |  83 ++++++++++
 .../devicetree/bindings/mfd/maxim,max7360.yaml     | 171 +++++++++++++++++++++
 2 files changed, 254 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpio/maxim,max7360-gpio.yaml b/Documentation/devicetree/bindings/gpio/maxim,max7360-gpio.yaml
new file mode 100644
index 000000000000..21d603d9504c
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/maxim,max7360-gpio.yaml
@@ -0,0 +1,83 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/maxim,max7360-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim MAX7360 GPIO controller
+
+maintainers:
+  - Kamel Bouhara <kamel.bouhara@bootlin.com>
+  - Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
+
+description: |
+  Maxim MAX7360 GPIO controller, in MAX7360 chipset
+  https://www.analog.com/en/products/max7360.html
+
+  The device provide two series of GPIOs, referred here as GPIOs and GPOs.
+
+  PORT0 to PORT7 pins can be used as GPIOs, with support for interrupts and
+  constant-current mode. These pins will also be used by the torary encoder and
+  PWM functionalities.
+
+  COL2 to COL7 pins can be used as GPOs, there is no input capability. COL pins
+  will be partitionned, with the first pins being affected to the keypad
+  functionality and the last ones as GPOs.
+
+properties:
+  compatible:
+    enum:
+      - maxim,max7360-gpio
+      - maxim,max7360-gpo
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+
+  maxim,constant-current-disable:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Bit field, each bit disables constant-current output of the associated
+      GPIO, starting from the least significant bit for the first GPIO.
+    maximum: 0xff
+
+required:
+  - compatible
+  - gpio-controller
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - maxim,max7360-gpio
+        ngpios: false
+    then:
+      required:
+        - interrupt-controller
+    else:
+      properties:
+        interrupt-controller: false
+        maxim,constant-current-disable: false
+
+additionalProperties: false
+
+examples:
+  - |
+    gpio {
+      compatible = "maxim,max7360-gpio";
+
+      gpio-controller;
+      #gpio-cells = <2>;
+      maxim,constant-current-disable = <0x06>;
+
+      interrupt-controller;
+      #interrupt-cells = <2>;
+    };
diff --git a/Documentation/devicetree/bindings/mfd/maxim,max7360.yaml b/Documentation/devicetree/bindings/mfd/maxim,max7360.yaml
new file mode 100644
index 000000000000..ae3969d8dc8a
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/maxim,max7360.yaml
@@ -0,0 +1,171 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/maxim,max7360.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim MAX7360 Keypad, Rotary encoder, PWM and GPIO controller
+
+maintainers:
+  - Kamel Bouhara <kamel.bouhara@bootlin.com>
+  - Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
+
+description: |
+  Maxim MAX7360 device, with following functions:
+    - keypad controller
+    - rotary controller
+    - GPIO and GPO controller
+    - PWM controller
+
+  https://www.analog.com/en/products/max7360.html
+
+allOf:
+  - $ref: /schemas/input/matrix-keymap.yaml#
+  - $ref: /schemas/input/input.yaml#
+
+properties:
+  compatible:
+    enum:
+      - maxim,max7360
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 2
+
+  interrupt-names:
+    items:
+      - const: inti
+      - const: intk
+
+  keypad-debounce-delay-ms:
+    description: Keypad debounce delay in ms
+    minimum: 9
+    maximum: 40
+    default: 9
+
+  rotary-debounce-delay-ms:
+    description: Rotary encoder debounce delay in ms
+    minimum: 0
+    maximum: 15
+    default: 0
+
+  linux,axis:
+    $ref: /schemas/input/rotary-encoder.yaml#/properties/linux,axis
+
+  "#pwm-cells":
+    const: 3
+
+  gpio:
+    $ref: /schemas/gpio/maxim,max7360-gpio.yaml#
+    description:
+      PORT0 to PORT7 general purpose input/output pins configuration.
+
+  gpo:
+    $ref: /schemas/gpio/maxim,max7360-gpio.yaml#
+    description: >
+      COL2 to COL7 general purpose output pins configuration. Allows to use
+      unused keypad columns as outputs.
+
+      The MAX7360 has 8 column lines and 6 of them can be used as GPOs. GPIOs
+      numbers used for this gpio-controller node do correspond to the column
+      numbers: values 0 and 1 are never valid, values from 2 to 7 might be valid
+      depending on the value of the keypad,num-column property.
+
+patternProperties:
+  '-pins$':
+    type: object
+    description:
+      Pinctrl node's client devices use subnodes for desired pin configuration.
+      Client device subnodes use below standard properties.
+    $ref: /schemas/pinctrl/pincfg-node.yaml
+
+    properties:
+      pins:
+        description:
+          List of gpio pins affected by the properties specified in this
+          subnode.
+        items:
+          pattern: '^(PORT[0-7]|ROTARY)$'
+        minItems: 1
+        maxItems: 8
+
+      function:
+        description:
+          Specify the alternative function to be configured for the specified
+          pins.
+        enum: [gpio, pwm, rotary]
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+  - linux,keymap
+  - linux,axis
+  - "#pwm-cells"
+  - gpio
+  - gpo
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/input/input.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      io-expander@38 {
+        compatible = "maxim,max7360";
+        reg = <0x38>;
+
+        interrupt-parent = <&gpio1>;
+        interrupts = <23 IRQ_TYPE_LEVEL_LOW>,
+                     <24 IRQ_TYPE_LEVEL_LOW>;
+        interrupt-names = "inti", "intk";
+
+        keypad,num-rows = <8>;
+        keypad,num-columns = <4>;
+        linux,keymap = <
+          MATRIX_KEY(0x00, 0x00, KEY_F5)
+          MATRIX_KEY(0x01, 0x00, KEY_F4)
+          MATRIX_KEY(0x02, 0x01, KEY_F6)
+          >;
+        keypad-debounce-delay-ms = <10>;
+        autorepeat;
+
+        rotary-debounce-delay-ms = <2>;
+        linux,axis = <0>; /* REL_X */
+
+        #pwm-cells = <3>;
+
+        max7360_gpio: gpio {
+          compatible = "maxim,max7360-gpio";
+
+          gpio-controller;
+          #gpio-cells = <2>;
+          maxim,constant-current-disable = <0x06>;
+
+          interrupt-controller;
+          #interrupt-cells = <0x2>;
+        };
+
+        max7360_gpo: gpo {
+          compatible = "maxim,max7360-gpo";
+
+          gpio-controller;
+          #gpio-cells = <2>;
+        };
+
+        backlight_pins: backlight-pins {
+          pins = "PORT2";
+          function = "pwm";
+        };
+      };
+    };

-- 
2.39.5

[PATCH v6 02/12] mfd: Add max7360 support

[mathieu.dubois-briand]

From: Kamel Bouhara <kamel.bouhara@bootlin.com>

Add core driver to support MAX7360 i2c chip, multi function device
with keypad, GPIO, PWM, GPO and rotary encoder submodules.

Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
Co-developed-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
---
 drivers/mfd/Kconfig         |  14 ++++
 drivers/mfd/Makefile        |   1 +
 drivers/mfd/max7360.c       | 186 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/max7360.h | 109 ++++++++++++++++++++++++++
 4 files changed, 310 insertions(+)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 22b936310039..c2998c6ce54c 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2422,5 +2422,19 @@ config MFD_UPBOARD_FPGA
 	  To compile this driver as a module, choose M here: the module will be
 	  called upboard-fpga.
 
+config MFD_MAX7360
+	tristate "Maxim MAX7360 I2C IO Expander"
+	depends on I2C
+	select MFD_CORE
+	select REGMAP_I2C
+	select REGMAP_IRQ
+	help
+	  Say yes here to add support for Maxim MAX7360 device, embedding
+	  keypad, rotary encoder, PWM and GPIO features.
+
+	  This driver provides common support for accessing the device;
+	  additional drivers must be enabled in order to use the functionality
+	  of the device.
+
 endmenu
 endif
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 948cbdf42a18..add9ff58eb25 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -162,6 +162,7 @@ obj-$(CONFIG_MFD_DA9063)	+= da9063.o
 obj-$(CONFIG_MFD_DA9150)	+= da9150-core.o
 
 obj-$(CONFIG_MFD_MAX14577)	+= max14577.o
+obj-$(CONFIG_MFD_MAX7360)	+= max7360.o
 obj-$(CONFIG_MFD_MAX77541)	+= max77541.o
 obj-$(CONFIG_MFD_MAX77620)	+= max77620.o
 obj-$(CONFIG_MFD_MAX77650)	+= max77650.o
diff --git a/drivers/mfd/max7360.c b/drivers/mfd/max7360.c
new file mode 100644
index 000000000000..97a508ff7347
--- /dev/null
+++ b/drivers/mfd/max7360.c
@@ -0,0 +1,186 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Maxim MAX7360 Core Driver
+ *
+ * Copyright 2025 Bootlin
+ *
+ * Author: Kamel Bouhara <kamel.bouhara@bootlin.com>
+ * Author: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
+ */
+
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/device/devres.h>
+#include <linux/dev_printk.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/max7360.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+static const struct mfd_cell max7360_cells[] = {
+	{
+		.name           = "max7360-pinctrl",
+	},
+	{
+		.name           = "max7360-pwm",
+	},
+	{
+		.name           = "max7360-gpo",
+		.of_compatible	= "maxim,max7360-gpo",
+	},
+	{
+		.name           = "max7360-gpio",
+		.of_compatible	= "maxim,max7360-gpio",
+	},
+	{
+		.name           = "max7360-keypad",
+	},
+	{
+		.name           = "max7360-rotary",
+	},
+};
+
+static const struct regmap_range max7360_volatile_ranges[] = {
+	{
+		.range_min = MAX7360_REG_KEYFIFO,
+		.range_max = MAX7360_REG_KEYFIFO,
+	}, {
+		.range_min = MAX7360_REG_I2C_TIMEOUT,
+		.range_max = MAX7360_REG_RTR_CNT,
+	},
+};
+
+static const struct regmap_access_table max7360_volatile_table = {
+	.yes_ranges = max7360_volatile_ranges,
+	.n_yes_ranges = ARRAY_SIZE(max7360_volatile_ranges),
+};
+
+static const struct regmap_config max7360_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = MAX7360_REG_PWMCFG(MAX7360_PORT_PWM_COUNT - 1),
+	.volatile_table = &max7360_volatile_table,
+	.cache_type = REGCACHE_MAPLE,
+};
+
+static int max7360_mask_irqs(struct regmap *regmap)
+{
+	struct device *dev = regmap_get_device(regmap);
+	unsigned int val;
+	int ret;
+
+	/*
+	 * GPIO/PWM interrupts are not masked on reset: as the MAX7360 "INTI"
+	 * interrupt line is shared between GPIOs and rotary encoder, this could
+	 * result in repeated spurious interrupts on the rotary encoder driver
+	 * if the GPIO driver is not loaded. Mask them now to avoid this
+	 * situation.
+	 */
+	for (unsigned int i = 0; i < MAX7360_PORT_PWM_COUNT; i++) {
+		ret = regmap_write_bits(regmap, MAX7360_REG_PWMCFG(i),
+					MAX7360_PORT_CFG_INTERRUPT_MASK,
+					MAX7360_PORT_CFG_INTERRUPT_MASK);
+		if (ret) {
+			dev_err(dev, "Failed to write max7360 port configuration");
+			return ret;
+		}
+	}
+
+	/* Read GPIO in register, to ACK any pending IRQ. */
+	ret = regmap_read(regmap, MAX7360_REG_GPIOIN, &val);
+	if (ret)
+		dev_err(dev, "Failed to read gpio values: %d\n", ret);
+
+	return ret;
+}
+
+static int max7360_reset(struct regmap *regmap)
+{
+	struct device *dev = regmap_get_device(regmap);
+	int ret;
+
+	ret = regmap_write(regmap, MAX7360_REG_GPIOCFG, MAX7360_GPIO_CFG_GPIO_RST);
+	if (ret) {
+		dev_err(dev, "Failed to reset GPIO configuration: %x\n", ret);
+		return ret;
+	}
+
+	ret = regcache_drop_region(regmap, MAX7360_REG_GPIOCFG, MAX7360_REG_GPIO_LAST);
+	if (ret) {
+		dev_err(dev, "Failed to drop regmap cache: %x\n", ret);
+		return ret;
+	}
+
+	ret = regmap_write(regmap, MAX7360_REG_SLEEP, 0);
+	if (ret) {
+		dev_err(dev, "Failed to reset autosleep configuration: %x\n", ret);
+		return ret;
+	}
+
+	ret = regmap_write(regmap, MAX7360_REG_DEBOUNCE, 0);
+	if (ret)
+		dev_err(dev, "Failed to reset GPO port count: %x\n", ret);
+
+	return ret;
+}
+
+static int max7360_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct regmap *regmap;
+	int ret;
+
+	regmap = devm_regmap_init_i2c(client, &max7360_regmap_config);
+	if (IS_ERR(regmap))
+		return dev_err_probe(dev, PTR_ERR(regmap), "Failed to initialise regmap\n");
+
+	i2c_set_clientdata(client, regmap);
+
+	ret = max7360_reset(regmap);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to reset device\n");
+
+	/* Get the device out of shutdown mode. */
+	ret = regmap_write_bits(regmap, MAX7360_REG_GPIOCFG,
+				MAX7360_GPIO_CFG_GPIO_EN,
+				MAX7360_GPIO_CFG_GPIO_EN);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable GPIO and PWM module\n");
+
+	ret = max7360_mask_irqs(regmap);
+	if (ret)
+		return dev_err_probe(dev, ret, "Could not mask interrupts\n");
+
+	ret =  devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
+				    max7360_cells, ARRAY_SIZE(max7360_cells),
+				    NULL, 0, NULL);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to register child devices\n");
+
+	return 0;
+}
+
+static const struct of_device_id max7360_dt_match[] = {
+	{ .compatible = "maxim,max7360" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, max7360_dt_match);
+
+static struct i2c_driver max7360_driver = {
+	.driver = {
+		.name = "max7360",
+		.of_match_table = max7360_dt_match,
+	},
+	.probe = max7360_probe,
+};
+module_i2c_driver(max7360_driver);
+
+MODULE_DESCRIPTION("Maxim MAX7360 I2C IO Expander core driver");
+MODULE_AUTHOR("Kamel Bouhara <kamel.bouhara@bootlin.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/max7360.h b/include/linux/mfd/max7360.h
new file mode 100644
index 000000000000..b1d4cbee2385
--- /dev/null
+++ b/include/linux/mfd/max7360.h
@@ -0,0 +1,109 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __LINUX_MFD_MAX7360_H
+#define __LINUX_MFD_MAX7360_H
+
+#include <linux/bits.h>
+
+#define MAX7360_MAX_KEY_ROWS		8
+#define MAX7360_MAX_KEY_COLS		8
+#define MAX7360_MAX_KEY_NUM		(MAX7360_MAX_KEY_ROWS * MAX7360_MAX_KEY_COLS)
+#define MAX7360_ROW_SHIFT		3
+
+#define MAX7360_MAX_GPIO		8
+#define MAX7360_MAX_GPO			6
+#define MAX7360_PORT_PWM_COUNT		8
+#define MAX7360_PORT_RTR_PIN		(MAX7360_PORT_PWM_COUNT - 1)
+
+/*
+ * MAX7360 registers
+ */
+#define MAX7360_REG_KEYFIFO		0x00
+#define MAX7360_REG_CONFIG		0x01
+#define MAX7360_REG_DEBOUNCE		0x02
+#define MAX7360_REG_INTERRUPT		0x03
+#define MAX7360_REG_PORTS		0x04
+#define MAX7360_REG_KEYREP		0x05
+#define MAX7360_REG_SLEEP		0x06
+
+/*
+ * MAX7360 GPIO registers
+ *
+ * All these registers are reset together when writing bit 3 of
+ * MAX7360_REG_GPIOCFG.
+ */
+#define MAX7360_REG_GPIOCFG		0x40
+#define MAX7360_REG_GPIOCTRL		0x41
+#define MAX7360_REG_GPIODEB		0x42
+#define MAX7360_REG_GPIOCURR		0x43
+#define MAX7360_REG_GPIOOUTM		0x44
+#define MAX7360_REG_PWMCOM		0x45
+#define MAX7360_REG_RTRCFG		0x46
+#define MAX7360_REG_I2C_TIMEOUT		0x48
+#define MAX7360_REG_GPIOIN		0x49
+#define MAX7360_REG_RTR_CNT		0x4A
+#define MAX7360_REG_PWMBASE		0x50
+#define MAX7360_REG_PWMCFGBASE		0x58
+
+#define MAX7360_REG_GPIO_LAST		0x5F
+
+#define MAX7360_REG_PWM(x)		(MAX7360_REG_PWMBASE + (x))
+#define MAX7360_REG_PWMCFG(x)		(MAX7360_REG_PWMCFGBASE + (x))
+
+/*
+ * Configuration register bits
+ */
+#define MAX7360_FIFO_EMPTY		0x3f
+#define MAX7360_FIFO_OVERFLOW		0x7f
+#define MAX7360_FIFO_RELEASE		BIT(6)
+#define MAX7360_FIFO_COL		GENMASK(5, 3)
+#define MAX7360_FIFO_ROW		GENMASK(2, 0)
+
+#define MAX7360_CFG_SLEEP		BIT(7)
+#define MAX7360_CFG_INTERRUPT		BIT(5)
+#define MAX7360_CFG_KEY_RELEASE		BIT(3)
+#define MAX7360_CFG_WAKEUP		BIT(1)
+#define MAX7360_CFG_TIMEOUT		BIT(0)
+
+#define MAX7360_DEBOUNCE		GENMASK(4, 0)
+#define MAX7360_DEBOUNCE_MIN		9
+#define MAX7360_DEBOUNCE_MAX		40
+#define MAX7360_PORTS			GENMASK(8, 5)
+
+#define MAX7360_INTERRUPT_TIME_MASK	GENMASK(4, 0)
+#define MAX7360_INTERRUPT_FIFO_MASK	GENMASK(7, 5)
+
+#define MAX7360_PORT_CFG_INTERRUPT_MASK		BIT(7)
+#define MAX7360_PORT_CFG_INTERRUPT_EDGES	BIT(6)
+#define MAX7360_PORT_CFG_COMMON_PWM		BIT(5)
+
+/*
+ * Autosleep register values
+ */
+#define MAX7360_AUTOSLEEP_8192MS	0x01
+#define MAX7360_AUTOSLEEP_4096MS	0x02
+#define MAX7360_AUTOSLEEP_2048MS	0x03
+#define MAX7360_AUTOSLEEP_1024MS	0x04
+#define MAX7360_AUTOSLEEP_512MS		0x05
+#define MAX7360_AUTOSLEEP_256MS		0x06
+
+#define MAX7360_GPIO_CFG_RTR_EN		BIT(7)
+#define MAX7360_GPIO_CFG_GPIO_EN	BIT(4)
+#define MAX7360_GPIO_CFG_GPIO_RST	BIT(3)
+
+#define MAX7360_ROT_DEBOUNCE		GENMASK(3, 0)
+#define MAX7360_ROT_DEBOUNCE_MIN	0
+#define MAX7360_ROT_DEBOUNCE_MAX	15
+#define MAX7360_ROT_INTCNT		GENMASK(6, 4)
+#define MAX7360_ROT_INTCNT_DLY		BIT(7)
+
+#define MAX7360_INT_INTI		0
+#define MAX7360_INT_INTK		1
+
+#define MAX7360_INT_GPIO		0
+#define MAX7360_INT_KEYPAD		1
+#define MAX7360_INT_ROTARY		2
+
+#define MAX7360_NR_INTERNAL_IRQS	3
+
+#endif

-- 
2.39.5

[PATCH v6 03/12] pinctrl: Add MAX7360 pinctrl driver

[Mathieu Dubois-Briand]

Add driver for Maxim Integrated MAX7360 pinctrl on the PORT pins. Pins
can be used either for GPIO, PWM or rotary encoder functionalities.

Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/pinctrl/Kconfig           |  11 ++
 drivers/pinctrl/Makefile          |   1 +
 drivers/pinctrl/pinctrl-max7360.c | 208 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 220 insertions(+)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 464cc9aca157..276695c7a92e 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -348,6 +348,17 @@ config PINCTRL_LPC18XX
 	help
 	  Pinctrl driver for NXP LPC18xx/43xx System Control Unit (SCU).
 
+config PINCTRL_MAX7360
+	tristate "MAX7360 Pincontrol support"
+	depends on MFD_MAX7360
+	select PINMUX
+	select GENERIC_PINCONF
+	help
+	  Say Y here to enable pin control support for Maxim MAX7360 keypad
+	  controller.
+	  This keypad controller has 8 GPIO pins that may work as GPIO, or PWM,
+	  or rotary encoder alternate modes.
+
 config PINCTRL_MAX77620
 	tristate "MAX77620/MAX20024 Pincontrol support"
 	depends on MFD_MAX77620 && OF
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index ac27e88677d1..974a179b5593 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_PINCTRL_FALCON)	+= pinctrl-falcon.o
 obj-$(CONFIG_PINCTRL_LOONGSON2) += pinctrl-loongson2.o
 obj-$(CONFIG_PINCTRL_XWAY)	+= pinctrl-xway.o
 obj-$(CONFIG_PINCTRL_LPC18XX)	+= pinctrl-lpc18xx.o
+obj-$(CONFIG_PINCTRL_MAX7360)	+= pinctrl-max7360.o
 obj-$(CONFIG_PINCTRL_MAX77620)	+= pinctrl-max77620.o
 obj-$(CONFIG_PINCTRL_MCP23S08_I2C)	+= pinctrl-mcp23s08_i2c.o
 obj-$(CONFIG_PINCTRL_MCP23S08_SPI)	+= pinctrl-mcp23s08_spi.o
diff --git a/drivers/pinctrl/pinctrl-max7360.c b/drivers/pinctrl/pinctrl-max7360.c
new file mode 100644
index 000000000000..ebd98666cd39
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-max7360.c
@@ -0,0 +1,208 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2025 Bootlin
+ *
+ * Author: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
+ */
+
+#include <linux/array_size.h>
+#include <linux/dev_printk.h>
+#include <linux/device.h>
+#include <linux/device/devres.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/mfd/max7360.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinmux.h>
+
+#include "core.h"
+#include "pinmux.h"
+
+struct max7360_pinctrl {
+	struct pinctrl_dev *pctldev;
+	struct pinctrl_desc pinctrl_desc;
+};
+
+static const struct pinctrl_pin_desc max7360_pins[] = {
+	PINCTRL_PIN(0, "PORT0"),
+	PINCTRL_PIN(1, "PORT1"),
+	PINCTRL_PIN(2, "PORT2"),
+	PINCTRL_PIN(3, "PORT3"),
+	PINCTRL_PIN(4, "PORT4"),
+	PINCTRL_PIN(5, "PORT5"),
+	PINCTRL_PIN(6, "PORT6"),
+	PINCTRL_PIN(7, "PORT7"),
+};
+
+static const unsigned int port0_pins[] = {0};
+static const unsigned int port1_pins[] = {1};
+static const unsigned int port2_pins[] = {2};
+static const unsigned int port3_pins[] = {3};
+static const unsigned int port4_pins[] = {4};
+static const unsigned int port5_pins[] = {5};
+static const unsigned int port6_pins[] = {6};
+static const unsigned int port7_pins[] = {7};
+static const unsigned int rotary_pins[] = {6, 7};
+
+static const struct pingroup max7360_groups[] = {
+	PINCTRL_PINGROUP("PORT0", port0_pins, ARRAY_SIZE(port0_pins)),
+	PINCTRL_PINGROUP("PORT1", port1_pins, ARRAY_SIZE(port1_pins)),
+	PINCTRL_PINGROUP("PORT2", port2_pins, ARRAY_SIZE(port2_pins)),
+	PINCTRL_PINGROUP("PORT3", port3_pins, ARRAY_SIZE(port3_pins)),
+	PINCTRL_PINGROUP("PORT4", port4_pins, ARRAY_SIZE(port4_pins)),
+	PINCTRL_PINGROUP("PORT5", port5_pins, ARRAY_SIZE(port5_pins)),
+	PINCTRL_PINGROUP("PORT6", port6_pins, ARRAY_SIZE(port6_pins)),
+	PINCTRL_PINGROUP("PORT7", port7_pins, ARRAY_SIZE(port7_pins)),
+	PINCTRL_PINGROUP("ROTARY", rotary_pins, ARRAY_SIZE(rotary_pins)),
+};
+
+static int max7360_pinctrl_get_groups_count(struct pinctrl_dev *pctldev)
+{
+	return ARRAY_SIZE(max7360_groups);
+}
+
+static const char *max7360_pinctrl_get_group_name(struct pinctrl_dev *pctldev,
+						  unsigned int group)
+{
+	return max7360_groups[group].name;
+}
+
+static int max7360_pinctrl_get_group_pins(struct pinctrl_dev *pctldev,
+					  unsigned int group,
+					  const unsigned int **pins,
+					  unsigned int *num_pins)
+{
+	*pins = max7360_groups[group].pins;
+	*num_pins = max7360_groups[group].npins;
+	return 0;
+}
+
+static const struct pinctrl_ops max7360_pinctrl_ops = {
+	.get_groups_count = max7360_pinctrl_get_groups_count,
+	.get_group_name = max7360_pinctrl_get_group_name,
+	.get_group_pins = max7360_pinctrl_get_group_pins,
+#ifdef CONFIG_OF
+	.dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
+	.dt_free_map = pinconf_generic_dt_free_map,
+#endif
+};
+
+static const char * const simple_groups[] = {
+	"PORT0", "PORT1", "PORT2", "PORT3",
+	"PORT4", "PORT5", "PORT6", "PORT7",
+};
+
+static const char * const rotary_groups[] = { "ROTARY" };
+
+#define MAX7360_PINCTRL_FN_GPIO		0
+#define MAX7360_PINCTRL_FN_PWM		1
+#define MAX7360_PINCTRL_FN_ROTARY	2
+static const struct pinfunction max7360_functions[] = {
+	[MAX7360_PINCTRL_FN_GPIO] = PINCTRL_PINFUNCTION("gpio", simple_groups,
+							ARRAY_SIZE(simple_groups)),
+	[MAX7360_PINCTRL_FN_PWM] = PINCTRL_PINFUNCTION("pwm", simple_groups,
+						       ARRAY_SIZE(simple_groups)),
+	[MAX7360_PINCTRL_FN_ROTARY] = PINCTRL_PINFUNCTION("rotary", rotary_groups,
+							  ARRAY_SIZE(rotary_groups)),
+};
+
+static int max7360_get_functions_count(struct pinctrl_dev *pctldev)
+{
+	return ARRAY_SIZE(max7360_functions);
+}
+
+static const char *max7360_get_function_name(struct pinctrl_dev *pctldev, unsigned int selector)
+{
+	return max7360_functions[selector].name;
+}
+
+static int max7360_get_function_groups(struct pinctrl_dev *pctldev, unsigned int selector,
+				       const char * const **groups,
+				       unsigned int * const num_groups)
+{
+	*groups = max7360_functions[selector].groups;
+	*num_groups = max7360_functions[selector].ngroups;
+
+	return 0;
+}
+
+static int max7360_set_mux(struct pinctrl_dev *pctldev, unsigned int selector,
+			   unsigned int group)
+{
+	struct regmap *regmap;
+	int val;
+
+	/*
+	 * GPIO and PWM functions are the same: we only need to handle the
+	 * rotary encoder function, on pins 6 and 7.
+	 */
+	if (max7360_groups[group].pins[0] >= 6) {
+		if (selector == MAX7360_PINCTRL_FN_ROTARY)
+			val = MAX7360_GPIO_CFG_RTR_EN;
+		else
+			val = 0;
+
+		regmap = dev_get_regmap(pctldev->dev->parent, NULL);
+		return regmap_write_bits(regmap, MAX7360_REG_GPIOCFG, MAX7360_GPIO_CFG_RTR_EN, val);
+	}
+
+	return 0;
+}
+
+static const struct pinmux_ops max7360_pmxops = {
+	.get_functions_count = max7360_get_functions_count,
+	.get_function_name = max7360_get_function_name,
+	.get_function_groups = max7360_get_function_groups,
+	.set_mux = max7360_set_mux,
+	.strict = true,
+};
+
+static int max7360_pinctrl_probe(struct platform_device *pdev)
+{
+	struct regmap *regmap;
+	struct pinctrl_desc *pd;
+	struct max7360_pinctrl *chip;
+	struct device *dev = &pdev->dev;
+
+	regmap = dev_get_regmap(dev->parent, NULL);
+	if (!regmap)
+		dev_err_probe(dev, -ENODEV, "Could not get parent regmap\n");
+
+	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	pd = &chip->pinctrl_desc;
+
+	pd->pctlops = &max7360_pinctrl_ops;
+	pd->pmxops = &max7360_pmxops;
+	pd->name = dev_name(dev);
+	pd->pins = max7360_pins;
+	pd->npins = MAX7360_MAX_GPIO;
+	pd->owner = THIS_MODULE;
+
+	device_set_of_node_from_dev(dev, dev->parent);
+	chip->pctldev = devm_pinctrl_register(dev, pd, chip);
+	if (IS_ERR(chip->pctldev))
+		return dev_err_probe(dev, PTR_ERR(chip->pctldev), "can't register controller\n");
+
+	return 0;
+}
+
+static struct platform_driver max7360_pinctrl_driver = {
+	.driver = {
+		.name	= "max7360-pinctrl",
+	},
+	.probe		= max7360_pinctrl_probe,
+};
+module_platform_driver(max7360_pinctrl_driver);
+
+MODULE_DESCRIPTION("MAX7360 pinctrl driver");
+MODULE_AUTHOR("Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>");
+MODULE_LICENSE("GPL");

-- 
2.39.5

[PATCH v6 04/12] pwm: max7360: Add MAX7360 PWM support

[mathieu.dubois-briand]

From: Kamel Bouhara <kamel.bouhara@bootlin.com>

Add driver for Maxim Integrated MAX7360 PWM controller, supporting up to
8 independent PWM outputs.

Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
Co-developed-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
---
 drivers/pwm/Kconfig       |  10 +++
 drivers/pwm/Makefile      |   1 +
 drivers/pwm/pwm-max7360.c | 190 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 201 insertions(+)

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 4731d5b90d7e..0b22141cbf85 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -755,4 +755,14 @@ config PWM_XILINX
 	  To compile this driver as a module, choose M here: the module
 	  will be called pwm-xilinx.
 
+config PWM_MAX7360
+	tristate "MAX7360 PWMs"
+	depends on MFD_MAX7360
+	help
+	  PWM driver for Maxim Integrated MAX7360 multifunction device, with
+	  support for up to 8 PWM outputs.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called pwm-max7360.
+
 endif
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 539e0def3f82..9c7701d8070b 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_PWM_LPC32XX)	+= pwm-lpc32xx.o
 obj-$(CONFIG_PWM_LPSS)		+= pwm-lpss.o
 obj-$(CONFIG_PWM_LPSS_PCI)	+= pwm-lpss-pci.o
 obj-$(CONFIG_PWM_LPSS_PLATFORM)	+= pwm-lpss-platform.o
+obj-$(CONFIG_PWM_MAX7360)	+= pwm-max7360.o
 obj-$(CONFIG_PWM_MESON)		+= pwm-meson.o
 obj-$(CONFIG_PWM_MEDIATEK)	+= pwm-mediatek.o
 obj-$(CONFIG_PWM_MICROCHIP_CORE)	+= pwm-microchip-core.o
diff --git a/drivers/pwm/pwm-max7360.c b/drivers/pwm/pwm-max7360.c
new file mode 100644
index 000000000000..e68a5a0c69e6
--- /dev/null
+++ b/drivers/pwm/pwm-max7360.c
@@ -0,0 +1,190 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2025 Bootlin
+ *
+ * Author: Kamel BOUHARA <kamel.bouhara@bootlin.com>
+ * Author: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
+ *
+ * Limitations:
+ * - Only supports normal polarity.
+ * - The period is fixed to 2 ms.
+ * - Only the duty cycle can be changed, new values are applied at the beginning
+ *   of the next cycle.
+ * - When disabled, the output is put in Hi-Z.
+ */
+#include <linux/bits.h>
+#include <linux/dev_printk.h>
+#include <linux/err.h>
+#include <linux/math64.h>
+#include <linux/mfd/max7360.h>
+#include <linux/minmax.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/regmap.h>
+#include <linux/time.h>
+#include <linux/types.h>
+
+#define MAX7360_NUM_PWMS			8
+#define MAX7360_PWM_MAX_RES			255
+#define MAX7360_PWM_PERIOD_NS			(2 * NSEC_PER_MSEC)
+
+struct max7360_pwm_waveform {
+	u8 duty_steps;
+	bool enabled;
+};
+
+static int max7360_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct regmap *regmap;
+	int ret;
+
+	regmap = pwmchip_get_drvdata(chip);
+	ret = regmap_write_bits(regmap, MAX7360_REG_PWMCFG(pwm->hwpwm),
+				MAX7360_PORT_CFG_COMMON_PWM, 0);
+	if (ret)
+		return ret;
+
+	return regmap_write_bits(regmap, MAX7360_REG_PORTS, BIT(pwm->hwpwm), BIT(pwm->hwpwm));
+}
+
+static void max7360_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct regmap *regmap;
+	struct device *dev;
+
+	regmap = pwmchip_get_drvdata(chip);
+	dev = regmap_get_device(regmap);
+}
+
+static int max7360_pwm_round_waveform_tohw(struct pwm_chip *chip,
+					   struct pwm_device *pwm,
+					   const struct pwm_waveform *wf,
+					   void *_wfhw)
+{
+	struct max7360_pwm_waveform *wfhw = _wfhw;
+	u64 duty_steps;
+
+	/*
+	 * Ignore user provided values for period_length_ns and duty_offset_ns:
+	 * we only support fixed period of MAX7360_PWM_PERIOD_NS and offset of 0.
+	 */
+	duty_steps = mul_u64_u64_div_u64(wf->duty_length_ns, MAX7360_PWM_MAX_RES,
+					 MAX7360_PWM_PERIOD_NS);
+
+	wfhw->duty_steps = min(MAX7360_PWM_MAX_RES, duty_steps);
+	wfhw->enabled = (wf->duty_length_ns != 0);
+
+	return 0;
+}
+
+static int max7360_pwm_round_waveform_fromhw(struct pwm_chip *chip, struct pwm_device *pwm,
+					     const void *_wfhw, struct pwm_waveform *wf)
+{
+	const struct max7360_pwm_waveform *wfhw = _wfhw;
+
+	wf->period_length_ns = wfhw->enabled ? MAX7360_PWM_PERIOD_NS : 0;
+	wf->duty_offset_ns = 0;
+	wf->duty_length_ns = DIV64_U64_ROUND_UP(wfhw->duty_steps * MAX7360_PWM_PERIOD_NS,
+						MAX7360_PWM_MAX_RES);
+
+	return 0;
+}
+
+static int max7360_pwm_write_waveform(struct pwm_chip *chip,
+				      struct pwm_device *pwm,
+				      const void *_wfhw)
+{
+	const struct max7360_pwm_waveform *wfhw = _wfhw;
+	struct regmap *regmap;
+	unsigned int val;
+	int ret;
+
+	regmap = pwmchip_get_drvdata(chip);
+	val = wfhw->enabled ? BIT(pwm->hwpwm) : 0;
+	ret = regmap_write_bits(regmap, MAX7360_REG_GPIOCTRL, BIT(pwm->hwpwm), val);
+	if (ret)
+		return ret;
+
+	if (wfhw->duty_steps)
+		return regmap_write(regmap, MAX7360_REG_PWM(pwm->hwpwm), wfhw->duty_steps);
+
+	return 0;
+}
+
+static int max7360_pwm_read_waveform(struct pwm_chip *chip,
+				     struct pwm_device *pwm,
+				     void *_wfhw)
+{
+	struct max7360_pwm_waveform *wfhw = _wfhw;
+	struct regmap *regmap;
+	unsigned int val;
+	int ret;
+
+	regmap = pwmchip_get_drvdata(chip);
+
+	ret = regmap_read(regmap, MAX7360_REG_GPIOCTRL, &val);
+	if (ret)
+		return ret;
+
+	if (val & BIT(pwm->hwpwm)) {
+		wfhw->enabled = true;
+		ret = regmap_read(regmap, MAX7360_REG_PWM(pwm->hwpwm), &val);
+		if (!ret)
+			wfhw->duty_steps = val;
+	} else {
+		wfhw->enabled = false;
+		wfhw->duty_steps = 0;
+	}
+
+	return ret;
+}
+
+static const struct pwm_ops max7360_pwm_ops = {
+	.request = max7360_pwm_request,
+	.free = max7360_pwm_free,
+	.round_waveform_tohw = max7360_pwm_round_waveform_tohw,
+	.round_waveform_fromhw = max7360_pwm_round_waveform_fromhw,
+	.read_waveform = max7360_pwm_read_waveform,
+	.write_waveform = max7360_pwm_write_waveform,
+};
+
+static int max7360_pwm_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct pwm_chip *chip;
+	struct regmap *regmap;
+	int ret;
+
+	regmap = dev_get_regmap(dev->parent, NULL);
+	if (!regmap)
+		return dev_err_probe(dev, -ENODEV, "could not get parent regmap\n");
+
+	device_set_of_node_from_dev(dev, dev->parent);
+	chip = devm_pwmchip_alloc(dev, MAX7360_NUM_PWMS, 0);
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+	chip->ops = &max7360_pwm_ops;
+
+	pwmchip_set_drvdata(chip, regmap);
+
+	ret = devm_pwmchip_add(dev, chip);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to add PWM chip\n");
+
+	return 0;
+}
+
+static struct platform_driver max7360_pwm_driver = {
+	.driver = {
+		.name = "max7360-pwm",
+	},
+	.probe = max7360_pwm_probe,
+};
+module_platform_driver(max7360_pwm_driver);
+
+MODULE_DESCRIPTION("MAX7360 PWM driver");
+MODULE_AUTHOR("Kamel BOUHARA <kamel.bouhara@bootlin.com>");
+MODULE_AUTHOR("Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>");
+MODULE_LICENSE("GPL");

-- 
2.39.5

[PATCH v6 05/12] regmap: irq: Remove unreachable goto

[Mathieu Dubois-Briand]

BUG() never returns, so code after it is unreachable: remove it.

Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
---
 drivers/base/regmap/regmap-irq.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index 6c6869188c31..14f5fcc3ec1d 100644
--- a/drivers/base/regmap/regmap-irq.c
+++ b/drivers/base/regmap/regmap-irq.c
@@ -436,7 +436,6 @@ static irqreturn_t regmap_irq_thread(int irq, void *d)
 				break;
 			default:
 				BUG();
-				goto exit;
 			}
 		}
 

-- 
2.39.5

[PATCH v6 06/12] regmap: irq: Add support for chips without separate IRQ status

[Mathieu Dubois-Briand]

Some GPIO chips allow to rise an IRQ on GPIO level changes but do not
provide an IRQ status for each separate line: only the current gpio
level can be retrieved.

Add support for these chips, emulating IRQ status by comparing GPIO
levels with the levels during the previous interrupt.

Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/regmap/regmap-irq.c | 95 +++++++++++++++++++++++++++-------------
 include/linux/regmap.h           |  3 ++
 2 files changed, 68 insertions(+), 30 deletions(-)

diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index 14f5fcc3ec1d..b4a72750ff6d 100644
--- a/drivers/base/regmap/regmap-irq.c
+++ b/drivers/base/regmap/regmap-irq.c
@@ -6,6 +6,7 @@
 //
 // Author: Mark Brown <broonie@opensource.wolfsonmicro.com>
 
+#include <linux/array_size.h>
 #include <linux/device.h>
 #include <linux/export.h>
 #include <linux/interrupt.h>
@@ -33,6 +34,7 @@ struct regmap_irq_chip_data {
 	void *status_reg_buf;
 	unsigned int *main_status_buf;
 	unsigned int *status_buf;
+	unsigned int *prev_status_buf;
 	unsigned int *mask_buf;
 	unsigned int *mask_buf_def;
 	unsigned int *wake_buf;
@@ -332,27 +334,13 @@ static inline int read_sub_irq_data(struct regmap_irq_chip_data *data,
 	return ret;
 }
 
-static irqreturn_t regmap_irq_thread(int irq, void *d)
+static int read_irq_data(struct regmap_irq_chip_data *data)
 {
-	struct regmap_irq_chip_data *data = d;
 	const struct regmap_irq_chip *chip = data->chip;
 	struct regmap *map = data->map;
 	int ret, i;
-	bool handled = false;
 	u32 reg;
 
-	if (chip->handle_pre_irq)
-		chip->handle_pre_irq(chip->irq_drv_data);
-
-	if (chip->runtime_pm) {
-		ret = pm_runtime_get_sync(map->dev);
-		if (ret < 0) {
-			dev_err(map->dev, "IRQ thread failed to resume: %d\n",
-				ret);
-			goto exit;
-		}
-	}
-
 	/*
 	 * Read only registers with active IRQs if the chip has 'main status
 	 * register'. Else read in the statuses, using a single bulk read if
@@ -379,10 +367,8 @@ static irqreturn_t regmap_irq_thread(int irq, void *d)
 			reg = data->get_irq_reg(data, chip->main_status, i);
 			ret = regmap_read(map, reg, &data->main_status_buf[i]);
 			if (ret) {
-				dev_err(map->dev,
-					"Failed to read IRQ status %d\n",
-					ret);
-				goto exit;
+				dev_err(map->dev, "Failed to read IRQ status %d\n", ret);
+				return ret;
 			}
 		}
 
@@ -398,10 +384,8 @@ static irqreturn_t regmap_irq_thread(int irq, void *d)
 				ret = read_sub_irq_data(data, b);
 
 				if (ret != 0) {
-					dev_err(map->dev,
-						"Failed to read IRQ status %d\n",
-						ret);
-					goto exit;
+					dev_err(map->dev, "Failed to read IRQ status %d\n", ret);
+					return ret;
 				}
 			}
 
@@ -418,9 +402,8 @@ static irqreturn_t regmap_irq_thread(int irq, void *d)
 				       data->status_reg_buf,
 				       chip->num_regs);
 		if (ret != 0) {
-			dev_err(map->dev, "Failed to read IRQ status: %d\n",
-				ret);
-			goto exit;
+			dev_err(map->dev, "Failed to read IRQ status: %d\n", ret);
+			return ret;
 		}
 
 		for (i = 0; i < data->chip->num_regs; i++) {
@@ -446,10 +429,8 @@ static irqreturn_t regmap_irq_thread(int irq, void *d)
 			ret = regmap_read(map, reg, &data->status_buf[i]);
 
 			if (ret != 0) {
-				dev_err(map->dev,
-					"Failed to read IRQ status: %d\n",
-					ret);
-				goto exit;
+				dev_err(map->dev, "Failed to read IRQ status: %d\n", ret);
+				return ret;
 			}
 		}
 	}
@@ -458,6 +439,42 @@ static irqreturn_t regmap_irq_thread(int irq, void *d)
 		for (i = 0; i < data->chip->num_regs; i++)
 			data->status_buf[i] = ~data->status_buf[i];
 
+	return 0;
+}
+
+static irqreturn_t regmap_irq_thread(int irq, void *d)
+{
+	struct regmap_irq_chip_data *data = d;
+	const struct regmap_irq_chip *chip = data->chip;
+	struct regmap *map = data->map;
+	int ret, i;
+	bool handled = false;
+	u32 reg;
+
+	if (chip->handle_pre_irq)
+		chip->handle_pre_irq(chip->irq_drv_data);
+
+	if (chip->runtime_pm) {
+		ret = pm_runtime_get_sync(map->dev);
+		if (ret < 0) {
+			dev_err(map->dev, "IRQ thread failed to resume: %d\n", ret);
+			goto exit;
+		}
+	}
+
+	ret = read_irq_data(data);
+	if (ret < 0)
+		goto exit;
+
+	if (chip->status_is_level) {
+		for (i = 0; i < data->chip->num_regs; i++) {
+			unsigned int val = data->status_buf[i];
+
+			data->status_buf[i] ^= data->prev_status_buf[i];
+			data->prev_status_buf[i] = val;
+		}
+	}
+
 	/*
 	 * Ignore masked IRQs and ack if we need to; we ack early so
 	 * there is no race between handling and acknowledging the
@@ -704,6 +721,13 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
 	if (!d->status_buf)
 		goto err_alloc;
 
+	if (chip->status_is_level) {
+		d->prev_status_buf = kcalloc(chip->num_regs, sizeof(*d->prev_status_buf),
+					     GFP_KERNEL);
+		if (!d->prev_status_buf)
+			goto err_alloc;
+	}
+
 	d->mask_buf = kcalloc(chip->num_regs, sizeof(*d->mask_buf),
 			      GFP_KERNEL);
 	if (!d->mask_buf)
@@ -880,6 +904,15 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
 		}
 	}
 
+	/* Store current levels */
+	if (chip->status_is_level) {
+		ret = read_irq_data(d);
+		if (ret < 0)
+			goto err_alloc;
+
+		memcpy(d->prev_status_buf, d->status_buf, array_size(d->prev_status_buf));
+	}
+
 	ret = regmap_irq_create_domain(fwnode, irq_base, chip, d);
 	if (ret)
 		goto err_alloc;
@@ -907,6 +940,7 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
 	kfree(d->mask_buf);
 	kfree(d->main_status_buf);
 	kfree(d->status_buf);
+	kfree(d->prev_status_buf);
 	kfree(d->status_reg_buf);
 	if (d->config_buf) {
 		for (i = 0; i < chip->num_config_bases; i++)
@@ -984,6 +1018,7 @@ void regmap_del_irq_chip(int irq, struct regmap_irq_chip_data *d)
 	kfree(d->main_status_buf);
 	kfree(d->status_reg_buf);
 	kfree(d->status_buf);
+	kfree(d->prev_status_buf);
 	if (d->config_buf) {
 		for (i = 0; i < d->chip->num_config_bases; i++)
 			kfree(d->config_buf[i]);
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index d17c5ea3d55d..02b83f5499b8 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -1641,6 +1641,8 @@ struct regmap_irq_chip_data;
  * @ack_invert:  Inverted ack register: cleared bits for ack.
  * @clear_ack:  Use this to set 1 and 0 or vice-versa to clear interrupts.
  * @status_invert: Inverted status register: cleared bits are active interrupts.
+ * @status_is_level: Status register is actuall signal level: Xor status
+ *		     register with previous value to get active interrupts.
  * @wake_invert: Inverted wake register: cleared bits are wake enabled.
  * @type_in_mask: Use the mask registers for controlling irq type. Use this if
  *		  the hardware provides separate bits for rising/falling edge
@@ -1704,6 +1706,7 @@ struct regmap_irq_chip {
 	unsigned int ack_invert:1;
 	unsigned int clear_ack:1;
 	unsigned int status_invert:1;
+	unsigned int status_is_level:1;
 	unsigned int wake_invert:1;
 	unsigned int type_in_mask:1;
 	unsigned int clear_on_unmask:1;

-- 
2.39.5

[PATCH v6 07/12] gpio: regmap: Allow to allocate regmap-irq device

[Mathieu Dubois-Briand]

GPIO controller often have support for IRQ: allow to easily allocate
both gpio-regmap and regmap-irq in one operation.

Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
---
 drivers/base/regmap/regmap-irq.c |  5 +++--
 drivers/gpio/gpio-regmap.c       | 21 +++++++++++++++++++--
 include/linux/gpio/regmap.h      | 14 ++++++++++++++
 3 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index b4a72750ff6d..882c6516301b 100644
--- a/drivers/base/regmap/regmap-irq.c
+++ b/drivers/base/regmap/regmap-irq.c
@@ -6,12 +6,12 @@
 //
 // Author: Mark Brown <broonie@opensource.wolfsonmicro.com>
 
-#include <linux/array_size.h>
 #include <linux/device.h>
 #include <linux/export.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
+#include <linux/overflow.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
@@ -910,7 +910,8 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
 		if (ret < 0)
 			goto err_alloc;
 
-		memcpy(d->prev_status_buf, d->status_buf, array_size(d->prev_status_buf));
+		memcpy(d->prev_status_buf, d->status_buf,
+		       array_size(d->chip->num_regs, sizeof(d->prev_status_buf[0])));
 	}
 
 	ret = regmap_irq_create_domain(fwnode, irq_base, chip, d);
diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
index 87c4225784cf..c27cc78f9025 100644
--- a/drivers/gpio/gpio-regmap.c
+++ b/drivers/gpio/gpio-regmap.c
@@ -215,6 +215,7 @@ EXPORT_SYMBOL_GPL(gpio_regmap_get_drvdata);
  */
 struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config)
 {
+	struct irq_domain *irq_domain;
 	struct gpio_regmap *gpio;
 	struct gpio_chip *chip;
 	int ret;
@@ -295,8 +296,24 @@ struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config
 	if (ret < 0)
 		goto err_free_gpio;
 
-	if (config->irq_domain) {
-		ret = gpiochip_irqchip_add_domain(chip, config->irq_domain);
+#ifdef CONFIG_REGMAP_IRQ
+	if (config->regmap_irq_chip) {
+		struct regmap_irq_chip_data *irq_chip_data;
+
+		ret = devm_regmap_add_irq_chip_fwnode(config->parent, dev_fwnode(config->parent),
+						      config->regmap, config->regmap_irq_irqno,
+						      config->regmap_irq_flags, 0,
+						      config->regmap_irq_chip, &irq_chip_data);
+		if (ret)
+			goto err_free_gpio;
+
+		irq_domain = regmap_irq_get_domain(irq_chip_data);
+	} else
+#endif
+	irq_domain = config->irq_domain;
+
+	if (irq_domain) {
+		ret = gpiochip_irqchip_add_domain(chip, irq_domain);
 		if (ret)
 			goto err_remove_gpiochip;
 	}
diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
index c722c67668c6..7ed7927ae2e9 100644
--- a/include/linux/gpio/regmap.h
+++ b/include/linux/gpio/regmap.h
@@ -40,6 +40,14 @@ struct regmap;
  * @drvdata:		(Optional) Pointer to driver specific data which is
  *			not used by gpio-remap but is provided "as is" to the
  *			driver callback(s).
+ * @regmap_irq_chip:	(Optional) Pointer on an regmap_irq_chip structure. If
+ *			set, a regmap-irq device will be created and the IRQ
+ *			domain will be set accordingly.
+ * @regmap_irq_chip_data: (Optional) Pointer on an regmap_irq_chip_data
+ *                      structure pointer. If set, it will be populated with a
+ *                      pointer on allocated regmap_irq data.
+ * @regmap_irq_irqno	(Optional) The IRQ the device uses to signal interrupts.
+ * @regmap_irq_flags	(Optional) The IRQF_ flags to use for the interrupt.
  *
  * The ->reg_mask_xlate translates a given base address and GPIO offset to
  * register and mask pair. The base address is one of the given register
@@ -78,6 +86,12 @@ struct gpio_regmap_config {
 	int ngpio_per_reg;
 	struct irq_domain *irq_domain;
 
+#ifdef CONFIG_REGMAP_IRQ
+	struct regmap_irq_chip *regmap_irq_chip;
+	int regmap_irq_irqno;
+	unsigned long regmap_irq_flags;
+#endif
+
 	int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base,
 			      unsigned int offset, unsigned int *reg,
 			      unsigned int *mask);

-- 
2.39.5

[PATCH v6 08/12] gpio: regmap: Allow to provide init_valid_mask callback

[Mathieu Dubois-Briand]

Allows to populate the gpio_regmap_config structure with
init_valid_mask() callback to set on the final gpio_chip structure.

Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
Reviewed-by: Michael Walle <mwalle@kernel.org>
---
 drivers/gpio/gpio-regmap.c  | 1 +
 include/linux/gpio/regmap.h | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
index c27cc78f9025..e8987707feaf 100644
--- a/drivers/gpio/gpio-regmap.c
+++ b/drivers/gpio/gpio-regmap.c
@@ -256,6 +256,7 @@ struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config
 	chip->names = config->names;
 	chip->label = config->label ?: dev_name(config->parent);
 	chip->can_sleep = regmap_might_sleep(config->regmap);
+	chip->init_valid_mask = config->init_valid_mask;
 
 	chip->request = gpiochip_generic_request;
 	chip->free = gpiochip_generic_free;
diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
index 7ed7927ae2e9..32ca4b4cccf1 100644
--- a/include/linux/gpio/regmap.h
+++ b/include/linux/gpio/regmap.h
@@ -6,6 +6,7 @@
 struct device;
 struct fwnode_handle;
 struct gpio_regmap;
+struct gpio_chip;
 struct irq_domain;
 struct regmap;
 
@@ -40,6 +41,8 @@ struct regmap;
  * @drvdata:		(Optional) Pointer to driver specific data which is
  *			not used by gpio-remap but is provided "as is" to the
  *			driver callback(s).
+ * @init_valid_mask:	(Optional) Routine to initialize @valid_mask, to be used
+ *			if not all GPIOs are valid.
  * @regmap_irq_chip:	(Optional) Pointer on an regmap_irq_chip structure. If
  *			set, a regmap-irq device will be created and the IRQ
  *			domain will be set accordingly.
@@ -96,6 +99,10 @@ struct gpio_regmap_config {
 			      unsigned int offset, unsigned int *reg,
 			      unsigned int *mask);
 
+	int (*init_valid_mask)(struct gpio_chip *gc,
+			       unsigned long *valid_mask,
+			       unsigned int ngpios);
+
 	void *drvdata;
 };
 

-- 
2.39.5

[PATCH v6 09/12] gpio: max7360: Add MAX7360 gpio support

[Mathieu Dubois-Briand]

Add driver for Maxim Integrated MAX7360 GPIO/GPO controller.

Two sets of GPIOs are provided by the device:
- Up to 8 GPIOs, shared with the PWM and rotary encoder functionalities.
  These GPIOs also provide interrupts on input changes.
- Up to 6 GPOs, on unused keypad columns pins.

Co-developed-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
---
 drivers/gpio/Kconfig        |  12 +++
 drivers/gpio/Makefile       |   1 +
 drivers/gpio/gpio-max7360.c | 250 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 263 insertions(+)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index f2c39bbff83a..42fae2ff86de 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1446,6 +1446,18 @@ config GPIO_MADERA
 	help
 	  Support for GPIOs on Cirrus Logic Madera class codecs.
 
+config GPIO_MAX7360
+	tristate "MAX7360 GPIO support"
+	depends on MFD_MAX7360
+	select GPIO_REGMAP
+	select REGMAP_IRQ
+	help
+	  Allows to use MAX7360 I/O Expander PWM lines as GPIO and keypad COL
+	  lines as GPO.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called gpio-max7360.
+
 config GPIO_MAX77620
 	tristate "GPIO support for PMIC MAX77620 and MAX20024"
 	depends on MFD_MAX77620
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index af130882ffee..b34ecc97bab9 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -103,6 +103,7 @@ obj-$(CONFIG_GPIO_MAX7300)		+= gpio-max7300.o
 obj-$(CONFIG_GPIO_MAX7301)		+= gpio-max7301.o
 obj-$(CONFIG_GPIO_MAX730X)		+= gpio-max730x.o
 obj-$(CONFIG_GPIO_MAX732X)		+= gpio-max732x.o
+obj-$(CONFIG_GPIO_MAX7360)		+= gpio-max7360.o
 obj-$(CONFIG_GPIO_MAX77620)		+= gpio-max77620.o
 obj-$(CONFIG_GPIO_MAX77650)		+= gpio-max77650.o
 obj-$(CONFIG_GPIO_MB86S7X)		+= gpio-mb86s7x.o
diff --git a/drivers/gpio/gpio-max7360.c b/drivers/gpio/gpio-max7360.c
new file mode 100644
index 000000000000..7779062b02dd
--- /dev/null
+++ b/drivers/gpio/gpio-max7360.c
@@ -0,0 +1,250 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2025 Bootlin
+ *
+ * Author: Kamel BOUHARA <kamel.bouhara@bootlin.com>
+ * Author: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bitmap.h>
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/gpio/regmap.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/max7360.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#define MAX7360_GPIO_PORT	1
+#define MAX7360_GPIO_COL	2
+
+static int max7360_get_available_gpos(struct device *dev, unsigned int *available_gpios)
+{
+	u32 columns;
+	int ret;
+
+	ret = device_property_read_u32(dev->parent, "keypad,num-columns", &columns);
+	if (ret) {
+		dev_err(dev, "Failed to read columns count\n");
+		return ret;
+	}
+
+	*available_gpios = min(MAX7360_MAX_GPO, MAX7360_MAX_KEY_COLS - columns);
+
+	return 0;
+}
+
+static int max7360_gpo_init_valid_mask(struct gpio_chip *gc,
+				       unsigned long *valid_mask,
+				       unsigned int ngpios)
+{
+	unsigned int available_gpios;
+	int ret;
+
+	ret = max7360_get_available_gpos(gc->parent, &available_gpios);
+	if (ret)
+		return ret;
+
+	bitmap_clear(valid_mask, 0, MAX7360_MAX_KEY_COLS - ngpios);
+
+	return 0;
+}
+
+static int max7360_set_gpos_count(struct device *dev, struct regmap *regmap)
+{
+	/*
+	 * MAX7360 COL0 to COL7 pins can be used either as keypad columns,
+	 * general purpose output or a mix of both.
+	 * By default, all pins are used as keypad, here we update this
+	 * configuration to allow to use some of them as GPIOs.
+	 */
+	unsigned int available_gpios;
+	unsigned int val;
+	int ret;
+
+	ret = max7360_get_available_gpos(dev, &available_gpios);
+	if (ret)
+		return ret;
+
+	/*
+	 * Configure which GPIOs will be used for keypad.
+	 * MAX7360_REG_DEBOUNCE contains configuration both for keypad debounce
+	 * timings and gpos/keypad columns repartition. Only the later is
+	 * modified here.
+	 */
+	val = FIELD_PREP(MAX7360_PORTS, available_gpios);
+	ret = regmap_write_bits(regmap, MAX7360_REG_DEBOUNCE, MAX7360_PORTS, val);
+	if (ret)
+		dev_err(dev, "Failed to write max7360 columns/gpos configuration");
+
+	return ret;
+}
+
+static int max7360_gpio_reg_mask_xlate(struct gpio_regmap *gpio,
+				       unsigned int base, unsigned int offset,
+				       unsigned int *reg, unsigned int *mask)
+{
+	if (base == MAX7360_REG_PWMBASE) {
+		/*
+		 * GPIO output is using PWM duty cycle registers: one register
+		 * per line, with value being either 0 or 255.
+		 */
+		*reg = base + offset;
+		*mask = GENMASK(7, 0);
+	} else {
+		*reg = base;
+		*mask = BIT(offset);
+	}
+
+	return 0;
+}
+
+static const struct regmap_irq max7360_regmap_irqs[MAX7360_MAX_GPIO] = {
+	REGMAP_IRQ_REG(0, 0, BIT(0)),
+	REGMAP_IRQ_REG(1, 0, BIT(1)),
+	REGMAP_IRQ_REG(2, 0, BIT(2)),
+	REGMAP_IRQ_REG(3, 0, BIT(3)),
+	REGMAP_IRQ_REG(4, 0, BIT(4)),
+	REGMAP_IRQ_REG(5, 0, BIT(5)),
+	REGMAP_IRQ_REG(6, 0, BIT(6)),
+	REGMAP_IRQ_REG(7, 0, BIT(7)),
+};
+
+static int max7360_handle_mask_sync(const int index,
+				    const unsigned int mask_buf_def,
+				    const unsigned int mask_buf,
+				    void *const irq_drv_data)
+{
+	struct regmap *regmap = irq_drv_data;
+	int ret;
+
+	for (unsigned int i = 0; i < MAX7360_MAX_GPIO; ++i) {
+		ret = regmap_assign_bits(regmap, MAX7360_REG_PWMCFG(i),
+					 MAX7360_PORT_CFG_INTERRUPT_MASK, mask_buf & BIT(i));
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int max7360_gpio_probe(struct platform_device *pdev)
+{
+	struct regmap_irq_chip *irq_chip;
+	struct gpio_regmap_config gpio_config = { };
+	struct device *dev = &pdev->dev;
+	unsigned long gpio_function;
+	struct regmap *regmap;
+	unsigned int outconf;
+	int ret;
+
+	regmap = dev_get_regmap(dev->parent, NULL);
+	if (!regmap)
+		return dev_err_probe(dev, -ENODEV, "could not get parent regmap\n");
+
+	gpio_function = (uintptr_t)device_get_match_data(dev);
+	if (gpio_function == MAX7360_GPIO_PORT) {
+		if (device_property_read_bool(dev, "interrupt-controller")) {
+			/*
+			 * Port GPIOs with interrupt-controller property: add IRQ
+			 * controller.
+			 */
+			gpio_config.regmap_irq_flags = IRQF_ONESHOT | IRQF_SHARED;
+			gpio_config.regmap_irq_irqno =
+				fwnode_irq_get_byname(dev_fwnode(dev->parent), "inti");
+			if (gpio_config.regmap_irq_irqno < 0)
+				return dev_err_probe(dev, gpio_config.regmap_irq_irqno,
+						     "Failed to get IRQ\n");
+
+			irq_chip = devm_kzalloc(dev, sizeof(*irq_chip), GFP_KERNEL);
+			gpio_config.regmap_irq_chip = irq_chip;
+			if (!irq_chip)
+				return -ENOMEM;
+
+			irq_chip->name = dev_name(dev);
+			irq_chip->status_base = MAX7360_REG_GPIOIN;
+			irq_chip->status_is_level = true;
+			irq_chip->num_regs = 1;
+			irq_chip->num_irqs = MAX7360_MAX_GPIO;
+			irq_chip->irqs = max7360_regmap_irqs;
+			irq_chip->handle_mask_sync = max7360_handle_mask_sync;
+			irq_chip->irq_drv_data = regmap;
+
+			for (unsigned int i = 0; i < MAX7360_MAX_GPIO; i++) {
+				ret = regmap_write_bits(regmap, MAX7360_REG_PWMCFG(i),
+							MAX7360_PORT_CFG_INTERRUPT_EDGES,
+							MAX7360_PORT_CFG_INTERRUPT_EDGES);
+				if (ret)
+					return dev_err_probe(dev, ret,
+							     "Failed to enable interrupts\n");
+			}
+		}
+
+		/*
+		 * Port GPIOs: set output mode configuration (constant-current or not).
+		 * This property is optional.
+		 */
+		outconf = 0;
+		ret = device_property_read_u32(dev, "maxim,constant-current-disable", &outconf);
+		if (!ret) {
+			ret = regmap_write(regmap, MAX7360_REG_GPIOOUTM, outconf);
+			if (ret)
+				return dev_err_probe(dev, ret,
+						     "Failed to set constant-current configuration\n");
+		}
+	}
+
+	/* Add gpio device. */
+	gpio_config.parent = dev;
+	gpio_config.regmap = regmap;
+	if (gpio_function == MAX7360_GPIO_PORT) {
+		gpio_config.ngpio = MAX7360_MAX_GPIO;
+		gpio_config.reg_dat_base = GPIO_REGMAP_ADDR(MAX7360_REG_GPIOIN);
+		gpio_config.reg_set_base = GPIO_REGMAP_ADDR(MAX7360_REG_PWMBASE);
+		gpio_config.reg_dir_out_base = GPIO_REGMAP_ADDR(MAX7360_REG_GPIOCTRL);
+		gpio_config.ngpio_per_reg = MAX7360_MAX_GPIO;
+		gpio_config.reg_mask_xlate = max7360_gpio_reg_mask_xlate;
+	} else {
+		ret = max7360_set_gpos_count(dev, regmap);
+		if (ret)
+			return dev_err_probe(dev, ret, "Failed to set GPOS pin count\n");
+
+		gpio_config.reg_set_base = GPIO_REGMAP_ADDR(MAX7360_REG_PORTS);
+		gpio_config.ngpio = MAX7360_MAX_KEY_COLS;
+		gpio_config.init_valid_mask = max7360_gpo_init_valid_mask;
+	}
+
+	return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &gpio_config));
+}
+
+static const struct of_device_id max7360_gpio_of_match[] = {
+	{
+		.compatible = "maxim,max7360-gpo",
+		.data = (void *)MAX7360_GPIO_COL
+	}, {
+		.compatible = "maxim,max7360-gpio",
+		.data = (void *)MAX7360_GPIO_PORT
+	}, {
+	}
+};
+MODULE_DEVICE_TABLE(of, max7360_gpio_of_match);
+
+static struct platform_driver max7360_gpio_driver = {
+	.driver = {
+		.name	= "max7360-gpio",
+		.of_match_table = max7360_gpio_of_match,
+	},
+	.probe		= max7360_gpio_probe,
+};
+module_platform_driver(max7360_gpio_driver);
+
+MODULE_DESCRIPTION("MAX7360 GPIO driver");
+MODULE_AUTHOR("Kamel BOUHARA <kamel.bouhara@bootlin.com>");
+MODULE_AUTHOR("Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>");
+MODULE_LICENSE("GPL");

-- 
2.39.5

[PATCH v6 10/12] input: keyboard: Add support for MAX7360 keypad

[Mathieu Dubois-Briand]

Add driver for Maxim Integrated MAX7360 keypad controller, providing
support for up to 64 keys, with a matrix of 8 columns and 8 rows.

Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
---
 drivers/input/keyboard/Kconfig          |  12 ++
 drivers/input/keyboard/Makefile         |   1 +
 drivers/input/keyboard/max7360-keypad.c | 299 ++++++++++++++++++++++++++++++++
 3 files changed, 312 insertions(+)

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 721ab69e84ac..93b5cccf6892 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -421,6 +421,18 @@ config KEYBOARD_MAX7359
 	  To compile this driver as a module, choose M here: the
 	  module will be called max7359_keypad.
 
+config KEYBOARD_MAX7360
+	tristate "Maxim MAX7360 Key Switch Controller"
+	select INPUT_MATRIXKMAP
+	depends on I2C
+	depends on MFD_MAX7360
+	help
+	  If you say yes here you get support for the keypad controller on the
+	  Maxim MAX7360 I/O Expander.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called max7360_keypad.
+
 config KEYBOARD_MPR121
 	tristate "Freescale MPR121 Touchkey"
 	depends on I2C
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 1e0721c30709..b49d32d4003d 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_KEYBOARD_LPC32XX)		+= lpc32xx-keys.o
 obj-$(CONFIG_KEYBOARD_MAPLE)		+= maple_keyb.o
 obj-$(CONFIG_KEYBOARD_MATRIX)		+= matrix_keypad.o
 obj-$(CONFIG_KEYBOARD_MAX7359)		+= max7359_keypad.o
+obj-$(CONFIG_KEYBOARD_MAX7360)		+= max7360-keypad.o
 obj-$(CONFIG_KEYBOARD_MPR121)		+= mpr121_touchkey.o
 obj-$(CONFIG_KEYBOARD_MT6779)		+= mt6779-keypad.o
 obj-$(CONFIG_KEYBOARD_MTK_PMIC) 	+= mtk-pmic-keys.o
diff --git a/drivers/input/keyboard/max7360-keypad.c b/drivers/input/keyboard/max7360-keypad.c
new file mode 100644
index 000000000000..d0066636e5c2
--- /dev/null
+++ b/drivers/input/keyboard/max7360-keypad.c
@@ -0,0 +1,299 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2025 Bootlin
+ *
+ * Author: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/dev_printk.h>
+#include <linux/device/devres.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/input/matrix_keypad.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/max7360.h>
+#include <linux/mod_devicetable.h>
+#include <linux/minmax.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/platform_device.h>
+#include <linux/pm_wakeirq.h>
+#include <linux/regmap.h>
+
+struct max7360_keypad {
+	struct input_dev *input;
+	unsigned int rows;
+	unsigned int cols;
+	unsigned int debounce_ms;
+	int irq;
+	struct regmap *regmap;
+	unsigned short keycodes[MAX7360_MAX_KEY_ROWS * MAX7360_MAX_KEY_COLS];
+};
+
+static irqreturn_t max7360_keypad_irq(int irq, void *data)
+{
+	struct max7360_keypad *max7360_keypad = data;
+	unsigned int val;
+	unsigned int row, col;
+	unsigned int release;
+	unsigned int code;
+	int ret;
+
+	do {
+		ret = regmap_read(max7360_keypad->regmap, MAX7360_REG_KEYFIFO, &val);
+		if (ret) {
+			dev_err(&max7360_keypad->input->dev, "Failed to read max7360 FIFO");
+			return IRQ_NONE;
+		}
+
+		/* FIFO overflow: ignore it and get next event. */
+		if (val == MAX7360_FIFO_OVERFLOW)
+			dev_warn(&max7360_keypad->input->dev, "max7360 FIFO overflow");
+	} while (val == MAX7360_FIFO_OVERFLOW);
+
+	if (val == MAX7360_FIFO_EMPTY) {
+		dev_dbg(&max7360_keypad->input->dev, "Got a spurious interrupt");
+
+		return IRQ_NONE;
+	}
+
+	row = FIELD_GET(MAX7360_FIFO_ROW, val);
+	col = FIELD_GET(MAX7360_FIFO_COL, val);
+	release = val & MAX7360_FIFO_RELEASE;
+
+	code = MATRIX_SCAN_CODE(row, col, MAX7360_ROW_SHIFT);
+
+	dev_dbg(&max7360_keypad->input->dev, "key[%d:%d] %s\n", row, col,
+		release ? "release" : "press");
+
+	input_event(max7360_keypad->input, EV_MSC, MSC_SCAN, code);
+	input_report_key(max7360_keypad->input, max7360_keypad->keycodes[code], !release);
+	input_sync(max7360_keypad->input);
+
+	return IRQ_HANDLED;
+}
+
+static int max7360_keypad_open(struct input_dev *pdev)
+{
+	struct max7360_keypad *max7360_keypad = input_get_drvdata(pdev);
+	int ret;
+
+	/* Somebody is using the device: get out of sleep. */
+	ret = regmap_write_bits(max7360_keypad->regmap, MAX7360_REG_CONFIG,
+				MAX7360_CFG_SLEEP, MAX7360_CFG_SLEEP);
+	if (ret)
+		dev_err(&max7360_keypad->input->dev, "Failed to write max7360 configuration\n");
+
+	return ret;
+}
+
+static void max7360_keypad_close(struct input_dev *pdev)
+{
+	struct max7360_keypad *max7360_keypad = input_get_drvdata(pdev);
+	int ret;
+
+	/* Nobody is using the device anymore: go to sleep. */
+	ret = regmap_write_bits(max7360_keypad->regmap, MAX7360_REG_CONFIG, MAX7360_CFG_SLEEP, 0);
+	if (ret)
+		dev_err(&max7360_keypad->input->dev,
+			"Failed to write max7360 configuration\n");
+}
+
+static int max7360_keypad_hw_init(struct max7360_keypad *max7360_keypad)
+{
+	unsigned int val;
+	int ret;
+
+	val = max7360_keypad->debounce_ms - MAX7360_DEBOUNCE_MIN;
+	ret = regmap_write_bits(max7360_keypad->regmap, MAX7360_REG_DEBOUNCE,
+				MAX7360_DEBOUNCE,
+				FIELD_PREP(MAX7360_DEBOUNCE, val));
+	if (ret) {
+		return dev_err_probe(&max7360_keypad->input->dev, ret,
+			"Failed to write max7360 debounce configuration\n");
+	}
+
+	ret = regmap_write_bits(max7360_keypad->regmap, MAX7360_REG_INTERRUPT,
+				MAX7360_INTERRUPT_TIME_MASK,
+				FIELD_PREP(MAX7360_INTERRUPT_TIME_MASK, 1));
+	if (ret) {
+		return dev_err_probe(&max7360_keypad->input->dev, ret,
+			"Failed to write max7360 keypad interrupt configuration\n");
+	}
+
+	return 0;
+}
+
+static int max7360_keypad_build_keymap(struct max7360_keypad *max7360_keypad)
+{
+	struct input_dev *input_dev = max7360_keypad->input;
+	struct device *dev = input_dev->dev.parent->parent;
+	struct matrix_keymap_data keymap_data;
+	const char *propname = "linux,keymap";
+	unsigned int max_keys;
+	int size;
+	int ret;
+
+	size = device_property_count_u32(dev, propname);
+	if (size <= 0) {
+		dev_err(dev, "missing or malformed property %s: %d\n", propname, size);
+		return size < 0 ? size : -EINVAL;
+	}
+
+	max_keys = max7360_keypad->cols * max7360_keypad->rows;
+	if (size > max_keys) {
+		dev_err(dev, "%s size overflow (%d vs max %u)\n", propname, size, max_keys);
+		return -EINVAL;
+	}
+
+	u32 *keys __free(kfree) = kmalloc_array(size, sizeof(*keys), GFP_KERNEL);
+	if (!keys)
+		return -ENOMEM;
+
+	ret = device_property_read_u32_array(dev, propname, keys, size);
+	if (ret) {
+		dev_err(dev, "failed to read %s property: %d\n", propname, ret);
+		return ret;
+	}
+
+	keymap_data.keymap = keys;
+	keymap_data.keymap_size = size;
+	ret = matrix_keypad_build_keymap(&keymap_data, NULL, max7360_keypad->rows, max7360_keypad->cols,
+					 max7360_keypad->keycodes, max7360_keypad->input);
+
+	return 0;
+}
+
+static int max7360_keypad_parse_fw(struct device *dev,
+				   struct max7360_keypad *max7360_keypad,
+				   bool *autorepeat)
+{
+	int ret;
+
+	ret = matrix_keypad_parse_properties(dev->parent, &max7360_keypad->rows,
+					     &max7360_keypad->cols);
+	if (ret)
+		return ret;
+
+	if (!max7360_keypad->rows || !max7360_keypad->cols ||
+	    max7360_keypad->rows > MAX7360_MAX_KEY_ROWS ||
+	    max7360_keypad->cols > MAX7360_MAX_KEY_COLS) {
+		dev_err(dev, "Invalid number of columns or rows (%ux%u)\n",
+			max7360_keypad->cols, max7360_keypad->rows);
+		return -EINVAL;
+	}
+
+	*autorepeat = device_property_read_bool(dev->parent, "autorepeat");
+
+	max7360_keypad->debounce_ms = MAX7360_DEBOUNCE_MIN;
+	ret = device_property_read_u32(dev->parent, "keypad-debounce-delay-ms",
+				       &max7360_keypad->debounce_ms);
+	if (ret == -EINVAL) {
+		dev_info(dev, "Using default keypad-debounce-delay-ms: %u\n",
+			 max7360_keypad->debounce_ms);
+	} else if (ret < 0) {
+		dev_err(dev, "Failed to read keypad-debounce-delay-ms property\n");
+		return ret;
+	}
+
+	if (!in_range(max7360_keypad->debounce_ms, MAX7360_DEBOUNCE_MIN,
+		      MAX7360_DEBOUNCE_MAX - MAX7360_DEBOUNCE_MIN)) {
+		dev_err(dev, "Invalid keypad-debounce-delay-ms: %u, should be between %u and %u.\n",
+			max7360_keypad->debounce_ms, MAX7360_DEBOUNCE_MIN, MAX7360_DEBOUNCE_MAX);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int max7360_keypad_probe(struct platform_device *pdev)
+{
+	struct max7360_keypad *max7360_keypad;
+	struct device *dev = &pdev->dev;
+	struct input_dev *input;
+	struct regmap *regmap;
+	bool autorepeat;
+	int ret;
+	int irq;
+
+	regmap = dev_get_regmap(dev->parent, NULL);
+	if (!regmap)
+		dev_err_probe(dev, -ENODEV, "Could not get parent regmap\n");
+
+	irq = fwnode_irq_get_byname(dev_fwnode(dev->parent), "intk");
+	if (irq < 0)
+		return dev_err_probe(dev, irq, "Failed to get IRQ\n");
+
+	max7360_keypad = devm_kzalloc(dev, sizeof(*max7360_keypad), GFP_KERNEL);
+	if (!max7360_keypad)
+		return -ENOMEM;
+
+	max7360_keypad->regmap = regmap;
+
+	ret = max7360_keypad_parse_fw(dev, max7360_keypad, &autorepeat);
+	if (ret)
+		return ret;
+
+	input = devm_input_allocate_device(dev);
+	if (!input)
+		return -ENOMEM;
+
+	max7360_keypad->input = input;
+
+	input->id.bustype = BUS_I2C;
+	input->name = pdev->name;
+	input->open = max7360_keypad_open;
+	input->close = max7360_keypad_close;
+
+	ret = max7360_keypad_build_keymap(max7360_keypad);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to build keymap\n");
+
+	input_set_capability(input, EV_MSC, MSC_SCAN);
+	if (autorepeat)
+		__set_bit(EV_REP, input->evbit);
+
+	input_set_drvdata(input, max7360_keypad);
+
+	ret = devm_request_threaded_irq(dev, irq, NULL, max7360_keypad_irq,
+					IRQF_ONESHOT,
+					"max7360-keypad", max7360_keypad);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to register interrupt\n");
+
+	ret = input_register_device(input);
+	if (ret)
+		return dev_err_probe(dev, ret, "Could not register input device\n");
+
+	ret = max7360_keypad_hw_init(max7360_keypad);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to initialize max7360 keypad\n");
+
+	device_init_wakeup(dev, true);
+	ret = dev_pm_set_wake_irq(dev, irq);
+	if (ret)
+		dev_warn(dev, "Failed to set up wakeup irq: %d\n", ret);
+
+	return 0;
+}
+
+static void max7360_keypad_remove(struct platform_device *pdev)
+{
+	dev_pm_clear_wake_irq(&pdev->dev);
+}
+
+static struct platform_driver max7360_keypad_driver = {
+	.driver = {
+		.name	= "max7360-keypad",
+	},
+	.probe		= max7360_keypad_probe,
+	.remove		= max7360_keypad_remove,
+};
+module_platform_driver(max7360_keypad_driver);
+
+MODULE_DESCRIPTION("MAX7360 Keypad driver");
+MODULE_AUTHOR("Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>");
+MODULE_LICENSE("GPL");

-- 
2.39.5

[PATCH v6 11/12] input: misc: Add support for MAX7360 rotary

[Mathieu Dubois-Briand]

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          |  10 +++
 drivers/input/misc/Makefile         |   1 +
 drivers/input/misc/max7360-rotary.c | 148 ++++++++++++++++++++++++++++++++++++
 3 files changed, 159 insertions(+)

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index f5496ca0c0d2..0bc9d121c984 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -230,6 +230,16 @@ config INPUT_M68K_BEEP
 	tristate "M68k Beeper support"
 	depends on M68K
 
+config INPUT_MAX7360_ROTARY
+	tristate "Maxim MAX7360 Rotary Encoder"
+	depends on MFD_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..c9630e79730a
--- /dev/null
+++ b/drivers/input/misc/max7360-rotary.c
@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2025 Bootlin
+ *
+ * Author: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/device/devres.h>
+#include <linux/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/platform_device.h>
+#include <linux/pm_wakeirq.h>
+#include <linux/regmap.h>
+#include <linux/types.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;
+	unsigned 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, sign_extend32(val, 7));
+	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;
+}
+
+static int max7360_rotary_probe(struct platform_device *pdev)
+{
+	struct max7360_rotary *max7360_rotary;
+	struct device *dev = &pdev->dev;
+	struct input_dev *input;
+	struct regmap *regmap;
+	int irq;
+	int ret;
+
+	regmap = dev_get_regmap(dev->parent, NULL);
+	if (!regmap)
+		dev_err_probe(dev, -ENODEV, "Could not get parent regmap\n");
+
+	irq = fwnode_irq_get_byname(dev_fwnode(dev->parent), "inti");
+	if (irq < 0)
+		return dev_err_probe(dev, irq, "Failed to get IRQ\n");
+
+	max7360_rotary = devm_kzalloc(dev, sizeof(*max7360_rotary), GFP_KERNEL);
+	if (!max7360_rotary)
+		return -ENOMEM;
+
+	max7360_rotary->regmap = regmap;
+	device_property_read_u32(dev->parent, "linux,axis", &max7360_rotary->axis);
+	device_property_read_u32(dev->parent, "rotary-debounce-delay-ms",
+				 &max7360_rotary->debounce_ms);
+	if (max7360_rotary->debounce_ms > MAX7360_ROT_DEBOUNCE_MAX)
+		return dev_err_probe(dev, -EINVAL, "Invalid debounce timing: %u\n",
+				     max7360_rotary->debounce_ms);
+
+	input = devm_input_allocate_device(dev);
+	if (!input)
+		return -ENOMEM;
+
+	max7360_rotary->input = input;
+
+	input->id.bustype = BUS_I2C;
+	input->name = pdev->name;
+	input->dev.parent = dev;
+
+	input_set_capability(input, EV_REL, max7360_rotary->axis);
+
+	ret = devm_request_threaded_irq(dev, irq, NULL, max7360_rotary_irq,
+					IRQF_ONESHOT | IRQF_SHARED,
+					"max7360-rotary", max7360_rotary);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to register interrupt\n");
+
+	ret = input_register_device(input);
+	if (ret)
+		return dev_err_probe(dev, ret, "Could not register input device\n");
+
+	ret = max7360_rotary_hw_init(max7360_rotary);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to initialize max7360 rotary\n");
+
+	device_init_wakeup(dev, true);
+	ret = dev_pm_set_wake_irq(dev, irq);
+	if (ret)
+		dev_warn(dev, "Failed to set up wakeup irq: %d\n", ret);
+
+	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.5

[PATCH v6 12/12] MAINTAINERS: Add entry on MAX7360 driver

[Mathieu Dubois-Briand]

Add myself as maintainer of Maxim MAX7360 driver and device-tree bindings.

Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
---
 MAINTAINERS | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 96b827049501..0c4988ec7052 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14554,6 +14554,19 @@ L:	linux-iio@vger.kernel.org
 S:	Maintained
 F:	drivers/iio/temperature/max30208.c
 
+MAXIM MAX7360 KEYPAD LED MFD DRIVER
+M:	Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/gpio/maxim,max7360-gpio.yaml
+F:	Documentation/devicetree/bindings/mfd/maxim,max7360.yaml
+F:	drivers/gpio/gpio-max7360.c
+F:	drivers/input/keyboard/max7360-keypad.c
+F:	drivers/input/misc/max7360-rotary.c
+F:	drivers/mfd/max7360.c
+F:	drivers/pinctrl/pinctrl-max7360.c
+F:	drivers/pwm/pwm-max7360.c
+F:	include/linux/mfd/max7360.h
+
 MAXIM MAX77650 PMIC MFD DRIVER
 M:	Bartosz Golaszewski <brgl@bgdev.pl>
 L:	linux-kernel@vger.kernel.org

-- 
2.39.5

Re: [PATCH v6 03/12] pinctrl: Add MAX7360 pinctrl driver

[Mathieu Dubois-Briand]

On Wed Apr 9, 2025 at 4:55 PM CEST, Mathieu Dubois-Briand wrote:
> Add driver for Maxim Integrated MAX7360 pinctrl on the PORT pins. Pins
> can be used either for GPIO, PWM or rotary encoder functionalities.
>
> Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ...
> diff --git a/drivers/pinctrl/pinctrl-max7360.c b/drivers/pinctrl/pinctrl-max7360.c
> ...
> +static int max7360_pinctrl_probe(struct platform_device *pdev)
> +{
> +	struct regmap *regmap;
> +	struct pinctrl_desc *pd;
> +	struct max7360_pinctrl *chip;
> +	struct device *dev = &pdev->dev;
> +
> +	regmap = dev_get_regmap(dev->parent, NULL);
> +	if (!regmap)
> +		dev_err_probe(dev, -ENODEV, "Could not get parent regmap\n");
> +
> +	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
> +	if (!chip)
> +		return -ENOMEM;
> +
> +	pd = &chip->pinctrl_desc;
> +
> +	pd->pctlops = &max7360_pinctrl_ops;
> +	pd->pmxops = &max7360_pmxops;
> +	pd->name = dev_name(dev);
> +	pd->pins = max7360_pins;
> +	pd->npins = MAX7360_MAX_GPIO;
> +	pd->owner = THIS_MODULE;
> +
> +	device_set_of_node_from_dev(dev, dev->parent);

Ok, so this goes a bit against what I said I was going to do on my
previous series, let me explain why. Same reasoning applies for both
uses, in PWM and pinctrl drivers.

With my previous experiments, I came to the conclusion that:
- Either we should use device_set_of_node_from_dev() as I do here.
- Or we should add more subnodes in the device tree binding.
- Also, copying the fwnode with device_set_node() was not possible, as
  the kernel would then try to apply pinctrl on both the parent and
  child device.

I previously said the second solution was probably the way to go, but I
changed my mind for two reasons.

First having more subnodes in the device tree was already rejected in
the past in the reviews of the dt-bindings patch. This do makes sense as
it would be describing device internals (which should not be made in
DT), just to ease one specific software implementation (which should
also be avoided). So I believe this change would again be rejected.
https://lore.kernel.org/lkml/58c80c2a-2532-4bc5-9c9f-52480b3af52a@kernel.org/

But the the second reason is, doing
'git grep "device_set_of_node_from_dev.*parent"', I found several
drivers using device_set_of_node_from_dev() for a similar need. Some of
these uses are also for MFD child devices:
- gpio-adp5585.c / pwm-adp5585.c,
- pwm-ntxec.c,
- max77620-regulator.c / max77620_thermal.c.

So, based on this, I believe using device_set_of_node_from_dev() in
these two drivers is the way to go.

> +	chip->pctldev = devm_pinctrl_register(dev, pd, chip);
> +	if (IS_ERR(chip->pctldev))
> +		return dev_err_probe(dev, PTR_ERR(chip->pctldev), "can't register controller\n");
> +
> +	return 0;
> +}
> +




-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v6 05/12] regmap: irq: Remove unreachable goto

[Mark Brown]

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

On Wed, Apr 09, 2025 at 04:55:52PM +0200, Mathieu Dubois-Briand wrote:
> BUG() never returns, so code after it is unreachable: remove it.

BUG() can be compiled out, CONFIG_BUG.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v6 05/12] regmap: irq: Remove unreachable goto

[Mark Brown]

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

On Wed, Apr 09, 2025 at 04:19:34PM +0100, Mark Brown wrote:
> On Wed, Apr 09, 2025 at 04:55:52PM +0200, Mathieu Dubois-Briand wrote:
> > BUG() never returns, so code after it is unreachable: remove it.
> 
> BUG() can be compiled out, CONFIG_BUG.

Also, please don't mix irrelevant patches into random serieses.  It just
makes everything noisier for everyone.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [External] : [PATCH v6 01/12] dt-bindings: mfd: gpio: Add MAX7360

[ALOK TIWARI]

On 09-04-2025 20:25, Mathieu Dubois-Briand wrote:
> Add device tree bindings for Maxim Integrated MAX7360 device with
> support for keypad, rotary, gpios and pwm functionalities.
> 
> Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> ---
>   .../bindings/gpio/maxim,max7360-gpio.yaml          |  83 ++++++++++
>   .../devicetree/bindings/mfd/maxim,max7360.yaml     | 171 +++++++++++++++++++++
>   2 files changed, 254 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/maxim,max7360-gpio.yaml b/Documentation/devicetree/bindings/gpio/maxim,max7360-gpio.yaml
> new file mode 100644
> index 000000000000..21d603d9504c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/maxim,max7360-gpio.yaml
> @@ -0,0 +1,83 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: https://urldefense.com/v3/__http://devicetree.org/schemas/gpio/maxim,max7360-gpio.yaml*__;Iw!!ACWV5N9M2RV99hQ!LySDuQZdU3DANTEmkRlntMCbFm69zp24O0wAwuujlnN1Zh9-xPEHZu7fj5d_O7vIxUHn9b6gqg9MHtd9ntPvXQvakCad_v0$
> +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!ACWV5N9M2RV99hQ!LySDuQZdU3DANTEmkRlntMCbFm69zp24O0wAwuujlnN1Zh9-xPEHZu7fj5d_O7vIxUHn9b6gqg9MHtd9ntPvXQvacsB3d9k$
> +
> +title: Maxim MAX7360 GPIO controller
> +
> +maintainers:
> +  - Kamel Bouhara <kamel.bouhara@bootlin.com>
> +  - Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> +
> +description: |
> +  Maxim MAX7360 GPIO controller, in MAX7360 chipset
> +  https://urldefense.com/v3/__https://www.analog.com/en/products/max7360.html__;!!ACWV5N9M2RV99hQ!LySDuQZdU3DANTEmkRlntMCbFm69zp24O0wAwuujlnN1Zh9-xPEHZu7fj5d_O7vIxUHn9b6gqg9MHtd9ntPvXQvavZnHZJk$
> +
> +  The device provide two series of GPIOs, referred here as GPIOs and GPOs.
typo: The device provides two series of GPIOs,
> +
> +  PORT0 to PORT7 pins can be used as GPIOs, with support for interrupts and
> +  constant-current mode. These pins will also be used by the torary encoder and
typo: ie rotary encoder ?
> +  PWM functionalities.
> +
> +  COL2 to COL7 pins can be used as GPOs, there is no input capability. COL pins
> +  will be partitionned, with the first pins being affected to the keypad
> +  functionality and the last ones as GPOs.
> +
typo: partitionned -> partitioned
> +properties:
> +  compatible:
> +    enum:
> +      - maxim,max7360-gpio
> +      - maxim,max7360-gpo
> +
> +  gpio-controller: true
> +
> +  "#gpio-cells":
> +    const: 2
> +
> +  interrupt-controller: true
> +
> +  "#interrupt-cells":
> +    const: 2
> +
> +  maxim,constant-current-disable:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Bit field, each bit disables constant-current output of the associated
> +      GPIO, starting from the least significant bit for the first GPIO.
> +    maximum: 0xff
> +
> +required:
> +  - compatible
> +  - gpio-controller
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - maxim,max7360-gpio
> +        ngpios: false
> +    then:
> +      required:
> +        - interrupt-controller
> +    else:
> +      properties:
> +        interrupt-controller: false
> +        maxim,constant-current-disable: false
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    gpio {
> +      compatible = "maxim,max7360-gpio";
> +
> +      gpio-controller;
> +      #gpio-cells = <2>;
> +      maxim,constant-current-disable = <0x06>;
> +
> +      interrupt-controller;
> +      #interrupt-cells = <2>;
> +    };
> diff --git a/Documentation/devicetree/bindings/mfd/maxim,max7360.yaml b/Documentation/devicetree/bindings/mfd/maxim,max7360.yaml
> new file mode 100644
> index 000000000000..ae3969d8dc8a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/maxim,max7360.yaml
> @@ -0,0 +1,171 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: https://urldefense.com/v3/__http://devicetree.org/schemas/mfd/maxim,max7360.yaml*__;Iw!!ACWV5N9M2RV99hQ!LySDuQZdU3DANTEmkRlntMCbFm69zp24O0wAwuujlnN1Zh9-xPEHZu7fj5d_O7vIxUHn9b6gqg9MHtd9ntPvXQva3_poDnI$
> +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!ACWV5N9M2RV99hQ!LySDuQZdU3DANTEmkRlntMCbFm69zp24O0wAwuujlnN1Zh9-xPEHZu7fj5d_O7vIxUHn9b6gqg9MHtd9ntPvXQvacsB3d9k$
> +
> +title: Maxim MAX7360 Keypad, Rotary encoder, PWM and GPIO controller
> +
> +maintainers:
> +  - Kamel Bouhara <kamel.bouhara@bootlin.com>
> +  - Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> +
> +description: |
> +  Maxim MAX7360 device, with following functions:
> +    - keypad controller
> +    - rotary controller
> +    - GPIO and GPO controller
> +    - PWM controller
> +
> +  https://urldefense.com/v3/__https://www.analog.com/en/products/max7360.html__;!!ACWV5N9M2RV99hQ!LySDuQZdU3DANTEmkRlntMCbFm69zp24O0wAwuujlnN1Zh9-xPEHZu7fj5d_O7vIxUHn9b6gqg9MHtd9ntPvXQvavZnHZJk$
> +
> +allOf:
> +  - $ref: /schemas/input/matrix-keymap.yaml#
> +  - $ref: /schemas/input/input.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - maxim,max7360
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 2
> +
> +  interrupt-names:
> +    items:
> +      - const: inti
> +      - const: intk
> +
> +  keypad-debounce-delay-ms:
> +    description: Keypad debounce delay in ms
> +    minimum: 9
> +    maximum: 40
> +    default: 9
> +
> +  rotary-debounce-delay-ms:
> +    description: Rotary encoder debounce delay in ms
> +    minimum: 0
> +    maximum: 15
> +    default: 0
> +
> +  linux,axis:
> +    $ref: /schemas/input/rotary-encoder.yaml#/properties/linux,axis
> +
> +  "#pwm-cells":
> +    const: 3
> +
> +  gpio:
> +    $ref: /schemas/gpio/maxim,max7360-gpio.yaml#
> +    description:
> +      PORT0 to PORT7 general purpose input/output pins configuration.
> +
> +  gpo:
> +    $ref: /schemas/gpio/maxim,max7360-gpio.yaml#
> +    description: >
> +      COL2 to COL7 general purpose output pins configuration. Allows to use
> +      unused keypad columns as outputs.
> +
> +      The MAX7360 has 8 column lines and 6 of them can be used as GPOs. GPIOs
> +      numbers used for this gpio-controller node do correspond to the column
> +      numbers: values 0 and 1 are never valid, values from 2 to 7 might be valid
> +      depending on the value of the keypad,num-column property.
> +
> +patternProperties:
> +  '-pins$':
> +    type: object
> +    description:
> +      Pinctrl node's client devices use subnodes for desired pin configuration.
> +      Client device subnodes use below standard properties.
> +    $ref: /schemas/pinctrl/pincfg-node.yaml
> +
> +    properties:
> +      pins:
> +        description:
> +          List of gpio pins affected by the properties specified in this
> +          subnode.
> +        items:
> +          pattern: '^(PORT[0-7]|ROTARY)$'
> +        minItems: 1
> +        maxItems: 8
> +
> +      function:
> +        description:
> +          Specify the alternative function to be configured for the specified
> +          pins.
> +        enum: [gpio, pwm, rotary]
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - interrupt-names
> +  - linux,keymap
> +  - linux,axis
> +  - "#pwm-cells"
> +  - gpio
> +  - gpo
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/input/input.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      io-expander@38 {
> +        compatible = "maxim,max7360";
> +        reg = <0x38>;
> +
> +        interrupt-parent = <&gpio1>;
> +        interrupts = <23 IRQ_TYPE_LEVEL_LOW>,
> +                     <24 IRQ_TYPE_LEVEL_LOW>;
> +        interrupt-names = "inti", "intk";
> +
> +        keypad,num-rows = <8>;
> +        keypad,num-columns = <4>;
> +        linux,keymap = <
> +          MATRIX_KEY(0x00, 0x00, KEY_F5)
> +          MATRIX_KEY(0x01, 0x00, KEY_F4)
> +          MATRIX_KEY(0x02, 0x01, KEY_F6)
> +          >;
> +        keypad-debounce-delay-ms = <10>;
> +        autorepeat;
> +
> +        rotary-debounce-delay-ms = <2>;
> +        linux,axis = <0>; /* REL_X */
> +
> +        #pwm-cells = <3>;
> +
> +        max7360_gpio: gpio {
> +          compatible = "maxim,max7360-gpio";
> +
> +          gpio-controller;
> +          #gpio-cells = <2>;
> +          maxim,constant-current-disable = <0x06>;
> +
> +          interrupt-controller;
> +          #interrupt-cells = <0x2>;
> +        };
> +
> +        max7360_gpo: gpo {
> +          compatible = "maxim,max7360-gpo";
> +
> +          gpio-controller;
> +          #gpio-cells = <2>;
> +        };
> +
> +        backlight_pins: backlight-pins {
> +          pins = "PORT2";
> +          function = "pwm";
> +        };
> +      };
> +    };
> 

Thanks,
Alok

Re: [PATCH v6 05/12] regmap: irq: Remove unreachable goto

[Andy Shevchenko]

On Wed, Apr 09, 2025 at 04:19:27PM +0100, Mark Brown wrote:
> On Wed, Apr 09, 2025 at 04:55:52PM +0200, Mathieu Dubois-Briand wrote:
> > BUG() never returns, so code after it is unreachable: remove it.
> 
> BUG() can be compiled out, CONFIG_BUG.

Yes, and it's still has unreachable() there. So, this change is correct.
See include/asm-generic/bug.h for the details of the implementation.
And yes, if we have an architecture that does not do this way, it has to
be fixed.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 05/12] regmap: irq: Remove unreachable goto

[Andy Shevchenko]

On Wed, Apr 09, 2025 at 04:55:52PM +0200, Mathieu Dubois-Briand wrote:
> BUG() never returns, so code after it is unreachable: remove it.

Thank you, this is the right change to do.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 05/12] regmap: irq: Remove unreachable goto

[Mark Brown]

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

On Wed, Apr 09, 2025 at 06:38:32PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 09, 2025 at 04:19:27PM +0100, Mark Brown wrote:

> > BUG() can be compiled out, CONFIG_BUG.

> Yes, and it's still has unreachable() there. So, this change is correct.
> See include/asm-generic/bug.h for the details of the implementation.
> And yes, if we have an architecture that does not do this way, it has to
> be fixed.

unreachable() just annotates things, AFAICT it doesn't actually
guarantee to do anything in particular if the annotation turns out to be
incorrect.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 484 bytes --]

Re: [PATCH v6 05/12] regmap: irq: Remove unreachable goto

[Andy Shevchenko]

On Wed, Apr 09, 2025 at 06:38:33PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 09, 2025 at 04:19:27PM +0100, Mark Brown wrote:
> > On Wed, Apr 09, 2025 at 04:55:52PM +0200, Mathieu Dubois-Briand wrote:
> > > BUG() never returns, so code after it is unreachable: remove it.
> > 
> > BUG() can be compiled out, CONFIG_BUG.
> 
> Yes, and it's still has unreachable() there. So, this change is correct.
> See include/asm-generic/bug.h for the details of the implementation.
> And yes, if we have an architecture that does not do this way, it has to
> be fixed.

FWIW, I just have briefly checked the architectures and all of them include
asm-generic/bug.h independently on the configuration option, some of them
even define BUG() despite the configuration option (e.g. arc).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 05/12] regmap: irq: Remove unreachable goto

[Andy Shevchenko]

On Wed, Apr 09, 2025 at 04:46:04PM +0100, Mark Brown wrote:
> On Wed, Apr 09, 2025 at 06:38:32PM +0300, Andy Shevchenko wrote:
> > On Wed, Apr 09, 2025 at 04:19:27PM +0100, Mark Brown wrote:
> 
> > > BUG() can be compiled out, CONFIG_BUG.
> 
> > Yes, and it's still has unreachable() there. So, this change is correct.
> > See include/asm-generic/bug.h for the details of the implementation.
> > And yes, if we have an architecture that does not do this way, it has to
> > be fixed.
> 
> unreachable() just annotates things, AFAICT it doesn't actually
> guarantee to do anything in particular if the annotation turns out to be
> incorrect.

I;m not sure I follow. unreachable is a wrapper on top of
__builtin_unreachable() which is intrinsic of the compiler.

https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005funreachable

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 03/12] pinctrl: Add MAX7360 pinctrl driver

[Andy Shevchenko]

On Wed, Apr 09, 2025 at 05:03:02PM +0200, Mathieu Dubois-Briand wrote:
> On Wed Apr 9, 2025 at 4:55 PM CEST, Mathieu Dubois-Briand wrote:
> > Add driver for Maxim Integrated MAX7360 pinctrl on the PORT pins. Pins
> > can be used either for GPIO, PWM or rotary encoder functionalities.

...

The all the rest of the driver LGTM, but the below.

> > +	device_set_of_node_from_dev(dev, dev->parent);
> 
> Ok, so this goes a bit against what I said I was going to do on my
> previous series, let me explain why. Same reasoning applies for both
> uses, in PWM and pinctrl drivers.
> 
> With my previous experiments, I came to the conclusion that:
> - Either we should use device_set_of_node_from_dev() as I do here.
> - Or we should add more subnodes in the device tree binding.

> - Also, copying the fwnode with device_set_node() was not possible, as
>   the kernel would then try to apply pinctrl on both the parent and
>   child device.

Hmm... I need to refresh my memory with the old discussions. Can you point out
to the problem statement with that approach?

> I previously said the second solution was probably the way to go, but I
> changed my mind for two reasons.
> 
> First having more subnodes in the device tree was already rejected in
> the past in the reviews of the dt-bindings patch. This do makes sense as
> it would be describing device internals (which should not be made in
> DT), just to ease one specific software implementation (which should
> also be avoided). So I believe this change would again be rejected.
> https://lore.kernel.org/lkml/58c80c2a-2532-4bc5-9c9f-52480b3af52a@kernel.org/
> 
> But the the second reason is, doing
> 'git grep "device_set_of_node_from_dev.*parent"', I found several
> drivers using device_set_of_node_from_dev() for a similar need. Some of
> these uses are also for MFD child devices:
> - gpio-adp5585.c / pwm-adp5585.c,
> - pwm-ntxec.c,
> - max77620-regulator.c / max77620_thermal.c.
> 
> So, based on this, I believe using device_set_of_node_from_dev() in
> these two drivers is the way to go.

The problem with this solution is that, It's OF-centric. Which shouldn't be
done in a new code (and I don't see impediments to avoid it). Yes, it does
the right thing for the case, but only on OF systems. Note, fwnode is a list
of maximum of two entries (yeah, designed like that right now), can you utilise
that somehow?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 05/12] regmap: irq: Remove unreachable goto

[Mark Brown]

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

On Wed, Apr 09, 2025 at 07:00:24PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 09, 2025 at 04:46:04PM +0100, Mark Brown wrote:

> > unreachable() just annotates things, AFAICT it doesn't actually
> > guarantee to do anything in particular if the annotation turns out to be
> > incorrect.

> I;m not sure I follow. unreachable is a wrapper on top of
> __builtin_unreachable() which is intrinsic of the compiler.

> https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005funreachable

That just says that the program is undefined if we get to the
__builtin_undefined() and documents some behaviour around warnings.  One
example of undefined behaviour would be doing nothing.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v6 07/12] gpio: regmap: Allow to allocate regmap-irq device

[Andy Shevchenko]

On Wed, Apr 09, 2025 at 04:55:54PM +0200, Mathieu Dubois-Briand wrote:
> GPIO controller often have support for IRQ: allow to easily allocate
> both gpio-regmap and regmap-irq in one operation.

>  
> -		memcpy(d->prev_status_buf, d->status_buf, array_size(d->prev_status_buf));
> +		memcpy(d->prev_status_buf, d->status_buf,
> +		       array_size(d->chip->num_regs, sizeof(d->prev_status_buf[0])));

...

> +#ifdef CONFIG_REGMAP_IRQ
> +	if (config->regmap_irq_chip) {
> +		struct regmap_irq_chip_data *irq_chip_data;
> +
> +		ret = devm_regmap_add_irq_chip_fwnode(config->parent, dev_fwnode(config->parent),
> +						      config->regmap, config->regmap_irq_irqno,
> +						      config->regmap_irq_flags, 0,
> +						      config->regmap_irq_chip, &irq_chip_data);
> +		if (ret)
> +			goto err_free_gpio;
> +
> +		irq_domain = regmap_irq_get_domain(irq_chip_data);
> +	} else
> +#endif
> +	irq_domain = config->irq_domain;

> +

This is blank line is not needed, but I not mind either way.

> +	if (irq_domain) {
> +		ret = gpiochip_irqchip_add_domain(chip, irq_domain);
>  		if (ret)
>  			goto err_remove_gpiochip;
>  	}

...

> + * @regmap_irq_chip:	(Optional) Pointer on an regmap_irq_chip structure. If
> + *			set, a regmap-irq device will be created and the IRQ
> + *			domain will be set accordingly.

> + * @regmap_irq_chip_data: (Optional) Pointer on an regmap_irq_chip_data
> + *                      structure pointer. If set, it will be populated with a
> + *                      pointer on allocated regmap_irq data.

Leftover?

> + * @regmap_irq_irqno	(Optional) The IRQ the device uses to signal interrupts.
> + * @regmap_irq_flags	(Optional) The IRQF_ flags to use for the interrupt.

...

> +#ifdef CONFIG_REGMAP_IRQ
> +	struct regmap_irq_chip *regmap_irq_chip;
> +	int regmap_irq_irqno;

Perhaps call it line?

	int regmap_irq_line;

> +	unsigned long regmap_irq_flags;
> +#endif

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 05/12] regmap: irq: Remove unreachable goto

[Andy Shevchenko]

On Wed, Apr 09, 2025 at 05:32:55PM +0100, Mark Brown wrote:
> On Wed, Apr 09, 2025 at 07:00:24PM +0300, Andy Shevchenko wrote:
> > On Wed, Apr 09, 2025 at 04:46:04PM +0100, Mark Brown wrote:
> 
> > > unreachable() just annotates things, AFAICT it doesn't actually
> > > guarantee to do anything in particular if the annotation turns out to be
> > > incorrect.
> 
> > I;m not sure I follow. unreachable is a wrapper on top of
> > __builtin_unreachable() which is intrinsic of the compiler.
> 
> > https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005funreachable
> 
> That just says that the program is undefined if we get to the
> __builtin_undefined() and documents some behaviour around warnings.  One
> example of undefined behaviour would be doing nothing.

Theoretically yes, practically return after a BUG() makes no sense. Note,
that compiler effectively removes 'goto exit;' here (that's also mentioned
in the documentation independently on the control flow behaviour), so
I don't know what you expect from it.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 04/12] pwm: max7360: Add MAX7360 PWM support

[Andy Shevchenko]

On Wed, Apr 09, 2025 at 04:55:51PM +0200, mathieu.dubois-briand@bootlin.com wrote:
> From: Kamel Bouhara <kamel.bouhara@bootlin.com>
> 
> Add driver for Maxim Integrated MAX7360 PWM controller, supporting up to
> 8 independent PWM outputs.

...

> +static void max7360_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> +	struct regmap *regmap;
> +	struct device *dev;
> +
> +	regmap = pwmchip_get_drvdata(chip);
> +	dev = regmap_get_device(regmap);
> +}

This will produce compiler warnings. Why do you have this at all?

...

> +static int max7360_pwm_round_waveform_tohw(struct pwm_chip *chip,
> +					   struct pwm_device *pwm,
> +					   const struct pwm_waveform *wf,
> +					   void *_wfhw)
> +{
> +	struct max7360_pwm_waveform *wfhw = _wfhw;
> +	u64 duty_steps;
> +
> +	/*
> +	 * Ignore user provided values for period_length_ns and duty_offset_ns:
> +	 * we only support fixed period of MAX7360_PWM_PERIOD_NS and offset of 0.
> +	 */
> +	duty_steps = mul_u64_u64_div_u64(wf->duty_length_ns, MAX7360_PWM_MAX_RES,
> +					 MAX7360_PWM_PERIOD_NS);
> +
> +	wfhw->duty_steps = min(MAX7360_PWM_MAX_RES, duty_steps);

> +	wfhw->enabled = (wf->duty_length_ns != 0);

Can be written as

	wfhw->enabled = wf->duty_length_ns;

Or if you want really to be picky about rules,

	wfhw->enabled = !!wf->duty_length_ns;

> +	return 0;
> +}

...

> +static int max7360_pwm_read_waveform(struct pwm_chip *chip,
> +				     struct pwm_device *pwm,
> +				     void *_wfhw)
> +{
> +	struct max7360_pwm_waveform *wfhw = _wfhw;
> +	struct regmap *regmap;
> +	unsigned int val;
> +	int ret;

> +	regmap = pwmchip_get_drvdata(chip);

Why not move this to the definition block above?

	struct regmap *regmap = pwmchip_get_drvdata(chip);


> +	ret = regmap_read(regmap, MAX7360_REG_GPIOCTRL, &val);
> +	if (ret)
> +		return ret;
> +
> +	if (val & BIT(pwm->hwpwm)) {
> +		wfhw->enabled = true;
> +		ret = regmap_read(regmap, MAX7360_REG_PWM(pwm->hwpwm), &val);

		if (ret)
			return ret;

> +		if (!ret)
> +			wfhw->duty_steps = val;
> +	} else {
> +		wfhw->enabled = false;
> +		wfhw->duty_steps = 0;
> +	}
> +
> +	return ret;

	return 0;

> +}

...

> +	device_set_of_node_from_dev(dev, dev->parent);

This needs broader discussion.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 08/12] gpio: regmap: Allow to provide init_valid_mask callback

[Andy Shevchenko]

On Wed, Apr 09, 2025 at 04:55:55PM +0200, Mathieu Dubois-Briand wrote:
> Allows to populate the gpio_regmap_config structure with
> init_valid_mask() callback to set on the final gpio_chip structure.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 05/12] regmap: irq: Remove unreachable goto

[Dmitry Torokhov]

On Wed, Apr 09, 2025 at 07:45:42PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 09, 2025 at 05:32:55PM +0100, Mark Brown wrote:
> > On Wed, Apr 09, 2025 at 07:00:24PM +0300, Andy Shevchenko wrote:
> > > On Wed, Apr 09, 2025 at 04:46:04PM +0100, Mark Brown wrote:
> > 
> > > > unreachable() just annotates things, AFAICT it doesn't actually
> > > > guarantee to do anything in particular if the annotation turns out to be
> > > > incorrect.
> > 
> > > I;m not sure I follow. unreachable is a wrapper on top of
> > > __builtin_unreachable() which is intrinsic of the compiler.
> > 
> > > https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005funreachable
> > 
> > That just says that the program is undefined if we get to the
> > __builtin_undefined() and documents some behaviour around warnings.  One
> > example of undefined behaviour would be doing nothing.
> 
> Theoretically yes, practically return after a BUG() makes no sense. Note,
> that compiler effectively removes 'goto exit;' here (that's also mentioned
> in the documentation independently on the control flow behaviour), so
> I don't know what you expect from it.

So unreachable() sometimes lears to weird behavior from compiler, for
example as mentioned here where we ended up removing it to prevent
miscompilations:

https://lore.kernel.org/all/20241010222451.GA3571761@thelio-3990X/

Thanks.

-- 
Dmitry

Re: [PATCH v6 10/12] input: keyboard: Add support for MAX7360 keypad

[Dmitry Torokhov]

Hi Mathieu,

On Wed, Apr 09, 2025 at 04:55:57PM +0200, Mathieu Dubois-Briand wrote:
> +struct max7360_keypad {
> +	struct input_dev *input;
> +	unsigned int rows;
> +	unsigned int cols;
> +	unsigned int debounce_ms;
> +	int irq;
> +	struct regmap *regmap;
> +	unsigned short keycodes[MAX7360_MAX_KEY_ROWS * MAX7360_MAX_KEY_COLS];
> +};
> +
> +static irqreturn_t max7360_keypad_irq(int irq, void *data)
> +{
> +	struct max7360_keypad *max7360_keypad = data;
> +	unsigned int val;
> +	unsigned int row, col;
> +	unsigned int release;
> +	unsigned int code;
> +	int ret;

int error;

> +
> +	do {
> +		ret = regmap_read(max7360_keypad->regmap, MAX7360_REG_KEYFIFO, &val);
> +		if (ret) {
> +			dev_err(&max7360_keypad->input->dev, "Failed to read max7360 FIFO");

This will return name pf the input device, whereas logging name of the
platform device (representing the hardware device) would be much more
interesting. You can either use max7360_keypad->input->dev.parent, or,
better yet, add *dev pointer to struct max7360_keypad.

> +			return IRQ_NONE;
> +		}
> +
> +		/* FIFO overflow: ignore it and get next event. */
> +		if (val == MAX7360_FIFO_OVERFLOW)
> +			dev_warn(&max7360_keypad->input->dev, "max7360 FIFO overflow");
> +	} while (val == MAX7360_FIFO_OVERFLOW);
> +
> +	if (val == MAX7360_FIFO_EMPTY) {
> +		dev_dbg(&max7360_keypad->input->dev, "Got a spurious interrupt");
> +
> +		return IRQ_NONE;
> +	}
> +
> +	row = FIELD_GET(MAX7360_FIFO_ROW, val);
> +	col = FIELD_GET(MAX7360_FIFO_COL, val);
> +	release = val & MAX7360_FIFO_RELEASE;
> +
> +	code = MATRIX_SCAN_CODE(row, col, MAX7360_ROW_SHIFT);
> +
> +	dev_dbg(&max7360_keypad->input->dev, "key[%d:%d] %s\n", row, col,
> +		release ? "release" : "press");
> +
> +	input_event(max7360_keypad->input, EV_MSC, MSC_SCAN, code);
> +	input_report_key(max7360_keypad->input, max7360_keypad->keycodes[code], !release);
> +	input_sync(max7360_keypad->input);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int max7360_keypad_open(struct input_dev *pdev)
> +{
> +	struct max7360_keypad *max7360_keypad = input_get_drvdata(pdev);
> +	int ret;

"int error" for variables holding error codes or 0. Also elsewhere in
the driver.

> +
> +	/* Somebody is using the device: get out of sleep. */
> +	ret = regmap_write_bits(max7360_keypad->regmap, MAX7360_REG_CONFIG,
> +				MAX7360_CFG_SLEEP, MAX7360_CFG_SLEEP);
> +	if (ret)
> +		dev_err(&max7360_keypad->input->dev, "Failed to write max7360 configuration\n");

Log error code?

Explicit error return please.
		retrun error;
	}
> +
> +	return ret;

	return 0;

> +}
> +
> +static void max7360_keypad_close(struct input_dev *pdev)
> +{
> +	struct max7360_keypad *max7360_keypad = input_get_drvdata(pdev);
> +	int ret;
> +
> +	/* Nobody is using the device anymore: go to sleep. */
> +	ret = regmap_write_bits(max7360_keypad->regmap, MAX7360_REG_CONFIG, MAX7360_CFG_SLEEP, 0);
> +	if (ret)
> +		dev_err(&max7360_keypad->input->dev,
> +			"Failed to write max7360 configuration\n");

Log error code?

> +}
> +
> +static int max7360_keypad_hw_init(struct max7360_keypad *max7360_keypad)
> +{
> +	unsigned int val;
> +	int ret;
> +
> +	val = max7360_keypad->debounce_ms - MAX7360_DEBOUNCE_MIN;
> +	ret = regmap_write_bits(max7360_keypad->regmap, MAX7360_REG_DEBOUNCE,
> +				MAX7360_DEBOUNCE,
> +				FIELD_PREP(MAX7360_DEBOUNCE, val));
> +	if (ret) {
> +		return dev_err_probe(&max7360_keypad->input->dev, ret,
> +			"Failed to write max7360 debounce configuration\n");
> +	}

No need for braces with single line statements.

> +
> +	ret = regmap_write_bits(max7360_keypad->regmap, MAX7360_REG_INTERRUPT,
> +				MAX7360_INTERRUPT_TIME_MASK,
> +				FIELD_PREP(MAX7360_INTERRUPT_TIME_MASK, 1));
> +	if (ret) {
> +		return dev_err_probe(&max7360_keypad->input->dev, ret,
> +			"Failed to write max7360 keypad interrupt configuration\n");
> +	}
> +
> +	return 0;
> +}
> +
> +static int max7360_keypad_build_keymap(struct max7360_keypad *max7360_keypad)
> +{
> +	struct input_dev *input_dev = max7360_keypad->input;
> +	struct device *dev = input_dev->dev.parent->parent;
> +	struct matrix_keymap_data keymap_data;
> +	const char *propname = "linux,keymap";
> +	unsigned int max_keys;
> +	int size;
> +	int ret;
> +
> +	size = device_property_count_u32(dev, propname);
> +	if (size <= 0) {
> +		dev_err(dev, "missing or malformed property %s: %d\n", propname, size);
> +		return size < 0 ? size : -EINVAL;
> +	}
> +
> +	max_keys = max7360_keypad->cols * max7360_keypad->rows;
> +	if (size > max_keys) {
> +		dev_err(dev, "%s size overflow (%d vs max %u)\n", propname, size, max_keys);
> +		return -EINVAL;
> +	}
> +
> +	u32 *keys __free(kfree) = kmalloc_array(size, sizeof(*keys), GFP_KERNEL);
> +	if (!keys)
> +		return -ENOMEM;
> +
> +	ret = device_property_read_u32_array(dev, propname, keys, size);
> +	if (ret) {
> +		dev_err(dev, "failed to read %s property: %d\n", propname, ret);
> +		return ret;
> +	}
> +
> +	keymap_data.keymap = keys;
> +	keymap_data.keymap_size = size;
> +	ret = matrix_keypad_build_keymap(&keymap_data, NULL, max7360_keypad->rows, max7360_keypad->cols,
> +					 max7360_keypad->keycodes, max7360_keypad->input);

What if it fails? Error handling please.

Also, it looks like you are repeating what matrix_keypad_build_keymap()
is already doing. If you pass NULL as keymap data, won't it do the right
thing?

> +
> +	return 0;
> +}
> +
> +static int max7360_keypad_parse_fw(struct device *dev,
> +				   struct max7360_keypad *max7360_keypad,
> +				   bool *autorepeat)
> +{
> +	int ret;
> +
> +	ret = matrix_keypad_parse_properties(dev->parent, &max7360_keypad->rows,
> +					     &max7360_keypad->cols);
> +	if (ret)
> +		return ret;
> +
> +	if (!max7360_keypad->rows || !max7360_keypad->cols ||
> +	    max7360_keypad->rows > MAX7360_MAX_KEY_ROWS ||
> +	    max7360_keypad->cols > MAX7360_MAX_KEY_COLS) {
> +		dev_err(dev, "Invalid number of columns or rows (%ux%u)\n",
> +			max7360_keypad->cols, max7360_keypad->rows);
> +		return -EINVAL;
> +	}
> +
> +	*autorepeat = device_property_read_bool(dev->parent, "autorepeat");
> +
> +	max7360_keypad->debounce_ms = MAX7360_DEBOUNCE_MIN;
> +	ret = device_property_read_u32(dev->parent, "keypad-debounce-delay-ms",
> +				       &max7360_keypad->debounce_ms);
> +	if (ret == -EINVAL) {
> +		dev_info(dev, "Using default keypad-debounce-delay-ms: %u\n",
> +			 max7360_keypad->debounce_ms);
> +	} else if (ret < 0) {
> +		dev_err(dev, "Failed to read keypad-debounce-delay-ms property\n");
> +		return ret;
> +	}
> +
> +	if (!in_range(max7360_keypad->debounce_ms, MAX7360_DEBOUNCE_MIN,
> +		      MAX7360_DEBOUNCE_MAX - MAX7360_DEBOUNCE_MIN)) {
> +		dev_err(dev, "Invalid keypad-debounce-delay-ms: %u, should be between %u and %u.\n",
> +			max7360_keypad->debounce_ms, MAX7360_DEBOUNCE_MIN, MAX7360_DEBOUNCE_MAX);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int max7360_keypad_probe(struct platform_device *pdev)
> +{
> +	struct max7360_keypad *max7360_keypad;
> +	struct device *dev = &pdev->dev;
> +	struct input_dev *input;
> +	struct regmap *regmap;
> +	bool autorepeat;
> +	int ret;
> +	int irq;
> +
> +	regmap = dev_get_regmap(dev->parent, NULL);
> +	if (!regmap)
> +		dev_err_probe(dev, -ENODEV, "Could not get parent regmap\n");

		return dev_err_probe(...) ?

Thanks.

-- 
Dmitry

Re: [PATCH v6 11/12] input: misc: Add support for MAX7360 rotary

[Dmitry Torokhov]

Hi Mathieu,

On Wed, Apr 09, 2025 at 04:55:58PM +0200, Mathieu Dubois-Briand wrote:
> Add driver for Maxim Integrated MAX7360 rotary encoder controller,
> supporting a single rotary switch.

Largely same comments as for the keypad driver: use "int error" for erro
variable, selection of the device for logging. Also:

> +
> +	input = devm_input_allocate_device(dev);
> +	if (!input)
> +		return -ENOMEM;
> +
> +	max7360_rotary->input = input;
> +
> +	input->id.bustype = BUS_I2C;
> +	input->name = pdev->name;
> +	input->dev.parent = dev;

No need to be setting/overriding this, devm_input_allocate_device()
already sets this up.

> +
> +	input_set_capability(input, EV_REL, max7360_rotary->axis);

The event type should come from the DT data I believe. Could we use at
least parts of the regular rotary encoding bindings?

Thanks.

-- 
Dmitry

Re: [PATCH v6 01/12] dt-bindings: mfd: gpio: Add MAX7360

[Dmitry Torokhov]

Hi Mathieu,

On Wed, Apr 09, 2025 at 04:55:48PM +0200, Mathieu Dubois-Briand wrote:
> Add device tree bindings for Maxim Integrated MAX7360 device with
> support for keypad, rotary, gpios and pwm functionalities.
> 
> +
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      io-expander@38 {
> +        compatible = "maxim,max7360";
> +        reg = <0x38>;
> +
> +        interrupt-parent = <&gpio1>;
> +        interrupts = <23 IRQ_TYPE_LEVEL_LOW>,
> +                     <24 IRQ_TYPE_LEVEL_LOW>;
> +        interrupt-names = "inti", "intk";
> +
> +        keypad,num-rows = <8>;
> +        keypad,num-columns = <4>;
> +        linux,keymap = <
> +          MATRIX_KEY(0x00, 0x00, KEY_F5)
> +          MATRIX_KEY(0x01, 0x00, KEY_F4)
> +          MATRIX_KEY(0x02, 0x01, KEY_F6)
> +          >;
> +        keypad-debounce-delay-ms = <10>;
> +        autorepeat;
> +
> +        rotary-debounce-delay-ms = <2>;
> +        linux,axis = <0>; /* REL_X */

Probably this has been already discussed, but shouldn't keyboard and
rotary encoder be represented as sub-nodes here, similar to how GPIO
block is represented?

Thanks.

-- 
Dmitry

Re: [External] : [PATCH v6 01/12] dt-bindings: mfd: gpio: Add MAX7360

[Lee Jones]

On Wed, 09 Apr 2025, ALOK TIWARI wrote:

> 
> 
> On 09-04-2025 20:25, Mathieu Dubois-Briand wrote:
> > Add device tree bindings for Maxim Integrated MAX7360 device with
> > support for keypad, rotary, gpios and pwm functionalities.
> > 
> > Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> > ---
> >   .../bindings/gpio/maxim,max7360-gpio.yaml          |  83 ++++++++++
> >   .../devicetree/bindings/mfd/maxim,max7360.yaml     | 171 +++++++++++++++++++++
> >   2 files changed, 254 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/gpio/maxim,max7360-gpio.yaml b/Documentation/devicetree/bindings/gpio/maxim,max7360-gpio.yaml
> > new file mode 100644
> > index 000000000000..21d603d9504c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/maxim,max7360-gpio.yaml
> > @@ -0,0 +1,83 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: https://urldefense.com/v3/__http://devicetree.org/schemas/gpio/maxim,max7360-gpio.yaml*__;Iw!!ACWV5N9M2RV99hQ!LySDuQZdU3DANTEmkRlntMCbFm69zp24O0wAwuujlnN1Zh9-xPEHZu7fj5d_O7vIxUHn9b6gqg9MHtd9ntPvXQvakCad_v0$
> > +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!ACWV5N9M2RV99hQ!LySDuQZdU3DANTEmkRlntMCbFm69zp24O0wAwuujlnN1Zh9-xPEHZu7fj5d_O7vIxUHn9b6gqg9MHtd9ntPvXQvacsB3d9k$
> > +
> > +title: Maxim MAX7360 GPIO controller
> > +
> > +maintainers:
> > +  - Kamel Bouhara <kamel.bouhara@bootlin.com>
> > +  - Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> > +
> > +description: |
> > +  Maxim MAX7360 GPIO controller, in MAX7360 chipset
> > +  https://urldefense.com/v3/__https://www.analog.com/en/products/max7360.html__;!!ACWV5N9M2RV99hQ!LySDuQZdU3DANTEmkRlntMCbFm69zp24O0wAwuujlnN1Zh9-xPEHZu7fj5d_O7vIxUHn9b6gqg9MHtd9ntPvXQvavZnHZJk$
> > +
> > +  The device provide two series of GPIOs, referred here as GPIOs and GPOs.
> typo: The device provides two series of GPIOs,
> > +
> > +  PORT0 to PORT7 pins can be used as GPIOs, with support for interrupts and
> > +  constant-current mode. These pins will also be used by the torary encoder and
> typo: ie rotary encoder ?
> > +  PWM functionalities.
> > +
> > +  COL2 to COL7 pins can be used as GPOs, there is no input capability. COL pins
> > +  will be partitionned, with the first pins being affected to the keypad
> > +  functionality and the last ones as GPOs.
> > +
> typo: partitionned -> partitioned

Please trim your responses.

You comments should have blank lines above below your comments too please.

-- 
Lee Jones [李琼斯]

Re: [External] : [PATCH v6 01/12] dt-bindings: mfd: gpio: Add MAX7360

[Mathieu Dubois-Briand]

On Wed Apr 9, 2025 at 5:22 PM CEST, ALOK TIWARI wrote:
>
>
> On 09-04-2025 20:25, Mathieu Dubois-Briand wrote:
>> Add device tree bindings for Maxim Integrated MAX7360 device with
>> support for keypad, rotary, gpios and pwm functionalities.
>> 
>> Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
>> ---
>>   .../bindings/gpio/maxim,max7360-gpio.yaml          |  83 ++++++++++
>>   .../devicetree/bindings/mfd/maxim,max7360.yaml     | 171 +++++++++++++++++++++
>>   2 files changed, 254 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/gpio/maxim,max7360-gpio.yaml b/Documentation/devicetree/bindings/gpio/maxim,max7360-gpio.yaml
>> new file mode 100644
>> index 000000000000..21d603d9504c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpio/maxim,max7360-gpio.yaml
>> @@ -0,0 +1,83 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: https://urldefense.com/v3/__http://devicetree.org/schemas/gpio/maxim,max7360-gpio.yaml*__;Iw!!ACWV5N9M2RV99hQ!LySDuQZdU3DANTEmkRlntMCbFm69zp24O0wAwuujlnN1Zh9-xPEHZu7fj5d_O7vIxUHn9b6gqg9MHtd9ntPvXQvakCad_v0$
>> +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!ACWV5N9M2RV99hQ!LySDuQZdU3DANTEmkRlntMCbFm69zp24O0wAwuujlnN1Zh9-xPEHZu7fj5d_O7vIxUHn9b6gqg9MHtd9ntPvXQvacsB3d9k$
>> +
>> +title: Maxim MAX7360 GPIO controller
>> +
>> +maintainers:
>> +  - Kamel Bouhara <kamel.bouhara@bootlin.com>
>> +  - Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
>> +
>> +description: |
>> +  Maxim MAX7360 GPIO controller, in MAX7360 chipset
>> +  https://urldefense.com/v3/__https://www.analog.com/en/products/max7360.html__;!!ACWV5N9M2RV99hQ!LySDuQZdU3DANTEmkRlntMCbFm69zp24O0wAwuujlnN1Zh9-xPEHZu7fj5d_O7vIxUHn9b6gqg9MHtd9ntPvXQvavZnHZJk$
>> +
>> +  The device provide two series of GPIOs, referred here as GPIOs and GPOs.
> typo: The device provides two series of GPIOs,
>> +
>> +  PORT0 to PORT7 pins can be used as GPIOs, with support for interrupts and
>> +  constant-current mode. These pins will also be used by the torary encoder and
> typo: ie rotary encoder ?
>> +  PWM functionalities.
>> +
>> +  COL2 to COL7 pins can be used as GPOs, there is no input capability. COL pins
>> +  will be partitionned, with the first pins being affected to the keypad
>> +  functionality and the last ones as GPOs.
>> +
> typo: partitionned -> partitioned

Thanks for your review, I fixed all 3 typos.

-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v6 01/12] dt-bindings: mfd: gpio: Add MAX7360

[Mathieu Dubois-Briand]

On Wed Apr 9, 2025 at 10:58 PM CEST, Dmitry Torokhov wrote:
> Hi Mathieu,

Hi Dmitry,

>
> On Wed, Apr 09, 2025 at 04:55:48PM +0200, Mathieu Dubois-Briand wrote:
>> Add device tree bindings for Maxim Integrated MAX7360 device with
>> support for keypad, rotary, gpios and pwm functionalities.
>> 
>> +
>> +    i2c {
>> +      #address-cells = <1>;
>> +      #size-cells = <0>;
>> +
>> +      io-expander@38 {
>> +        compatible = "maxim,max7360";
>> +        reg = <0x38>;
>> +
>> +        interrupt-parent = <&gpio1>;
>> +        interrupts = <23 IRQ_TYPE_LEVEL_LOW>,
>> +                     <24 IRQ_TYPE_LEVEL_LOW>;
>> +        interrupt-names = "inti", "intk";
>> +
>> +        keypad,num-rows = <8>;
>> +        keypad,num-columns = <4>;
>> +        linux,keymap = <
>> +          MATRIX_KEY(0x00, 0x00, KEY_F5)
>> +          MATRIX_KEY(0x01, 0x00, KEY_F4)
>> +          MATRIX_KEY(0x02, 0x01, KEY_F6)
>> +          >;
>> +        keypad-debounce-delay-ms = <10>;
>> +        autorepeat;
>> +
>> +        rotary-debounce-delay-ms = <2>;
>> +        linux,axis = <0>; /* REL_X */
>
> Probably this has been already discussed, but shouldn't keyboard and
> rotary encoder be represented as sub-nodes here, similar to how GPIO
> block is represented?
>
> Thanks.

Yes, this has been discussed on v1 and I was asked to remove most of the
subnodes.

https://lore.kernel.org/lkml/58c80c2a-2532-4bc5-9c9f-52480b3af52a@kernel.org/

Thanks for your review.

-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v6 03/12] pinctrl: Add MAX7360 pinctrl driver

[Mathieu Dubois-Briand]

On Wed Apr 9, 2025 at 6:32 PM CEST, Andy Shevchenko wrote:
> On Wed, Apr 09, 2025 at 05:03:02PM +0200, Mathieu Dubois-Briand wrote:
>> On Wed Apr 9, 2025 at 4:55 PM CEST, Mathieu Dubois-Briand wrote:
>> > Add driver for Maxim Integrated MAX7360 pinctrl on the PORT pins. Pins
>> > can be used either for GPIO, PWM or rotary encoder functionalities.
>
> ...
>
> The all the rest of the driver LGTM, but the below.
>
>> > +	device_set_of_node_from_dev(dev, dev->parent);
>> 
>> Ok, so this goes a bit against what I said I was going to do on my
>> previous series, let me explain why. Same reasoning applies for both
>> uses, in PWM and pinctrl drivers.
>> 
>> With my previous experiments, I came to the conclusion that:
>> - Either we should use device_set_of_node_from_dev() as I do here.
>> - Or we should add more subnodes in the device tree binding.
>
>> - Also, copying the fwnode with device_set_node() was not possible, as
>>   the kernel would then try to apply pinctrl on both the parent and
>>   child device.
>
> Hmm... I need to refresh my memory with the old discussions. Can you point out
> to the problem statement with that approach?
>

I mentioned here briefly in my previous series: https://lore.kernel.org/lkml/D8R4B2PKIWSU.2LWTN50YP7SMX@bootlin.com/

So the issue is, if I copy the parent fwnode using device_set_node(),
the kernel is trying to apply any pinctrl defined on the node with
pinctrl- properties on both the parent and the child node. Of course,
only the first one will succeed, as two devices cannot request the same
pins at the same time.

>> I previously said the second solution was probably the way to go, but I
>> changed my mind for two reasons.
>> 
>> First having more subnodes in the device tree was already rejected in
>> the past in the reviews of the dt-bindings patch. This do makes sense as
>> it would be describing device internals (which should not be made in
>> DT), just to ease one specific software implementation (which should
>> also be avoided). So I believe this change would again be rejected.
>> https://lore.kernel.org/lkml/58c80c2a-2532-4bc5-9c9f-52480b3af52a@kernel.org/
>> 
>> But the the second reason is, doing
>> 'git grep "device_set_of_node_from_dev.*parent"', I found several
>> drivers using device_set_of_node_from_dev() for a similar need. Some of
>> these uses are also for MFD child devices:
>> - gpio-adp5585.c / pwm-adp5585.c,
>> - pwm-ntxec.c,
>> - max77620-regulator.c / max77620_thermal.c.
>> 
>> So, based on this, I believe using device_set_of_node_from_dev() in
>> these two drivers is the way to go.
>
> The problem with this solution is that, It's OF-centric. Which shouldn't be
> done in a new code (and I don't see impediments to avoid it). Yes, it does
> the right thing for the case, but only on OF systems. Note, fwnode is a list
> of maximum of two entries (yeah, designed like that right now), can you utilise
> that somehow?

Looking at MFD code, I believe ACPI MFD child devices already get the
parent fwnode, except if a fwnode exists for them.

https://elixir.bootlin.com/linux/v6.13.7/source/drivers/mfd/mfd-core.c#L90

Thanks for your review.

-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v6 04/12] pwm: max7360: Add MAX7360 PWM support

[Mathieu Dubois-Briand]

On Wed Apr 9, 2025 at 7:00 PM CEST, Andy Shevchenko wrote:
> On Wed, Apr 09, 2025 at 04:55:51PM +0200, mathieu.dubois-briand@bootlin.com wrote:
>> From: Kamel Bouhara <kamel.bouhara@bootlin.com>
>> 
>> Add driver for Maxim Integrated MAX7360 PWM controller, supporting up to
>> 8 independent PWM outputs.
>
> ...
>
>> +static void max7360_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
>> +{
>> +	struct regmap *regmap;
>> +	struct device *dev;
>> +
>> +	regmap = pwmchip_get_drvdata(chip);
>> +	dev = regmap_get_device(regmap);
>> +}
>
> This will produce compiler warnings. Why do you have this at all?

Some leftover of a previous version, having this clearly does not make
any sense. I'm surprised I didn't get any compiler warning about this.

>
> ...
>
>> +	device_set_of_node_from_dev(dev, dev->parent);
>
> This needs broader discussion.

Yes, we can continue our discussion in the pinctrl thread.

Ok with all other comments. Thanks for your review.

-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v6 05/12] regmap: irq: Remove unreachable goto

[Mathieu Dubois-Briand]

On Wed Apr 9, 2025 at 5:21 PM CEST, Mark Brown wrote:
> On Wed, Apr 09, 2025 at 04:19:34PM +0100, Mark Brown wrote:
>> On Wed, Apr 09, 2025 at 04:55:52PM +0200, Mathieu Dubois-Briand wrote:
>> > BUG() never returns, so code after it is unreachable: remove it.
>> 
>> BUG() can be compiled out, CONFIG_BUG.
>
> Also, please don't mix irrelevant patches into random serieses.  It just
> makes everything noisier for everyone.

Hi Mark,

Just to provide the context about why this change is part of this
series: this goto, if left unmodified, would have to replaced by a
return. This is how the topic of dropping it came in the previous
iteration of this series.

Thanks for your review.
Mathieu

-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v6 07/12] gpio: regmap: Allow to allocate regmap-irq device

[Mathieu Dubois-Briand]

On Wed Apr 9, 2025 at 6:39 PM CEST, Andy Shevchenko wrote:
> On Wed, Apr 09, 2025 at 04:55:54PM +0200, Mathieu Dubois-Briand wrote:
>> GPIO controller often have support for IRQ: allow to easily allocate
>> both gpio-regmap and regmap-irq in one operation.
>
>>  
>> -		memcpy(d->prev_status_buf, d->status_buf, array_size(d->prev_status_buf));
>> +		memcpy(d->prev_status_buf, d->status_buf,
>> +		       array_size(d->chip->num_regs, sizeof(d->prev_status_buf[0])));
>
> ...
>
>> +#ifdef CONFIG_REGMAP_IRQ
>> +	if (config->regmap_irq_chip) {
>> +		struct regmap_irq_chip_data *irq_chip_data;
>> +
>> +		ret = devm_regmap_add_irq_chip_fwnode(config->parent, dev_fwnode(config->parent),
>> +						      config->regmap, config->regmap_irq_irqno,
>> +						      config->regmap_irq_flags, 0,
>> +						      config->regmap_irq_chip, &irq_chip_data);
>> +		if (ret)
>> +			goto err_free_gpio;
>> +
>> +		irq_domain = regmap_irq_get_domain(irq_chip_data);
>> +	} else
>> +#endif
>> +	irq_domain = config->irq_domain;
>
>> +
>
> This is blank line is not needed, but I not mind either way.
>

I can remove it, but as the line above is potentially part of the
"else", I have a small preference for keeping it.

>> +	if (irq_domain) {
>> +		ret = gpiochip_irqchip_add_domain(chip, irq_domain);
>>  		if (ret)
>>  			goto err_remove_gpiochip;
>>  	}
>
> ...
>
>> + * @regmap_irq_chip:	(Optional) Pointer on an regmap_irq_chip structure. If
>> + *			set, a regmap-irq device will be created and the IRQ
>> + *			domain will be set accordingly.
>
>> + * @regmap_irq_chip_data: (Optional) Pointer on an regmap_irq_chip_data
>> + *                      structure pointer. If set, it will be populated with a
>> + *                      pointer on allocated regmap_irq data.
>
> Leftover?

Yes, sorry...

>
>> + * @regmap_irq_irqno	(Optional) The IRQ the device uses to signal interrupts.
>> + * @regmap_irq_flags	(Optional) The IRQF_ flags to use for the interrupt.
>
> ...
>
>> +#ifdef CONFIG_REGMAP_IRQ
>> +	struct regmap_irq_chip *regmap_irq_chip;
>> +	int regmap_irq_irqno;
>
> Perhaps call it line?
>
> 	int regmap_irq_line;
>

Makes sense, thanks.

>> +	unsigned long regmap_irq_flags;
>> +#endif

Thanks for your review.
Mathieu

-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v6 08/12] gpio: regmap: Allow to provide init_valid_mask callback

[Mathieu Dubois-Briand]

On Wed Apr 9, 2025 at 7:02 PM CEST, Andy Shevchenko wrote:
> On Wed, Apr 09, 2025 at 04:55:55PM +0200, Mathieu Dubois-Briand wrote:
>> Allows to populate the gpio_regmap_config structure with
>> init_valid_mask() callback to set on the final gpio_chip structure.
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Thanks for the tag!


-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v6 10/12] input: keyboard: Add support for MAX7360 keypad

[Mathieu Dubois-Briand]

On Wed Apr 9, 2025 at 8:40 PM CEST, Dmitry Torokhov wrote:
> Hi Mathieu,
>
> On Wed, Apr 09, 2025 at 04:55:57PM +0200, Mathieu Dubois-Briand wrote:
> ...
>> +static irqreturn_t max7360_keypad_irq(int irq, void *data)
>> +{
>> +	struct max7360_keypad *max7360_keypad = data;
>> +	unsigned int val;
>> +	unsigned int row, col;
>> +	unsigned int release;
>> +	unsigned int code;
>> +	int ret;
>
> int error;
>

Ok using error on all similar cases.

>> +
>> +	do {
>> +		ret = regmap_read(max7360_keypad->regmap, MAX7360_REG_KEYFIFO, &val);
>> +		if (ret) {
>> +			dev_err(&max7360_keypad->input->dev, "Failed to read max7360 FIFO");
>
> This will return name pf the input device, whereas logging name of the
> platform device (representing the hardware device) would be much more
> interesting. You can either use max7360_keypad->input->dev.parent, or,
> better yet, add *dev pointer to struct max7360_keypad.
>

Makes sense, thanks.

> ...
>
>> +static int max7360_keypad_build_keymap(struct max7360_keypad *max7360_keypad)
>> +{
>> +	struct input_dev *input_dev = max7360_keypad->input;
>> +	struct device *dev = input_dev->dev.parent->parent;
>> +	struct matrix_keymap_data keymap_data;
>> +	const char *propname = "linux,keymap";
>> +	unsigned int max_keys;
>> +	int size;
>> +	int ret;
>> +
>> +	size = device_property_count_u32(dev, propname);
>> +	if (size <= 0) {
>> +		dev_err(dev, "missing or malformed property %s: %d\n", propname, size);
>> +		return size < 0 ? size : -EINVAL;
>> +	}
>> +
>> +	max_keys = max7360_keypad->cols * max7360_keypad->rows;
>> +	if (size > max_keys) {
>> +		dev_err(dev, "%s size overflow (%d vs max %u)\n", propname, size, max_keys);
>> +		return -EINVAL;
>> +	}
>> +
>> +	u32 *keys __free(kfree) = kmalloc_array(size, sizeof(*keys), GFP_KERNEL);
>> +	if (!keys)
>> +		return -ENOMEM;
>> +
>> +	ret = device_property_read_u32_array(dev, propname, keys, size);
>> +	if (ret) {
>> +		dev_err(dev, "failed to read %s property: %d\n", propname, ret);
>> +		return ret;
>> +	}
>> +
>> +	keymap_data.keymap = keys;
>> +	keymap_data.keymap_size = size;
>> +	ret = matrix_keypad_build_keymap(&keymap_data, NULL, max7360_keypad->rows, max7360_keypad->cols,
>> +					 max7360_keypad->keycodes, max7360_keypad->input);
>
> What if it fails? Error handling please.

Yes, forgot to return ret just below. Not adding logs as in most cases
matrix_keypad_build_keymap() will already print some logs. OK with that?

>
> Also, it looks like you are repeating what matrix_keypad_build_keymap()
> is already doing. If you pass NULL as keymap data, won't it do the right
> thing?
>

No, because matrix_keypad_parse_keymap() is using input_dev->dev.parent
and this device will not have any associated device tree node. It should
use input_dev->dev.parent->parent to get correct values.

There is a discussion ongoing about using device_set_of_node_from_dev(),
so the MFD child device reuse the node of the parent. But I tried to
avoid using it here, as I was able to come with another solution.
Discussion is in the thread of the pinctrl driver (patch #3).

Another solution would be to modify matrix_keypad_parse_keymap(),
allowing to pass a custom device. But again, I tried to avoid doing this
modification just for my own need.

> ...
>
> Thanks.

Ok with all other comments.

Thanks for your review.
Mathieu

-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v6 11/12] input: misc: Add support for MAX7360 rotary

[Mathieu Dubois-Briand]

On Wed Apr 9, 2025 at 9:17 PM CEST, Dmitry Torokhov wrote:
> Hi Mathieu,
>
> On Wed, Apr 09, 2025 at 04:55:58PM +0200, Mathieu Dubois-Briand wrote:
>> Add driver for Maxim Integrated MAX7360 rotary encoder controller,
>> supporting a single rotary switch.
>
> Largely same comments as for the keypad driver: use "int error" for erro
> variable, selection of the device for logging. Also:
>

OK

>> +
>> +	input = devm_input_allocate_device(dev);
>> +	if (!input)
>> +		return -ENOMEM;
>> +
>> +	max7360_rotary->input = input;
>> +
>> +	input->id.bustype = BUS_I2C;
>> +	input->name = pdev->name;
>> +	input->dev.parent = dev;
>
> No need to be setting/overriding this, devm_input_allocate_device()
> already sets this up.
>

Ok, thanks!

>> +
>> +	input_set_capability(input, EV_REL, max7360_rotary->axis);
>
> The event type should come from the DT data I believe. Could we use at
> least parts of the regular rotary encoding bindings?

Ok, I should be able to add "rotary-encoder,relative-axis" property, as
for rotary_encoder.c.

>
> Thanks.

Thanks for your review.
Mathieu

-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v6 05/12] regmap: irq: Remove unreachable goto

[Andy Shevchenko]

Wed, Apr 09, 2025 at 10:16:40AM -0700, Dmitry Torokhov kirjoitti:
> On Wed, Apr 09, 2025 at 07:45:42PM +0300, Andy Shevchenko wrote:
> > On Wed, Apr 09, 2025 at 05:32:55PM +0100, Mark Brown wrote:
> > > On Wed, Apr 09, 2025 at 07:00:24PM +0300, Andy Shevchenko wrote:
> > > > On Wed, Apr 09, 2025 at 04:46:04PM +0100, Mark Brown wrote:
> > > 
> > > > > unreachable() just annotates things, AFAICT it doesn't actually
> > > > > guarantee to do anything in particular if the annotation turns out to be
> > > > > incorrect.
> > > 
> > > > I;m not sure I follow. unreachable is a wrapper on top of
> > > > __builtin_unreachable() which is intrinsic of the compiler.
> > > 
> > > > https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005funreachable
> > > 
> > > That just says that the program is undefined if we get to the
> > > __builtin_undefined() and documents some behaviour around warnings.  One
> > > example of undefined behaviour would be doing nothing.
> > 
> > Theoretically yes, practically return after a BUG() makes no sense. Note,
> > that compiler effectively removes 'goto exit;' here (that's also mentioned
> > in the documentation independently on the control flow behaviour), so
> > I don't know what you expect from it.
> 
> So unreachable() sometimes lears to weird behavior from compiler, for
> example as mentioned here where we ended up removing it to prevent
> miscompilations:
> 
> https://lore.kernel.org/all/20241010222451.GA3571761@thelio-3990X/

How does it affect the BUG()?

From your link:
"I tested using BUG() in lieu of unreachable() like the second change
 I mentioned above, which resolves the issue cleanly, ..."

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 01/12] dt-bindings: mfd: gpio: Add MAX7360

[Rob Herring (Arm)]

On Wed, 09 Apr 2025 16:55:48 +0200, Mathieu Dubois-Briand wrote:
> Add device tree bindings for Maxim Integrated MAX7360 device with
> support for keypad, rotary, gpios and pwm functionalities.
> 
> Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> ---
>  .../bindings/gpio/maxim,max7360-gpio.yaml          |  83 ++++++++++
>  .../devicetree/bindings/mfd/maxim,max7360.yaml     | 171 +++++++++++++++++++++
>  2 files changed, 254 insertions(+)
> 

With the typos fixed,

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH v6 07/12] gpio: regmap: Allow to allocate regmap-irq device

[Andy Shevchenko]

On Thu, Apr 10, 2025 at 11:03:46AM +0200, Mathieu Dubois-Briand wrote:
> On Wed Apr 9, 2025 at 6:39 PM CEST, Andy Shevchenko wrote:
> > On Wed, Apr 09, 2025 at 04:55:54PM +0200, Mathieu Dubois-Briand wrote:

...

> >> +#ifdef CONFIG_REGMAP_IRQ
> >> +	if (config->regmap_irq_chip) {
> >> +		struct regmap_irq_chip_data *irq_chip_data;
> >> +
> >> +		ret = devm_regmap_add_irq_chip_fwnode(config->parent, dev_fwnode(config->parent),
> >> +						      config->regmap, config->regmap_irq_irqno,
> >> +						      config->regmap_irq_flags, 0,
> >> +						      config->regmap_irq_chip, &irq_chip_data);
> >> +		if (ret)
> >> +			goto err_free_gpio;
> >> +
> >> +		irq_domain = regmap_irq_get_domain(irq_chip_data);
> >> +	} else
> >> +#endif
> >> +	irq_domain = config->irq_domain;
> >
> >> +
> >
> > This is blank line is not needed, but I not mind either way.
> 
> I can remove it, but as the line above is potentially part of the
> "else", I have a small preference for keeping it.

Yes, but it's still coupled with the flow. But okay to leave as is.

> >> +	if (irq_domain) {
> >> +		ret = gpiochip_irqchip_add_domain(chip, irq_domain);
> >>  		if (ret)
> >>  			goto err_remove_gpiochip;
> >>  	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 01/12] dt-bindings: mfd: gpio: Add MAX7360

[Mathieu Dubois-Briand]

On Fri Apr 11, 2025 at 6:05 PM CEST, Rob Herring (Arm) wrote:
>
> On Wed, 09 Apr 2025 16:55:48 +0200, Mathieu Dubois-Briand wrote:
>> Add device tree bindings for Maxim Integrated MAX7360 device with
>> support for keypad, rotary, gpios and pwm functionalities.
>> 
>> Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
>> ---
>>  .../bindings/gpio/maxim,max7360-gpio.yaml          |  83 ++++++++++
>>  .../devicetree/bindings/mfd/maxim,max7360.yaml     | 171 +++++++++++++++++++++
>>  2 files changed, 254 insertions(+)
>> 
>
> With the typos fixed,
>
> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Thanks for the tag!

As a quick note, I believe the bindings will be slightly modified in
next version, to address requests on the rotary encoder driver:

--- c/Documentation/devicetree/bindings/mfd/maxim,max7360.yaml
+++ w/Documentation/devicetree/bindings/mfd/maxim,max7360.yaml
@@ -54,6 +54,17 @@ properties:
   linux,axis:
     $ref: /schemas/input/rotary-encoder.yaml#/properties/linux,axis

+  rotary-encoder,relative-axis:
+    $ref: /schemas/input/rotary-encoder.yaml#/properties/rotary-encoder,relative-axis
+    description:
+      Register a relative axis rather than an absolute one.
+
+  rotary-encoder,steps:
+    $ref: /schemas/input/rotary-encoder.yaml#/properties/rotary-encoder,steps
+
+  rotary-encoder,rollover:
+    $ref: /schemas/input/rotary-encoder.yaml#/properties/rotary-encoder,rollover
+
   "#pwm-cells":
     const: 3


Best regards,
Mathieu

-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v6 08/12] gpio: regmap: Allow to provide init_valid_mask callback

[Linus Walleij]

On Wed, Apr 9, 2025 at 4:56 PM Mathieu Dubois-Briand
<mathieu.dubois-briand@bootlin.com> wrote:

> Allows to populate the gpio_regmap_config structure with
> init_valid_mask() callback to set on the final gpio_chip structure.
>
> Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> Reviewed-by: Michael Walle <mwalle@kernel.org>

Makes perfect sense!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v6 09/12] gpio: max7360: Add MAX7360 gpio support

[Bartosz Golaszewski]

On Wed, Apr 9, 2025 at 4:56 PM Mathieu Dubois-Briand
<mathieu.dubois-briand@bootlin.com> wrote:
>
> Add driver for Maxim Integrated MAX7360 GPIO/GPO controller.
>
> Two sets of GPIOs are provided by the device:
> - Up to 8 GPIOs, shared with the PWM and rotary encoder functionalities.
>   These GPIOs also provide interrupts on input changes.
> - Up to 6 GPOs, on unused keypad columns pins.
>
> Co-developed-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
> Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> ---

Looks good to me.

Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Re: [PATCH v6 09/12] gpio: max7360: Add MAX7360 gpio support

[Andy Shevchenko]

On Wed, Apr 09, 2025 at 04:55:56PM +0200, Mathieu Dubois-Briand wrote:
> Add driver for Maxim Integrated MAX7360 GPIO/GPO controller.
> 
> Two sets of GPIOs are provided by the device:
> - Up to 8 GPIOs, shared with the PWM and rotary encoder functionalities.
>   These GPIOs also provide interrupts on input changes.
> - Up to 6 GPOs, on unused keypad columns pins.

...

> +#include <linux/bitfield.h>
> +#include <linux/bitmap.h>
> +#include <linux/err.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/gpio/regmap.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/max7360.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>

> +#include <linux/slab.h>

I don't think you use this header directly.

...

> +static int max7360_gpio_probe(struct platform_device *pdev)
> +{
> +	struct regmap_irq_chip *irq_chip;
> +	struct gpio_regmap_config gpio_config = { };
> +	struct device *dev = &pdev->dev;
> +	unsigned long gpio_function;
> +	struct regmap *regmap;
> +	unsigned int outconf;
> +	int ret;
> +
> +	regmap = dev_get_regmap(dev->parent, NULL);
> +	if (!regmap)
> +		return dev_err_probe(dev, -ENODEV, "could not get parent regmap\n");

> +	gpio_function = (uintptr_t)device_get_match_data(dev);

Somebody pointed me out the Linus' rant on uintptr_t, so he prefers not to see
this in the entire kernel. He suggested to use (unsigned long), but ideally one
should operate with the info structures instead.

...

> +	if (gpio_function == MAX7360_GPIO_PORT) {
> +		if (device_property_read_bool(dev, "interrupt-controller")) {
> +			/*
> +			 * Port GPIOs with interrupt-controller property: add IRQ
> +			 * controller.
> +			 */
> +			gpio_config.regmap_irq_flags = IRQF_ONESHOT | IRQF_SHARED;
> +			gpio_config.regmap_irq_irqno =
> +				fwnode_irq_get_byname(dev_fwnode(dev->parent), "inti");
> +			if (gpio_config.regmap_irq_irqno < 0)
> +				return dev_err_probe(dev, gpio_config.regmap_irq_irqno,
> +						     "Failed to get IRQ\n");
> +
> +			irq_chip = devm_kzalloc(dev, sizeof(*irq_chip), GFP_KERNEL);
> +			gpio_config.regmap_irq_chip = irq_chip;
> +			if (!irq_chip)
> +				return -ENOMEM;
> +
> +			irq_chip->name = dev_name(dev);
> +			irq_chip->status_base = MAX7360_REG_GPIOIN;
> +			irq_chip->status_is_level = true;
> +			irq_chip->num_regs = 1;
> +			irq_chip->num_irqs = MAX7360_MAX_GPIO;
> +			irq_chip->irqs = max7360_regmap_irqs;
> +			irq_chip->handle_mask_sync = max7360_handle_mask_sync;
> +			irq_chip->irq_drv_data = regmap;
> +
> +			for (unsigned int i = 0; i < MAX7360_MAX_GPIO; i++) {
> +				ret = regmap_write_bits(regmap, MAX7360_REG_PWMCFG(i),
> +							MAX7360_PORT_CFG_INTERRUPT_EDGES,
> +							MAX7360_PORT_CFG_INTERRUPT_EDGES);
> +				if (ret)
> +					return dev_err_probe(dev, ret,
> +							     "Failed to enable interrupts\n");
> +			}
> +		}
> +
> +		/*
> +		 * Port GPIOs: set output mode configuration (constant-current or not).
> +		 * This property is optional.
> +		 */
> +		outconf = 0;
> +		ret = device_property_read_u32(dev, "maxim,constant-current-disable", &outconf);
> +		if (!ret) {
> +			ret = regmap_write(regmap, MAX7360_REG_GPIOOUTM, outconf);
> +			if (ret)
> +				return dev_err_probe(dev, ret,
> +						     "Failed to set constant-current configuration\n");
> +		}

This will look better as if-else:

		ret = device_property_read_u32(dev, "maxim,constant-current-disable", &outconf);
		if (ret) {
			outconf = 0;
		} else {
			ret = regmap_write(regmap, MAX7360_REG_GPIOOUTM, outconf);
			if (ret)
				return dev_err_probe(dev, ret,
						     "Failed to set constant-current configuration\n");
		}

> +	}
> +
> +	/* Add gpio device. */
> +	gpio_config.parent = dev;
> +	gpio_config.regmap = regmap;
> +	if (gpio_function == MAX7360_GPIO_PORT) {
> +		gpio_config.ngpio = MAX7360_MAX_GPIO;
> +		gpio_config.reg_dat_base = GPIO_REGMAP_ADDR(MAX7360_REG_GPIOIN);
> +		gpio_config.reg_set_base = GPIO_REGMAP_ADDR(MAX7360_REG_PWMBASE);
> +		gpio_config.reg_dir_out_base = GPIO_REGMAP_ADDR(MAX7360_REG_GPIOCTRL);
> +		gpio_config.ngpio_per_reg = MAX7360_MAX_GPIO;
> +		gpio_config.reg_mask_xlate = max7360_gpio_reg_mask_xlate;
> +	} else {
> +		ret = max7360_set_gpos_count(dev, regmap);
> +		if (ret)
> +			return dev_err_probe(dev, ret, "Failed to set GPOS pin count\n");
> +
> +		gpio_config.reg_set_base = GPIO_REGMAP_ADDR(MAX7360_REG_PORTS);
> +		gpio_config.ngpio = MAX7360_MAX_KEY_COLS;
> +		gpio_config.init_valid_mask = max7360_gpo_init_valid_mask;
> +	}
> +
> +	return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &gpio_config));
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 02/12] mfd: Add max7360 support

[Andy Shevchenko]

On Wed, Apr 09, 2025 at 04:55:49PM +0200, mathieu.dubois-briand@bootlin.com wrote:
> From: Kamel Bouhara <kamel.bouhara@bootlin.com>
> 
> Add core driver to support MAX7360 i2c chip, multi function device
> with keypad, GPIO, PWM, GPO and rotary encoder submodules.

...

> +static int max7360_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct regmap *regmap;
> +	int ret;
> +
> +	regmap = devm_regmap_init_i2c(client, &max7360_regmap_config);
> +	if (IS_ERR(regmap))
> +		return dev_err_probe(dev, PTR_ERR(regmap), "Failed to initialise regmap\n");

> +	i2c_set_clientdata(client, regmap);

Is it used somehow? In children?

> +	ret = max7360_reset(regmap);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to reset device\n");
> +
> +	/* Get the device out of shutdown mode. */
> +	ret = regmap_write_bits(regmap, MAX7360_REG_GPIOCFG,
> +				MAX7360_GPIO_CFG_GPIO_EN,
> +				MAX7360_GPIO_CFG_GPIO_EN);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to enable GPIO and PWM module\n");
> +
> +	ret = max7360_mask_irqs(regmap);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Could not mask interrupts\n");
> +
> +	ret =  devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
> +				    max7360_cells, ARRAY_SIZE(max7360_cells),
> +				    NULL, 0, NULL);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to register child devices\n");
> +
> +	return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 09/12] gpio: max7360: Add MAX7360 gpio support

[Mathieu Dubois-Briand]

On Thu Apr 17, 2025 at 8:13 PM CEST, Andy Shevchenko wrote:
> On Wed, Apr 09, 2025 at 04:55:56PM +0200, Mathieu Dubois-Briand wrote:
>
>> +#include <linux/slab.h>
>
> I don't think you use this header directly.
>

Right.

> ...
>
>> +static int max7360_gpio_probe(struct platform_device *pdev)
>> +{
>> +	struct regmap_irq_chip *irq_chip;
>> +	struct gpio_regmap_config gpio_config = { };
>> +	struct device *dev = &pdev->dev;
>> +	unsigned long gpio_function;
>> +	struct regmap *regmap;
>> +	unsigned int outconf;
>> +	int ret;
>> +
>> +	regmap = dev_get_regmap(dev->parent, NULL);
>> +	if (!regmap)
>> +		return dev_err_probe(dev, -ENODEV, "could not get parent regmap\n");
>
>> +	gpio_function = (uintptr_t)device_get_match_data(dev);
>
> Somebody pointed me out the Linus' rant on uintptr_t, so he prefers not to see
> this in the entire kernel. He suggested to use (unsigned long), but ideally one
> should operate with the info structures instead.
>

Ok, let's define my own platform data structure, this is not a lot of
work to be honest.

> ...
>
>> +
>> +		/*
>> +		 * Port GPIOs: set output mode configuration (constant-current or not).
>> +		 * This property is optional.
>> +		 */
>> +		outconf = 0;
>> +		ret = device_property_read_u32(dev, "maxim,constant-current-disable", &outconf);
>> +		if (!ret) {
>> +			ret = regmap_write(regmap, MAX7360_REG_GPIOOUTM, outconf);
>> +			if (ret)
>> +				return dev_err_probe(dev, ret,
>> +						     "Failed to set constant-current configuration\n");
>> +		}
>
> This will look better as if-else:
>
> 		ret = device_property_read_u32(dev, "maxim,constant-current-disable", &outconf);
> 		if (ret) {
> 			outconf = 0;
> 		} else {
> 			ret = regmap_write(regmap, MAX7360_REG_GPIOOUTM, outconf);
> 			if (ret)
> 				return dev_err_probe(dev, ret,
> 						     "Failed to set constant-current configuration\n");
> 		}

Yes, actually there is no need to set outconf if the property was not
specified.


Thanks for your review.

-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v6 02/12] mfd: Add max7360 support

[Mathieu Dubois-Briand]

On Thu Apr 17, 2025 at 8:16 PM CEST, Andy Shevchenko wrote:
> On Wed, Apr 09, 2025 at 04:55:49PM +0200, mathieu.dubois-briand@bootlin.com wrote:
>> From: Kamel Bouhara <kamel.bouhara@bootlin.com>
>> 
>> Add core driver to support MAX7360 i2c chip, multi function device
>> with keypad, GPIO, PWM, GPO and rotary encoder submodules.
>
> ...
>
>> +static int max7360_probe(struct i2c_client *client)
>> +{
>> +	struct device *dev = &client->dev;
>> +	struct regmap *regmap;
>> +	int ret;
>> +
>> +	regmap = devm_regmap_init_i2c(client, &max7360_regmap_config);
>> +	if (IS_ERR(regmap))
>> +		return dev_err_probe(dev, PTR_ERR(regmap), "Failed to initialise regmap\n");
>
>> +	i2c_set_clientdata(client, regmap);
>
> Is it used somehow? In children?
>

I cannot see where it would be used in the code, and removing it has no
effect. I will remove it.

Thanks for your review.
Mathieu

-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v6 02/12] mfd: Add max7360 support

[Christophe JAILLET]

Le 09/04/2025 à 16:55, mathieu.dubois-briand@bootlin.com a écrit :
> From: Kamel Bouhara <kamel.bouhara@bootlin.com>
> 
> Add core driver to support MAX7360 i2c chip, multi function device
> with keypad, GPIO, PWM, GPO and rotary encoder submodules.
> 
> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
> Co-developed-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> ---

...

> +	ret = max7360_mask_irqs(regmap);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Could not mask interrupts\n");
> +
> +	ret =  devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,

Nitpick: 2 spaces after =

> +				    max7360_cells, ARRAY_SIZE(max7360_cells),
> +				    NULL, 0, NULL);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to register child devices\n");
> +
> +	return 0;
> +}
> +

Re: [PATCH v6 02/12] mfd: Add max7360 support

[Mathieu Dubois-Briand]

On Fri Apr 18, 2025 at 5:52 PM CEST, Christophe JAILLET wrote:
> Le 09/04/2025 à 16:55, mathieu.dubois-briand@bootlin.com a écrit :
>> From: Kamel Bouhara <kamel.bouhara@bootlin.com>
>> 
>> Add core driver to support MAX7360 i2c chip, multi function device
>> with keypad, GPIO, PWM, GPO and rotary encoder submodules.
>> 
>> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
>> Co-developed-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
>> Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
>> ---
>
> ...
>
>> +	ret = max7360_mask_irqs(regmap);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "Could not mask interrupts\n");
>> +
>> +	ret =  devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
>
> Nitpick: 2 spaces after =
>

Fixed, thanks!

-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v6 05/12] regmap: irq: Remove unreachable goto

[Uwe Kleine-König]

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

On Wed, Apr 09, 2025 at 04:19:27PM +0100, Mark Brown wrote:
> On Wed, Apr 09, 2025 at 04:55:52PM +0200, Mathieu Dubois-Briand wrote:
> > BUG() never returns, so code after it is unreachable: remove it.
> 
> BUG() can be compiled out, CONFIG_BUG.

In that case BUG is defined as:

	#define BUG() do {              \
		do {} while (1);        \
		unreachable();          \
	} while (0)

so the return can be dropped as suggested in the patch.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v6 05/12] regmap: irq: Remove unreachable goto

[Mathieu Dubois-Briand]

On Wed Apr 9, 2025 at 5:19 PM CEST, Mark Brown wrote:
> On Wed, Apr 09, 2025 at 04:55:52PM +0200, Mathieu Dubois-Briand wrote:
>> BUG() never returns, so code after it is unreachable: remove it.
>
> BUG() can be compiled out, CONFIG_BUG.

Hi Mark,

Thanks for your review.

As there has been a bit of discussion on this patch, may I ask you for
your current opinion about this change?

Should I drop it from my next series?

Best regards,
Mathieu

-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v6 05/12] regmap: irq: Remove unreachable goto

[Mark Brown]

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

On Wed, Apr 23, 2025 at 02:41:47PM +0200, Mathieu Dubois-Briand wrote:

> As there has been a bit of discussion on this patch, may I ask you for
> your current opinion about this change?

> Should I drop it from my next series?

Please.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]