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

[Thomas Perrot <thomas.perrot@bootlin.com>]

[-- Attachment #1: Type: text/plain, Size: 3120 bytes --]

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 659 bytes --]