From: Kamel Bouhara <kamel.bouhara@bootlin.com>
Add core driver to support MAX7360 i2c chip, multi function device
with keypad, GPIO, PWM, GPO and rotary encoder submodules.
Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
Co-developed-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
---
drivers/mfd/Kconfig | 14 ++++
drivers/mfd/Makefile | 1 +
drivers/mfd/max7360.c | 184 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/mfd/max7360.h | 109 ++++++++++++++++++++++++++
4 files changed, 308 insertions(+)
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 22b936310039..c2998c6ce54c 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2422,5 +2422,19 @@ config MFD_UPBOARD_FPGA
To compile this driver as a module, choose M here: the module will be
called upboard-fpga.
+config MFD_MAX7360
+ tristate "Maxim MAX7360 I2C IO Expander"
+ depends on I2C
+ select MFD_CORE
+ select REGMAP_I2C
+ select REGMAP_IRQ
+ help
+ Say yes here to add support for Maxim MAX7360 device, embedding
+ keypad, rotary encoder, PWM and GPIO features.
+
+ This driver provides common support for accessing the device;
+ additional drivers must be enabled in order to use the functionality
+ of the device.
+
endmenu
endif
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 948cbdf42a18..add9ff58eb25 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -162,6 +162,7 @@ obj-$(CONFIG_MFD_DA9063) += da9063.o
obj-$(CONFIG_MFD_DA9150) += da9150-core.o
obj-$(CONFIG_MFD_MAX14577) += max14577.o
+obj-$(CONFIG_MFD_MAX7360) += max7360.o
obj-$(CONFIG_MFD_MAX77541) += max77541.o
obj-$(CONFIG_MFD_MAX77620) += max77620.o
obj-$(CONFIG_MFD_MAX77650) += max77650.o
diff --git a/drivers/mfd/max7360.c b/drivers/mfd/max7360.c
new file mode 100644
index 000000000000..9a223a9b409d
--- /dev/null
+++ b/drivers/mfd/max7360.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Maxim MAX7360 Core Driver
+ *
+ * Copyright 2025 Bootlin
+ *
+ * Author: Kamel Bouhara <kamel.bouhara@bootlin.com>
+ * Author: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
+ */
+
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/device/devres.h>
+#include <linux/dev_printk.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/max7360.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+static const struct mfd_cell max7360_cells[] = {
+ {
+ .name = "max7360-pinctrl",
+ },
+ {
+ .name = "max7360-pwm",
+ },
+ {
+ .name = "max7360-gpo",
+ .of_compatible = "maxim,max7360-gpo",
+ },
+ {
+ .name = "max7360-gpio",
+ .of_compatible = "maxim,max7360-gpio",
+ },
+ {
+ .name = "max7360-keypad",
+ },
+ {
+ .name = "max7360-rotary",
+ },
+};
+
+static const struct regmap_range max7360_volatile_ranges[] = {
+ {
+ .range_min = MAX7360_REG_KEYFIFO,
+ .range_max = MAX7360_REG_KEYFIFO,
+ }, {
+ .range_min = MAX7360_REG_I2C_TIMEOUT,
+ .range_max = MAX7360_REG_RTR_CNT,
+ },
+};
+
+static const struct regmap_access_table max7360_volatile_table = {
+ .yes_ranges = max7360_volatile_ranges,
+ .n_yes_ranges = ARRAY_SIZE(max7360_volatile_ranges),
+};
+
+static const struct regmap_config max7360_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = MAX7360_REG_PWMCFG(MAX7360_PORT_PWM_COUNT - 1),
+ .volatile_table = &max7360_volatile_table,
+ .cache_type = REGCACHE_MAPLE,
+};
+
+static int max7360_mask_irqs(struct regmap *regmap)
+{
+ struct device *dev = regmap_get_device(regmap);
+ unsigned int val;
+ int ret;
+
+ /*
+ * GPIO/PWM interrupts are not masked on reset: as the MAX7360 "INTI"
+ * interrupt line is shared between GPIOs and rotary encoder, this could
+ * result in repeated spurious interrupts on the rotary encoder driver
+ * if the GPIO driver is not loaded. Mask them now to avoid this
+ * situation.
+ */
+ for (unsigned int i = 0; i < MAX7360_PORT_PWM_COUNT; i++) {
+ ret = regmap_write_bits(regmap, MAX7360_REG_PWMCFG(i),
+ MAX7360_PORT_CFG_INTERRUPT_MASK,
+ MAX7360_PORT_CFG_INTERRUPT_MASK);
+ if (ret) {
+ dev_err(dev, "Failed to write max7360 port configuration");
+ return ret;
+ }
+ }
+
+ /* Read GPIO in register, to ACK any pending IRQ. */
+ ret = regmap_read(regmap, MAX7360_REG_GPIOIN, &val);
+ if (ret)
+ dev_err(dev, "Failed to read gpio values: %d\n", ret);
+
+ return ret;
+}
+
+static int max7360_reset(struct regmap *regmap)
+{
+ struct device *dev = regmap_get_device(regmap);
+ int ret;
+
+ ret = regmap_write(regmap, MAX7360_REG_GPIOCFG, MAX7360_GPIO_CFG_GPIO_RST);
+ if (ret) {
+ dev_err(dev, "Failed to reset GPIO configuration: %x\n", ret);
+ return ret;
+ }
+
+ ret = regcache_drop_region(regmap, MAX7360_REG_GPIOCFG, MAX7360_REG_GPIO_LAST);
+ if (ret) {
+ dev_err(dev, "Failed to drop regmap cache: %x\n", ret);
+ return ret;
+ }
+
+ ret = regmap_write(regmap, MAX7360_REG_SLEEP, 0);
+ if (ret) {
+ dev_err(dev, "Failed to reset autosleep configuration: %x\n", ret);
+ return ret;
+ }
+
+ ret = regmap_write(regmap, MAX7360_REG_DEBOUNCE, 0);
+ if (ret)
+ dev_err(dev, "Failed to reset GPO port count: %x\n", ret);
+
+ return ret;
+}
+
+static int max7360_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct regmap *regmap;
+ int ret;
+
+ regmap = devm_regmap_init_i2c(client, &max7360_regmap_config);
+ if (IS_ERR(regmap))
+ return dev_err_probe(dev, PTR_ERR(regmap), "Failed to initialise regmap\n");
+
+ ret = max7360_reset(regmap);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to reset device\n");
+
+ /* Get the device out of shutdown mode. */
+ ret = regmap_write_bits(regmap, MAX7360_REG_GPIOCFG,
+ MAX7360_GPIO_CFG_GPIO_EN,
+ MAX7360_GPIO_CFG_GPIO_EN);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to enable GPIO and PWM module\n");
+
+ ret = max7360_mask_irqs(regmap);
+ if (ret)
+ return dev_err_probe(dev, ret, "Could not mask interrupts\n");
+
+ ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
+ max7360_cells, ARRAY_SIZE(max7360_cells),
+ NULL, 0, NULL);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to register child devices\n");
+
+ return 0;
+}
+
+static const struct of_device_id max7360_dt_match[] = {
+ { .compatible = "maxim,max7360" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, max7360_dt_match);
+
+static struct i2c_driver max7360_driver = {
+ .driver = {
+ .name = "max7360",
+ .of_match_table = max7360_dt_match,
+ },
+ .probe = max7360_probe,
+};
+module_i2c_driver(max7360_driver);
+
+MODULE_DESCRIPTION("Maxim MAX7360 I2C IO Expander core driver");
+MODULE_AUTHOR("Kamel Bouhara <kamel.bouhara@bootlin.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/max7360.h b/include/linux/mfd/max7360.h
new file mode 100644
index 000000000000..b1d4cbee2385
--- /dev/null
+++ b/include/linux/mfd/max7360.h
@@ -0,0 +1,109 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __LINUX_MFD_MAX7360_H
+#define __LINUX_MFD_MAX7360_H
+
+#include <linux/bits.h>
+
+#define MAX7360_MAX_KEY_ROWS 8
+#define MAX7360_MAX_KEY_COLS 8
+#define MAX7360_MAX_KEY_NUM (MAX7360_MAX_KEY_ROWS * MAX7360_MAX_KEY_COLS)
+#define MAX7360_ROW_SHIFT 3
+
+#define MAX7360_MAX_GPIO 8
+#define MAX7360_MAX_GPO 6
+#define MAX7360_PORT_PWM_COUNT 8
+#define MAX7360_PORT_RTR_PIN (MAX7360_PORT_PWM_COUNT - 1)
+
+/*
+ * MAX7360 registers
+ */
+#define MAX7360_REG_KEYFIFO 0x00
+#define MAX7360_REG_CONFIG 0x01
+#define MAX7360_REG_DEBOUNCE 0x02
+#define MAX7360_REG_INTERRUPT 0x03
+#define MAX7360_REG_PORTS 0x04
+#define MAX7360_REG_KEYREP 0x05
+#define MAX7360_REG_SLEEP 0x06
+
+/*
+ * MAX7360 GPIO registers
+ *
+ * All these registers are reset together when writing bit 3 of
+ * MAX7360_REG_GPIOCFG.
+ */
+#define MAX7360_REG_GPIOCFG 0x40
+#define MAX7360_REG_GPIOCTRL 0x41
+#define MAX7360_REG_GPIODEB 0x42
+#define MAX7360_REG_GPIOCURR 0x43
+#define MAX7360_REG_GPIOOUTM 0x44
+#define MAX7360_REG_PWMCOM 0x45
+#define MAX7360_REG_RTRCFG 0x46
+#define MAX7360_REG_I2C_TIMEOUT 0x48
+#define MAX7360_REG_GPIOIN 0x49
+#define MAX7360_REG_RTR_CNT 0x4A
+#define MAX7360_REG_PWMBASE 0x50
+#define MAX7360_REG_PWMCFGBASE 0x58
+
+#define MAX7360_REG_GPIO_LAST 0x5F
+
+#define MAX7360_REG_PWM(x) (MAX7360_REG_PWMBASE + (x))
+#define MAX7360_REG_PWMCFG(x) (MAX7360_REG_PWMCFGBASE + (x))
+
+/*
+ * Configuration register bits
+ */
+#define MAX7360_FIFO_EMPTY 0x3f
+#define MAX7360_FIFO_OVERFLOW 0x7f
+#define MAX7360_FIFO_RELEASE BIT(6)
+#define MAX7360_FIFO_COL GENMASK(5, 3)
+#define MAX7360_FIFO_ROW GENMASK(2, 0)
+
+#define MAX7360_CFG_SLEEP BIT(7)
+#define MAX7360_CFG_INTERRUPT BIT(5)
+#define MAX7360_CFG_KEY_RELEASE BIT(3)
+#define MAX7360_CFG_WAKEUP BIT(1)
+#define MAX7360_CFG_TIMEOUT BIT(0)
+
+#define MAX7360_DEBOUNCE GENMASK(4, 0)
+#define MAX7360_DEBOUNCE_MIN 9
+#define MAX7360_DEBOUNCE_MAX 40
+#define MAX7360_PORTS GENMASK(8, 5)
+
+#define MAX7360_INTERRUPT_TIME_MASK GENMASK(4, 0)
+#define MAX7360_INTERRUPT_FIFO_MASK GENMASK(7, 5)
+
+#define MAX7360_PORT_CFG_INTERRUPT_MASK BIT(7)
+#define MAX7360_PORT_CFG_INTERRUPT_EDGES BIT(6)
+#define MAX7360_PORT_CFG_COMMON_PWM BIT(5)
+
+/*
+ * Autosleep register values
+ */
+#define MAX7360_AUTOSLEEP_8192MS 0x01
+#define MAX7360_AUTOSLEEP_4096MS 0x02
+#define MAX7360_AUTOSLEEP_2048MS 0x03
+#define MAX7360_AUTOSLEEP_1024MS 0x04
+#define MAX7360_AUTOSLEEP_512MS 0x05
+#define MAX7360_AUTOSLEEP_256MS 0x06
+
+#define MAX7360_GPIO_CFG_RTR_EN BIT(7)
+#define MAX7360_GPIO_CFG_GPIO_EN BIT(4)
+#define MAX7360_GPIO_CFG_GPIO_RST BIT(3)
+
+#define MAX7360_ROT_DEBOUNCE GENMASK(3, 0)
+#define MAX7360_ROT_DEBOUNCE_MIN 0
+#define MAX7360_ROT_DEBOUNCE_MAX 15
+#define MAX7360_ROT_INTCNT GENMASK(6, 4)
+#define MAX7360_ROT_INTCNT_DLY BIT(7)
+
+#define MAX7360_INT_INTI 0
+#define MAX7360_INT_INTK 1
+
+#define MAX7360_INT_GPIO 0
+#define MAX7360_INT_KEYPAD 1
+#define MAX7360_INT_ROTARY 2
+
+#define MAX7360_NR_INTERNAL_IRQS 3
+
+#endif
--
2.39.5On Mon, Apr 28, 2025 at 01:57:20PM +0200, mathieu.dubois-briand@bootlin.com wrote:
> From: Kamel Bouhara <kamel.bouhara@bootlin.com>
>
> Add core driver to support MAX7360 i2c chip, multi function device
> with keypad, GPIO, PWM, GPO and rotary encoder submodules.
...
> +static int max7360_mask_irqs(struct regmap *regmap)
So, AFAICS this is used only at probe stage...
> +{
> + struct device *dev = regmap_get_device(regmap);
> + unsigned int val;
> + int ret;
> +
> + /*
> + * GPIO/PWM interrupts are not masked on reset: as the MAX7360 "INTI"
> + * interrupt line is shared between GPIOs and rotary encoder, this could
> + * result in repeated spurious interrupts on the rotary encoder driver
> + * if the GPIO driver is not loaded. Mask them now to avoid this
> + * situation.
> + */
> + for (unsigned int i = 0; i < MAX7360_PORT_PWM_COUNT; i++) {
> + ret = regmap_write_bits(regmap, MAX7360_REG_PWMCFG(i),
> + MAX7360_PORT_CFG_INTERRUPT_MASK,
> + MAX7360_PORT_CFG_INTERRUPT_MASK);
> + if (ret) {
> + dev_err(dev, "Failed to write max7360 port configuration");
> + return ret;
...if it's the case, use return dev_err_probe(...) here...
> + }
> + }
> +
> + /* Read GPIO in register, to ACK any pending IRQ. */
> + ret = regmap_read(regmap, MAX7360_REG_GPIOIN, &val);
> + if (ret)
> + dev_err(dev, "Failed to read gpio values: %d\n", ret);
> +
> + return ret;
...and here.
> +}
--
With Best Regards,
Andy Shevchenko
On Mon, 28 Apr 2025, mathieu.dubois-briand@bootlin.com wrote:
> From: Kamel Bouhara <kamel.bouhara@bootlin.com>
>
> Add core driver to support MAX7360 i2c chip, multi function device
> with keypad, GPIO, PWM, GPO and rotary encoder submodules.
>
> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
> Co-developed-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> ---
> drivers/mfd/Kconfig | 14 ++++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/max7360.c | 184 ++++++++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/max7360.h | 109 ++++++++++++++++++++++++++
> 4 files changed, 308 insertions(+)
Getting there. Couple of nits. Last push!
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 22b936310039..c2998c6ce54c 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2422,5 +2422,19 @@ config MFD_UPBOARD_FPGA
> To compile this driver as a module, choose M here: the module will be
> called upboard-fpga.
>
> +config MFD_MAX7360
> + tristate "Maxim MAX7360 I2C IO Expander"
> + depends on I2C
> + select MFD_CORE
> + select REGMAP_I2C
> + select REGMAP_IRQ
> + help
> + Say yes here to add support for Maxim MAX7360 device, embedding
> + keypad, rotary encoder, PWM and GPIO features.
> +
> + This driver provides common support for accessing the device;
> + additional drivers must be enabled in order to use the functionality
> + of the device.
> +
> endmenu
> endif
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 948cbdf42a18..add9ff58eb25 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -162,6 +162,7 @@ obj-$(CONFIG_MFD_DA9063) += da9063.o
> obj-$(CONFIG_MFD_DA9150) += da9150-core.o
>
> obj-$(CONFIG_MFD_MAX14577) += max14577.o
> +obj-$(CONFIG_MFD_MAX7360) += max7360.o
> obj-$(CONFIG_MFD_MAX77541) += max77541.o
> obj-$(CONFIG_MFD_MAX77620) += max77620.o
> obj-$(CONFIG_MFD_MAX77650) += max77650.o
> diff --git a/drivers/mfd/max7360.c b/drivers/mfd/max7360.c
> new file mode 100644
> index 000000000000..9a223a9b409d
> --- /dev/null
> +++ b/drivers/mfd/max7360.c
> @@ -0,0 +1,184 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Maxim MAX7360 Core Driver
> + *
> + * Copyright 2025 Bootlin
> + *
> + * Author: Kamel Bouhara <kamel.bouhara@bootlin.com>
> + * Author: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/bits.h>
> +#include <linux/delay.h>
> +#include <linux/device/devres.h>
> +#include <linux/dev_printk.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/max7360.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/types.h>
> +
> +static const struct mfd_cell max7360_cells[] = {
> + {
> + .name = "max7360-pinctrl",
> + },
All of these single line entries should be placed on a single line.
{ .name = "max7360-pinctrl" },
{ .name = "max7360-pwm" },
If ordering is not important. Please group them.
> + {
> + .name = "max7360-pwm",
> + },
> + {
> + .name = "max7360-gpo",
> + .of_compatible = "maxim,max7360-gpo",
> + },
> + {
> + .name = "max7360-gpio",
> + .of_compatible = "maxim,max7360-gpio",
> + },
> + {
> + .name = "max7360-keypad",
> + },
> + {
> + .name = "max7360-rotary",
> + },
> +};
> +
> +static const struct regmap_range max7360_volatile_ranges[] = {
> + {
> + .range_min = MAX7360_REG_KEYFIFO,
> + .range_max = MAX7360_REG_KEYFIFO,
> + }, {
> + .range_min = MAX7360_REG_I2C_TIMEOUT,
> + .range_max = MAX7360_REG_RTR_CNT,
> + },
> +};
Use regmap_reg_range()
> +static const struct regmap_access_table max7360_volatile_table = {
> + .yes_ranges = max7360_volatile_ranges,
> + .n_yes_ranges = ARRAY_SIZE(max7360_volatile_ranges),
> +};
> +
> +static const struct regmap_config max7360_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = MAX7360_REG_PWMCFG(MAX7360_PORT_PWM_COUNT - 1),
> + .volatile_table = &max7360_volatile_table,
> + .cache_type = REGCACHE_MAPLE,
> +};
> +
> +static int max7360_mask_irqs(struct regmap *regmap)
> +{
> + struct device *dev = regmap_get_device(regmap);
> + unsigned int val;
> + int ret;
> +
> + /*
> + * GPIO/PWM interrupts are not masked on reset: as the MAX7360 "INTI"
> + * interrupt line is shared between GPIOs and rotary encoder, this could
> + * result in repeated spurious interrupts on the rotary encoder driver
> + * if the GPIO driver is not loaded. Mask them now to avoid this
> + * situation.
> + */
> + for (unsigned int i = 0; i < MAX7360_PORT_PWM_COUNT; i++) {
> + ret = regmap_write_bits(regmap, MAX7360_REG_PWMCFG(i),
> + MAX7360_PORT_CFG_INTERRUPT_MASK,
> + MAX7360_PORT_CFG_INTERRUPT_MASK);
> + if (ret) {
> + dev_err(dev, "Failed to write max7360 port configuration");
MAX7360
> + return ret;
> + }
> + }
> +
> + /* Read GPIO in register, to ACK any pending IRQ. */
> + ret = regmap_read(regmap, MAX7360_REG_GPIOIN, &val);
> + if (ret)
> + dev_err(dev, "Failed to read gpio values: %d\n", ret);
GPIO
> +
> + return ret;
> +}
> +
> +static int max7360_reset(struct regmap *regmap)
> +{
> + struct device *dev = regmap_get_device(regmap);
> + int ret;
> +
> + ret = regmap_write(regmap, MAX7360_REG_GPIOCFG, MAX7360_GPIO_CFG_GPIO_RST);
> + if (ret) {
> + dev_err(dev, "Failed to reset GPIO configuration: %x\n", ret);
> + return ret;
> + }
> +
> + ret = regcache_drop_region(regmap, MAX7360_REG_GPIOCFG, MAX7360_REG_GPIO_LAST);
> + if (ret) {
> + dev_err(dev, "Failed to drop regmap cache: %x\n", ret);
> + return ret;
> + }
> +
> + ret = regmap_write(regmap, MAX7360_REG_SLEEP, 0);
> + if (ret) {
> + dev_err(dev, "Failed to reset autosleep configuration: %x\n", ret);
> + return ret;
> + }
> +
> + ret = regmap_write(regmap, MAX7360_REG_DEBOUNCE, 0);
> + if (ret)
> + dev_err(dev, "Failed to reset GPO port count: %x\n", ret);
> +
> + return ret;
> +}
> +
> +static int max7360_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct regmap *regmap;
> + int ret;
> +
> + regmap = devm_regmap_init_i2c(client, &max7360_regmap_config);
> + if (IS_ERR(regmap))
> + return dev_err_probe(dev, PTR_ERR(regmap), "Failed to initialise regmap\n");
dev_err_ptr_probe()
> +
> + ret = max7360_reset(regmap);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to reset device\n");
> +
> + /* Get the device out of shutdown mode. */
> + ret = regmap_write_bits(regmap, MAX7360_REG_GPIOCFG,
> + MAX7360_GPIO_CFG_GPIO_EN,
> + MAX7360_GPIO_CFG_GPIO_EN);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to enable GPIO and PWM module\n");
> +
> + ret = max7360_mask_irqs(regmap);
> + if (ret)
> + return dev_err_probe(dev, ret, "Could not mask interrupts\n");
> +
> + ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
> + max7360_cells, ARRAY_SIZE(max7360_cells),
> + NULL, 0, NULL);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to register child devices\n");
> +
> + return 0;
> +}
> +
> +static const struct of_device_id max7360_dt_match[] = {
> + { .compatible = "maxim,max7360" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, max7360_dt_match);
> +
> +static struct i2c_driver max7360_driver = {
> + .driver = {
> + .name = "max7360",
> + .of_match_table = max7360_dt_match,
> + },
> + .probe = max7360_probe,
> +};
> +module_i2c_driver(max7360_driver);
> +
> +MODULE_DESCRIPTION("Maxim MAX7360 I2C IO Expander core driver");
> +MODULE_AUTHOR("Kamel Bouhara <kamel.bouhara@bootlin.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/max7360.h b/include/linux/mfd/max7360.h
> new file mode 100644
> index 000000000000..b1d4cbee2385
> --- /dev/null
> +++ b/include/linux/mfd/max7360.h
> @@ -0,0 +1,109 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __LINUX_MFD_MAX7360_H
> +#define __LINUX_MFD_MAX7360_H
> +
> +#include <linux/bits.h>
> +
> +#define MAX7360_MAX_KEY_ROWS 8
> +#define MAX7360_MAX_KEY_COLS 8
> +#define MAX7360_MAX_KEY_NUM (MAX7360_MAX_KEY_ROWS * MAX7360_MAX_KEY_COLS)
> +#define MAX7360_ROW_SHIFT 3
> +
> +#define MAX7360_MAX_GPIO 8
> +#define MAX7360_MAX_GPO 6
> +#define MAX7360_PORT_PWM_COUNT 8
> +#define MAX7360_PORT_RTR_PIN (MAX7360_PORT_PWM_COUNT - 1)
> +
> +/*
> + * MAX7360 registers
> + */
> +#define MAX7360_REG_KEYFIFO 0x00
> +#define MAX7360_REG_CONFIG 0x01
> +#define MAX7360_REG_DEBOUNCE 0x02
> +#define MAX7360_REG_INTERRUPT 0x03
> +#define MAX7360_REG_PORTS 0x04
> +#define MAX7360_REG_KEYREP 0x05
> +#define MAX7360_REG_SLEEP 0x06
> +
> +/*
> + * MAX7360 GPIO registers
> + *
> + * All these registers are reset together when writing bit 3 of
> + * MAX7360_REG_GPIOCFG.
> + */
> +#define MAX7360_REG_GPIOCFG 0x40
> +#define MAX7360_REG_GPIOCTRL 0x41
> +#define MAX7360_REG_GPIODEB 0x42
> +#define MAX7360_REG_GPIOCURR 0x43
> +#define MAX7360_REG_GPIOOUTM 0x44
> +#define MAX7360_REG_PWMCOM 0x45
> +#define MAX7360_REG_RTRCFG 0x46
> +#define MAX7360_REG_I2C_TIMEOUT 0x48
> +#define MAX7360_REG_GPIOIN 0x49
> +#define MAX7360_REG_RTR_CNT 0x4A
> +#define MAX7360_REG_PWMBASE 0x50
> +#define MAX7360_REG_PWMCFGBASE 0x58
> +
> +#define MAX7360_REG_GPIO_LAST 0x5F
> +
> +#define MAX7360_REG_PWM(x) (MAX7360_REG_PWMBASE + (x))
> +#define MAX7360_REG_PWMCFG(x) (MAX7360_REG_PWMCFGBASE + (x))
> +
> +/*
> + * Configuration register bits
> + */
> +#define MAX7360_FIFO_EMPTY 0x3f
> +#define MAX7360_FIFO_OVERFLOW 0x7f
> +#define MAX7360_FIFO_RELEASE BIT(6)
> +#define MAX7360_FIFO_COL GENMASK(5, 3)
> +#define MAX7360_FIFO_ROW GENMASK(2, 0)
> +
> +#define MAX7360_CFG_SLEEP BIT(7)
> +#define MAX7360_CFG_INTERRUPT BIT(5)
> +#define MAX7360_CFG_KEY_RELEASE BIT(3)
> +#define MAX7360_CFG_WAKEUP BIT(1)
> +#define MAX7360_CFG_TIMEOUT BIT(0)
> +
> +#define MAX7360_DEBOUNCE GENMASK(4, 0)
> +#define MAX7360_DEBOUNCE_MIN 9
> +#define MAX7360_DEBOUNCE_MAX 40
> +#define MAX7360_PORTS GENMASK(8, 5)
> +
> +#define MAX7360_INTERRUPT_TIME_MASK GENMASK(4, 0)
> +#define MAX7360_INTERRUPT_FIFO_MASK GENMASK(7, 5)
> +
> +#define MAX7360_PORT_CFG_INTERRUPT_MASK BIT(7)
> +#define MAX7360_PORT_CFG_INTERRUPT_EDGES BIT(6)
> +#define MAX7360_PORT_CFG_COMMON_PWM BIT(5)
> +
> +/*
> + * Autosleep register values
> + */
> +#define MAX7360_AUTOSLEEP_8192MS 0x01
> +#define MAX7360_AUTOSLEEP_4096MS 0x02
> +#define MAX7360_AUTOSLEEP_2048MS 0x03
> +#define MAX7360_AUTOSLEEP_1024MS 0x04
> +#define MAX7360_AUTOSLEEP_512MS 0x05
> +#define MAX7360_AUTOSLEEP_256MS 0x06
> +
> +#define MAX7360_GPIO_CFG_RTR_EN BIT(7)
> +#define MAX7360_GPIO_CFG_GPIO_EN BIT(4)
> +#define MAX7360_GPIO_CFG_GPIO_RST BIT(3)
> +
> +#define MAX7360_ROT_DEBOUNCE GENMASK(3, 0)
> +#define MAX7360_ROT_DEBOUNCE_MIN 0
> +#define MAX7360_ROT_DEBOUNCE_MAX 15
> +#define MAX7360_ROT_INTCNT GENMASK(6, 4)
> +#define MAX7360_ROT_INTCNT_DLY BIT(7)
> +
> +#define MAX7360_INT_INTI 0
> +#define MAX7360_INT_INTK 1
> +
> +#define MAX7360_INT_GPIO 0
> +#define MAX7360_INT_KEYPAD 1
> +#define MAX7360_INT_ROTARY 2
> +
> +#define MAX7360_NR_INTERNAL_IRQS 3
> +
> +#endif
>
> --
> 2.39.5
>
--
Lee Jones [李琼斯]
On Thu, May 01, 2025 at 01:59:43PM +0100, Lee Jones wrote:
> On Mon, 28 Apr 2025, mathieu.dubois-briand@bootlin.com wrote:
> >
> > Add core driver to support MAX7360 i2c chip, multi function device
> > with keypad, GPIO, PWM, GPO and rotary encoder submodules.
...
> > +static int max7360_probe(struct i2c_client *client)
> > +{
> > + struct device *dev = &client->dev;
> > + struct regmap *regmap;
> > + int ret;
> > +
> > + regmap = devm_regmap_init_i2c(client, &max7360_regmap_config);
> > + if (IS_ERR(regmap))
> > + return dev_err_probe(dev, PTR_ERR(regmap), "Failed to initialise regmap\n");
>
> dev_err_ptr_probe()
Hmm... This one to return an error pointer casted to the needed type. I think
the given code is correct as is.
> > + ret = max7360_reset(regmap);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to reset device\n");
> > +
> > + /* Get the device out of shutdown mode. */
> > + ret = regmap_write_bits(regmap, MAX7360_REG_GPIOCFG,
> > + MAX7360_GPIO_CFG_GPIO_EN,
> > + MAX7360_GPIO_CFG_GPIO_EN);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to enable GPIO and PWM module\n");
> > +
> > + ret = max7360_mask_irqs(regmap);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Could not mask interrupts\n");
> > +
> > + ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
> > + max7360_cells, ARRAY_SIZE(max7360_cells),
> > + NULL, 0, NULL);
> > + if (ret)
> > + return dev_err_probe(dev, ret, "Failed to register child devices\n");
> > +
> > + return 0;
> > +}
--
With Best Regards,
Andy Shevchenko
On Thu May 1, 2025 at 2:59 PM CEST, Lee Jones wrote:
> On Mon, 28 Apr 2025, mathieu.dubois-briand@bootlin.com wrote:
>
>> From: Kamel Bouhara <kamel.bouhara@bootlin.com>
>> +static int max7360_probe(struct i2c_client *client)
>> +{
>> + struct device *dev = &client->dev;
>> + struct regmap *regmap;
>> + int ret;
>> +
>> + regmap = devm_regmap_init_i2c(client, &max7360_regmap_config);
>> + if (IS_ERR(regmap))
>> + return dev_err_probe(dev, PTR_ERR(regmap), "Failed to initialise regmap\n");
>
> dev_err_ptr_probe()
>
I believe dev_err_ptr_probe() is meant to be used for the opposite case,
where the variable holding the error is an int but the function has to
return a pointer. Here regmap is a pointer but we have to return an int,
so I believe neither dev_err_ptr_probe() or any similar macro can really
help us.
OK for all other points.
Thanks for your review,
Mathieu
--
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On Fri, 02 May 2025, Mathieu Dubois-Briand wrote:
> On Thu May 1, 2025 at 2:59 PM CEST, Lee Jones wrote:
> > On Mon, 28 Apr 2025, mathieu.dubois-briand@bootlin.com wrote:
> >
> >> From: Kamel Bouhara <kamel.bouhara@bootlin.com>
> >> +static int max7360_probe(struct i2c_client *client)
> >> +{
> >> + struct device *dev = &client->dev;
> >> + struct regmap *regmap;
> >> + int ret;
> >> +
> >> + regmap = devm_regmap_init_i2c(client, &max7360_regmap_config);
> >> + if (IS_ERR(regmap))
> >> + return dev_err_probe(dev, PTR_ERR(regmap), "Failed to initialise regmap\n");
> >
> > dev_err_ptr_probe()
> >
>
> I believe dev_err_ptr_probe() is meant to be used for the opposite case,
> where the variable holding the error is an int but the function has to
> return a pointer. Here regmap is a pointer but we have to return an int,
> so I believe neither dev_err_ptr_probe() or any similar macro can really
> help us.
Ah yes, you're right. Disregard.
--
Lee Jones [李琼斯]
© 2016 - 2025 Red Hat, Inc.