Hi, On 4 March 2013 00:41, Sylwester Nawrocki <sylvester.nawrocki@xxxxxxxxx> wrote: > On 02/28/2013 10:42 AM, Vikas Sajjan wrote: >> >> Adds FIMD DT binding documentation both Samsung SoC and Board, with an >> example >> >> Signed-off-by: Vikas Sajjan<vikas.sajjan@xxxxxxxxxx> >> --- >> .../devicetree/bindings/video/samsung-fimd.txt | 54 >> ++++++++++++++++++++ >> 1 file changed, 54 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/video/samsung-fimd.txt >> >> diff --git a/Documentation/devicetree/bindings/video/samsung-fimd.txt >> b/Documentation/devicetree/bindings/video/samsung-fimd.txt >> new file mode 100644 >> index 0000000..8d201e7 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/video/samsung-fimd.txt >> @@ -0,0 +1,54 @@ >> +Device-Tree bindings for Samsung SoC display controller (FIMD) >> + >> +FIMD stands for Fully Interactive Mobile Display, is the Display >> Controller for >> +the Samsung series of SoCs which transfers the image data from a video >> buffer >> +located in the system memory to an external LCD interface. >> + >> +Required properties: >> +- compatible := value should be one of the following >> + "samsung,s3c2443-fimd"; /* for S3C24XX SoCs */ >> + "samsung,s3c6400-fimd"; /* for S3C64XX SoCs */ >> + "samsung,s5p6440-fimd"; /* for S5P64X0 SoCs */ >> + "samsung,s5pc100-fimd"; /* for S5PC100 SoC */ >> + "samsung,s5pv210-fimd"; /* for S5PV210 SoC */ >> + "samsung,exynos4210-fimd"; /* for Exynos4 SoCs */ >> + "samsung,exynos5250-fimd"; /* for Exynos5 SoCs */ >> + >> +- reg := physical base address of the fimd and length of memory mapped >> region > > > I think FIMD should be capitalized. > Right. > >> + >> +- interrupt-parent := reference to the interrupt combiner node with >> phandle > > > Perhaps this would have been more clear: > > - interrupt-parent : a phandle to the interrupt combiner node; > > ? OK. > > And could we just use a colon instead of ":=", to keep it more consistent > with other Samsung SoC DT bindings documentation ? > OK. > >> + >> +- interrupts := interrupt number from the combiner to the cpu. >> + We have 3 interrupts and the interrupt combiner order is >> + FIFO Level, VSYNC, and LCD_SYSTEM. >> + but since the driver expects VSYNC to be the first IRQ, >> + make sure to mention order as VSYNC, FIFO Level and >> LCD_SYSTEM >> + keeping VSYNC as first IRQ as shown below. >> + for example: interrupts =<11 1>,<11 0>,<11 2>; > > > I have a suggestion here. What about using reg-names property to make this > more explicit, e.g. > > - interrupts : should contain a list of all FIMD IP block interrupts: > FIFO Level, VSYNC, LCD_SYSTEM. The interrupt specifier format depends > on the interrupt controller used; > > - interrupt-names : should contain the interrupt names: "fifo", "vsync", > "lcd_sys", in the order in which they were listed in the interrupts > property; > Good idea, but i am just wondering is it a good idea to modify the fimd driver ? > Or something similar to that. Then the drivers would need to be modified > to get interrupt resource by name, and not to rely on the VSYNC interrupt > be listed as the first one. > > >> + >> +- pinctrl := property defining the pinctrl configurations with a phandle >> + >> +- pinctrl-names := name of the pinctrl > > > Might be sufficient to just mention that the "default" state needs to be > specified in the fimd node, with reference to the pinctrl bindings > documentation, e.g. > > "The pinctrl bindings defined in ../../pinctrl/pinctrl-bindings.txt must be > used to define a pinctrl state named "default". > > See https://patchwork.kernel.org/patch/2082311. > OK. > >> +Optional Properties: >> +- samsung,power-domain := power domain property defined with a phandle > > > "a phandle to FIMD power domain node" ? OK -- Thanks and Regards Vikas Sajjan -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
 |