Re: [PATCH v8 2/5] iio: adc: add Versal SysMon driver

[Andy Shevchenko <andriy.shevchenko@intel.com>]

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@intel.com>

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