From: t.figa@samsung•com (Tomasz Figa)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH] cpufreq: exynos: Fix driver compilation with ARCH_MULTIPLATFORM
Date: Wed, 21 May 2014 14:11:48 +0200 [thread overview]
Message-ID: <537C9804.8030508@samsung.com> (raw)
In-Reply-To: <CAKohpo=4nzSiqcMzgOka_jDco7FZ+3PHPa3ZWv++Pf7EAera2Q@mail.gmail.com>
Hi Viresh,
Thanks for the review.
On 21.05.2014 13:22, Viresh Kumar wrote:
> On 21 May 2014 16:47, Tomasz Figa <t.figa@samsung•com> wrote:
>
> Mostly nitpicks ..
>
>>> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
>>> config ARM_EXYNOS4X12_CPUFREQ
>>> bool "SAMSUNG EXYNOS4x12"
>>> - depends on (SOC_EXYNOS4212 || SOC_EXYNOS4412) && !ARCH_MULTIPLATFORM
>>> + depends on (SOC_EXYNOS4212 || SOC_EXYNOS4412)
>
> Get rid of () as well..
Right.
>
>>> diff --git a/drivers/cpufreq/exynos-cpufreq.h b/drivers/cpufreq/exynos-cpufreq.h
>>> index a28ee9d..8dfebac 100644
>>> --- a/drivers/cpufreq/exynos-cpufreq.h
>>> +++ b/drivers/cpufreq/exynos-cpufreq.h
>>> @@ -50,6 +50,7 @@ struct exynos_dvfs_info {
>>> struct cpufreq_frequency_table *freq_table;
>>> void (*set_freq)(unsigned int, unsigned int);
>>> bool (*need_apll_change)(unsigned int, unsigned int);
>>> + void __iomem *cmu_regs;
>
> s/tab/space ? before *cmu_regs ..
Other fields in this struct have their names aligned with a tab as well.
>
>>> diff --git a/drivers/cpufreq/exynos4210-cpufreq.c b/drivers/cpufreq/exynos4210-cpufreq.c
>>> @@ -143,6 +160,8 @@ int exynos4210_cpufreq_init(struct exynos_dvfs_info *info)
>>> info->freq_table = exynos4210_freq_table;
>>> info->set_freq = exynos4210_set_frequency;
>>>
>>> + cpufreq = info;
>
> I couldn't find this variable .. i.e. 'cpufreq'
It is a static global variable that is being added at the top of the file.
>
>>> +
>>> return 0;
>>>
>>> err_mout_apll:
>>> diff --git a/drivers/cpufreq/exynos4x12-cpufreq.c b/drivers/cpufreq/exynos4x12-cpufreq.c
>
>>> int exynos4x12_cpufreq_init(struct exynos_dvfs_info *info)
>>> {
>>> + struct device_node *np;
>>> unsigned long rate;
>>>
>>> + np = of_find_compatible_node(NULL, NULL, "samsung,exynos4412-clock");
>>> + if (!np) {
>>> + pr_err("%s: failed to find clock controller DT node\n",
>>> + __func__);
>>> + return -ENODEV;
>>> + }
>>> +
>>> + info->cmu_regs = of_iomap(np, 0);
>>> + if (!info->cmu_regs) {
>>> + pr_err("%s: failed to map CMU registers\n", __func__);
>>> + return -EFAULT;
>>> + }
>>> +
>
> Don't replicate. Create a routine for all this..
>
While I agree that all three drivers basically use the same look-up and
mapping code replicated, this patch is a temporary hack, until all those
three drivers are completely removed, most likely in 3.17, so I would
prefer doing this in the most ugly way, so that people don't follow this.
Still, I think a comment added before of_find_compatible_node() in each
driver saying that this is a hack and why it is there would be nice, though.
Best regards,
Tomasz
next prev parent reply other threads:[~2014-05-21 12:11 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-21 6:52 [PATCH 0/3] Exynos multi-platform support Sachin Kamat
2014-05-21 6:52 ` [PATCH 1/3] ARM: EXYNOS: Consolidate Kconfig entries Sachin Kamat
2014-05-21 6:52 ` [PATCH 2/3] ARM: EXYNOS: Enable multi-platform build support Sachin Kamat
2014-05-21 8:16 ` Arnd Bergmann
2014-05-21 8:20 ` Sachin Kamat
2014-05-21 10:36 ` Bartlomiej Zolnierkiewicz
2014-05-21 11:14 ` Sachin Kamat
2014-05-21 13:40 ` Kukjin Kim
2014-05-21 14:02 ` Bartlomiej Zolnierkiewicz
2014-05-21 6:52 ` [PATCH 3/3] ARM: multi_v7_defconfig: Enable Exynos platform Sachin Kamat
2014-05-21 8:26 ` [PATCH 0/3] Exynos multi-platform support Tomasz Figa
2014-05-21 11:12 ` [PATCH] cpufreq: exynos: Fix driver compilation with ARCH_MULTIPLATFORM Tomasz Figa
2014-05-21 11:17 ` Tomasz Figa
2014-05-21 11:22 ` Viresh Kumar
2014-05-21 12:11 ` Tomasz Figa [this message]
2014-05-21 12:21 ` Arnd Bergmann
2014-05-21 12:26 ` Tomasz Figa
2014-05-21 12:32 ` Thomas Abraham
2014-05-21 13:31 ` Kukjin Kim
2014-05-21 14:05 ` Viresh Kumar
2014-05-23 14:52 ` Tomasz Figa
2014-05-23 15:22 ` [PATCH v2] " Tomasz Figa
2014-05-25 21:41 ` Kukjin Kim
2014-05-26 5:40 ` Viresh Kumar
2014-05-26 22:06 ` Kukjin Kim
2014-05-26 5:40 ` Viresh Kumar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=537C9804.8030508@samsung.com \
--to=t.figa@samsung$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox