On 05/14/2013 01:53 PM, George Joseph wrote:
From: George Joseph Palathingal<george.jp@xxxxxxxxxxx> Adding DT support to the driver. Driver supports Exynos 4210, 4412 and 5250. Signed-off-by: George Joseph Palathingal<george.jp@xxxxxxxxxxx> Cc: devicetree-discuss@xxxxxxxxxxxxxxxx --- 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html