From: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
To: Jisheng Zhang <jszhang@marvell.com>,
Antoine Tenart <antoine.tenart@free-electrons.com>
Cc: zmxu@marvell.com, linux-arm-kernel@lists.infradead.org,
linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] ARM: berlin: register cpufreq-dt for the BG2Q
Date: Mon, 10 Aug 2015 22:22:44 +0200 [thread overview]
Message-ID: <55C90814.3050505@gmail.com> (raw)
In-Reply-To: <20150731141602.28b28f21@xhacker>
On 31.07.2015 08:16, Jisheng Zhang wrote:
> On Thu, 30 Jul 2015 14:35:51 +0200
> Antoine Tenart <antoine.tenart@free-electrons.com> wrote:
>
>> The BG2Q SoC uses cpufreq-dt for cpufreq. Register a platform device for
>> this.
>>
>> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
>> ---
[...]
>> diff --git a/arch/arm/mach-berlin/berlin.c b/arch/arm/mach-berlin/berlin.c
>> index ac181c6797ee..0de906374994 100644
>> --- a/arch/arm/mach-berlin/berlin.c
>> +++ b/arch/arm/mach-berlin/berlin.c
>> @@ -15,9 +15,21 @@
[...]
>> +static void __init berlin_init_late(void)
>> +{
>> + if (of_find_matching_node(NULL, berlin2q_match))
>> + platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
>
> I'm not sure is it acceptable to always register cpufreq-dt platform device.
> In cpufreq-dt driver, the probe will exit if there's no cpuclk node.
Jisheng,
registering the _device_ without checking for any requirements of
the _driver_ is usual practice. It it the driver's job to check
all of them and report an error or warning. So, I am fine with
adding the device unconditionally.
Using the .init_late() callback seems reasonable - but can you
evaluate if we can have cpufreq-dt also for BG2/BG2CD? I'd also
accept a single, default operating-point.
BTW, there is one true point about Jisheng's comment: try to order
your patches logically correct to avoid unnecessary warnings, i.e.
1/3 add cpuclk
2/3 add cpufreq-dt properties
3/3 register cpufreq-dt device
If you consider a git-bisect run, you can land in between any of the
three patches but they are always in the above order - so ordering is
important.
Sebastian
>> +}
>> +
>> static const char * const berlin_dt_compat[] = {
>> "marvell,berlin",
>> NULL,
>> @@ -25,6 +37,7 @@ static const char * const berlin_dt_compat[] = {
>>
>> DT_MACHINE_START(BERLIN_DT, "Marvell Berlin")
>> .dt_compat = berlin_dt_compat,
>> + .init_late = berlin_init_late,
>> /*
>> * with DT probing for L2CCs, berlin_init_machine can be removed.
>> * Note: 88DE3005 (Armada 1500-mini) uses pl310 l2cc
>
next prev parent reply other threads:[~2015-08-10 20:22 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-30 12:35 [PATCH 0/3] ARM: berlin: add cpufreq support for the BG2Q Antoine Tenart
2015-07-30 12:35 ` [PATCH 1/3] clk: berlin: add cpuclk Antoine Tenart
2015-08-07 23:59 ` Stephen Boyd
2015-08-10 19:55 ` Sebastian Hesselbarth
2015-07-30 12:35 ` [PATCH 2/3] ARM: berlin: register cpufreq-dt for the BG2Q Antoine Tenart
2015-07-31 6:16 ` Jisheng Zhang
2015-08-10 20:22 ` Sebastian Hesselbarth [this message]
2015-08-11 2:44 ` Jisheng Zhang
2015-07-30 12:35 ` [PATCH 3/3] ARM: berlin: dts: add the cpufreq-dt bindings on " Antoine Tenart
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=55C90814.3050505@gmail.com \
--to=sebastian.hesselbarth@gmail.com \
--cc=antoine.tenart@free-electrons.com \
--cc=jszhang@marvell.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=zmxu@marvell.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).