Hi Tomasz, Am Dienstag, den 24.11.2015, 14:32 +0900 schrieb Tomasz Figa: > Hi Philipp, CK, > > Please see my comments inline. > > On Thu, Nov 19, 2015 at 2:34 AM, Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> wrote: [...] > > +Required properties (all function blocks): > > +- compatible: "mediatek,<chip>-disp-<function>", one of > > + "mediatek,<chip>-disp-ovl" - overlay (4 layers, blending, csc) > > + "mediatek,<chip>-disp-rdma" - read DMA / line buffer > > + "mediatek,<chip>-disp-wdma" - write DMA > > + "mediatek,<chip>-disp-color" - color processor > > + "mediatek,<chip>-disp-aal" - adaptive ambient light controller > > + "mediatek,<chip>-disp-gamma" - gamma correction > > + "mediatek,<chip>-disp-ufoe" - data compression engine > > + "mediatek,<chip>-dsi" - DSI controller, see mediatek,dsi.txt > > + "mediatek,<chip>-dpi" - DPI controller, see mediatek,dpi.txt > > + "mediatek,<chip>-disp-mutex" - display mutex > > + "mediatek,<chip>-disp-od" - overdrive > > +- reg: Physical base address and length of the function block register space > > +- interrupts: The interrupt signal from the function block. > > Do all the blocks have only one interrupt signal? As far as I am aware, yes. They all have their own multiplexer, with single output to the GIC each. > > +- clocks: device clocks > > + See Documentation/devicetree/bindings/clock/clock-bindings.txt for details. > > What about clock-names? Also all required clocks for particular > compatible strings should be listed in documentation, All the internal blocks just have a single clock gate. The exceptions are the DSI and DPI controllers, which are documented separately. I'll add a note. > > +- compatible: "mediatek,<chip>-ddp" > > Is this a rebase error? compatible property was already described few > lines above and also missing description. Yes, I'll drop this line. > > + > > +Required properties (DMA function blocks): > > +- compatible: Should be one of > > + "mediatek,<chip>-disp-ovl" > > + "mediatek,<chip>-disp-rdma" > > + "mediatek,<chip>-disp-wdma" > > Perhaps these 3 could also have some short description like the ones > listed above? These three are already included in the list above. Should I make this "Additional required properties (DMA function blocks):" to clarify? > > +- larb: Should contain a phandle pointing to the local arbiter device as defined > > + in Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt > > +- iommus: required a iommu node > > Maybe rewrite to: "Should point to respective IOMMU block as defined > in <path to IOMMU binding documentation>"? I'll change this. thank you Philipp -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
