devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] Add device tree and clock support for Gscaler.
@ 2012-07-20 13:08 Shaik Ameer Basha
  2012-07-20 13:08 ` [PATCH v3 1/2] ARM: EXYNOS: Add " Shaik Ameer Basha
  2012-07-20 13:08 ` [PATCH v3 2/2] ARM: EXYNOS: Add Gscaler device from DT Shaik Ameer Basha
  0 siblings, 2 replies; 18+ messages in thread
From: Shaik Ameer Basha @ 2012-07-20 13:08 UTC (permalink / raw)
  To: linux-samsung-soc, devicetree-discuss
  Cc: kgene.kim, sy0816.kang, olofj, thomas.ab, sylvester.nawrocki,
	sachin.kamat, joshi, shaik.samsung, shaik.ameer

This patch series adds clock support for Gscaler and device node
 entries for Gscaler on exynos5.

This patch is based on Kukjin Kim's (linux-samsung) for-next branch.
    https://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git

changes since v2:
- Addressed review comments from Sylwester Nawrocki
    http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg11372.html
- Addressed review comments from Kukjin Kim and Sunyoung Kang
    http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg11377.html
    http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg11429.html

Shaik Ameer Basha (2):
  ARM: EXYNOS: Add clock support for Gscaler
  ARM: EXYNOS: Add Gscaler device from DT

 .../devicetree/bindings/media/exynos5-gsc.txt      |   32 ++++++
 arch/arm/boot/dts/exynos5250.dtsi                  |   28 ++++++
 arch/arm/mach-exynos/clock-exynos5.c               |  100 ++++++++++++++++++++
 arch/arm/mach-exynos/include/mach/map.h            |    3 +
 arch/arm/mach-exynos/mach-exynos5-dt.c             |    8 ++
 5 files changed, 171 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/exynos5-gsc.txt

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

* [PATCH v3 1/2] ARM: EXYNOS: Add clock support for Gscaler
  2012-07-20 13:08 [PATCH v3 0/2] Add device tree and clock support for Gscaler Shaik Ameer Basha
@ 2012-07-20 13:08 ` Shaik Ameer Basha
  2012-08-01  6:24   ` Kukjin Kim
  2012-07-20 13:08 ` [PATCH v3 2/2] ARM: EXYNOS: Add Gscaler device from DT Shaik Ameer Basha
  1 sibling, 1 reply; 18+ messages in thread
From: Shaik Ameer Basha @ 2012-07-20 13:08 UTC (permalink / raw)
  To: linux-samsung-soc, devicetree-discuss
  Cc: kgene.kim, sy0816.kang, olofj, thomas.ab, sylvester.nawrocki,
	sachin.kamat, joshi, shaik.samsung, shaik.ameer

Add required clock support for Gscaler for exynos5

Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
Signed-off-by: Leela Krishna Amudala <l.krishna@samsung.com>
Signed-off-by: Prathyush K <prathyush.k@samsung.com>
Signed-off-by: Shaik Ameer Basha <shaik.ameer@samsung.com>
---
 arch/arm/mach-exynos/clock-exynos5.c |  100 ++++++++++++++++++++++++++++++++++
 1 files changed, 100 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-exynos/clock-exynos5.c b/arch/arm/mach-exynos/clock-exynos5.c
index 774533c..49a76b1 100644
--- a/arch/arm/mach-exynos/clock-exynos5.c
+++ b/arch/arm/mach-exynos/clock-exynos5.c
@@ -552,6 +552,81 @@ static struct clksrc_clk exynos5_clk_aclk_66 = {
 	.reg_div = { .reg = EXYNOS5_CLKDIV_TOP0, .shift = 0, .size = 3 },
 };
 
+/* for aclk_300_gscl_mid */
+static struct clksrc_clk exynos5_clk_mout_aclk_300_gscl_mid = {
+	.clk = {
+		.name		= "mout_aclk_300_gscl_mid",
+	},
+	.sources = &exynos5_clkset_aclk,
+	.reg_src = { .reg = EXYNOS5_CLKSRC_TOP0, .shift = 24, .size = 1 },
+};
+
+/* for aclk_300_gscl_mid1 */
+static struct clk *exynos5_clkset_aclk_300_gscl_mid1_list[] = {
+	[0] = &exynos5_clk_sclk_vpll.clk,
+	[1] = &exynos5_clk_mout_cpll.clk,
+};
+
+static struct clksrc_sources exynos5_clkset_aclk_300_gscl_mid1 = {
+	.sources	= exynos5_clkset_aclk_300_gscl_mid1_list,
+	.nr_sources	= ARRAY_SIZE(exynos5_clkset_aclk_300_gscl_mid1_list),
+};
+
+
+static struct clksrc_clk exynos5_clk_mout_aclk_300_gscl_mid1 = {
+	.clk    = {
+		.name		= "mout_aclk_300_gscl_mid1",
+	},
+	.sources = &exynos5_clkset_aclk_300_gscl_mid1,
+	.reg_src = { .reg = EXYNOS5_CLKSRC_TOP1, .shift = 12, .size = 1 },
+};
+
+/* for aclk_300_gscl */
+static struct clk *exynos5_clkset_aclk_300_gscl_list[] = {
+	[0] = &exynos5_clk_mout_aclk_300_gscl_mid.clk,
+	[1] = &exynos5_clk_mout_aclk_300_gscl_mid1.clk,
+};
+
+static struct clksrc_sources exynos5_clkset_aclk_300_gscl = {
+	.sources	= exynos5_clkset_aclk_300_gscl_list,
+	.nr_sources	= ARRAY_SIZE(exynos5_clkset_aclk_300_gscl_list),
+};
+
+static struct clksrc_clk exynos5_clk_mout_aclk_300_gscl = {
+	.clk    = {
+		.name		= "mout_aclk_300_gscl",
+	},
+	.sources = &exynos5_clkset_aclk_300_gscl,
+	.reg_src = { .reg = EXYNOS5_CLKSRC_TOP0, .shift = 25, .size = 1 },
+};
+
+static struct clksrc_clk exynos5_clk_dout_aclk_300_gscl = {
+	.clk    = {
+		.name		= "dout_aclk_300_gscl",
+		.parent		= &exynos5_clk_mout_aclk_300_gscl.clk,
+	},
+	.reg_div = { .reg = EXYNOS5_CLKDIV_TOP1, .shift = 12, .size = 3 },
+};
+
+/* for aclk_300_gscl_sub mux */
+static struct clk *exynos5_clk_src_gscl_300_list[] = {
+	[0] = &clk_ext_xtal_mux,
+	[1] = &exynos5_clk_dout_aclk_300_gscl.clk,
+};
+
+static struct clksrc_sources exynos5_clk_src_gscl_300 = {
+	.sources	= exynos5_clk_src_gscl_300_list,
+	.nr_sources	= ARRAY_SIZE(exynos5_clk_src_gscl_300_list),
+};
+
+static struct clksrc_clk exynos5_clk_aclk_300_gscl = {
+	.clk    = {
+		.name		= "aclk_300_gscl",
+	},
+	.sources = &exynos5_clk_src_gscl_300,
+	.reg_src = { .reg = EXYNOS5_CLKSRC_TOP3, .shift = 10, .size = 1 },
+};
+
 static struct clk exynos5_init_clocks_off[] = {
 	{
 		.name		= "timers",
@@ -764,6 +839,26 @@ static struct clk exynos5_init_clocks_off[] = {
 		.enable		= exynos5_clk_ip_peric_ctrl,
 		.ctrlbit	= (1 << 18),
 	}, {
+		.name		= "gscl",
+		.devname	= "exynos-gsc.0",
+		.enable		= exynos5_clk_ip_gscl_ctrl,
+		.ctrlbit	= (1 << 0),
+	}, {
+		.name		= "gscl",
+		.devname	= "exynos-gsc.1",
+		.enable		= exynos5_clk_ip_gscl_ctrl,
+		.ctrlbit	= (1 << 1),
+	}, {
+		.name		= "gscl",
+		.devname	= "exynos-gsc.2",
+		.enable		= exynos5_clk_ip_gscl_ctrl,
+		.ctrlbit	= (1 << 2),
+	}, {
+		.name		= "gscl",
+		.devname	= "exynos-gsc.3",
+		.enable		= exynos5_clk_ip_gscl_ctrl,
+		.ctrlbit	= (1 << 3),
+	}, {
 		.name		= SYSMMU_CLOCK_NAME,
 		.devname	= SYSMMU_CLOCK_DEVNAME(mfc_l, 0),
 		.enable		= &exynos5_clk_ip_mfc_ctrl,
@@ -1225,6 +1320,11 @@ static struct clksrc_clk *exynos5_sysclks[] = {
 	&exynos5_clk_aclk_266,
 	&exynos5_clk_aclk_200,
 	&exynos5_clk_aclk_166,
+	&exynos5_clk_aclk_300_gscl,
+	&exynos5_clk_dout_aclk_300_gscl,
+	&exynos5_clk_mout_aclk_300_gscl,
+	&exynos5_clk_mout_aclk_300_gscl_mid,
+	&exynos5_clk_mout_aclk_300_gscl_mid1,
 	&exynos5_clk_aclk_66_pre,
 	&exynos5_clk_aclk_66,
 	&exynos5_clk_dout_mmc0,
-- 
1.7.0.4

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

* [PATCH v3 2/2] ARM: EXYNOS: Add Gscaler device from DT
  2012-07-20 13:08 [PATCH v3 0/2] Add device tree and clock support for Gscaler Shaik Ameer Basha
  2012-07-20 13:08 ` [PATCH v3 1/2] ARM: EXYNOS: Add " Shaik Ameer Basha
@ 2012-07-20 13:08 ` Shaik Ameer Basha
  2012-08-01  6:40   ` Kukjin Kim
  1 sibling, 1 reply; 18+ messages in thread
From: Shaik Ameer Basha @ 2012-07-20 13:08 UTC (permalink / raw)
  To: linux-samsung-soc, devicetree-discuss
  Cc: kgene.kim, sy0816.kang, olofj, thomas.ab, sylvester.nawrocki,
	sachin.kamat, joshi, shaik.samsung, shaik.ameer

This patch adds,
- 4 Gscaler devices to the DT device list
- Gscaler specific entries to the machine file
- binding documentation for Gscaler entries

Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
Signed-off-by: Leela Krishna Amudala <l.krishna@samsung.com>
Signed-off-by: Shaik Ameer Basha <shaik.ameer@samsung.com>
---
 .../devicetree/bindings/media/exynos5-gsc.txt      |   32 ++++++++++++++++++++
 arch/arm/boot/dts/exynos5250.dtsi                  |   28 +++++++++++++++++
 arch/arm/mach-exynos/include/mach/map.h            |    3 ++
 arch/arm/mach-exynos/mach-exynos5-dt.c             |    8 +++++
 4 files changed, 71 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/exynos5-gsc.txt

diff --git a/Documentation/devicetree/bindings/media/exynos5-gsc.txt b/Documentation/devicetree/bindings/media/exynos5-gsc.txt
new file mode 100644
index 0000000..1cb4ea0
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/exynos5-gsc.txt
@@ -0,0 +1,32 @@
+* Samsung Exynos5 Gscaler device
+
+Gscaler is used for scaling and color space conversion on EXYNOS5 SoCs.
+
+Required properties:
+- compatible: should be "samsung,exynos5250-gsc"
+- reg: should contain Gscaler physical address location and length.
+- interrupts: should contain Gscaler interrupt number
+
+Example:
+
+gsc_0:  gsc@0x13e00000 {
+	compatible = "samsung,exynos5250-gsc";
+	reg = <0x13e00000 0x1000>;
+	interrupts = <0 85 0>;
+};
+
+Aliases:
+Each Gscaler node should have a numbered alias in the aliases node,
+in the form of gscN, N = 0...3. Gscaler driver uses these aliases
+to retrieve the device IDs using "of_alias_get_id()" call.
+
+Example:
+
+aliases {
+	gsc0 =&gsc_0;
+	gsc1 =&gsc_1;
+	gsc2 =&gsc_2;
+	gsc3 =&gsc_3;
+};
+
+
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index a3a2eb2..ad6c3c5 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -27,6 +27,10 @@
 		spi0 = &spi_0;
 		spi1 = &spi_1;
 		spi2 = &spi_2;
+		gsc0 = &gsc_0;
+		gsc1 = &gsc_1;
+		gsc2 = &gsc_2;
+		gsc3 = &gsc_3;
 	};
 
 	gic:interrupt-controller@10481000 {
@@ -460,4 +464,28 @@
 			#gpio-cells = <4>;
 		};
 	};
+
+	gsc_0:  gsc@0x13e00000 {
+		compatible = "samsung,exynos5250-gsc";
+		reg = <0x13e00000 0x1000>;
+		interrupts = <0 85 0>;
+	};
+
+	gsc_1:  gsc@0x13e10000 {
+		compatible = "samsung,exynos5250-gsc";
+		reg = <0x13e10000 0x1000>;
+		interrupts = <0 86 0>;
+	};
+
+	gsc_2:  gsc@0x13e20000 {
+		compatible = "samsung,exynos5250-gsc";
+		reg = <0x13e20000 0x1000>;
+		interrupts = <0 87 0>;
+	};
+
+	gsc_3:  gsc@0x13e30000 {
+		compatible = "samsung,exynos5250-gsc";
+		reg = <0x13e30000 0x1000>;
+		interrupts = <0 88 0>;
+	};
 };
diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
index c72b675..217e470 100644
--- a/arch/arm/mach-exynos/include/mach/map.h
+++ b/arch/arm/mach-exynos/include/mach/map.h
@@ -121,6 +121,9 @@
 #define EXYNOS4_PA_SYSMMU_MFC_L		0x13620000
 #define EXYNOS4_PA_SYSMMU_MFC_R		0x13630000
 
+/* x = 0...3 */
+#define EXYNOS5_PA_GSC(x)		(0x13e00000 + ((x) * 0x10000))
+
 #define EXYNOS5_PA_SYSMMU_MDMA1		0x10A40000
 #define EXYNOS5_PA_SYSMMU_SSS		0x10A50000
 #define EXYNOS5_PA_SYSMMU_2D		0x10A60000
diff --git a/arch/arm/mach-exynos/mach-exynos5-dt.c b/arch/arm/mach-exynos/mach-exynos5-dt.c
index ef770bc..fd8f1ca 100644
--- a/arch/arm/mach-exynos/mach-exynos5-dt.c
+++ b/arch/arm/mach-exynos/mach-exynos5-dt.c
@@ -56,6 +56,14 @@ static const struct of_dev_auxdata exynos5250_auxdata_lookup[] __initconst = {
 	OF_DEV_AUXDATA("arm,pl330", EXYNOS5_PA_PDMA0, "dma-pl330.0", NULL),
 	OF_DEV_AUXDATA("arm,pl330", EXYNOS5_PA_PDMA1, "dma-pl330.1", NULL),
 	OF_DEV_AUXDATA("arm,pl330", EXYNOS5_PA_MDMA1, "dma-pl330.2", NULL),
+	OF_DEV_AUXDATA("samsung,exynos5250-gsc", EXYNOS5_PA_GSC(0),
+				"exynos-gsc.0", NULL),
+	OF_DEV_AUXDATA("samsung,exynos5250-gsc", EXYNOS5_PA_GSC(1),
+				"exynos-gsc.1", NULL),
+	OF_DEV_AUXDATA("samsung,exynos5250-gsc", EXYNOS5_PA_GSC(2),
+				"exynos-gsc.2", NULL),
+	OF_DEV_AUXDATA("samsung,exynos5250-gsc", EXYNOS5_PA_GSC(3),
+				"exynos-gsc.3", NULL),
 	{},
 };
 
-- 
1.7.0.4

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

* RE: [PATCH v3 1/2] ARM: EXYNOS: Add clock support for Gscaler
  2012-07-20 13:08 ` [PATCH v3 1/2] ARM: EXYNOS: Add " Shaik Ameer Basha
@ 2012-08-01  6:24   ` Kukjin Kim
  2012-08-01  7:07     ` Shaik Ameer Basha
  0 siblings, 1 reply; 18+ messages in thread
From: Kukjin Kim @ 2012-08-01  6:24 UTC (permalink / raw)
  To: 'Shaik Ameer Basha', linux-samsung-soc,
	devicetree-discuss
  Cc: sy0816.kang, olofj, thomas.ab, sylvester.nawrocki, sachin.kamat,
	joshi, shaik.samsung

Shaik Ameer Basha wrote:
> 
> Add required clock support for Gscaler for exynos5
> 
> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
> Signed-off-by: Leela Krishna Amudala <l.krishna@samsung.com>
> Signed-off-by: Prathyush K <prathyush.k@samsung.com>
> Signed-off-by: Shaik Ameer Basha <shaik.ameer@samsung.com>
> ---
>  arch/arm/mach-exynos/clock-exynos5.c |  100
> ++++++++++++++++++++++++++++++++++
>  1 files changed, 100 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/clock-exynos5.c b/arch/arm/mach-
> exynos/clock-exynos5.c
> index 774533c..49a76b1 100644
> --- a/arch/arm/mach-exynos/clock-exynos5.c
> +++ b/arch/arm/mach-exynos/clock-exynos5.c
> @@ -552,6 +552,81 @@ static struct clksrc_clk exynos5_clk_aclk_66 = {
>  	.reg_div = { .reg = EXYNOS5_CLKDIV_TOP0, .shift = 0, .size = 3 },
>  };
> 
> +/* for aclk_300_gscl_mid */

No need above comment which is certain.

> +static struct clksrc_clk exynos5_clk_mout_aclk_300_gscl_mid = {
> +	.clk = {
> +		.name		= "mout_aclk_300_gscl_mid",
> +	},
> +	.sources = &exynos5_clkset_aclk,
> +	.reg_src = { .reg = EXYNOS5_CLKSRC_TOP0, .shift = 24, .size = 1 },
> +};
> +
> +/* for aclk_300_gscl_mid1 */

Same as above.

> +static struct clk *exynos5_clkset_aclk_300_gscl_mid1_list[] = {
> +	[0] = &exynos5_clk_sclk_vpll.clk,
> +	[1] = &exynos5_clk_mout_cpll.clk,
> +};

In this case, the above sources can be used for gscl_mid1 and disp1_mid as
well. So how about exynos5_clkset_mid1_list?

> +
> +static struct clksrc_sources exynos5_clkset_aclk_300_gscl_mid1 = {
> +	.sources	= exynos5_clkset_aclk_300_gscl_mid1_list,
> +	.nr_sources	=
ARRAY_SIZE(exynos5_clkset_aclk_300_gscl_mid1_list),
> +};

If so, need to update this.

> +
> +

no need double empty lines.

> +static struct clksrc_clk exynos5_clk_mout_aclk_300_gscl_mid1 = {
> +	.clk    = {
> +		.name		= "mout_aclk_300_gscl_mid1",
> +	},
> +	.sources = &exynos5_clkset_aclk_300_gscl_mid1,
> +	.reg_src = { .reg = EXYNOS5_CLKSRC_TOP1, .shift = 12, .size = 1 },
> +};
> +
> +/* for aclk_300_gscl */

no need useless comment.

> +static struct clk *exynos5_clkset_aclk_300_gscl_list[] = {
> +	[0] = &exynos5_clk_mout_aclk_300_gscl_mid.clk,
> +	[1] = &exynos5_clk_mout_aclk_300_gscl_mid1.clk,
> +};
> +
> +static struct clksrc_sources exynos5_clkset_aclk_300_gscl = {
> +	.sources	= exynos5_clkset_aclk_300_gscl_list,
> +	.nr_sources	= ARRAY_SIZE(exynos5_clkset_aclk_300_gscl_list),
> +};
> +
> +static struct clksrc_clk exynos5_clk_mout_aclk_300_gscl = {
> +	.clk    = {
            ^^^^
Tap please.

> +		.name		= "mout_aclk_300_gscl",
> +	},
> +	.sources = &exynos5_clkset_aclk_300_gscl,
> +	.reg_src = { .reg = EXYNOS5_CLKSRC_TOP0, .shift = 25, .size = 1 },
> +};
> +
> +static struct clksrc_clk exynos5_clk_dout_aclk_300_gscl = {
> +	.clk    = {
            ^^^^
Same as above.

> +		.name		= "dout_aclk_300_gscl",
> +		.parent		= &exynos5_clk_mout_aclk_300_gscl.clk,
> +	},
> +	.reg_div = { .reg = EXYNOS5_CLKDIV_TOP1, .shift = 12, .size = 3 },
> +};

And I think, we don't need to define above 'clksrc_clk's?

+static struct clksrc_clk exynos5_clk_mdout_aclk_300_gscl = {
+	.clk    = {
+		.name		= "mdout_aclk_300_gscl",
+	},
+	.sources = &exynos5_clkset_aclk_300_gscl,
+	.reg_src = { .reg = EXYNOS5_CLKSRC_TOP0, .shift = 25, .size = 1 },
> +	.reg_div = { .reg = EXYNOS5_CLKDIV_TOP1, .shift = 12, .size = 3 },
+};

[...]

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

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

* RE: [PATCH v3 2/2] ARM: EXYNOS: Add Gscaler device from DT
  2012-07-20 13:08 ` [PATCH v3 2/2] ARM: EXYNOS: Add Gscaler device from DT Shaik Ameer Basha
@ 2012-08-01  6:40   ` Kukjin Kim
  2012-08-01  7:10     ` Shaik Ameer Basha
  2012-08-01  7:48     ` Thomas Abraham
  0 siblings, 2 replies; 18+ messages in thread
From: Kukjin Kim @ 2012-08-01  6:40 UTC (permalink / raw)
  To: 'Shaik Ameer Basha', linux-samsung-soc,
	devicetree-discuss
  Cc: sy0816.kang, olofj, thomas.ab, sylvester.nawrocki, sachin.kamat,
	joshi, shaik.samsung

Shaik Ameer Basha wrote:
> 
> This patch adds,
> - 4 Gscaler devices to the DT device list
> - Gscaler specific entries to the machine file
> - binding documentation for Gscaler entries
> 
> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
> Signed-off-by: Leela Krishna Amudala <l.krishna@samsung.com>
> Signed-off-by: Shaik Ameer Basha <shaik.ameer@samsung.com>
> ---
>  .../devicetree/bindings/media/exynos5-gsc.txt      |   32
> ++++++++++++++++++++
>  arch/arm/boot/dts/exynos5250.dtsi                  |   28
+++++++++++++++++
>  arch/arm/mach-exynos/include/mach/map.h            |    3 ++
>  arch/arm/mach-exynos/mach-exynos5-dt.c             |    8 +++++
>  4 files changed, 71 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/exynos5-
> gsc.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/exynos5-gsc.txt
> b/Documentation/devicetree/bindings/media/exynos5-gsc.txt
> new file mode 100644
> index 0000000..1cb4ea0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/exynos5-gsc.txt
> @@ -0,0 +1,32 @@
> +* Samsung Exynos5 Gscaler device
> +
> +Gscaler is used for scaling and color space conversion on EXYNOS5 SoCs.
> +
> +Required properties:
> +- compatible: should be "samsung,exynos5250-gsc"

IMO, should be "samsung,exynos5-gsc" because upcoming EXYNOS5 SoCs can use
same gscaler driver.

> +- reg: should contain Gscaler physical address location and length.
> +- interrupts: should contain Gscaler interrupt number
> +
> +Example:
> +
> +gsc_0:  gsc@0x13e00000 {
> +	compatible = "samsung,exynos5250-gsc";

+	compatible = "samsung,exynos5-gsc";

> +	reg = <0x13e00000 0x1000>;
> +	interrupts = <0 85 0>;
> +};
> +

[...]


> diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-
> exynos/include/mach/map.h
> index c72b675..217e470 100644
> --- a/arch/arm/mach-exynos/include/mach/map.h
> +++ b/arch/arm/mach-exynos/include/mach/map.h
> @@ -121,6 +121,9 @@
>  #define EXYNOS4_PA_SYSMMU_MFC_L		0x13620000
>  #define EXYNOS4_PA_SYSMMU_MFC_R		0x13630000
> 
> +/* x = 0...3 */
> +#define EXYNOS5_PA_GSC(x)		(0x13e00000 + ((x) * 0x10000))

I think, separated definitions would be nice because the number of channel
can be changed on other upcoming EXYNOS5 SoCs.

+#define EXYNOS5_PA_GSC0			0x13E00000
+#define EXYNOS5_PA_GSC1			0x13E10000
+#define EXYNOS5_PA_GSC2			0x13E20000
+#define EXYNOS5_PA_GSC3			0x13E30000

[...]

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

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

* Re: [PATCH v3 1/2] ARM: EXYNOS: Add clock support for Gscaler
  2012-08-01  6:24   ` Kukjin Kim
@ 2012-08-01  7:07     ` Shaik Ameer Basha
  0 siblings, 0 replies; 18+ messages in thread
From: Shaik Ameer Basha @ 2012-08-01  7:07 UTC (permalink / raw)
  To: Kukjin Kim
  Cc: Shaik Ameer Basha, linux-samsung-soc, devicetree-discuss,
	sy0816.kang, olofj, thomas.ab, sylvester.nawrocki, sachin.kamat,
	joshi

Hi Kukjin Kim,

thanks for the review comments.

On Wed, Aug 1, 2012 at 11:54 AM, Kukjin Kim <kgene.kim@samsung.com> wrote:
> Shaik Ameer Basha wrote:
>>
>> Add required clock support for Gscaler for exynos5
>>
>> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
>> Signed-off-by: Leela Krishna Amudala <l.krishna@samsung.com>
>> Signed-off-by: Prathyush K <prathyush.k@samsung.com>
>> Signed-off-by: Shaik Ameer Basha <shaik.ameer@samsung.com>
>> ---
>>  arch/arm/mach-exynos/clock-exynos5.c |  100
>> ++++++++++++++++++++++++++++++++++
>>  1 files changed, 100 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/clock-exynos5.c b/arch/arm/mach-
>> exynos/clock-exynos5.c
>> index 774533c..49a76b1 100644
>> --- a/arch/arm/mach-exynos/clock-exynos5.c
>> +++ b/arch/arm/mach-exynos/clock-exynos5.c
>> @@ -552,6 +552,81 @@ static struct clksrc_clk exynos5_clk_aclk_66 = {
>>       .reg_div = { .reg = EXYNOS5_CLKDIV_TOP0, .shift = 0, .size = 3 },
>>  };
>>
>> +/* for aclk_300_gscl_mid */
>
> No need above comment which is certain.
>

Ok. I will remove all unnecessary comments.

>> +static struct clksrc_clk exynos5_clk_mout_aclk_300_gscl_mid = {
>> +     .clk = {
>> +             .name           = "mout_aclk_300_gscl_mid",
>> +     },
>> +     .sources = &exynos5_clkset_aclk,
>> +     .reg_src = { .reg = EXYNOS5_CLKSRC_TOP0, .shift = 24, .size = 1 },
>> +};
>> +
>> +/* for aclk_300_gscl_mid1 */
>
> Same as above.
>
>> +static struct clk *exynos5_clkset_aclk_300_gscl_mid1_list[] = {
>> +     [0] = &exynos5_clk_sclk_vpll.clk,
>> +     [1] = &exynos5_clk_mout_cpll.clk,
>> +};
>
> In this case, the above sources can be used for gscl_mid1 and disp1_mid as
> well. So how about exynos5_clkset_mid1_list?

yes. you are right.
I will change the name as per your suggestion. (i.e.,  exynos5_clkset_mid1_list)

>
>> +
>> +static struct clksrc_sources exynos5_clkset_aclk_300_gscl_mid1 = {
>> +     .sources        = exynos5_clkset_aclk_300_gscl_mid1_list,
>> +     .nr_sources     =
> ARRAY_SIZE(exynos5_clkset_aclk_300_gscl_mid1_list),
>> +};
>
> If so, need to update this.

Ok. I will update here too...

>
>> +
>> +
>
> no need double empty lines.

ok. will remove the extra lines.

>
>> +static struct clksrc_clk exynos5_clk_mout_aclk_300_gscl_mid1 = {
>> +     .clk    = {
>> +             .name           = "mout_aclk_300_gscl_mid1",
>> +     },
>> +     .sources = &exynos5_clkset_aclk_300_gscl_mid1,
>> +     .reg_src = { .reg = EXYNOS5_CLKSRC_TOP1, .shift = 12, .size = 1 },
>> +};
>> +
>> +/* for aclk_300_gscl */
>
> no need useless comment.
>
>> +static struct clk *exynos5_clkset_aclk_300_gscl_list[] = {
>> +     [0] = &exynos5_clk_mout_aclk_300_gscl_mid.clk,
>> +     [1] = &exynos5_clk_mout_aclk_300_gscl_mid1.clk,
>> +};
>> +
>> +static struct clksrc_sources exynos5_clkset_aclk_300_gscl = {
>> +     .sources        = exynos5_clkset_aclk_300_gscl_list,
>> +     .nr_sources     = ARRAY_SIZE(exynos5_clkset_aclk_300_gscl_list),
>> +};
>> +
>> +static struct clksrc_clk exynos5_clk_mout_aclk_300_gscl = {
>> +     .clk    = {
>             ^^^^
> Tap please.

Ok. i will change that.

>
>> +             .name           = "mout_aclk_300_gscl",
>> +     },
>> +     .sources = &exynos5_clkset_aclk_300_gscl,
>> +     .reg_src = { .reg = EXYNOS5_CLKSRC_TOP0, .shift = 25, .size = 1 },
>> +};
>> +
>> +static struct clksrc_clk exynos5_clk_dout_aclk_300_gscl = {
>> +     .clk    = {
>             ^^^^
> Same as above.

Ok. i will change that.

>
>> +             .name           = "dout_aclk_300_gscl",
>> +             .parent         = &exynos5_clk_mout_aclk_300_gscl.clk,
>> +     },
>> +     .reg_div = { .reg = EXYNOS5_CLKDIV_TOP1, .shift = 12, .size = 3 },
>> +};
>
> And I think, we don't need to define above 'clksrc_clk's?
>

Sorry. you are right. i will remove that.

> +static struct clksrc_clk exynos5_clk_mdout_aclk_300_gscl = {
> +       .clk    = {
> +               .name           = "mdout_aclk_300_gscl",
> +       },
> +       .sources = &exynos5_clkset_aclk_300_gscl,
> +       .reg_src = { .reg = EXYNOS5_CLKSRC_TOP0, .shift = 25, .size = 1 },
>> +     .reg_div = { .reg = EXYNOS5_CLKDIV_TOP1, .shift = 12, .size = 3 },
> +};
>
> [...]
>
> Thanks.
>
> Best regards,
> Kgene.
> --
> Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
> SW Solution Development Team, Samsung Electronics Co., Ltd.
>

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

* Re: [PATCH v3 2/2] ARM: EXYNOS: Add Gscaler device from DT
  2012-08-01  6:40   ` Kukjin Kim
@ 2012-08-01  7:10     ` Shaik Ameer Basha
  2012-08-01  7:48     ` Thomas Abraham
  1 sibling, 0 replies; 18+ messages in thread
From: Shaik Ameer Basha @ 2012-08-01  7:10 UTC (permalink / raw)
  To: Kukjin Kim
  Cc: Shaik Ameer Basha, linux-samsung-soc, devicetree-discuss,
	sy0816.kang, olofj, thomas.ab, sylvester.nawrocki, sachin.kamat,
	joshi

Hi Kukjin Kim,


On Wed, Aug 1, 2012 at 12:10 PM, Kukjin Kim <kgene.kim@samsung.com> wrote:
> Shaik Ameer Basha wrote:
>>
>> This patch adds,
>> - 4 Gscaler devices to the DT device list
>> - Gscaler specific entries to the machine file
>> - binding documentation for Gscaler entries
>>
>> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
>> Signed-off-by: Leela Krishna Amudala <l.krishna@samsung.com>
>> Signed-off-by: Shaik Ameer Basha <shaik.ameer@samsung.com>
>> ---
>>  .../devicetree/bindings/media/exynos5-gsc.txt      |   32
>> ++++++++++++++++++++
>>  arch/arm/boot/dts/exynos5250.dtsi                  |   28
> +++++++++++++++++
>>  arch/arm/mach-exynos/include/mach/map.h            |    3 ++
>>  arch/arm/mach-exynos/mach-exynos5-dt.c             |    8 +++++
>>  4 files changed, 71 insertions(+), 0 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/media/exynos5-
>> gsc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/exynos5-gsc.txt
>> b/Documentation/devicetree/bindings/media/exynos5-gsc.txt
>> new file mode 100644
>> index 0000000..1cb4ea0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/exynos5-gsc.txt
>> @@ -0,0 +1,32 @@
>> +* Samsung Exynos5 Gscaler device
>> +
>> +Gscaler is used for scaling and color space conversion on EXYNOS5 SoCs.
>> +
>> +Required properties:
>> +- compatible: should be "samsung,exynos5250-gsc"
>
> IMO, should be "samsung,exynos5-gsc" because upcoming EXYNOS5 SoCs can use
> same gscaler driver.
>

yes. thats true. i will change that.

>> +- reg: should contain Gscaler physical address location and length.
>> +- interrupts: should contain Gscaler interrupt number
>> +
>> +Example:
>> +
>> +gsc_0:  gsc@0x13e00000 {
>> +     compatible = "samsung,exynos5250-gsc";
>
> +       compatible = "samsung,exynos5-gsc";
>

ok. will update this accordingly.

>> +     reg = <0x13e00000 0x1000>;
>> +     interrupts = <0 85 0>;
>> +};
>> +
>
> [...]
>
>
>> diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-
>> exynos/include/mach/map.h
>> index c72b675..217e470 100644
>> --- a/arch/arm/mach-exynos/include/mach/map.h
>> +++ b/arch/arm/mach-exynos/include/mach/map.h
>> @@ -121,6 +121,9 @@
>>  #define EXYNOS4_PA_SYSMMU_MFC_L              0x13620000
>>  #define EXYNOS4_PA_SYSMMU_MFC_R              0x13630000
>>
>> +/* x = 0...3 */
>> +#define EXYNOS5_PA_GSC(x)            (0x13e00000 + ((x) * 0x10000))
>
> I think, separated definitions would be nice because the number of channel
> can be changed on other upcoming EXYNOS5 SoCs.
>
> +#define EXYNOS5_PA_GSC0                        0x13E00000
> +#define EXYNOS5_PA_GSC1                        0x13E10000
> +#define EXYNOS5_PA_GSC2                        0x13E20000
> +#define EXYNOS5_PA_GSC3                        0x13E30000

Ok. I will update as per your suggestion.

>
> [...]
>
> Thanks.
>
> Best regards,
> Kgene.
> --
> Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
> SW Solution Development Team, Samsung Electronics Co., Ltd.
>

Thanks,
Shaik Ameer Basha

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

* Re: [PATCH v3 2/2] ARM: EXYNOS: Add Gscaler device from DT
  2012-08-01  6:40   ` Kukjin Kim
  2012-08-01  7:10     ` Shaik Ameer Basha
@ 2012-08-01  7:48     ` Thomas Abraham
  2012-08-01  8:15       ` Kukjin Kim
  2012-08-01  8:18       ` Sylwester Nawrocki
  1 sibling, 2 replies; 18+ messages in thread
From: Thomas Abraham @ 2012-08-01  7:48 UTC (permalink / raw)
  To: Kukjin Kim
  Cc: Shaik Ameer Basha, linux-samsung-soc, devicetree-discuss,
	sy0816.kang, olofj, thomas.ab, sylvester.nawrocki, sachin.kamat,
	joshi, shaik.samsung

On 1 August 2012 12:10, Kukjin Kim <kgene.kim@samsung.com> wrote:
> Shaik Ameer Basha wrote:

[...]

>> +* Samsung Exynos5 Gscaler device
>> +
>> +Gscaler is used for scaling and color space conversion on EXYNOS5 SoCs.
>> +
>> +Required properties:
>> +- compatible: should be "samsung,exynos5250-gsc"
>
> IMO, should be "samsung,exynos5-gsc" because upcoming EXYNOS5 SoCs can use
> same gscaler driver.

The compatible string should always be specific and it should clearly
identify the type of the controller. If there are other variants of
the GSC controller in previous of upcoming SoC's, then those
controllers will have a different compatible value.

This allows device drivers to know the type of the controller and
handle the differences among them. And, the node in the dts/dtsi file
should always claim compatibility to the base version of the
controller that the platform supports.

So the compatible value "samsung,exynos5250-gsc" is right one. If a
new SoC in the Exynos5 family has the same GSC controller as that in
Exynos5250 (no difference at all), then GSC device node in its dts
file can continue to claim compatibility to Exynos5250 type. The
"samsung,s3c2410-wdt is an example of this case which has been used on
all Samsung SoC's .

Thanks,
Thomas.

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

* RE: [PATCH v3 2/2] ARM: EXYNOS: Add Gscaler device from DT
  2012-08-01  7:48     ` Thomas Abraham
@ 2012-08-01  8:15       ` Kukjin Kim
  2012-08-01  8:18       ` Sylwester Nawrocki
  1 sibling, 0 replies; 18+ messages in thread
From: Kukjin Kim @ 2012-08-01  8:15 UTC (permalink / raw)
  To: 'Thomas Abraham'
  Cc: 'Shaik Ameer Basha', linux-samsung-soc,
	devicetree-discuss, sy0816.kang, olofj, thomas.ab,
	sylvester.nawrocki, sachin.kamat, joshi, shaik.samsung

Thomas Abraham wrote:
> 
> On 1 August 2012 12:10, Kukjin Kim <kgene.kim@samsung.com> wrote:
> > Shaik Ameer Basha wrote:
> 
> [...]
> 
> >> +* Samsung Exynos5 Gscaler device
> >> +
> >> +Gscaler is used for scaling and color space conversion on EXYNOS5
SoCs.
> >> +
> >> +Required properties:
> >> +- compatible: should be "samsung,exynos5250-gsc"
> >
> > IMO, should be "samsung,exynos5-gsc" because upcoming EXYNOS5 SoCs can
> use
> > same gscaler driver.
> 
> The compatible string should always be specific and it should clearly
> identify the type of the controller. If there are other variants of
> the GSC controller in previous of upcoming SoC's, then those
> controllers will have a different compatible value.
> 
As I commented, there will be no differences for gscaler between EXYNOS5
SoCs. So IMO, the 'exynos5-gsc' is clear enough for compatible string of
EXYNOS5 Gscaler.

> This allows device drivers to know the type of the controller and
> handle the differences among them. And, the node in the dts/dtsi file
> should always claim compatibility to the base version of the
> controller that the platform supports.
> 
> So the compatible value "samsung,exynos5250-gsc" is right one. If a
> new SoC in the Exynos5 family has the same GSC controller as that in
> Exynos5250 (no difference at all), then GSC device node in its dts
> file can continue to claim compatibility to Exynos5250 type. The

I don't think so...

> "samsung,s3c2410-wdt is an example of this case which has been used on
> all Samsung SoC's .
> 
I think, the case of 's3c2410-wdt' is different because you know, the name
has been used for a long time in addition, the name of driver is
s3c2410_wdt.c. But in this case, gscaler should be same on EXYNOS5 SoCs
which is including upcoming, so I don't see why we should use
'exynos5250-gsc' for all of EXYNOS5 series.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

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

* Re: [PATCH v3 2/2] ARM: EXYNOS: Add Gscaler device from DT
  2012-08-01  7:48     ` Thomas Abraham
  2012-08-01  8:15       ` Kukjin Kim
@ 2012-08-01  8:18       ` Sylwester Nawrocki
  2012-08-01  9:00         ` Kukjin Kim
  1 sibling, 1 reply; 18+ messages in thread
From: Sylwester Nawrocki @ 2012-08-01  8:18 UTC (permalink / raw)
  To: Kukjin Kim
  Cc: Thomas Abraham, Shaik Ameer Basha, linux-samsung-soc,
	devicetree-discuss, sy0816.kang, olofj, thomas.ab, sachin.kamat,
	joshi, shaik.samsung

On 08/01/2012 09:48 AM, Thomas Abraham wrote:
> On 1 August 2012 12:10, Kukjin Kim <kgene.kim@samsung.com> wrote:
>> Shaik Ameer Basha wrote:
> 
> [...]
> 
>>> +* Samsung Exynos5 Gscaler device
>>> +
>>> +Gscaler is used for scaling and color space conversion on EXYNOS5 SoCs.
>>> +
>>> +Required properties:
>>> +- compatible: should be "samsung,exynos5250-gsc"
>>
>> IMO, should be "samsung,exynos5-gsc" because upcoming EXYNOS5 SoCs can use
>> same gscaler driver.

In addition to the below explanation, perhaps it's obvious, but the driver
can claim compatibility with multiple devices, i.e. match with multiple
'compatible' properties.

Regards,
Sylwester

> The compatible string should always be specific and it should clearly
> identify the type of the controller. If there are other variants of
> the GSC controller in previous of upcoming SoC's, then those
> controllers will have a different compatible value.
> 
> This allows device drivers to know the type of the controller and
> handle the differences among them. And, the node in the dts/dtsi file
> should always claim compatibility to the base version of the
> controller that the platform supports.
> 
> So the compatible value "samsung,exynos5250-gsc" is right one. If a
> new SoC in the Exynos5 family has the same GSC controller as that in
> Exynos5250 (no difference at all), then GSC device node in its dts
> file can continue to claim compatibility to Exynos5250 type. The
> "samsung,s3c2410-wdt is an example of this case which has been used on
> all Samsung SoC's .
> 
> Thanks,
> Thomas.

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

* RE: [PATCH v3 2/2] ARM: EXYNOS: Add Gscaler device from DT
  2012-08-01  8:18       ` Sylwester Nawrocki
@ 2012-08-01  9:00         ` Kukjin Kim
  2012-08-01 19:03           ` Sylwester Nawrocki
  0 siblings, 1 reply; 18+ messages in thread
From: Kukjin Kim @ 2012-08-01  9:00 UTC (permalink / raw)
  To: 'Sylwester Nawrocki'
  Cc: 'Thomas Abraham', 'Shaik Ameer Basha',
	linux-samsung-soc, devicetree-discuss, sy0816.kang, olofj,
	thomas.ab, sachin.kamat, joshi, shaik.samsung, sungchun.kang

Sylwester Nawrocki wrote:
> 
> On 08/01/2012 09:48 AM, Thomas Abraham wrote:
> > On 1 August 2012 12:10, Kukjin Kim <kgene.kim@samsung.com> wrote:
> >> Shaik Ameer Basha wrote:
> >
> > [...]
> >
> >>> +* Samsung Exynos5 Gscaler device
> >>> +
> >>> +Gscaler is used for scaling and color space conversion on EXYNOS5
> SoCs.
> >>> +
> >>> +Required properties:
> >>> +- compatible: should be "samsung,exynos5250-gsc"
> >>
> >> IMO, should be "samsung,exynos5-gsc" because upcoming EXYNOS5 SoCs can
> use
> >> same gscaler driver.
> 
> In addition to the below explanation, perhaps it's obvious, but the driver
> can claim compatibility with multiple devices, i.e. match with multiple
> 'compatible' properties.
> 

The name of exact model is 'gscaler' for EXYNOS5 SoCs not only for
EXYNOS5250. So the driver which has been submitted is also 'exynos-gsc' not
'exynos5250-gsc'. Note that there is no gscaler on EXYNOS4 SoCs now, it can
be either 'exynos5-gsc' or 'exynos-gsc'. In addition, since some
peripherals/drivers can be used for multiple SoCs and actually it does, I'm
still thinking the compatible should be represent its usage. I don't know
why restricted name is much clearer. And if multiple 'compatible' is
required, we can add it later.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

> 
> > The compatible string should always be specific and it should clearly
> > identify the type of the controller. If there are other variants of
> > the GSC controller in previous of upcoming SoC's, then those
> > controllers will have a different compatible value.
> >
> > This allows device drivers to know the type of the controller and
> > handle the differences among them. And, the node in the dts/dtsi file
> > should always claim compatibility to the base version of the
> > controller that the platform supports.
> >
> > So the compatible value "samsung,exynos5250-gsc" is right one. If a
> > new SoC in the Exynos5 family has the same GSC controller as that in
> > Exynos5250 (no difference at all), then GSC device node in its dts
> > file can continue to claim compatibility to Exynos5250 type. The
> > "samsung,s3c2410-wdt is an example of this case which has been used on
> > all Samsung SoC's .
> >

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

* Re: [PATCH v3 2/2] ARM: EXYNOS: Add Gscaler device from DT
  2012-08-01  9:00         ` Kukjin Kim
@ 2012-08-01 19:03           ` Sylwester Nawrocki
       [not found]             ` <50197D66.1050108-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 18+ messages in thread
From: Sylwester Nawrocki @ 2012-08-01 19:03 UTC (permalink / raw)
  To: Kukjin Kim
  Cc: 'Sylwester Nawrocki', 'Thomas Abraham',
	'Shaik Ameer Basha', linux-samsung-soc,
	devicetree-discuss, sy0816.kang, olofj, thomas.ab, sachin.kamat,
	joshi, shaik.samsung, sungchun.kang, Rob Herring

Hi Kgene,

Cc: Rob Herring

On 08/01/2012 11:00 AM, Kukjin Kim wrote:
>>>>> +* Samsung Exynos5 Gscaler device
>>>>> +
>>>>> +Gscaler is used for scaling and color space conversion on EXYNOS5
>> SoCs.
>>>>> +
>>>>> +Required properties:
>>>>> +- compatible: should be "samsung,exynos5250-gsc"
>>>>
>>>> IMO, should be "samsung,exynos5-gsc" because upcoming EXYNOS5 SoCs can
>> use
>>>> same gscaler driver.
>>
>> In addition to the below explanation, perhaps it's obvious, but the driver
>> can claim compatibility with multiple devices, i.e. match with multiple
>> 'compatible' properties.
>>
> 
> The name of exact model is 'gscaler' for EXYNOS5 SoCs not only for
> EXYNOS5250. So the driver which has been submitted is also 'exynos-gsc' not
> 'exynos5250-gsc'. Note that there is no gscaler on EXYNOS4 SoCs now, it can
> be either 'exynos5-gsc' or 'exynos-gsc'. In addition, since some
> peripherals/drivers can be used for multiple SoCs and actually it does, I'm
> still thinking the compatible should be represent its usage. I don't know
> why restricted name is much clearer. And if multiple 'compatible' is
> required, we can add it later.

Sorry, but I cannot agree with that. :) There is no exynos5 SoC, it's just
a common name for the whole series that includes: exynos5250, exynos5450 ones 
and more. We shouldn't use such common name for just a subset of exynos5 chips, 
in case different GScaler versions get deployed in future SoCs.

It wouldn't be clear what specific SoCs the "samsung,exynos5-gsc" compatible 
string applies to, would it ? I believe there are already minor differences
in GScaler parameters on currently available exynos5 SoC. The variant data
structures are used to handle this and the compatible string determines which
variant data structure is selected during driver's initialization.
If you use a wildcard 'compatible' string this won't be possible any more.

Also it would look odd IMO to have two compatible strings like:
compatible = "samsung,exynos5-gsc", "samsung,exynos5400-gsc";

Please also see:
http://devicetree.org/Device_Tree_Usage#Understanding_the_compatible_Property

--

Regards,
Sylwester

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

* Re: [PATCH v3 2/2] ARM: EXYNOS: Add Gscaler device from DT
       [not found]             ` <50197D66.1050108-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2012-08-02 16:33               ` Olof Johansson
  2012-08-02 23:40                 ` Sylwester Nawrocki
  0 siblings, 1 reply; 18+ messages in thread
From: Olof Johansson @ 2012-08-02 16:33 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Shaik Ameer Basha,
	sy0816.kang-Sze3O3UU22JBDgjK7y7TUQ,
	sachin.kamat-QSEj5FYQhm4dnm+yROfE0A,
	sungchun.kang-Sze3O3UU22JBDgjK7y7TUQ,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	joshi-Sze3O3UU22JBDgjK7y7TUQ, Rob Herring, Kukjin Kim,
	thomas.ab-Sze3O3UU22JBDgjK7y7TUQ, Sylwester Nawrocki,
	shaik.samsung-Re5JQEeQqe8AvxtiuMwx3w,
	olofj-hpIqsD4AKlfQT0dZR+AlfA

On Wed, Aug 1, 2012 at 12:03 PM, Sylwester Nawrocki
<sylvester.nawrocki-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> It wouldn't be clear what specific SoCs the "samsung,exynos5-gsc" compatible
> string applies to, would it ? I believe there are already minor differences
> in GScaler parameters on currently available exynos5 SoC. The variant data
> structures are used to handle this and the compatible string determines which
> variant data structure is selected during driver's initialization.
> If you use a wildcard 'compatible' string this won't be possible any more.
>
> Also it would look odd IMO to have two compatible strings like:
> compatible = "samsung,exynos5-gsc", "samsung,exynos5400-gsc";

In this particular case, since you're saying that there are subtle
differences between different part numbers, I'm guessing there's good
reason to go specific, but in general there's no need to avoid
exynos5-gsc.

Your example is also false, since the strings would be in reverse
order (from specific to generic). That would look perfectly normal.

So, bottom line: I agree in this particular instance, but I disagree
that it's a hard generic rule.


-Olof

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

* Re: [PATCH v3 2/2] ARM: EXYNOS: Add Gscaler device from DT
  2012-08-02 16:33               ` Olof Johansson
@ 2012-08-02 23:40                 ` Sylwester Nawrocki
  2012-08-06  6:27                   ` Shaik Ameer Basha
  0 siblings, 1 reply; 18+ messages in thread
From: Sylwester Nawrocki @ 2012-08-02 23:40 UTC (permalink / raw)
  To: Olof Johansson
  Cc: Kukjin Kim, Sylwester Nawrocki, Thomas Abraham, Shaik Ameer Basha,
	linux-samsung-soc, devicetree-discuss, sy0816.kang, olofj,
	thomas.ab, sachin.kamat, joshi, shaik.samsung, sungchun.kang,
	Rob Herring

On 08/02/2012 06:33 PM, Olof Johansson wrote:
> On Wed, Aug 1, 2012 at 12:03 PM, Sylwester Nawrocki
> <sylvester.nawrocki@gmail.com>  wrote:
> 
>> It wouldn't be clear what specific SoCs the "samsung,exynos5-gsc" compatible
>> string applies to, would it ? I believe there are already minor differences
>> in GScaler parameters on currently available exynos5 SoC. The variant data
>> structures are used to handle this and the compatible string determines which
>> variant data structure is selected during driver's initialization.
>> If you use a wildcard 'compatible' string this won't be possible any more.
>>
>> Also it would look odd IMO to have two compatible strings like:
>> compatible = "samsung,exynos5-gsc", "samsung,exynos5400-gsc";
> 
> In this particular case, since you're saying that there are subtle
> differences between different part numbers, I'm guessing there's good
> reason to go specific, but in general there's no need to avoid
> exynos5-gsc.
> 
> Your example is also false, since the strings would be in reverse
> order (from specific to generic). That would look perfectly normal.

You're right, but my intention was more to say that there would have been 
two entries in the driver's of_match_table, where "samsung,exynos5-gsc"
wouldn't have obvious meaning. Devices within these SoCs tend to differ 
across part numbers and usually there is one common driver handling them.

I can't tell for sure now there are differences, but I would have been
surprised if there wouldn't.

> So, bottom line: I agree in this particular instance, but I disagree
> that it's a hard generic rule.

Thanks, sorry if it sounded like I'm advocating it as a general rule. 
I'm no DT expert whatsoever, but in this particular case it just sounded 
messy to use only exynos5-gsc.

--
Regards,
Sylwester

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

* Re: [PATCH v3 2/2] ARM: EXYNOS: Add Gscaler device from DT
  2012-08-02 23:40                 ` Sylwester Nawrocki
@ 2012-08-06  6:27                   ` Shaik Ameer Basha
  2012-08-06 19:09                     ` Sylwester Nawrocki
  0 siblings, 1 reply; 18+ messages in thread
From: Shaik Ameer Basha @ 2012-08-06  6:27 UTC (permalink / raw)
  To: Sylwester Nawrocki, Kukjin Kim
  Cc: Olof Johansson, Sylwester Nawrocki, Thomas Abraham,
	Shaik Ameer Basha, linux-samsung-soc, devicetree-discuss,
	sy0816.kang, olofj, thomas.ab, sachin.kamat, joshi, sungchun.kang,
	Rob Herring

Hi Sylwester,

On Fri, Aug 3, 2012 at 5:10 AM, Sylwester Nawrocki
<sylvester.nawrocki@gmail.com> wrote:
> On 08/02/2012 06:33 PM, Olof Johansson wrote:
>> On Wed, Aug 1, 2012 at 12:03 PM, Sylwester Nawrocki
>> <sylvester.nawrocki@gmail.com>  wrote:
>>
>>> It wouldn't be clear what specific SoCs the "samsung,exynos5-gsc" compatible
>>> string applies to, would it ? I believe there are already minor differences
>>> in GScaler parameters on currently available exynos5 SoC. The variant data
>>> structures are used to handle this and the compatible string determines which
>>> variant data structure is selected during driver's initialization.
>>> If you use a wildcard 'compatible' string this won't be possible any more.
>>>
>>> Also it would look odd IMO to have two compatible strings like:
>>> compatible = "samsung,exynos5-gsc", "samsung,exynos5400-gsc";
>>
>> In this particular case, since you're saying that there are subtle
>> differences between different part numbers, I'm guessing there's good
>> reason to go specific, but in general there's no need to avoid
>> exynos5-gsc.

After all this discussion, I can see two possibilities here.
1] If Kukjin Kim is sure about G-Scaler remains unchanged, across all
the exynos5 series SoCs,
        It is fine to go with the compatible string  "samsung,exynos5-gsc".
2] Otherwise in case of any doubts about G-Scaler is going to change,
        It is safe to go with the compatible string specific to
current SoC i.e., "samsung,exynos5250-gsc".

If we all can agree on this, lets Kukjin Kim decide which string to
use as he has good knowledge about upcoming exynos5 series SoCs.

Regards,
Shaik Ameer Basha

>>
>> Your example is also false, since the strings would be in reverse
>> order (from specific to generic). That would look perfectly normal.
>
> You're right, but my intention was more to say that there would have been
> two entries in the driver's of_match_table, where "samsung,exynos5-gsc"
> wouldn't have obvious meaning. Devices within these SoCs tend to differ
> across part numbers and usually there is one common driver handling them.
>
> I can't tell for sure now there are differences, but I would have been
> surprised if there wouldn't.
>
>> So, bottom line: I agree in this particular instance, but I disagree
>> that it's a hard generic rule.
>
> Thanks, sorry if it sounded like I'm advocating it as a general rule.
> I'm no DT expert whatsoever, but in this particular case it just sounded
> messy to use only exynos5-gsc.
>
> --
> Regards,
> Sylwester

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

* Re: [PATCH v3 2/2] ARM: EXYNOS: Add Gscaler device from DT
  2012-08-06  6:27                   ` Shaik Ameer Basha
@ 2012-08-06 19:09                     ` Sylwester Nawrocki
  2012-08-07 10:01                       ` Kukjin Kim
  2012-08-11  4:08                       ` Kukjin Kim
  0 siblings, 2 replies; 18+ messages in thread
From: Sylwester Nawrocki @ 2012-08-06 19:09 UTC (permalink / raw)
  To: Shaik Ameer Basha
  Cc: Kukjin Kim, Olof Johansson, Sylwester Nawrocki, Thomas Abraham,
	Shaik Ameer Basha, linux-samsung-soc, devicetree-discuss,
	sy0816.kang, olofj, thomas.ab, sachin.kamat, joshi, sungchun.kang,
	Rob Herring

On 08/06/2012 08:27 AM, Shaik Ameer Basha wrote:
> After all this discussion, I can see two possibilities here.
> 1] If Kukjin Kim is sure about G-Scaler remains unchanged, across all
> the exynos5 series SoCs,
>          It is fine to go with the compatible string  "samsung,exynos5-gsc".
> 2] Otherwise in case of any doubts about G-Scaler is going to change,
>          It is safe to go with the compatible string specific to
> current SoC i.e., "samsung,exynos5250-gsc".
> 
> If we all can agree on this, lets Kukjin Kim decide which string to
> use as he has good knowledge about upcoming exynos5 series SoCs.

I don't have strong opinion on this, but my vote goes for using more 
specific properties. Of course the final word belongs to Mr. Kim.

--

Regards,
Sylwester

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

* RE: [PATCH v3 2/2] ARM: EXYNOS: Add Gscaler device from DT
  2012-08-06 19:09                     ` Sylwester Nawrocki
@ 2012-08-07 10:01                       ` Kukjin Kim
  2012-08-11  4:08                       ` Kukjin Kim
  1 sibling, 0 replies; 18+ messages in thread
From: Kukjin Kim @ 2012-08-07 10:01 UTC (permalink / raw)
  To: 'Sylwester Nawrocki', 'Shaik Ameer Basha'
  Cc: 'Olof Johansson', 'Sylwester Nawrocki',
	'Thomas Abraham', 'Shaik Ameer Basha',
	linux-samsung-soc, devicetree-discuss, sy0816.kang, olofj,
	thomas.ab, sachin.kamat, joshi, sungchun.kang,
	'Rob Herring'

Sylwester Nawrocki wrote:
> 
> On 08/06/2012 08:27 AM, Shaik Ameer Basha wrote:
> > After all this discussion, I can see two possibilities here.
> > 1] If Kukjin Kim is sure about G-Scaler remains unchanged, across all
> > the exynos5 series SoCs,
> >          It is fine to go with the compatible string
"samsung,exynos5-gsc".
> > 2] Otherwise in case of any doubts about G-Scaler is going to change,
> >          It is safe to go with the compatible string specific to
> > current SoC i.e., "samsung,exynos5250-gsc".
> >
> > If we all can agree on this, lets Kukjin Kim decide which string to
> > use as he has good knowledge about upcoming exynos5 series SoCs.
> 
> I don't have strong opinion on this, but my vote goes for using more
> specific properties. Of course the final word belongs to Mr. Kim.
> 
Thanks, and I will discuss with exynos hardware chip designers about that,
then let you know.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

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

* RE: [PATCH v3 2/2] ARM: EXYNOS: Add Gscaler device from DT
  2012-08-06 19:09                     ` Sylwester Nawrocki
  2012-08-07 10:01                       ` Kukjin Kim
@ 2012-08-11  4:08                       ` Kukjin Kim
  1 sibling, 0 replies; 18+ messages in thread
From: Kukjin Kim @ 2012-08-11  4:08 UTC (permalink / raw)
  To: 'Kukjin Kim', 'Sylwester Nawrocki',
	'Shaik Ameer Basha'
  Cc: 'Olof Johansson', 'Sylwester Nawrocki',
	'Thomas Abraham', 'Shaik Ameer Basha',
	linux-samsung-soc, devicetree-discuss, sy0816.kang, olofj,
	thomas.ab, sachin.kamat, joshi, sungchun.kang,
	'Rob Herring'

Kukjin Kim wrote:
> 
> Sylwester Nawrocki wrote:
> >
> > On 08/06/2012 08:27 AM, Shaik Ameer Basha wrote:
> > > After all this discussion, I can see two possibilities here.
> > > 1] If Kukjin Kim is sure about G-Scaler remains unchanged, across all
> > > the exynos5 series SoCs,
> > >          It is fine to go with the compatible string
"samsung,exynos5-
> gsc".
> > > 2] Otherwise in case of any doubts about G-Scaler is going to change,
> > >          It is safe to go with the compatible string specific to
> > > current SoC i.e., "samsung,exynos5250-gsc".
> > >
> > > If we all can agree on this, lets Kukjin Kim decide which string to
> > > use as he has good knowledge about upcoming exynos5 series SoCs.
> >
> > I don't have strong opinion on this, but my vote goes for using more
> > specific properties. Of course the final word belongs to Mr. Kim.
> >
> Thanks, and I will discuss with exynos hardware chip designers about that,
> then let you know.
> 
'exynos5-gsc' should be ok on EXYNOS5250 and upcoming EXYNOS5 SoCs.

If any updates, I'll let you know.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

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

end of thread, other threads:[~2012-08-11  4:08 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-20 13:08 [PATCH v3 0/2] Add device tree and clock support for Gscaler Shaik Ameer Basha
2012-07-20 13:08 ` [PATCH v3 1/2] ARM: EXYNOS: Add " Shaik Ameer Basha
2012-08-01  6:24   ` Kukjin Kim
2012-08-01  7:07     ` Shaik Ameer Basha
2012-07-20 13:08 ` [PATCH v3 2/2] ARM: EXYNOS: Add Gscaler device from DT Shaik Ameer Basha
2012-08-01  6:40   ` Kukjin Kim
2012-08-01  7:10     ` Shaik Ameer Basha
2012-08-01  7:48     ` Thomas Abraham
2012-08-01  8:15       ` Kukjin Kim
2012-08-01  8:18       ` Sylwester Nawrocki
2012-08-01  9:00         ` Kukjin Kim
2012-08-01 19:03           ` Sylwester Nawrocki
     [not found]             ` <50197D66.1050108-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2012-08-02 16:33               ` Olof Johansson
2012-08-02 23:40                 ` Sylwester Nawrocki
2012-08-06  6:27                   ` Shaik Ameer Basha
2012-08-06 19:09                     ` Sylwester Nawrocki
2012-08-07 10:01                       ` Kukjin Kim
2012-08-11  4:08                       ` Kukjin Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).