From: sashiko-bot@kernel.org
To: "Thomas Perrot (Schneider Electric)" <thomas.perrot@bootlin.com>
Cc: imx@lists.linux.dev, Frank.Li@kernel.org, robh@kernel.org,
devicetree@vger.kernel.org, linux-watchdog@vger.kernel.org,
conor+dt@kernel.org
Subject: Re: [PATCH v8 4/5] gpio: aaeon: Add GPIO driver for SRG-IMX8P MCU
Date: Fri, 31 Jul 2026 10:01:49 +0000 [thread overview]
Message-ID: <20260731100149.AC6D61F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260731-dev-b4-aaeon-mcu-driver-v8-4-9ab00e80df2a@bootlin.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditionally resetting all GPIO and GPO pin states during probe breaks bootloader handoff and can cause severe hardware glitches.
--
--- Patch [4]: [PATCH v8 4/5] gpio: aaeon: Add GPIO driver for SRG-IMX8P MCU ---
commit 039db3f85a7d9e8ca0a6aa50fb1e051d88e24a0b
Author: Thomas Perrot (Schneider Electric) <thomas.perrot@bootlin.com>
gpio: aaeon: Add GPIO driver for SRG-IMX8P MCU
Add GPIO driver for the Aaeon SRG-IMX8P embedded controller. This
driver supports 7 GPO (General Purpose Output) pins and 12 GPIO pins
that can be configured as inputs or outputs.
> diff --git a/drivers/gpio/gpio-aaeon-mcu.c b/drivers/gpio/gpio-aaeon-mcu.c
> new file mode 100644
> index 0000000000000..a9e048c865f5c
> --- /dev/null
> +++ b/drivers/gpio/gpio-aaeon-mcu.c
[ ... ]
> +static void aaeon_mcu_gpio_reset(struct aaeon_mcu_gpio *data, struct device *dev)
> +{
> + unsigned int i;
> + int ret;
> +
> + /* Reset all GPOs */
> + for (i = 0; i < MAX_GPOS; i++) {
> + ret = aaeon_mcu_gpo_set_cmd(data, i, 0);
> + if (ret < 0)
> + dev_warn(dev, "Failed to reset GPO %u state: %d\n", i, ret);
> + clear_bit(i, data->dir_in);
> + }
> +
> + /* Reset all GPIOs */
> + for (i = MAX_GPOS; i < MAX_GPOS + MAX_GPIOS; i++) {
> + ret = aaeon_mcu_gpio_config_input_cmd(data, i);
> + if (ret < 0)
> + dev_warn(dev, "Failed to reset GPIO %u state: %d\n", i, ret);
> + set_bit(i, data->dir_in);
> + }
> +}
[ ... ]
> +static int aaeon_mcu_gpio_probe(struct platform_device *pdev)
> +{
> + struct aaeon_mcu_gpio *data;
[ ... ]
> + /*
> + * Reset all GPIO states to a known configuration. The MCU does not
> + * reset GPIO state on soft reboot, only on power cycle (hard reboot).
> + * Without this reset, GPIOs would retain their previous state across
> + * reboots, which could lead to unexpected behavior.
> + */
> + aaeon_mcu_gpio_reset(data, &pdev->dev);
[Severity: High]
Does this unconditional reset during probe interfere with bootloader handoff?
If the bootloader has set a GPO high or configured a GPIO as an output to
drive a critical component (like a voltage regulator or display backlight),
forcing it low or to an input state could immediately cut power or cause a
hardware glitch.
Could the driver read the hardware registers to populate its internal state
instead of wiping the physical state during initialization?
> +
> + return devm_gpiochip_add_data(&pdev->dev, &data->gc, data);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-dev-b4-aaeon-mcu-driver-v8-0-9ab00e80df2a@bootlin.com?part=4
next prev parent reply other threads:[~2026-07-31 10:01 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-31 9:51 [PATCH v8 0/5] Add support for AAEON SRG-IMX8P MCU Thomas Perrot (Schneider Electric)
2026-07-31 9:51 ` [PATCH v8 1/5] dt-bindings: vendor-prefixes: Add AAEON vendor prefix Thomas Perrot (Schneider Electric)
2026-07-31 9:51 ` [PATCH v8 2/5] dt-bindings: mfd: Add AAEON embedded controller Thomas Perrot (Schneider Electric)
2026-07-31 9:51 ` [PATCH v8 3/5] mfd: aaeon: Add SRG-IMX8P MCU driver Thomas Perrot (Schneider Electric)
2026-07-31 10:07 ` sashiko-bot
2026-07-31 9:51 ` [PATCH v8 4/5] gpio: aaeon: Add GPIO driver for SRG-IMX8P MCU Thomas Perrot (Schneider Electric)
2026-07-31 10:01 ` sashiko-bot [this message]
2026-07-31 9:51 ` [PATCH v8 5/5] watchdog: aaeon: Add watchdog " Thomas Perrot (Schneider Electric)
2026-07-31 10:04 ` sashiko-bot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260731100149.AC6D61F000E9@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=Frank.Li@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=imx@lists.linux.dev \
--cc=linux-watchdog@vger.kernel.org \
--cc=robh@kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
--cc=thomas.perrot@bootlin.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.