All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/5] Add DRM FIMD DT support for Exynos4 DT Machines
@ 2013-02-28  9:42 Vikas Sajjan
  2013-02-28  9:42 ` [PATCH v6 1/5] ARM: dts: Add FIMD node to exynos4 Vikas Sajjan
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Vikas Sajjan @ 2013-02-28  9:42 UTC (permalink / raw)
  To: linux-samsung-soc
  Cc: kgene.kim, sachin.kamat, t.figa, sylvester.nawrocki,
	devicetree-discuss, inki.dae, jy0922.shim

This patch series adds support for DRM FIMD DT for Exynos4 DT Machines,
specifically for Exynos4412 SoC.

changes since v5:
	- renamed the fimd binding documentation file name as "samsung.fimd.txt",
	since it not only talks about exynos display controller but also about
	previous samsung display controllers.
	- rephrased an abmigious statement about the interrupt combiner in the
	fimd binding documentation as pointed out by 
	Sachin Kamat <sachin.kamat.linaro.org>

changes since v4:
	- moved the fimd binding documentation to Documentation/devicetree/bindings/video/
	as suggested by Sylwester Nawrocki <sylvester.nawrocki@gmail.com>

	- added more fimd compatiblity strings in fimd documentation as
	discussed at  https://patchwork.kernel.org/patch/2144861/ with
	Sylwester Nawrocki <sylvester.nawrocki@gmail.com> and
	Tomasz Figa <tomasz.figa@gmail.com>

	- modified compatible string for exynos4 fimd as "exynos4210-fimd"
	exynos5 fimd as "exynos5250-fimd" to stick to the rule that compatible
	value should be named after first specific SoC model in which this
	particular IP version was included as discussed at
	https://patchwork.kernel.org/patch/2144861/

	- documented more about the interrupt combiner and their order as 
	suggested by Sylwester Nawrocki <sylvester.nawrocki@gmail.com>

changes since v3:
	- rebased on
	http://git.kernel.org/?p=linux/kernel/git/kgene/linux-samsung.git;a=shortlog;h=refs/heads/for-next-next

changes since v2:
	- added alias to 'fimd@11c00000' node
		(reported by: Rahul Sharma <r.sh.open@gmail.com>)
	- removed 'lcd0_data' node as there was already a similar node lcd_data24
		(reported by: Jingoo Han <jg1.han@samsung.com>
	- replaced spaces with tabs in display-timing node

changes since v1:
	- added new patch to add FIMD DT binding Documentation
	- removed patch enabling SAMSUNG_DEV_BACKLIGHT and SAMSUNG_DEV_PMW 
	for mach-exynos4 DT
	- added 'status' property to fimd DT node

Is based on branch "for-next-next"
http://git.kernel.org/?p=linux/kernel/git/kgene/linux-samsung.git;a=shortlog;h=refs/heads/for-next-next


Sachin Kamat (1):
  ARM: dts: Add lcd pinctrl node entries for EXYNOS4412 SoC

Vikas Sajjan (4):
  ARM: dts: Add FIMD node to exynos4
  ARM: dts: Add FIMD node and display timing node to
    exynos4412-origen.dts
  ARM: dts: add FIMD AUXDATA node entry for exynos4 DT
  ARM: dts: Add FIMD DT binding Documentation

 .../devicetree/bindings/video/samsung-fimd.txt     |   54 ++++++++++++++++++++
 arch/arm/boot/dts/exynos4.dtsi                     |    7 +++
 arch/arm/boot/dts/exynos4412-origen.dts            |   22 ++++++++
 arch/arm/boot/dts/exynos4x12-pinctrl.dtsi          |   14 +++++
 arch/arm/mach-exynos/mach-exynos4-dt.c             |    2 +
 5 files changed, 99 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/samsung-fimd.txt

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v6 1/5] ARM: dts: Add FIMD node to exynos4
  2013-02-28  9:42 [PATCH v6 0/5] Add DRM FIMD DT support for Exynos4 DT Machines Vikas Sajjan
@ 2013-02-28  9:42 ` Vikas Sajjan
  2013-02-28  9:42 ` [PATCH v6 2/5] ARM: dts: Add lcd pinctrl node entries for EXYNOS4412 SoC Vikas Sajjan
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Vikas Sajjan @ 2013-02-28  9:42 UTC (permalink / raw)
  To: linux-samsung-soc
  Cc: kgene.kim, sachin.kamat, t.figa, sylvester.nawrocki,
	devicetree-discuss, inki.dae, jy0922.shim

This adds common FIMD device node for all Exynos4 SoCs.

Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
---
 arch/arm/boot/dts/exynos4.dtsi |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index 6581bb2..416a639 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -297,4 +297,11 @@
 			interrupts = <0 34 0>;
 		};
 	};
+
+	fimd: fimd@11c00000 {
+		compatible = "samsung,exynos4210-fimd";
+		interrupt-parent = <&combiner>;
+		reg = <0x11c00000 0x20000>;
+		interrupts = <11 1>, <11 0>, <11 2>;
+	};
 };
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 2/5] ARM: dts: Add lcd pinctrl node entries for EXYNOS4412 SoC
  2013-02-28  9:42 [PATCH v6 0/5] Add DRM FIMD DT support for Exynos4 DT Machines Vikas Sajjan
  2013-02-28  9:42 ` [PATCH v6 1/5] ARM: dts: Add FIMD node to exynos4 Vikas Sajjan
@ 2013-02-28  9:42 ` Vikas Sajjan
  2013-02-28  9:42 ` [PATCH v6 3/5] ARM: dts: Add FIMD node and display timing node to exynos4412-origen.dts Vikas Sajjan
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Vikas Sajjan @ 2013-02-28  9:42 UTC (permalink / raw)
  To: linux-samsung-soc
  Cc: kgene.kim, sachin.kamat, t.figa, sylvester.nawrocki,
	devicetree-discuss, inki.dae, jy0922.shim

From: Sachin Kamat <sachin.kamat@linaro.org>

This patch adds the lcd panel related picntrl nodes for Exynos4412 SoC

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
---
 arch/arm/boot/dts/exynos4x12-pinctrl.dtsi |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi b/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi
index 099cec7..a59d69c 100644
--- a/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi
+++ b/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi
@@ -354,6 +354,20 @@
 			samsung,pin-drv = <0>;
 		};
 
+		lcd_sync: lcd-sync {
+			samsung,pins = "gpf0-0", "gpf0-1";
+			samsung,pin-function = <2>;
+			samsung,pin-pud = <0>;
+			samsung,pin-drv = <0>;
+		};
+
+		lcd_en: lcd-en {
+			samsung,pins = "gpf0-3";
+			samsung,pin-function = <2>;
+			samsung,pin-pud = <0>;
+			samsung,pin-drv = <0>;
+		};
+
 		lcd_clk: lcd-clk {
 			samsung,pins = "gpf0-0", "gpf0-1", "gpf0-2", "gpf0-3";
 			samsung,pin-function = <2>;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 3/5] ARM: dts: Add FIMD node and display timing node to exynos4412-origen.dts
  2013-02-28  9:42 [PATCH v6 0/5] Add DRM FIMD DT support for Exynos4 DT Machines Vikas Sajjan
  2013-02-28  9:42 ` [PATCH v6 1/5] ARM: dts: Add FIMD node to exynos4 Vikas Sajjan
  2013-02-28  9:42 ` [PATCH v6 2/5] ARM: dts: Add lcd pinctrl node entries for EXYNOS4412 SoC Vikas Sajjan
@ 2013-02-28  9:42 ` Vikas Sajjan
  2013-02-28  9:42 ` [PATCH v6 4/5] ARM: dts: Add FIMD AUXDATA node entry for exynos4 DT Vikas Sajjan
  2013-02-28  9:42 ` [PATCH v6 5/5] ARM: dts: Add FIMD DT binding Documentation Vikas Sajjan
  4 siblings, 0 replies; 13+ messages in thread
From: Vikas Sajjan @ 2013-02-28  9:42 UTC (permalink / raw)
  To: linux-samsung-soc
  Cc: kgene.kim, sachin.kamat, t.figa, sylvester.nawrocki,
	devicetree-discuss, inki.dae, jy0922.shim

Adds FIMD DT support to Origen quad board

Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
---
 arch/arm/boot/dts/exynos4412-origen.dts |   22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4412-origen.dts b/arch/arm/boot/dts/exynos4412-origen.dts
index 31e14c4..a6fd41f 100644
--- a/arch/arm/boot/dts/exynos4412-origen.dts
+++ b/arch/arm/boot/dts/exynos4412-origen.dts
@@ -70,6 +70,28 @@
 		status = "okay";
 	};
 
+       fimd@11c00000 {
+		samsung,power-domain = <&pd_lcd0>;
+		pinctrl-0 = <&lcd_sync &lcd_clk &lcd_en &lcd_data24 &pwm1_out>;
+		pinctrl-names = "default";
+		status = "okay";
+	};
+
+	display-timings {
+		native-mode = <&timing0>;
+		timing0: timing@0 {
+			clock-frequency = <50000>;
+			hactive = <1024>;
+			vactive = <600>;
+			hfront-porch = <64>;
+			hback-porch = <16>;
+			hsync-len = <48>;
+			vback-porch = <64>;
+			vfront-porch = <16>;
+			vsync-len = <3>;
+		};
+	};
+
 	serial@13800000 {
 		status = "okay";
 	};
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 4/5] ARM: dts: Add FIMD AUXDATA node entry for exynos4 DT
  2013-02-28  9:42 [PATCH v6 0/5] Add DRM FIMD DT support for Exynos4 DT Machines Vikas Sajjan
                   ` (2 preceding siblings ...)
  2013-02-28  9:42 ` [PATCH v6 3/5] ARM: dts: Add FIMD node and display timing node to exynos4412-origen.dts Vikas Sajjan
@ 2013-02-28  9:42 ` Vikas Sajjan
  2013-02-28  9:42 ` [PATCH v6 5/5] ARM: dts: Add FIMD DT binding Documentation Vikas Sajjan
  4 siblings, 0 replies; 13+ messages in thread
From: Vikas Sajjan @ 2013-02-28  9:42 UTC (permalink / raw)
  To: linux-samsung-soc
  Cc: kgene.kim, sachin.kamat, t.figa, sylvester.nawrocki,
	devicetree-discuss, inki.dae, jy0922.shim

Adds the FIMD AUXDATA node

Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
---
 arch/arm/mach-exynos/mach-exynos4-dt.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-exynos/mach-exynos4-dt.c b/arch/arm/mach-exynos/mach-exynos4-dt.c
index b68115a..b32a929 100644
--- a/arch/arm/mach-exynos/mach-exynos4-dt.c
+++ b/arch/arm/mach-exynos/mach-exynos4-dt.c
@@ -84,6 +84,8 @@ static const struct of_dev_auxdata exynos4_auxdata_lookup[] __initconst = {
 	OF_DEV_AUXDATA("samsung,exynos4210-tmu", EXYNOS4_PA_TMU,
 				"exynos-tmu", NULL),
 	OF_DEV_AUXDATA("samsung,mfc-v5", 0x13400000, "s5p-mfc", NULL),
+	OF_DEV_AUXDATA("samsung,exynos4210-fimd", EXYNOS4_PA_FIMD0,
+			"exynos4-fb.0", NULL),
 	{},
 };
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 5/5] ARM: dts: Add FIMD DT binding Documentation
  2013-02-28  9:42 [PATCH v6 0/5] Add DRM FIMD DT support for Exynos4 DT Machines Vikas Sajjan
                   ` (3 preceding siblings ...)
  2013-02-28  9:42 ` [PATCH v6 4/5] ARM: dts: Add FIMD AUXDATA node entry for exynos4 DT Vikas Sajjan
@ 2013-02-28  9:42 ` Vikas Sajjan
  2013-03-03 19:11   ` Sylwester Nawrocki
  4 siblings, 1 reply; 13+ messages in thread
From: Vikas Sajjan @ 2013-02-28  9:42 UTC (permalink / raw)
  To: linux-samsung-soc
  Cc: kgene.kim, sachin.kamat, t.figa, sylvester.nawrocki,
	devicetree-discuss, inki.dae, jy0922.shim

Adds FIMD DT binding documentation both Samsung SoC and Board, with an example

Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
---
 .../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
+
+- interrupt-parent := reference to the interrupt combiner node with phandle
+
+- 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>;
+
+- pinctrl := property defining the pinctrl configurations with a phandle
+
+- pinctrl-names := name of the pinctrl
+
+Optional Properties:
+- samsung,power-domain := power domain property defined with a phandle
+
+Example:
+
+SoC specific DT Entry:
+
+	fimd@11c00000 {
+		compatible = "samsung,exynos4210-fimd";
+		interrupt-parent = <&combiner>;
+		reg = <0x11c00000 0x20000>;
+		interrupts = <11 1>, <11 0>, <11 2>;
+	};
+
+Board specific DT Entry:
+
+	fimd@11c00000 {
+		samsung,power-domain = <&pd_lcd0>;
+		pinctrl-0 = <&lcd_sync &lcd_clk &lcd_en &lcd0_data &pwm1_out>;
+		pinctrl-names = "default";
+		status = "okay";
+	};
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 5/5] ARM: dts: Add FIMD DT binding Documentation
  2013-02-28  9:42 ` [PATCH v6 5/5] ARM: dts: Add FIMD DT binding Documentation Vikas Sajjan
@ 2013-03-03 19:11   ` Sylwester Nawrocki
  2013-03-03 20:03     ` Sylwester Nawrocki
  2013-03-06 10:18     ` Vikas Sajjan
  0 siblings, 2 replies; 13+ messages in thread
From: Sylwester Nawrocki @ 2013-03-03 19:11 UTC (permalink / raw)
  To: Vikas Sajjan
  Cc: linux-samsung-soc, kgene.kim, sachin.kamat, t.figa,
	sylvester.nawrocki, devicetree-discuss, inki.dae, jy0922.shim

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@linaro.org>
> ---
>   .../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.

> +
> +- 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;

?

And could we just use a colon instead of ":=", to keep it more consistent
with other Samsung SoC DT bindings documentation ?

> +
> +- 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;

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.

> +Optional Properties:
> +- samsung,power-domain := power domain property defined with a phandle

"a phandle to FIMD power domain node" ?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 5/5] ARM: dts: Add FIMD DT binding Documentation
  2013-03-03 19:11   ` Sylwester Nawrocki
@ 2013-03-03 20:03     ` Sylwester Nawrocki
  2013-03-06 10:18     ` Vikas Sajjan
  1 sibling, 0 replies; 13+ messages in thread
From: Sylwester Nawrocki @ 2013-03-03 20:03 UTC (permalink / raw)
  To: Vikas Sajjan
  Cc: linux-samsung-soc, kgene.kim, sachin.kamat, t.figa,
	devicetree-discuss, inki.dae, jy0922.shim

On 03/03/2013 08:11 PM, Sylwester Nawrocki wrote:
> I have a suggestion here. What about using reg-names property to make this

Oops, this should read "interrupt-names", not "reg-names".

> 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;

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 5/5] ARM: dts: Add FIMD DT binding Documentation
  2013-03-03 19:11   ` Sylwester Nawrocki
  2013-03-03 20:03     ` Sylwester Nawrocki
@ 2013-03-06 10:18     ` Vikas Sajjan
  2013-03-06 21:13       ` Sylwester Nawrocki
  1 sibling, 1 reply; 13+ messages in thread
From: Vikas Sajjan @ 2013-03-06 10:18 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: linux-samsung-soc, kgene.kim, sachin.kamat, t.figa,
	devicetree-discuss, inki.dae, jy0922.shim, sunil joshi

Hi,

On 4 March 2013 00:41, Sylwester Nawrocki <sylvester.nawrocki@gmail.com> 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@linaro.org>
>> ---
>>   .../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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 5/5] ARM: dts: Add FIMD DT binding Documentation
  2013-03-06 10:18     ` Vikas Sajjan
@ 2013-03-06 21:13       ` Sylwester Nawrocki
  2013-03-06 21:20         ` Sylwester Nawrocki
  2013-03-07  6:50         ` Vikas Sajjan
  0 siblings, 2 replies; 13+ messages in thread
From: Sylwester Nawrocki @ 2013-03-06 21:13 UTC (permalink / raw)
  To: Vikas Sajjan
  Cc: linux-samsung-soc, kgene.kim, sachin.kamat, t.figa,
	devicetree-discuss, inki.dae, jy0922.shim, sunil joshi

On 03/06/2013 11:18 AM, Vikas Sajjan wrote:
> Good idea, but i am just wondering is it a good idea to modify the
> fimd driver ?

I think it is. But it depends what code quality you aim for.

Is there any issue in making a change as below ? ;)

Of course it's all up to you, I've just suggested what I thought was
an improvement to the current situation.

diff --git a/arch/arm/plat-samsung/devs.c b/arch/arm/plat-samsung/devs.c
index 51afedd..4180247 100644
--- a/arch/arm/plat-samsung/devs.c
+++ b/arch/arm/plat-samsung/devs.c
@@ -160,9 +160,9 @@ struct platform_device samsung_asoc_idma = {
  #ifdef CONFIG_S3C_DEV_FB
  static struct resource s3c_fb_resource[] = {
         [0] = DEFINE_RES_MEM(S3C_PA_FB, SZ_16K),
-       [1] = DEFINE_RES_IRQ(IRQ_LCD_VSYNC),
-       [2] = DEFINE_RES_IRQ(IRQ_LCD_FIFO),
-       [3] = DEFINE_RES_IRQ(IRQ_LCD_SYSTEM),
+       [1] = DEFINE_RES_IRQ_NAMED(IRQ_LCD_VSYNC, "vsync"),
+       [2] = DEFINE_RES_IRQ_NAMED(IRQ_LCD_FIFO, "fifo"),
+       [3] = DEFINE_RES_IRQ_NAMED(IRQ_LCD_SYSTEM, "system"),
  };

  struct platform_device s3c_device_fb = {
@@ -305,9 +305,9 @@ struct platform_device s5p_device_jpeg = {
  #ifdef CONFIG_S5P_DEV_FIMD0
  static struct resource s5p_fimd0_resource[] = {
         [0] = DEFINE_RES_MEM(S5P_PA_FIMD0, SZ_32K),
-       [1] = DEFINE_RES_IRQ(IRQ_FIMD0_VSYNC),
-       [2] = DEFINE_RES_IRQ(IRQ_FIMD0_FIFO),
-       [3] = DEFINE_RES_IRQ(IRQ_FIMD0_SYSTEM),
+       [1] = DEFINE_RES_IRQ_NAMED(IRQ_FIMD0_VSYNC, "vsync"),
+       [2] = DEFINE_RES_IRQ(IRQ_FIMD0_FIFO, "fifo"),
+       [3] = DEFINE_RES_IRQ(IRQ_FIMD0_SYSTEM, "system"),
  };

  struct platform_device s5p_device_fimd0 = {
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c 
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 36493ce..5efb4c7 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -917,7 +917,7 @@ static int fimd_probe(struct platform_device *pdev)
         if (IS_ERR(ctx->regs))
                 return PTR_ERR(ctx->regs);

-       res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+       res = platform_get_resource_byname(pdev, IORESOURCE_IRQ, "vsync");
         if (!res) {
                 dev_err(dev, "irq request failed.\n");
                 return -ENXIO;
diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index 968a625..43af0108 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -1427,7 +1427,7 @@ static int s3c_fb_probe(struct platform_device *pdev)
                 goto err_lcd_clk;
         }

-       res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+       res = platform_get_resource_byname(pdev, IORESOURCE_IRQ, "vsync");
         if (!res) {
                 dev_err(dev, "failed to acquire irq resource\n");
                 ret = -ENOENT;

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 5/5] ARM: dts: Add FIMD DT binding Documentation
  2013-03-06 21:13       ` Sylwester Nawrocki
@ 2013-03-06 21:20         ` Sylwester Nawrocki
  2013-03-06 23:56           ` Kukjin Kim
  2013-03-07  6:50         ` Vikas Sajjan
  1 sibling, 1 reply; 13+ messages in thread
From: Sylwester Nawrocki @ 2013-03-06 21:20 UTC (permalink / raw)
  To: Vikas Sajjan
  Cc: linux-samsung-soc, kgene.kim, sachin.kamat, t.figa,
	devicetree-discuss, inki.dae, jy0922.shim, sunil joshi

On 03/06/2013 10:13 PM, Sylwester Nawrocki wrote:
> @@ -305,9 +305,9 @@ struct platform_device s5p_device_jpeg = {
>   #ifdef CONFIG_S5P_DEV_FIMD0
>   static struct resource s5p_fimd0_resource[] = {
>          [0] = DEFINE_RES_MEM(S5P_PA_FIMD0, SZ_32K),
> -       [1] = DEFINE_RES_IRQ(IRQ_FIMD0_VSYNC),
> -       [2] = DEFINE_RES_IRQ(IRQ_FIMD0_FIFO),
> -       [3] = DEFINE_RES_IRQ(IRQ_FIMD0_SYSTEM),
> +       [1] = DEFINE_RES_IRQ_NAMED(IRQ_FIMD0_VSYNC, "vsync"),
> +       [2] = DEFINE_RES_IRQ(IRQ_FIMD0_FIFO, "fifo"),
> +       [3] = DEFINE_RES_IRQ(IRQ_FIMD0_SYSTEM, "system"),
>   };

Oops, something missing here. Of course should be:

@@ -160,9 +160,9 @@ struct platform_device samsung_asoc_idma = {
  #ifdef CONFIG_S3C_DEV_FB
  static struct resource s3c_fb_resource[] = {
         [0] = DEFINE_RES_MEM(S3C_PA_FB, SZ_16K),
-       [1] = DEFINE_RES_IRQ(IRQ_LCD_VSYNC),
-       [2] = DEFINE_RES_IRQ(IRQ_LCD_FIFO),
-       [3] = DEFINE_RES_IRQ(IRQ_LCD_SYSTEM),
+       [1] = DEFINE_RES_IRQ_NAMED(IRQ_LCD_VSYNC, "vsync"),
+       [2] = DEFINE_RES_IRQ_NAMED(IRQ_LCD_FIFO, "fifo"),
+       [3] = DEFINE_RES_IRQ_NAMED(IRQ_LCD_SYSTEM, "system"),

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v6 5/5] ARM: dts: Add FIMD DT binding Documentation
  2013-03-06 21:20         ` Sylwester Nawrocki
@ 2013-03-06 23:56           ` Kukjin Kim
  0 siblings, 0 replies; 13+ messages in thread
From: Kukjin Kim @ 2013-03-06 23:56 UTC (permalink / raw)
  To: 'Sylwester Nawrocki', 'Vikas Sajjan'
  Cc: linux-samsung-soc, sachin.kamat, t.figa, devicetree-discuss,
	inki.dae, jy0922.shim, 'sunil joshi'

Sylwester Nawrocki wrote:
> 
> On 03/06/2013 10:13 PM, Sylwester Nawrocki wrote:
> > @@ -305,9 +305,9 @@ struct platform_device s5p_device_jpeg = {
> >   #ifdef CONFIG_S5P_DEV_FIMD0
> >   static struct resource s5p_fimd0_resource[] = {
> >          [0] = DEFINE_RES_MEM(S5P_PA_FIMD0, SZ_32K),
> > -       [1] = DEFINE_RES_IRQ(IRQ_FIMD0_VSYNC),
> > -       [2] = DEFINE_RES_IRQ(IRQ_FIMD0_FIFO),
> > -       [3] = DEFINE_RES_IRQ(IRQ_FIMD0_SYSTEM),
> > +       [1] = DEFINE_RES_IRQ_NAMED(IRQ_FIMD0_VSYNC, "vsync"),
> > +       [2] = DEFINE_RES_IRQ(IRQ_FIMD0_FIFO, "fifo"),
> > +       [3] = DEFINE_RES_IRQ(IRQ_FIMD0_SYSTEM, "system"),
> >   };
> 
> Oops, something missing here. Of course should be:
> 
> @@ -160,9 +160,9 @@ struct platform_device samsung_asoc_idma = {
>   #ifdef CONFIG_S3C_DEV_FB
>   static struct resource s3c_fb_resource[] = {
>          [0] = DEFINE_RES_MEM(S3C_PA_FB, SZ_16K),
> -       [1] = DEFINE_RES_IRQ(IRQ_LCD_VSYNC),
> -       [2] = DEFINE_RES_IRQ(IRQ_LCD_FIFO),
> -       [3] = DEFINE_RES_IRQ(IRQ_LCD_SYSTEM),
> +       [1] = DEFINE_RES_IRQ_NAMED(IRQ_LCD_VSYNC, "vsync"),
> +       [2] = DEFINE_RES_IRQ_NAMED(IRQ_LCD_FIFO, "fifo"),
> +       [3] = DEFINE_RES_IRQ_NAMED(IRQ_LCD_SYSTEM, "system"),
   ^^^^^^^

BTW, can you use 'tab' instead of white space at mark '^'

- Kukjin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 5/5] ARM: dts: Add FIMD DT binding Documentation
  2013-03-06 21:13       ` Sylwester Nawrocki
  2013-03-06 21:20         ` Sylwester Nawrocki
@ 2013-03-07  6:50         ` Vikas Sajjan
  1 sibling, 0 replies; 13+ messages in thread
From: Vikas Sajjan @ 2013-03-07  6:50 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: linux-samsung-soc, kgene.kim, sachin.kamat, t.figa,
	devicetree-discuss, inki.dae, jy0922.shim, sunil joshi

Hi,

On 7 March 2013 02:43, Sylwester Nawrocki <sylvester.nawrocki@gmail.com> wrote:
> On 03/06/2013 11:18 AM, Vikas Sajjan wrote:
>>
>> Good idea, but i am just wondering is it a good idea to modify the
>> fimd driver ?
>
>
> I think it is. But it depends what code quality you aim for.
>
> Is there any issue in making a change as below ? ;)

Certainly no issue :-)  Will check this idea both on exyno4 and
exynos5 and shall update.
>
> Of course it's all up to you, I've just suggested what I thought was
> an improvement to the current situation.
>
> diff --git a/arch/arm/plat-samsung/devs.c b/arch/arm/plat-samsung/devs.c
> index 51afedd..4180247 100644
> --- a/arch/arm/plat-samsung/devs.c
> +++ b/arch/arm/plat-samsung/devs.c
> @@ -160,9 +160,9 @@ struct platform_device samsung_asoc_idma = {
>  #ifdef CONFIG_S3C_DEV_FB
>  static struct resource s3c_fb_resource[] = {
>         [0] = DEFINE_RES_MEM(S3C_PA_FB, SZ_16K),
> -       [1] = DEFINE_RES_IRQ(IRQ_LCD_VSYNC),
> -       [2] = DEFINE_RES_IRQ(IRQ_LCD_FIFO),
> -       [3] = DEFINE_RES_IRQ(IRQ_LCD_SYSTEM),
> +       [1] = DEFINE_RES_IRQ_NAMED(IRQ_LCD_VSYNC, "vsync"),
> +       [2] = DEFINE_RES_IRQ_NAMED(IRQ_LCD_FIFO, "fifo"),
> +       [3] = DEFINE_RES_IRQ_NAMED(IRQ_LCD_SYSTEM, "system"),
>  };
>
>  struct platform_device s3c_device_fb = {
> @@ -305,9 +305,9 @@ struct platform_device s5p_device_jpeg = {
>  #ifdef CONFIG_S5P_DEV_FIMD0
>  static struct resource s5p_fimd0_resource[] = {
>         [0] = DEFINE_RES_MEM(S5P_PA_FIMD0, SZ_32K),
> -       [1] = DEFINE_RES_IRQ(IRQ_FIMD0_VSYNC),
> -       [2] = DEFINE_RES_IRQ(IRQ_FIMD0_FIFO),
> -       [3] = DEFINE_RES_IRQ(IRQ_FIMD0_SYSTEM),
> +       [1] = DEFINE_RES_IRQ_NAMED(IRQ_FIMD0_VSYNC, "vsync"),
> +       [2] = DEFINE_RES_IRQ(IRQ_FIMD0_FIFO, "fifo"),
> +       [3] = DEFINE_RES_IRQ(IRQ_FIMD0_SYSTEM, "system"),
>  };
>
>  struct platform_device s5p_device_fimd0 = {
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 36493ce..5efb4c7 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -917,7 +917,7 @@ static int fimd_probe(struct platform_device *pdev)
>         if (IS_ERR(ctx->regs))
>                 return PTR_ERR(ctx->regs);
>
> -       res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +       res = platform_get_resource_byname(pdev, IORESOURCE_IRQ, "vsync");
>         if (!res) {
>                 dev_err(dev, "irq request failed.\n");
>                 return -ENXIO;
> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> index 968a625..43af0108 100644
> --- a/drivers/video/s3c-fb.c
> +++ b/drivers/video/s3c-fb.c
> @@ -1427,7 +1427,7 @@ static int s3c_fb_probe(struct platform_device *pdev)
>                 goto err_lcd_clk;
>         }
>
> -       res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +       res = platform_get_resource_byname(pdev, IORESOURCE_IRQ, "vsync");
>         if (!res) {
>                 dev_err(dev, "failed to acquire irq resource\n");
>                 ret = -ENOENT;



-- 
Thanks and Regards
 Vikas Sajjan

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2013-03-07  6:50 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-28  9:42 [PATCH v6 0/5] Add DRM FIMD DT support for Exynos4 DT Machines Vikas Sajjan
2013-02-28  9:42 ` [PATCH v6 1/5] ARM: dts: Add FIMD node to exynos4 Vikas Sajjan
2013-02-28  9:42 ` [PATCH v6 2/5] ARM: dts: Add lcd pinctrl node entries for EXYNOS4412 SoC Vikas Sajjan
2013-02-28  9:42 ` [PATCH v6 3/5] ARM: dts: Add FIMD node and display timing node to exynos4412-origen.dts Vikas Sajjan
2013-02-28  9:42 ` [PATCH v6 4/5] ARM: dts: Add FIMD AUXDATA node entry for exynos4 DT Vikas Sajjan
2013-02-28  9:42 ` [PATCH v6 5/5] ARM: dts: Add FIMD DT binding Documentation Vikas Sajjan
2013-03-03 19:11   ` Sylwester Nawrocki
2013-03-03 20:03     ` Sylwester Nawrocki
2013-03-06 10:18     ` Vikas Sajjan
2013-03-06 21:13       ` Sylwester Nawrocki
2013-03-06 21:20         ` Sylwester Nawrocki
2013-03-06 23:56           ` Kukjin Kim
2013-03-07  6:50         ` Vikas Sajjan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.