From: khilman@deeprootsystems•com (Kevin Hilman)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v3 01/13] OMAP: DMA: Replace read/write macros with functions
Date: Wed, 10 Nov 2010 09:35:41 -0800 [thread overview]
Message-ID: <87lj51axoy.fsf@deeprootsystems.com> (raw)
In-Reply-To: <E0D41E29EB0DAC4E9F3FF173962E9E9402DC1A7D7B@dbde02.ent.ti.com> (Manjunath Kondaiah G.'s message of "Wed, 10 Nov 2010 22:46:04 +0530")
"G, Manjunath Kondaiah" <manjugk@ti•com> writes:
>> -----Original Message-----
>> From: Kevin Hilman [mailto:khilman at deeprootsystems.com]
>> Sent: Wednesday, November 10, 2010 9:33 PM
>> To: G, Manjunath Kondaiah
>> Cc: linux-omap at vger.kernel.org;
>> linux-arm-kernel at lists.infradead.org; Cousson, Benoit;
>> Shilimkar, Santosh
>> Subject: Re: [PATCH v3 01/13] OMAP: DMA: Replace read/write
>> macros with functions
>>
>> "G, Manjunath Kondaiah" <manjugk@ti•com> writes:
>>
>> [...]
>>
>> >> > + if (reg > OMAP1_CH_COMMON_START)
>> >> > + __raw_writew(val, dma_base +
>> >> > + (reg_map_omap1[reg] +
>> 0x40 * lch));
>> >> > + else
>> >> > + __raw_writew(val, dma_base +
>> >> reg_map_omap1[reg]);
>> >> > + } else {
>> >> > + if (reg > OMAP2_CH_COMMON_START)
>> >> > + __raw_writel(val, dma_base +
>> >> > + (reg_map_omap2[reg] +
>> 0x60 * lch));
>> >> > + else
>> >> > + __raw_writel(val, dma_base +
>> >> reg_map_omap2[reg]);
>> >> > + }
>> >> > +}
>> >>
>> >> The register base offset, register array and the stride (offset
>> >> between instances: 0x40 or 0x60) are detectable at init time, and
>> >> there's no reason to have conditional code for them. IOW, they
>> >> should be init time constants. Doing so would greatly simply these
>> >> functions. In fact the CH_COMMON_START could also be an init time
>> >> constant as well. So, given the following init_time constants:
>> >> dma_ch_common_start, dma_stride, reg_map, the above would
>> be reduced
>> >> to something actually worth inlining, for example (not actually
>> >> tested):
>> >>
>> >> static inline void dma_write(u32 val, int reg, int lch)
>> >> {
>> >> u8 stride = (reg > dma_ch_common_start) ? dma_stride : 0;
>> >>
>> >> __raw_writel(val, dma_base + (reg_map[reg] +
>> stride * lch));
>> >> }
>> >>
>> >> Same applies to dma_read().
>> >
>> > Thanks for the suggestion. It is taken care along with
>> Tony's comment
>> > for handling these read/write's between omap1 and omap2+.
>> >
>> > Here is code snippet for handling both omap1(includes 16 bit
>> > registers) and omap2+
>> >
>> > static inline void dma_write(u32 val, int reg, int lch)
>> > {
>> > u8 stride;
>> > u32 offset;
>> >
>> > stride = (reg >= dma_common_ch_start) ? dma_stride : 0;
>> > offset = reg_map[reg] + (stride * lch);
>> >
>> > if (dma_stride == 0x40) {
>>
>> The use of hard-coded constants still isn't right here. This is
>> bascally the same as a cpu_is_omap1 check. After you separate out the
>> device files, I thought you had separate omap1 and omap2+ versions of
>> these, so I don't follow the need for this.
>
> This code will be moved to respective mach-omapx dma files and this
> conditional check vanishes automatically in PATCH 10/13. Since this patch
> targets replacing read/write macros with inline functions, no functionality
> changes(except change in logic for handling 16bit registers for omap1)
> are done with new patch hence handling omap1 and omap2+ is
> done this way.
>
> I hope having the conditional check in this interim patch is ok.
Personally, I would rather not have an intermediate step, but if it
makes the series smoother, I guess it's OK.
Kevin
next prev parent reply other threads:[~2010-11-10 17:35 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-10-26 13:25 [PATCH v3 00/13] OMAP: DMA: hwmod and DMA as platform device G, Manjunath Kondaiah
2010-10-26 13:25 ` [PATCH v3 01/13] OMAP: DMA: Replace read/write macros with functions G, Manjunath Kondaiah
2010-10-26 14:48 ` Nishanth Menon
2010-10-27 3:54 ` G, Manjunath Kondaiah
2010-10-27 14:26 ` Menon, Nishanth
2010-10-29 8:15 ` G, Manjunath Kondaiah
2010-11-09 21:37 ` Kevin Hilman
2010-11-10 14:01 ` G, Manjunath Kondaiah
2010-11-10 16:03 ` Kevin Hilman
2010-11-10 17:16 ` G, Manjunath Kondaiah
2010-11-10 17:35 ` Kevin Hilman [this message]
2010-10-26 13:25 ` [PATCH v3 02/13] OMAP: DMA: Introduce errata handling feature G, Manjunath Kondaiah
2010-11-09 22:12 ` Kevin Hilman
2010-11-10 14:02 ` G, Manjunath Kondaiah
2010-11-10 16:26 ` Kevin Hilman
2010-11-10 17:39 ` G, Manjunath Kondaiah
2010-10-26 13:25 ` [PATCH v3 03/13] OMAP: DMA: Introduce DMA device attributes G, Manjunath Kondaiah
2010-11-09 21:46 ` Kevin Hilman
2010-10-26 13:25 ` [PATCH v3 04/13] OMAP2420: DMA: hwmod: add system DMA G, Manjunath Kondaiah
2010-11-09 23:13 ` Kevin Hilman
2010-11-11 23:04 ` Kevin Hilman
2010-10-26 13:25 ` [PATCH v3 05/13] OMAP2430: " G, Manjunath Kondaiah
2010-10-26 13:25 ` [PATCH v3 06/13] OMAP3: " G, Manjunath Kondaiah
2010-11-03 12:59 ` G, Manjunath Kondaiah
2010-11-04 4:29 ` Cousson, Benoit
2010-11-04 7:01 ` G, Manjunath Kondaiah
2010-11-04 12:30 ` Cousson, Benoit
2010-11-04 15:48 ` Kevin Hilman
2010-10-26 13:25 ` [PATCH v3 07/13] OMAP4: " G, Manjunath Kondaiah
2010-10-26 13:25 ` [PATCH v3 08/13] OMAP1: DMA: Introduce DMA driver as platform device G, Manjunath Kondaiah
2010-11-09 22:23 ` Kevin Hilman
2010-11-10 14:02 ` G, Manjunath Kondaiah
2010-10-26 13:25 ` [PATCH v3 09/13] OMAP2+: DMA: hwmod: Device registration G, Manjunath Kondaiah
2010-10-26 13:25 ` [PATCH v3 10/13] OMAP: DMA: Convert DMA library into DMA platform Driver G, Manjunath Kondaiah
2010-11-09 22:26 ` Kevin Hilman
2010-11-10 14:02 ` G, Manjunath Kondaiah
2010-11-10 16:24 ` Kevin Hilman
2010-11-10 17:23 ` G, Manjunath Kondaiah
2010-11-10 17:56 ` Kevin Hilman
2010-11-09 23:29 ` Kevin Hilman
2010-11-10 14:02 ` G, Manjunath Kondaiah
2010-10-26 13:25 ` [PATCH v3 11/13] OMAP: DMA: Use DMA device attributes G, Manjunath Kondaiah
2010-10-26 13:25 ` [PATCH v3 12/13] OMAP2+: DMA: descriptor autoloading feature G, Manjunath Kondaiah
2010-10-26 13:25 ` [PATCH v3 13/13] OMAP: PM: DMA: Enable runtime pm G, Manjunath Kondaiah
2010-11-09 22:48 ` Kevin Hilman
2010-11-10 14:02 ` G, Manjunath Kondaiah
2010-11-10 16:14 ` Kevin Hilman
2010-10-27 4:57 ` [PATCH v3 00/13] OMAP: DMA: hwmod and DMA as platform device G, Manjunath Kondaiah
2010-11-09 23:11 ` Kevin Hilman
2010-11-10 14:02 ` G, Manjunath Kondaiah
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=87lj51axoy.fsf@deeprootsystems.com \
--to=khilman@deeprootsystems$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox