Hello. On 20-12-2010 16:54, Manjunath Hadli wrote:
This patch implements the overall device creation for the Video display driver
Signed-off-by: Manjunath Hadli<manjunath.hadli@xxxxxx> Acked-by: Muralidharan Karicheri<m-karicheri2@xxxxxx> Acked-by: Hans Verkuil<hverkuil@xxxxxxxxx>
[...]
diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach-davinci/dm644x.c index 5e5b0a7..e8b8e94 100644 --- a/arch/arm/mach-davinci/dm644x.c +++ b/arch/arm/mach-davinci/dm644x.c @@ -640,6 +640,142 @@ void dm644x_set_vpfe_config(struct vpfe_config *cfg) vpfe_capture_dev.dev.platform_data = cfg; } +static struct resource dm644x_osd_resources[] = { + { + .start = 0x01C72600, + .end = 0x01C72600 + 0x200,
^^^^^^ Rather 0x1ff? Else it looks like off-by-one error.
+ .flags = IORESOURCE_MEM, + }, +};
[...]
+static struct resource dm644x_venc_resources[] = { + /* venc registers io space */ + { + .start = 0x01C72400, + .end = 0x01C72400 + 0x180,
Same here...
+ .flags = IORESOURCE_MEM, + }, +}; + +static u64 dm644x_venc_dma_mask = DMA_BIT_MASK(32); + +#define VPSS_CLKCTL 0x01C40044
Empty line here, please.
+static void __iomem *vpss_clkctl_reg; + +/* TBD. Check what VENC_CLOCK_SEL settings for HDTV and EDTV */ +static int dm644x_venc_setup_clock(enum vpbe_enc_timings_type type, __u64 mode) +{ + int ret = 0; + + if (NULL == vpss_clkctl_reg) + return -EINVAL; + if (type == VPBE_ENC_STD) {
*switch* would look more natural here.
+ __raw_writel(0x18, vpss_clkctl_reg); + } else if (type == VPBE_ENC_DV_PRESET) { + switch ((unsigned int)mode) { + case V4L2_DV_480P59_94: + case V4L2_DV_576P50: + __raw_writel(0x19, vpss_clkctl_reg); + break; + case V4L2_DV_720P60: + case V4L2_DV_1080I60: + case V4L2_DV_1080P30: + /* + * For HD, use external clock source since HD requires higher + * clock rate + */
Please indent the comment properly. And the preferred style is this: /* * For HD, use external clock source since HD * requires higher clock rate */
+ __raw_writel(0xa, vpss_clkctl_reg); + break; + default: + ret = -EINVAL; + break; + } + } else + ret = -EINVAL; + + return ret; +} + +
One empty line too many?
+static inline u32 dm644x_reg_modify(void *reg, u32 val, u32 mask) +{ + u32 new_val = (__raw_readl(reg) & ~mask) | (val & mask); + __raw_writel(new_val, reg); + return new_val; +} + +static u64 vpbe_display_dma_mask = DMA_BIT_MASK(32); + +static struct resource dm644x_v4l2_disp_resources[] = { + { + .start = IRQ_VENCINT, + .end = IRQ_VENCINT, + .flags = IORESOURCE_IRQ, + }, + { + .start = 0x01C72400,
Haven't you already declared resources with the same base address?
+ .end = 0x01C72400 + 0x180,
^^^^^ 0x17f?
+ .flags = IORESOURCE_MEM, + }, + +};
Empty line wouldn't hurt here...
+static struct platform_device dm644x_venc_dev = { + .name = VPBE_VENC_SUBDEV_NAME, + .id = -1, + .num_resources = ARRAY_SIZE(dm644x_venc_resources), + .resource = dm644x_venc_resources, + .dev = { + .dma_mask =&dm644x_venc_dma_mask, + .coherent_dma_mask = DMA_BIT_MASK(32), + .platform_data = (void *)&dm644x_venc_pdata,
There's no need to cast to 'void *' explicitly.
@@ -767,20 +903,36 @@ void __init dm644x_init(void)
[...]
+static int __init dm644x_init_video(void) +{ + /* Add ccdc clock aliases */ + clk_add_alias("master", dm644x_ccdc_dev.name, "vpss_master", NULL); + clk_add_alias("slave", dm644x_ccdc_dev.name, "vpss_slave", NULL); + vpss_clkctl_reg = ioremap_nocache(VPSS_CLKCTL, 4);
ioremap_nocache() can fail -- you should check the result. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html