[PATCH v4 1/4] dt-bindings: firmware: Add bindings for xilinx firmware

[Nava kishore Manne]

Hi Rob,

	Please find my response inline.

> -----Original Message-----
> From: Rob Herring <robh at kernel.org>
> Sent: Saturday, May 1, 2021 1:10 AM
> To: Nava kishore Manne <navam at xilinx.com>
> Cc: Michal Simek <michals at xilinx.com>; mdf at kernel.org; trix at redhat.com;
> gregkh at linuxfoundation.org; arnd at arndb.de; Rajan Vaja
> <RAJANV at xilinx.com>; Amit Sunil Dhamne <amitsuni at xlnx.xilinx.com>;
> Manish Narani <MNARANI at xilinx.com>; zou_wei at huawei.com; Sai Krishna
> Potthuri <lakshmis at xilinx.com>; iwamatsu at nigauri.org; Jiaying Liang
> <jliang at xilinx.com>; linus.walleij at linaro.org; devicetree at vger.kernel.org;
> linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org; linux-
> fpga at vger.kernel.org; chinnikishore369 at gmail.com; git <git at xilinx.com>
> Subject: Re: [PATCH v4 1/4] dt-bindings: firmware: Add bindings for xilinx
> firmware
> 
> On Thu, Apr 29, 2021 at 07:34:05PM +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 at xilinx.com>
> > ---
> > Changes for v4:
> >                 -Added new yaml file for xilinx firmware
> >                  as suggested by Rob.
> >
> >  .../firmware/xilinx/xlnx,zynqmp-firmware.yaml | 63
> > +++++++++++++++++++
> >  1 file changed, 63 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-
> firmware
> > .yaml
> 
> What about the old doc?:
> 

As you suggested i have added only the fpga node relevant info here so it's not representing the complete firmware file with other sub node like clk, Aes, ...etc.
Once it completely mature we can deprecate the Old doc.

> Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-
> firmware.txt
> 
> >
> > 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..4b97f005bed7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-fi
> > +++ rmware.yaml
> > @@ -0,0 +1,63 @@
> > +# 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 at xilinx.com>
> > +
> > +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
> 
> Drop the '#'. If you want to maintain the formatting, then use '|' after
> 'description:' for a literal block.
> 

Will fix in v5.

> > +    $ref: /schemas/types.yaml#/definitions/string-array
> > +    enum:
> > +      - smc
> > +      - hvc
> > +
> > +patternProperties:
> > +  "fpga":
> 
> So 'foofpgabar' is valid?
> 

Yes, it's a valid for fpga node.

> > +    description: Compatible of the FPGA device.
> > +    type: object
> > +
> > +    required:
> > +      - compatible
> 
> This belongs in the fpga schema which should have a $ref here. (Which
> means this patch needs to come 2nd.)
> 

Will fix in v5.

Regards,
Navakishore.