Messages in this thread | | | Date | Tue, 30 Jul 2013 21:59:04 +0530 | From | Sekhar Nori <> | Subject | Re: [PATCH v2] ARM: EDMA: Fix clearing of unused list for DT DMA resources |
| |
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
| |