On 07/30/2013 11:29 AM, Sekhar Nori wrote: > On 7/30/2013 9:17 AM, Joel Fernandes wrote: > >>>> diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c >>>> index a432e6c..765d578 100644 >>>> --- a/arch/arm/common/edma.c >>>> +++ b/arch/arm/common/edma.c > >>>> + } else { >>>> + for (; i < pdev->num_resources; i++) { >>>> + if ((pdev->resource[i].flags & IORESOURCE_DMA) && >>>> + (int)pdev->resource[i].start >= 0) { >>>> + ctlr = EDMA_CTLR(pdev->resource[i].start); >>>> + clear_bit(EDMA_CHAN_SLOT( >>>> + pdev->resource[i].start), >>>> + edma_cc[ctlr]->edma_unused); >>>> + } >>> >>> So there is very little in common between OF and non-OF versions of this >>> function. Why not have two different versions of this function for the >>> two cases? The OF version can reside under the CONFIG_OF conditional >>> already in use in the file. This will also save you the ugly line breaks >>> you had to resort to due to too deep indentation. >> >> Actually those line breaks are not necessary and wouldn't result in >> compilation errors. I was planning to drop them. I'll go ahead and split >> it out anyway, now that also the OF version of the function is going to >> be bit longer if we use the of_parse functions. >> >> Thanks for your review, > > It turns out, I gave a bad idea. What I suggested will break the case of > non-DT boot with CONFIG_OF enabled. So what you had was fine. May be > just return from "if (dev->of_node)" so you don't need to have an else > block and can save on the indentation.> Ok, sure. I will go ahead and return from the if block. Thanks, -Joel -- 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