[PATCH v4 3/5] mfd: aaeon: Add SRG-IMX8P MCU driver

Thomas Perrot (Schneider Electric) posted 5 patches 1 week, 1 day ago
[PATCH v4 3/5] mfd: aaeon: Add SRG-IMX8P MCU driver
Posted by Thomas Perrot (Schneider Electric) 1 week, 1 day ago
Add Multi-Function Device (MFD) driver for the Aaeon SRG-IMX8P
embedded controller. This driver provides the core I2C communication
interface and registers child devices (GPIO and watchdog controllers).

The driver implements a custom regmap bus over I2C to match the MCU's
fixed 3-byte command format [opcode, arg, value]. Register addresses
are encoded as 16-bit values (opcode << 8 | arg) using the
AAEON_MCU_REG() macro defined in the shared header. The regmap
instance is shared with child drivers via dev_get_regmap(). Concurrent
I2C accesses from child drivers are serialized by regmap's built-in
locking.

Co-developed-by: Jérémie Dautheribes (Schneider Electric) <jeremie.dautheribes@bootlin.com>
Signed-off-by: Jérémie Dautheribes (Schneider Electric) <jeremie.dautheribes@bootlin.com>
Signed-off-by: Thomas Perrot (Schneider Electric) <thomas.perrot@bootlin.com>
---
 MAINTAINERS                   |   2 +
 drivers/mfd/Kconfig           |  10 +++
 drivers/mfd/Makefile          |   1 +
 drivers/mfd/aaeon-mcu.c       | 155 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/aaeon-mcu.h |  20 ++++++
 5 files changed, 188 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ea9d55f76f3509c7f6ba6d1bc86ca2e2e71aa954..f91b6a1826d04bef8a0f88221f6c8e8a3652cd77 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -191,6 +191,8 @@ M:	Thomas Perrot <thomas.perrot@bootlin.com>
 R:	Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>
 S:	Maintained
 F:	Documentation/devicetree/bindings/mfd/aaeon,srg-imx8p-mcu.yaml
+F:	drivers/mfd/aaeon-mcu.c
+F:	include/linux/mfd/aaeon-mcu.h
 
 AAEON UPBOARD FPGA MFD DRIVER
 M:	Thomas Richard <thomas.richard@bootlin.com>
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index aace5766b38aa5e46e32a8a7b42eea238159fbcf..7a1ceedece899faad7a03a1fe7b1c91b72253c05 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1574,6 +1574,16 @@ config AB8500_CORE
 	  the irq_chip parts for handling the Mixed Signal chip events.
 	  This chip embeds various other multimedia functionalities as well.
 
+config MFD_AAEON_MCU
+	tristate "Aaeon SRG-IMX8P MCU Driver"
+	depends on I2C || COMPILE_TEST
+	select MFD_CORE
+	help
+	  Select this option to enable support for the Aaeon SRG-IMX8P
+	  onboard microcontroller (MCU). This driver provides the core
+	  functionality to communicate with the MCU over I2C. The MCU
+	  provides GPIO and watchdog functionality.
+
 config MFD_DB8500_PRCMU
 	bool "ST-Ericsson DB8500 Power Reset Control Management Unit"
 	depends on UX500_SOC_DB8500
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index e75e8045c28afae975ac61d282b3b85af5440119..34db5b033584368b7a269b1eef12528a74baf8f5 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_MFD_88PM860X)	+= 88pm860x.o
 obj-$(CONFIG_MFD_88PM800)	+= 88pm800.o 88pm80x.o
 obj-$(CONFIG_MFD_88PM805)	+= 88pm805.o 88pm80x.o
 obj-$(CONFIG_MFD_88PM886_PMIC)	+= 88pm886.o
+obj-$(CONFIG_MFD_AAEON_MCU)	+= aaeon-mcu.o
 obj-$(CONFIG_MFD_ACT8945A)	+= act8945a.o
 obj-$(CONFIG_MFD_SM501)		+= sm501.o
 obj-$(CONFIG_ARCH_BCM2835)	+= bcm2835-pm.o
diff --git a/drivers/mfd/aaeon-mcu.c b/drivers/mfd/aaeon-mcu.c
new file mode 100644
index 0000000000000000000000000000000000000000..5a969890d201c027eb25c324b4d4d89b1f8c563e
--- /dev/null
+++ b/drivers/mfd/aaeon-mcu.c
@@ -0,0 +1,155 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Aaeon MCU driver
+ *
+ * Copyright (C) 2025 Bootlin
+ * Author: Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>
+ * Author: Thomas Perrot <thomas.perrot@bootlin.com>
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/mfd/core.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+static const struct mfd_cell aaeon_mcu_devs[] = {
+	{
+		.name = "aaeon-mcu-wdt",
+	},
+	{
+		.name = "aaeon-mcu-gpio",
+	},
+};
+
+/*
+ * Custom regmap bus for the Aaeon MCU I2C protocol.
+ *
+ * The MCU uses a fixed 3-byte command format [opcode, arg, value] followed
+ * by a 1-byte response.  It requires a STOP condition between the command
+ * write and the response read, so two separate i2c_transfer() calls are
+ * issued.  The regmap lock serialises concurrent accesses from the GPIO
+ * and watchdog child drivers.
+ *
+ * Register addresses are encoded as a 16-bit big-endian value where the
+ * high byte is the opcode and the low byte is the argument, matching the
+ * wire layout produced by regmap for reg_bits=16.
+ */
+
+static int aaeon_mcu_regmap_write(void *context, const void *data, size_t count)
+{
+	struct i2c_client *client = context;
+	/* data = [opcode, arg, value] as formatted by regmap */
+	struct i2c_msg write_msg = {
+		.addr  = client->addr,
+		.flags = 0,
+		.buf   = (u8 *)data,
+		.len   = count,
+	};
+	u8 rsp;
+	/* The MCU always sends a response byte after each command; discard it. */
+	struct i2c_msg rsp_msg = {
+		.addr  = client->addr,
+		.flags = I2C_M_RD,
+		.buf   = &rsp,
+		.len   = 1,
+	};
+	int ret;
+
+	ret = i2c_transfer(client->adapter, &write_msg, 1);
+	if (ret < 0)
+		return ret;
+	if (ret != 1)
+		return -EIO;
+
+	ret = i2c_transfer(client->adapter, &rsp_msg, 1);
+	if (ret < 0)
+		return ret;
+	if (ret != 1)
+		return -EIO;
+
+	return 0;
+}
+
+static int aaeon_mcu_regmap_read(void *context, const void *reg_buf,
+				 size_t reg_size, void *val_buf, size_t val_size)
+{
+	struct i2c_client *client = context;
+	/*
+	 * reg_buf holds the 2-byte big-endian register address [opcode, arg].
+	 * Append a trailing 0x00 to form the full 3-byte MCU command.
+	 */
+	u8 cmd[3] = { ((u8 *)reg_buf)[0], ((u8 *)reg_buf)[1], 0x00 };
+	struct i2c_msg write_msg = {
+		.addr  = client->addr,
+		.flags = 0,
+		.buf   = cmd,
+		.len   = sizeof(cmd),
+	};
+	struct i2c_msg read_msg = {
+		.addr  = client->addr,
+		.flags = I2C_M_RD,
+		.buf   = val_buf,
+		.len   = val_size,
+	};
+	int ret;
+
+	ret = i2c_transfer(client->adapter, &write_msg, 1);
+	if (ret < 0)
+		return ret;
+	if (ret != 1)
+		return -EIO;
+
+	ret = i2c_transfer(client->adapter, &read_msg, 1);
+	if (ret < 0)
+		return ret;
+	if (ret != 1)
+		return -EIO;
+
+	return 0;
+}
+
+static const struct regmap_bus aaeon_mcu_regmap_bus = {
+	.write = aaeon_mcu_regmap_write,
+	.read  = aaeon_mcu_regmap_read,
+};
+
+static const struct regmap_config aaeon_mcu_regmap_config = {
+	.reg_bits          = 16,
+	.val_bits          = 8,
+	.reg_format_endian = REGMAP_ENDIAN_BIG,
+	.cache_type        = REGCACHE_NONE,
+};
+
+static int aaeon_mcu_probe(struct i2c_client *client)
+{
+	struct regmap *regmap;
+
+	regmap = devm_regmap_init(&client->dev, &aaeon_mcu_regmap_bus,
+				  client, &aaeon_mcu_regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	return devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
+				    aaeon_mcu_devs, ARRAY_SIZE(aaeon_mcu_devs),
+				    NULL, 0, NULL);
+}
+
+static const struct of_device_id aaeon_mcu_of_match[] = {
+	{ .compatible = "aaeon,srg-imx8p-mcu" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, aaeon_mcu_of_match);
+
+static struct i2c_driver aaeon_mcu_driver = {
+	.driver = {
+		.name = "aaeon_mcu",
+		.of_match_table = aaeon_mcu_of_match,
+	},
+	.probe = aaeon_mcu_probe,
+};
+module_i2c_driver(aaeon_mcu_driver);
+
+MODULE_DESCRIPTION("Aaeon MCU Driver");
+MODULE_AUTHOR("Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/aaeon-mcu.h b/include/linux/mfd/aaeon-mcu.h
new file mode 100644
index 0000000000000000000000000000000000000000..861003f6dfd20424c3785008bd2cf89aaa1715b9
--- /dev/null
+++ b/include/linux/mfd/aaeon-mcu.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Aaeon MCU driver definitions
+ *
+ * Copyright (C) 2025 Bootlin
+ * Author: Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>
+ * Author: Thomas Perrot <thomas.perrot@bootlin.com>
+ */
+
+#ifndef __LINUX_MFD_AAEON_MCU_H
+#define __LINUX_MFD_AAEON_MCU_H
+
+/*
+ * MCU register address: the high byte is the command opcode, the low
+ * byte is the argument.  This matches the 3-byte wire format
+ * [opcode, arg, value] used by the MCU I2C protocol.
+ */
+#define AAEON_MCU_REG(op, arg)	(((op) << 8) | (arg))
+
+#endif /* __LINUX_MFD_AAEON_MCU_H */

-- 
2.53.0

Re: [PATCH v4 3/5] mfd: aaeon: Add SRG-IMX8P MCU driver
Posted by Lee Jones 1 day, 14 hours ago
On Tue, 24 Mar 2026, Thomas Perrot (Schneider Electric) wrote:

> Add Multi-Function Device (MFD) driver for the Aaeon SRG-IMX8P
> embedded controller. This driver provides the core I2C communication
> interface and registers child devices (GPIO and watchdog controllers).
> 
> The driver implements a custom regmap bus over I2C to match the MCU's
> fixed 3-byte command format [opcode, arg, value]. Register addresses
> are encoded as 16-bit values (opcode << 8 | arg) using the
> AAEON_MCU_REG() macro defined in the shared header. The regmap
> instance is shared with child drivers via dev_get_regmap(). Concurrent
> I2C accesses from child drivers are serialized by regmap's built-in
> locking.
> 
> Co-developed-by: Jérémie Dautheribes (Schneider Electric) <jeremie.dautheribes@bootlin.com>
> Signed-off-by: Jérémie Dautheribes (Schneider Electric) <jeremie.dautheribes@bootlin.com>
> Signed-off-by: Thomas Perrot (Schneider Electric) <thomas.perrot@bootlin.com>
> ---
>  MAINTAINERS                   |   2 +
>  drivers/mfd/Kconfig           |  10 +++
>  drivers/mfd/Makefile          |   1 +
>  drivers/mfd/aaeon-mcu.c       | 155 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/aaeon-mcu.h |  20 ++++++
>  5 files changed, 188 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ea9d55f76f3509c7f6ba6d1bc86ca2e2e71aa954..f91b6a1826d04bef8a0f88221f6c8e8a3652cd77 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -191,6 +191,8 @@ M:	Thomas Perrot <thomas.perrot@bootlin.com>
>  R:	Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/mfd/aaeon,srg-imx8p-mcu.yaml
> +F:	drivers/mfd/aaeon-mcu.c
> +F:	include/linux/mfd/aaeon-mcu.h
>  
>  AAEON UPBOARD FPGA MFD DRIVER
>  M:	Thomas Richard <thomas.richard@bootlin.com>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index aace5766b38aa5e46e32a8a7b42eea238159fbcf..7a1ceedece899faad7a03a1fe7b1c91b72253c05 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1574,6 +1574,16 @@ config AB8500_CORE
>  	  the irq_chip parts for handling the Mixed Signal chip events.
>  	  This chip embeds various other multimedia functionalities as well.
>  
> +config MFD_AAEON_MCU
> +	tristate "Aaeon SRG-IMX8P MCU Driver"
> +	depends on I2C || COMPILE_TEST
> +	select MFD_CORE
> +	help
> +	  Select this option to enable support for the Aaeon SRG-IMX8P
> +	  onboard microcontroller (MCU). This driver provides the core
> +	  functionality to communicate with the MCU over I2C. The MCU
> +	  provides GPIO and watchdog functionality.
> +
>  config MFD_DB8500_PRCMU
>  	bool "ST-Ericsson DB8500 Power Reset Control Management Unit"
>  	depends on UX500_SOC_DB8500
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index e75e8045c28afae975ac61d282b3b85af5440119..34db5b033584368b7a269b1eef12528a74baf8f5 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_MFD_88PM860X)	+= 88pm860x.o
>  obj-$(CONFIG_MFD_88PM800)	+= 88pm800.o 88pm80x.o
>  obj-$(CONFIG_MFD_88PM805)	+= 88pm805.o 88pm80x.o
>  obj-$(CONFIG_MFD_88PM886_PMIC)	+= 88pm886.o
> +obj-$(CONFIG_MFD_AAEON_MCU)	+= aaeon-mcu.o
>  obj-$(CONFIG_MFD_ACT8945A)	+= act8945a.o
>  obj-$(CONFIG_MFD_SM501)		+= sm501.o
>  obj-$(CONFIG_ARCH_BCM2835)	+= bcm2835-pm.o
> diff --git a/drivers/mfd/aaeon-mcu.c b/drivers/mfd/aaeon-mcu.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..5a969890d201c027eb25c324b4d4d89b1f8c563e
> --- /dev/null
> +++ b/drivers/mfd/aaeon-mcu.c
> @@ -0,0 +1,155 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Aaeon MCU driver
> + *
> + * Copyright (C) 2025 Bootlin
> + * Author: Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>
> + * Author: Thomas Perrot <thomas.perrot@bootlin.com>
> + */

Consider updating the Copyright date - we're pretty deep into 2026 at this point.

> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/mfd/core.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +static const struct mfd_cell aaeon_mcu_devs[] = {
> +	{
> +		.name = "aaeon-mcu-wdt",
> +	},
> +	{
> +		.name = "aaeon-mcu-gpio",
> +	},
> +};

MFD_CELL_BASIC()

> +/*
> + * Custom regmap bus for the Aaeon MCU I2C protocol.
> + *
> + * The MCU uses a fixed 3-byte command format [opcode, arg, value] followed
> + * by a 1-byte response.  It requires a STOP condition between the command
> + * write and the response read, so two separate i2c_transfer() calls are
> + * issued.  The regmap lock serialises concurrent accesses from the GPIO
> + * and watchdog child drivers.
> + *
> + * Register addresses are encoded as a 16-bit big-endian value where the
> + * high byte is the opcode and the low byte is the argument, matching the
> + * wire layout produced by regmap for reg_bits=16.
> + */
> +
> +static int aaeon_mcu_regmap_write(void *context, const void *data, size_t count)
> +{
> +	struct i2c_client *client = context;
> +	/* data = [opcode, arg, value] as formatted by regmap */
> +	struct i2c_msg write_msg = {
> +		.addr  = client->addr,
> +		.flags = 0,
> +		.buf   = (u8 *)data,
> +		.len   = count,
> +	};
> +	u8 rsp;
> +	/* The MCU always sends a response byte after each command; discard it. */
> +	struct i2c_msg rsp_msg = {

Assuming 'rsp' means response, let's just write that out in full.

Readability wins over brevity every time.

> +		.addr  = client->addr,
> +		.flags = I2C_M_RD,
> +		.buf   = &rsp,
> +		.len   = 1,
> +	};
> +	int ret;

Since some I2C host controllers might use DMA, should we ensure that the
'rsp' buffer is allocated in DMA-safe memory rather than on the stack to
prevent potential cache-line corruption?

Also allocation of structs during in declaration statements is rough!

And adding that u8 in the middle is just rubbing it in.

> +	ret = i2c_transfer(client->adapter, &write_msg, 1);
> +	if (ret < 0)
> +		return ret;
> +	if (ret != 1)
> +		return -EIO;
> +
> +	ret = i2c_transfer(client->adapter, &rsp_msg, 1);
> +	if (ret < 0)
> +		return ret;
> +	if (ret != 1)
> +		return -EIO;
> +
> +	return 0;
> +}
> +
> +static int aaeon_mcu_regmap_read(void *context, const void *reg_buf,
> +				 size_t reg_size, void *val_buf, size_t val_size)
> +{
> +	struct i2c_client *client = context;
> +	/*
> +	 * reg_buf holds the 2-byte big-endian register address [opcode, arg].
> +	 * Append a trailing 0x00 to form the full 3-byte MCU command.
> +	 */
> +	u8 cmd[3] = { ((u8 *)reg_buf)[0], ((u8 *)reg_buf)[1], 0x00 };
> +	struct i2c_msg write_msg = {
> +		.addr  = client->addr,
> +		.flags = 0,
> +		.buf   = cmd,
> +		.len   = sizeof(cmd),
> +	};
> +	struct i2c_msg read_msg = {
> +		.addr  = client->addr,
> +		.flags = I2C_M_RD,
> +		.buf   = val_buf,
> +		.len   = val_size,
> +	};
> +	int ret;
> +
> +	ret = i2c_transfer(client->adapter, &write_msg, 1);
> +	if (ret < 0)
> +		return ret;
> +	if (ret != 1)
> +		return -EIO;
> +
> +	ret = i2c_transfer(client->adapter, &read_msg, 1);
> +	if (ret < 0)
> +		return ret;
> +	if (ret != 1)
> +		return -EIO;
> +
> +	return 0;
> +}
> +
> +static const struct regmap_bus aaeon_mcu_regmap_bus = {
> +	.write = aaeon_mcu_regmap_write,
> +	.read  = aaeon_mcu_regmap_read,
> +};
> +
> +static const struct regmap_config aaeon_mcu_regmap_config = {
> +	.reg_bits          = 16,
> +	.val_bits          = 8,
> +	.reg_format_endian = REGMAP_ENDIAN_BIG,
> +	.cache_type        = REGCACHE_NONE,

Are you sure?  Why none?

> +};
> +
> +static int aaeon_mcu_probe(struct i2c_client *client)
> +{
> +	struct regmap *regmap;
> +
> +	regmap = devm_regmap_init(&client->dev, &aaeon_mcu_regmap_bus,
> +				  client, &aaeon_mcu_regmap_config);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);

dev_err_probe()

> +
> +	return devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
> +				    aaeon_mcu_devs, ARRAY_SIZE(aaeon_mcu_devs),
> +				    NULL, 0, NULL);

Why PLATFORM_DEVID_NONE over AUTO here?

> +}
> +
> +static const struct of_device_id aaeon_mcu_of_match[] = {
> +	{ .compatible = "aaeon,srg-imx8p-mcu" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, aaeon_mcu_of_match);
> +
> +static struct i2c_driver aaeon_mcu_driver = {
> +	.driver = {
> +		.name = "aaeon_mcu",
> +		.of_match_table = aaeon_mcu_of_match,
> +	},
> +	.probe = aaeon_mcu_probe,
> +};
> +module_i2c_driver(aaeon_mcu_driver);
> +
> +MODULE_DESCRIPTION("Aaeon MCU Driver");
> +MODULE_AUTHOR("Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/aaeon-mcu.h b/include/linux/mfd/aaeon-mcu.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..861003f6dfd20424c3785008bd2cf89aaa1715b9
> --- /dev/null
> +++ b/include/linux/mfd/aaeon-mcu.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Aaeon MCU driver definitions
> + *
> + * Copyright (C) 2025 Bootlin
> + * Author: Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>
> + * Author: Thomas Perrot <thomas.perrot@bootlin.com>
> + */

As above.

> +
> +#ifndef __LINUX_MFD_AAEON_MCU_H
> +#define __LINUX_MFD_AAEON_MCU_H
> +
> +/*
> + * MCU register address: the high byte is the command opcode, the low
> + * byte is the argument.  This matches the 3-byte wire format
> + * [opcode, arg, value] used by the MCU I2C protocol.
> + */
> +#define AAEON_MCU_REG(op, arg)	(((op) << 8) | (arg))

Where else is this used?

> +#endif /* __LINUX_MFD_AAEON_MCU_H */
> 
> -- 
> 2.53.0
> 

-- 
Lee Jones [李琼斯]
Re: [PATCH v4 3/5] mfd: aaeon: Add SRG-IMX8P MCU driver
Posted by Thomas Perrot 13 hours ago
Hello Lee,

Thank you for the review. Please find answers to your questions inline,
and the remaining items will be addressed in v5.

On Tue, 2026-03-31 at 14:08 +0100, Lee Jones wrote:
> On Tue, 24 Mar 2026, Thomas Perrot (Schneider Electric) wrote:
> 
> > Add Multi-Function Device (MFD) driver for the Aaeon SRG-IMX8P
> > embedded controller. This driver provides the core I2C
> > communication
> > interface and registers child devices (GPIO and watchdog
> > controllers).
> > 
> > The driver implements a custom regmap bus over I2C to match the
> > MCU's
> > fixed 3-byte command format [opcode, arg, value]. Register
> > addresses
> > are encoded as 16-bit values (opcode << 8 | arg) using the
> > AAEON_MCU_REG() macro defined in the shared header. The regmap
> > instance is shared with child drivers via dev_get_regmap().
> > Concurrent
> > I2C accesses from child drivers are serialized by regmap's built-in
> > locking.
> > 
> > Co-developed-by: Jérémie Dautheribes (Schneider Electric)
> > <jeremie.dautheribes@bootlin.com>
> > Signed-off-by: Jérémie Dautheribes (Schneider Electric)
> > <jeremie.dautheribes@bootlin.com>
> > Signed-off-by: Thomas Perrot (Schneider Electric)
> > <thomas.perrot@bootlin.com>
> > ---
> >  MAINTAINERS                   |   2 +
> >  drivers/mfd/Kconfig           |  10 +++
> >  drivers/mfd/Makefile          |   1 +
> >  drivers/mfd/aaeon-mcu.c       | 155
> > ++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/mfd/aaeon-mcu.h |  20 ++++++
> >  5 files changed, 188 insertions(+)
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index
> > ea9d55f76f3509c7f6ba6d1bc86ca2e2e71aa954..f91b6a1826d04bef8a0f88221
> > f6c8e8a3652cd77 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -191,6 +191,8 @@ M:	Thomas Perrot <thomas.perrot@bootlin.com>
> >  R:	Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>
> >  S:	Maintained
> >  F:	Documentation/devicetree/bindings/mfd/aaeon,srg-imx8p-
> > mcu.yaml
> > +F:	drivers/mfd/aaeon-mcu.c
> > +F:	include/linux/mfd/aaeon-mcu.h
> >  
> >  AAEON UPBOARD FPGA MFD DRIVER
> >  M:	Thomas Richard <thomas.richard@bootlin.com>
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index
> > aace5766b38aa5e46e32a8a7b42eea238159fbcf..7a1ceedece899faad7a03a1fe
> > 7b1c91b72253c05 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -1574,6 +1574,16 @@ config AB8500_CORE
> >  	  the irq_chip parts for handling the Mixed Signal chip
> > events.
> >  	  This chip embeds various other multimedia
> > functionalities as well.
> >  
> > +config MFD_AAEON_MCU
> > +	tristate "Aaeon SRG-IMX8P MCU Driver"
> > +	depends on I2C || COMPILE_TEST
> > +	select MFD_CORE
> > +	help
> > +	  Select this option to enable support for the Aaeon SRG-
> > IMX8P
> > +	  onboard microcontroller (MCU). This driver provides the
> > core
> > +	  functionality to communicate with the MCU over I2C. The
> > MCU
> > +	  provides GPIO and watchdog functionality.
> > +
> >  config MFD_DB8500_PRCMU
> >  	bool "ST-Ericsson DB8500 Power Reset Control Management
> > Unit"
> >  	depends on UX500_SOC_DB8500
> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > index
> > e75e8045c28afae975ac61d282b3b85af5440119..34db5b033584368b7a269b1ee
> > f12528a74baf8f5 100644
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -8,6 +8,7 @@ obj-$(CONFIG_MFD_88PM860X)	+= 88pm860x.o
> >  obj-$(CONFIG_MFD_88PM800)	+= 88pm800.o 88pm80x.o
> >  obj-$(CONFIG_MFD_88PM805)	+= 88pm805.o 88pm80x.o
> >  obj-$(CONFIG_MFD_88PM886_PMIC)	+= 88pm886.o
> > +obj-$(CONFIG_MFD_AAEON_MCU)	+= aaeon-mcu.o
> >  obj-$(CONFIG_MFD_ACT8945A)	+= act8945a.o
> >  obj-$(CONFIG_MFD_SM501)		+= sm501.o
> >  obj-$(CONFIG_ARCH_BCM2835)	+= bcm2835-pm.o
> > diff --git a/drivers/mfd/aaeon-mcu.c b/drivers/mfd/aaeon-mcu.c
> > new file mode 100644
> > index
> > 0000000000000000000000000000000000000000..5a969890d201c027eb25c324b
> > 4d4d89b1f8c563e
> > --- /dev/null
> > +++ b/drivers/mfd/aaeon-mcu.c
> > @@ -0,0 +1,155 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Aaeon MCU driver
> > + *
> > + * Copyright (C) 2025 Bootlin
> > + * Author: Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>
> > + * Author: Thomas Perrot <thomas.perrot@bootlin.com>
> > + */
> 
> Consider updating the Copyright date - we're pretty deep into 2026 at
> this point.
> 
> > +#include <linux/err.h>
> > +#include <linux/i2c.h>
> > +#include <linux/mfd/core.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +
> > +static const struct mfd_cell aaeon_mcu_devs[] = {
> > +	{
> > +		.name = "aaeon-mcu-wdt",
> > +	},
> > +	{
> > +		.name = "aaeon-mcu-gpio",
> > +	},
> > +};
> 
> MFD_CELL_BASIC()
> 
> > +/*
> > + * Custom regmap bus for the Aaeon MCU I2C protocol.
> > + *
> > + * The MCU uses a fixed 3-byte command format [opcode, arg, value]
> > followed
> > + * by a 1-byte response.  It requires a STOP condition between the
> > command
> > + * write and the response read, so two separate i2c_transfer()
> > calls are
> > + * issued.  The regmap lock serialises concurrent accesses from
> > the GPIO
> > + * and watchdog child drivers.
> > + *
> > + * Register addresses are encoded as a 16-bit big-endian value
> > where the
> > + * high byte is the opcode and the low byte is the argument,
> > matching the
> > + * wire layout produced by regmap for reg_bits=16.
> > + */
> > +
> > +static int aaeon_mcu_regmap_write(void *context, const void *data,
> > size_t count)
> > +{
> > +	struct i2c_client *client = context;
> > +	/* data = [opcode, arg, value] as formatted by regmap */
> > +	struct i2c_msg write_msg = {
> > +		.addr  = client->addr,
> > +		.flags = 0,
> > +		.buf   = (u8 *)data,
> > +		.len   = count,
> > +	};
> > +	u8 rsp;
> > +	/* The MCU always sends a response byte after each
> > command; discard it. */
> > +	struct i2c_msg rsp_msg = {
> 
> Assuming 'rsp' means response, let's just write that out in full.
> 
> Readability wins over brevity every time.
> 
> > +		.addr  = client->addr,
> > +		.flags = I2C_M_RD,
> > +		.buf   = &rsp,
> > +		.len   = 1,
> > +	};
> > +	int ret;
> 
> Since some I2C host controllers might use DMA, should we ensure that
> the
> 'rsp' buffer is allocated in DMA-safe memory rather than on the stack
> to
> prevent potential cache-line corruption?
> 
> Also allocation of structs during in declaration statements is rough!
> 
> And adding that u8 in the middle is just rubbing it in.
> 
> > +	ret = i2c_transfer(client->adapter, &write_msg, 1);
> > +	if (ret < 0)
> > +		return ret;
> > +	if (ret != 1)
> > +		return -EIO;
> > +
> > +	ret = i2c_transfer(client->adapter, &rsp_msg, 1);
> > +	if (ret < 0)
> > +		return ret;
> > +	if (ret != 1)
> > +		return -EIO;
> > +
> > +	return 0;
> > +}
> > +
> > +static int aaeon_mcu_regmap_read(void *context, const void
> > *reg_buf,
> > +				 size_t reg_size, void *val_buf,
> > size_t val_size)
> > +{
> > +	struct i2c_client *client = context;
> > +	/*
> > +	 * reg_buf holds the 2-byte big-endian register address
> > [opcode, arg].
> > +	 * Append a trailing 0x00 to form the full 3-byte MCU
> > command.
> > +	 */
> > +	u8 cmd[3] = { ((u8 *)reg_buf)[0], ((u8 *)reg_buf)[1], 0x00
> > };
> > +	struct i2c_msg write_msg = {
> > +		.addr  = client->addr,
> > +		.flags = 0,
> > +		.buf   = cmd,
> > +		.len   = sizeof(cmd),
> > +	};
> > +	struct i2c_msg read_msg = {
> > +		.addr  = client->addr,
> > +		.flags = I2C_M_RD,
> > +		.buf   = val_buf,
> > +		.len   = val_size,
> > +	};
> > +	int ret;
> > +
> > +	ret = i2c_transfer(client->adapter, &write_msg, 1);
> > +	if (ret < 0)
> > +		return ret;
> > +	if (ret != 1)
> > +		return -EIO;
> > +
> > +	ret = i2c_transfer(client->adapter, &read_msg, 1);
> > +	if (ret < 0)
> > +		return ret;
> > +	if (ret != 1)
> > +		return -EIO;
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct regmap_bus aaeon_mcu_regmap_bus = {
> > +	.write = aaeon_mcu_regmap_write,
> > +	.read  = aaeon_mcu_regmap_read,
> > +};
> > +
> > +static const struct regmap_config aaeon_mcu_regmap_config = {
> > +	.reg_bits          = 16,
> > +	.val_bits          = 8,
> > +	.reg_format_endian = REGMAP_ENDIAN_BIG,
> > +	.cache_type        = REGCACHE_NONE,
> 
> Are you sure?  Why none?

The GPIO and watchdog states are managed entirely by the MCU firmware,
which makes the design safer because every access goes directly to the
hardware. I will look into adding a cache; otherwise I will add a
comment in v5.

> 
> > +};
> > +
> > +static int aaeon_mcu_probe(struct i2c_client *client)
> > +{
> > +	struct regmap *regmap;
> > +
> > +	regmap = devm_regmap_init(&client->dev,
> > &aaeon_mcu_regmap_bus,
> > +				  client,
> > &aaeon_mcu_regmap_config);
> > +	if (IS_ERR(regmap))
> > +		return PTR_ERR(regmap);
> 
> dev_err_probe()
> 
> > +
> > +	return devm_mfd_add_devices(&client->dev,
> > PLATFORM_DEVID_NONE,
> > +				    aaeon_mcu_devs,
> > ARRAY_SIZE(aaeon_mcu_devs),
> > +				    NULL, 0, NULL);
> 
> Why PLATFORM_DEVID_NONE over AUTO here?

No strong reason, it was an oversight. Since multiple instances of this
MCU could theoretically be present, AUTO is the safer choice and avoids
potential ID collisions.
Fixed in v5.

> 
> > +}
> > +
> > +static const struct of_device_id aaeon_mcu_of_match[] = {
> > +	{ .compatible = "aaeon,srg-imx8p-mcu" },
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, aaeon_mcu_of_match);
> > +
> > +static struct i2c_driver aaeon_mcu_driver = {
> > +	.driver = {
> > +		.name = "aaeon_mcu",
> > +		.of_match_table = aaeon_mcu_of_match,
> > +	},
> > +	.probe = aaeon_mcu_probe,
> > +};
> > +module_i2c_driver(aaeon_mcu_driver);
> > +
> > +MODULE_DESCRIPTION("Aaeon MCU Driver");
> > +MODULE_AUTHOR("Jérémie Dautheribes
> > <jeremie.dautheribes@bootlin.com>");
> > +MODULE_LICENSE("GPL");
> > diff --git a/include/linux/mfd/aaeon-mcu.h
> > b/include/linux/mfd/aaeon-mcu.h
> > new file mode 100644
> > index
> > 0000000000000000000000000000000000000000..861003f6dfd20424c3785008b
> > d2cf89aaa1715b9
> > --- /dev/null
> > +++ b/include/linux/mfd/aaeon-mcu.h
> > @@ -0,0 +1,20 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Aaeon MCU driver definitions
> > + *
> > + * Copyright (C) 2025 Bootlin
> > + * Author: Jérémie Dautheribes <jeremie.dautheribes@bootlin.com>
> > + * Author: Thomas Perrot <thomas.perrot@bootlin.com>
> > + */
> 
> As above.
> 
> > +
> > +#ifndef __LINUX_MFD_AAEON_MCU_H
> > +#define __LINUX_MFD_AAEON_MCU_H
> > +
> > +/*
> > + * MCU register address: the high byte is the command opcode, the
> > low
> > + * byte is the argument.  This matches the 3-byte wire format
> > + * [opcode, arg, value] used by the MCU I2C protocol.
> > + */
> > +#define AAEON_MCU_REG(op, arg)	(((op) << 8) | (arg))
> 
> Where else is this used?

It is used by both child drivers:                                     
  - drivers/gpio/gpio-aaeon-mcu.c
  - drivers/watchdog/aaeon_mcu_wdt.c                                  
                                                            
This macro encodes the regmap register address from the opcode and
argument that form the first two bytes of the MCU's 3-byte wire
command, so keeping it in the shared header avoids duplicating that
encoding in each child.

Kind regards,
Thomas Perrot

> 
> > +#endif /* __LINUX_MFD_AAEON_MCU_H */
> > 
> > -- 
> > 2.53.0
> > 

-- 
Thomas Perrot, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com