On 05/14/2013 01:53 PM, George Joseph wrote:
From: George Joseph Palathingal<[email protected]>

Adding DT support to the driver. Driver supports Exynos 4210, 4412 and 5250.

Signed-off-by: George Joseph Palathingal<[email protected]>
Cc: [email protected]
---
  drivers/media/platform/s5p-jpeg/jpeg-core.c |   36 +++++++++++++++++++++++++--
  1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index f964566..b2bf412 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -970,6 +970,8 @@ const struct jpeg_vb2 jpeg_vb2_dma = {
        .plane_addr     = vb2_dma_contig_plane_dma_addr,
  };

+static void *jpeg_get_drv_data(struct platform_device *pdev);

How about putting definition of the jpeg_get_drv_data() function and
the exynos_jpeg_match array above jpeg_probe() to avoid this forward
declaration ?

  static int jpeg_probe(struct platform_device *pdev)
  {
        struct jpeg_dev *dev;
@@ -982,8 +984,7 @@ static int jpeg_probe(struct platform_device *pdev)
                return -ENOMEM;

        dev->plat_dev = pdev;
-       dev->variant = (struct s5p_jpeg_variant *)
-                               platform_get_device_id(pdev)->driver_data;
+       dev->variant = (struct s5p_jpeg_variant *)jpeg_get_drv_data(pdev);

Casting is not needed here.

        spin_lock_init(&dev->slock);

        /* setup jpeg control */
@@ -1232,6 +1233,36 @@ static struct platform_device_id jpeg_driver_ids[] = {

  MODULE_DEVICE_TABLE(platform, jpeg_driver_ids);

+static const struct of_device_id exynos_jpeg_match[] = {
+       {
+               .compatible = "samsung,s5pv210-jpeg",
+               .data =&jpeg_drvdata_v1,
+       }, {
+               .compatible = "samsung,exynos4212-jpeg",
+               .data =&jpeg_drvdata_v2,
+       },
+       {},
+};
+
+MODULE_DEVICE_TABLE(of, exynos_jpeg_match);
+
+static void *jpeg_get_drv_data(struct platform_device *pdev)
+{
+       struct s5p_jpeg_variant *driver_data = NULL;

Unnecessary NULL assignment.

+       if (pdev->dev.of_node) {
+               const struct of_device_id *match;
+               match = of_match_node(of_match_ptr(exynos_jpeg_match),
+                                        pdev->dev.of_node);
+               if (match)
+                       driver_data = (struct s5p_jpeg_variant *)match->data;
+               } else {

Indentation is wrong here.

+                       driver_data = (struct s5p_jpeg_variant *)
+                               platform_get_device_id(pdev)->driver_data;
+               }
+       return driver_data;
+}

How about rewriting this function to something like:

static const void *jpeg_get_drv_data(struct platform_device *pdev)
{
        struct device_node *node = pdev->dev.of_node;
        const struct of_device_id *match;

        if (node) {
                match = of_match_node(exynos_jpeg_match, node);
                return match ? match->data : NULL;
        }

        return (void *)platform_get_device_id(pdev)->driver_data;
}

?
  static struct platform_driver jpeg_driver = {
        .probe          = jpeg_probe,
        .remove         = jpeg_remove,
@@ -1241,6 +1272,7 @@ static struct platform_driver jpeg_driver = {
        .driver = {
                .owner                  = THIS_MODULE,
                .name                   = JPEG_NAME,
+               .of_match_table         = exynos_jpeg_match,
                .pm                     = NULL,
        },
  };

Thanks,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to