linux-kernel - Re: [PATCH v2 2/3] iio: adc: exynos-adc: Add support for ADC FSD-HW controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Fri, 3 Jun 2022 16:10:35 +0100
From:   Jonathan Cameron <jic23@...nel.org>
To:     "m.shams" <m.shams@...sung.com>
Cc:     <lars@...afoo.de>, <robh+dt@...nel.org>, <krzk+dt@...nel.org>,
        <geert@...ux-m68k.org>, <devicetree@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-samsung-soc@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-iio@...r.kernel.org>,
        <alim.akhtar@...sung.com>, <paul@...pouillou.net>,
        <linux-fsd@...la.com>
Subject: Re: [PATCH v2 2/3] iio: adc: exynos-adc: Add support for ADC FSD-HW
 controller

On Tue, 31 May 2022 14:12:46 +0530
"m.shams" <m.shams@...sung.com> wrote:

> Hi Jonathan,
> 
> On Fri, 20 May 2022 20:28:19 +0530
> Tamseel Shams <m.shams@...sung.com> wrote:
> 
> >> From: Alim Akhtar <alim.akhtar@...sung.com>
> >> 
> >> 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@...sung.com>
> >> Signed-off-by: Tamseel Shams <m.shams@...sung.com>  
> >
> > 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?

Any registers addresses that are different for the different chip variants
supported by the driver.

In cases where the only difference between versions is a register address then
define something like
#define ADC_FSD_DAT_BASE 0x08

In the structure have a

dat_addr = ADC_FSD_DAT_BASE

and use dat_addr + x to access.

If things are more complex (and I haven't looked closely so that may apply to
the example give above, the wrap the different access sequence and register
addresses in a callback similar to already done for clear_irq.


Jonathan


> 
> 
> Thanks & Regards,
> Tamseel Shams
>