Re: [PATCH v11 3/3] DMA: Freescale: update driver to support 8-channelDMA engine

From: Hongbo Zhang
Date: Tue Nov 12 2013 - 02:06:19 EST


On 11/12/2013 08:09 AM, Dan Williams wrote:
On Mon, Nov 11, 2013 at 1:12 AM, Hongbo Zhang
<hongbo.zhang@xxxxxxxxxxxxx> wrote:
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 49e8fbd..16a9a48 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1261,7 +1261,9 @@ static int fsl_dma_chan_probe(struct
fsldma_device
*fdev,
WARN_ON(fdev->feature != chan->feature);
chan->dev = fdev->dev;
- chan->id = ((res.start - 0x100) & 0xfff) >> 7;
+ chan->id = (res.start & 0xfff) < 0x300 ?
+ ((res.start - 0x100) & 0xfff) >> 7 :
+ ((res.start - 0x200) & 0xfff) >> 7;
if (chan->id >= FSL_DMA_MAX_CHANS_PER_DEVICE) {
Isn't it a bit fragile to have this based on the resource address?
Can't device tree tell you the channel id directly by an index into
the "dma0: dma@100300" node?

Yes, both this way and putting a "cell-index" into device tree work.
This won't be fragile, because the resource address should always be defined
correctly, otherwise even if we can tell a channel id by "cell-index" but
with wrong resource address, nothing will work.
This piece of code only doesn't seem as neat as using "cell-index", but we
prefer the style that let the device tree describes as true as what hardware
really has. This doesn't mean "cell-index" isn't acceptable, if it is
necessary and unavoidable, we can send another patch to add it, but
currently there is no need and we don't have to do this.

I'm pointing it out because we just had a bug fix to another driver
motivated by the fact that resource addresses may move from one
implementation to another, whereas the cell index provided by device
tree is static. Just a note, no need to fix it now.

Get it, and will remember it, thank you Dan.
--
Dan




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/