On Wed, 27 Mar 2019 13:57:10 +0100, Hans Verkuil wrote: > On 3/1/19 4:27 PM, Michael Tretter wrote: > > Add device-tree bindings for the Allegro DVT video IP core found on the > > Xilinx ZynqMP EV family. > > > > Signed-off-by: Michael Tretter <m.tretter@xxxxxxxxxxxxxx> > > Reviewed-by: Rob Herring <robh@xxxxxxxxxx> > > --- > > v3 -> v4: > > none > > > > v2 -> v3: > > - rename node to video-codec > > - drop interrupt-names > > - fix compatible in example > > - add clocks to required properties > > > > v1 -> v2: > > none > > --- > > .../devicetree/bindings/media/allegro.txt | 43 +++++++++++++++++++ > > 1 file changed, 43 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/media/allegro.txt > > > > diff --git a/Documentation/devicetree/bindings/media/allegro.txt b/Documentation/devicetree/bindings/media/allegro.txt > > new file mode 100644 > > index 000000000000..a92e2fbf26c9 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/allegro.txt > > @@ -0,0 +1,43 @@ > > +Device-tree bindings for the Allegro DVT video IP codecs present in the Xilinx > > +ZynqMP SoC. The IP core may either be a H.264/H.265 encoder or H.264/H.265 > > +decoder ip core. > > + > > +Each actual codec engines is controlled by a microcontroller (MCU). Host > > +software uses a provided mailbox interface to communicate with the MCU. The > > +MCU share an interrupt. > > + > > +Required properties: > > + - compatible: value should be one of the following > > + "allegro,al5e-1.1", "allegro,al5e": encoder IP core > > + "allegro,al5d-1.1", "allegro,al5d": decoder IP core > > checkpatch give me: > > WARNING: DT compatible string vendor "allegro" appears un-documented -- check ./Documentation/devicetree/bindings/vendor-prefixes.txt > #2714: FILE: drivers/staging/media/allegro-dvt/allegro-core.c:2636: > + { .compatible = "allegro,al5e-1.1" }, > > I think you should probably replace allegro by xlnx. The video-codec and the firmware is actually Allegro IP. Xilinx integrated the IP cores into the SoC with their "VCU System-Level Control" module, which uses the "xlnx,vcu" binding. I would rather add "allegro" to the vendor-prefixes.txt than replacing it with xlnx. Michael > > Regards, > > Hans > > > + - reg: base and length of the memory mapped register region and base and > > + length of the memory mapped sram > > + - reg-names: must include "regs" and "sram" > > + - interrupts: shared interrupt from the MCUs to the processing system > > + - clocks: must contain an entry for each entry in clock-names > > + - clock-names: must include "core_clk", "mcu_clk", "m_axi_core_aclk", > > + "m_axi_mcu_aclk", "s_axi_lite_aclk" > > + > > +Example: > > + al5e: video-codec@a0009000 { > > + compatible = "allegro,al5e-1.1", "allegro,al5e"; > > + reg = <0 0xa0009000 0 0x1000>, > > + <0 0xa0000000 0 0x8000>; > > + reg-names = "regs", "sram"; > > + interrupts = <0 96 4>; > > + clocks = <&xlnx_vcu 0>, <&xlnx_vcu 1>, > > + <&clkc 71>, <&clkc 71>, <&clkc 71>; > > + clock-names = "core_clk", "mcu_clk", "m_axi_core_aclk", > > + "m_axi_mcu_aclk", "s_axi_lite_aclk" > > + }; > > + al5d: video-codec@a0029000 { > > + compatible = "allegro,al5d-1.1", "allegro,al5d"; > > + reg = <0 0xa0029000 0 0x1000>, > > + <0 0xa0020000 0 0x8000>; > > + reg-names = "regs", "sram"; > > + interrupts = <0 96 4>; > > + clocks = <&xlnx_vcu 2>, <&xlnx_vcu 3>, > > + <&clkc 71>, <&clkc 71>, <&clkc 71>; > > + clock-names = "core_clk", "mcu_clk", "m_axi_core_aclk", > > + "m_axi_mcu_aclk", "s_axi_lite_aclk" > > + }; > > > >