[{"id":3671670,"web_url":"http://patchwork.ozlabs.org/comment/3671670/","msgid":"<20260331130848.GG3795166@google.com>","list_archive_url":null,"date":"2026-03-31T13:08:48","subject":"Re: [PATCH v4 3/5] mfd: aaeon: Add SRG-IMX8P MCU driver","submitter":{"id":65833,"url":"http://patchwork.ozlabs.org/api/people/65833/","name":"Lee Jones","email":"lee@kernel.org"},"content":"On Tue, 24 Mar 2026, Thomas Perrot (Schneider Electric) wrote:\n\n> Add Multi-Function Device (MFD) driver for the Aaeon SRG-IMX8P\n> embedded controller. This driver provides the core I2C communication\n> interface and registers child devices (GPIO and watchdog controllers).\n> \n> The driver implements a custom regmap bus over I2C to match the MCU's\n> fixed 3-byte command format [opcode, arg, value]. Register addresses\n> are encoded as 16-bit values (opcode << 8 | arg) using the\n> AAEON_MCU_REG() macro defined in the shared header. The regmap\n> instance is shared with child drivers via dev_get_regmap(). Concurrent\n> I2C accesses from child drivers are serialized by regmap's built-in\n> locking.\n> \n> Co-developed-by: Jérémie Dautheribes (Schneider Electric) \n> Signed-off-by: Jérémie Dautheribes (Schneider Electric) \n> Signed-off-by: Thomas Perrot (Schneider Electric) \n> ---\n> MAINTAINERS | 2 +\n> drivers/mfd/Kconfig | 10 +++\n> drivers/mfd/Makefile | 1 +\n> drivers/mfd/aaeon-mcu.c | 155 ++++++++++++++++++++++++++++++++++++++++++\n> include/linux/mfd/aaeon-mcu.h | 20 ++++++\n> 5 files changed, 188 insertions(+)\n> \n> diff --git a/MAINTAINERS b/MAINTAINERS\n> index ea9d55f76f3509c7f6ba6d1bc86ca2e2e71aa954..f91b6a1826d04bef8a0f88221f6c8e8a3652cd77 100644\n> --- a/MAINTAINERS\n> +++ b/MAINTAINERS\n> @@ -191,6 +191,8 @@ M:\tThomas Perrot \n> R:\tJérémie Dautheribes \n> S:\tMaintained\n> F:\tDocumentation/devicetree/bindings/mfd/aaeon,srg-imx8p-mcu.yaml\n> +F:\tdrivers/mfd/aaeon-mcu.c\n> +F:\tinclude/linux/mfd/aaeon-mcu.h\n> \n> AAEON UPBOARD FPGA MFD DRIVER\n> M:\tThomas Richard \n> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig\n> index aace5766b38aa5e46e32a8a7b42eea238159fbcf..7a1ceedece899faad7a03a1fe7b1c91b72253c05 100644\n> --- a/drivers/mfd/Kconfig\n> +++ b/drivers/mfd/Kconfig\n> @@ -1574,6 +1574,16 @@ config AB8500_CORE\n> \t the irq_chip parts for handling the Mixed Signal chip events.\n> \t This chip embeds various other multimedia functionalities as well.\n> \n> +config MFD_AAEON_MCU\n> +\ttristate \"Aaeon SRG-IMX8P MCU Driver\"\n> +\tdepends on I2C || COMPILE_TEST\n> +\tselect MFD_CORE\n> +\thelp\n> +\t Select this option to enable support for the Aaeon SRG-IMX8P\n> +\t onboard microcontroller (MCU). This driver provides the core\n> +\t functionality to communicate with the MCU over I2C. The MCU\n> +\t provides GPIO and watchdog functionality.\n> +\n> config MFD_DB8500_PRCMU\n> \tbool \"ST-Ericsson DB8500 Power Reset Control Management Unit\"\n> \tdepends on UX500_SOC_DB8500\n> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile\n> index e75e8045c28afae975ac61d282b3b85af5440119..34db5b033584368b7a269b1eef12528a74baf8f5 100644\n> --- a/drivers/mfd/Makefile\n> +++ b/drivers/mfd/Makefile\n> @@ -8,6 +8,7 @@ obj-$(CONFIG_MFD_88PM860X)\t+= 88pm860x.o\n> obj-$(CONFIG_MFD_88PM800)\t+= 88pm800.o 88pm80x.o\n> obj-$(CONFIG_MFD_88PM805)\t+= 88pm805.o 88pm80x.o\n> obj-$(CONFIG_MFD_88PM886_PMIC)\t+= 88pm886.o\n> +obj-$(CONFIG_MFD_AAEON_MCU)\t+= aaeon-mcu.o\n> obj-$(CONFIG_MFD_ACT8945A)\t+= act8945a.o\n> obj-$(CONFIG_MFD_SM501)\t\t+= sm501.o\n> obj-$(CONFIG_ARCH_BCM2835)\t+= bcm2835-pm.o\n> diff --git a/drivers/mfd/aaeon-mcu.c b/drivers/mfd/aaeon-mcu.c\n> new file mode 100644\n> index 0000000000000000000000000000000000000000..5a969890d201c027eb25c324b4d4d89b1f8c563e\n> --- /dev/null\n> +++ b/drivers/mfd/aaeon-mcu.c\n> @@ -0,0 +1,155 @@\n> +// SPDX-License-Identifier: GPL-2.0-or-later\n> +/*\n> + * Aaeon MCU driver\n> + *\n> + * Copyright (C) 2025 Bootlin\n> + * Author: Jérémie Dautheribes \n> + * Author: Thomas Perrot \n> + */\n\nConsider updating the Copyright date - we're pretty deep into 2026 at this point.\n\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +static const struct mfd_cell aaeon_mcu_devs[] = {\n> +\t{\n> +\t\t.name = \"aaeon-mcu-wdt\",\n> +\t},\n> +\t{\n> +\t\t.name = \"aaeon-mcu-gpio\",\n> +\t},\n> +};\n\nMFD_CELL_BASIC()\n\n> +/*\n> + * Custom regmap bus for the Aaeon MCU I2C protocol.\n> + *\n> + * The MCU uses a fixed 3-byte command format [opcode, arg, value] followed\n> + * by a 1-byte response. It requires a STOP condition between the command\n> + * write and the response read, so two separate i2c_transfer() calls are\n> + * issued. The regmap lock serialises concurrent accesses from the GPIO\n> + * and watchdog child drivers.\n> + *\n> + * Register addresses are encoded as a 16-bit big-endian value where the\n> + * high byte is the opcode and the low byte is the argument, matching the\n> + * wire layout produced by regmap for reg_bits=16.\n> + */\n> +\n> +static int aaeon_mcu_regmap_write(void *context, const void *data, size_t count)\n> +{\n> +\tstruct i2c_client *client = context;\n> +\t/* data = [opcode, arg, value] as formatted by regmap */\n> +\tstruct i2c_msg write_msg = {\n> +\t\t.addr = client->addr,\n> +\t\t.flags = 0,\n> +\t\t.buf = (u8 *)data,\n> +\t\t.len = count,\n> +\t};\n> +\tu8 rsp;\n> +\t/* The MCU always sends a response byte after each command; discard it. */\n> +\tstruct i2c_msg rsp_msg = {\n\nAssuming 'rsp' means response, let's just write that out in full.\n\nReadability wins over brevity every time.\n\n> +\t\t.addr = client->addr,\n> +\t\t.flags = I2C_M_RD,\n> +\t\t.buf = &rsp,\n> +\t\t.len = 1,\n> +\t};\n> +\tint ret;\n\nSince some I2C host controllers might use DMA, should we ensure that the\n'rsp' buffer is allocated in DMA-safe memory rather than on the stack to\nprevent potential cache-line corruption?\n\nAlso allocation of structs during in declaration statements is rough!\n\nAnd adding that u8 in the middle is just rubbing it in.\n\n> +\tret = i2c_transfer(client->adapter, &write_msg, 1);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\tif (ret != 1)\n> +\t\treturn -EIO;\n> +\n> +\tret = i2c_transfer(client->adapter, &rsp_msg, 1);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\tif (ret != 1)\n> +\t\treturn -EIO;\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int aaeon_mcu_regmap_read(void *context, const void *reg_buf,\n> +\t\t\t\t size_t reg_size, void *val_buf, size_t val_size)\n> +{\n> +\tstruct i2c_client *client = context;\n> +\t/*\n> +\t * reg_buf holds the 2-byte big-endian register address [opcode, arg].\n> +\t * Append a trailing 0x00 to form the full 3-byte MCU command.\n> +\t */\n> +\tu8 cmd[3] = { ((u8 *)reg_buf)[0], ((u8 *)reg_buf)[1], 0x00 };\n> +\tstruct i2c_msg write_msg = {\n> +\t\t.addr = client->addr,\n> +\t\t.flags = 0,\n> +\t\t.buf = cmd,\n> +\t\t.len = sizeof(cmd),\n> +\t};\n> +\tstruct i2c_msg read_msg = {\n> +\t\t.addr = client->addr,\n> +\t\t.flags = I2C_M_RD,\n> +\t\t.buf = val_buf,\n> +\t\t.len = val_size,\n> +\t};\n> +\tint ret;\n> +\n> +\tret = i2c_transfer(client->adapter, &write_msg, 1);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\tif (ret != 1)\n> +\t\treturn -EIO;\n> +\n> +\tret = i2c_transfer(client->adapter, &read_msg, 1);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\tif (ret != 1)\n> +\t\treturn -EIO;\n> +\n> +\treturn 0;\n> +}\n> +\n> +static const struct regmap_bus aaeon_mcu_regmap_bus = {\n> +\t.write = aaeon_mcu_regmap_write,\n> +\t.read = aaeon_mcu_regmap_read,\n> +};\n> +\n> +static const struct regmap_config aaeon_mcu_regmap_config = {\n> +\t.reg_bits = 16,\n> +\t.val_bits = 8,\n> +\t.reg_format_endian = REGMAP_ENDIAN_BIG,\n> +\t.cache_type = REGCACHE_NONE,\n\nAre you sure? Why none?\n\n> +};\n> +\n> +static int aaeon_mcu_probe(struct i2c_client *client)\n> +{\n> +\tstruct regmap *regmap;\n> +\n> +\tregmap = devm_regmap_init(&client->dev, &aaeon_mcu_regmap_bus,\n> +\t\t\t\t client, &aaeon_mcu_regmap_config);\n> +\tif (IS_ERR(regmap))\n> +\t\treturn PTR_ERR(regmap);\n\ndev_err_probe()\n\n> +\n> +\treturn devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,\n> +\t\t\t\t aaeon_mcu_devs, ARRAY_SIZE(aaeon_mcu_devs),\n> +\t\t\t\t NULL, 0, NULL);\n\nWhy PLATFORM_DEVID_NONE over AUTO here?\n\n> +}\n> +\n> +static const struct of_device_id aaeon_mcu_of_match[] = {\n> +\t{ .compatible = \"aaeon,srg-imx8p-mcu\" },\n> +\t{},\n> +};\n> +MODULE_DEVICE_TABLE(of, aaeon_mcu_of_match);\n> +\n> +static struct i2c_driver aaeon_mcu_driver = {\n> +\t.driver = {\n> +\t\t.name = \"aaeon_mcu\",\n> +\t\t.of_match_table = aaeon_mcu_of_match,\n> +\t},\n> +\t.probe = aaeon_mcu_probe,\n> +};\n> +module_i2c_driver(aaeon_mcu_driver);\n> +\n> +MODULE_DESCRIPTION(\"Aaeon MCU Driver\");\n> +MODULE_AUTHOR(\"Jérémie Dautheribes \");\n> +MODULE_LICENSE(\"GPL\");\n> diff --git a/include/linux/mfd/aaeon-mcu.h b/include/linux/mfd/aaeon-mcu.h\n> new file mode 100644\n> index 0000000000000000000000000000000000000000..861003f6dfd20424c3785008bd2cf89aaa1715b9\n> --- /dev/null\n> +++ b/include/linux/mfd/aaeon-mcu.h\n> @@ -0,0 +1,20 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Aaeon MCU driver definitions\n> + *\n> + * Copyright (C) 2025 Bootlin\n> + * Author: Jérémie Dautheribes \n> + * Author: Thomas Perrot \n> + */\n\nAs above.\n\n> +\n> +#ifndef __LINUX_MFD_AAEON_MCU_H\n> +#define __LINUX_MFD_AAEON_MCU_H\n> +\n> +/*\n> + * MCU register address: the high byte is the command opcode, the low\n> + * byte is the argument. This matches the 3-byte wire format\n> + * [opcode, arg, value] used by the MCU I2C protocol.\n> + */\n> +#define AAEON_MCU_REG(op, arg)\t(((op) << 8) | (arg))\n\nWhere else is this used?\n\n> +#endif /* __LINUX_MFD_AAEON_MCU_H */\n> \n> -- \n> 2.53.0\n>","headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","linux-gpio@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256\n header.s=k20201202 header.b=P1b8yC8J;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c04:e001:36c::12fc:5321; helo=tor.lore.kernel.org;\n envelope-from=linux-gpio+bounces-34480-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=\"P1b8yC8J\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=10.30.226.201"],"Received":["from tor.lore.kernel.org (tor.lore.kernel.org\n [IPv6:2600:3c04:e001:36c::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4flT743chmz1yCp\n\tfor ; Wed, 01 Apr 2026 00:12:32 +1100 (AEDT)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby tor.lore.kernel.org (Postfix) with ESMTP id 166373030D7C\n\tfor ; Tue, 31 Mar 2026 13:09:00 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id E6A103E3C61;\n\tTue, 31 Mar 2026 13:08:55 +0000 (UTC)","from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org\n [10.30.226.201])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id A665E346FB5;\n\tTue, 31 Mar 2026 13:08:55 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id 26F71C19423;\n\tTue, 31 Mar 2026 13:08:50 +0000 (UTC)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1774962535; cv=none;\n b=emMlfxGOagOAyvB/pnIvTJyMPJ4v09uoJZvAP2UebaappX7FnaVRhtah+KJCfX5A/cNTycYF409Y1LNII39b18RA9B62zexf4KycNece0uovVtK61A3OQxV+y1zOIAPi+bbB1ePBwx9c9Af0RwWM0IYQArmz4L9a64LjnV5m504=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1774962535; c=relaxed/simple;\n\tbh=7G9vqJ4wgnpJpVy5WKsHllojY9IC+YZBWuAMWGOZ1wI=;\n\th=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version:\n\t Content-Type:Content-Disposition:In-Reply-To;\n b=YCLipF71jNL6OIGFfVd6DGqEksWihZGXJLMZtws/W7KLpac/PHCHcuCtVY6zuGqnxnFqb6L9MhUXOA5swR1CdWmDYwFANVesDppZLktbf8gyKVUYKj9YW0qR7O3mfFJyFjdqHamQ9ZXu984amRmk7w08Nkr6GN/Ium1Xd7zxvxw=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=P1b8yC8J; arc=none smtp.client-ip=10.30.226.201","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1774962535;\n\tbh=7G9vqJ4wgnpJpVy5WKsHllojY9IC+YZBWuAMWGOZ1wI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=P1b8yC8JyR4L1I+FqhRjqgpHUVOJKsajNu8+ZCU+ga9haoQ8sMrU9y/vwlj+fNd/P\n\t cuZKWYbwf1c0KooGYpeeKVsv6qryybEchw769h8nrtclQAsQlC8rJ2RsUgYQJ0yohO\n\t 0qpnw62jqBV1rKGmhdSLRIx3vrWNJbbQPCw1IpfCMtz8WAp6IxiaTja0s1PO8Vx0H5\n\t 4uskdqK1cExqK7W9lBTARlkVbJQujCLGjnfz29NZ1okFh/Y+1DtFkFpUAe6iUC42gI\n\t vfC6WE8XhxJPAgRA/eHlAu4cfsALbSGxD77OAgHjf7J6M1gvcJCViRq3RZMlKjPFWd\n\t mMs1U3SpSnW7Q==","Date":"Tue, 31 Mar 2026 14:08:48 +0100","From":"Lee Jones ","To":"\"Thomas Perrot (Schneider Electric)\" ","Cc":"Rob Herring , Krzysztof Kozlowski ,\n Conor Dooley , Linus Walleij ,\n Bartosz Golaszewski , Shawn Guo ,\n Sascha Hauer ,\n Pengutronix Kernel Team ,\n Fabio Estevam ,\n =?iso-8859-1?q?J=E9r=E9mie?= Dautheribes ,\n Wim Van Sebroeck ,\n Guenter Roeck , devicetree@vger.kernel.org,\n linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org,\n imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,\n linux-watchdog@vger.kernel.org,\n Thomas Petazzoni ,\n Miquel Raynal ","Subject":"Re: [PATCH v4 3/5] mfd: aaeon: Add SRG-IMX8P MCU driver","Message-ID":"<20260331130848.GG3795166@google.com>","References":"<20260324-dev-b4-aaeon-mcu-driver-v4-0-afb011df4794@bootlin.com>\n <20260324-dev-b4-aaeon-mcu-driver-v4-3-afb011df4794@bootlin.com>","Precedence":"bulk","X-Mailing-List":"linux-gpio@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":"","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20260324-dev-b4-aaeon-mcu-driver-v4-3-afb011df4794@bootlin.com>"}},{"id":3672291,"web_url":"http://patchwork.ozlabs.org/comment/3672291/","msgid":"<9a66f7612a8d96d7c44e86a9426a8c553329580b.camel@bootlin.com>","list_archive_url":null,"date":"2026-04-01T13:48:08","subject":"Re: [PATCH v4 3/5] mfd: aaeon: Add SRG-IMX8P MCU driver","submitter":{"id":82054,"url":"http://patchwork.ozlabs.org/api/people/82054/","name":"Thomas Perrot (Schneider Electric)","email":"thomas.perrot@bootlin.com"},"content":"Hello Lee,\n\nThank you for the review. Please find answers to your questions inline,\nand the remaining items will be addressed in v5.\n\nOn Tue, 2026-03-31 at 14:08 +0100, Lee Jones wrote:\n> On Tue, 24 Mar 2026, Thomas Perrot (Schneider Electric) wrote:\n> \n> > Add Multi-Function Device (MFD) driver for the Aaeon SRG-IMX8P\n> > embedded controller. This driver provides the core I2C\n> > communication\n> > interface and registers child devices (GPIO and watchdog\n> > controllers).\n> > \n> > The driver implements a custom regmap bus over I2C to match the\n> > MCU's\n> > fixed 3-byte command format [opcode, arg, value]. Register\n> > addresses\n> > are encoded as 16-bit values (opcode << 8 | arg) using the\n> > AAEON_MCU_REG() macro defined in the shared header. The regmap\n> > instance is shared with child drivers via dev_get_regmap().\n> > Concurrent\n> > I2C accesses from child drivers are serialized by regmap's built-in\n> > locking.\n> > \n> > Co-developed-by: Jérémie Dautheribes (Schneider Electric)\n> > \n> > Signed-off-by: Jérémie Dautheribes (Schneider Electric)\n> > \n> > Signed-off-by: Thomas Perrot (Schneider Electric)\n> > \n> > ---\n> >  MAINTAINERS                   |   2 +\n> >  drivers/mfd/Kconfig           |  10 +++\n> >  drivers/mfd/Makefile          |   1 +\n> >  drivers/mfd/aaeon-mcu.c       | 155\n> > ++++++++++++++++++++++++++++++++++++++++++\n> >  include/linux/mfd/aaeon-mcu.h |  20 ++++++\n> >  5 files changed, 188 insertions(+)\n> > \n> > diff --git a/MAINTAINERS b/MAINTAINERS\n> > index\n> > ea9d55f76f3509c7f6ba6d1bc86ca2e2e71aa954..f91b6a1826d04bef8a0f88221\n> > f6c8e8a3652cd77 100644\n> > --- a/MAINTAINERS\n> > +++ b/MAINTAINERS\n> > @@ -191,6 +191,8 @@ M:\tThomas Perrot \n> >  R:\tJérémie Dautheribes \n> >  S:\tMaintained\n> >  F:\tDocumentation/devicetree/bindings/mfd/aaeon,srg-imx8p-\n> > mcu.yaml\n> > +F:\tdrivers/mfd/aaeon-mcu.c\n> > +F:\tinclude/linux/mfd/aaeon-mcu.h\n> >  \n> >  AAEON UPBOARD FPGA MFD DRIVER\n> >  M:\tThomas Richard \n> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig\n> > index\n> > aace5766b38aa5e46e32a8a7b42eea238159fbcf..7a1ceedece899faad7a03a1fe\n> > 7b1c91b72253c05 100644\n> > --- a/drivers/mfd/Kconfig\n> > +++ b/drivers/mfd/Kconfig\n> > @@ -1574,6 +1574,16 @@ config AB8500_CORE\n> >  \t  the irq_chip parts for handling the Mixed Signal chip\n> > events.\n> >  \t  This chip embeds various other multimedia\n> > functionalities as well.\n> >  \n> > +config MFD_AAEON_MCU\n> > +\ttristate \"Aaeon SRG-IMX8P MCU Driver\"\n> > +\tdepends on I2C || COMPILE_TEST\n> > +\tselect MFD_CORE\n> > +\thelp\n> > +\t  Select this option to enable support for the Aaeon SRG-\n> > IMX8P\n> > +\t  onboard microcontroller (MCU). This driver provides the\n> > core\n> > +\t  functionality to communicate with the MCU over I2C. The\n> > MCU\n> > +\t  provides GPIO and watchdog functionality.\n> > +\n> >  config MFD_DB8500_PRCMU\n> >  \tbool \"ST-Ericsson DB8500 Power Reset Control Management\n> > Unit\"\n> >  \tdepends on UX500_SOC_DB8500\n> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile\n> > index\n> > e75e8045c28afae975ac61d282b3b85af5440119..34db5b033584368b7a269b1ee\n> > f12528a74baf8f5 100644\n> > --- a/drivers/mfd/Makefile\n> > +++ b/drivers/mfd/Makefile\n> > @@ -8,6 +8,7 @@ obj-$(CONFIG_MFD_88PM860X)\t+= 88pm860x.o\n> >  obj-$(CONFIG_MFD_88PM800)\t+= 88pm800.o 88pm80x.o\n> >  obj-$(CONFIG_MFD_88PM805)\t+= 88pm805.o 88pm80x.o\n> >  obj-$(CONFIG_MFD_88PM886_PMIC)\t+= 88pm886.o\n> > +obj-$(CONFIG_MFD_AAEON_MCU)\t+= aaeon-mcu.o\n> >  obj-$(CONFIG_MFD_ACT8945A)\t+= act8945a.o\n> >  obj-$(CONFIG_MFD_SM501)\t\t+= sm501.o\n> >  obj-$(CONFIG_ARCH_BCM2835)\t+= bcm2835-pm.o\n> > diff --git a/drivers/mfd/aaeon-mcu.c b/drivers/mfd/aaeon-mcu.c\n> > new file mode 100644\n> > index\n> > 0000000000000000000000000000000000000000..5a969890d201c027eb25c324b\n> > 4d4d89b1f8c563e\n> > --- /dev/null\n> > +++ b/drivers/mfd/aaeon-mcu.c\n> > @@ -0,0 +1,155 @@\n> > +// SPDX-License-Identifier: GPL-2.0-or-later\n> > +/*\n> > + * Aaeon MCU driver\n> > + *\n> > + * Copyright (C) 2025 Bootlin\n> > + * Author: Jérémie Dautheribes \n> > + * Author: Thomas Perrot \n> > + */\n> \n> Consider updating the Copyright date - we're pretty deep into 2026 at\n> this point.\n> \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +static const struct mfd_cell aaeon_mcu_devs[] = {\n> > +\t{\n> > +\t\t.name = \"aaeon-mcu-wdt\",\n> > +\t},\n> > +\t{\n> > +\t\t.name = \"aaeon-mcu-gpio\",\n> > +\t},\n> > +};\n> \n> MFD_CELL_BASIC()\n> \n> > +/*\n> > + * Custom regmap bus for the Aaeon MCU I2C protocol.\n> > + *\n> > + * The MCU uses a fixed 3-byte command format [opcode, arg, value]\n> > followed\n> > + * by a 1-byte response.  It requires a STOP condition between the\n> > command\n> > + * write and the response read, so two separate i2c_transfer()\n> > calls are\n> > + * issued.  The regmap lock serialises concurrent accesses from\n> > the GPIO\n> > + * and watchdog child drivers.\n> > + *\n> > + * Register addresses are encoded as a 16-bit big-endian value\n> > where the\n> > + * high byte is the opcode and the low byte is the argument,\n> > matching the\n> > + * wire layout produced by regmap for reg_bits=16.\n> > + */\n> > +\n> > +static int aaeon_mcu_regmap_write(void *context, const void *data,\n> > size_t count)\n> > +{\n> > +\tstruct i2c_client *client = context;\n> > +\t/* data = [opcode, arg, value] as formatted by regmap */\n> > +\tstruct i2c_msg write_msg = {\n> > +\t\t.addr  = client->addr,\n> > +\t\t.flags = 0,\n> > +\t\t.buf   = (u8 *)data,\n> > +\t\t.len   = count,\n> > +\t};\n> > +\tu8 rsp;\n> > +\t/* The MCU always sends a response byte after each\n> > command; discard it. */\n> > +\tstruct i2c_msg rsp_msg = {\n> \n> Assuming 'rsp' means response, let's just write that out in full.\n> \n> Readability wins over brevity every time.\n> \n> > +\t\t.addr  = client->addr,\n> > +\t\t.flags = I2C_M_RD,\n> > +\t\t.buf   = &rsp,\n> > +\t\t.len   = 1,\n> > +\t};\n> > +\tint ret;\n> \n> Since some I2C host controllers might use DMA, should we ensure that\n> the\n> 'rsp' buffer is allocated in DMA-safe memory rather than on the stack\n> to\n> prevent potential cache-line corruption?\n> \n> Also allocation of structs during in declaration statements is rough!\n> \n> And adding that u8 in the middle is just rubbing it in.\n> \n> > +\tret = i2c_transfer(client->adapter, &write_msg, 1);\n> > +\tif (ret < 0)\n> > +\t\treturn ret;\n> > +\tif (ret != 1)\n> > +\t\treturn -EIO;\n> > +\n> > +\tret = i2c_transfer(client->adapter, &rsp_msg, 1);\n> > +\tif (ret < 0)\n> > +\t\treturn ret;\n> > +\tif (ret != 1)\n> > +\t\treturn -EIO;\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +static int aaeon_mcu_regmap_read(void *context, const void\n> > *reg_buf,\n> > +\t\t\t\t size_t reg_size, void *val_buf,\n> > size_t val_size)\n> > +{\n> > +\tstruct i2c_client *client = context;\n> > +\t/*\n> > +\t * reg_buf holds the 2-byte big-endian register address\n> > [opcode, arg].\n> > +\t * Append a trailing 0x00 to form the full 3-byte MCU\n> > command.\n> > +\t */\n> > +\tu8 cmd[3] = { ((u8 *)reg_buf)[0], ((u8 *)reg_buf)[1], 0x00\n> > };\n> > +\tstruct i2c_msg write_msg = {\n> > +\t\t.addr  = client->addr,\n> > +\t\t.flags = 0,\n> > +\t\t.buf   = cmd,\n> > +\t\t.len   = sizeof(cmd),\n> > +\t};\n> > +\tstruct i2c_msg read_msg = {\n> > +\t\t.addr  = client->addr,\n> > +\t\t.flags = I2C_M_RD,\n> > +\t\t.buf   = val_buf,\n> > +\t\t.len   = val_size,\n> > +\t};\n> > +\tint ret;\n> > +\n> > +\tret = i2c_transfer(client->adapter, &write_msg, 1);\n> > +\tif (ret < 0)\n> > +\t\treturn ret;\n> > +\tif (ret != 1)\n> > +\t\treturn -EIO;\n> > +\n> > +\tret = i2c_transfer(client->adapter, &read_msg, 1);\n> > +\tif (ret < 0)\n> > +\t\treturn ret;\n> > +\tif (ret != 1)\n> > +\t\treturn -EIO;\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +static const struct regmap_bus aaeon_mcu_regmap_bus = {\n> > +\t.write = aaeon_mcu_regmap_write,\n> > +\t.read  = aaeon_mcu_regmap_read,\n> > +};\n> > +\n> > +static const struct regmap_config aaeon_mcu_regmap_config = {\n> > +\t.reg_bits          = 16,\n> > +\t.val_bits          = 8,\n> > +\t.reg_format_endian = REGMAP_ENDIAN_BIG,\n> > +\t.cache_type        = REGCACHE_NONE,\n> \n> Are you sure?  Why none?\n\nThe GPIO and watchdog states are managed entirely by the MCU firmware,\nwhich makes the design safer because every access goes directly to the\nhardware. I will look into adding a cache; otherwise I will add a\ncomment in v5.\n\n> \n> > +};\n> > +\n> > +static int aaeon_mcu_probe(struct i2c_client *client)\n> > +{\n> > +\tstruct regmap *regmap;\n> > +\n> > +\tregmap = devm_regmap_init(&client->dev,\n> > &aaeon_mcu_regmap_bus,\n> > +\t\t\t\t  client,\n> > &aaeon_mcu_regmap_config);\n> > +\tif (IS_ERR(regmap))\n> > +\t\treturn PTR_ERR(regmap);\n> \n> dev_err_probe()\n> \n> > +\n> > +\treturn devm_mfd_add_devices(&client->dev,\n> > PLATFORM_DEVID_NONE,\n> > +\t\t\t\t    aaeon_mcu_devs,\n> > ARRAY_SIZE(aaeon_mcu_devs),\n> > +\t\t\t\t    NULL, 0, NULL);\n> \n> Why PLATFORM_DEVID_NONE over AUTO here?\n\nNo strong reason, it was an oversight. Since multiple instances of this\nMCU could theoretically be present, AUTO is the safer choice and avoids\npotential ID collisions.\nFixed in v5.\n\n> \n> > +}\n> > +\n> > +static const struct of_device_id aaeon_mcu_of_match[] = {\n> > +\t{ .compatible = \"aaeon,srg-imx8p-mcu\" },\n> > +\t{},\n> > +};\n> > +MODULE_DEVICE_TABLE(of, aaeon_mcu_of_match);\n> > +\n> > +static struct i2c_driver aaeon_mcu_driver = {\n> > +\t.driver = {\n> > +\t\t.name = \"aaeon_mcu\",\n> > +\t\t.of_match_table = aaeon_mcu_of_match,\n> > +\t},\n> > +\t.probe = aaeon_mcu_probe,\n> > +};\n> > +module_i2c_driver(aaeon_mcu_driver);\n> > +\n> > +MODULE_DESCRIPTION(\"Aaeon MCU Driver\");\n> > +MODULE_AUTHOR(\"Jérémie Dautheribes\n> > \");\n> > +MODULE_LICENSE(\"GPL\");\n> > diff --git a/include/linux/mfd/aaeon-mcu.h\n> > b/include/linux/mfd/aaeon-mcu.h\n> > new file mode 100644\n> > index\n> > 0000000000000000000000000000000000000000..861003f6dfd20424c3785008b\n> > d2cf89aaa1715b9\n> > --- /dev/null\n> > +++ b/include/linux/mfd/aaeon-mcu.h\n> > @@ -0,0 +1,20 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Aaeon MCU driver definitions\n> > + *\n> > + * Copyright (C) 2025 Bootlin\n> > + * Author: Jérémie Dautheribes \n> > + * Author: Thomas Perrot \n> > + */\n> \n> As above.\n> \n> > +\n> > +#ifndef __LINUX_MFD_AAEON_MCU_H\n> > +#define __LINUX_MFD_AAEON_MCU_H\n> > +\n> > +/*\n> > + * MCU register address: the high byte is the command opcode, the\n> > low\n> > + * byte is the argument.  This matches the 3-byte wire format\n> > + * [opcode, arg, value] used by the MCU I2C protocol.\n> > + */\n> > +#define AAEON_MCU_REG(op, arg)\t(((op) << 8) | (arg))\n> \n> Where else is this used?\n\nIt is used by both child drivers: \n - drivers/gpio/gpio-aaeon-mcu.c\n - drivers/watchdog/aaeon_mcu_wdt.c \n \nThis macro encodes the regmap register address from the opcode and\nargument that form the first two bytes of the MCU's 3-byte wire\ncommand, so keeping it in the shared header avoids duplicating that\nencoding in each child.\n\nKind regards,\nThomas Perrot\n\n> \n> > +#endif /* __LINUX_MFD_AAEON_MCU_H */\n> > \n> > -- \n> > 2.53.0\n> >","headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","linux-gpio@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=bootlin.com header.i=@bootlin.com header.a=rsa-sha256\n header.s=dkim header.b=tkyzk5+b;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c15:e001:75::12fc:5321; helo=sin.lore.kernel.org;\n envelope-from=linux-gpio+bounces-34562-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com\n header.b=\"tkyzk5+b\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=185.246.84.56","smtp.subspace.kernel.org;\n dmarc=pass (p=reject dis=none) header.from=bootlin.com","smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=bootlin.com"],"Received":["from sin.lore.kernel.org (sin.lore.kernel.org\n [IPv6:2600:3c15:e001:75::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4fm6KQ591sz1yHD\n\tfor ; Thu, 02 Apr 2026 01:08:42 +1100 (AEDT)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sin.lore.kernel.org (Postfix) with ESMTP id 9B6C2304137D\n\tfor ; Wed, 1 Apr 2026 13:48:22 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id CE68443CECB;\n\tWed, 1 Apr 2026 13:48:20 +0000 (UTC)","from smtpout-02.galae.net (smtpout-02.galae.net [185.246.84.56])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id B4EB8408256;\n\tWed, 1 Apr 2026 13:48:17 +0000 (UTC)","from smtpout-01.galae.net (smtpout-01.galae.net [212.83.139.233])\n\tby smtpout-02.galae.net (Postfix) with ESMTPS id 322DB1A315D;\n\tWed, 1 Apr 2026 13:48:16 +0000 (UTC)","from mail.galae.net (mail.galae.net [212.83.136.155])\n\tby smtpout-01.galae.net (Postfix) with ESMTPS id E9686602BF;\n\tWed, 1 Apr 2026 13:48:15 +0000 (UTC)","from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon)\n with ESMTPSA id DB74A10450A06;\n\tWed, 1 Apr 2026 15:48:09 +0200 (CEST)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1775051300; cv=none;\n b=e1gZv80K3TttkFIU5hITfNs7HAxfwErLILt4ajEDjBNeYrTa7qZTeE4N+SZhJN3I9S4oljItx4S61kSCMSwoUiDuNYfkRu2fCr3x8eTGaGBwqx1T/gNYZMicvN2I64KTgFsE5v1v/oxVlGglU13O+SyQxs+jWXBDZ6teKsfKIMY=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1775051300; c=relaxed/simple;\n\tbh=ggdVXHkH3QDx7OVyZru0/lNedC1XWTM1v7xFBHfETDo=;\n\th=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References:\n\t Content-Type:MIME-Version;\n b=bNMYnxQKOcm2X/hK6IFZVgLvM48OIEUSv4oLz5qffMAkYtJ2PjvTCU80KV3HQN3VY5+jSYwBcXdH68BHk4DEIjmQO3nZeV5KJqEc6/MwsPk2fYuS4l+dYyn1QCwil/PaS/U49LSQilfDmZi/GGeT5XVsU27O4OZ8t30Tb+waBkA=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dmarc=pass (p=reject dis=none) header.from=bootlin.com;\n spf=pass smtp.mailfrom=bootlin.com;\n dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com\n header.b=tkyzk5+b; arc=none smtp.client-ip=185.246.84.56","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=dkim;\n\tt=1775051294; h=from:subject:date:message-id:to:cc:mime-version:content-type:\n\t in-reply-to:references:autocrypt;\n\tbh=HoWKSnsKLtxY9sRG37q7JopC8BGz/Pr3NVh3r3H0wtg=;\n\tb=tkyzk5+bcmjJPZWDQVYYUF6MQeyeC3EoCrpJN+CU5Qyq50wiRJGECWFhjdgQzapypJqxYE\n\toCslFr2SxiUnS5Q3TFEv1i+XuIsIYnJoQ6pVcK/UmOc9TOFO4/Hxkk5usw2SAB2ow9warh\n\thiz2tbnra9fG2YNoBe8gVJLwLYAQ191WjyUtZaIFfCQGEq2b16SFqJOI6yEl94mk/5P6UK\n\tpqOZJ4SEmbZJK0hb7h7QZA2GUXPdePGBXh3TP4VugoH2GhPA9/GubbFeMgrTnxm3yUr20u\n\tU2cg3A6aYvxiFLp4UJ2gqWIYP3ggIuNVuK64l7c2FjjPphrPlXeNjZt3uCcD9g==","Message-ID":"<9a66f7612a8d96d7c44e86a9426a8c553329580b.camel@bootlin.com>","Subject":"Re: [PATCH v4 3/5] mfd: aaeon: Add SRG-IMX8P MCU driver","From":"Thomas Perrot ","To":"Lee Jones ","Cc":"thomas.perrot@bootlin.com, Rob Herring ,\n Krzysztof Kozlowski , Conor Dooley ,\n Linus Walleij , Bartosz Golaszewski\t ,\n Shawn Guo , Sascha Hauer\t ,\n Pengutronix Kernel Team ,\n Fabio Estevam ,\n =?iso-8859-1?q?J=E9r=E9mie?= Dautheribes ,\n Wim Van Sebroeck ,\n Guenter Roeck , devicetree@vger.kernel.org,\n \tlinux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org,\n imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,\n linux-watchdog@vger.kernel.org,\n Thomas Petazzoni ,\n Miquel Raynal ","Date":"Wed, 01 Apr 2026 15:48:08 +0200","In-Reply-To":"<20260331130848.GG3795166@google.com>","References":"<20260324-dev-b4-aaeon-mcu-driver-v4-0-afb011df4794@bootlin.com>\n\t <20260324-dev-b4-aaeon-mcu-driver-v4-3-afb011df4794@bootlin.com>\n\t <20260331130848.GG3795166@google.com>","Autocrypt":"addr=thomas.perrot@bootlin.com; prefer-encrypt=mutual;\n keydata=mQGNBF+/ZOUBDAC2DghCjZvmgYcve02OG7dGZ7Iy58uEwne3LB7w7nRwdAxKw7ZaiVqwY\n O+yNGVi+GVx7oA6Wn4pv46z+QDRLQiq6OseuXhkSGCg7U/yBCUq12B/GRGO1Qt2Qi1mJJT1s+1qZ5\n Gxv6Nypz9qKVn94GM2bR1hXBga0t87vBpebThOHmX5d/0dqIcVxRCM7onNb0dDyRoVgLS5rBhQzrL\n CMrJaCy39xZUy0J1SOlH4Mgk6EhJIPYY4wlzikGX6urg+Tc9EjGd78ry0e0p5U5qgjFR5QGJDy1Gn\n U3CfwbT9sowdCASDbQDUoltlv2iWJCLa0xl97KVchCa0pr7HKbFA3J5SLKqFYUBCkFL+5WudYlz2n\n XxiUgyviMQxyK+ij66kEi6/2zFDAecd43pHV7790ptqZBC3Jc67Emj7Vo3ShX6RXPPxxbeCTOF2uk\n I45aJ9XcVFH/MFE96NjXj8uahnIsiTPyuCUoJu8tj7TSQyue874qJqVQvqlFyt2aZYJZ8ruq8AEQE\n AAbQpVGhvbWFzIFBlcnJvdCA8dGhvbWFzLnBlcnJvdEBib290bGluLmNvbT6JAc4EEwEIADgCGwMF\n CwkIBwIGFQoJCAsCBBYCAwECHgECF4AWIQSHQHfGpqMKIwOoEiGfwAsFcf4K7QUCX79mdwAKCRCfw\n AsFcf4K7fhbC/wP0kSl6id2E/K3+UdXk6CLMVRbCFLCREzQs5WFpQ6l/I0WGOamhrOgegdszheiVF\n orlUP8d37XSpFAqydhKGaN78V5Dps0Wmwm4lIlS4MtQXJtSLUHXDJLIZLW0pw8tiPLKsd1o/yDkXE\n dnpsjJTRG6SdDSHnyOB2/gh4p+yTaLytFdARk/r4/P26+L+FiH0fFl+RnBt19LPklfKgeDc7GwIif\n ja+nIWpp3W23DAUuI6xduEut25Q89yu7Ci8CliLfAiLy9bIGjBQWU2Y+1/j/7KuPj6VbBsZWLTZY0\n hUmpJSTnWAqc9SMsNxo7NSQuddgviz5e2tqucaRqxP02FGzNa8U4NAKdWaXrlHG5Dglj9XH0DK+SH\n +c96qqFewYD8VPQ6XAGxQcXbrtJmiMor1R2DfziispLRvJcfYs8xqabbCtoS3ouXB9XRi8hn7A2kh\n ME1ryS+Oh63JshXHnw6bmjCpVd/p+fGLIGU6A47pJOpviKR4jEO84pl2ejtDZ3Tc=","Content-Type":"multipart/signed; micalg=\"pgp-sha256\";\n\tprotocol=\"application/pgp-signature\"; boundary=\"=-qAFYFdvVYExu7LPUqa+z\"","User-Agent":"Evolution 3.58.3 (3.58.3-1.fc43) ","Precedence":"bulk","X-Mailing-List":"linux-gpio@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":"","MIME-Version":"1.0","X-Last-TLS-Session-Version":"TLSv1.3"}}]