Re: [PATCH v2 1/3] dt-bindings: mediatek: Add a binding for Mediatek JPEG Decoder
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: 2016-11-03 18:33:18
Also in:
linux-media, lkml
Hi Rick, Thank you for the patch. On Monday 31 Oct 2016 15:16:55 Rick Chang wrote:
quoted hunk ↗ jump to hunk
Add a DT binding documentation for Mediatek JPEG Decoder of MT2701 SoC. Signed-off-by: Rick Chang <redacted> Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com> --- .../bindings/media/mediatek-jpeg-codec.txt | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/mediatek-jpeg-codec.txtdiff --git a/Documentation/devicetree/bindings/media/mediatek-jpeg-codec.txtb/Documentation/devicetree/bindings/media/mediatek-jpeg-codec.txt new file mode 100644 index 0000000..514e656--- /dev/null +++ b/Documentation/devicetree/bindings/media/mediatek-jpeg-codec.txt@@ -0,0 +1,35 @@ +* Mediatek JPEG Codec
Is it a codec or a decoder only ?
+Mediatek JPEG Codec device driver is a v4l2 driver which can decode +JPEG-encoded video frames.
DT bindings should not reference drivers, they are OS-agnostic.
+Required properties: + - compatible : "mediatek,mt2701-jpgdec" + - reg : Physical base address of the jpeg codec registers and length of + memory mapped region. + - interrupts : interrupt number to the cpu.
That's actually not correct, the interrupt number is local to the interrupt controller, not to the CPU.
+ - clocks : clock name from clock manager
The clocks property doesn't contain a name. Until we provide standardized descriptions for those properties, I recommend copying the compatible, reg, interrupts, clocks, clock-names, power-domains and iommus properties descriptions from good DT bindings. Which DT bindings are good source of inspiration here is left as an exercise for the reader I'm afraid :-(
+ - clock-names: the clocks of the jpeg codec H/W + - power-domains : a phandle to the power domain. + - larb : must contain the larbes of current platform
Shouldn't this be mediatek,larb ? And what is a larb ?
+ - iommus : Mediatek IOMMU H/W has designed the fixed associations with + the multimedia H/W. and there is only one multimedia iommu domain. + "iommus = <&iommu portid>" the "portid" is from + dt-bindings\iommu\mt2701-iommu-port.h, it means that this portid will + enable iommu. The portid default is disable iommu if "<&iommu>
portid>"
+ don't be added.
There are two iommus instances in your example below, this should be documented. This description is not very clear I'm afraid.
+
+Example:
+ jpegdec: jpegdec@15004000 {
+ compatible = "mediatek,mt2701-jpgdec";
+ reg = <0 0x15004000 0 0x1000>;
+ interrupts = <GIC_SPI 143 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&imgsys CLK_IMG_JPGDEC_SMI>,
+ <&imgsys CLK_IMG_JPGDEC>;
+ clock-names = "jpgdec-smi",
+ "jpgdec";
+ power-domains = <&scpsys MT2701_POWER_DOMAIN_ISP>;
+ mediatek,larb = <&larb2>;
+ iommus = <&iommu MT2701_M4U_PORT_JPGDEC_WDMA>,
+ <&iommu MT2701_M4U_PORT_JPGDEC_BSDMA>;
+ };-- Regards, Laurent Pinchart