| Message ID | 1460510547-17611-3-git-send-email-ttnguyen@apm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Apr 12, 2016 at 06:22:25PM -0700, Tai Nguyen wrote: > Documentation: Add documentation for APM X-Gene SoC PMU DTS binding > > Signed-off-by: Tai Nguyen <ttnguyen@apm.com> > --- > .../devicetree/bindings/perf/apm-xgene-pmu.txt | 116 +++++++++++++++++++++ > 1 file changed, 116 insertions(+) > create mode 100644 Documentation/devicetree/bindings/perf/apm-xgene-pmu.txt > > diff --git a/Documentation/devicetree/bindings/perf/apm-xgene-pmu.txt b/Documentation/devicetree/bindings/perf/apm-xgene-pmu.txt > new file mode 100644 > index 0000000..40dfd4e > --- /dev/null > +++ b/Documentation/devicetree/bindings/perf/apm-xgene-pmu.txt > @@ -0,0 +1,116 @@ > +* APM X-Gene SoC PMU bindings > + > +This is APM X-Gene SoC PMU (Performance Monitoring Unit) module. > +The following PMU devices are supported: > + > + L3C - L3 cache controller > + IOB - IO bridge > + MCB - Memory controller bridge > + MC - Memory controller > + > +The following section describes the SoC PMU DT node binding. > + > +Required properties: > +- compatible : Shall be "apm,xgene-pmu" for revision 1 or > + "apm,xgene-pmu-v2" for revision 2. > +- regmap-csw : Regmap of the CPU switch fabric (CSW) resource. > +- regmap-mcba : Regmap of the MCB-A (memory bridge) resource. > +- regmap-mcbb : Regmap of the MCB-B (memory bridge) resource. > +- reg : First resource shall be the CPU bus PMU resource. > +- interrupts : Interrupt-specifier for PMU IRQ. > + > +Required properties for L3C subnode: > +- compatible : Shall be "apm,xgene-pmu-l3c". > +- reg : First resource shall be the L3C PMU resource. > +- index : Instance number of the L3C PMU. > + > +Required properties for IOB subnode: > +- compatible : Shall be "apm,xgene-pmu-iob". > +- reg : First resource shall be the IOB PMU resource. > +- index : Instance number of the IOB PMU. > + > +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. Rob
Hi Rob, [...] >> +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. Thanks,
On Mon, Apr 18, 2016 at 01:04:53PM -0700, Tai Tri Nguyen wrote: > >> +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. Will
Hi Mark and Rob, [...] > > Regardless, I'll need an ack from Rob or Mark before I can merge this. > > Will Do you have any more comments, please? Thanks,
On Wed, Apr 20, 2016 at 12:31:22PM +0100, Will Deacon wrote: > On Mon, Apr 18, 2016 at 01:04:53PM -0700, Tai Tri Nguyen wrote: > > >> +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
Hi Rob, On Mon, May 2, 2016 at 1:56 PM, Rob Herring <robh@kernel.org> wrote: > On Wed, Apr 20, 2016 at 12:31:22PM +0100, Will Deacon wrote: >> On Mon, Apr 18, 2016 at 01:04:53PM -0700, Tai Tri Nguyen wrote: >> > >> +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. Thanks,
Hi Rob/Mark, Do you have any more comments, please? Thanks, Tai On Mon, May 2, 2016 at 2:46 PM, Tai Tri Nguyen <ttnguyen@apm.com> wrote: > Hi Rob, > > On Mon, May 2, 2016 at 1:56 PM, Rob Herring <robh@kernel.org> wrote: >> On Wed, Apr 20, 2016 at 12:31:22PM +0100, Will Deacon wrote: >>> On Mon, Apr 18, 2016 at 01:04:53PM -0700, Tai Tri Nguyen wrote: >>> > >> +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. > > Thanks, > -- > Tai
Hi all, It's been weeks. I just want to ping again if you have any more comments on this patch set? Thanks, Tai On Tue, May 10, 2016 at 4:43 PM, Tai Tri Nguyen <ttnguyen@apm.com> wrote: > Hi Rob/Mark, > > Do you have any more comments, please? > > Thanks, > Tai > > On Mon, May 2, 2016 at 2:46 PM, Tai Tri Nguyen <ttnguyen@apm.com> wrote: >> Hi Rob, >> >> On Mon, May 2, 2016 at 1:56 PM, Rob Herring <robh@kernel.org> wrote: >>> On Wed, Apr 20, 2016 at 12:31:22PM +0100, Will Deacon wrote: >>>> On Mon, Apr 18, 2016 at 01:04:53PM -0700, Tai Tri Nguyen wrote: >>>> > >> +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. >> >> Thanks, >> -- >> Tai > > > > -- > Tai
On Tue, May 24, 2016 at 02:12:19PM -0700, Tai Tri Nguyen wrote: > It's been weeks. > I just want to ping again if you have any more comments on this patch set? We seem to have reached a stalemate whereby: - The devicetree maintainers don't agree with your binding - You don't agree with them - I won't merge the patch without their ack so I guess you either need to try harder to convince them about the index-based binding, or bite the bullet and fix it to make them happy. Will
On Mon, May 02, 2016 at 02:46:05PM -0700, Tai Tri Nguyen wrote: > Hi Rob, > > On Mon, May 2, 2016 at 1:56 PM, Rob Herring <robh@kernel.org> wrote: > > On Wed, Apr 20, 2016 at 12:31:22PM +0100, Will Deacon wrote: > >> On Mon, Apr 18, 2016 at 01:04:53PM -0700, Tai Tri Nguyen wrote: > >> > >> +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.
Hi Mark, On Tue, May 31, 2016 at 9:56 AM, Mark Rutland <mark.rutland@arm.com> wrote: > On Mon, May 02, 2016 at 02:46:05PM -0700, Tai Tri Nguyen wrote: >> Hi Rob, >> >> On Mon, May 2, 2016 at 1:56 PM, Rob Herring <robh@kernel.org> wrote: >> > On Wed, Apr 20, 2016 at 12:31:22PM +0100, Will Deacon wrote: >> >> On Mon, Apr 18, 2016 at 01:04:53PM -0700, Tai Tri Nguyen wrote: >> >> > >> +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,
Hi Will, On Tue, May 31, 2016 at 9:25 AM, Will Deacon <will.deacon@arm.com> wrote: > On Tue, May 24, 2016 at 02:12:19PM -0700, Tai Tri Nguyen wrote: >> It's been weeks. >> I just want to ping again if you have any more comments on this patch set? > > We seem to have reached a stalemate whereby: > > - The devicetree maintainers don't agree with your binding > - You don't agree with them > - I won't merge the patch without their ack > > so I guess you either need to try harder to convince them about the > index-based binding, or bite the bullet and fix it to make them happy. > > Will Thanks. Yes, I will fix it. Regards,
Hi Mark, On Tue, May 31, 2016 at 10:17 AM, Tai Tri Nguyen <ttnguyen@apm.com> wrote: > Hi Mark, > > On Tue, May 31, 2016 at 9:56 AM, Mark Rutland <mark.rutland@arm.com> wrote: >> On Mon, May 02, 2016 at 02:46:05PM -0700, Tai Tri Nguyen wrote: >>> Hi Rob, >>> >>> On Mon, May 2, 2016 at 1:56 PM, Rob Herring <robh@kernel.org> wrote: >>> > On Wed, Apr 20, 2016 at 12:31:22PM +0100, Will Deacon wrote: >>> >> On Mon, Apr 18, 2016 at 01:04:53PM -0700, Tai Tri Nguyen wrote: >>> >> > >> +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@7e710000 { compatible = "apm,xgene-pmu-mcb"; reg = <0x0 0x7e710000 0x0 0x1000>; enable-mask = <0x00000001>; }; pmucmcb@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,
On Tue, May 31, 2016 at 06:25:56PM -0700, Tai Tri Nguyen wrote: > Hi Mark, [...] > 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. Ah, I see. Can you elaborate on how the indicator bits are laid out? From the example binding, I see multiple nodes with the same index property, so I'm a little confused. I guess that there's a CSR per class of node (e.g. all MCBs in one CSR register)? Or do several nodes share the same bit? Is there a single CSR register? Are there several? Is that bit index used in other registers? > For MC and MCB nodes only, can I introduce an "enable-mask" field? > For example: > " > pmucmcb@7e710000 { > compatible = "apm,xgene-pmu-mcb"; > reg = <0x0 0x7e710000 0x0 0x1000>; > enable-mask = <0x00000001>; > }; > > pmucmcb@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? Assuming it's always a single bit, a *-bit-index property may be fine, and probably preferable. I'm a little confused, so I'd like to understand how it's used. Thanks, Mark.
Hi Mark, On Mon, Jun 6, 2016 at 10:29 AM, Mark Rutland <mark.rutland@arm.com> wrote: > On Tue, May 31, 2016 at 06:25:56PM -0700, Tai Tri Nguyen wrote: >> Hi Mark, > > [...] > >> 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. > > Ah, I see. > > Can you elaborate on how the indicator bits are laid out? From the > example binding, I see multiple nodes with the same index property, so > I'm a little confused. > > I guess that there's a CSR per class of node (e.g. all MCBs in one CSR > register)? Or do several nodes share the same bit? > > Is there a single CSR register? Are there several? Is that bit index > used in other registers? In our current XGene platforms, there are 2 MCBs and each MCB has 2 MCs. MCB0 --> MC0 and MC1 MCB1 --> MC2 and MC3 There are 2 CSR are being used (each uses 1 bit) together as indicators: CSR1 indicates if both MCBs are active. CSR2 indicates if both MCs of the same MCB are active. This is the true table: dual MCB dual MC 0 0 --> MC0 active 0 1 --> MC0/MC1 active 1 0 --> MC0/MC2 active 1 1 --> MC0/MC1/MC2/MC3 active At the time the driver initializes, I check the 2 CSRs to come up with the mcb_active_mask and mc_active_mask. The index bits are used to compare against the mcb_active_mask/mc_active_mask. > >> For MC and MCB nodes only, can I introduce an "enable-mask" field? >> For example: >> " >> pmucmcb@7e710000 { >> compatible = "apm,xgene-pmu-mcb"; >> reg = <0x0 0x7e710000 0x0 0x1000>; >> enable-mask = <0x00000001>; >> }; >> >> pmucmcb@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? > > Assuming it's always a single bit, a *-bit-index property may be fine, > and probably preferable. > Yes, it's always a single bit. I'll use *-bit-index for these sub-nodes. Thanks, -- Tai
diff --git a/Documentation/devicetree/bindings/perf/apm-xgene-pmu.txt b/Documentation/devicetree/bindings/perf/apm-xgene-pmu.txt new file mode 100644 index 0000000..40dfd4e --- /dev/null +++ b/Documentation/devicetree/bindings/perf/apm-xgene-pmu.txt @@ -0,0 +1,116 @@ +* APM X-Gene SoC PMU bindings + +This is APM X-Gene SoC PMU (Performance Monitoring Unit) module. +The following PMU devices are supported: + + L3C - L3 cache controller + IOB - IO bridge + MCB - Memory controller bridge + MC - Memory controller + +The following section describes the SoC PMU DT node binding. + +Required properties: +- compatible : Shall be "apm,xgene-pmu" for revision 1 or + "apm,xgene-pmu-v2" for revision 2. +- regmap-csw : Regmap of the CPU switch fabric (CSW) resource. +- regmap-mcba : Regmap of the MCB-A (memory bridge) resource. +- regmap-mcbb : Regmap of the MCB-B (memory bridge) resource. +- reg : First resource shall be the CPU bus PMU resource. +- interrupts : Interrupt-specifier for PMU IRQ. + +Required properties for L3C subnode: +- compatible : Shall be "apm,xgene-pmu-l3c". +- reg : First resource shall be the L3C PMU resource. +- index : Instance number of the L3C PMU. + +Required properties for IOB subnode: +- compatible : Shall be "apm,xgene-pmu-iob". +- reg : First resource shall be the IOB PMU resource. +- index : Instance number of the IOB PMU. + +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. + +Example: + csw: csw@7e200000 { + compatible = "apm,xgene-csw", "syscon"; + reg = <0x0 0x7e200000 0x0 0x1000>; + }; + + mcba: mcba@7e700000 { + compatible = "apm,xgene-mcb", "syscon"; + reg = <0x0 0x7e700000 0x0 0x1000>; + }; + + mcbb: mcbb@7e720000 { + compatible = "apm,xgene-mcb", "syscon"; + reg = <0x0 0x7e720000 0x0 0x1000>; + }; + + pmu: pmu@78810000 { + compatible = "apm,xgene-pmu-v2"; + #address-cells = <2>; + #size-cells = <2>; + ranges; + regmap-csw = <&csw>; + regmap-mcba = <&mcba>; + regmap-mcbb = <&mcbb>; + reg = <0x0 0x78810000 0x0 0x1000>; + interrupts = <0x0 0x22 0x4>; + + pmul3c@7e610000 { + compatible = "apm,xgene-pmu-l3c"; + reg = <0x0 0x7e610000 0x0 0x1000>; + index = <0>; + }; + + pmuiob@7e940000 { + compatible = "apm,xgene-pmu-iob"; + reg = <0x0 0x7e940000 0x0 0x1000>; + index = <0>; + }; + + pmucmcb@7e710000 { + compatible = "apm,xgene-pmu-mcb"; + reg = <0x0 0x7e710000 0x0 0x1000>; + index = <0>; + }; + + pmucmcb@7e730000 { + compatible = "apm,xgene-pmu-mcb"; + reg = <0x0 0x7e730000 0x0 0x1000>; + index = <1>; + }; + + pmucmc@7e810000 { + compatible = "apm,xgene-pmu-mc"; + reg = <0x0 0x7e810000 0x0 0x1000>; + index = <0>; + }; + + pmucmc@7e850000 { + compatible = "apm,xgene-pmu-mc"; + reg = <0x0 0x7e850000 0x0 0x1000>; + index = <1>; + }; + + pmucmc@7e890000 { + compatible = "apm,xgene-pmu-mc"; + reg = <0x0 0x7e890000 0x0 0x1000>; + index = <2>; + }; + + pmucmc@7e8d0000 { + compatible = "apm,xgene-pmu-mc"; + reg = <0x0 0x7e8d0000 0x0 0x1000>; + index = <3>; + }; + };
Documentation: Add documentation for APM X-Gene SoC PMU DTS binding Signed-off-by: Tai Nguyen <ttnguyen@apm.com> --- .../devicetree/bindings/perf/apm-xgene-pmu.txt | 116 +++++++++++++++++++++ 1 file changed, 116 insertions(+) create mode 100644 Documentation/devicetree/bindings/perf/apm-xgene-pmu.txt