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

["Erim, Salih" <salih.erim@amd.com>]

Hi Andy,

On 17/06/2026 15:52, Andy Shevchenko wrote:
> 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).

Thank you for the review!

> 
> ...
> 
>> +EXPORT_SYMBOL_GPL(devm_versal_sysmon_core_probe);
> 
> Please, also use symbol namespace.

Will use EXPORT_SYMBOL_NS_GPL(devm_versal_sysmon_core_probe, 
"VERSAL_SYSMON") and add MODULE_IMPORT_NS("VERSAL_SYSMON") to both 
platform and I2C drivers.

> 
> ...
> 
>> +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)

Done.
> 
> ...
> 
>> +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)

Done.

Thanks,
Salih

> 
> In both cases it's only 83 characters.
> 
> --
> With Best Regards,
> Andy Shevchenko
> 
>