From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id C7894C433EF for ; Sun, 22 May 2022 11:19:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=VcsofK77ukteq4tc2BXCIzoui6R9pfiiicMoapASx5w=; b=NfaN/XFGraGjVR bN5+al0reiUpwV8lPdE9NIazzspYrr59umQAfV87A03nG0x803ma2DGX9vwXFclKhM3QE4XjstRBB LHRynFiXWvOMMlA+sx7pKelrtYxeXJ6p6HmC4sE2DG0dvkJbvL0MeV8wv8n8QNP5yBeO+R3iJsU3c QMhCNSV3JrbT36uFc5IgY3BJWYYlIX15IutxF7L56mMQpKLN9rM5zY1OUsDr059FoIBy9H3O7HsRR dmo8oMb/l+CFeZFFjMQ7FwDomVvniKJ11AyG8QMG4P6ShEYegEWr7MC1NHxP+qt3z8rCGRBTjAhTt wG4wJVJcraZKI6u77iow==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nsjaK-001A37-C7; Sun, 22 May 2022 11:17:20 +0000 Received: from ams.source.kernel.org ([2604:1380:4601:e00::1]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nsjaF-001A2Q-RP for linux-arm-kernel@lists.infradead.org; Sun, 22 May 2022 11:17:17 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id DE230B80AC0; Sun, 22 May 2022 11:17:12 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 84466C385AA; Sun, 22 May 2022 11:17:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1653218231; bh=FMgjFVda0G8ynRHRjbeXhrPkJH/y/1VulbL8kZ8FO5U=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=JQP7/FX4pAUCuhCG47MYkdEGEsGVAtOmlHUI9e/RJrHFXYRIhQEUQ6Nthx/1dWbt0 1qmd8pel24MZj+rtu0xUsoZPdjV2/vXvLXevdbDFP9VOlqHywzVupWW3zqxr1vW6vv jqr+r7kDa8MKgvgLXcl6rs3E+g3RRnosp1kFcBffXOBg9xDyMytuJhQ4xHVilKVh54 WyIG6LCnVHqrxLtA5qENeMZ3H0/iI8pSsw2npKoeNraquCQcdKspfSbSie5zhz57F2 l1wz2BI9yd/T2RjAs7Gtmy+GrCin2aNdI5zjYeC1nWIxyrRNLXyMiSPO+FIOXOlR2f mOaF7U7Gx8wBQ== Date: Sun, 22 May 2022 12:25:55 +0100 From: Jonathan Cameron To: Tamseel Shams Cc: lars@metafoo.de, robh+dt@kernel.org, krzk+dt@kernel.org, geert@linux-m68k.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org, alim.akhtar@samsung.com, paul@crapouillou.net, linux-fsd@tesla.com Subject: Re: [PATCH v2 2/3] iio: adc: exynos-adc: Add support for ADC FSD-HW controller Message-ID: <20220522122555.6c65d2b6@jic23-huawei> In-Reply-To: <20220520145820.67667-3-m.shams@samsung.com> References: <20220520145820.67667-1-m.shams@samsung.com> <20220520145820.67667-3-m.shams@samsung.com> X-Mailer: Claws Mail 4.1.0 (GTK 3.24.34; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220522_041716_224447_34A9D622 X-CRM114-Status: GOOD ( 27.54 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, 20 May 2022 20:28:19 +0530 Tamseel Shams wrote: > From: Alim Akhtar > > 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 > Signed-off-by: Tamseel Shams 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 > --- > - 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. > +#define ADC_FSD_DAT_SUM(x) ((x) + 0x0C) > +#define ADC_FSD_DBG_DATA(x) ((x) + 0x1C) > + > /* Bit definitions for ADC_V1 */ > #define ADC_V1_CON_RES (1u << 16) > #define ADC_V1_CON_PRSCEN (1u << 14) > @@ -92,6 +97,7 @@ > > /* Bit definitions for ADC_V2 */ > #define ADC_V2_CON1_SOFT_RESET (1u << 2) > +#define ADC_V2_CON1_SOFT_NON_RESET (1u << 1) > > #define ADC_V2_CON2_OSEL (1u << 10) > #define ADC_V2_CON2_ESEL (1u << 9) > @@ -100,6 +106,7 @@ > #define ADC_V2_CON2_ACH_SEL(x) (((x) & 0xF) << 0) > #define ADC_V2_CON2_ACH_MASK 0xF > > +#define MAX_ADC_FSD_CHANNELS 16 > #define MAX_ADC_V2_CHANNELS 10 > #define MAX_ADC_V1_CHANNELS 8 > #define MAX_EXYNOS3250_ADC_CHANNELS 2 > @@ -484,6 +491,43 @@ static const struct exynos_adc_data exynos7_adc_data = { > .start_conv = exynos_adc_v2_start_conv, > }; > > +static void exynos_adc_fsd_init_hw(struct exynos_adc *info) > +{ > + u32 con2; > + > + writel(ADC_V2_CON1_SOFT_RESET, ADC_V2_CON1(info->regs)); > + > + writel(ADC_V2_CON1_SOFT_NON_RESET, ADC_V2_CON1(info->regs)); > + > + con2 = ADC_V2_CON2_C_TIME(6); > + writel(con2, ADC_V2_CON2(info->regs)); > + > + /* Enable interrupts */ > + writel(1, ADC_V2_INT_EN(info->regs)); > +} > + > +static void exynos_adc_fsd_exit_hw(struct exynos_adc *info) > +{ > + u32 con2; > + > + con2 = readl(ADC_V2_CON2(info->regs)); > + con2 &= ~ADC_V2_CON2_C_TIME(7); > + writel(con2, ADC_V2_CON2(info->regs)); > + > + /* Disable interrupts */ > + writel(0, ADC_V2_INT_EN(info->regs)); > +} > + > +static const struct exynos_adc_data fsd_hw_adc_data = { > + .num_channels = MAX_ADC_FSD_CHANNELS, > + .mask = ADC_DATX_MASK, /* 12 bit ADC resolution */ > + > + .init_hw = exynos_adc_fsd_init_hw, > + .exit_hw = exynos_adc_fsd_exit_hw, > + .clear_irq = exynos_adc_v2_clear_irq, > + .start_conv = exynos_adc_v2_start_conv, > +}; > + > static const struct of_device_id exynos_adc_match[] = { > { > .compatible = "samsung,s3c2410-adc", > @@ -518,6 +562,9 @@ static const struct of_device_id exynos_adc_match[] = { > }, { > .compatible = "samsung,exynos7-adc", > .data = &exynos7_adc_data, > + }, { > + .compatible = "samsung,exynos-adc-fsd-hw", > + .data = &fsd_hw_adc_data, > }, > {}, > }; > @@ -626,6 +673,8 @@ static irqreturn_t exynos_adc_isr(int irq, void *dev_id) > info->ts_x = readl(ADC_V1_DATX(info->regs)); > info->ts_y = readl(ADC_V1_DATY(info->regs)); > writel(ADC_TSC_WAIT4INT | ADC_S3C2443_TSC_UD_SEN, ADC_V1_TSC(info->regs)); > + } else if (of_device_is_compatible(info->dev->of_node, "samsung,exynos-adc-fsd-hw")) { Rather than a fairly expensive look up into a device tree node, why not add the information to the struct exynos_adc_adc in some fashion? Maybe as an offset for the register block? > + info->value = readl(ADC_FSD_DAT(info->regs)) & mask; > } else { > info->value = readl(ADC_V1_DATX(info->regs)) & mask; > } > @@ -719,6 +768,12 @@ static const struct iio_chan_spec exynos_adc_iio_channels[] = { > ADC_CHANNEL(7, "adc7"), > ADC_CHANNEL(8, "adc8"), > ADC_CHANNEL(9, "adc9"), > + ADC_CHANNEL(10, "adc10"), > + ADC_CHANNEL(11, "adc11"), > + ADC_CHANNEL(12, "adc12"), > + ADC_CHANNEL(13, "adc13"), > + ADC_CHANNEL(14, "adc14"), > + ADC_CHANNEL(15, "adc15"), > }; > > static int exynos_adc_remove_devices(struct device *dev, void *c) _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel