[AMD Official Use Only - General] > -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> > Sent: Wednesday, October 19, 2022 5:53 PM > To: Datta, Shubhrajyoti <shubhrajyoti.datta@xxxxxxx>; linux-arm- > kernel@xxxxxxxxxxxxxxxxxxx > Cc: git (AMD-Xilinx) <git@xxxxxxx>; devicetree@xxxxxxxxxxxxxxx; > will@xxxxxxxxxx; mark.rutland@xxxxxxx; robh+dt@xxxxxxxxxx; > krzysztof.kozlowski+dt@xxxxxxxxxx; michal.simek@xxxxxxxxxx > Subject: Re: [PATCH v1 1/2] dt-bindings: Add the binding doc for xilinx APM > > Caution: This message originated from an External Source. Use proper > caution when opening attachments, clicking links, or responding. > > > On 19/10/2022 05:17, Shubhrajyoti Datta wrote: > > The LogiCORE IP AXI Performance Monitor core enables AXI system > > performance measurement for multiple slots (AXI4/AXI3/ > > AXI4-Stream/AXI4-Lite) activity. Add the devicetree binding for xilinx > > APM. > > Subject: > Drop redundant "bindings" and add missing prefix, so: > > dt-bindings: perf: Add Xilinx APM Will update. > > > > > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xxxxxxx> > > --- > > > > Changes in v1: > > This should be then a v2. Earlier one was an RFC so I had made it v1. > > > - Use boolean for the values xlnx,enable-profile , xlnx,enable-trace > > and xlnx,enable-event-count > > - Update the file name > > - use generic node name pmu > > > > .../bindings/perf/xlnx,axi-perf-monitor.yaml | 133 > > ++++++++++++++++++ > > 1 file changed, 133 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/perf/xlnx,axi-perf-monitor.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/perf/xlnx,axi-perf-monitor.yaml > > b/Documentation/devicetree/bindings/perf/xlnx,axi-perf-monitor.yaml > > new file mode 100644 > > index 000000000000..bd3a9dbc1184 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/perf/xlnx,axi-perf-monitor.yam > > +++ l > > @@ -0,0 +1,133 @@ > > +# SPDX-License-Identifier: GPL-2.0-or-later OR BSD-2-Clause %YAML 1.2 > > +--- > > +$id: > > > +https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi > > +cetree.org%2Fschemas%2Fperf%2Fxlnx%2Caxi-perf- > monitor.yaml%23&dat > > > +a=05%7C01%7Cshubhrajyoti.datta%40amd.com%7C15905dd06b164f7de3d > 508dab1 > > > +ccb630%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638017790 > 04335677 > > > +2%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMz > IiLCJBTiI > > > +6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=zOrK%2FG > dlP87S%2FTp > > +XqdnrNSk0PyJgWRJYU4EZHgJtqMA%3D&reserved=0 > > +$schema: > > > +https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi > > +cetree.org%2Fmeta- > schemas%2Fcore.yaml%23&data=05%7C01%7Cshubhrajy > > > +oti.datta%40amd.com%7C15905dd06b164f7de3d508dab1ccb630%7C3dd89 > 61fe488 > > > +4e608e11a82d994e183d%7C0%7C0%7C638017790043356772%7CUnknown > %7CTWFpbGZ > > > +sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6 > Mn0% > > > +3D%7C3000%7C%7C%7C&sdata=Vl1TpXVHyuS6YmnSP%2BKPOO8D5ap > 0y9jtV52Q9s > > +%2F1pvQ%3D&reserved=0 > > + > > +title: Xilinx Axi Performance Monitor > > + > > +maintainers: > > + - Shubhrajyoti Datta <shubhrajyoti.datta@xxxxxxx> > > + > > +properties: > > + compatible: > > + const: xlnx,axi-perf-monitor > > + > > + reg: > > + maxItems: 1 > > + > > + clocks: > > + maxItems: 1 > > + > > + interrupts: > > + maxItems: 1 > > + > > + xlnx,enable-profile: > > + description: > > + Enables the profile mode. Event counting in profile mode consists of a > > + fixed number of accumulators for each AXI4/AXI3/AXI4-Lite slot. All the > > + events that can be counted are detected and given to the accumulator > > + which calculates the aggregate value. There is no selection of events, > > + and in this mode, event counting is done only on AXI4/AXI3/AXI4-Lite > > + monitor slots. > > + type: boolean > > + > > + xlnx,enable-trace: > > + description: > > + Enables trace mode. In trace mode, the APM provides event logging in > a > > + reduced dynamic configuration. It captures the specified AXI events, > > + external events and the time stamp difference between two successive > > + events into the streaming FIFO. The selection of events to be captured > > + is set through parameter configuration. Streaming agents are not > > + supported in trace mode. > > + type: boolean > > Both enable profile and enable trace do not look like hardware properties, > but rather runtime features. In what use case this enabling trace or profile > should be a property of a hardware? > The hardware being on FPGA is configurable what capabilities it will have. Once chosen to have say the trace it will have tracing capabilities else it will not have. > > + > > + xlnx,num-monitor-slots: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: > > + Number of monitor slots. > > + minimum: 1 > > + maximum: 8 > > + > > + xlnx,enable-event-count: > > + description: > > + Enable event count. > > The same > > > + type: boolean > > + > > + xlnx,enable-event-log: > > + type: boolean > > + description: > > + Enable event log. > > The same Similarly whether event logging capabilities will be there in the hardware is configurable. If enabled then it have event logging capabilities. > > > + > > + xlnx,have-sampled-metric-cnt: > > + type: boolean > > + description: > > + Sampled metric counters enabled in APM. > > + > > + xlnx,metric-count-width: > > + allOf: > > + - $ref: /schemas/types.yaml#/definitions/uint32 > > + - enum: [32, 64] > > This is a friendly reminder during the review process. > > It seems my previous comments were not fully addressed. Maybe my > feedback got lost between the quotes, maybe you just forgot to apply it. > Please go back to the previous discussion and either implement all requested > changes or keep discussing them. > I tried to address the comments in https://lore.kernel.org/linux-arm-kernel/BY5PR12MB4902474D74155E57BF5D7B9C814F9@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/ I thought I have implemented all the comments. Let me know if I missed something. Thanks and Regards, Shubhrajyoti > Thank you. > > Best regards, > Krzysztof
