Hi Chanwoo,
On 18.06.2014 04:20, Chanwoo Choi 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_adc' clock as following:
> - 'sclk_adc' clock: special clock for ADC which provide clock to internal ADC
>
> Exynos 4210/4212/4412 and Exynos5250/5420 has not included 'sclk_adc' clock
> in FSYS_BLK. But, Exynos3250 based on Cortex-A7 has only included 'sclk_adc'
> clock in FSYS_BLK.
>
> Signed-off-by: Chanwoo Choi <cw00.c...@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.p...@samsung.com>
> ---
> drivers/iio/adc/exynos_adc.c | 93
> ++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 81 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
> index c30def6..6b026ac 100644
> --- a/drivers/iio/adc/exynos_adc.c
> +++ b/drivers/iio/adc/exynos_adc.c
> @@ -41,7 +41,8 @@
>
> enum adc_version {
> ADC_V1,
> - ADC_V2
> + ADC_V2,
> + ADC_V2_EXYNOS3250,
> };
>
> /* EXYNOS4412/5250 ADC_V1 registers definitions */
> @@ -85,9 +86,11 @@ enum adc_version {
> #define EXYNOS_ADC_TIMEOUT (msecs_to_jiffies(100))
>
> struct exynos_adc {
> + struct device *dev;
> void __iomem *regs;
> void __iomem *enable_reg;
> struct clk *clk;
> + struct clk *sclk;
> unsigned int irq;
> struct regulator *vdd;
> struct exynos_adc_ops *ops;
> @@ -96,6 +99,7 @@ struct exynos_adc {
>
> u32 value;
> unsigned int version;
> + bool needs_sclk;
This should be rather a part of the variant struct. See my comments to
patch 1/4.
> };
>
> struct exynos_adc_ops {
> @@ -103,11 +107,21 @@ struct exynos_adc_ops {
> void (*clear_irq)(struct exynos_adc *info);
> void (*start_conv)(struct exynos_adc *info, unsigned long addr);
> void (*stop_conv)(struct exynos_adc *info);
> + void (*disable_clk)(struct exynos_adc *info);
> + int (*enable_clk)(struct exynos_adc *info);
> };
>
> 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-v1",
> + .data = (void *)ADC_V1,
> + }, {
> + .compatible = "samsung,exynos-adc-v2",
> + .data = (void *)ADC_V2,
> + }, {
> + .compatible = "samsung,exynos3250-adc-v2",
> + .data = (void *)ADC_V2_EXYNOS3250,
> + },
> {},
> };
> MODULE_DEVICE_TABLE(of, exynos_adc_match);
> @@ -156,11 +170,42 @@ static void exynos_adc_v1_stop_conv(struct exynos_adc
> *info)
> writel(con, ADC_V1_CON(info->regs));
> }
>
> +static void exynos_adc_disable_clk(struct exynos_adc *info)
> +{
> + if (info->needs_sclk)
> + clk_disable_unprepare(info->sclk);
> + clk_disable_unprepare(info->clk);
> +}
> +
> +static int exynos_adc_enable_clk(struct exynos_adc *info)
> +{
> + int ret;
> +
> + ret = clk_prepare_enable(info->clk);
> + if (ret) {
> + dev_err(info->dev, "failed enabling adc clock: %d\n", ret);
> + return ret;
> + }
> +
> + if (info->needs_sclk) {
> + ret = clk_prepare_enable(info->sclk);
> + if (ret) {
> + clk_disable_unprepare(info->clk);
> + dev_err(info->dev,
> + "failed enabling sclk_tsadc clock: %d\n", ret);
> + }
> + }
> +
> + return 0;
> +}
> +
> static struct exynos_adc_ops exynos_adc_v1_ops = {
> .init_hw = exynos_adc_v1_init_hw,
> .clear_irq = exynos_adc_v1_clear_irq,
> .start_conv = exynos_adc_v1_start_conv,
> .stop_conv = exynos_adc_v1_stop_conv,
> + .disable_clk = exynos_adc_disable_clk,
> + .enable_clk = exynos_adc_enable_clk,
> };
>
> static void exynos_adc_v2_init_hw(struct exynos_adc *info)
> @@ -210,6 +255,8 @@ static struct exynos_adc_ops exynos_adc_v2_ops = {
> .start_conv = exynos_adc_v2_start_conv,
> .clear_irq = exynos_adc_v2_clear_irq,
> .stop_conv = exynos_adc_v2_stop_conv,
> + .disable_clk = exynos_adc_disable_clk,
> + .enable_clk = exynos_adc_enable_clk,
Based on the fact that all variants use the same function, I don't think
there is a reason to add .{disable,enable}_clk in the ops struct. If
they diverge in future, they could be added later, but right now it
doesn't have any value.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
