| Message ID | 1436784692-40560-3-git-send-email-jianwei.wang@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@freescale.com> > Signed-off-by: Xiubo Li <lixiubo@cmss.chinamobile.com> > Signed-off-by: Jianwei Wang <b52261@freescale.com> > Reviewed-by: Alison Wang <alison.wang@freescale.com> > --- > .../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. Also the binding really belongs in the same patch as the driver, or a separate patch altogether. And, no need for the extra fsl-dcu subdirectory if you have only a single document in it. > @@ -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. > + > +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. > + 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@freescale.com> > L: dri-devel@lists.freedesktop.org > 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 >
> -----Original Message----- > From: Thierry Reding [mailto:thierry.reding@gmail.com] > Sent: Tuesday, July 14, 2015 7:00 PM > To: Wang Jianwei-B52261 > Cc: dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; linux- > arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; > airlied@linux.ie; daniel.vetter@ffwll.ch; mark.yao@rock-chips.com; 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@freescale.com> > > Signed-off-by: Xiubo Li <lixiubo@cmss.chinamobile.com> > > Signed-off-by: Jianwei Wang <b52261@freescale.com> > > Reviewed-by: Alison Wang <alison.wang@freescale.com> > > --- > > .../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@freescale.com> > > L: dri-devel@lists.freedesktop.org > > 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 > >
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 @@ -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. + +Examples: +dcu: dcu@2ce0000 { + compatible = "fsl,ls1021a-dcu"; + reg = <0x0 0x2ce0000 0x0 0x10000>; + clocks = <&platform_clk 0>; + clock-names = "dcu"; + big-endian; + 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@freescale.com> L: dri-devel@lists.freedesktop.org S: Supported F: drivers/gpu/drm/fsl-dcu/ +F: Documentation/devicetree/bindings/drm/fsl-dcu/ 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";