Re: [PATCH v2 2/2] iio: adc: Add the NXP SAR ADC support for the s32g2/3 platforms

[Jonathan Cameron <jonathan.cameron@huawei.com>]

On Thu, 11 Sep 2025 14:55:00 +0200
Daniel Lezcano <daniel.lezcano@linaro.org> wrote:

> Hi Jonathan,
> 
> thanks for the review
> 
> On 10/09/2025 19:32, Jonathan Cameron wrote:
> > On Wed, 10 Sep 2025 17:57:56 +0200
> > Daniel Lezcano <daniel.lezcano@linaro.org> wrote:  
> 
> [ ... ]
> 
> >> +/* Main Configuration Register */
> >> +#define REG_ADC_MCR(__base)		((__base) + 0x00)  
> > 
> > I'm not really convinced these macros help over just having
> > readl(info->regs + NXP_SADC_MCR_REG);  
> 
> That is really a matter of taste :)
> 
> I used to create this format in order to stick the macros with the 
> debugfs register code which is not part of these changes. There is a 
> similar format in drivers/clocksource/timer-nxp-stm.c or 
> driver/thermal/mediatek/lvts.c IMHO is less prone to error than base + 
> REG all around the code.
> 
> Do you want me to convert all the macros to info->__base + MACRO ?

I'm not that fussed if there is other code for related devices using this
style.  To me it adds little benefit but it doesn't hurt that much either!

> 
> [ ... ]
> 
> >> +static const struct iio_chan_spec nxp_sar_adc_iio_channels[] = {
> >> +	ADC_CHAN(0, IIO_VOLTAGE),
> >> +	ADC_CHAN(1, IIO_VOLTAGE),
> >> +	ADC_CHAN(2, IIO_VOLTAGE),
> >> +	ADC_CHAN(3, IIO_VOLTAGE),
> >> +	ADC_CHAN(4, IIO_VOLTAGE),
> >> +	ADC_CHAN(5, IIO_VOLTAGE),
> >> +	ADC_CHAN(6, IIO_VOLTAGE),
> >> +	ADC_CHAN(7, IIO_VOLTAGE),
> >> +	IIO_CHAN_SOFT_TIMESTAMP(32),  
> > 
> > Whilst we only insist on monotonic numbering, putting it all the way down
> > at 32 seems excessive. Why not 8?  Perhaps a comment if this is to avoid
> > moving it for some future feature.  
> 
> The ADC has 8 channels for external acquisition however others channels 
> 8->31 are described as reserved. They may evolve in the future to more 
> channels. That is probably the reason why 32 is used here.

Add a comment on that so we don't forget the reasoning.

Thanks,

Jonathan