diff mbox

[v2,1/2,media] s5p-g2d: Add DT based discovery support

Message ID 1360128584-23167-1-git-send-email-sachin.kamat@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Sachin Kamat Feb. 6, 2013, 5:29 a.m. UTC
This patch adds device tree based discovery support to G2D driver

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
Based on for_v3.9 branch of below tree:
git://linuxtv.org/snawrocki/samsung.git

Changes since v1:
* Addressed review comments from Sylwester <s.nawrocki@samsung.com>.
* Modified the compatible string as per the discussions at [1].
[1] https://patchwork1.kernel.org/patch/2045821/
---
 drivers/media/platform/s5p-g2d/g2d.c |   31 +++++++++++++++++++++++++++++--
 1 files changed, 29 insertions(+), 2 deletions(-)

Comments

Sachin Kamat Feb. 12, 2013, 5:30 p.m. UTC | #1
Hi Sylwester,

On Wednesday, 6 February 2013, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> This patch adds device tree based discovery support to G2D driver
>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
> Based on for_v3.9 branch of below tree:
> git://linuxtv.org/snawrocki/samsung.git
>
> Changes since v1:
> * Addressed review comments from Sylwester <s.nawrocki@samsung.com>.
> * Modified the compatible string as per the discussions at [1].
> [1] https://patchwork1.kernel.org/patch/2045821/
>

Does this patch look good?
Sylwester Nawrocki Feb. 13, 2013, 11:34 p.m. UTC | #2
On 02/12/2013 06:30 PM, Sachin Kamat wrote:
>
> Hi Sylwester,
>
> On Wednesday, 6 February 2013, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>>  This patch adds device tree based discovery support to G2D driver
>>
>>  Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>>  ---
>>  Based on for_v3.9 branch of below tree:
>>  git://linuxtv.org/snawrocki/samsung.git
>>
>>  Changes since v1:
>>  * Addressed review comments from Sylwester <s.nawrocki@samsung.com>.
>>  * Modified the compatible string as per the discussions at [1].
>>  [1] https://patchwork1.kernel.org/patch/2045821/
>>
>
> Does this patch look good?

It looks OK to me. I've sent a pull request including it, but it may
happen it ends up only in 3.10.

I tried to test this patch today and I had to correct some clock
definitions in the common clock API driver [1]. And we already have
quite a few fixes to that patch series.

Shouldn't you also provide a patch adding related OF_DEV_AUXDATA entry ?
How did you test this one ?

When the new clocks driver gets merged (I guess it happens only in 3.10)
I'd like to have the media devices' clock names cleaned up, instead of
names like: {"sclk_fimg2d", "fimg2d"}, {"sclk_fimc", "fimc"},
{"sclk_fimd"/"fimd"}, in clock-names property we could have common names,
e.g. { "sclk", "gate" }. This could simplify a bit subsystems like devfreq.

Also I noticed there are some issues caused by splitting mux + div + gate
clocks into 3 different clocks. One solution to this might be to use the
new composite clock type.

[1] http://www.spinics.net/lists/arm-kernel/msg214149.html
Sachin Kamat Feb. 14, 2013, 3:06 p.m. UTC | #3
On Thursday, 14 February 2013, Sylwester Nawrocki <
sylvester.nawrocki@gmail.com> wrote:
> On 02/12/2013 06:30 PM, Sachin Kamat wrote:
>>
>> Hi Sylwester,
>>
>> On Wednesday, 6 February 2013, Sachin Kamat <sachin.kamat@linaro.org>
wrote:
>>>
>>>  This patch adds device tree based discovery support to G2D driver
>>>
>>>  Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>>>  ---
>>>  Based on for_v3.9 branch of below tree:
>>>  git://linuxtv.org/snawrocki/samsung.git
>>>
>>>  Changes since v1:
>>>  * Addressed review comments from Sylwester <s.nawrocki@samsung.com>.
>>>  * Modified the compatible string as per the discussions at [1].
>>>  [1] https://patchwork1.kernel.org/patch/2045821/
>>>
>>
>> Does this patch look good?
>
> It looks OK to me. I've sent a pull request including it, but it may
> happen it ends up only in 3.10.

Thanks. Hope it gets picked for 3.9 itself.

>
> I tried to test this patch today and I had to correct some clock
> definitions in the common clock API driver [1]. And we already have
> quite a few fixes to that patch series.
>
> Shouldn't you also provide a patch adding related OF_DEV_AUXDATA entry ?
> How did you test this one ?

I tested this without the common clock patches, with the mainline tree. It
did not require any auxdata  entry.
>
> When the new clocks driver gets merged (I guess it happens only in 3.10)
> I'd like to have the media devices' clock names cleaned up, instead of
> names like: {"sclk_fimg2d", "fimg2d"}, {"sclk_fimc", "fimc"},
> {"sclk_fimd"/"fimd"}, in clock-names property we could have common names,
> e.g. { "sclk", "gate" }. This could simplify a bit subsystems like
devfreq.

Yes. That makes sense.

>
> Also I noticed there are some issues caused by splitting mux + div + gate
> clocks into 3 different clocks. One solution to this might be to use the
> new composite clock type.

Ok.

>
> [1] http://www.spinics.net/lists/arm-kernel/msg214149.html
>
diff mbox

Patch

diff --git a/drivers/media/platform/s5p-g2d/g2d.c b/drivers/media/platform/s5p-g2d/g2d.c
index 7e41529..6923be2 100644
--- a/drivers/media/platform/s5p-g2d/g2d.c
+++ b/drivers/media/platform/s5p-g2d/g2d.c
@@ -18,6 +18,7 @@ 
 #include <linux/slab.h>
 #include <linux/clk.h>
 #include <linux/interrupt.h>
+#include <linux/of.h>
 
 #include <linux/platform_device.h>
 #include <media/v4l2-mem2mem.h>
@@ -695,11 +696,14 @@  static struct v4l2_m2m_ops g2d_m2m_ops = {
 	.unlock		= g2d_unlock,
 };
 
+static const struct of_device_id exynos_g2d_match[];
+
 static int g2d_probe(struct platform_device *pdev)
 {
 	struct g2d_dev *dev;
 	struct video_device *vfd;
 	struct resource *res;
+	const struct of_device_id *of_id;
 	int ret = 0;
 
 	dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
@@ -796,7 +800,17 @@  static int g2d_probe(struct platform_device *pdev)
 	}
 
 	def_frame.stride = (def_frame.width * def_frame.fmt->depth) >> 3;
-	dev->variant = g2d_get_drv_data(pdev);
+
+	if (!pdev->dev.of_node) {
+		dev->variant = g2d_get_drv_data(pdev);
+	} else {
+		of_id = of_match_node(exynos_g2d_match, pdev->dev.of_node);
+		if (!of_id) {
+			ret = -ENODEV;
+			goto unreg_video_dev;
+		}
+		dev->variant = (struct g2d_variant *)of_id->data;
+	}
 
 	return 0;
 
@@ -837,13 +851,25 @@  static int g2d_remove(struct platform_device *pdev)
 }
 
 static struct g2d_variant g2d_drvdata_v3x = {
-	.hw_rev = TYPE_G2D_3X,
+	.hw_rev = TYPE_G2D_3X, /* Revision 3.0 for S5PV210 and Exynos4210 */
 };
 
 static struct g2d_variant g2d_drvdata_v4x = {
 	.hw_rev = TYPE_G2D_4X, /* Revision 4.1 for Exynos4X12 and Exynos5 */
 };
 
+static const struct of_device_id exynos_g2d_match[] = {
+	{
+		.compatible = "samsung,s5pv210-g2d",
+		.data = &g2d_drvdata_v3x,
+	}, {
+		.compatible = "samsung,exynos4212-g2d",
+		.data = &g2d_drvdata_v4x,
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, exynos_g2d_match);
+
 static struct platform_device_id g2d_driver_ids[] = {
 	{
 		.name = "s5p-g2d",
@@ -863,6 +889,7 @@  static struct platform_driver g2d_pdrv = {
 	.driver		= {
 		.name = G2D_NAME,
 		.owner = THIS_MODULE,
+		.of_match_table = exynos_g2d_match,
 	},
 };