From: Daniel Walker <dwalker@codeaurora.org>
To: Stephen Boyd <sboyd@codeaurora.org>
Cc: Russell King <linux@arm.linux.org.uk>,
Nicolas Pitre <nico@fluxnic.net>,
Kevin Hilman <khilman@deeprootsystems.com>,
linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
Saravana Kannan <skannan@codeaurora.org>,
Santosh Shilimkar <santosh.shilimkar@ti.com>,
Colin Cross <ccross@android.com>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCHv2 2/3] ARM: Allow machines to override __delay()
Date: Fri, 05 Nov 2010 16:43:12 -0700 [thread overview]
Message-ID: <1289000592.15381.23.camel@m0nster> (raw)
In-Reply-To: <4CD47C6E.3050702@codeaurora.org>
On Fri, 2010-11-05 at 14:51 -0700, Stephen Boyd wrote:
> On 11/04/2010 02:16 PM, Daniel Walker wrote:
> > On Thu, 2010-11-04 at 13:58 -0700, Stephen Boyd wrote:
> >> Wouldn't that mean delay_fn needs to be exposed in the header file too?
> >> I like the fact that it's static and scoped to this file.
> >
> > Yeah you would need to make an extern for that. Why is it better to have
> > it static in this file?
> >
> > If you make it inline you should have smaller code size since all your
> > doing is "delay_fn = fn;" and that's not expected to be used much (if at
> > all).
> >
>
> Ok. Doing that increases the size of my vmlinux.
>
> $ size vmlinux.orig vmlinux.new
> text data bss dec hex filename
> 7091426 594512 1244648 8930586 88451a vmlinux.orig
> 7091514 594512 1244648 8930674 884572 vmlinux.new
This is what I get,
text data bss dec hex filename
2168427 104288 186176 2458891 25850b ../build-test/vmlinux.orig
2168379 104288 186176 2458843 2584db ../build-test/vmlinux.new
Your patch has something wrong with it, which I fixed. Details below,
> Here's the interdiff.
>
> diff --git a/arch/arm/include/asm/delay.h b/arch/arm/include/asm/delay.h
> index 5c6b9a3..ef0ea48 100644
> --- a/arch/arm/include/asm/delay.h
> +++ b/arch/arm/include/asm/delay.h
> @@ -8,7 +8,20 @@
>
> #include <asm/param.h> /* HZ */
>
> -extern void __delay(unsigned long loops);
> +extern void (*delay_fn)(unsigned long);
> +
> +static inline void set_delay_fn(void (*fn)(unsigned long))
> +{
> + delay_fn = fn;
> +}
> +
> +/*
> + * loops = usecs * HZ * loops_per_jiffy / 1000000
> + */
> +static inline void __delay(unsigned long loops)
> +{
> + delay_fn(loops);
> +}
>
> /*
> * This function intentionally does not exist; if you see references to
> @@ -40,7 +53,6 @@ extern void __const_udelay(unsigned long);
> __const_udelay((n) * ((2199023U*HZ)>>11))) : \
> __udelay(n))
>
> -extern void set_delay_fn(void (*fn)(unsigned long));
> extern void read_current_timer_delay_loop(unsigned long loops);
>
> #endif /* defined(_ARM_DELAY_H) */
> diff --git a/arch/arm/lib/delay.c b/arch/arm/lib/delay.c
> index 59fdf64..a4b1ecd 100644
> --- a/arch/arm/lib/delay.c
> +++ b/arch/arm/lib/delay.c
> @@ -44,20 +44,8 @@ void read_current_timer_delay_loop(unsigned long loops)
> }
> #endif
>
> -static void (*delay_fn)(unsigned long) = delay_loop;
> +void (*delay_fn)(unsigned long) = delay_loop;
>
> -void set_delay_fn(void (*fn)(unsigned long))
> -{
> - delay_fn = fn;
> -}
> -
> -/*
> - * loops = usecs * HZ * loops_per_jiffy / 1000000
> - */
> -void __delay(unsigned long loops)
> -{
> - delay_fn(loops);
> -}
> EXPORT_SYMBOL(__delay);
You need to modify this EXPORT_SYMBOL to delay_fn since __delay doesn't
exist anymore.
Daniel
--
Sent by an consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum.
next prev parent reply other threads:[~2010-11-05 23:43 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-10-28 21:19 [PATCHv2 0/3] ARM: Fixing udelay() for SMP and non-SMP systems Stephen Boyd
2010-10-28 21:19 ` [PATCHv2 1/3] ARM: Translate delay.S into (mostly) C Stephen Boyd
2010-11-03 17:57 ` Daniel Walker
2010-11-04 2:40 ` Nicolas Pitre
2010-11-03 18:27 ` Will Deacon
2010-11-03 23:15 ` Stephen Boyd
2010-11-03 23:17 ` Daniel Walker
2010-10-28 21:19 ` [PATCHv2 2/3] ARM: Allow machines to override __delay() Stephen Boyd
2010-11-04 19:30 ` Daniel Walker
2010-11-04 20:58 ` Stephen Boyd
2010-11-04 21:16 ` Daniel Walker
2010-11-05 21:51 ` Stephen Boyd
2010-11-05 23:43 ` Daniel Walker [this message]
2010-11-06 3:36 ` Stephen Boyd
2010-11-08 18:11 ` Daniel Walker
2010-10-28 21:19 ` [PATCHv2 3/3] ARM: Implement a timer based __delay() loop Stephen Boyd
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=1289000592.15381.23.camel@m0nster \
--to=dwalker@codeaurora.org \
--cc=ccross@android.com \
--cc=khilman@deeprootsystems.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=nico@fluxnic.net \
--cc=santosh.shilimkar@ti.com \
--cc=sboyd@codeaurora.org \
--cc=skannan@codeaurora.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).