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

[Erim, Salih]

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@xxxxxxxxx>
>
> 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