All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hanjun Guo <hanjun.guo@linaro.org>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Russell King - ARM Linux <linux@arm.linux.org.uk>,
	Grant Likely <grant.likely@linaro.org>,
	Matthew Garrett <mjg59@srcf.ucam.org>,
	Olof Johansson <olof@lixom.net>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Rob Herring <rob.herring@calxeda.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Jon Masters <jonathan@jonmasters.org>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	Patch Tracking <patches@linaro.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	linaro-kernel <linaro-kernel@lists.linaro.org>,
	"linaro-acpi@lists.linaro.org" <linaro-acpi@lists.linaro.org>
Subject: Re: [RFC part3 PATCH 2/2] ARM64 / clocksource: Use arch_timer_acpi_init()
Date: Tue, 03 Dec 2013 21:52:30 +0800	[thread overview]
Message-ID: <529DE21E.4080707@linaro.org> (raw)
In-Reply-To: <CACRpkdYeMOMQzDn5=XpDg7X9L_zMZczGg-5SBA8q_WvwFPnH7A@mail.gmail.com>

On 2013年12月03日 20:27, Linus Walleij wrote:
> On Tue, Dec 3, 2013 at 12:15 PM, Hanjun Guo <hanjun.guo@linaro.org> wrote:
>
>> +       /* if can't be initialised from DT, try ACPI way */
>> +       if (!arch_timer_get_rate())
>> +               arch_timer_acpi_init();
>> +
>>          arch_timer_rate = arch_timer_get_rate();
> This looks a bit fragile. Having a call like arch_timer_get_rate()
> to check whether there is a DT node for the timer doesn't seem
> right, can you refactor the code to provide some
> has_arch_timer_node() or similar call instead, so it's a bit easier
> to understand & maintain at least?

Good point, thanks for the guidance.
I will introduce has_arch_timer_node() as you said and use
it as follows:

if (has_arch_timer_node())
clocksource_of_init();
esle
arch_timer_acpi_init(); /* try ACPI way */

Is this make sense to you?

Thanks
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: hanjun.guo@linaro.org (Hanjun Guo)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC part3 PATCH 2/2] ARM64 / clocksource: Use arch_timer_acpi_init()
Date: Tue, 03 Dec 2013 21:52:30 +0800	[thread overview]
Message-ID: <529DE21E.4080707@linaro.org> (raw)
In-Reply-To: <CACRpkdYeMOMQzDn5=XpDg7X9L_zMZczGg-5SBA8q_WvwFPnH7A@mail.gmail.com>

On 2013?12?03? 20:27, Linus Walleij wrote:
> On Tue, Dec 3, 2013 at 12:15 PM, Hanjun Guo <hanjun.guo@linaro.org> wrote:
>
>> +       /* if can't be initialised from DT, try ACPI way */
>> +       if (!arch_timer_get_rate())
>> +               arch_timer_acpi_init();
>> +
>>          arch_timer_rate = arch_timer_get_rate();
> This looks a bit fragile. Having a call like arch_timer_get_rate()
> to check whether there is a DT node for the timer doesn't seem
> right, can you refactor the code to provide some
> has_arch_timer_node() or similar call instead, so it's a bit easier
> to understand & maintain at least?

Good point, thanks for the guidance.
I will introduce has_arch_timer_node() as you said and use
it as follows:

if (has_arch_timer_node())
clocksource_of_init();
esle
arch_timer_acpi_init(); /* try ACPI way */

Is this make sense to you?

Thanks
Hanjun

WARNING: multiple messages have this Message-ID (diff)
From: Hanjun Guo <hanjun.guo@linaro.org>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Russell King - ARM Linux <linux@arm.linux.org.uk>,
	Grant Likely <grant.likely@linaro.org>,
	Matthew Garrett <mjg59@srcf.ucam.org>,
	Olof Johansson <olof@lixom.net>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Rob Herring <rob.herring@calxeda.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Jon Masters <jonathan@jonmasters.org>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	Patch Tracking <patches@linaro.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	linaro-kernel <linaro-kernel@lists.linaro.org>,
	"linaro-acpi@lists.linaro.org" <linaro-acpi@lists.linaro.org>
Subject: Re: [RFC part3 PATCH 2/2] ARM64 / clocksource: Use arch_timer_acpi_init()
Date: Tue, 03 Dec 2013 21:52:30 +0800	[thread overview]
Message-ID: <529DE21E.4080707@linaro.org> (raw)
In-Reply-To: <CACRpkdYeMOMQzDn5=XpDg7X9L_zMZczGg-5SBA8q_WvwFPnH7A@mail.gmail.com>

On 2013年12月03日 20:27, Linus Walleij wrote:
> On Tue, Dec 3, 2013 at 12:15 PM, Hanjun Guo <hanjun.guo@linaro.org> wrote:
>
>> +       /* if can't be initialised from DT, try ACPI way */
>> +       if (!arch_timer_get_rate())
>> +               arch_timer_acpi_init();
>> +
>>          arch_timer_rate = arch_timer_get_rate();
> This looks a bit fragile. Having a call like arch_timer_get_rate()
> to check whether there is a DT node for the timer doesn't seem
> right, can you refactor the code to provide some
> has_arch_timer_node() or similar call instead, so it's a bit easier
> to understand & maintain at least?

Good point, thanks for the guidance.
I will introduce has_arch_timer_node() as you said and use
it as follows:

if (has_arch_timer_node())
clocksource_of_init();
esle
arch_timer_acpi_init(); /* try ACPI way */

Is this make sense to you?

Thanks
Hanjun

  reply	other threads:[~2013-12-03 13:52 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1386069328-22502-1-git-send-email-hanjun.guo@linaro.org>
     [not found] ` <1386069328-22502-3-git-send-email-hanjun.guo@linaro.org>
2013-12-03 12:27   ` [RFC part3 PATCH 2/2] ARM64 / clocksource: Use arch_timer_acpi_init() Linus Walleij
2013-12-03 12:27     ` Linus Walleij
2013-12-03 13:52     ` Hanjun Guo [this message]
2013-12-03 13:52       ` Hanjun Guo
2013-12-03 13:52       ` Hanjun Guo
2013-12-03 14:13       ` Linus Walleij
2013-12-03 14:13         ` Linus Walleij
2013-12-03 14:43         ` Mark Rutland
2013-12-03 14:43           ` Mark Rutland
2013-12-03 16:30           ` Hanjun Guo
2013-12-03 16:30             ` Hanjun Guo
2013-12-09 18:37       ` Olof Johansson
2013-12-09 18:37         ` Olof Johansson
2013-12-09 18:37         ` Olof Johansson
     [not found] ` <1386069328-22502-2-git-send-email-hanjun.guo@linaro.org>
2013-12-04 15:33   ` [RFC part3 PATCH 1/2] clocksource / arch_timer: Use ACPI GTDT table to initialize arch timer Rob Herring
2013-12-04 15:33     ` Rob Herring
2013-12-05 13:26     ` Hanjun Guo
2013-12-05 13:26       ` Hanjun Guo
2013-12-03 16:41 [RFC part3 PATCH 0/2] Using " Hanjun Guo
2013-12-03 16:41 ` [RFC part3 PATCH 2/2] ARM64 / clocksource: Use arch_timer_acpi_init() Hanjun Guo
2013-12-03 16:41   ` Hanjun Guo
2013-12-03 17:08   ` Mark Rutland
2013-12-03 17:08     ` Mark Rutland
2013-12-03 17:08     ` Mark Rutland
2013-12-04 14:27     ` Hanjun Guo
2013-12-04 14:27       ` Hanjun Guo
2013-12-04 14:27       ` Hanjun Guo
2013-12-04 15:07       ` Mark Rutland
2013-12-04 15:07         ` Mark Rutland
2013-12-04 15:07         ` Mark Rutland
2013-12-05 13:09         ` Hanjun Guo
2013-12-05 13:09           ` Hanjun Guo
2013-12-05 13:09           ` Hanjun Guo

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=529DE21E.4080707@linaro.org \
    --to=hanjun.guo@linaro.org \
    --cc=bhelgaas@google.com \
    --cc=catalin.marinas@arm.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=grant.likely@linaro.org \
    --cc=jonathan@jonmasters.org \
    --cc=linaro-acpi@lists.linaro.org \
    --cc=linaro-kernel@lists.linaro.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=mark.rutland@arm.com \
    --cc=mjg59@srcf.ucam.org \
    --cc=olof@lixom.net \
    --cc=patches@linaro.org \
    --cc=rjw@rjwysocki.net \
    --cc=rob.herring@calxeda.com \
    --cc=will.deacon@arm.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.