Hi Sachin,

On 04/24/2012 12:38 PM, Sachin Kamat wrote:
> From: Ajay Kumar<ajaykumar...@samsung.com>
> 
> Modify the G2D driver(which initially supported only FIMG2D v3 style H/W)
> to support FIMG2D v4.1 style hardware present on Exynos4x12 and Exynos52x0 
> SOC.
> 
>       -- Set the SRC and DST type to 'memory' instead of using reset values.
>       -- FIMG2D v4.1 H/W uses different logic for stretching(scaling).
>       -- Use CACHECTL_REG only with FIMG2D v3.
> 
> Signed-off-by: Ajay Kumar<ajaykumar...@samsung.com>
> Signed-off-by: Sachin Kamat<sachin.ka...@linaro.org>
> ---
>   drivers/media/video/s5p-g2d/g2d-hw.c   |   17 +++++++++++++----
>   drivers/media/video/s5p-g2d/g2d-regs.h |    6 ++++++
>   drivers/media/video/s5p-g2d/g2d.c      |   23 +++++++++++++++++++++--
>   drivers/media/video/s5p-g2d/g2d.h      |    9 ++++++++-
>   4 files changed, 48 insertions(+), 7 deletions(-)
> 
...
> @@ -783,6 +788,8 @@ static int g2d_probe(struct platform_device *pdev)
> 
>       def_frame.stride = (def_frame.width * def_frame.fmt->depth)>>  3;
> 
> +     dev->device_type = platform_get_device_id(pdev)->driver_data;
> +
>       return 0;
> 
>   unreg_video_dev:
> @@ -832,9 +839,21 @@ static int g2d_remove(struct platform_device *pdev)
>       return 0;
>   }
> 
> +static struct platform_device_id g2d_driver_ids[] = {
> +     {
> +             .name           = "s5p-g2d",
> +             .driver_data    = TYPE_G2D_3X,

IMHO it would be better to define a separate data structure describing
the quirks. For an example please see http://patchwork.linuxtv.org/patch/10869
and the code using struct flite_variant. There was some lengthy 
discussion recently on linux-i2c mailing list, where someone tried
to add more quirks to the i2c-s3c2440 driver which uses 'driver_data'
like it is done in this patch. To avoid wasting time in future, 
I would suggest to make 'driver_data' right away holding a pointer 
to a data structure, rather than simple integer.

We could start, for example, with something like:

struct g2d_variant {
        unsigned short hw_rev;
};

> +     }, {
> +             .name           = "s5p-g2d41x",
> +             .driver_data    = TYPE_G2D_41X,
> +     }, { },

How about marking the last empty entry e.g.

        { /* sentinel */ }

? Or just putting it in new line ?

> +};
> +MODULE_DEVICE_TABLE(platform, s3c24xx_driver_ids);

Hmm, should be g2d_driver_ids. This isn't going to fly when you 
compile this driver as a module. You would get an error like:

error: ‘__mod_platform_device_table’ aliased to undefined symbol 
‘s3c24xx_driver_ids’


Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to