Hi Jonathan, On Fri, 20 May 2022 20:28:19 +0530 Tamseel Shams <m.shams@xxxxxxxxxxx> wrote: >> From: Alim Akhtar <alim.akhtar@xxxxxxxxxxx> >> >> Exynos's ADC-FSD-HW has some difference in registers set, number of >> programmable channels (16 channel) etc. This patch adds support for >> ADC-FSD-HW controller version. >> >> Signed-off-by: Alim Akhtar <alim.akhtar@xxxxxxxxxxx> >> Signed-off-by: Tamseel Shams <m.shams@xxxxxxxxxxx> > > Hi, > > One suggestion inline, otherwise LGTM. Plenty of time to tidy this up as this won't make the upcoming merge window - I'll be queuing it up for 5.20 > > Thanks, > > Jonathan > Okay, Thanks for reviewing. >> --- >> - Changes since v1 >> * Addressed Jonathan's comment by using already provided isr handle >> >> drivers/iio/adc/exynos_adc.c | 55 >> ++++++++++++++++++++++++++++++++++++ >> 1 file changed, 55 insertions(+) >> >> diff --git a/drivers/iio/adc/exynos_adc.c >> b/drivers/iio/adc/exynos_adc.c index cff1ba57fb16..183ae591327a 100644 >> --- a/drivers/iio/adc/exynos_adc.c >> +++ b/drivers/iio/adc/exynos_adc.c >> @@ -55,6 +55,11 @@ >> #define ADC_V2_INT_ST(x) ((x) + 0x14) >> #define ADC_V2_VER(x) ((x) + 0x20) >> >> +/* ADC_FSD_HW register definitions */ >> +#define ADC_FSD_DAT(x) ((x) + 0x08) > > I mention this below, but these different register sets should be in the struct exynos_adc_data to avoid the need for an if "compatible" == check on each use of > them. > Can you clarify on how exactly you want me to add these register sets to struct exynos_adc_data? Do you mean just for these registers or other registers too which are defined in this way only? Thanks & Regards, Tamseel Shams