On Tue, Nov 24, 2015 at 11:15 AM, Scott Wood <scottwood@xxxxxxxxxxxxx> wrote: > On Tue, 2015-11-24 at 11:12 -0500, Timur Tabi wrote: >> On Tue, Nov 24, 2015 at 1:27 AM, Dongsheng Wang >> <dongsheng.wang@xxxxxxxxxxxxx> wrote: >> > @@ -1697,6 +1700,9 @@ static int fsl_diu_probe(struct platform_device >> > *pdev) >> > unsigned int i; >> > int ret; >> > >> > + if (!diu_ops.set_pixel_clock) >> > + return -ENODEV; >> > + >> > data = dmam_alloc_coherent(&pdev->dev, sizeof(struct >> > fsl_diu_data), >> > &dma_addr, GFP_DMA | __GFP_ZERO); >> > if (!data) >> >> This doesn't make any sense. If set_pixel_clock() is not defined, >> then the whole driver aborts the probe. > > That's what this patch is trying to accomplish. Currently it crashes instead. > >> When could that ever happen? >> If the platform code does not exist, then don't let the driver be >> probed. > > How do you propose to accomplish that other than with such a check? Well, if you're concern is that there's no platform code, then there should be a check that says, "see if there's any platform code", not "let's check this obscure function and abort without explanation if it's not initialized." Alternatively, why can't you just do this, in update_lcdc(): > > -Scott > > -- > To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html