Hi Mark,

On Tue, May 31, 2016 at 10:17 AM, Tai Tri Nguyen [off-list ref] wrote:

Hi Mark,

On Tue, May 31, 2016 at 9:56 AM, Mark Rutland [off-list ref] wrote:

quoted

On Mon, May 02, 2016 at 02:46:05PM -0700, Tai Tri Nguyen wrote:

quoted

Hi Rob,

On Mon, May 2, 2016 at 1:56 PM, Rob Herring [off-list ref] wrote:

quoted

On Wed, Apr 20, 2016 at 12:31:22PM +0100, Will Deacon wrote:

quoted

On Mon, Apr 18, 2016 at 01:04:53PM -0700, Tai Tri Nguyen wrote:

quoted

quoted

quoted

+Required properties for MCB subnode:
+- compatible         : Shall be "apm,xgene-pmu-mcb".
+- reg                        : First resource shall be the MCB PMU resource.
+- index                      : Instance number of the MCB PMU.
+
+Required properties for MC subnode:
+- compatible         : Shall be "apm,xgene-pmu-mc".
+- reg                        : First resource shall be the MC PMU resource.
+- index                      : Instance number of the MC PMU.

Don't use indexes. You probably need phandles to the nodes these are
related to.

How many variations of child nodes do you expect to have? 2, 10, 50? You
might want to just collapse all this down to a single node and put this
information in the driver if it is fixed for each SoC and there's only a
handful.

For each kind of PMU, for example memory controller PMU, I expect to
have the number of instances up to 8.
They are actually all independent PMU nodes and have their own CSR memory bases.
The indexes are used for exposing the devices to perf user only. It
doesn't have an impact on the programming model.
Mark also had the same concern.

Regardless, I'll need an ack from Rob or Mark before I can merge this.

I still have a concern with this. Needing an index to expose to the user
is generally not a valid reason. That's OS specific and therefore
doesn't belong in DT.

Rob

I can use device name here. However, the perf event names will be
different between DT and ACPI which I want to avoid.
And the names don't look good at all.
Also, specifically for MC and MCB PMUs, the indexes are compared
against the active MC/MCB mask to find out whether they are populated
or not.
Without using the index property, I will also need a mapping function
of physical device addresses and their physical ids.

What's wrong with using ${device}.{physical_address} as the PMU name?
That would be unique and consistent regardless of the firmware, no
mapping nor index property necessary.

That's sufficient for any user already familiar with the topology, a
familiarity you seem to be assuming regardless by not explicitly
describing the topology in the DT.

Thanks,
Mark.

Okay. I'll do fix it for the next patches.

Thanks,
--
Tai

I'm facing a problem after removing the index for MCU and MC sub-nodes.
The MCUs and MCs aren't always enabled depending on how DRAM DIMMs are
installed on the system.
I still need a way to associate the MCU with its indicator bit in the
enable mask retrieved from CSR.
For MC and MCB nodes only, can I introduce an "enable-mask" field?
For example:
"
pmucmcb at 7e710000 {
        compatible = "apm,xgene-pmu-mcb";
        reg = <0x0 0x7e710000 0x0 0x1000>;
        enable-mask = <0x00000001>;
};

pmucmcb at 7e730000 {
        compatible = "apm,xgene-pmu-mcb";
        reg = <0x0 0x7e730000 0x0 0x1000>;
        enable-mask = <0x00000002>;
};
"
Or can you please give a suggestion how I can fix it?

Thanks,
-- 
Tai