Hi Kevin,
On Sat, Jan 8, 2011 at 5:04 AM, Kevin Hilman <khilman@xxxxxx> wrote:
> Sumit Semwal <sumit.semwal@xxxxxx> writes:
>
>> From: Senthilvadivu Guruswamy <svadivu@xxxxxx>
>>
>> Looks up the hwmod database for each of the given DSS HW IP and builds
>> omap_device which inturn does the platform device register for each of DSS HW IP
>>
>> Signed-off-by: Senthilvadivu Guruswamy <svadivu@xxxxxx>
>> Signed-off-by: Sumit Semwal <sumit.semwal@xxxxxx>
>> ---
>> arch/arm/mach-omap2/display.c | 44 +++++++++++++++++++++++++++++
>> arch/arm/plat-omap/include/plat/display.h | 6 ++++
>> 2 files changed, 50 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
>> index 26d3feb..276b800 100644
>> --- a/arch/arm/mach-omap2/display.c
>> +++ b/arch/arm/mach-omap2/display.c
>> @@ -36,10 +36,54 @@ static struct platform_device omap_display_device = {
>> },
>> };
>>
>> +static struct omap_device_pm_latency omap_dss_latency[] = {
>> + [0] = {
>> + .deactivate_func = omap_device_idle_hwmods,
>> + .activate_func = omap_device_enable_hwmods,
>
> Without any latency numbers or AUTO_ADJUST you're going to have noisy
> output from omap_device.
>
>> + },
>> +};
>> +
>> int __init omap_display_init(struct omap_dss_board_info
>> *board_data)
>> {
>> int r = 0;
>> + struct omap_hwmod *oh;
>> + struct omap_device *od;
>> + int i;
>> + struct omap_display_platform_data pdata;
>> + char *oh_name[] = { "dss_dss", /* omap2,3 */
>> + "dss_dispc", /* omap2,3 */
>> + "dss_rfbi", /* omap2,3 */
>> + "dss_venc", /* omap2,3 */
>> + "dss_dsi1" }; /* omap3 */
>
> Why all the extra whitespace before the strings?
>
>> + char *dev_name[] = { "omap_dss", "omap_dispc", "omap_rfbi",
>> + "omap_venc", "omap_dsi1" };
>
> ditto
>
>> + int oh_count;
>> +
>> + if (cpu_is_omap24xx()) {
>> + oh_count = ARRAY_SIZE(oh_name) - 1;
>> + /* last hwmod dev in oh_name is not available for omap2 */
>> + } else {
>> + oh_count = ARRAY_SIZE(oh_name);
>> + }
>
> braces not needed
>
>> + pdata.board_data = board_data;
>
> extra whitespace not necessary
>
>> + pdata.board_data->get_last_off_on_transaction_id = NULL;
>
> instead, you should probably zero all of pdata before using it since it
> is on the stack
>
>> + for (i = 0; i < oh_count; i++) {
>> + oh = omap_hwmod_lookup(oh_name[i]);
>> + if (!oh) {
>> + pr_err("Could not look up %s\n", oh_name[i]);
>> + return r;
>> + }
>> + od = omap_device_build(dev_name[i], -1, oh, &pdata,
>> + sizeof(struct omap_display_platform_data),
>> + omap_dss_latency,
>> + ARRAY_SIZE(omap_dss_latency), 0);
>> +
>> + WARN((IS_ERR(od)), "Could not build omap_device for %s\n",
>> + oh_name[i]);
>
> yet code below will still try and register it?
>
>> + }
>> omap_display_device.dev.platform_data = board_data;
>>
>> r = platform_device_register(&omap_display_device);
>> diff --git a/arch/arm/plat-omap/include/plat/display.h b/arch/arm/plat-omap/include/plat/display.h
>> index 871bbae..23afd6d 100644
>> --- a/arch/arm/plat-omap/include/plat/display.h
>> +++ b/arch/arm/plat-omap/include/plat/display.h
>> @@ -26,6 +26,7 @@
>> #include <linux/platform_device.h>
>> #include <asm/atomic.h>
>>
>> +
>
> stray whitespace change
>
>> #define DISPC_IRQ_FRAMEDONE (1 << 0)
>> #define DISPC_IRQ_VSYNC (1 << 1)
>> #define DISPC_IRQ_EVSYNC_EVEN (1 << 2)
>> @@ -224,6 +225,11 @@ struct omap_dss_board_info {
>> /* Init with the board info */
>> extern int omap_display_init(struct omap_dss_board_info *board_data);
>>
>> +struct omap_display_platform_data {
>> + struct omap_dss_board_info *board_data;
>> + /* TODO: Additional members to be added when PM is considered */
>> +};
>> +
>> struct omap_video_timings {
>> /* Unit: pixels */
>> u16 x_res;
>
Thanks for all your comments; will incorporate in the upcoming version.
Best regards,
~Sumit.
> Kevin
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
