linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: robherring2@gmail.com (Rob Herring)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH] ARM: pmu: add OF match support
Date: Wed, 09 Feb 2011 10:55:06 -0600	[thread overview]
Message-ID: <4D52C6EA.1030600@gmail.com> (raw)
In-Reply-To: <4D529E82.1000102@gmail.com>

Grant,

On 02/09/2011 08:02 AM, Rob Herring wrote:
> Will,
>
> On 02/09/2011 03:51 AM, Will Deacon wrote:
>> Hi Rob,
>>
>>> Add OF match table to enable OF style driver binding. The dts entry
>>> is like
>>> this:
>>>
>>> pmu {
>>> compatible = "arm,pmu";
>>> interrupts =<100 101>;
>>> };
>>>
>>> The use of pdev->id as an index breaks with OF device binding. Change
>>> to use
>>> a counter instead. If more than 1 pmu device is ever really supported, a
>>> better solution to match users with particular pmu is probably needed.
>>
>> We will want to support multiple PMU devices in the near future but
>> this is currently blocked on userspace issues. One such device would
>> be the event counters on the PL310, which could be used to complement
>> the CPU PMU on the Cortex-A9.
>>
>> I don't like the idea of a counter as it forces platforms to register
>> their devices in a specific order. Is it possible to separate different
>> types of PMU in the device tree and then have pmu.c work out what to do
>> with them?
>
> Yes, they are easily distinguished by the compatible strings. Adding a
> pmu_get function which returns the device based on a string or
> capabilities may be a better solution. If you had 2 pmu's of the same
> type, the current code would break.
>
> In any case, how about this:
>
> static int __devinit pmu_device_probe(struct platform_device *pdev)
> {
> enum arm_pmu_type type = pdev->id;
>
> if (of_device_is_compatible(pdev->dev.of_node, "arm,pmu"))
> type = ARM_PMU_DEVICE_CPU;

Actually, this needs to be:

#ifdef CONFIG_OF
	if (of_device_is_compatible(pdev->dev.of_node, "arm,pmu"))
		type = ARM_PMU_DEVICE_CPU;
#endif

Seems fixing the struct device conditional is just the tip of the 
iceberg. Should we do empty functions for all driver accessed OF functions?

Rob

  reply	other threads:[~2011-02-09 16:55 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-09  3:53 [RFC PATCH] ARM: pmu: add OF match support Rob Herring
2011-02-09  9:51 ` Will Deacon
2011-02-09 14:02   ` Rob Herring
2011-02-09 16:55     ` Rob Herring [this message]
2011-02-09 17:10       ` Grant Likely
2011-02-09 17:13 ` Grant Likely
2011-02-09 17:17   ` Will Deacon
     [not found]   ` <3399156206431206254@unknownmsgid>
2011-02-09 18:09     ` Grant Likely
2011-02-09 18:53       ` Will Deacon
     [not found]       ` <-4919269306841091192@unknownmsgid>
2011-02-09 19:03         ` Grant Likely
2011-02-09 19:13           ` Lorenzo Pieralisi
2011-02-09 23:00           ` Rob Herring
2011-02-13  6:23             ` Grant Likely

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=4D52C6EA.1030600@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
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).