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. Thanks, Sekhar -- 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