From: Joel Fernandes <joelf@ti.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Linux OMAP List <linux-omap@vger.kernel.org>,
"nsekhar@ti.com" <nsekhar@ti.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Linux ARM Kernel List <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] ARM: EDMA: Use platform_get_resource functions for DT
Date: Fri, 21 Feb 2014 11:36:27 -0600 [thread overview]
Message-ID: <53078E9B.5010806@ti.com> (raw)
In-Reply-To: <20140221121548.GF8783@e106331-lin.cambridge.arm.com>
On 02/21/2014 06:15 AM, Mark Rutland wrote:
>> Also, while at it get rid of the assumption in the code that "CC" is at reg
>> index 0 in the DT and xbar is at offset 1. Instead use reg-names to get the
>> memory resource in concern keeping things much cleaner and simpler. This also
>> makes it possible to have multiple channel controllers.
>
> While this is nice I think we have to have a fallback to the existing
> behaviour if there's no reg-names property present, unless we know for
> certain no-one is possibly using an existing DTB.
Yes, its true it break existing DTBs but currently only 2 TI SoCs use EDMA this
way, the vast majority of EDMA users are yet to follow where we can do it right.
Further, the old bindings are really limiting specially the 2 CC case and if
additionally memory maps are used in the future. So keeping the old binding is
limiting in this regard.
Here is what the platform_data used to look like when used by mach-davinci:
static struct resource da850_edma_resources[] = {
{
.name = "edma_cc0",
.start = DA8XX_TPCC_BASE,
.end = DA8XX_TPCC_BASE + SZ_32K - 1,
.flags = IORESOURCE_MEM,
},
{
.name = "edma_tc0",
.start = DA8XX_TPTC0_BASE,
.end = DA8XX_TPTC0_BASE + SZ_1K - 1,
.flags = IORESOURCE_MEM,
},
{
.name = "edma_tc1",
.start = DA8XX_TPTC1_BASE,
.end = DA8XX_TPTC1_BASE + SZ_1K - 1,
.flags = IORESOURCE_MEM,
},
{
.name = "edma_cc1",
.start = DA850_TPCC1_BASE,
.end = DA850_TPCC1_BASE + SZ_32K - 1,
.flags = IORESOURCE_MEM,
},
As you can see, there are several memory maps and different interpretations.
Considering this, IMO- it makes sense to pay a small price to keep the semantics
sane.
On the other hand, the other 2 options are:
1. We add a fallback if reg-names look up fails.
2. We inject reg-names property into edma DT nodes that don't have them, and
make sure all future dtsi with edma nodes mention the reg-names property.
These 2 are a bit error prone though, for example if someone deliberately
forgets to mention reg-names, and the code still works, but misbehaves in some way.
Regards,
-Joel
WARNING: multiple messages have this Message-ID (diff)
From: joelf@ti.com (Joel Fernandes)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: EDMA: Use platform_get_resource functions for DT
Date: Fri, 21 Feb 2014 11:36:27 -0600 [thread overview]
Message-ID: <53078E9B.5010806@ti.com> (raw)
In-Reply-To: <20140221121548.GF8783@e106331-lin.cambridge.arm.com>
On 02/21/2014 06:15 AM, Mark Rutland wrote:
>> Also, while at it get rid of the assumption in the code that "CC" is at reg
>> index 0 in the DT and xbar is at offset 1. Instead use reg-names to get the
>> memory resource in concern keeping things much cleaner and simpler. This also
>> makes it possible to have multiple channel controllers.
>
> While this is nice I think we have to have a fallback to the existing
> behaviour if there's no reg-names property present, unless we know for
> certain no-one is possibly using an existing DTB.
Yes, its true it break existing DTBs but currently only 2 TI SoCs use EDMA this
way, the vast majority of EDMA users are yet to follow where we can do it right.
Further, the old bindings are really limiting specially the 2 CC case and if
additionally memory maps are used in the future. So keeping the old binding is
limiting in this regard.
Here is what the platform_data used to look like when used by mach-davinci:
static struct resource da850_edma_resources[] = {
{
.name = "edma_cc0",
.start = DA8XX_TPCC_BASE,
.end = DA8XX_TPCC_BASE + SZ_32K - 1,
.flags = IORESOURCE_MEM,
},
{
.name = "edma_tc0",
.start = DA8XX_TPTC0_BASE,
.end = DA8XX_TPTC0_BASE + SZ_1K - 1,
.flags = IORESOURCE_MEM,
},
{
.name = "edma_tc1",
.start = DA8XX_TPTC1_BASE,
.end = DA8XX_TPTC1_BASE + SZ_1K - 1,
.flags = IORESOURCE_MEM,
},
{
.name = "edma_cc1",
.start = DA850_TPCC1_BASE,
.end = DA850_TPCC1_BASE + SZ_32K - 1,
.flags = IORESOURCE_MEM,
},
As you can see, there are several memory maps and different interpretations.
Considering this, IMO- it makes sense to pay a small price to keep the semantics
sane.
On the other hand, the other 2 options are:
1. We add a fallback if reg-names look up fails.
2. We inject reg-names property into edma DT nodes that don't have them, and
make sure all future dtsi with edma nodes mention the reg-names property.
These 2 are a bit error prone though, for example if someone deliberately
forgets to mention reg-names, and the code still works, but misbehaves in some way.
Regards,
-Joel
WARNING: multiple messages have this Message-ID (diff)
From: Joel Fernandes <joelf@ti.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Linux OMAP List <linux-omap@vger.kernel.org>,
Linux ARM Kernel List <linux-arm-kernel@lists.infradead.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
"nsekhar@ti.com" <nsekhar@ti.com>
Subject: Re: [PATCH] ARM: EDMA: Use platform_get_resource functions for DT
Date: Fri, 21 Feb 2014 11:36:27 -0600 [thread overview]
Message-ID: <53078E9B.5010806@ti.com> (raw)
In-Reply-To: <20140221121548.GF8783@e106331-lin.cambridge.arm.com>
On 02/21/2014 06:15 AM, Mark Rutland wrote:
>> Also, while at it get rid of the assumption in the code that "CC" is at reg
>> index 0 in the DT and xbar is at offset 1. Instead use reg-names to get the
>> memory resource in concern keeping things much cleaner and simpler. This also
>> makes it possible to have multiple channel controllers.
>
> While this is nice I think we have to have a fallback to the existing
> behaviour if there's no reg-names property present, unless we know for
> certain no-one is possibly using an existing DTB.
Yes, its true it break existing DTBs but currently only 2 TI SoCs use EDMA this
way, the vast majority of EDMA users are yet to follow where we can do it right.
Further, the old bindings are really limiting specially the 2 CC case and if
additionally memory maps are used in the future. So keeping the old binding is
limiting in this regard.
Here is what the platform_data used to look like when used by mach-davinci:
static struct resource da850_edma_resources[] = {
{
.name = "edma_cc0",
.start = DA8XX_TPCC_BASE,
.end = DA8XX_TPCC_BASE + SZ_32K - 1,
.flags = IORESOURCE_MEM,
},
{
.name = "edma_tc0",
.start = DA8XX_TPTC0_BASE,
.end = DA8XX_TPTC0_BASE + SZ_1K - 1,
.flags = IORESOURCE_MEM,
},
{
.name = "edma_tc1",
.start = DA8XX_TPTC1_BASE,
.end = DA8XX_TPTC1_BASE + SZ_1K - 1,
.flags = IORESOURCE_MEM,
},
{
.name = "edma_cc1",
.start = DA850_TPCC1_BASE,
.end = DA850_TPCC1_BASE + SZ_32K - 1,
.flags = IORESOURCE_MEM,
},
As you can see, there are several memory maps and different interpretations.
Considering this, IMO- it makes sense to pay a small price to keep the semantics
sane.
On the other hand, the other 2 options are:
1. We add a fallback if reg-names look up fails.
2. We inject reg-names property into edma DT nodes that don't have them, and
make sure all future dtsi with edma nodes mention the reg-names property.
These 2 are a bit error prone though, for example if someone deliberately
forgets to mention reg-names, and the code still works, but misbehaves in some way.
Regards,
-Joel
next prev parent reply other threads:[~2014-02-21 17:36 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-21 4:24 [PATCH] ARM: EDMA: Use platform_get_resource functions for DT Joel Fernandes
2014-02-21 4:24 ` Joel Fernandes
2014-02-21 4:24 ` Joel Fernandes
2014-02-21 12:15 ` Mark Rutland
2014-02-21 12:15 ` Mark Rutland
2014-02-21 12:15 ` Mark Rutland
2014-02-21 17:36 ` Joel Fernandes [this message]
2014-02-21 17:36 ` Joel Fernandes
2014-02-21 17:36 ` Joel Fernandes
2014-02-25 16:22 ` Rob Herring
2014-02-25 16:22 ` Rob Herring
2014-02-25 16:42 ` Joel Fernandes
2014-02-25 16:42 ` Joel Fernandes
2014-02-21 13:24 ` Russell King - ARM Linux
2014-02-21 13:24 ` Russell King - ARM Linux
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=53078E9B.5010806@ti.com \
--to=joelf@ti.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=nsekhar@ti.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.