Re: [PATCH 1/2] [media] s5p-g2d: Add DT based discovery support

[Sylwester Nawrocki]

Hi Sachin,

On 01/25/2013 10:55 AM, Sachin Kamat wrote:

This patch adds device tree based discovery support to G2D driver

Signed-off-by: Sachin Kamat<sachin.ka...@linaro.org>
---
  drivers/media/platform/s5p-g2d/g2d.c |   17 ++++++++++++++++-
  1 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/drivers/media/platform/s5p-g2d/g2d.c 
b/drivers/media/platform/s5p-g2d/g2d.c
index 7e41529..210e142 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>
@@ -796,7 +797,8 @@ 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);

Don' you need something like:

        else {
                of_id = of_match_node(exynos_g2d_match, pdev->dev.of_node);
                if (!of_id)
                        return -ENODEV;
                dev->variant = (struct g2d_variant *)of_id->data;
        }
?

Otherwise dev->variant is left uninitialized...?

        return 0;

@@ -844,6 +846,18 @@ 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,g2d-v3",
+               .data =&g2d_drvdata_v3x,
+       }, {
+               .compatible = "samsung,g2d-v41",
+               .data =&g2d_drvdata_v4x,

Didn't you consider adding "exynos" to these compatible strings ?
I'm afraid g2d may be too generic.

+       },
+       {},
+};
+MODULE_DEVICE_TABLE(of, exynos_g2d_match);
+
  static struct platform_device_id g2d_driver_ids[] = {
        {
                .name = "s5p-g2d",
@@ -863,6 +877,7 @@ static struct platform_driver g2d_pdrv = {
        .driver         = {
                .name = G2D_NAME,
                .owner = THIS_MODULE,
+               .of_match_table = of_match_ptr(exynos_g2d_match),

of_match_ptr() could be dropped, since exynos_g2d_match[] is
always compiled in.

--

Thanks,
Sylwester
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel