Re: [PATCH v5 0/5] Add support for AAEON SRG-IMX8P MCU

Hello Guenter,

On Sat, 2026-04-11 at 17:12 -0700, Guenter Roeck wrote:
> snip
> 
> Sashiko has some interesting feedback that might be worth looking
> into.
> 
> https://sashiko.dev/#/patchset/20260408-dev-b4-aaeon-mcu-driver-v5-0-ad98bd481668%40bootlin.com
> 

Thanks for the pointer. I went through all findings and addressed the
  valid ones in v6:

  MFD driver:
   - Set I2C_M_DMA_SAFE on all i2c_msg flags. The buffers were already
     heap-allocated for DMA safety but the flag was missing, which
     would have caused unnecessary bounce-buffering by the host driver.
   - Add select REGMAP to config MFD_AAEON_MCU
   - "Kconfig COMPILE_TEST link failure": I2C || COMPILE_TEST lets
     MFD_AAEON_MCU=y even when I2C=m (tristate OR caps to y), which
     would fail to link since i2c_transfer(), only exist when I2C
     itself is built in. Will drop the COMPILE_TEST escape and just use
     depends on I2C, matching the other I2C MFD drivers in this file.

  GPIO driver:
   - Replace __set_bit/__clear_bit/__assign_bit with their atomic
     counterparts. gpiolib does not serialize across pins, so
     concurrent direction changes on different pins could race on the
     shared bitmaps.
   - Reverse the order in aaeon_mcu_gpio_config_output_cmd(): write the
     output value first, then switch the pin to output mode, to avoid a
     potential glitch if the previously latched value differs.
   - Add MODULE_ALIAS("platform:aaeon-mcu-gpio") for udev auto-loading.

  Watchdog driver:
   - Add WDIOF_SETTIMEOUT and watchdog_init_timeout() so the software
     timeout is configurable via ioctl, DT timeout-sec, or the
     watchdog_timeout boot parameter. This also addresses the concern
     you raised about the hardcoded 240s timeout.
   - Add watchdog_stop_on_reboot() so the MCU watchdog is stopped
     during system shutdown, preventing a spurious reset from the
     external MCU.
   - Add MODULE_ALIAS("platform:aaeon-mcu-wdt") for udev auto-loading.

  The following findings were considered false positives:

   - "Heap buffer overflow during bulk writes": with reg_bits=16 and
     val_bits=8, regcache_sync() calls _regmap_write() per register, so
     the write callback always receives exactly 3 bytes (2 reg + 1
val).
     No bulk path reaches the custom bus callback.

   - "Stack DMA violation in read path": val_buf comes from regmap's
own
     heap-allocated work_buf, not a stack pointer, so DMA safety is
     guaranteed by the regmap core.

   - "I2C interleaving race": Concurrent access from child drivers
     (GPIO and watchdog) is serialized by regmap's internal mutex,
     which is held for the entire bus transaction ; both i2c_transfer()
     calls complete under that lock before another caller can enter.

   - "Missing PM suspend/resume callbacks": the watchdog core already
     handles this via watchdog_pm_ops, which calls wdt->ops->stop() on
     system suspend.

Kind regards,
Thomas


> Guenter
> 

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

-----BEGIN PGP SIGNATURE-----

iQGzBAABCAAdFiEEh0B3xqajCiMDqBIhn8ALBXH+Cu0FAmpCl6sACgkQn8ALBXH+
Cu3gtwv/WZRg+KDlEazT/qmm8H0EzbCYvvDakncblFOk6oJq75kaC7outXtOGBPs
RK5rwW7VB2GP37sLS2vrA+JhrJ4gUgY53HnLtpgZhYgV5q449bqqb/8VGSBFYvw/
CBdvzUlHzP4zA6fxilChd6n8LVFAvxNLMe+wzm5iq8Q5687omtSQG6/Z1DDy/RHo
B8xahs7i3PGGPoWD3TQXKptPBfMBuZHZb391TGsWs4dmG7JLi1QTM1Z30NBRY3ov
CQfyzsvn44wcFZosCgWBVtM8n6UZZrIpuUtq/EbRFnk7UzdNB7Tv025plUbXJbRV
cuKEFUs1DaW7Rlv4DL6Q2sV0DeoLYM24/7Tac7txF4D+8Caqd4u8QPtlqpWmzMfI
HbHdcQB3sIkbGvVn4wNxmzKNgGqjOUs+p/UqwVhb7w9B7W7DxuH5gTswBWBo2X9O
vx+YKBejGUw52tqYkAedq8Xi+KwwFwWI5oP2wa/cOmP4YHn1jabk0DCIGmlKfxlj
hKzlP3Xi
=aAXr
-----END PGP SIGNATURE-----