On Thursday, May 12, 2011 5:35 PM, Russell King wrote: > A few more points... >> +static int __init atmel_isi_probe(struct platform_device *pdev) > Should be __devinit otherwise you'll have section errors. Ok, will be fixed in V2 patch. >> +{ >> + unsigned int irq; >> + struct atmel_isi *isi; >> + struct clk *pclk; >> + struct resource *regs; >> + int ret; >> + struct device *dev = &pdev->dev; >> + struct isi_platform_data *pdata; >> + struct soc_camera_host *soc_host; >> + >> + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + if (!regs) >> + return -ENXIO; >> + >> + pclk = clk_get(&pdev->dev, "isi_clk"); >> + if (IS_ERR(pclk)) >> + return PTR_ERR(pclk); >> + >> + clk_enable(pclk); > Return value of clk_enable() should be checked. Yes. I will add code to check the return value in V2 patch. >> + >> + isi = kzalloc(sizeof(struct atmel_isi), GFP_KERNEL); >> + if (!isi) { >> + ret = -ENOMEM; >> [...] >> + isi_writel(isi, V2_CTRL, ISI_BIT(V2_DIS)); >> + /* Check if module disable */ >> + while (isi_readl(isi, V2_STATUS) & ISI_BIT(V2_DIS)) >> + cpu_relax(); >> + >> + irq = platform_get_irq(pdev, 0); > This should also be checked. Ditto, thank you. -- 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