> -----Original Message----- > From: Thierry Reding [mailto:thierry.reding@xxxxxxxxx] > Sent: Tuesday, July 14, 2015 7:00 PM > To: Wang Jianwei-B52261 > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux- > arm-kernel@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; > airlied@xxxxxxxx; daniel.vetter@xxxxxxxx; mark.yao@xxxxxxxxxxxxxx; Wood > Scott-B07421; Wang Huan-B18965; Xiubo Li; Wang Jianwei-B52261 > Subject: Re: [PATCH v9 3/4] arm/dts/ls1021a: Add DCU dts node > > On Mon, Jul 13, 2015 at 06:51:31PM +0800, Jianwei Wang wrote: > > Add DCU node, DCU is a display controller of Freescale named 2D-ACE. > > > > Signed-off-by: Alison Wang <b18965@xxxxxxxxxxxxx> > > Signed-off-by: Xiubo Li <lixiubo@xxxxxxxxxxxxxxxxxxxx> > > Signed-off-by: Jianwei Wang <b52261@xxxxxxxxxxxxx> > > Reviewed-by: Alison Wang <alison.wang@xxxxxxxxxxxxx> > > --- > > .../devicetree/bindings/drm/fsl-dcu/fsl,dcu.txt | 20 > ++++++++++++++++++++ > > MAINTAINERS | 1 + > > arch/arm/boot/dts/ls1021a.dtsi | 10 ++++++++++ > > 3 files changed, 31 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/drm/fsl-dcu/fsl,dcu.txt > > > > diff --git a/Documentation/devicetree/bindings/drm/fsl-dcu/fsl,dcu.txt > > b/Documentation/devicetree/bindings/drm/fsl-dcu/fsl,dcu.txt > > new file mode 100644 > > index 0000000..eb61ea5 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/drm/fsl-dcu/fsl,dcu.txt > > That's not the proper location for this file. DRM is a Linux-specific > subsystem and hence shouldn't be used in anything devicetree-related. > Documentation/devicetree/bindings/video would be a better location. > > Yes, I know other DRM drivers put their binding in the drm subdirectory > but that just makes them equally wrong. I'll make a note to move these > around at some point. > Ok! > Also the binding really belongs in the same patch as the driver, or a > separate patch altogether. > Ok! > And, no need for the extra fsl-dcu subdirectory if you have only a single > document in it. > Ok > > @@ -0,0 +1,20 @@ > > +Device Tree bindings for Freescale DCU DRM Driver > > + > > +Required properties: > > +- compatible: Should be one of > > + * "fsl,ls1021a-dcu". > > + * "fsl,vf610-dcu". > > +- reg: Address and length of the register set for dcu. > > +- clocks: From common clock binding: handle to dcu clock. > > +- clock-names: From common clock binding: Shall be "dcu". > > +- panel: The phandle to panel node. > > This isn't a standard property and hence should be prefixed by the vendor > prefix. That is: "fsl,panel". > > Also the above uses a weird mix of spaces and tabs for padding. Please be > more consistent. > Ok > > + > > +Examples: > > +dcu: dcu@2ce0000 { > > + compatible = "fsl,ls1021a-dcu"; > > + reg = <0x0 0x2ce0000 0x0 0x10000>; > > + clocks = <&platform_clk 0>; > > + clock-names = "dcu"; > > + big-endian; > > This property isn't mentioned above. I know it's pretty obvious what it > does, but might still be worth briefly describing what it is. I'm also > wondering if that's not something that could be inferred from the > compatible string. > OK! Different SoCs maybe different endian. For example, on some powerpc SoC, DCU is little endian Thanks Jianwei > > + panel = <&panel>; > > +}; > > diff --git a/MAINTAINERS b/MAINTAINERS index e191ded..fffb8c9 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -3410,6 +3410,7 @@ M: Alison Wang <alison.wang@xxxxxxxxxxxxx> > > L: dri-devel@xxxxxxxxxxxxxxxxxxxxx > > S: Supported > > F: drivers/gpu/drm/fsl-dcu/ > > +F: Documentation/devicetree/bindings/drm/fsl-dcu/ > > You might want to shuffle around some of these hunks, so that this > particular hunk is included in the driver patch along with the binding and > the panel patch doesn't remove it only for it to be readded here. > > Thierry > > > F: Documentation/devicetree/bindings/panel/nec,nl4827hc19_05b.txt > > > > DRM DRIVERS FOR NVIDIA TEGRA > > diff --git a/arch/arm/boot/dts/ls1021a.dtsi > > b/arch/arm/boot/dts/ls1021a.dtsi index c70bb27..6d6e3e2 100644 > > --- a/arch/arm/boot/dts/ls1021a.dtsi > > +++ b/arch/arm/boot/dts/ls1021a.dtsi > > @@ -383,6 +383,16 @@ > > <&platform_clk 1>; > > }; > > > > + dcu: dcu@2ce0000 { > > + compatible = "fsl,ls1021a-dcu"; > > + reg = <0x0 0x2ce0000 0x0 0x10000>; > > + interrupts = <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&platform_clk 0>; > > + clock-names = "dcu"; > > + big-endian; > > + status = "disabled"; > > + }; > > + > > mdio0: mdio@2d24000 { > > compatible = "gianfar"; > > device_type = "mdio"; > > -- > > 2.1.0.27.g96db324 > > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
