From: Hongbo Zhang <hongbo.zhang@freescale.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
ian.campbell@citrix.com, pawel.moll@arm.com,
Stephen Warren <swarren@wwwdotorg.org>,
"Koul, Vinod" <vinod.koul@intel.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
rob.herring@calxeda.com, dmaengine@vger.kernel.org,
linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH v11 3/3] DMA: Freescale: update driver to support 8-channel DMA engine
Date: Tue, 12 Nov 2013 15:05:43 +0800 [thread overview]
Message-ID: <5281D347.3020903@freescale.com> (raw)
In-Reply-To: <CAPcyv4h00zGc2=ZLwnCpP3WTQ8JbFteV-torU+qCo98YXrfq5A@mail.gmail.com>
On 11/12/2013 08:09 AM, Dan Williams wrote:
> On Mon, Nov 11, 2013 at 1:12 AM, Hongbo Zhang
> <hongbo.zhang@freescale.com> 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
>
WARNING: multiple messages have this Message-ID (diff)
From: Hongbo Zhang <hongbo.zhang-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
To: Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: "Koul,
Vinod" <vinod.koul-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org,
pawel.moll-5wv7dgnIgG8@public.gmane.org,
mark.rutland-5wv7dgnIgG8@public.gmane.org,
Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>,
ian.campbell-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linuxppc-dev
<linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org>,
Linux Kernel Mailing List
<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
dmaengine-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH v11 3/3] DMA: Freescale: update driver to support 8-channel DMA engine
Date: Tue, 12 Nov 2013 15:05:43 +0800 [thread overview]
Message-ID: <5281D347.3020903@freescale.com> (raw)
In-Reply-To: <CAPcyv4h00zGc2=ZLwnCpP3WTQ8JbFteV-torU+qCo98YXrfq5A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 11/12/2013 08:09 AM, Dan Williams wrote:
> On Mon, Nov 11, 2013 at 1:12 AM, Hongbo Zhang
> <hongbo.zhang-KZfg59tc24xl57MIdRCFDg@public.gmane.org> 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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: Hongbo Zhang <hongbo.zhang@freescale.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: "Koul, Vinod" <vinod.koul@intel.com>, <rob.herring@calxeda.com>,
<pawel.moll@arm.com>, <mark.rutland@arm.com>,
Stephen Warren <swarren@wwwdotorg.org>, <ian.campbell@citrix.com>,
<devicetree@vger.kernel.org>,
linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
<dmaengine@vger.kernel.org>
Subject: Re: [PATCH v11 3/3] DMA: Freescale: update driver to support 8-channel DMA engine
Date: Tue, 12 Nov 2013 15:05:43 +0800 [thread overview]
Message-ID: <5281D347.3020903@freescale.com> (raw)
In-Reply-To: <CAPcyv4h00zGc2=ZLwnCpP3WTQ8JbFteV-torU+qCo98YXrfq5A@mail.gmail.com>
On 11/12/2013 08:09 AM, Dan Williams wrote:
> On Mon, Nov 11, 2013 at 1:12 AM, Hongbo Zhang
> <hongbo.zhang@freescale.com> 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
>
next prev parent reply other threads:[~2013-11-12 7:06 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-26 9:33 [PATCH v11 0/3] DMA: Freescale: Add support for 8-channel DMA engine hongbo.zhang
2013-09-26 9:33 ` hongbo.zhang
2013-09-26 9:33 ` hongbo.zhang-KZfg59tc24xl57MIdRCFDg
2013-09-26 9:33 ` [PATCH v11 1/3] DMA: Freescale: revise device tree binding document hongbo.zhang
2013-09-26 9:33 ` hongbo.zhang
2013-09-26 9:33 ` hongbo.zhang
2013-09-26 9:33 ` [PATCH v11 2/3] DMA: Freescale: Add new 8-channel DMA engine device tree nodes hongbo.zhang
2013-09-26 9:33 ` hongbo.zhang
2013-09-26 9:33 ` hongbo.zhang
2013-10-08 3:22 ` Hongbo Zhang
2013-10-08 3:22 ` Hongbo Zhang
2013-10-08 3:22 ` Hongbo Zhang
2013-10-15 13:38 ` Mark Rutland
2013-10-15 13:38 ` Mark Rutland
2013-10-15 13:38 ` Mark Rutland
2013-10-16 1:59 ` Hongbo Zhang
2013-10-16 1:59 ` Hongbo Zhang
2013-09-26 9:33 ` [PATCH v11 3/3] DMA: Freescale: update driver to support 8-channel DMA engine hongbo.zhang
2013-09-26 9:33 ` hongbo.zhang
2013-09-26 9:33 ` hongbo.zhang
2013-10-17 5:56 ` Hongbo Zhang
2013-10-17 5:56 ` Hongbo Zhang
2013-10-17 5:56 ` Hongbo Zhang
2013-11-05 2:31 ` Hongbo Zhang
2013-11-05 2:31 ` Hongbo Zhang
2013-11-05 2:31 ` Hongbo Zhang
2013-11-08 2:45 ` Dan Williams
2013-11-08 2:45 ` Dan Williams
2013-11-08 2:45 ` Dan Williams
2013-11-11 9:12 ` Hongbo Zhang
2013-11-11 9:12 ` Hongbo Zhang
2013-11-11 9:12 ` Hongbo Zhang
2013-11-12 0:09 ` Dan Williams
2013-11-12 0:09 ` Dan Williams
2013-11-12 0:09 ` Dan Williams
2013-11-12 7:05 ` Hongbo Zhang [this message]
2013-11-12 7:05 ` Hongbo Zhang
2013-11-12 7:05 ` Hongbo Zhang
2013-11-13 8:57 ` [PATCH v11 0/3] DMA: Freescale: Add support for " Vinod Koul
2013-11-13 8:57 ` Vinod Koul
2013-11-13 8:57 ` Vinod Koul
2013-11-13 9:54 ` Hongbo Zhang
2013-11-13 9:54 ` Hongbo Zhang
2013-11-13 9:54 ` Hongbo Zhang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5281D347.3020903@freescale.com \
--to=hongbo.zhang@freescale.com \
--cc=dan.j.williams@intel.com \
--cc=devicetree@vger.kernel.org \
--cc=dmaengine@vger.kernel.org \
--cc=ian.campbell@citrix.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mark.rutland@arm.com \
--cc=pawel.moll@arm.com \
--cc=rob.herring@calxeda.com \
--cc=swarren@wwwdotorg.org \
--cc=vinod.koul@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.