Philipp Zabel <philipp.zabel@xxxxxxxxx> writes: > On Sat, Oct 3, 2015 at 6:11 PM, Robert Jarzmik <robert.jarzmik@xxxxxxx> wrote: >> Add documentation for the PXA frambuffer devicetree binding. >> >> Signed-off-by: Robert Jarzmik <robert.jarzmik@xxxxxxx> >> Cc: Jean-Christophe Plagniol-Villard <plagnioj@xxxxxxxxxxxx> >> Cc: Tomi Valkeinen <tomi.valkeinen@xxxxxx> >> Cc: linux-fbdev@xxxxxxxxxxxxxxx >> >> --- >> .../devicetree/bindings/video/marvell,pxafb.txt | 75 ++++++++++++++++++++++ >> 1 file changed, 75 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/video/marvell,pxafb.txt >> >> diff --git a/Documentation/devicetree/bindings/video/marvell,pxafb.txt b/Documentation/devicetree/bindings/video/marvell,pxafb.txt >> new file mode 100644 >> index 000000000000..489055bf3c57 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/video/marvell,pxafb.txt >> @@ -0,0 +1,75 @@ >> +PXA LCDC Framebuffer >> +----------------------------------------------------- >> + >> +Required properties: >> +- compatible : >> + "marvell,pxa2xx-fb", > > Should be "marvell,pxa2xx-lcd-controller", "marvell,pxa2xx-lcdc" or > something like this. Whichever you see fit. > >> +- reg : Should contain 1 register ranges(address and length). >> + Can contain an additional register range(address and length) >> + for fixed framebuffer memory. Useful for dedicated memories. >> +- interrupts : framebuffer controller interrupt >> +- display: a phandle pointing to the display node >> + >> +Required nodes: >> +- display: a display node is required to initialize the lcd panel >> + This should be in the board dts. > > I'd prefer to use an of-graph link to a panel node with a proper > compatible value for the panel, instead of this custom display > property. > That way, if somebody ever decides convert the fbdev driver to a drm > driver, we don't have to change the device tree and can directly use > drm_panel. Ok, if you give me an example it would be easier for me. >> +- default-mode: a videomode within the display with timing parameters >> + as specified below. >> +- bits-per-pixel: pixel data bus width of the LCD panel > > Would bus-width be better here? bus-width yes, but I think I should remove this property, and only keep the one in the panel/display. >> +Optional properties: >> +- lcd-supply: Regulator for LCD supply voltage. > > How does this differ from the regulator below? Ah yes, good point. In the end I couldn't decide which one was the correct one ... My feeling is that it's the display's one, as hardware wise the power is necessary for the display, not the framebuffer. > >> +- enable-transparency-bit: if framebuffer colorspace reserves a bit for >> + transparency > > That doesn't belong in the device tree. > >> +- enable-greyscale-cmap: true if palette is a grayscale based instead of color > > I suspect this doesn't belong in the device tree either. Does this > specify the pixel format of the memory framebuffer? Yes, both these values specify the pixel format. I was thinking this was a hardware capability of the IP, I was wrong, just cross-checked. I'll remove these 2 properties. >> + enable-transparency-bit = <0>; >> + enable-greyscale-cmap = <0>; >> + #address-cells = <1>; >> + #size-cells = <1>; > > What are the #address/size-cells needed for? Copy-paste from another binding, atmel's I think. Poor leftover obviously. >> + }; >> + >> +PXA LCDC Display >> +----------------------------------------------------- >> +Required properties (as per of_videomode_helper): >> + - lcd-type: either "mono-stn", "mono-dstn", "color-stn", "color-dstn", >> + "color-tft", "smart-panel" >> + - bits-per-pixel: LCD data bus width > > This is already found in the lcd controller node above. I think the bus-width should be here. It represents the number of data lines between the SoC and the panel. Cheers. -- Robert -- 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