Re: [PATCH 3/4] perf: xgene: Add APM X-Gene SoC Performance Monitoring Unit driver

[Tai Tri Nguyen]

Hi Mark,

On Tue, Apr 5, 2016 at 12:21 PM, Mark Rutland <mark.rutland@xxxxxxx> wrote:
> On Tue, Apr 05, 2016 at 11:50:10AM -0700, Tai Tri Nguyen wrote:
>> On Mon, Apr 4, 2016 at 4:33 PM, Mark Rutland <mark.rutland@xxxxxxx> wrote:
>> > On Mon, Apr 04, 2016 at 04:42:11PM -0700, Tai Tri Nguyen wrote:
>> >> On Fri, Apr 1, 2016 at 5:18 AM, Mark Rutland <mark.rutland@xxxxxxx> wrote:
>> >> >> +     hwc->config = config;
>> >> >> +     if (config1)
>> >> >> +             hwc->extra_reg.config = config1;
>> >> >> +     else
>> >> >> +             /* Enable all Agents */
>> >> >> +             hwc->extra_reg.config = 0xFFFFFFFFFFFFFFFFULL;
>> >> >
>> >> > I'm not sure I follow what's going on here.
>> >> >
>> >> > It would be good to document precisely what this means.
>> >>
>> >> These are X-Gene PMU specific for monitoring performance of a specific
>> >> data path.
>> >> X-Gene PMUs have 2 registers capable of masking the Agents from which
>> >> the request come from. If the bit with the bit number corresponding to
>> >> the AgentID
>> >> is set, the event will be counted only if it is caused by a request
>> >> from that agent.
>> >> Each PMU has different set of Agents. By default, the event will be counted for
>> >> all agent requests.
>> >>
>> >> I'll have it commented better for next revision of the patch.
>> >
>> > It might be worth having something under Documentation/ for this, similarly to
>> > what we do for CCN in Documentation/arm/CCN.txt.
>> >
>> > How is the user expected to determine agent IDs? Is there a listing somewhere?
>> > Does this change between reivisions? This may be worth documenting.
>> >
>>
>> Each of the SoC PMU has an agent ID list in our product User Manual
>> documentation.
>> An user is expected to refer to the list to determine the agent ID.
>> The agent ID list
>> per each PMU is different. Also we may change or add more agents to the list for
>> next generations of APM X-Gene. I think it would be too much to document it in
>> the Documentation/ folder.
>
> Given that the IDs are so variable, you can simply defer to user manuals in the
> documentation. However, there should definitely be documentation describing the
> format of the config and config1 fields.
>
> Thanks,
> Mark.

Yes, I agree and will add it.

Thanks,
-- 
Tai