[PATCH] drivers: CCI: add ARM CCI PMU support

[Kumar Gala]

On Aug 16, 2013, at 5:31 AM, Punit Agrawal wrote:

> On 15/08/13 17:25, Kumar Gala wrote:
>> 
>> On Aug 15, 2013, at 4:10 AM, Punit Agrawal wrote:
>> 
>>> Hi Kumar,
>>> 
>>> Thanks for a review of the bindings.
>>> 
>>> On 14/08/13 22:03, Kumar Gala wrote:
>>>> 
>>>> On Jul 23, 2013, at 4:19 AM, Punit Agrawal wrote:
>>>> 
>>>>> The CCI PMU can profile bus transactions at the master and slave
>>>>> interfaces of the CCI. The PMU can be used to observe an aggregated view
>>>>> of the bus traffic between the various components connected to the CCI.
>>>>> 
>>>>> Extend the existing CCI driver to support the PMU by registering a perf
>>>>> backend for it.
>>>>> 
>>>>> Document the device tree binding to describe the CCI PMU.
>>>>> 
>>>>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi at arm.com>
>>>>> Cc: Nicolas Pitre <nico at linaro.org>
>>>>> Cc: Dave Martin <dave.martin at linaro.org>
>>>>> Cc: Rob Herring <rob.herring at calxeda.com>
>>>>> Cc: Will Deacon <will.deacon at arm.com>
>>>>> Signed-off-by: Punit Agrawal <punit.agrawal at arm.com>
>>>>> Reviewed-by: Will Deacon <will.deacon at arm.com>
>>>>> ---
>>>>> Documentation/devicetree/bindings/arm/cci.txt |   38 ++
>>>>> drivers/bus/arm-cci.c                         |  642 +++++++++++++++++++++++++
>>>>> 2 files changed, 680 insertions(+)
>>>>> 
>>>>> diff --git a/Documentation/devicetree/bindings/arm/cci.txt b/Documentation/devicetree/bindings/arm/cci.txt
>>>>> index 92d36e2..5bc95e5 100644
>>>>> --- a/Documentation/devicetree/bindings/arm/cci.txt
>>>>> +++ b/Documentation/devicetree/bindings/arm/cci.txt
>>>>> @@ -79,6 +79,34 @@ specific to ARM.
>>>>> 				    corresponding interface programming
>>>>> 				    registers.
>>>>> 
>>>>> +	- CCI PMU node
>>>>> +
>>>>> +		Node name must be "pmu".
>>>>> +		Parent node must be CCI interconnect node.
>>>>> +
>>>>> +		A CCI pmu node must contain the following properties:
>>>>> +
>>>>> +		- compatible
>>>>> +			Usage: required
>>>>> +			Value type: <string>
>>>>> +			Definition: must be set to one of
>>>>> +				    "arm,cci-400-pmu"
>>>>> +				    "arm,cci-400-pmu,rev0"
>>>>> +				    "arm,cci-400-pmu,rev1"
>>>> 
>>>> Do you really mean only one?  Seems like ""arm,cci-400-pmu,rev0", "arm,cci-400-pmu" would be valid.
>>>> 
>>> 
>>> Hmm... yes both would be valid. But...
>>> 
>>> The event numbering scheme changed between Rev 0 and Rev 1 of the CCI. If the revision is specified then it is used to get the event ranges to validate the events. If not, i.e., "arm,cci-400-pmu" is used, then the driver tries to find the the revision by reading the peripheral id registers.
>>> 
>>> I was trying to make the bindings robust in the face of change in behaviour between different revisons of the IP.
>> 
>> If there is a periph id register why bother with the device tree having different version info in it?
>> 
> 
> The different version strings are useful when the identification registers are either incorrect or broken.
> 
> But I am not aware of any such platforms currently out there. I can remove the additional compatible strings and rely on the peripheral id register solely. Do you prefer that?


Yes please make this change.  While I agree the compat field is useful when ID registers are broken, if they are known to be correct I would recommend utilizing them until the situation arises that they arent.

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation