Re: [PATCH v8 2/5] iio: adc: add Versal SysMon driver
From: Andy Shevchenko
Date: Wed Jun 17 2026 - 10:53:16 EST
On Tue, Jun 16, 2026 at 02:15:56PM +0100, Salih Erim wrote:
> Add the core driver and MMIO platform driver for the AMD/Xilinx Versal
> System Monitor (SysMon) block.
>
> The SysMon block resides in the platform management controller (PMC) and
> provides on-chip voltage and temperature monitoring through a 10-bit,
> 200 kSPS ADC. It can monitor up to 160 voltage channels and 64
> temperature satellites distributed across the SoC, with a consistent
> sample rate of 8 kSPS per channel regardless of how many channels are
> enabled.
>
> The hardware also provides four aggregate temperature registers that
> are always present regardless of the device tree configuration: the
> current max and min across all active satellites, and the peak and
> trough values recorded since the last hardware reset.
>
> The driver is split into two compilation units:
> - versal-sysmon-core: Channel parsing, IIO registration, read_raw
> - versal-sysmon: MMIO platform driver with custom regmap accessors
>
> Voltage results are stored in a 19-bit modified floating-point format
> and converted to millivolts. Temperature results are stored in Q8.7
> signed fixed-point Celsius format and converted to millicelsius.
>
> The MMIO regmap backend uses a custom reg_write accessor that
> automatically unlocks the NPI (NoC programming interface) lock
> register before each write, as required by the hardware. The regmap
> is configured with fast_io since the underlying MMIO accessors are
> safe to call from atomic context.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
with a caveat of using namespaced exports (see below).
...
> +EXPORT_SYMBOL_GPL(devm_versal_sysmon_core_probe);
Please, also use symbol namespace.
...
> +static int sysmon_mmio_reg_read(void *context, unsigned int reg,
> + unsigned int *val)
Make it a single line. It will be more readable.
static int sysmon_mmio_reg_read(void *context, unsigned int reg, unsigned int *val)
...
> +static int sysmon_mmio_reg_write(void *context, unsigned int reg,
> + unsigned int val)
In the similar way.
static int sysmon_mmio_reg_write(void *context, unsigned int reg, unsigned int val)
In both cases it's only 83 characters.
--
With Best Regards,
Andy Shevchenko