[PATCH v3 0/5] iio: adc: add AMD/Xilinx Versal SysMon driver

Salih Erim posted 5 patches 1 week, 5 days ago
There is a newer version of this series
.../bindings/iio/adc/xlnx,versal-sysmon.yaml  |  154 +++
MAINTAINERS                                   |    7 +
drivers/iio/adc/Kconfig                       |   33 +
drivers/iio/adc/Makefile                      |    3 +
drivers/iio/adc/versal-sysmon-core.c          | 1094 +++++++++++++++++
drivers/iio/adc/versal-sysmon-i2c.c           |  153 +++
drivers/iio/adc/versal-sysmon.c               |   92 ++
drivers/iio/adc/versal-sysmon.h               |  125 ++
8 files changed, 1661 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/xlnx,versal-sysmon.yaml
create mode 100644 drivers/iio/adc/versal-sysmon-core.c
create mode 100644 drivers/iio/adc/versal-sysmon-i2c.c
create mode 100644 drivers/iio/adc/versal-sysmon.c
create mode 100644 drivers/iio/adc/versal-sysmon.h
[PATCH v3 0/5] iio: adc: add AMD/Xilinx Versal SysMon driver
Posted by Salih Erim 1 week, 5 days ago
This series adds a new IIO driver for the AMD/Xilinx Versal System
Monitor (SysMon), providing on-chip voltage and temperature monitoring.

The Versal SysMon measures up to 160 supply voltages and reads up to
64 temperature satellites distributed across the SoC. The hardware
also provides aggregated device temperature registers: the current
max and min across all active satellites, and peak/trough values
recorded since last hardware reset. The device can be accessed via
memory-mapped I/O or via an I2C interface.

The driver is split into a bus-agnostic core module using the regmap
API, an MMIO platform driver, and an I2C driver. This allows the
same IIO logic to be shared across different bus transports.

Previous submissions:
  v2: https://lore.kernel.org/all/cover.1746182670.git.salih.erim@amd.com/
  v1: https://lore.kernel.org/all/cover.1757061697.git.michal.simek@amd.com/

Changes in v3:
  - DT binding: single compatible, voltage-channels rename, single
    quotes, drop label/bipolar/xlnx,aie-temp (Krzysztof)
  - Core: IWYU throughout, __free(fwnode_handle), sign_extend32(),
    size_add(), dev_err_probe(), s16 param, remove (int) casts,
    drop SYSMON_MILLI in favor of (int)MILLI, rename _ext to _name
    in SYSMON_CHAN_TEMP macro (Andy, Jonathan)
  - Core: fwnode_irq_get() moved to core_probe, remove sysmon->dev/
    indio_dev/irq from struct, describe protected data in lock
    comment, add RAW+PROCESSED comment (Jonathan)
  - I2C: IWYU, remove wrapper struct, explicit enum values, sizeof()
    for buffers, = { } initializers, adapt to core_probe interface
    change (Andy, Krzysztof)
  - Events: IWYU, FIELD_GET/FIELD_PREP, regmap_set/clear_bits,
    clamp_t, !!, IRQ_RETVAL(), devm_delayed_work_autocancel,
    loop var scope, error checks, remove redundant else, logical
    param splits, spinlock safety comment (Andy)
  - Events: hysteresis rework -- store as millicelsius, hardcode
    ALARM_CONFIG to hysteresis mode, compute lower threshold from
    (upper - hysteresis), remove falling threshold for temperature,
    single event spec per channel with IIO_EV_DIR_RISING, push
    IIO_EV_DIR_RISING for temp and IIO_EV_DIR_EITHER for voltage
    (Jonathan)

Tested on VCK190 (single SLR, MMIO path, 7 supplies, 10 temperature
satellites) and VPK180 (System Controller, I2C path, 7 supplies).

A follow-up series will add thermal zone integration, secure firmware
access, and I2C remote monitoring.

Salih Erim (5):
  dt-bindings: iio: adc: add xlnx,versal-sysmon binding
  iio: adc: add Versal SysMon driver
  iio: adc: versal-sysmon: add I2C driver
  iio: adc: versal-sysmon: add threshold event support
  iio: adc: versal-sysmon: add oversampling support

 .../bindings/iio/adc/xlnx,versal-sysmon.yaml  |  154 +++
 MAINTAINERS                                   |    7 +
 drivers/iio/adc/Kconfig                       |   33 +
 drivers/iio/adc/Makefile                      |    3 +
 drivers/iio/adc/versal-sysmon-core.c          | 1094 +++++++++++++++++
 drivers/iio/adc/versal-sysmon-i2c.c           |  153 +++
 drivers/iio/adc/versal-sysmon.c               |   92 ++
 drivers/iio/adc/versal-sysmon.h               |  125 ++
 8 files changed, 1661 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/xlnx,versal-sysmon.yaml
 create mode 100644 drivers/iio/adc/versal-sysmon-core.c
 create mode 100644 drivers/iio/adc/versal-sysmon-i2c.c
 create mode 100644 drivers/iio/adc/versal-sysmon.c
 create mode 100644 drivers/iio/adc/versal-sysmon.h

-- 
2.48.1
Re: [PATCH v3 0/5] iio: adc: add AMD/Xilinx Versal SysMon driver
Posted by Jonathan Cameron 1 week, 4 days ago
On Wed, 27 May 2026 12:42:06 +0100
Salih Erim <salih.erim@amd.com> wrote:

> This series adds a new IIO driver for the AMD/Xilinx Versal System
> Monitor (SysMon), providing on-chip voltage and temperature monitoring.
> 
> The Versal SysMon measures up to 160 supply voltages and reads up to
> 64 temperature satellites distributed across the SoC. The hardware
> also provides aggregated device temperature registers: the current
> max and min across all active satellites, and peak/trough values
> recorded since last hardware reset. The device can be accessed via
> memory-mapped I/O or via an I2C interface.
> 
> The driver is split into a bus-agnostic core module using the regmap
> API, an MMIO platform driver, and an I2C driver. This allows the
> same IIO logic to be shared across different bus transports.
> 
> Previous submissions:
>   v2: https://lore.kernel.org/all/cover.1746182670.git.salih.erim@amd.com/
>   v1: https://lore.kernel.org/all/cover.1757061697.git.michal.simek@amd.com/
> 
https://sashiko.dev/#/patchset/20260527114211.174288-1-salih.erim%40amd.com
Quite a bit of feedback.  Some of which is clearly garbage, like the
ARCH_VERSAL suggestion, but take a close look as it does tend to pick up on
stuff that humans miss.
Re: [PATCH v3 0/5] iio: adc: add AMD/Xilinx Versal SysMon driver
Posted by Erim, Salih 1 week, 3 days ago
Hi Jonathan,

On 28/05/2026 13:06, Jonathan Cameron wrote:
> 
> 
> On Wed, 27 May 2026 12:42:06 +0100
> Salih Erim <salih.erim@amd.com> wrote:
> 
>> This series adds a new IIO driver for the AMD/Xilinx Versal System
>> Monitor (SysMon), providing on-chip voltage and temperature monitoring.
>>
>> The Versal SysMon measures up to 160 supply voltages and reads up to
>> 64 temperature satellites distributed across the SoC. The hardware
>> also provides aggregated device temperature registers: the current
>> max and min across all active satellites, and peak/trough values
>> recorded since last hardware reset. The device can be accessed via
>> memory-mapped I/O or via an I2C interface.
>>
>> The driver is split into a bus-agnostic core module using the regmap
>> API, an MMIO platform driver, and an I2C driver. This allows the
>> same IIO logic to be shared across different bus transports.
>>
>> Previous submissions:
>>    v2: https://lore.kernel.org/all/cover.1746182670.git.salih.erim@amd.com/
>>    v1: https://lore.kernel.org/all/cover.1757061697.git.michal.simek@amd.com/
>>
> https://sashiko.dev/#/patchset/20260527114211.174288-1-salih.erim%40amd.com
> Quite a bit of feedback.  Some of which is clearly garbage, like the
> ARCH_VERSAL suggestion, but take a close look as it does tend to pick up on
> stuff that humans miss.

Thanks Jonathan. I've gone through all the Sashiko findings.

False positives:
- ARCH_VERSAL: doesn't exist in mainline; all Versal drivers use
   ARCH_ZYNQMP (EDAC_VERSAL, Versal TRNG, etc.)
- I2C bus atomicity: the SysMon I2C slave protocol requires a
   STOP between command and response; the vendor driver uses the
   same send/recv sequence
- Hardirq on I2C: IRQ handler is never registered on the I2C
   path (has_irq is false when fwnode_irq_get returns negative)
- Voltage threshold clobbering format bits: threshold registers
   only use the mantissa (bits 15:0); vendor driver does the same
- IRQ storm on regmap error: ISR is cleared before handle_events
   is called, so the interrupt won't re-fire
- VERSAL_SYSMON_I2C missing ARCH: no I2C ADC driver in the
   kernel has an ARCH dependency; I2C clients are bus-agnostic

Reviewed but keeping as-is:
- Left-shift of negative in millicelsius_to_q8p7: GCC defines
   this behavior and it's consistent with the read direction
   (right-shift); Andy asked for this symmetry in v2
- Oversampling read without mutex: reading a single int is
   atomic on arm64; adding contention for no practical benefit
- Voltage alarms one-shot: follows the IIO event model where
   userspace re-arms via write_event_config
- Event config reads 0 during masked alarm: standard behavior,
   xilinx-ams does the same
- TOCTOU on masked_temp after spinlock drop: benign; worst case
   the worker reschedules one extra cycle
- Interrupts not disabled on teardown: devm_request_irq frees
   the IRQ handler; probe already disables all interrupts via
   IDR at init, and LIFO devm ordering is correct
- Oversampling shared_by_type for all temp: this is a per-type
   HW setting; static channels report the aggregate result
- val*scale overflow in processedtoraw: realistic voltages
   (< 3.3V) are well within int32 range; overflow requires
   > 32V input which is beyond the ADC range
- sysmon_update_temp_lower underflow: hysteresis is validated
   as non-negative and realistic thresholds keep the subtraction
   well within Q8.7 range

Will address in v4:
- scan_type: dropping entirely (no buffered support), which
   also resolves the realbits question
- RAW + PROCESSED: dropping RAW for voltage (PROCESSED only),
   and using RAW + SCALE for temperature (linear Q8.7)
- temp_mask: take irq_lock in write_event_config to
   synchronize with the unmask worker

Happy to fold these into v4 along with your review feedback.

Thanks,
Salih
Re: [PATCH v3 0/5] iio: adc: add AMD/Xilinx Versal SysMon driver
Posted by Jonathan Cameron 1 week, 3 days ago
On Thu, 28 May 2026 22:46:58 +0100
"Erim, Salih" <salih.erim@amd.com> wrote:

> Hi Jonathan,
> 
> On 28/05/2026 13:06, Jonathan Cameron wrote:
> > 
> > 
> > On Wed, 27 May 2026 12:42:06 +0100
> > Salih Erim <salih.erim@amd.com> wrote:
> >   
> >> This series adds a new IIO driver for the AMD/Xilinx Versal System
> >> Monitor (SysMon), providing on-chip voltage and temperature monitoring.
> >>
> >> The Versal SysMon measures up to 160 supply voltages and reads up to
> >> 64 temperature satellites distributed across the SoC. The hardware
> >> also provides aggregated device temperature registers: the current
> >> max and min across all active satellites, and peak/trough values
> >> recorded since last hardware reset. The device can be accessed via
> >> memory-mapped I/O or via an I2C interface.
> >>
> >> The driver is split into a bus-agnostic core module using the regmap
> >> API, an MMIO platform driver, and an I2C driver. This allows the
> >> same IIO logic to be shared across different bus transports.
> >>
> >> Previous submissions:
> >>    v2: https://lore.kernel.org/all/cover.1746182670.git.salih.erim@amd.com/
> >>    v1: https://lore.kernel.org/all/cover.1757061697.git.michal.simek@amd.com/
> >>  
> > https://sashiko.dev/#/patchset/20260527114211.174288-1-salih.erim%40amd.com
> > Quite a bit of feedback.  Some of which is clearly garbage, like the
> > ARCH_VERSAL suggestion, but take a close look as it does tend to pick up on
> > stuff that humans miss.  
> 
> Thanks Jonathan. I've gone through all the Sashiko findings.
Thanks,
> 
> Reviewed but keeping as-is:
> - Left-shift of negative in millicelsius_to_q8p7: GCC defines
>    this behavior and it's consistent with the read direction
>    (right-shift); Andy asked for this symmetry in v2
> - Oversampling read without mutex: reading a single int is
>    atomic on arm64; adding contention for no practical benefit

This is potentially a bit messier if the compiler gets creative
(see Will Deacon's various talks on this for instance).  Still
we neglect this for most IIO drivers today.  One day maybe we'll
fix all that up - lots of careful READ_ONCE()/WRITE_ONCE() markings.