* [PATCH 0/8] Add support for MAX7360 multifunction device
@ 2024-12-19 16:21 Mathieu Dubois-Briand
2024-12-19 16:21 ` [PATCH 1/8] dt-bindings: Add MAX7360 MFD device Mathieu Dubois-Briand
` (7 more replies)
0 siblings, 8 replies; 25+ messages in thread
From: Mathieu Dubois-Briand @ 2024-12-19 16:21 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Kamel Bouhara, Linus Walleij, Bartosz Golaszewski,
Dmitry Torokhov, Uwe Kleine-König
Cc: devicetree, linux-kernel, linux-gpio, linux-input, linux-pwm,
Grégory Clement, Thomas Petazzoni, 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>
---
Kamel Bouhara (2):
mfd: Add max7360 support
pwm: max7360: Add MAX7360 PWM support
Mathieu Dubois-Briand (6):
dt-bindings: Add MAX7360 MFD device
dt-bindings: Add MAX7360 subdevices
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
.../devicetree/bindings/gpio/max7360-gpio.yaml | 96 +++++
.../devicetree/bindings/input/max7360-keypad.yaml | 67 +++
.../devicetree/bindings/input/max7360-rotary.yaml | 52 +++
Documentation/devicetree/bindings/mfd/max7360.yaml | 56 +++
.../devicetree/bindings/pwm/max7360-pwm.yaml | 35 ++
MAINTAINERS | 12 +
drivers/gpio/Kconfig | 11 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-max7360.c | 453 +++++++++++++++++++++
drivers/input/keyboard/Kconfig | 12 +
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/max7360-keypad.c | 297 ++++++++++++++
drivers/input/misc/Kconfig | 11 +
drivers/input/misc/Makefile | 1 +
drivers/input/misc/max7360-rotary.c | 194 +++++++++
drivers/mfd/Kconfig | 12 +
drivers/mfd/Makefile | 1 +
drivers/mfd/max7360.c | 302 ++++++++++++++
drivers/pwm/Kconfig | 11 +
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-max7360.c | 173 ++++++++
include/linux/mfd/max7360.h | 103 +++++
22 files changed, 1902 insertions(+)
---
base-commit: 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8
change-id: 20241219-mdb-max7360-support-223a8ce45ba3
Best regards,
--
Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/8] dt-bindings: Add MAX7360 MFD device
2024-12-19 16:21 [PATCH 0/8] Add support for MAX7360 multifunction device Mathieu Dubois-Briand
@ 2024-12-19 16:21 ` Mathieu Dubois-Briand
2024-12-21 20:28 ` Krzysztof Kozlowski
2024-12-19 16:21 ` [PATCH 2/8] dt-bindings: Add MAX7360 subdevices Mathieu Dubois-Briand
` (6 subsequent siblings)
7 siblings, 1 reply; 25+ messages in thread
From: Mathieu Dubois-Briand @ 2024-12-19 16:21 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Kamel Bouhara, Linus Walleij, Bartosz Golaszewski,
Dmitry Torokhov, Uwe Kleine-König
Cc: devicetree, linux-kernel, linux-gpio, linux-input, linux-pwm,
Grégory Clement, Thomas Petazzoni, Mathieu Dubois-Briand
Add device tree bindings for Maxim Integrated MAX7360 MFD device with
support for keypad, rotary, gpios and pwm functionalities.
Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
---
Documentation/devicetree/bindings/mfd/max7360.yaml | 56 ++++++++++++++++++++++
1 file changed, 56 insertions(+)
diff --git a/Documentation/devicetree/bindings/mfd/max7360.yaml b/Documentation/devicetree/bindings/mfd/max7360.yaml
new file mode 100644
index 000000000000..49dd437fd313
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/max7360.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/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 MFD device, with following functions:
+ - keypad controller
+ - rotary controller
+ - GPIO and GPO controller
+ - PWM controller
+
+ https://www.analog.com/en/products/max7360.html
+
+properties:
+ compatible:
+ enum:
+ - maxim,max7360
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ description: The interrupt line the device is connected to.
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ max7360@38 {
+ compatible = "maxim,max7360";
+ reg = <0x38>;
+
+ interrupt-parent = <&gpio1>;
+ interrupts = <23 IRQ_TYPE_LEVEL_LOW>;
+ };
+ };
--
2.39.5
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 2/8] dt-bindings: Add MAX7360 subdevices
2024-12-19 16:21 [PATCH 0/8] Add support for MAX7360 multifunction device Mathieu Dubois-Briand
2024-12-19 16:21 ` [PATCH 1/8] dt-bindings: Add MAX7360 MFD device Mathieu Dubois-Briand
@ 2024-12-19 16:21 ` Mathieu Dubois-Briand
2024-12-19 21:54 ` Uwe Kleine-König
2024-12-21 20:34 ` Krzysztof Kozlowski
2024-12-19 16:21 ` [PATCH 3/8] mfd: Add max7360 support mathieu.dubois-briand
` (5 subsequent siblings)
7 siblings, 2 replies; 25+ messages in thread
From: Mathieu Dubois-Briand @ 2024-12-19 16:21 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Kamel Bouhara, Linus Walleij, Bartosz Golaszewski,
Dmitry Torokhov, Uwe Kleine-König
Cc: devicetree, linux-kernel, linux-gpio, linux-input, linux-pwm,
Grégory Clement, Thomas Petazzoni, Mathieu Dubois-Briand
Add device tree bindings for Maxim Integrated MAX7360 MFD functions:
keypad, rotary, gpios and pwm.
Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
---
.../devicetree/bindings/gpio/max7360-gpio.yaml | 96 ++++++++++++++++++++++
.../devicetree/bindings/input/max7360-keypad.yaml | 67 +++++++++++++++
.../devicetree/bindings/input/max7360-rotary.yaml | 52 ++++++++++++
.../devicetree/bindings/pwm/max7360-pwm.yaml | 35 ++++++++
4 files changed, 250 insertions(+)
diff --git a/Documentation/devicetree/bindings/gpio/max7360-gpio.yaml b/Documentation/devicetree/bindings/gpio/max7360-gpio.yaml
new file mode 100644
index 000000000000..3c006dc0380b
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/max7360-gpio.yaml
@@ -0,0 +1,96 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/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 MFD
+ https://www.analog.com/en/products/max7360.html
+
+properties:
+ compatible:
+ enum:
+ - maxim,max7360-gpio
+ - maxim,max7360-gpo
+
+ gpio-controller: true
+
+ "#gpio-cells":
+ const: 2
+
+ ngpios:
+ minimum: 0
+ maximum: 8
+
+ interrupt-controller: true
+
+ "#interrupt-cells":
+ const: 2
+
+ interrupts:
+ description: The interrupt line the device is connected to.
+ maxItems: 1
+
+ constant-current-disable:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: Bit field, each bit disables constant-current output of the
+ associated GPIO.
+
+
+required:
+ - compatible
+ - gpio-controller
+ - ngpios
+
+if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - maxim,max7360-gpio
+then:
+ required:
+ - interrupt-controller
+ - interrupts
+else:
+ properties:
+ interrupt-controller: false
+ interrupts: false
+ constant-current-disable: false
+
+ ngpios:
+ maximum: 6
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ max7360_gpo: max7360_gpo {
+ compatible = "maxim,max7360-gpo";
+ gpio-controller;
+ #gpio-cells = <0x2>;
+ ngpios = <4>;
+ };
+
+ max7360_gpio: max7360_gpio {
+ compatible = "maxim,max7360-gpio";
+
+ gpio-controller;
+ #gpio-cells = <0x2>;
+ ngpios = <8>;
+ constant-current-disable = <0x06>;
+
+ interrupt-controller;
+ #interrupt-cells = <0x2>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <23 IRQ_TYPE_LEVEL_LOW>;
+ };
diff --git a/Documentation/devicetree/bindings/input/max7360-keypad.yaml b/Documentation/devicetree/bindings/input/max7360-keypad.yaml
new file mode 100644
index 000000000000..8bc3c841465b
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/max7360-keypad.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/max7360-keypad.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim MAX7360 Keypad Controller
+
+maintainers:
+ - Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
+
+description: |
+ Maxim MAX7360 Keypad Controller, in MAX7360 MFD
+ https://www.analog.com/en/products/max7360.html
+
+allOf:
+ - $ref: matrix-keymap.yaml#
+ - $ref: input.yaml#
+
+properties:
+ compatible:
+ enum:
+ - maxim,max7360-keypad
+
+ interrupts:
+ description: The interrupt line the device is connected to.
+ maxItems: 1
+
+ debounce-delay-ms:
+ description: Debounce delay in ms
+ minimum: 9
+ maximum: 40
+ default: 9
+
+ linux,input-no-autorepeat:
+ description: If present, the keys will not autorepeat when pressed
+
+required:
+ - compatible
+ - interrupts
+ - linux,keymap
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/input/input.h>
+
+ max7360_keypad {
+ compatible = "maxim,max7360-keypad";
+
+ 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)
+ >;
+
+ interrupt-parent = <&gpio1>;
+ interrupts = <23 IRQ_TYPE_LEVEL_LOW>;
+
+ debounce-delay-ms = <10>;
+ linux,input-no-autorepeat;
+ };
diff --git a/Documentation/devicetree/bindings/input/max7360-rotary.yaml b/Documentation/devicetree/bindings/input/max7360-rotary.yaml
new file mode 100644
index 000000000000..19afa8344249
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/max7360-rotary.yaml
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/max7360-rotary.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim MAX7360 Rotary Encoder
+
+maintainers:
+ - Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
+
+description: |
+ Maxim MAX7360 Rotary Encoder, in MAX7360 MFD
+ https://www.analog.com/en/products/max7360.html
+
+properties:
+ compatible:
+ enum:
+ - maxim,max7360-rotary
+
+ interrupts:
+ description: The interrupt line the device is connected to.
+ maxItems: 1
+
+ debounce-delay-ms:
+ description: Debounce delay in ms
+ minimum: 0
+ maximum: 15
+ default: 0
+
+ linux,axis:
+ description: The input subsystem axis to map to this rotary encoder.
+
+required:
+ - compatible
+ - interrupts
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ max7360_rotary: max7360_rotary {
+ compatible = "maxim,max7360-rotary";
+
+ debounce-delay-ms = <2>;
+ linux,axis = <0>; /* REL_X */
+
+ interrupt-parent = <&gpio1>;
+ interrupts = <23 IRQ_TYPE_LEVEL_LOW>;
+ };
diff --git a/Documentation/devicetree/bindings/pwm/max7360-pwm.yaml b/Documentation/devicetree/bindings/pwm/max7360-pwm.yaml
new file mode 100644
index 000000000000..68d48969e542
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/max7360-pwm.yaml
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/max7360-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim MAX7360 PWM controller
+
+maintainers:
+ - Kamel Bouhara <kamel.bouhara@bootlin.com>
+
+description: |
+ Maxim MAX7360 PWM controller, in MAX7360 MFD
+ https://www.analog.com/en/products/max7360.html
+
+properties:
+ compatible:
+ enum:
+ - maxim,max7360-pwm
+
+ "#pwm-cells":
+ const: 2
+
+
+required:
+ - compatible
+
+additionalProperties: false
+
+examples:
+ - |
+ max7360_pwm: max7360_pwm {
+ compatible = "maxim,max7360-pwm";
+ #pwm-cells = <2>;
+ };
--
2.39.5
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 3/8] mfd: Add max7360 support
2024-12-19 16:21 [PATCH 0/8] Add support for MAX7360 multifunction device Mathieu Dubois-Briand
2024-12-19 16:21 ` [PATCH 1/8] dt-bindings: Add MAX7360 MFD device Mathieu Dubois-Briand
2024-12-19 16:21 ` [PATCH 2/8] dt-bindings: Add MAX7360 subdevices Mathieu Dubois-Briand
@ 2024-12-19 16:21 ` mathieu.dubois-briand
2024-12-20 23:01 ` kernel test robot
2024-12-19 16:21 ` [PATCH 4/8] pwm: max7360: Add MAX7360 PWM support mathieu.dubois-briand
` (4 subsequent siblings)
7 siblings, 1 reply; 25+ messages in thread
From: mathieu.dubois-briand @ 2024-12-19 16:21 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Kamel Bouhara, Linus Walleij, Bartosz Golaszewski,
Dmitry Torokhov, Uwe Kleine-König
Cc: devicetree, linux-kernel, linux-gpio, linux-input, linux-pwm,
Grégory Clement, Thomas Petazzoni, 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 | 12 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/max7360.c | 302 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/mfd/max7360.h | 103 +++++++++++++++
4 files changed, 418 insertions(+)
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index ae23b317a64e..c1ddda480922 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2414,5 +2414,17 @@ config MFD_RSMU_SPI
Additional drivers must be enabled in order to use the functionality
of the device.
+config MFD_MAX7360
+ tristate "Maxim MAX7360 Support"
+ depends on I2C
+ select MFD_CORE
+ select REGMAP_I2C
+ select REGMAP_IRQ
+ help
+ Say yes here to add support for Maxim MAX7360.
+ 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 e057d6d6faef..6cd55504106d 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -163,6 +163,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..c53b42f00a9d
--- /dev/null
+++ b/drivers/mfd/max7360.c
@@ -0,0 +1,302 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Maxim MAX7360 Core Driver
+ *
+ * Copyright (C) 2024 Kamel Bouhara
+ * Author: Kamel Bouhara <kamel.bouhara@bootlin.com>
+ */
+
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/max7360.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+
+static DEFINE_SPINLOCK(request_lock);
+
+struct max7360_mfd {
+ struct regmap *regmap;
+ unsigned int requested_ports;
+ struct device *dev;
+};
+
+#define GPO_COMPATIBLE "maxim,max7360-gpo"
+#define KEYPAD_COMPATIBLE "maxim,max7360-keypad"
+
+static const struct mfd_cell max7360_cells[] = {
+ {
+ .name = "max7360-pwm",
+ .of_compatible = "maxim,max7360-pwm",
+ },
+ {
+ .name = "max7360-gpo",
+ .of_compatible = GPO_COMPATIBLE,
+ },
+ {
+ .name = "max7360-gpio",
+ .of_compatible = "maxim,max7360-gpio",
+ },
+ {
+ .name = "max7360-keypad",
+ .of_compatible = KEYPAD_COMPATIBLE,
+ },
+ {
+ .name = "max7360-rotary",
+ .of_compatible = "maxim,max7360-rotary",
+ },
+};
+
+const struct regmap_range max7360_volatile_ranges[] = {
+ {
+ .range_min = MAX7360_REG_KEYFIFO,
+ .range_max = MAX7360_REG_KEYFIFO,
+ }, {
+ .range_min = 0x48,
+ .range_max = 0x4a,
+ },
+};
+
+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 = 0xff,
+ .volatile_table = &max7360_volatile_table,
+ .cache_type = REGCACHE_RBTREE,
+};
+
+static int max7360_set_gpos_count(struct max7360_mfd *max7360_mfd)
+{
+ /*
+ * Max7360 COL0 to COL7 pins can be used either as keypad columns,
+ * general purpose output or a mix of both.
+ * Get the number of pins requested by the corresponding drivers, ensure
+ * they are compatible with each others and apply the corresponding
+ * configuration.
+ */
+ struct device_node *np;
+ u32 gpos = 0;
+ u32 columns = 0;
+ unsigned int val;
+ int ret;
+
+ np = of_get_compatible_child(max7360_mfd->dev->of_node, GPO_COMPATIBLE);
+ if (np) {
+ ret = of_property_read_u32(np, "ngpios", &gpos);
+ if (ret < 0) {
+ dev_err(max7360_mfd->dev, "Failed to read gpos count\n");
+ return ret;
+ }
+ }
+
+ np = of_get_compatible_child(max7360_mfd->dev->of_node,
+ KEYPAD_COMPATIBLE);
+ if (np) {
+ ret = of_property_read_u32(np, "keypad,num-columns", &columns);
+ if (ret < 0) {
+ dev_err(max7360_mfd->dev, "Failed to read columns count\n");
+ return ret;
+ }
+ }
+
+ if (gpos > MAX7360_MAX_GPO ||
+ (gpos + columns > MAX7360_MAX_KEY_COLS)) {
+ dev_err(max7360_mfd->dev,
+ "Incompatible gpos and columns count (%u, %u)\n",
+ gpos, columns);
+ return -EINVAL;
+ }
+
+ /*
+ * 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, gpos);
+ ret = regmap_write_bits(max7360_mfd->regmap, MAX7360_REG_DEBOUNCE,
+ MAX7360_PORTS, val);
+ if (ret) {
+ dev_err(max7360_mfd->dev,
+ "Failed to write max7360 columns/gpos configuration");
+ return ret;
+ }
+
+ return 0;
+}
+
+int max7360_port_pin_request(struct device *dev, unsigned int pin, bool request)
+{
+ struct i2c_client *client;
+ struct max7360_mfd *max7360_mfd;
+ unsigned long flags;
+ int ret = 0;
+
+ client = to_i2c_client(dev);
+ max7360_mfd = i2c_get_clientdata(client);
+
+ spin_lock_irqsave(&request_lock, flags);
+ if (request) {
+ if (max7360_mfd->requested_ports & BIT(pin))
+ ret = -EBUSY;
+ else
+ max7360_mfd->requested_ports |= BIT(pin);
+ } else {
+ max7360_mfd->requested_ports &= ~BIT(pin);
+ }
+ spin_unlock_irqrestore(&request_lock, flags);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(max7360_port_pin_request);
+
+static int max7360_mask_irqs(struct max7360_mfd *max7360_mfd)
+{
+ unsigned int i;
+ unsigned int val;
+ int ret;
+
+ /*
+ * GPIO/PWM interrupts are not masked on reset: mask the during probe,
+ * avoiding repeated spurious interrupts if the corresponding drivers
+ * are not present.
+ */
+ for (i = 0; i < MAX7360_PORT_PWM_COUNT; i++) {
+ ret = regmap_write_bits(max7360_mfd->regmap,
+ MAX7360_REG_PWMCFG + i,
+ MAX7360_PORT_CFG_INTERRUPT_MASK,
+ MAX7360_PORT_CFG_INTERRUPT_MASK);
+ if (ret) {
+ dev_err(max7360_mfd->dev,
+ "failed to write max7360 port configuration");
+ return ret;
+ }
+ }
+
+ /* Read gpio in register, to ack any pending IRQ.
+ */
+ ret = regmap_read(max7360_mfd->regmap, MAX7360_REG_GPIOIN, &val);
+ if (ret) {
+ dev_err(max7360_mfd->dev, "Failed to read gpio values: %d\n",
+ ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int max7360_reset(struct max7360_mfd *max7360_mfd)
+{
+ int err;
+
+ /*
+ * Set back the default values.
+ * We do not use GPIO reset function here, as it does not work reliably.
+ */
+ err = regmap_write(max7360_mfd->regmap, MAX7360_REG_GPIODEB, 0x00);
+ if (err) {
+ dev_err(max7360_mfd->dev, "Failed to set configuration\n");
+ return err;
+ }
+
+ err = regmap_write(max7360_mfd->regmap, MAX7360_REG_GPIOCURR, MAX7360_REG_GPIOCURR_FIXED);
+ if (err) {
+ dev_err(max7360_mfd->dev, "Failed to set configuration\n");
+ return err;
+ }
+
+ err = regmap_write(max7360_mfd->regmap, MAX7360_REG_GPIOOUTM, 0x00);
+ if (err) {
+ dev_err(max7360_mfd->dev, "Failed to set configuration\n");
+ return err;
+ }
+
+ err = regmap_write(max7360_mfd->regmap, MAX7360_REG_PWMCOM, 0x00);
+ if (err) {
+ dev_err(max7360_mfd->dev, "Failed to set configuration\n");
+ return err;
+ }
+
+ err = regmap_write(max7360_mfd->regmap, MAX7360_REG_SLEEP, 0);
+ if (err) {
+ dev_err(max7360_mfd->dev, "Failed to set configuration\n");
+ return err;
+ }
+
+ return 0;
+}
+
+static int max7360_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct regmap *regmap;
+ struct max7360_mfd *max7360_mfd;
+ int err;
+
+ 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");
+
+ max7360_mfd = devm_kzalloc(dev, sizeof(*max7360_mfd), GFP_KERNEL);
+ if (!max7360_mfd)
+ return -ENOMEM;
+
+ max7360_mfd->regmap = regmap;
+ max7360_mfd->dev = dev;
+ i2c_set_clientdata(client, max7360_mfd);
+
+ err = max7360_reset(max7360_mfd);
+ if (err)
+ return dev_err_probe(dev, err, "Failed to reset device\n");
+
+ err = max7360_set_gpos_count(max7360_mfd);
+ if (err)
+ return dev_err_probe(dev, err, "Failed to set GPOS pin count\n");
+
+ /*
+ * Get the device out of shutdown mode.
+ */
+ err = regmap_write_bits(regmap, MAX7360_REG_GPIOCFG,
+ MAX7360_GPIO_CFG_GPIO_EN,
+ MAX7360_GPIO_CFG_GPIO_EN);
+ if (err)
+ return dev_err_probe(dev, err, "Failed to set device out of shutdown\n");
+
+ err = max7360_mask_irqs(max7360_mfd);
+ if (err)
+ return dev_err_probe(dev, err, "could not mask interrupts\n");
+
+ err = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
+ max7360_cells, ARRAY_SIZE(max7360_cells),
+ NULL, 0, NULL);
+ if (err)
+ return dev_err_probe(dev, err, "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 MFD 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..2b77110acd80
--- /dev/null
+++ b/include/linux/mfd/max7360.h
@@ -0,0 +1,103 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __LINUX_MFD_MAX7360_H
+#define __LINUX_MFD_MAX7360_H
+#include <linux/bitfield.h>
+#include <linux/device.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 registers
+ */
+#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_GPIOIN 0x49
+#define MAX7360_REG_RTR_CNT 0x4A
+#define MAX7360_REG_PWMBASE 0x50
+#define MAX7360_REG_PWMCFG 0x58
+
+#define MAX7360_REG_PORTCFGBASE 0x58
+
+/*
+ * 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_REG_GPIOCURR_FIXED 0xC0
+
+/*
+ * Autosleep register values (ms)
+ */
+#define MAX7360_AUTOSLEEP_8192 0x01
+#define MAX7360_AUTOSLEEP_4096 0x02
+#define MAX7360_AUTOSLEEP_2048 0x03
+#define MAX7360_AUTOSLEEP_1024 0x04
+#define MAX7360_AUTOSLEEP_512 0x05
+#define MAX7360_AUTOSLEEP_256 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
+
+int max7360_port_pin_request(struct device *dev, unsigned int pin, bool request);
+
+#endif
--
2.39.5
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 4/8] pwm: max7360: Add MAX7360 PWM support
2024-12-19 16:21 [PATCH 0/8] Add support for MAX7360 multifunction device Mathieu Dubois-Briand
` (2 preceding siblings ...)
2024-12-19 16:21 ` [PATCH 3/8] mfd: Add max7360 support mathieu.dubois-briand
@ 2024-12-19 16:21 ` mathieu.dubois-briand
2024-12-19 21:53 ` Uwe Kleine-König
` (2 more replies)
2024-12-19 16:21 ` [PATCH 5/8] gpio: max7360: Add MAX7360 gpio support Mathieu Dubois-Briand
` (3 subsequent siblings)
7 siblings, 3 replies; 25+ messages in thread
From: mathieu.dubois-briand @ 2024-12-19 16:21 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Kamel Bouhara, Linus Walleij, Bartosz Golaszewski,
Dmitry Torokhov, Uwe Kleine-König
Cc: devicetree, linux-kernel, linux-gpio, linux-input, linux-pwm,
Grégory Clement, Thomas Petazzoni, 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>
Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
---
drivers/pwm/Kconfig | 11 +++
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-max7360.c | 173 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 185 insertions(+)
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 0915c1e7df16..399dc3f76e92 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -745,4 +745,15 @@ 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
+ depends on OF_GPIO
+ 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 9081e0c0e9e0..ae8908ffc892 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..b1cde3e86864
--- /dev/null
+++ b/drivers/pwm/pwm-max7360.c
@@ -0,0 +1,173 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2024 Bootlin
+ *
+ * Author: Kamel BOUHARA <kamel.bouhara@bootlin.com>
+ */
+#include <linux/math.h>
+#include <linux/mfd/max7360.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/regmap.h>
+
+#define MAX7360_NUM_PWMS 8
+#define MAX7360_PWM_MAX_RES 256
+#define MAX7360_PWM_PERIOD_NS 2000000 /* 500 Hz */
+#define MAX7360_PWM_COMMON_PWN BIT(5)
+#define MAX7360_PWM_CTRL_ENABLE(n) BIT(n)
+#define MAX7360_PWM_PORT(n) BIT(n)
+
+struct max7360_pwm {
+ struct device *dev;
+ struct regmap *regmap;
+};
+
+static inline struct max7360_pwm *to_max7360_pwm(struct pwm_chip *chip)
+{
+ return pwmchip_get_drvdata(chip);
+}
+
+static int max7360_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+ struct max7360_pwm *max7360_pwm;
+ int ret;
+
+ max7360_pwm = to_max7360_pwm(chip);
+ ret = max7360_port_pin_request(max7360_pwm->dev->parent, pwm->hwpwm,
+ true);
+ if (ret) {
+ dev_err(&chip->dev, "failed to request pwm-%d\n", pwm->hwpwm);
+ return ret;
+ }
+
+ ret = regmap_write_bits(max7360_pwm->regmap,
+ MAX7360_REG_PWMCFG + pwm->hwpwm,
+ MAX7360_PWM_COMMON_PWN,
+ 0);
+ if (ret) {
+ dev_err(&chip->dev, "failed to write pwm-%d cfg register, error %d\n",
+ pwm->hwpwm, ret);
+ return ret;
+ }
+
+ ret = regmap_write_bits(max7360_pwm->regmap, MAX7360_REG_PORTS,
+ MAX7360_PWM_PORT(pwm->hwpwm),
+ MAX7360_PWM_PORT(pwm->hwpwm));
+ if (ret) {
+ dev_err(&chip->dev, "failed to write pwm-%d ports register, error %d\n",
+ pwm->hwpwm, ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static void max7360_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+ struct max7360_pwm *max7360_pwm;
+ int ret;
+
+ max7360_pwm = to_max7360_pwm(chip);
+ ret = regmap_write_bits(max7360_pwm->regmap, MAX7360_REG_GPIOCTRL,
+ MAX7360_PWM_CTRL_ENABLE(pwm->hwpwm),
+ 0);
+ if (ret)
+ dev_err(&chip->dev, "failed to enable pwm-%d , error %d\n",
+ pwm->hwpwm, ret);
+
+ max7360_port_pin_request(max7360_pwm->dev->parent, pwm->hwpwm,
+ false);
+}
+
+static int max7360_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+ const struct pwm_state *state)
+{
+ struct max7360_pwm *max7360_pwm;
+ u64 duty_steps = state->duty_cycle * MAX7360_PWM_MAX_RES;
+ int ret;
+
+ if (state->polarity != PWM_POLARITY_NORMAL)
+ return -EINVAL;
+
+ max7360_pwm = to_max7360_pwm(chip);
+ ret = regmap_write_bits(max7360_pwm->regmap, MAX7360_REG_GPIOCTRL,
+ MAX7360_PWM_CTRL_ENABLE(pwm->hwpwm),
+ MAX7360_PWM_CTRL_ENABLE(pwm->hwpwm));
+ if (ret) {
+ dev_err(&chip->dev, "failed to enable pwm-%d , error %d\n",
+ pwm->hwpwm, ret);
+ return ret;
+ }
+
+ do_div(duty_steps, MAX7360_PWM_PERIOD_NS);
+
+ ret = regmap_write(max7360_pwm->regmap, MAX7360_REG_PWMBASE + pwm->hwpwm,
+ duty_steps >= 255 ? 255 : duty_steps);
+ if (ret) {
+ dev_err(&chip->dev,
+ "failed to apply pwm duty_cycle %llu on pwm-%d, error %d\n",
+ duty_steps, pwm->hwpwm, ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct pwm_ops max7360_pwm_ops = {
+ .request = max7360_pwm_request,
+ .free = max7360_pwm_free,
+ .apply = max7360_pwm_apply,
+};
+
+static int max7360_pwm_probe(struct platform_device *pdev)
+{
+ struct max7360_pwm *max7360_pwm;
+ struct pwm_chip *chip;
+
+ if (!pdev->dev.parent) {
+ dev_err(&pdev->dev, "no parent device\n");
+ return -ENODEV;
+ }
+
+ chip = devm_pwmchip_alloc(&pdev->dev, MAX7360_NUM_PWMS,
+ sizeof(*max7360_pwm));
+ if (IS_ERR(chip))
+ return PTR_ERR(chip);
+ chip->ops = &max7360_pwm_ops;
+
+ max7360_pwm = to_max7360_pwm(chip);
+ max7360_pwm->dev = &pdev->dev;
+
+ max7360_pwm->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+ if (!max7360_pwm->regmap) {
+ dev_err(&pdev->dev, "could not get parent regmap\n");
+ return -ENODEV;
+ }
+
+ return devm_pwmchip_add(&pdev->dev, chip);
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id max7360_pwm_of_match[] = {
+ { .compatible = "maxim,max7360-pwm", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, max7360_pwm_of_match);
+#endif
+
+static struct platform_driver max7360_pwm_driver = {
+ .driver = {
+ .name = "max7360-pwm",
+ .of_match_table = of_match_ptr(max7360_pwm_of_match),
+ },
+ .probe = max7360_pwm_probe,
+};
+module_platform_driver(max7360_pwm_driver);
+
+MODULE_DESCRIPTION("MAX7360 PWM driver");
+MODULE_AUTHOR("Kamel BOUHARA <kamel.bouhara@bootlin.com>");
+MODULE_ALIAS("platform:max7360-pwm");
+MODULE_LICENSE("GPL");
--
2.39.5
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 5/8] gpio: max7360: Add MAX7360 gpio support
2024-12-19 16:21 [PATCH 0/8] Add support for MAX7360 multifunction device Mathieu Dubois-Briand
` (3 preceding siblings ...)
2024-12-19 16:21 ` [PATCH 4/8] pwm: max7360: Add MAX7360 PWM support mathieu.dubois-briand
@ 2024-12-19 16:21 ` Mathieu Dubois-Briand
2024-12-20 15:54 ` kernel test robot
2024-12-20 16:39 ` kernel test robot
2024-12-19 16:21 ` [PATCH 6/8] input: keyboard: Add support for MAX7360 keypad Mathieu Dubois-Briand
` (2 subsequent siblings)
7 siblings, 2 replies; 25+ messages in thread
From: Mathieu Dubois-Briand @ 2024-12-19 16:21 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Kamel Bouhara, Linus Walleij, Bartosz Golaszewski,
Dmitry Torokhov, Uwe Kleine-König
Cc: devicetree, linux-kernel, linux-gpio, linux-input, linux-pwm,
Grégory Clement, Thomas Petazzoni, 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 | 11 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-max7360.c | 453 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 465 insertions(+)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 93ee3aa092f8..efe07e21c442 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1444,6 +1444,17 @@ config GPIO_MADERA
help
Support for GPIOs on Cirrus Logic Madera class codecs.
+config GPIO_MAX7360
+ tristate "MAX7360 GPIO support"
+ depends on MFD_MAX7360
+ depends on OF_GPIO
+ 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 af3ba4d81b58..581341b3e3e4 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -100,6 +100,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..0e587aa53cf8
--- /dev/null
+++ b/drivers/gpio/gpio-max7360.c
@@ -0,0 +1,453 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2024 Bootlin
+ *
+ * Author: Kamel BOUHARA <kamel.bouhara@bootlin.com>
+ * Author: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/max7360.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#define MAX7360_GPIO_PORT 1
+#define MAX7360_GPIO_COL 2
+
+struct max7360_gpio {
+ struct gpio_chip chip;
+ struct device *dev;
+ struct regmap *regmap;
+ int gpio_function;
+
+ /*
+ * Interrupts handling data: only used when gpio_function is
+ * MAX7360_GPIO_PORT.
+ */
+ u8 masked_interrupts;
+ u8 in_values;
+ unsigned int irq_types[MAX7360_MAX_GPIO];
+};
+
+static void max7360_gpio_set_value(struct gpio_chip *gc,
+ unsigned int pin, int state)
+{
+ struct max7360_gpio *max7360_gpio = gpiochip_get_data(gc);
+ unsigned int val;
+ int ret;
+
+ if (max7360_gpio->gpio_function == MAX7360_GPIO_COL) {
+ int off = MAX7360_MAX_GPIO - (gc->ngpio - pin);
+
+ ret = regmap_write_bits(max7360_gpio->regmap, MAX7360_REG_PORTS,
+ BIT(off), state ? BIT(off) : 0);
+ } else {
+ ret = regmap_write(max7360_gpio->regmap,
+ MAX7360_REG_PWMBASE + pin, state ? 0xFF : 0);
+ }
+
+ if (ret)
+ dev_err(max7360_gpio->dev,
+ "failed to set value %d on gpio-%d", val, pin);
+}
+
+static int max7360_gpio_get_value(struct gpio_chip *gc, unsigned int pin)
+{
+ struct max7360_gpio *max7360_gpio = gpiochip_get_data(gc);
+ unsigned int val;
+ int off;
+ int ret;
+
+ if (max7360_gpio->gpio_function == MAX7360_GPIO_COL) {
+ off = MAX7360_MAX_GPIO - (gc->ngpio - pin);
+
+ ret = regmap_read(max7360_gpio->regmap, MAX7360_REG_PORTS, &val);
+ } else {
+ off = pin;
+ ret = regmap_read(max7360_gpio->regmap, MAX7360_REG_GPIOIN, &val);
+ }
+
+ if (ret) {
+ dev_err(max7360_gpio->dev, "failed to read gpio-%d", pin);
+ return ret;
+ }
+
+ return !!(val & BIT(off));
+}
+
+static int max7360_gpio_get_direction(struct gpio_chip *gc, unsigned int pin)
+{
+ struct max7360_gpio *max7360_gpio = gpiochip_get_data(gc);
+ unsigned int val;
+ int ret;
+
+ if (max7360_gpio->gpio_function == MAX7360_GPIO_COL)
+ return GPIO_LINE_DIRECTION_OUT;
+
+ ret = regmap_read(max7360_gpio->regmap, MAX7360_REG_GPIOCTRL, &val);
+ if (ret) {
+ dev_err(max7360_gpio->dev, "failed to read gpio-%d direction",
+ pin);
+ return ret;
+ }
+
+ if (val & BIT(pin))
+ return GPIO_LINE_DIRECTION_OUT;
+
+ return GPIO_LINE_DIRECTION_IN;
+}
+
+static int max7360_gpio_direction_input(struct gpio_chip *gc, unsigned int pin)
+{
+ struct max7360_gpio *max7360_gpio = gpiochip_get_data(gc);
+ int ret;
+
+ if (max7360_gpio->gpio_function == MAX7360_GPIO_COL)
+ return -EIO;
+
+ ret = regmap_write_bits(max7360_gpio->regmap, MAX7360_REG_GPIOCTRL,
+ BIT(pin), 0);
+ if (ret) {
+ dev_err(max7360_gpio->dev, "failed to set gpio-%d direction",
+ pin);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int max7360_gpio_direction_output(struct gpio_chip *gc, unsigned int pin,
+ int state)
+{
+ struct max7360_gpio *max7360_gpio = gpiochip_get_data(gc);
+ int ret;
+
+ if (max7360_gpio->gpio_function == MAX7360_GPIO_PORT) {
+ ret = regmap_write_bits(max7360_gpio->regmap,
+ MAX7360_REG_GPIOCTRL, BIT(pin),
+ BIT(pin));
+ if (ret) {
+ dev_err(max7360_gpio->dev,
+ "failed to set gpio-%d direction", pin);
+ return ret;
+ }
+ }
+
+ max7360_gpio_set_value(gc, pin, state);
+
+ return 0;
+}
+
+static int max7360_gpio_request(struct gpio_chip *gc, unsigned int pin)
+{
+ struct max7360_gpio *max7360_gpio = gpiochip_get_data(gc);
+
+ /*
+ * GPOs on COL pins (keypad columns) can always be requested: this
+ * driver has full access to them, up to the number set in chip.ngpio.
+ * GPIOs on PORT pins are shared with the PWM and rotary encoder
+ * drivers: they have to be requested from the MFD driver.
+ */
+ if (max7360_gpio->gpio_function == MAX7360_GPIO_COL)
+ return 0;
+
+ return max7360_port_pin_request(max7360_gpio->dev->parent, pin, true);
+}
+
+static void max7360_gpio_free(struct gpio_chip *gc, unsigned int pin)
+{
+ struct max7360_gpio *max7360_gpio = gpiochip_get_data(gc);
+
+ if (max7360_gpio->gpio_function == MAX7360_GPIO_COL)
+ return;
+
+ max7360_port_pin_request(max7360_gpio->dev->parent, pin, false);
+}
+
+static struct gpio_chip max7360_gpio_chip = {
+ .label = "max7360",
+ .request = max7360_gpio_request,
+ .free = max7360_gpio_free,
+ .get_direction = max7360_gpio_get_direction,
+ .direction_input = max7360_gpio_direction_input,
+ .direction_output = max7360_gpio_direction_output,
+ .get = max7360_gpio_get_value,
+ .set = max7360_gpio_set_value,
+ .base = -1,
+ .can_sleep = true,
+};
+
+static irqreturn_t max7360_gpio_irq(int irq, void *data)
+{
+ struct max7360_gpio *max7360_gpio = data;
+ unsigned long pending;
+ unsigned int gpio_irq;
+ unsigned int type;
+ unsigned int count = 0;
+ int val;
+ int irqn;
+ int ret;
+
+ ret = regmap_read(max7360_gpio->regmap, MAX7360_REG_GPIOIN, &val);
+ if (ret) {
+ dev_err(max7360_gpio->dev, "Failed to read gpio values: %d\n",
+ ret);
+ return IRQ_NONE;
+ }
+
+ /* MAX7360 generates interrupts but does not report which pins changed:
+ * compare the pin value with the value they had in previous interrupt
+ * and report interrupt if the change match the set IRQ type.
+ */
+ pending = val ^ max7360_gpio->in_values;
+ for_each_set_bit(irqn, &pending, max7360_gpio->chip.ngpio) {
+ type = max7360_gpio->irq_types[irqn];
+ if (max7360_gpio->masked_interrupts & BIT(irqn))
+ continue;
+ if ((val & BIT(irqn)) && type == IRQ_TYPE_EDGE_FALLING)
+ continue;
+ if (!(val & BIT(irqn)) && type == IRQ_TYPE_EDGE_RISING)
+ continue;
+ gpio_irq = irq_find_mapping(max7360_gpio->chip.irq.domain, irqn);
+ handle_nested_irq(gpio_irq);
+ count++;
+ }
+
+ max7360_gpio->in_values = val;
+
+ if (count == 0)
+ return IRQ_NONE;
+
+ return IRQ_HANDLED;
+}
+
+static void max7360_gpio_irq_unmask(struct irq_data *data)
+{
+ struct max7360_gpio *max7360_gpio;
+ unsigned int pin = irqd_to_hwirq(data);
+ unsigned int val;
+ int ret;
+
+ max7360_gpio = gpiochip_get_data(irq_data_get_irq_chip_data(data));
+
+ /* Read current pin value, so we know if the pin changed in the next
+ * interrupt.
+ * No lock should be needed regarding the interrupt handler: as long as
+ * the corresponding bit has not been cleared in masked_interrupts, this
+ * gpio is ignored.
+ */
+ ret = regmap_read(max7360_gpio->regmap, MAX7360_REG_GPIOIN, &val);
+ if (ret)
+ dev_err(max7360_gpio->dev, "Failed to read gpio values: %d\n",
+ ret);
+
+ max7360_gpio->in_values &= ~BIT(pin);
+ max7360_gpio->in_values |= val & BIT(pin);
+
+ ret = regmap_write_bits(max7360_gpio->regmap, MAX7360_REG_PWMCFG + pin,
+ MAX7360_PORT_CFG_INTERRUPT_MASK, 0);
+
+ if (ret)
+ dev_err(max7360_gpio->dev, "failed to unmask gpio-%d", pin);
+
+ max7360_gpio->masked_interrupts &= ~BIT(pin);
+}
+
+static void max7360_gpio_irq_mask(struct irq_data *data)
+{
+ struct max7360_gpio *max7360_gpio;
+ unsigned int pin = irqd_to_hwirq(data);
+ int ret;
+
+ max7360_gpio = gpiochip_get_data(irq_data_get_irq_chip_data(data));
+
+ max7360_gpio->masked_interrupts |= BIT(pin);
+
+ ret = regmap_write_bits(max7360_gpio->regmap, MAX7360_REG_PWMCFG + pin,
+ MAX7360_PORT_CFG_INTERRUPT_MASK,
+ MAX7360_PORT_CFG_INTERRUPT_MASK);
+
+ if (ret)
+ dev_err(max7360_gpio->dev, "failed to mask gpio-%d", pin);
+}
+
+static void max7360_gpio_irq_enable(struct irq_data *data)
+{
+ max7360_gpio_irq_unmask(data);
+}
+
+static void max7360_gpio_irq_disable(struct irq_data *data)
+{
+ max7360_gpio_irq_mask(data);
+}
+
+static int max7360_gpio_irq_set_type(struct irq_data *data,
+ unsigned int flow_type)
+{
+ struct max7360_gpio *max7360_gpio;
+ unsigned int pin;
+ unsigned int val;
+ int ret;
+
+ pin = irqd_to_hwirq(data);
+ max7360_gpio = gpiochip_get_data(irq_data_get_irq_chip_data(data));
+
+ switch (flow_type) {
+ case IRQ_TYPE_EDGE_RISING:
+ val = 0;
+ break;
+ case IRQ_TYPE_EDGE_FALLING:
+ case IRQ_TYPE_EDGE_BOTH:
+ val = MAX7360_PORT_CFG_INTERRUPT_EDGES;
+ break;
+ default:
+ return -EINVAL;
+ }
+ ret = regmap_write_bits(max7360_gpio->regmap, MAX7360_REG_PWMCFG + pin,
+ MAX7360_PORT_CFG_INTERRUPT_EDGES, val);
+
+ if (ret)
+ dev_err(max7360_gpio->dev, "failed to unmask gpio-%d", pin);
+
+ max7360_gpio->irq_types[pin] = flow_type;
+
+ return 0;
+}
+
+static const struct irq_chip max7360_gpio_irqchip = {
+ .name = "max7360",
+ .irq_enable = max7360_gpio_irq_enable,
+ .irq_disable = max7360_gpio_irq_disable,
+ .irq_mask = max7360_gpio_irq_mask,
+ .irq_unmask = max7360_gpio_irq_unmask,
+ .irq_set_type = max7360_gpio_irq_set_type,
+ .flags = IRQCHIP_IMMUTABLE,
+ GPIOCHIP_IRQ_RESOURCE_HELPERS,
+};
+
+static int max7360_gpio_probe(struct platform_device *pdev)
+{
+ struct max7360_gpio *max7360_gpio;
+ unsigned int ngpios;
+ unsigned int outconf;
+ struct gpio_irq_chip *girq;
+ unsigned long flags;
+ int irq;
+ int ret;
+
+ if (!pdev->dev.parent) {
+ dev_err(&pdev->dev, "no parent device\n");
+ return -ENODEV;
+ }
+
+ max7360_gpio = devm_kzalloc(&pdev->dev, sizeof(struct max7360_gpio),
+ GFP_KERNEL);
+ if (!max7360_gpio)
+ return -ENOMEM;
+
+ if (of_property_read_u32(pdev->dev.of_node, "ngpios", &ngpios)) {
+ dev_err(&pdev->dev, "Missing ngpios OF property\n");
+ return -ENODEV;
+ }
+
+ max7360_gpio->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+ if (!max7360_gpio->regmap) {
+ dev_err(&pdev->dev, "could not get parent regmap\n");
+ return -ENODEV;
+ }
+
+ max7360_gpio->dev = &pdev->dev;
+ max7360_gpio->chip = max7360_gpio_chip;
+ max7360_gpio->chip.ngpio = ngpios;
+ max7360_gpio->chip.parent = &pdev->dev;
+ max7360_gpio->chip.base = -1;
+ max7360_gpio->gpio_function = (int)device_get_match_data(&pdev->dev);
+
+ dev_dbg(&pdev->dev, "gpio count: %d\n", max7360_gpio->chip.ngpio);
+
+ if (max7360_gpio->gpio_function == MAX7360_GPIO_PORT) {
+ /* Port GPIOs: set output mode configuration (constant-current
+ * or not).
+ * This property is optional.
+ */
+ outconf = 0;
+ ret = of_property_read_u32(pdev->dev.of_node,
+ "constant-current-disable", &outconf);
+ if (ret && (ret != -EINVAL)) {
+ dev_err(&pdev->dev,
+ "Failed to read constant-current-disable OF property\n");
+ return -ENODEV;
+ }
+
+ regmap_write(max7360_gpio->regmap, MAX7360_REG_GPIOOUTM, outconf);
+ }
+
+ if (max7360_gpio->gpio_function == MAX7360_GPIO_PORT &&
+ of_property_read_bool(pdev->dev.of_node, "interrupt-controller")) {
+ /* Port GPIOs: declare IRQ chip, if configuration was provided.
+ */
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return dev_err_probe(&pdev->dev, irq,
+ "Failed to get IRQ");
+
+ flags = IRQF_TRIGGER_LOW | IRQF_ONESHOT | IRQF_SHARED;
+ ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+ max7360_gpio_irq, flags,
+ "max7360-gpio", max7360_gpio);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "Failed to register interrupt: %d\n",
+ ret);
+
+ girq = &max7360_gpio->chip.irq;
+ gpio_irq_chip_set_chip(girq, &max7360_gpio_irqchip);
+ girq->parent_handler = NULL;
+ girq->num_parents = 0;
+ girq->parents = NULL;
+ girq->threaded = true;
+ girq->default_type = IRQ_TYPE_NONE;
+ girq->handler = handle_simple_irq;
+ }
+
+ ret = devm_gpiochip_add_data(&pdev->dev, &max7360_gpio->chip, max7360_gpio);
+ if (ret) {
+ dev_err(&pdev->dev, "unable to add gpiochip: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+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 = of_match_ptr(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_ALIAS("platform:max7360-gpio");
+MODULE_LICENSE("GPL");
--
2.39.5
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 6/8] input: keyboard: Add support for MAX7360 keypad
2024-12-19 16:21 [PATCH 0/8] Add support for MAX7360 multifunction device Mathieu Dubois-Briand
` (4 preceding siblings ...)
2024-12-19 16:21 ` [PATCH 5/8] gpio: max7360: Add MAX7360 gpio support Mathieu Dubois-Briand
@ 2024-12-19 16:21 ` Mathieu Dubois-Briand
2024-12-19 19:35 ` Dmitry Torokhov
2024-12-20 17:12 ` kernel test robot
2024-12-19 16:21 ` [PATCH 7/8] input: misc: Add support for MAX7360 rotary Mathieu Dubois-Briand
2024-12-19 16:21 ` [PATCH 8/8] MAINTAINERS: Add entry on MAX7360 driver Mathieu Dubois-Briand
7 siblings, 2 replies; 25+ messages in thread
From: Mathieu Dubois-Briand @ 2024-12-19 16:21 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Kamel Bouhara, Linus Walleij, Bartosz Golaszewski,
Dmitry Torokhov, Uwe Kleine-König
Cc: devicetree, linux-kernel, linux-gpio, linux-input, linux-pwm,
Grégory Clement, Thomas Petazzoni, 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 | 297 ++++++++++++++++++++++++++++++++
3 files changed, 310 insertions(+)
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 721ab69e84ac..bba029f65cfa 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..fbc51c89dba1
--- /dev/null
+++ b/drivers/input/keyboard/max7360-keypad.c
@@ -0,0 +1,297 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2024 Bootlin
+ *
+ * Author: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
+ */
+
+#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/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_wakeirq.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+struct max7360_keypad {
+ struct input_dev *input;
+ unsigned int rows;
+ unsigned int cols;
+ unsigned int debounce_ms;
+ int irq;
+ bool no_autorepeat;
+ 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;
+
+ ret = regmap_read(max7360_keypad->regmap, MAX7360_REG_KEYFIFO, &val);
+ if (!ret && val == MAX7360_FIFO_OVERFLOW) {
+ /* FIFO overflow: ignore it and get next event. */
+ dev_err(&max7360_keypad->input->dev, "max7360 FIFO overflow");
+ ret = regmap_read(max7360_keypad->regmap, MAX7360_REG_KEYFIFO,
+ &val);
+ }
+ if (ret) {
+ dev_err(&max7360_keypad->input->dev,
+ "Failed to read max7360 FIFO");
+ return ret;
+ }
+
+ 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");
+ 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, ~MAX7360_CFG_SLEEP);
+ if (ret) {
+ dev_err(&max7360_keypad->input->dev,
+ "Failed to write max7360 configuration");
+ }
+}
+
+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) {
+ dev_err(&max7360_keypad->input->dev,
+ "Failed to write max7360 debounce configuration");
+ return ret;
+ }
+
+ ret = regmap_write_bits(max7360_keypad->regmap, MAX7360_REG_INTERRUPT,
+ MAX7360_INTERRUPT_TIME_MASK,
+ FIELD_PREP(MAX7360_INTERRUPT_TIME_MASK, 1));
+ if (ret) {
+ dev_err(&max7360_keypad->input->dev,
+ "Failed to write max7360 keypad interrupt configuration");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int max7360_keypad_parse_dt(struct platform_device *pdev,
+ struct max7360_keypad *max7360_keypad)
+{
+ bool no_autorepeat;
+ int ret;
+
+ ret = matrix_keypad_parse_properties(&pdev->dev, &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(&pdev->dev,
+ "Invalid number of columns or rows (%ux%u)\n",
+ max7360_keypad->cols, max7360_keypad->rows);
+ return -EINVAL;
+ }
+
+ max7360_keypad->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+ if (!max7360_keypad->regmap) {
+ dev_err(&pdev->dev, "Could not get parent regmap\n");
+ return -ENODEV;
+ }
+
+ no_autorepeat = of_property_read_bool(pdev->dev.of_node,
+ "linux,input-no-autorepeat");
+ max7360_keypad->no_autorepeat = no_autorepeat;
+
+ max7360_keypad->debounce_ms = MAX7360_DEBOUNCE_MIN;
+ ret = of_property_read_u32(pdev->dev.of_node, "debounce-delay-ms",
+ &max7360_keypad->debounce_ms);
+ if (ret == -EINVAL) {
+ dev_info(&pdev->dev, "Using default debounce-delay-ms: %u\n",
+ max7360_keypad->debounce_ms);
+ } else if (ret < 0) {
+ dev_err(&pdev->dev, "Failed to read debounce-delay-ms property\n");
+ return ret;
+ } else if (max7360_keypad->debounce_ms < MAX7360_DEBOUNCE_MIN ||
+ max7360_keypad->debounce_ms > MAX7360_DEBOUNCE_MAX) {
+ dev_err(&pdev->dev,
+ "Invalid 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 input_dev *input;
+ unsigned long flags;
+ int irq;
+ int ret;
+
+ if (!pdev->dev.parent)
+ return dev_err_probe(&pdev->dev, -ENODEV, "No parent device\n");
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return irq;
+
+ max7360_keypad = devm_kzalloc(&pdev->dev, sizeof(*max7360_keypad),
+ GFP_KERNEL);
+ if (!max7360_keypad)
+ return -ENOMEM;
+
+ ret = max7360_keypad_parse_dt(pdev, max7360_keypad);
+ if (ret)
+ return ret;
+
+ input = devm_input_allocate_device(&pdev->dev);
+ if (!input)
+ return dev_err_probe(&pdev->dev, -ENOMEM,
+ "Failed to allocate input device\n");
+
+ max7360_keypad->input = input;
+
+ input->id.bustype = BUS_I2C;
+ input->name = pdev->name;
+ input->dev.parent = &pdev->dev;
+
+ input->open = max7360_keypad_open;
+ input->close = max7360_keypad_close;
+
+ ret = matrix_keypad_build_keymap(NULL, NULL,
+ MAX7360_MAX_KEY_ROWS,
+ MAX7360_MAX_KEY_COLS,
+ max7360_keypad->keycodes, input);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "Failed to build keymap\n");
+
+ input_set_capability(input, EV_MSC, MSC_SCAN);
+ if (!max7360_keypad->no_autorepeat)
+ __set_bit(EV_REP, input->evbit);
+
+ input_set_drvdata(input, max7360_keypad);
+
+ flags = IRQF_TRIGGER_LOW | IRQF_ONESHOT | IRQF_SHARED;
+ ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+ max7360_keypad_irq, flags,
+ "max7360-keypad", max7360_keypad);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "Failed to register interrupt: %d\n", ret);
+
+ ret = input_register_device(input);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "Could not register input device: %d\n",
+ ret);
+
+ platform_set_drvdata(pdev, max7360_keypad);
+
+ device_init_wakeup(&pdev->dev, true);
+ ret = dev_pm_set_wake_irq(&pdev->dev, irq);
+ if (ret)
+ dev_warn(&pdev->dev, "Failed to set up wakeup irq: %d\n", ret);
+
+ ret = max7360_keypad_hw_init(max7360_keypad);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "Failed to initialize max7360 keypad\n");
+
+ return 0;
+}
+
+static void max7360_keypad_remove(struct platform_device *pdev)
+{
+ dev_pm_clear_wake_irq(&pdev->dev);
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id max7360_keypad_of_match[] = {
+ { .compatible = "maxim,max7360-keypad", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, max7360_keypad_of_match);
+#endif
+
+static struct platform_driver max7360_keypad_driver = {
+ .driver = {
+ .name = "max7360-keypad",
+ .of_match_table = of_match_ptr(max7360_keypad_of_match),
+ },
+ .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_ALIAS("platform:max7360-keypad");
+MODULE_LICENSE("GPL");
--
2.39.5
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 7/8] input: misc: Add support for MAX7360 rotary
2024-12-19 16:21 [PATCH 0/8] Add support for MAX7360 multifunction device Mathieu Dubois-Briand
` (5 preceding siblings ...)
2024-12-19 16:21 ` [PATCH 6/8] input: keyboard: Add support for MAX7360 keypad Mathieu Dubois-Briand
@ 2024-12-19 16:21 ` Mathieu Dubois-Briand
2024-12-19 16:21 ` [PATCH 8/8] MAINTAINERS: Add entry on MAX7360 driver Mathieu Dubois-Briand
7 siblings, 0 replies; 25+ messages in thread
From: Mathieu Dubois-Briand @ 2024-12-19 16:21 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Kamel Bouhara, Linus Walleij, Bartosz Golaszewski,
Dmitry Torokhov, Uwe Kleine-König
Cc: devicetree, linux-kernel, linux-gpio, linux-input, linux-pwm,
Grégory Clement, Thomas Petazzoni, 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 | 11 ++
drivers/input/misc/Makefile | 1 +
drivers/input/misc/max7360-rotary.c | 194 ++++++++++++++++++++++++++++++++++++
3 files changed, 206 insertions(+)
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 6a852c76331b..8430aaf08c04 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -230,6 +230,17 @@ config INPUT_M68K_BEEP
tristate "M68k Beeper support"
depends on M68K
+config INPUT_MAX7360_ROTARY
+ tristate "Maxim MAX7360 Rotary Encoder"
+ depends on I2C
+ 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 4f7f736831ba..0ed447543e43 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..c6c7deb309c2
--- /dev/null
+++ b/drivers/input/misc/max7360-rotary.c
@@ -0,0 +1,194 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2024 Bootlin
+ *
+ * Author: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
+ */
+
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/max7360.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_wakeirq.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+struct max7360_rotary {
+ u32 axis;
+ struct input_dev *input;
+ unsigned int debounce_ms;
+ struct regmap *regmap;
+};
+
+static irqreturn_t max7360_rotary_irq(int irq, void *data)
+{
+ struct max7360_rotary *max7360_rotary = data;
+ int val;
+ int ret;
+
+ ret = regmap_read(max7360_rotary->regmap, MAX7360_REG_RTR_CNT, &val);
+ if (ret < 0) {
+ dev_err(&max7360_rotary->input->dev,
+ "Failed to read rotary counter");
+ return IRQ_NONE;
+ }
+
+ if (val == 0) {
+ dev_dbg(&max7360_rotary->input->dev,
+ "Got a spurious interrupt");
+
+ return IRQ_NONE;
+ }
+
+ input_report_rel(max7360_rotary->input, max7360_rotary->axis,
+ (int8_t)val);
+ input_sync(max7360_rotary->input);
+
+ return IRQ_HANDLED;
+}
+
+static int max7360_rotary_hw_init(struct max7360_rotary *max7360_rotary)
+{
+ int val;
+ int ret;
+
+ ret = regmap_write_bits(max7360_rotary->regmap, MAX7360_REG_GPIOCFG,
+ MAX7360_GPIO_CFG_RTR_EN,
+ MAX7360_GPIO_CFG_RTR_EN);
+ if (ret) {
+ dev_err(&max7360_rotary->input->dev,
+ "Failed to enable max7360 rotary encoder");
+ return 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");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int max7360_rotary_probe(struct platform_device *pdev)
+{
+ struct max7360_rotary *max7360_rotary;
+ struct input_dev *input;
+ unsigned long flags;
+ int irq;
+ int ret;
+
+ if (!pdev->dev.parent)
+ return dev_err_probe(&pdev->dev, -ENODEV, "No parent device\n");
+
+ ret = max7360_port_pin_request(pdev->dev.parent, MAX7360_PORT_RTR_PIN,
+ true);
+ if (ret)
+ dev_err_probe(&pdev->dev, ret,
+ "Could not request rotary pin\n");
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return irq;
+
+ max7360_rotary = devm_kzalloc(&pdev->dev, sizeof(*max7360_rotary),
+ GFP_KERNEL);
+ if (!max7360_rotary)
+ return -ENOMEM;
+
+ max7360_rotary->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+ if (!max7360_rotary->regmap)
+ dev_err_probe(&pdev->dev, -ENODEV,
+ "Could not get parent regmap\n");
+
+ device_property_read_u32(&pdev->dev, "linux,axis",
+ &max7360_rotary->axis);
+ device_property_read_u32(&pdev->dev, "debounce-delay-ms",
+ &max7360_rotary->debounce_ms);
+ if (max7360_rotary->debounce_ms > MAX7360_ROT_DEBOUNCE_MAX)
+ return dev_err_probe(&pdev->dev, -EINVAL,
+ "Invalid debounce timing: %u\n",
+ max7360_rotary->debounce_ms);
+
+ input = devm_input_allocate_device(&pdev->dev);
+ if (!input)
+ return dev_err_probe(&pdev->dev, -ENOMEM,
+ "Failed to allocate input device\n");
+
+ max7360_rotary->input = input;
+
+ input->id.bustype = BUS_I2C;
+ input->name = pdev->name;
+ input->dev.parent = &pdev->dev;
+
+ input_set_capability(input, EV_REL, max7360_rotary->axis);
+ input_set_drvdata(input, max7360_rotary);
+
+ flags = IRQF_TRIGGER_LOW | IRQF_ONESHOT | IRQF_SHARED;
+ ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+ max7360_rotary_irq, flags,
+ "max7360-rotary", max7360_rotary);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "Failed to register interrupt: %d\n", ret);
+
+ ret = input_register_device(input);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "Could not register input device: %d\n",
+ ret);
+
+ platform_set_drvdata(pdev, max7360_rotary);
+
+ device_init_wakeup(&pdev->dev, true);
+ ret = dev_pm_set_wake_irq(&pdev->dev, irq);
+ if (ret)
+ dev_warn(&pdev->dev, "Failed to set up wakeup irq: %d\n", ret);
+
+ ret = max7360_rotary_hw_init(max7360_rotary);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "Failed to initialize max7360 rotary\n");
+
+ return 0;
+}
+
+static void max7360_rotary_remove(struct platform_device *pdev)
+{
+ dev_pm_clear_wake_irq(&pdev->dev);
+
+ /*
+ * Free the MAX7360_PORT_RTR_PIN pin, so it can be requested later by
+ * this driver, the MAX7360 GPIO driver or the MAX7360 PWM driver.
+ */
+ max7360_port_pin_request(pdev->dev.parent, MAX7360_PORT_RTR_PIN, false);
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id max7360_rotary_of_match[] = {
+ { .compatible = "maxim,max7360-rotary", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, max7360_rotary_of_match);
+#endif
+
+static struct platform_driver max7360_rotary_driver = {
+ .driver = {
+ .name = "max7360-rotary",
+ .of_match_table = of_match_ptr(max7360_rotary_of_match),
+ },
+ .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_ALIAS("platform:max7360-rotary");
+MODULE_LICENSE("GPL");
--
2.39.5
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 8/8] MAINTAINERS: Add entry on MAX7360 driver
2024-12-19 16:21 [PATCH 0/8] Add support for MAX7360 multifunction device Mathieu Dubois-Briand
` (6 preceding siblings ...)
2024-12-19 16:21 ` [PATCH 7/8] input: misc: Add support for MAX7360 rotary Mathieu Dubois-Briand
@ 2024-12-19 16:21 ` Mathieu Dubois-Briand
7 siblings, 0 replies; 25+ messages in thread
From: Mathieu Dubois-Briand @ 2024-12-19 16:21 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Kamel Bouhara, Linus Walleij, Bartosz Golaszewski,
Dmitry Torokhov, Uwe Kleine-König
Cc: devicetree, linux-kernel, linux-gpio, linux-input, linux-pwm,
Grégory Clement, Thomas Petazzoni, 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 | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index baf0eeb9a355..6e9b8caebb38 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14132,6 +14132,18 @@ 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/*/max7360-*.yaml
+F: Documentation/devicetree/bindings/mfd/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/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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 6/8] input: keyboard: Add support for MAX7360 keypad
2024-12-19 16:21 ` [PATCH 6/8] input: keyboard: Add support for MAX7360 keypad Mathieu Dubois-Briand
@ 2024-12-19 19:35 ` Dmitry Torokhov
2024-12-23 15:38 ` Mathieu Dubois-Briand
2024-12-20 17:12 ` kernel test robot
1 sibling, 1 reply; 25+ messages in thread
From: Dmitry Torokhov @ 2024-12-19 19:35 UTC (permalink / raw)
To: Mathieu Dubois-Briand
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Kamel Bouhara, Linus Walleij, Bartosz Golaszewski,
Uwe Kleine-König, devicetree, linux-kernel, linux-gpio,
linux-input, linux-pwm, Grégory Clement, Thomas Petazzoni
Hi Mathieu,
On Thu, Dec 19, 2024 at 05:21:23PM +0100, Mathieu Dubois-Briand wrote:
> 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 | 297 ++++++++++++++++++++++++++++++++
> 3 files changed, 310 insertions(+)
>
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 721ab69e84ac..bba029f65cfa 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..fbc51c89dba1
> --- /dev/null
> +++ b/drivers/input/keyboard/max7360-keypad.c
> @@ -0,0 +1,297 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2024 Bootlin
> + *
> + * Author: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> + */
> +
> +#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/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_wakeirq.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +struct max7360_keypad {
> + struct input_dev *input;
> + unsigned int rows;
> + unsigned int cols;
> + unsigned int debounce_ms;
> + int irq;
> + bool no_autorepeat;
> + 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;
> +
> + ret = regmap_read(max7360_keypad->regmap, MAX7360_REG_KEYFIFO, &val);
> + if (!ret && val == MAX7360_FIFO_OVERFLOW) {
> + /* FIFO overflow: ignore it and get next event. */
> + dev_err(&max7360_keypad->input->dev, "max7360 FIFO overflow");
> + ret = regmap_read(max7360_keypad->regmap, MAX7360_REG_KEYFIFO,
> + &val);
> + }
Maybe
do {
error = regmap_read(...);
if (error) {
dev_err(...);
return IRQ_HANDLED;
}
} while (val == MAX7360_FIFO_OVERFLOW);
> + if (ret) {
> + dev_err(&max7360_keypad->input->dev,
> + "Failed to read max7360 FIFO");
> + return ret;
Wrong value for irqreturn_t.
> + }
> +
> + 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;
> +
> + /*
> + * 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");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void max7360_keypad_close(struct input_dev *pdev)
> +{
> + struct max7360_keypad *max7360_keypad = input_get_drvdata(pdev);
> + int ret;
int error;
> +
> + /*
> + * Nobody is using the device anymore: go to 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");
> + }
No need for braces here.
> +}
> +
> +static int max7360_keypad_hw_init(struct max7360_keypad *max7360_keypad)
> +{
> + unsigned int val;
> + int ret;
int error;
> +
> + 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) {
> + dev_err(&max7360_keypad->input->dev,
> + "Failed to write max7360 debounce configuration");
> + return ret;
> + }
> +
> + ret = regmap_write_bits(max7360_keypad->regmap, MAX7360_REG_INTERRUPT,
> + MAX7360_INTERRUPT_TIME_MASK,
> + FIELD_PREP(MAX7360_INTERRUPT_TIME_MASK, 1));
> + if (ret) {
> + dev_err(&max7360_keypad->input->dev,
> + "Failed to write max7360 keypad interrupt configuration");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int max7360_keypad_parse_dt(struct platform_device *pdev,
> + struct max7360_keypad *max7360_keypad)
> +{
> + bool no_autorepeat;
> + int ret;
> +
> + ret = matrix_keypad_parse_properties(&pdev->dev, &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(&pdev->dev,
> + "Invalid number of columns or rows (%ux%u)\n",
> + max7360_keypad->cols, max7360_keypad->rows);
> + return -EINVAL;
> + }
> +
> + max7360_keypad->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> + if (!max7360_keypad->regmap) {
> + dev_err(&pdev->dev, "Could not get parent regmap\n");
> + return -ENODEV;
> + }
What this has to do with parsing DT?
> +
> + no_autorepeat = of_property_read_bool(pdev->dev.of_node,
> + "linux,input-no-autorepeat");
This is not a standard property, please using positive "autorepeat".
> + max7360_keypad->no_autorepeat = no_autorepeat;
I do not think you need to store this in driver data, just check and
immediately set EV_REP bit.
> +
> + max7360_keypad->debounce_ms = MAX7360_DEBOUNCE_MIN;
> + ret = of_property_read_u32(pdev->dev.of_node, "debounce-delay-ms",
> + &max7360_keypad->debounce_ms);
Please use generic device properties:
error = device_property_read_u32(...);
> + if (ret == -EINVAL) {
> + dev_info(&pdev->dev, "Using default debounce-delay-ms: %u\n",
> + max7360_keypad->debounce_ms);
> + } else if (ret < 0) {
> + dev_err(&pdev->dev, "Failed to read debounce-delay-ms property\n");
> + return ret;
> + } else if (max7360_keypad->debounce_ms < MAX7360_DEBOUNCE_MIN ||
> + max7360_keypad->debounce_ms > MAX7360_DEBOUNCE_MAX) {
> + dev_err(&pdev->dev,
> + "Invalid 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 input_dev *input;
> + unsigned long flags;
> + int irq;
> + int ret;
int error;
> +
> + if (!pdev->dev.parent)
> + return dev_err_probe(&pdev->dev, -ENODEV, "No parent device\n");
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0)
> + return irq;
> +
> + max7360_keypad = devm_kzalloc(&pdev->dev, sizeof(*max7360_keypad),
> + GFP_KERNEL);
> + if (!max7360_keypad)
> + return -ENOMEM;
> +
> + ret = max7360_keypad_parse_dt(pdev, max7360_keypad);
> + if (ret)
> + return ret;
> +
> + input = devm_input_allocate_device(&pdev->dev);
> + if (!input)
> + return dev_err_probe(&pdev->dev, -ENOMEM,
> + "Failed to allocate input device\n");
> +
> + max7360_keypad->input = input;
> +
> + input->id.bustype = BUS_I2C;
> + input->name = pdev->name;
> + input->dev.parent = &pdev->dev;
This is done by devm_input_allocate_device(), no need to repeat.
> +
> + input->open = max7360_keypad_open;
> + input->close = max7360_keypad_close;
> +
> + ret = matrix_keypad_build_keymap(NULL, NULL,
> + MAX7360_MAX_KEY_ROWS,
> + MAX7360_MAX_KEY_COLS,
> + max7360_keypad->keycodes, input);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "Failed to build keymap\n");
> +
> + input_set_capability(input, EV_MSC, MSC_SCAN);
> + if (!max7360_keypad->no_autorepeat)
> + __set_bit(EV_REP, input->evbit);
> +
> + input_set_drvdata(input, max7360_keypad);
> +
> + flags = IRQF_TRIGGER_LOW | IRQF_ONESHOT | IRQF_SHARED;
Shared? Why? And why do you need a temp variable?
> + ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> + max7360_keypad_irq, flags,
> + "max7360-keypad", max7360_keypad);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "Failed to register interrupt: %d\n", ret);
> +
> + ret = input_register_device(input);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "Could not register input device: %d\n",
> + ret);
> +
> + platform_set_drvdata(pdev, max7360_keypad);
> +
> + device_init_wakeup(&pdev->dev, true);
> + ret = dev_pm_set_wake_irq(&pdev->dev, irq);
> + if (ret)
> + dev_warn(&pdev->dev, "Failed to set up wakeup irq: %d\n", ret);
> +
> + ret = max7360_keypad_hw_init(max7360_keypad);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "Failed to initialize max7360 keypad\n");
You need to clear wake irq here. Or move setting it to after HW init.
> +
> + return 0;
> +}
> +
> +static void max7360_keypad_remove(struct platform_device *pdev)
> +{
> + dev_pm_clear_wake_irq(&pdev->dev);
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id max7360_keypad_of_match[] = {
> + { .compatible = "maxim,max7360-keypad", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, max7360_keypad_of_match);
> +#endif
> +
> +static struct platform_driver max7360_keypad_driver = {
> + .driver = {
> + .name = "max7360-keypad",
> + .of_match_table = of_match_ptr(max7360_keypad_of_match),
> + },
> + .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_ALIAS("platform:max7360-keypad");
> +MODULE_LICENSE("GPL");
>
> --
> 2.39.5
>
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/8] pwm: max7360: Add MAX7360 PWM support
2024-12-19 16:21 ` [PATCH 4/8] pwm: max7360: Add MAX7360 PWM support mathieu.dubois-briand
@ 2024-12-19 21:53 ` Uwe Kleine-König
2024-12-23 15:26 ` Mathieu Dubois-Briand
2024-12-20 5:51 ` Uwe Kleine-König
2024-12-21 20:35 ` Krzysztof Kozlowski
2 siblings, 1 reply; 25+ messages in thread
From: Uwe Kleine-König @ 2024-12-19 21:53 UTC (permalink / raw)
To: mathieu.dubois-briand
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Kamel Bouhara, Linus Walleij, Bartosz Golaszewski,
Dmitry Torokhov, devicetree, linux-kernel, linux-gpio,
linux-input, linux-pwm, Grégory Clement, Thomas Petazzoni
[-- Attachment #1: Type: text/plain, Size: 7526 bytes --]
Hello,
On Thu, Dec 19, 2024 at 05:21:21PM +0100, 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.
>
> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
> Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> ---
> drivers/pwm/Kconfig | 11 +++
> drivers/pwm/Makefile | 1 +
> drivers/pwm/pwm-max7360.c | 173 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 185 insertions(+)
>
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index 0915c1e7df16..399dc3f76e92 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -745,4 +745,15 @@ 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
> + depends on OF_GPIO
> + 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 9081e0c0e9e0..ae8908ffc892 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..b1cde3e86864
> --- /dev/null
> +++ b/drivers/pwm/pwm-max7360.c
> @@ -0,0 +1,173 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2024 Bootlin
> + *
> + * Author: Kamel BOUHARA <kamel.bouhara@bootlin.com>
> + */
> +#include <linux/math.h>
> +#include <linux/mfd/max7360.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pwm.h>
> +#include <linux/regmap.h>
> +
> +#define MAX7360_NUM_PWMS 8
> +#define MAX7360_PWM_MAX_RES 256
> +#define MAX7360_PWM_PERIOD_NS 2000000 /* 500 Hz */
> +#define MAX7360_PWM_COMMON_PWN BIT(5)
> +#define MAX7360_PWM_CTRL_ENABLE(n) BIT(n)
> +#define MAX7360_PWM_PORT(n) BIT(n)
> +
> +struct max7360_pwm {
> + struct device *dev;
> + struct regmap *regmap;
> +};
> +
> +static inline struct max7360_pwm *to_max7360_pwm(struct pwm_chip *chip)
> +{
> + return pwmchip_get_drvdata(chip);
> +}
> +
> +static int max7360_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> + struct max7360_pwm *max7360_pwm;
> + int ret;
> +
> + max7360_pwm = to_max7360_pwm(chip);
> + ret = max7360_port_pin_request(max7360_pwm->dev->parent, pwm->hwpwm,
> + true);
> + if (ret) {
> + dev_err(&chip->dev, "failed to request pwm-%d\n", pwm->hwpwm);
Please no error messages after probe.
> + return ret;
> + }
> +
> + ret = regmap_write_bits(max7360_pwm->regmap,
> + MAX7360_REG_PWMCFG + pwm->hwpwm,
> + MAX7360_PWM_COMMON_PWN,
> + 0);
> + if (ret) {
> + dev_err(&chip->dev, "failed to write pwm-%d cfg register, error %d\n",
> + pwm->hwpwm, ret);
> + return ret;
> + }
> +
> + ret = regmap_write_bits(max7360_pwm->regmap, MAX7360_REG_PORTS,
> + MAX7360_PWM_PORT(pwm->hwpwm),
> + MAX7360_PWM_PORT(pwm->hwpwm));
> + if (ret) {
> + dev_err(&chip->dev, "failed to write pwm-%d ports register, error %d\n",
> + pwm->hwpwm, ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void max7360_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> + struct max7360_pwm *max7360_pwm;
> + int ret;
> +
> + max7360_pwm = to_max7360_pwm(chip);
> + ret = regmap_write_bits(max7360_pwm->regmap, MAX7360_REG_GPIOCTRL,
> + MAX7360_PWM_CTRL_ENABLE(pwm->hwpwm),
> + 0);
> + if (ret)
> + dev_err(&chip->dev, "failed to enable pwm-%d , error %d\n",
> + pwm->hwpwm, ret);
> +
> + max7360_port_pin_request(max7360_pwm->dev->parent, pwm->hwpwm,
> + false);
> +}
> +
> +static int max7360_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> + const struct pwm_state *state)
> +{
> + struct max7360_pwm *max7360_pwm;
> + u64 duty_steps = state->duty_cycle * MAX7360_PWM_MAX_RES;
This might overflow.
> + int ret;
> +
> + if (state->polarity != PWM_POLARITY_NORMAL)
> + return -EINVAL;
> +
> + max7360_pwm = to_max7360_pwm(chip);
> + ret = regmap_write_bits(max7360_pwm->regmap, MAX7360_REG_GPIOCTRL,
> + MAX7360_PWM_CTRL_ENABLE(pwm->hwpwm),
> + MAX7360_PWM_CTRL_ENABLE(pwm->hwpwm));
> + if (ret) {
> + dev_err(&chip->dev, "failed to enable pwm-%d , error %d\n",
> + pwm->hwpwm, ret);
> + return ret;
> + }
> +
> + do_div(duty_steps, MAX7360_PWM_PERIOD_NS);
> +
> + ret = regmap_write(max7360_pwm->regmap, MAX7360_REG_PWMBASE + pwm->hwpwm,
> + duty_steps >= 255 ? 255 : duty_steps);
> + if (ret) {
> + dev_err(&chip->dev,
> + "failed to apply pwm duty_cycle %llu on pwm-%d, error %d\n",
> + duty_steps, pwm->hwpwm, ret);
> + return ret;
> + }
Huh, state->period isn't used at all. That is wrong for sure.
> +
> + return 0;
> +}
> +
> +static const struct pwm_ops max7360_pwm_ops = {
> + .request = max7360_pwm_request,
> + .free = max7360_pwm_free,
> + .apply = max7360_pwm_apply,
Please implement .get_state() (or the new waveform callbacks)
> +};
> +
> +static int max7360_pwm_probe(struct platform_device *pdev)
> +{
> + struct max7360_pwm *max7360_pwm;
> + struct pwm_chip *chip;
> +
> + if (!pdev->dev.parent) {
> + dev_err(&pdev->dev, "no parent device\n");
> + return -ENODEV;
return dev_err_probe(...);
> + }
> +
> + chip = devm_pwmchip_alloc(&pdev->dev, MAX7360_NUM_PWMS,
> + sizeof(*max7360_pwm));
> + if (IS_ERR(chip))
> + return PTR_ERR(chip);
> + chip->ops = &max7360_pwm_ops;
> +
> + max7360_pwm = to_max7360_pwm(chip);
> + max7360_pwm->dev = &pdev->dev;
This is only ever used to get max7360_pwm->dev->parent; so better store
the parent directly in max7360_pwm.
> +
> + max7360_pwm->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> + if (!max7360_pwm->regmap) {
> + dev_err(&pdev->dev, "could not get parent regmap\n");
> + return -ENODEV;
> + }
> +
> + return devm_pwmchip_add(&pdev->dev, chip);
error message please
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id max7360_pwm_of_match[] = {
> + { .compatible = "maxim,max7360-pwm", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, max7360_pwm_of_match);
> +#endif
Please drop the CONFIG_OF guard.
> +
> +static struct platform_driver max7360_pwm_driver = {
> + .driver = {
> + .name = "max7360-pwm",
> + .of_match_table = of_match_ptr(max7360_pwm_of_match),
> + },
> + .probe = max7360_pwm_probe,
> +};
> +module_platform_driver(max7360_pwm_driver);
> +
> +MODULE_DESCRIPTION("MAX7360 PWM driver");
> +MODULE_AUTHOR("Kamel BOUHARA <kamel.bouhara@bootlin.com>");
> +MODULE_ALIAS("platform:max7360-pwm");
> +MODULE_LICENSE("GPL");
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/8] dt-bindings: Add MAX7360 subdevices
2024-12-19 16:21 ` [PATCH 2/8] dt-bindings: Add MAX7360 subdevices Mathieu Dubois-Briand
@ 2024-12-19 21:54 ` Uwe Kleine-König
2024-12-21 20:34 ` Krzysztof Kozlowski
1 sibling, 0 replies; 25+ messages in thread
From: Uwe Kleine-König @ 2024-12-19 21:54 UTC (permalink / raw)
To: Mathieu Dubois-Briand
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Kamel Bouhara, Linus Walleij, Bartosz Golaszewski,
Dmitry Torokhov, devicetree, linux-kernel, linux-gpio,
linux-input, linux-pwm, Grégory Clement, Thomas Petazzoni
[-- Attachment #1: Type: text/plain, Size: 1143 bytes --]
Hello,
On Thu, Dec 19, 2024 at 05:21:19PM +0100, Mathieu Dubois-Briand wrote:
> diff --git a/Documentation/devicetree/bindings/pwm/max7360-pwm.yaml b/Documentation/devicetree/bindings/pwm/max7360-pwm.yaml
> new file mode 100644
> index 000000000000..68d48969e542
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/max7360-pwm.yaml
> @@ -0,0 +1,35 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pwm/max7360-pwm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Maxim MAX7360 PWM controller
> +
> +maintainers:
> + - Kamel Bouhara <kamel.bouhara@bootlin.com>
> +
> +description: |
> + Maxim MAX7360 PWM controller, in MAX7360 MFD
> + https://www.analog.com/en/products/max7360.html
> +
> +properties:
> + compatible:
> + enum:
> + - maxim,max7360-pwm
> +
> + "#pwm-cells":
> + const: 2
Please make this 3.
> +required:
> + - compatible
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + max7360_pwm: max7360_pwm {
> + compatible = "maxim,max7360-pwm";
> + #pwm-cells = <2>;
> + };
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/8] pwm: max7360: Add MAX7360 PWM support
2024-12-19 16:21 ` [PATCH 4/8] pwm: max7360: Add MAX7360 PWM support mathieu.dubois-briand
2024-12-19 21:53 ` Uwe Kleine-König
@ 2024-12-20 5:51 ` Uwe Kleine-König
2024-12-21 20:35 ` Krzysztof Kozlowski
2 siblings, 0 replies; 25+ messages in thread
From: Uwe Kleine-König @ 2024-12-20 5:51 UTC (permalink / raw)
To: mathieu.dubois-briand
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Kamel Bouhara, Linus Walleij, Bartosz Golaszewski,
Dmitry Torokhov, devicetree, linux-kernel, linux-gpio,
linux-input, linux-pwm, Grégory Clement, Thomas Petazzoni
[-- Attachment #1: Type: text/plain, Size: 852 bytes --]
Hello again,
On Thu, Dec 19, 2024 at 05:21:21PM +0100, mathieu.dubois-briand@bootlin.com wrote:
> diff --git a/drivers/pwm/pwm-max7360.c b/drivers/pwm/pwm-max7360.c
> new file mode 100644
> index 000000000000..b1cde3e86864
> --- /dev/null
> +++ b/drivers/pwm/pwm-max7360.c
> @@ -0,0 +1,173 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2024 Bootlin
> + *
I forgot: Please also add a paragraph here describing the relevant
features here in the format that several other drivers use. Such that
sed -rn '/Limitations:/,/\*\/?$/p' drivers/pwm/*.c
emits it. Relevant are at least:
- Only supports normal polarity
- How does it behave on disable? (Constant low? Freeze at the level
where it just happens to be? High-Z?)
- How does it behave on reconfiguration? (counter reset? Mixed output
possible?
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/8] gpio: max7360: Add MAX7360 gpio support
2024-12-19 16:21 ` [PATCH 5/8] gpio: max7360: Add MAX7360 gpio support Mathieu Dubois-Briand
@ 2024-12-20 15:54 ` kernel test robot
2024-12-20 16:39 ` kernel test robot
1 sibling, 0 replies; 25+ messages in thread
From: kernel test robot @ 2024-12-20 15:54 UTC (permalink / raw)
To: Mathieu Dubois-Briand, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Kamel Bouhara, Linus Walleij,
Bartosz Golaszewski, Dmitry Torokhov, Uwe Kleine-König
Cc: llvm, oe-kbuild-all, devicetree, linux-kernel, linux-gpio,
linux-input, linux-pwm, Grégory Clement, Thomas Petazzoni,
Mathieu Dubois-Briand
Hi Mathieu,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8]
url: https://github.com/intel-lab-lkp/linux/commits/Mathieu-Dubois-Briand/dt-bindings-Add-MAX7360-MFD-device/20241220-002541
base: 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8
patch link: https://lore.kernel.org/r/20241219-mdb-max7360-support-v1-5-8e8317584121%40bootlin.com
patch subject: [PATCH 5/8] gpio: max7360: Add MAX7360 gpio support
config: arm64-allmodconfig (https://download.01.org/0day-ci/archive/20241220/202412202337.8jOygMaK-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241220/202412202337.8jOygMaK-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412202337.8jOygMaK-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/gpio/gpio-max7360.c:57:41: warning: variable 'val' is uninitialized when used here [-Wuninitialized]
57 | "failed to set value %d on gpio-%d", val, pin);
| ^~~
include/linux/dev_printk.h:154:65: note: expanded from macro 'dev_err'
154 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~
include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~~~~~~~~~
drivers/gpio/gpio-max7360.c:42:18: note: initialize the variable 'val' to silence this warning
42 | unsigned int val;
| ^
| = 0
>> drivers/gpio/gpio-max7360.c:370:32: warning: cast to smaller integer type 'int' from 'const void *' [-Wvoid-pointer-to-int-cast]
370 | max7360_gpio->gpio_function = (int)device_get_match_data(&pdev->dev);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 warnings generated.
vim +370 drivers/gpio/gpio-max7360.c
333
334 static int max7360_gpio_probe(struct platform_device *pdev)
335 {
336 struct max7360_gpio *max7360_gpio;
337 unsigned int ngpios;
338 unsigned int outconf;
339 struct gpio_irq_chip *girq;
340 unsigned long flags;
341 int irq;
342 int ret;
343
344 if (!pdev->dev.parent) {
345 dev_err(&pdev->dev, "no parent device\n");
346 return -ENODEV;
347 }
348
349 max7360_gpio = devm_kzalloc(&pdev->dev, sizeof(struct max7360_gpio),
350 GFP_KERNEL);
351 if (!max7360_gpio)
352 return -ENOMEM;
353
354 if (of_property_read_u32(pdev->dev.of_node, "ngpios", &ngpios)) {
355 dev_err(&pdev->dev, "Missing ngpios OF property\n");
356 return -ENODEV;
357 }
358
359 max7360_gpio->regmap = dev_get_regmap(pdev->dev.parent, NULL);
360 if (!max7360_gpio->regmap) {
361 dev_err(&pdev->dev, "could not get parent regmap\n");
362 return -ENODEV;
363 }
364
365 max7360_gpio->dev = &pdev->dev;
366 max7360_gpio->chip = max7360_gpio_chip;
367 max7360_gpio->chip.ngpio = ngpios;
368 max7360_gpio->chip.parent = &pdev->dev;
369 max7360_gpio->chip.base = -1;
> 370 max7360_gpio->gpio_function = (int)device_get_match_data(&pdev->dev);
371
372 dev_dbg(&pdev->dev, "gpio count: %d\n", max7360_gpio->chip.ngpio);
373
374 if (max7360_gpio->gpio_function == MAX7360_GPIO_PORT) {
375 /* Port GPIOs: set output mode configuration (constant-current
376 * or not).
377 * This property is optional.
378 */
379 outconf = 0;
380 ret = of_property_read_u32(pdev->dev.of_node,
381 "constant-current-disable", &outconf);
382 if (ret && (ret != -EINVAL)) {
383 dev_err(&pdev->dev,
384 "Failed to read constant-current-disable OF property\n");
385 return -ENODEV;
386 }
387
388 regmap_write(max7360_gpio->regmap, MAX7360_REG_GPIOOUTM, outconf);
389 }
390
391 if (max7360_gpio->gpio_function == MAX7360_GPIO_PORT &&
392 of_property_read_bool(pdev->dev.of_node, "interrupt-controller")) {
393 /* Port GPIOs: declare IRQ chip, if configuration was provided.
394 */
395 irq = platform_get_irq(pdev, 0);
396 if (irq < 0)
397 return dev_err_probe(&pdev->dev, irq,
398 "Failed to get IRQ");
399
400 flags = IRQF_TRIGGER_LOW | IRQF_ONESHOT | IRQF_SHARED;
401 ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
402 max7360_gpio_irq, flags,
403 "max7360-gpio", max7360_gpio);
404 if (ret)
405 return dev_err_probe(&pdev->dev, ret,
406 "Failed to register interrupt: %d\n",
407 ret);
408
409 girq = &max7360_gpio->chip.irq;
410 gpio_irq_chip_set_chip(girq, &max7360_gpio_irqchip);
411 girq->parent_handler = NULL;
412 girq->num_parents = 0;
413 girq->parents = NULL;
414 girq->threaded = true;
415 girq->default_type = IRQ_TYPE_NONE;
416 girq->handler = handle_simple_irq;
417 }
418
419 ret = devm_gpiochip_add_data(&pdev->dev, &max7360_gpio->chip, max7360_gpio);
420 if (ret) {
421 dev_err(&pdev->dev, "unable to add gpiochip: %d\n", ret);
422 return ret;
423 }
424
425 return 0;
426 }
427
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/8] gpio: max7360: Add MAX7360 gpio support
2024-12-19 16:21 ` [PATCH 5/8] gpio: max7360: Add MAX7360 gpio support Mathieu Dubois-Briand
2024-12-20 15:54 ` kernel test robot
@ 2024-12-20 16:39 ` kernel test robot
1 sibling, 0 replies; 25+ messages in thread
From: kernel test robot @ 2024-12-20 16:39 UTC (permalink / raw)
To: Mathieu Dubois-Briand, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Kamel Bouhara, Linus Walleij,
Bartosz Golaszewski, Dmitry Torokhov, Uwe Kleine-König
Cc: oe-kbuild-all, devicetree, linux-kernel, linux-gpio, linux-input,
linux-pwm, Grégory Clement, Thomas Petazzoni,
Mathieu Dubois-Briand
Hi Mathieu,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8]
url: https://github.com/intel-lab-lkp/linux/commits/Mathieu-Dubois-Briand/dt-bindings-Add-MAX7360-MFD-device/20241220-002541
base: 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8
patch link: https://lore.kernel.org/r/20241219-mdb-max7360-support-v1-5-8e8317584121%40bootlin.com
patch subject: [PATCH 5/8] gpio: max7360: Add MAX7360 gpio support
config: loongarch-allyesconfig (https://download.01.org/0day-ci/archive/20241221/202412210008.Saks0Eu4-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241221/202412210008.Saks0Eu4-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412210008.Saks0Eu4-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/gpio/gpio-max7360.c: In function 'max7360_gpio_probe':
>> drivers/gpio/gpio-max7360.c:370:39: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
370 | max7360_gpio->gpio_function = (int)device_get_match_data(&pdev->dev);
| ^
vim +370 drivers/gpio/gpio-max7360.c
333
334 static int max7360_gpio_probe(struct platform_device *pdev)
335 {
336 struct max7360_gpio *max7360_gpio;
337 unsigned int ngpios;
338 unsigned int outconf;
339 struct gpio_irq_chip *girq;
340 unsigned long flags;
341 int irq;
342 int ret;
343
344 if (!pdev->dev.parent) {
345 dev_err(&pdev->dev, "no parent device\n");
346 return -ENODEV;
347 }
348
349 max7360_gpio = devm_kzalloc(&pdev->dev, sizeof(struct max7360_gpio),
350 GFP_KERNEL);
351 if (!max7360_gpio)
352 return -ENOMEM;
353
354 if (of_property_read_u32(pdev->dev.of_node, "ngpios", &ngpios)) {
355 dev_err(&pdev->dev, "Missing ngpios OF property\n");
356 return -ENODEV;
357 }
358
359 max7360_gpio->regmap = dev_get_regmap(pdev->dev.parent, NULL);
360 if (!max7360_gpio->regmap) {
361 dev_err(&pdev->dev, "could not get parent regmap\n");
362 return -ENODEV;
363 }
364
365 max7360_gpio->dev = &pdev->dev;
366 max7360_gpio->chip = max7360_gpio_chip;
367 max7360_gpio->chip.ngpio = ngpios;
368 max7360_gpio->chip.parent = &pdev->dev;
369 max7360_gpio->chip.base = -1;
> 370 max7360_gpio->gpio_function = (int)device_get_match_data(&pdev->dev);
371
372 dev_dbg(&pdev->dev, "gpio count: %d\n", max7360_gpio->chip.ngpio);
373
374 if (max7360_gpio->gpio_function == MAX7360_GPIO_PORT) {
375 /* Port GPIOs: set output mode configuration (constant-current
376 * or not).
377 * This property is optional.
378 */
379 outconf = 0;
380 ret = of_property_read_u32(pdev->dev.of_node,
381 "constant-current-disable", &outconf);
382 if (ret && (ret != -EINVAL)) {
383 dev_err(&pdev->dev,
384 "Failed to read constant-current-disable OF property\n");
385 return -ENODEV;
386 }
387
388 regmap_write(max7360_gpio->regmap, MAX7360_REG_GPIOOUTM, outconf);
389 }
390
391 if (max7360_gpio->gpio_function == MAX7360_GPIO_PORT &&
392 of_property_read_bool(pdev->dev.of_node, "interrupt-controller")) {
393 /* Port GPIOs: declare IRQ chip, if configuration was provided.
394 */
395 irq = platform_get_irq(pdev, 0);
396 if (irq < 0)
397 return dev_err_probe(&pdev->dev, irq,
398 "Failed to get IRQ");
399
400 flags = IRQF_TRIGGER_LOW | IRQF_ONESHOT | IRQF_SHARED;
401 ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
402 max7360_gpio_irq, flags,
403 "max7360-gpio", max7360_gpio);
404 if (ret)
405 return dev_err_probe(&pdev->dev, ret,
406 "Failed to register interrupt: %d\n",
407 ret);
408
409 girq = &max7360_gpio->chip.irq;
410 gpio_irq_chip_set_chip(girq, &max7360_gpio_irqchip);
411 girq->parent_handler = NULL;
412 girq->num_parents = 0;
413 girq->parents = NULL;
414 girq->threaded = true;
415 girq->default_type = IRQ_TYPE_NONE;
416 girq->handler = handle_simple_irq;
417 }
418
419 ret = devm_gpiochip_add_data(&pdev->dev, &max7360_gpio->chip, max7360_gpio);
420 if (ret) {
421 dev_err(&pdev->dev, "unable to add gpiochip: %d\n", ret);
422 return ret;
423 }
424
425 return 0;
426 }
427
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 6/8] input: keyboard: Add support for MAX7360 keypad
2024-12-19 16:21 ` [PATCH 6/8] input: keyboard: Add support for MAX7360 keypad Mathieu Dubois-Briand
2024-12-19 19:35 ` Dmitry Torokhov
@ 2024-12-20 17:12 ` kernel test robot
1 sibling, 0 replies; 25+ messages in thread
From: kernel test robot @ 2024-12-20 17:12 UTC (permalink / raw)
To: Mathieu Dubois-Briand, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Kamel Bouhara, Linus Walleij,
Bartosz Golaszewski, Dmitry Torokhov, Uwe Kleine-König
Cc: llvm, oe-kbuild-all, devicetree, linux-kernel, linux-gpio,
linux-input, linux-pwm, Grégory Clement, Thomas Petazzoni,
Mathieu Dubois-Briand
Hi Mathieu,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8]
url: https://github.com/intel-lab-lkp/linux/commits/Mathieu-Dubois-Briand/dt-bindings-Add-MAX7360-MFD-device/20241220-002541
base: 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8
patch link: https://lore.kernel.org/r/20241219-mdb-max7360-support-v1-6-8e8317584121%40bootlin.com
patch subject: [PATCH 6/8] input: keyboard: Add support for MAX7360 keypad
config: arm64-allmodconfig (https://download.01.org/0day-ci/archive/20241221/202412210024.DavK6KEl-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241221/202412210024.DavK6KEl-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412210024.DavK6KEl-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/input/keyboard/max7360-keypad.c:105:24: warning: implicit conversion from 'unsigned long' to 'unsigned int' changes value from 18446744073709551487 to 4294967167 [-Wconstant-conversion]
104 | ret = regmap_write_bits(max7360_keypad->regmap, MAX7360_REG_CONFIG,
| ~~~~~~~~~~~~~~~~~
105 | MAX7360_CFG_SLEEP, ~MAX7360_CFG_SLEEP);
| ^~~~~~~~~~~~~~~~~~
1 warning generated.
vim +105 drivers/input/keyboard/max7360-keypad.c
95
96 static void max7360_keypad_close(struct input_dev *pdev)
97 {
98 struct max7360_keypad *max7360_keypad = input_get_drvdata(pdev);
99 int ret;
100
101 /*
102 * Nobody is using the device anymore: go to sleep.
103 */
104 ret = regmap_write_bits(max7360_keypad->regmap, MAX7360_REG_CONFIG,
> 105 MAX7360_CFG_SLEEP, ~MAX7360_CFG_SLEEP);
106 if (ret) {
107 dev_err(&max7360_keypad->input->dev,
108 "Failed to write max7360 configuration");
109 }
110 }
111
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/8] mfd: Add max7360 support
2024-12-19 16:21 ` [PATCH 3/8] mfd: Add max7360 support mathieu.dubois-briand
@ 2024-12-20 23:01 ` kernel test robot
0 siblings, 0 replies; 25+ messages in thread
From: kernel test robot @ 2024-12-20 23:01 UTC (permalink / raw)
To: mathieu.dubois-briand, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Kamel Bouhara, Linus Walleij,
Bartosz Golaszewski, Dmitry Torokhov, Uwe Kleine-König
Cc: oe-kbuild-all, devicetree, linux-kernel, linux-gpio, linux-input,
linux-pwm, Grégory Clement, Thomas Petazzoni,
Mathieu Dubois-Briand
Hi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8]
url: https://github.com/intel-lab-lkp/linux/commits/Mathieu-Dubois-Briand/dt-bindings-Add-MAX7360-MFD-device/20241220-002541
base: 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8
patch link: https://lore.kernel.org/r/20241219-mdb-max7360-support-v1-3-8e8317584121%40bootlin.com
patch subject: [PATCH 3/8] mfd: Add max7360 support
config: openrisc-randconfig-r122-20241220 (https://download.01.org/0day-ci/archive/20241221/202412210613.xoQvMKlk-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 14.2.0
reproduce: (https://download.01.org/0day-ci/archive/20241221/202412210613.xoQvMKlk-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412210613.xoQvMKlk-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/mfd/max7360.c:52:27: sparse: sparse: symbol 'max7360_volatile_ranges' was not declared. Should it be static?
vim +/max7360_volatile_ranges +52 drivers/mfd/max7360.c
51
> 52 const struct regmap_range max7360_volatile_ranges[] = {
53 {
54 .range_min = MAX7360_REG_KEYFIFO,
55 .range_max = MAX7360_REG_KEYFIFO,
56 }, {
57 .range_min = 0x48,
58 .range_max = 0x4a,
59 },
60 };
61
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/8] dt-bindings: Add MAX7360 MFD device
2024-12-19 16:21 ` [PATCH 1/8] dt-bindings: Add MAX7360 MFD device Mathieu Dubois-Briand
@ 2024-12-21 20:28 ` Krzysztof Kozlowski
0 siblings, 0 replies; 25+ messages in thread
From: Krzysztof Kozlowski @ 2024-12-21 20:28 UTC (permalink / raw)
To: Mathieu Dubois-Briand, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Kamel Bouhara, Linus Walleij,
Bartosz Golaszewski, Dmitry Torokhov, Uwe Kleine-König
Cc: devicetree, linux-kernel, linux-gpio, linux-input, linux-pwm,
Grégory Clement, Thomas Petazzoni
On 19/12/2024 17:21, Mathieu Dubois-Briand wrote:
> Add device tree bindings for Maxim Integrated MAX7360 MFD device with
> support for keypad, rotary, gpios and pwm functionalities.
Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
Subject/commit msg: drop MFD and explain what the hardware is. MFD is
Linuxism.
>
> Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> ---
> Documentation/devicetree/bindings/mfd/max7360.yaml | 56 ++++++++++++++++++++++
Use compatible as filename.
> 1 file changed, 56 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mfd/max7360.yaml b/Documentation/devicetree/bindings/mfd/max7360.yaml
> new file mode 100644
> index 000000000000..49dd437fd313
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/max7360.yaml
> @@ -0,0 +1,56 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/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 MFD device, with following functions:
> + - keypad controller
> + - rotary controller
> + - GPIO and GPO controller
> + - PWM controller
> +
> + https://www.analog.com/en/products/max7360.html
> +
> +properties:
> + compatible:
> + enum:
> + - maxim,max7360
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + description: The interrupt line the device is connected to.
Drop description,
> + maxItems: 1
I don't think this was tested at all. It is heavily incomplete,
considering this is sort of MFD device.
Or you split patches in odd way. Look how other PMIC-style things are
upstreamed.
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + max7360@38 {
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> + compatible = "maxim,max7360";
> + reg = <0x38>;
> +
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/8] dt-bindings: Add MAX7360 subdevices
2024-12-19 16:21 ` [PATCH 2/8] dt-bindings: Add MAX7360 subdevices Mathieu Dubois-Briand
2024-12-19 21:54 ` Uwe Kleine-König
@ 2024-12-21 20:34 ` Krzysztof Kozlowski
2024-12-23 15:20 ` Mathieu Dubois-Briand
1 sibling, 1 reply; 25+ messages in thread
From: Krzysztof Kozlowski @ 2024-12-21 20:34 UTC (permalink / raw)
To: Mathieu Dubois-Briand, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Kamel Bouhara, Linus Walleij,
Bartosz Golaszewski, Dmitry Torokhov, Uwe Kleine-König
Cc: devicetree, linux-kernel, linux-gpio, linux-input, linux-pwm,
Grégory Clement, Thomas Petazzoni
On 19/12/2024 17:21, Mathieu Dubois-Briand wrote:
> ---
> .../devicetree/bindings/gpio/max7360-gpio.yaml | 96 ++++++++++++++++++++++
> .../devicetree/bindings/input/max7360-keypad.yaml | 67 +++++++++++++++
> .../devicetree/bindings/input/max7360-rotary.yaml | 52 ++++++++++++
> .../devicetree/bindings/pwm/max7360-pwm.yaml | 35 ++++++++
> 4 files changed, 250 insertions(+)
I don't understand how this patchset was split. MFD binding cannot be
empty and cannot be before child devices.
All filenames are wrong here: use compatibles.
>
> diff --git a/Documentation/devicetree/bindings/gpio/max7360-gpio.yaml b/Documentation/devicetree/bindings/gpio/max7360-gpio.yaml
> new file mode 100644
> index 000000000000..3c006dc0380b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/max7360-gpio.yaml
> @@ -0,0 +1,96 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/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 MFD
> + https://www.analog.com/en/products/max7360.html
> +
> +properties:
> + compatible:
> + enum:
> + - maxim,max7360-gpio
> + - maxim,max7360-gpo
Why? What are the differences?
> +
> + gpio-controller: true
> +
> + "#gpio-cells":
> + const: 2
> +
> + ngpios:
> + minimum: 0
> + maximum: 8
Why this is flexible?
> +
> + interrupt-controller: true
> +
> + "#interrupt-cells":
> + const: 2
> +
> + interrupts:
> + description: The interrupt line the device is connected to.
Drop
> + maxItems: 1
> +
> + constant-current-disable:
You always need vendor prefix.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: Bit field, each bit disables constant-current output of the
> + associated GPIO.
Oddly aligned.
Missing constraints.
> +
> +
> +required:
> + - compatible
> + - gpio-controller
> + - ngpios
> +
> +if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - maxim,max7360-gpio
> +then:
> + required:
> + - interrupt-controller
> + - interrupts
> +else:
> + properties:
> + interrupt-controller: false
> + interrupts: false
> + constant-current-disable: false
> +
> + ngpios:
> + maximum: 6
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + max7360_gpo: max7360_gpo {
Plaese follow DTS coding style.
Keep only one, complete example.
> + compatible = "maxim,max7360-gpo";
> + gpio-controller;
> + #gpio-cells = <0x2>;
> + ngpios = <4>;
Odd indentation. Your MFD patch had very different one.
> + };
> +
> + max7360_gpio: max7360_gpio {
> + compatible = "maxim,max7360-gpio";
> +
> + gpio-controller;
> + #gpio-cells = <0x2>;
> + ngpios = <8>;
> + constant-current-disable = <0x06>;
> +
> + interrupt-controller;
> + #interrupt-cells = <0x2>;
> + interrupt-parent = <&gpio1>;
> + interrupts = <23 IRQ_TYPE_LEVEL_LOW>;
> + };
> diff --git a/Documentation/devicetree/bindings/input/max7360-keypad.yaml b/Documentation/devicetree/bindings/input/max7360-keypad.yaml
> new file mode 100644
> index 000000000000..8bc3c841465b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/max7360-keypad.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/max7360-keypad.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Maxim MAX7360 Keypad Controller
> +
> +maintainers:
> + - Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> +
> +description: |
> + Maxim MAX7360 Keypad Controller, in MAX7360 MFD
> + https://www.analog.com/en/products/max7360.html
> +
> +allOf:
> + - $ref: matrix-keymap.yaml#
> + - $ref: input.yaml#
> +
> +properties:
> + compatible:
> + enum:
> + - maxim,max7360-keypad
> +
> + interrupts:
> + description: The interrupt line the device is connected to.
Really? Each separate device has its own interrupt line? How is it
possible if diagram here:
https://www.analog.com/en/products/max7360.html
has only one interrupt?
Fold the binding into the parent node.
> + maxItems: 1
> +
> + debounce-delay-ms:
> + description: Debounce delay in ms
> + minimum: 9
> + maximum: 40
> + default: 9
> +
> + linux,input-no-autorepeat:
> + description: If present, the keys will not autorepeat when pressed
> +
> +required:
> + - compatible
> + - interrupts
> + - linux,keymap
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/input/input.h>
> +
> + max7360_keypad {
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
Please read and follow DTS coding style.
> + compatible = "maxim,max7360-keypad";
> +
> + 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)
> + >;
> +
> + interrupt-parent = <&gpio1>;
> + interrupts = <23 IRQ_TYPE_LEVEL_LOW>;
> +
> + debounce-delay-ms = <10>;
> + linux,input-no-autorepeat;
> + };
> diff --git a/Documentation/devicetree/bindings/input/max7360-rotary.yaml b/Documentation/devicetree/bindings/input/max7360-rotary.yaml
> new file mode 100644
> index 000000000000..19afa8344249
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/max7360-rotary.yaml
> @@ -0,0 +1,52 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/max7360-rotary.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Maxim MAX7360 Rotary Encoder
> +
> +maintainers:
> + - Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> +
> +description: |
> + Maxim MAX7360 Rotary Encoder, in MAX7360 MFD
> + https://www.analog.com/en/products/max7360.html
> +
> +properties:
> + compatible:
> + enum:
> + - maxim,max7360-rotary
> +
> + interrupts:
> + description: The interrupt line the device is connected to.
> + maxItems: 1
> +
> + debounce-delay-ms:
> + description: Debounce delay in ms
> + minimum: 0
> + maximum: 15
> + default: 0
> +
> + linux,axis:
> + description: The input subsystem axis to map to this rotary encoder.
> +
Fold into parent node.
> +description: |
> + Maxim MAX7360 PWM controller, in MAX7360 MFD
> + https://www.analog.com/en/products/max7360.html
> +
> +properties:
> + compatible:
> + enum:
> + - maxim,max7360-pwm
> +
> + "#pwm-cells":
> + const: 2
> +
> +
Fold into parent.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/8] pwm: max7360: Add MAX7360 PWM support
2024-12-19 16:21 ` [PATCH 4/8] pwm: max7360: Add MAX7360 PWM support mathieu.dubois-briand
2024-12-19 21:53 ` Uwe Kleine-König
2024-12-20 5:51 ` Uwe Kleine-König
@ 2024-12-21 20:35 ` Krzysztof Kozlowski
2 siblings, 0 replies; 25+ messages in thread
From: Krzysztof Kozlowski @ 2024-12-21 20:35 UTC (permalink / raw)
To: mathieu.dubois-briand, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Kamel Bouhara, Linus Walleij,
Bartosz Golaszewski, Dmitry Torokhov, Uwe Kleine-König
Cc: devicetree, linux-kernel, linux-gpio, linux-input, linux-pwm,
Grégory Clement, Thomas Petazzoni
On 19/12/2024 17:21, 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.
>
...
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id max7360_pwm_of_match[] = {
> + { .compatible = "maxim,max7360-pwm", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, max7360_pwm_of_match);
> +#endif
> +
> +static struct platform_driver max7360_pwm_driver = {
> + .driver = {
> + .name = "max7360-pwm",
> + .of_match_table = of_match_ptr(max7360_pwm_of_match),
Better to drop of_match_ptr and #ifdef earlier, so ACPI could use it.
> + },
> + .probe = max7360_pwm_probe,
> +};
> +module_platform_driver(max7360_pwm_driver);
> +
> +MODULE_DESCRIPTION("MAX7360 PWM driver");
> +MODULE_AUTHOR("Kamel BOUHARA <kamel.bouhara@bootlin.com>");
> +MODULE_ALIAS("platform:max7360-pwm");
You should not need MODULE_ALIAS() in normal cases. If you need it,
usually it means your device ID table is wrong (e.g. misses either
entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
for incomplete ID table.
Maybe you need it because OF table will get dropped, then it would be
fine. But in current code it's unnecessary and copy-pasta from other
drivers.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/8] dt-bindings: Add MAX7360 subdevices
2024-12-21 20:34 ` Krzysztof Kozlowski
@ 2024-12-23 15:20 ` Mathieu Dubois-Briand
2024-12-24 9:00 ` Krzysztof Kozlowski
0 siblings, 1 reply; 25+ messages in thread
From: Mathieu Dubois-Briand @ 2024-12-23 15:20 UTC (permalink / raw)
To: Krzysztof Kozlowski, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Kamel Bouhara, Linus Walleij, Bartosz Golaszewski,
Dmitry Torokhov, Uwe Kleine-König
Cc: devicetree, linux-kernel, linux-gpio, linux-input, linux-pwm,
Grégory Clement, Thomas Petazzoni
On Sat Dec 21, 2024 at 9:34 PM CET, Krzysztof Kozlowski wrote:
> On 19/12/2024 17:21, Mathieu Dubois-Briand wrote:
> > ---
> > .../devicetree/bindings/gpio/max7360-gpio.yaml | 96 ++++++++++++++++++++++
> > .../devicetree/bindings/input/max7360-keypad.yaml | 67 +++++++++++++++
> > .../devicetree/bindings/input/max7360-rotary.yaml | 52 ++++++++++++
> > .../devicetree/bindings/pwm/max7360-pwm.yaml | 35 ++++++++
> > 4 files changed, 250 insertions(+)
>
>
> I don't understand how this patchset was split. MFD binding cannot be
> empty and cannot be before child devices.
>
Ok, my bad. So I believe squashing both dt-bindings commit should fix
this.
> > diff --git a/Documentation/devicetree/bindings/gpio/max7360-gpio.yaml b/Documentation/devicetree/bindings/gpio/max7360-gpio.yaml
> > new file mode 100644
> > index 000000000000..3c006dc0380b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/max7360-gpio.yaml
> > @@ -0,0 +1,96 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/gpio/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 MFD
> > + https://www.analog.com/en/products/max7360.html
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - maxim,max7360-gpio
> > + - maxim,max7360-gpo
>
> Why? What are the differences?
>
Ok, so maybe my approach here is completely wrong. I'm not sure what
would be the best way to describe the device here, if you have any
suggestion I would be happy to use it. Let me try to summarize the GPIO
setup of the chip.
First we have two series of GPIOs on the chips, which I tend to think
about as two separate "banks". Thus two separate subnodes of the max7360
node.
- On one side we have what I refer to as GPIOs, here with
maxim,max7360-gpio:
- PORT0 to PORT7 pins of the chip.
- Shared with PWM and rotary encoder functionalities. Functionality
selection can be made independently for each pin. This selection is
not described here. Runtime will have to ensure the same pin is not
used by two drivers at the same time. E.g. we cannot have at the
same time GPIO4 and PWM4.
- Supports input and interrupts.
- Outputs may be configured as constant current.
- 8 GPIOS supported, so ngpios maximum is 8. Thinking about it now, we
should probably also set minimum to 8, I don't see any reason to
have ngpios set to something less.
On the other side, we have what I refer to as GPOs, here with
maxim,max7360-gpo compatible:
- COL2 to COL7 pins of the chip.
- Shared with the keypad functionality. Selections is made by
partitioning the pins: first pins for keypad columns, last pins for
GPOs. Partition is described here by ngpios and on keypad node by
keypad,num-columns. Runtime will have to ensure values are coherent
and configure the chip accordingly.
- Only support outputs.
- No support for constant current mode.
- Supports 0 to 6 GPOs, so ngpios maximum is 6.
> > +
> > + gpio-controller: true
> > +
> > + "#gpio-cells":
> > + const: 2
> > +
> > + ngpios:
> > + minimum: 0
> > + maximum: 8
>
> Why this is flexible?
>
I believe this makes sense, as this keypad/gpos partition really changes
the actual number of GPIOS. Yet we could argue that this is just runtime
configuration. Tell me what you think about it, if you think this should
be a fixed value, I will find a way.
>
> Best regards,
> Krzysztof
Thanks a lot for your review. I am preparing a new version of this
series that should address all of your other comments.
--
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/8] pwm: max7360: Add MAX7360 PWM support
2024-12-19 21:53 ` Uwe Kleine-König
@ 2024-12-23 15:26 ` Mathieu Dubois-Briand
0 siblings, 0 replies; 25+ messages in thread
From: Mathieu Dubois-Briand @ 2024-12-23 15:26 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Kamel Bouhara, Linus Walleij, Bartosz Golaszewski,
Dmitry Torokhov, devicetree, linux-kernel, linux-gpio,
linux-input, linux-pwm, Grégory Clement, Thomas Petazzoni
On Thu Dec 19, 2024 at 10:53 PM CET, Uwe Kleine-König wrote:
> Hello,
>
> On Thu, Dec 19, 2024 at 05:21:21PM +0100, mathieu.dubois-briand@bootlin.com wrote:
> > From: Kamel Bouhara <kamel.bouhara@bootlin.com>
> >
> > + int ret;
> > +
> > + if (state->polarity != PWM_POLARITY_NORMAL)
> > + return -EINVAL;
> > +
> > + max7360_pwm = to_max7360_pwm(chip);
> > + ret = regmap_write_bits(max7360_pwm->regmap, MAX7360_REG_GPIOCTRL,
> > + MAX7360_PWM_CTRL_ENABLE(pwm->hwpwm),
> > + MAX7360_PWM_CTRL_ENABLE(pwm->hwpwm));
> > + if (ret) {
> > + dev_err(&chip->dev, "failed to enable pwm-%d , error %d\n",
> > + pwm->hwpwm, ret);
> > + return ret;
> > + }
> > +
> > + do_div(duty_steps, MAX7360_PWM_PERIOD_NS);
> > +
> > + ret = regmap_write(max7360_pwm->regmap, MAX7360_REG_PWMBASE + pwm->hwpwm,
> > + duty_steps >= 255 ? 255 : duty_steps);
> > + if (ret) {
> > + dev_err(&chip->dev,
> > + "failed to apply pwm duty_cycle %llu on pwm-%d, error %d\n",
> > + duty_steps, pwm->hwpwm, ret);
> > + return ret;
> > + }
>
> Huh, state->period isn't used at all. That is wrong for sure.
>
Yes this was definitely missing. Period is fixed by the chip, so I will
make sure the requested one is valid or return -EINVAL.
Thanks a lot for your review, I am preparing a new version of this
series that should address all your comments.
--
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 6/8] input: keyboard: Add support for MAX7360 keypad
2024-12-19 19:35 ` Dmitry Torokhov
@ 2024-12-23 15:38 ` Mathieu Dubois-Briand
0 siblings, 0 replies; 25+ messages in thread
From: Mathieu Dubois-Briand @ 2024-12-23 15:38 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Kamel Bouhara, Linus Walleij, Bartosz Golaszewski,
Uwe Kleine-König, devicetree, linux-kernel, linux-gpio,
linux-input, linux-pwm, Grégory Clement, Thomas Petazzoni
On Thu Dec 19, 2024 at 8:35 PM CET, Dmitry Torokhov wrote:
> Hi Mathieu,
>
> On Thu, Dec 19, 2024 at 05:21:23PM +0100, Mathieu Dubois-Briand wrote:
> > +
> > + input_set_capability(input, EV_MSC, MSC_SCAN);
> > + if (!max7360_keypad->no_autorepeat)
> > + __set_bit(EV_REP, input->evbit);
> > +
> > + input_set_drvdata(input, max7360_keypad);
> > +
> > + flags = IRQF_TRIGGER_LOW | IRQF_ONESHOT | IRQF_SHARED;
>
> Shared? Why? And why do you need a temp variable?
>
Ok, this is probably wrong.
I have a board using the MAX7360, were both interrupt lines of the
chipset (INTI, interrupt for GPIO and rotary encoder and INTK, interrupt
for the keypad) have been both wired to the same pin on the CPU side. So
on this board, the interrupt line is indeed shared between the three
corresponding drivers.
Yet this is probably very specific to my board, as I have seen no data
about MAXIM suggesting this design. So having IRQF_SHARED is probably
more a hack than a valid implementation. I will drop it in my next
series.
>
> Thanks.
Thanks a lot for you review, I am preparing a new version of this series
that should address all your comments.
--
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/8] dt-bindings: Add MAX7360 subdevices
2024-12-23 15:20 ` Mathieu Dubois-Briand
@ 2024-12-24 9:00 ` Krzysztof Kozlowski
2024-12-24 12:10 ` Mathieu Dubois-Briand
0 siblings, 1 reply; 25+ messages in thread
From: Krzysztof Kozlowski @ 2024-12-24 9:00 UTC (permalink / raw)
To: Mathieu Dubois-Briand, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Kamel Bouhara, Linus Walleij,
Bartosz Golaszewski, Dmitry Torokhov, Uwe Kleine-König
Cc: devicetree, linux-kernel, linux-gpio, linux-input, linux-pwm,
Grégory Clement, Thomas Petazzoni
On 23/12/2024 16:20, Mathieu Dubois-Briand wrote:
> On Sat Dec 21, 2024 at 9:34 PM CET, Krzysztof Kozlowski wrote:
>> On 19/12/2024 17:21, Mathieu Dubois-Briand wrote:
>>> ---
>>> .../devicetree/bindings/gpio/max7360-gpio.yaml | 96 ++++++++++++++++++++++
>>> .../devicetree/bindings/input/max7360-keypad.yaml | 67 +++++++++++++++
>>> .../devicetree/bindings/input/max7360-rotary.yaml | 52 ++++++++++++
>>> .../devicetree/bindings/pwm/max7360-pwm.yaml | 35 ++++++++
>>> 4 files changed, 250 insertions(+)
>>
>>
>> I don't understand how this patchset was split. MFD binding cannot be
>> empty and cannot be before child devices.
>>
>
> Ok, my bad. So I believe squashing both dt-bindings commit should fix
> this.
>
>>> diff --git a/Documentation/devicetree/bindings/gpio/max7360-gpio.yaml b/Documentation/devicetree/bindings/gpio/max7360-gpio.yaml
>>> new file mode 100644
>>> index 000000000000..3c006dc0380b
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/gpio/max7360-gpio.yaml
>>> @@ -0,0 +1,96 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/gpio/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 MFD
>>> + https://www.analog.com/en/products/max7360.html
>>> +
>>> +properties:
>>> + compatible:
>>> + enum:
>>> + - maxim,max7360-gpio
>>> + - maxim,max7360-gpo
>>
>> Why? What are the differences?
>>
>
> Ok, so maybe my approach here is completely wrong. I'm not sure what
> would be the best way to describe the device here, if you have any
> suggestion I would be happy to use it. Let me try to summarize the GPIO
> setup of the chip.
>
> First we have two series of GPIOs on the chips, which I tend to think
> about as two separate "banks". Thus two separate subnodes of the max7360
> node.
First, splitting MFD device into multiple children is pretty often wrong
approach because it tries to mimic Linux driver design.
Such split in DT makes sense if these are really separate blocks, e.g.
separate I2C addresses, re-usable on different designs.
In this case Functional Block Diagram shows separate blocks, but still
the same I2C block. This can be one device. This can be also two devices
if that's easier to represent in DT.
But in any case binding description should explain this.
>
> - On one side we have what I refer to as GPIOs, here with
> maxim,max7360-gpio:
> - PORT0 to PORT7 pins of the chip.
> - Shared with PWM and rotary encoder functionalities. Functionality
> selection can be made independently for each pin. This selection is
> not described here. Runtime will have to ensure the same pin is not
> used by two drivers at the same time. E.g. we cannot have at the
> same time GPIO4 and PWM4.
> - Supports input and interrupts.
> - Outputs may be configured as constant current.
> - 8 GPIOS supported, so ngpios maximum is 8. Thinking about it now, we
> should probably also set minimum to 8, I don't see any reason to
> have ngpios set to something less.
>
> On the other side, we have what I refer to as GPOs, here with
> maxim,max7360-gpo compatible:
> - COL2 to COL7 pins of the chip.
> - Shared with the keypad functionality. Selections is made by
> partitioning the pins: first pins for keypad columns, last pins for
> GPOs. Partition is described here by ngpios and on keypad node by
> keypad,num-columns. Runtime will have to ensure values are coherent
> and configure the chip accordingly.
> - Only support outputs.
> - No support for constant current mode.
> - Supports 0 to 6 GPOs, so ngpios maximum is 6.
>
>>> +
>>> + gpio-controller: true
>>> +
>>> + "#gpio-cells":
>>> + const: 2
>>> +
>>> + ngpios:
>>> + minimum: 0
>>> + maximum: 8
>>
>> Why this is flexible?
>>
>
> I believe this makes sense, as this keypad/gpos partition really changes
> the actual number of GPIOS. Yet we could argue that this is just runtime
> configuration. Tell me what you think about it, if you think this should
> be a fixed value, I will find a way.
Depends whether this is actual runtime configuration. If you configure
keypad in DT, then the pins go away from GPIOs (especially considering
that board might have these pins really connected to keypad). Anyway,
explain this briefly in binding description.
>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/8] dt-bindings: Add MAX7360 subdevices
2024-12-24 9:00 ` Krzysztof Kozlowski
@ 2024-12-24 12:10 ` Mathieu Dubois-Briand
0 siblings, 0 replies; 25+ messages in thread
From: Mathieu Dubois-Briand @ 2024-12-24 12:10 UTC (permalink / raw)
To: Krzysztof Kozlowski, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Kamel Bouhara, Linus Walleij, Bartosz Golaszewski,
Dmitry Torokhov, Uwe Kleine-König
Cc: devicetree, linux-kernel, linux-gpio, linux-input, linux-pwm,
Grégory Clement, Thomas Petazzoni
On Tue Dec 24, 2024 at 10:00 AM CET, Krzysztof Kozlowski wrote:
> On 23/12/2024 16:20, Mathieu Dubois-Briand wrote:
> > On Sat Dec 21, 2024 at 9:34 PM CET, Krzysztof Kozlowski wrote:
> >> On 19/12/2024 17:21, Mathieu Dubois-Briand wrote:
> >>> ---
> >>> diff --git a/Documentation/devicetree/bindings/gpio/max7360-gpio.yaml b/Documentation/devicetree/bindings/gpio/max7360-gpio.yaml
> >>> new file mode 100644
> >>> index 000000000000..3c006dc0380b
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/gpio/max7360-gpio.yaml
> >>> @@ -0,0 +1,96 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/gpio/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 MFD
> >>> + https://www.analog.com/en/products/max7360.html
> >>> +
> >>> +properties:
> >>> + compatible:
> >>> + enum:
> >>> + - maxim,max7360-gpio
> >>> + - maxim,max7360-gpo
> >>
> >> Why? What are the differences?
> >>
> >
> > Ok, so maybe my approach here is completely wrong. I'm not sure what
> > would be the best way to describe the device here, if you have any
> > suggestion I would be happy to use it. Let me try to summarize the GPIO
> > setup of the chip.
> >
> > First we have two series of GPIOs on the chips, which I tend to think
> > about as two separate "banks". Thus two separate subnodes of the max7360
> > node.
>
> First, splitting MFD device into multiple children is pretty often wrong
> approach because it tries to mimic Linux driver design.
>
> Such split in DT makes sense if these are really separate blocks, e.g.
> separate I2C addresses, re-usable on different designs.
>
> In this case Functional Block Diagram shows separate blocks, but still
> the same I2C block. This can be one device. This can be also two devices
> if that's easier to represent in DT.
Ok, I get it. So I could try to remove the children, but I'm not really
sure about the way to go:
- About the two series of GPIOs, how should I represent them? In a
continuous way, like 0-7 is gpios and 8+ is gpos? Or maybe setting
#gpio-cells to 3 and using the added cell to select between gpios and
gpos ?
- About the interrupt-controller: today we have a children where all
gpios have a corresponding interrupt and another one without any
interrupt. If I remove the children, we will have a mix of both. I
don't think there is anything preventing to do this, but is this ok?
So I'm keeping the two children for now, but I'm open to the possibility
of removing them.
>
> But in any case binding description should explain this.
>
Ok, I will add some documentation.
> >
> > - On one side we have what I refer to as GPIOs, here with
> > maxim,max7360-gpio:
> > - PORT0 to PORT7 pins of the chip.
> > - Shared with PWM and rotary encoder functionalities. Functionality
> > selection can be made independently for each pin. This selection is
> > not described here. Runtime will have to ensure the same pin is not
> > used by two drivers at the same time. E.g. we cannot have at the
> > same time GPIO4 and PWM4.
> > - Supports input and interrupts.
> > - Outputs may be configured as constant current.
> > - 8 GPIOS supported, so ngpios maximum is 8. Thinking about it now, we
> > should probably also set minimum to 8, I don't see any reason to
> > have ngpios set to something less.
> >
> > On the other side, we have what I refer to as GPOs, here with
> > maxim,max7360-gpo compatible:
> > - COL2 to COL7 pins of the chip.
> > - Shared with the keypad functionality. Selections is made by
> > partitioning the pins: first pins for keypad columns, last pins for
> > GPOs. Partition is described here by ngpios and on keypad node by
> > keypad,num-columns. Runtime will have to ensure values are coherent
> > and configure the chip accordingly.
> > - Only support outputs.
> > - No support for constant current mode.
> > - Supports 0 to 6 GPOs, so ngpios maximum is 6.
> >
> >>> +
> >>> + gpio-controller: true
> >>> +
> >>> + "#gpio-cells":
> >>> + const: 2
> >>> +
> >>> + ngpios:
> >>> + minimum: 0
> >>> + maximum: 8
> >>
> >> Why this is flexible?
> >>
> >
> > I believe this makes sense, as this keypad/gpos partition really changes
> > the actual number of GPIOS. Yet we could argue that this is just runtime
> > configuration. Tell me what you think about it, if you think this should
> > be a fixed value, I will find a way.
>
> Depends whether this is actual runtime configuration. If you configure
> keypad in DT, then the pins go away from GPIOs (especially considering
> that board might have these pins really connected to keypad). Anyway,
> explain this briefly in binding description.
Keypad is configured in DT and yes, the pins partition is a consequence
of the hardware implementation on the board. So on second thought I
believe this is cannot be a runtime configuration and should be
described in the DT.
I will add some documentation about it.
>
> >
> Best regards,
> Krzysztof
Thanks again for your review.
Mathieu
--
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2024-12-24 12:10 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-19 16:21 [PATCH 0/8] Add support for MAX7360 multifunction device Mathieu Dubois-Briand
2024-12-19 16:21 ` [PATCH 1/8] dt-bindings: Add MAX7360 MFD device Mathieu Dubois-Briand
2024-12-21 20:28 ` Krzysztof Kozlowski
2024-12-19 16:21 ` [PATCH 2/8] dt-bindings: Add MAX7360 subdevices Mathieu Dubois-Briand
2024-12-19 21:54 ` Uwe Kleine-König
2024-12-21 20:34 ` Krzysztof Kozlowski
2024-12-23 15:20 ` Mathieu Dubois-Briand
2024-12-24 9:00 ` Krzysztof Kozlowski
2024-12-24 12:10 ` Mathieu Dubois-Briand
2024-12-19 16:21 ` [PATCH 3/8] mfd: Add max7360 support mathieu.dubois-briand
2024-12-20 23:01 ` kernel test robot
2024-12-19 16:21 ` [PATCH 4/8] pwm: max7360: Add MAX7360 PWM support mathieu.dubois-briand
2024-12-19 21:53 ` Uwe Kleine-König
2024-12-23 15:26 ` Mathieu Dubois-Briand
2024-12-20 5:51 ` Uwe Kleine-König
2024-12-21 20:35 ` Krzysztof Kozlowski
2024-12-19 16:21 ` [PATCH 5/8] gpio: max7360: Add MAX7360 gpio support Mathieu Dubois-Briand
2024-12-20 15:54 ` kernel test robot
2024-12-20 16:39 ` kernel test robot
2024-12-19 16:21 ` [PATCH 6/8] input: keyboard: Add support for MAX7360 keypad Mathieu Dubois-Briand
2024-12-19 19:35 ` Dmitry Torokhov
2024-12-23 15:38 ` Mathieu Dubois-Briand
2024-12-20 17:12 ` kernel test robot
2024-12-19 16:21 ` [PATCH 7/8] input: misc: Add support for MAX7360 rotary Mathieu Dubois-Briand
2024-12-19 16:21 ` [PATCH 8/8] MAINTAINERS: Add entry on MAX7360 driver Mathieu Dubois-Briand
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).