Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
From: robherring2@gmail.com (Rob Herring)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 06/11] ARM: vexpress: use clocksource_of_init for sp804
Date: Wed, 13 Mar 2013 11:32:01 -0500
Message-ID: <5140AA01.1010007@gmail.com> (raw)
In-Reply-To: <CAD6h2NTufx2rRwbwsgGHN42t7+_KL418++zk_C++EvFKU+ZwLg@mail.gmail.com>

On 03/13/2013 10:59 AM, Haojian Zhuang wrote:
> On 13 March 2013 23:19, Pawel Moll <pawel.moll@arm.com> wrote:
>> On Wed, 2013-03-13 at 15:01 +0000, Haojian Zhuang wrote:
>>> Yes, it's possible. Since only clocksource / clockevent is using SP804 timer
>>> on the VE platform. If user app or something else is using other SP804
>>> timer, it would be mess because of resource conflict.
>>
>> How would you expose this device to the app or something else? It's an
>> amba_bus device, so it must be bound with an amba_driver. Otherwise
>> you're abusing the device model and, well, you're on your own.
>>
>> The devices are managed by the system. The device tree is merely a
>> description of the available hardware.
>>
> Maybe I didn't express my idea well. My point is that timer could be
> clock source
> /clock event. It could also just be a timer that user app or other device driver
> may want to use it directly. There're a lot of reason that user app or some
>  driver wants to use hardware timer. For example, the specific timer may
> keep running even system is in low power mode.
> 
> Although there's so much hardware timers available, system must choose
> one for the usage of clock source / clock event / other hardware timer usage.
> 
>>>>> Imagine that only TIMINT1 signal in sp804 is routed to interrupt controller.
>>>>> Could system still pick any one it wants?
>>>>
>>> Then we need another new property. I try to find using minim properties.
>>
>> No, we don't. See the discussion with Rob. He has an idea (using the
>> interrupt property only), I have another one (using already defined
>> interrupt-names property). I see you've just came with another one
>> (TIMINTC treated as TIMINT1), which is fine with me. Either way you've
>> got what you need to make the right decision.
>>
> After considered as second time, I think that we still need a property to meet
> alias usage. Excuse me expand it in this topic.
> 
> arm,sp804-has-irq = <offset>;

Interrupt/timer number not offset please. The offset is implied by the
fact that the h/w is an sp804. The Integrator AP is not actually an
sp804, so it's init should be handled differently with a different
compatible property.

> The offset means TIMER register offset. If it's not zero, TIMER2 is using
> irq. And it could be configured as clock event. Otherwise, TIMER1 is clock
> event.
> We must make sure that only one of all sp804 timers is using this property.
> 
> alias:
> linux,clocksource = <&sp804-A>
> linux,clockevent = <&sp804-A>
> 
> Although both of them points to timerA, only TIMER1 or TIMER2 of sp804
> is using irq. So one is clock source, and the other is clock event.
> 
> alias:
> linux,clocksource = <&sp804-A>
> linux,clockevent = <&sp804-B>
> The issue comes since system don't know whether TIMER1 or TIMER2 could
> be configured as clock source.

If you have 2 sp804's, why do you care which one is used by the OS? If
they are identical, then it doesn't matter. If they are not, then
describe how they are not identical.

The realview and versatile boards currently do this, but there is no
good reason that I see and I intend to change this.

> We still need a new property to claim it. It seems that we can't drop
> "arm,sp804-clocksource", although everybody don't like it.
> 
> What's your opinion?

As previously stated, I don't think we need this or the alias based on
current in tree users.

There is an issue that you don't know about all available timers so you
can select the one with the minimum required features leaving others
with more features (i.e. an irq) free. If you need more control like
this then you can do custom setup before or instead of calling
clocksource_of_init. None of the current platforms have this issue nor
do they use sp804 for anything beyond clksrc and clkevt. I would not
expect lots of new users of sp804 as I expect most new platforms will
use architected timers. So let's not solve imaginary problems now.

Rob

  parent reply index

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-13  5:05 [PATCH v3 00/11] add hisilicon SoC support Haojian Zhuang
2013-03-13  5:05 ` [PATCH v3 01/11] clocksource: move sp timer driver Haojian Zhuang
2013-03-13  5:05 ` [PATCH v3 02/11] clocksource: select USE_OF by default Haojian Zhuang
2013-03-13  5:05 ` [PATCH v3 03/11] clocksource: sp804: add device tree support Haojian Zhuang
2013-03-13 11:05   ` Pawel Moll
2013-03-13 11:37     ` Haojian Zhuang
2013-03-13 11:41       ` Pawel Moll
2013-03-13 14:17     ` Rob Herring
2013-03-13 14:42       ` Pawel Moll
2013-03-13 14:51         ` Rob Herring
2013-03-13 14:55           ` Pawel Moll
2013-03-13 15:11             ` Haojian Zhuang
2013-03-13 15:23               ` Pawel Moll
2013-03-13 15:25                 ` Haojian Zhuang
2013-03-13 15:29                   ` Pawel Moll
2013-03-13 15:39                     ` Rob Herring
2013-03-13 15:41                       ` Pawel Moll
2013-03-13 15:44                         ` Haojian Zhuang
2013-03-13 15:42                     ` Haojian Zhuang
2013-03-13 15:49                       ` Pawel Moll
2013-03-13 16:35                         ` Arnd Bergmann
2013-03-13 16:41                           ` Pawel Moll
2013-03-15 12:20       ` Russell King - ARM Linux
2013-03-13  5:05 ` [PATCH v3 04/11] ARM: integrator: use clocksource_of_init for sp804 Haojian Zhuang
2013-03-13  5:05 ` [PATCH v3 05/11] ARM: highbank: " Haojian Zhuang
2013-03-13  5:05 ` [PATCH v3 06/11] ARM: vexpress: " Haojian Zhuang
2013-03-13 11:10   ` Pawel Moll
2013-03-13 11:42     ` Haojian Zhuang
2013-03-13 11:46       ` Pawel Moll
2013-03-13 12:21         ` Haojian Zhuang
2013-03-13 14:48           ` Pawel Moll
2013-03-13 15:01             ` Haojian Zhuang
2013-03-13 15:19               ` Pawel Moll
2013-03-13 15:59                 ` Haojian Zhuang
2013-03-13 16:28                   ` Pawel Moll
2013-03-13 16:32                   ` Rob Herring [this message]
2013-03-15 12:34                     ` Russell King - ARM Linux
2013-03-15 12:58                       ` Pawel Moll
2013-03-15 18:10                         ` Russell King - ARM Linux
2013-03-13  5:05 ` [PATCH v3 07/11] ARM: debug: support debug ll on hisilicon soc Haojian Zhuang
2013-03-13  5:05 ` [PATCH v3 08/11] clk: hi3xxx: add clock support Haojian Zhuang
2013-03-13  5:05 ` [PATCH v3 09/11] ARM: hi3xxx: add board support with device tree Haojian Zhuang
2013-03-13  5:05 ` [PATCH v3 10/11] ARM: hi3xxx: enable hi4511 " Haojian Zhuang
2013-03-13  5:05 ` [PATCH v3 11/11] ARM: config: append arch hi3xxx into multi defconfig Haojian Zhuang

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=5140AA01.1010007@gmail.com \
    --to=robherring2@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org
	public-inbox-index linux-arm-kernel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git