Hi Sachin, On 04/16/2014 01:44 PM, Chanwoo Choi wrote: > Hi Sachin, > > On 04/16/2014 12:48 PM, Sachin Kamat wrote: >> Hi Chanwoo, >> >> On 14 April 2014 14:37, Chanwoo Choi <cw00.choi@xxxxxxxxxxx> wrote: >>> This patch control special clock for ADC in Exynos series's FSYS block. >>> If special clock of ADC is registerd on clock list of common clk framework, >>> Exynos ADC drvier have to control this clock. >>> >>> Exynos3250/Exynos4/Exynos5 has 'adc' clock as following: >>> - 'adc' clock: bus clock for ADC >>> >>> Exynos3250 has additional 'sclk_tsadc' clock as following: >>> - 'sclk_tsadc' clock: special clock for ADC which provide clock to internal ADC >>> >>> Exynos 4210/4212/4412 and Exynos5250/5420 has not included 'sclk_tsadc' clock >>> in FSYS_BLK. But, Exynos3250 based on Cortex-A7 has only included 'sclk_tsadc' >>> clock in FSYS_BLK. >>> >>> Cc: Jonathan Cameron <jic23@xxxxxxxxxx> >>> Cc: Kukjin Kim <kgene.kim@xxxxxxxxxxx> >>> Cc: Naveen Krishna Chatradhi >>> Cc: linux-iio@xxxxxxxxxxxxxxx >>> Signed-off-by: Chanwoo Choi <cw00.choi@xxxxxxxxxxx> >>> Acked-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx> >>> --- >>> drivers/iio/adc/exynos_adc.c | 54 +++++++++++++++++++++++++++++++++----------- >>> 1 file changed, 41 insertions(+), 13 deletions(-) >>> >>> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c >>> index d25b262..3c99243 100644 >>> --- a/drivers/iio/adc/exynos_adc.c >>> +++ b/drivers/iio/adc/exynos_adc.c >>> @@ -40,8 +40,9 @@ >>> #include <linux/iio/driver.h> >>> >>> enum adc_version { >>> - ADC_V1, >>> - ADC_V2 >>> + ADC_V1 = 0x1, >>> + ADC_V2 = 0x2, >>> + ADC_V3 = (ADC_V1 | ADC_V2), >> >> Can't this be simply 0x3? Or is this not really a h/w version? > > Even thought ADC_V3 isn't h/w revision, ADC_V3 include all featues of ADC_V2 > and only one difference of clock(sclk_tsadc) from ADC_V2. > I want to describethat ADC_V3 include ADC_V2 feature So, I add as following: > >> + ADC_V3 = (ADC_V1 | ADC_V2), > >> >>> }; >>> >>> /* EXYNOS4412/5250 ADC_V1 registers definitions */ >>> @@ -88,6 +89,7 @@ struct exynos_adc { >>> void __iomem *regs; >>> void __iomem *enable_reg; >>> struct clk *clk; >>> + struct clk *sclk; >>> unsigned int irq; >>> struct regulator *vdd; >>> >>> @@ -100,6 +102,7 @@ struct exynos_adc { >>> static const struct of_device_id exynos_adc_match[] = { >>> { .compatible = "samsung,exynos-adc-v1", .data = (void *)ADC_V1 }, >>> { .compatible = "samsung,exynos-adc-v2", .data = (void *)ADC_V2 }, >>> + { .compatible = "samsung,exynos-adc-v3", .data = (void *)ADC_V3 }, >>> {}, >>> }; >>> MODULE_DEVICE_TABLE(of, exynos_adc_match); >>> @@ -128,7 +131,7 @@ static int exynos_read_raw(struct iio_dev *indio_dev, >>> mutex_lock(&indio_dev->mlock); >>> >>> /* Select the channel to be used and Trigger conversion */ >>> - if (info->version == ADC_V2) { >>> + if (info->version & ADC_V2) { >> >> So, now this would be applicable for ADC_V3 too, right? ADC_V3 isn't h/w version. So, I think this code is proper instead of using ADC_V3 direclty. I want to use ADC_V3 version on checking clock(sclk_tsadc). >> >> >>> con2 = readl(ADC_V2_CON2(info->regs)); >>> con2 &= ~ADC_V2_CON2_ACH_MASK; >>> con2 |= ADC_V2_CON2_ACH_SEL(chan->address); >>> @@ -165,7 +168,7 @@ static irqreturn_t exynos_adc_isr(int irq, void *dev_id) >>> info->value = readl(ADC_V1_DATX(info->regs)) & >>> ADC_DATX_MASK; >>> /* clear irq */ >>> - if (info->version == ADC_V2) >>> + if (info->version & ADC_V2) >>> writel(1, ADC_V2_INT_ST(info->regs)); >>> else >>> writel(1, ADC_V1_INTCLR(info->regs)); >>> @@ -226,11 +229,25 @@ static int exynos_adc_remove_devices(struct device *dev, void *c) >>> return 0; >>> } >>> >>> +static void exynos_adc_enable_clock(struct exynos_adc *info, bool enable) >>> +{ >>> + if (enable) { >>> + clk_prepare_enable(info->clk); >> >> This could fail. Is it OK without any checks? > > OK, I'll check return value. Do you want to check return value always? I think again, Some device drivers in mainline would not check return value of clock function. If maintainer confirm this modification, I'll fix it as your comment. Best Regards, Chanwoo Choi -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html