public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: khilman@ti•com (Kevin Hilman)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH-V3 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param
Date: Mon, 23 Apr 2012 17:32:57 -0700	[thread overview]
Message-ID: <87ehreezpi.fsf@ti.com> (raw)
In-Reply-To: <79CD15C6BA57404B839C016229A409A83E9F9AD0@DBDE01.ent.ti.com> (Vaibhav Hiremath's message of "Mon, 23 Apr 2012 17:35:19 +0000")

"Hiremath, Vaibhav" <hvaibhav@ti•com> writes:

> On Fri, Apr 20, 2012 at 06:04:20, Hilman, Kevin wrote:

[...]

>> Also note that because you're adding an offset of 0x10 to the start, the
>> ioremap later is actually mapping 0x10 past the end.  More on the base
>> address later...
>> 
>
> Comment on top of this code will enough, isn't it?
>

No.  ioremapping() the range that goes past the end of the device is
just wrong.  But if you fix the offset as described below, that would be
fine.

[...]

>> > @@ -68,54 +66,43 @@ void read_persistent_clock(struct timespec *ts)
>> >  	*ts = *tsp;
>> >  }
>> >
>> > -int __init omap_init_clocksource_32k(void)
>> > +int __init omap_init_clocksource_32k(u32 pbase, unsigned long size)
>> 
>> Now that this takes some arguments, this function wants some kerneldoc.
>> 
>
> Should I create new documentation inside Documentation/arm/OMAP/
> with name "counter_32k"?

by kerneldoc, I mean comments in the code, not in Documenation.

IOW, just add a comment before this function in the code, but make sure
it is of the format described in Documentation/kernel-doc-nano-HOWTO.txt.

>> In particular, you should document that 'pbase' is the address of the
>> counter register, not the base of the IP.
>> 
>> That being said, I think the pbase passed in should probably be the base
>> of the IP register, not the address of the counter register. It seems to
>> me that it's at offset 0x10 on all SoCs anyways.
>> 
>> Doing that will also ensure that the ioremap'd range covers the whole
>> IP, and not from the counter register to 0x10 past the end.
>> 
>
> Yeah, you are right, I can get rid of this offset and move it to 
> counter_32k.c file.

Perfect.

Kevin

  reply	other threads:[~2012-04-24  0:32 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-13  9:39 [PATCH-V3 0/3] ARM: OMAP: Make OMAP clocksource source selection runtime Vaibhav Hiremath
2012-04-13  9:39 ` [PATCH-V3 1/3] ARM: OMAP2/3: Add idle_st bits for ST_32KSYNC timer to prcm-common header Vaibhav Hiremath
2012-04-13  9:39 ` [PATCH-V3 2/3] ARM: OMAP2/3: hwmod data: Add 32k-sync timer data to hwmod database Vaibhav Hiremath
2012-04-13  9:39 ` [PATCH-V3 3/3] ARM: OMAP: Make OMAP clocksource source selection using kernel param Vaibhav Hiremath
2012-04-20  0:34   ` Kevin Hilman
2012-04-23 17:35     ` Hiremath, Vaibhav
2012-04-24  0:32       ` Kevin Hilman [this message]
2012-04-16  8:40 ` [PATCH-V3 0/3] ARM: OMAP: Make OMAP clocksource source selection runtime Santosh Shilimkar

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=87ehreezpi.fsf@ti.com \
    --to=khilman@ti$(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