Hi Rob, Thanks for providing the review comments. Please find my response inline. > -----Original Message----- > From: Rob Herring <robh@xxxxxxxxxx> > Sent: Thursday, June 3, 2021 12:02 AM > To: Nava kishore Manne <navam@xxxxxxxxxx> > Cc: Michal Simek <michals@xxxxxxxxxx>; mdf@xxxxxxxxxx; trix@xxxxxxxxxx; > arnd@xxxxxxxx; Rajan Vaja <RAJANV@xxxxxxxxxx>; > gregkh@xxxxxxxxxxxxxxxxxxx; Amit Sunil Dhamne > <amitsuni@xxxxxxxxxxxxxxx>; Tejas Patel <tejasp@xxxxxxxxxxxxxxx>; > zou_wei@xxxxxxxxxx; Sai Krishna Potthuri <lakshmis@xxxxxxxxxx>; Ravi > Patel <RAVIPATE@xxxxxxxxxx>; iwamatsu@xxxxxxxxxxx; Jiaying Liang > <jliang@xxxxxxxxxx>; devicetree@xxxxxxxxxxxxxxx; linux-arm- > kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux- > fpga@xxxxxxxxxxxxxxx; git <git@xxxxxxxxxx>; chinnikishore369@xxxxxxxxx > Subject: Re: [PATCH v6 3/4] dt-bindings: firmware: Add bindings for xilinx > firmware > > On Thu, May 20, 2021 at 01:39:53PM +0530, Nava kishore Manne wrote: > > Add documentation to describe Xilinx firmware driver bindings. > > Firmware driver provides an interface to firmware APIs. > > Interface APIs can be used by any driver to communicate to Platform > > Management Unit. > > > > Signed-off-by: Nava kishore Manne <nava.manne@xxxxxxxxxx> > > --- > > Changes for v4: > > -Added new yaml file for xilinx firmware > > as suggested by Rob. > > Changes for v5: > > -Fixed some minor issues and updated the fpga node name to > versal_fpga. > > > > Changes for v6: > > -Added AES and Clk nodes as a sub nodes to the firmware node. > > > > .../firmware/xilinx/xlnx,zynqmp-firmware.yaml | 102 > > ++++++++++++++++++ > > 1 file changed, 102 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp- > firmware > > .yaml > > > > diff --git > > a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp- > firmwa > > re.yaml > > b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp- > firmwa > > re.yaml > > new file mode 100644 > > index 000000000000..58016191e150 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-fi > > +++ rmware.yaml > > @@ -0,0 +1,102 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: > > +http://devicetree.org/schemas/firmware/xilinx/xlnx,zynqmp-firmware.ya > > +ml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Xilinx firmware driver > > + > > +maintainers: > > + - Nava kishore Manne <nava.manne@xxxxxxxxxx> > > + > > +description: > > + The zynqmp-firmware node describes the interface to platform > firmware. > > + ZynqMP has an interface to communicate with secure firmware. > > +Firmware > > + driver provides an interface to firmware APIs. Interface APIs can > > +be > > + used by any driver to communicate to PMUFW(Platform Management > Unit). > > + These requests include clock management, pin control, device > > +control, > > + power management service, FPGA service and other platform > > +management > > + services. > > + > > +properties: > > + compatible: > > + oneOf: > > + - description: > > + For implementations complying for Zynq Ultrascale+ MPSoC. > > + const: xlnx,zynqmp-firmware > > + > > + - description: > > + For implementations complying for Versal. > > + const: xlnx,versal-firmware > > + > > + method: > > + description: | > > + The method of calling the PM-API firmware layer. > > + Permitted values are. > > + - "smc" : SMC #0, following the SMCCC > > + - "hvc" : HVC #0, following the SMCCC > > + > > + $ref: /schemas/types.yaml#/definitions/string-array > > + enum: > > + - smc > > + - hvc > > + > > +patternProperties: > > + "versal_fpga": > > This says 'fooversal_fpgabar' is a valid node name. > > You don't need a pattern, move under 'properties'. Same for the other child > nodes. > Will fix in v7. > > + $ref: "../../fpga/xlnx,versal-fpga.yaml#" > > /schemas/fpga/... > > Don't need quotes. > Will fix in v7. > > + description: Compatible of the FPGA device. > > + type: object > > + required: > > + - compatible > > Drop. What's required should be in xlnx,versal-fpga.yaml. > Will fix in v7. > > + > > + "zynqmp-aes": > > Same comments as above on the rest of the child nodes. > Will fix in v7. > > + $ref: "../../crypto/xlnx,zynqmp-aes.yaml#" > > + description: | > > + The ZynqMP AES-GCM hardened cryptographic accelerator is > > + used to encrypt or decrypt the data with provided key and > > + initialization vector. > > + type: object > > + required: > > + - compatible > > + > > + "clock-controller": > > + $ref: "../../clock/xlnx,versal-clk.yaml#" > > + description: | > > + The clock controller is a hardware block of Xilinx versal > > + clock tree. It reads required input clock frequencies from > > + the devicetree and acts as clock provider for all clock > > + consumers of PS clocks.list of clock specifiers which are > > + external input clocks to the given clock controller. > > + type: object > > + required: > > + - compatible > > + - "#clock-cells" > > + - clocks > > + - clock-names > > + > > +required: > > + - compatible > > + > > +examples: > > + - | > > + versal-firmware { > > + compatible = "xlnx,versal-firmware"; > > + method = "smc"; > > + > > + versal_fpga: versal_fpga { > > + compatible = "xlnx,versal-fpga"; > > + }; > > + > > + xlnx_aes: zynqmp-aes { > > + compatible = "xlnx,zynqmp-aes"; > > + }; > > + > > + versal_clk: clock-controller { > > + #clock-cells = <1>; > > + compatible = "xlnx,versal-clk"; > > + clocks = <&ref>, <&alt_ref>, <&pl_alt_ref>; > > + clock-names = "ref", "alt_ref", "pl_alt_ref"; > > + }; > > + }; > > + > > +additionalProperties: false > > Move this before the example. Will fix in v7. Regards, Navakishore.