| Message ID | 1381231068-6053-3-git-send-email-sachin.kamat@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10/08/2013 05:17 AM, Sachin Kamat wrote:
> Local variables used only in this file are made static.
This might conflict with some of the cod re-org patches that Peter has
sent. Peter, can you please check. If it does, Peter may as well roll
this into his repost of all the Tegra clock patches...
On Tue, Oct 08, 2013 at 06:09:24PM +0200, Stephen Warren wrote: > On 10/08/2013 05:17 AM, Sachin Kamat wrote: > > Local variables used only in this file are made static. > Conceptually they are still exported. So I think it's counterintuitive to declare them static. Unless you expect namespace problems, I would rather leave it as is. Cheers, Peter.
On 10/10/2013 05:13 AM, Peter De Schrijver wrote: > On Tue, Oct 08, 2013 at 06:09:24PM +0200, Stephen Warren wrote: >> On 10/08/2013 05:17 AM, Sachin Kamat wrote: >>> Local variables used only in this file are made static. >> > > Conceptually they are still exported. So I think it's counterintuitive to > declare them static. Unless you expect namespace problems, I would rather > leave it as is. I forget exactly which symbols this patch changed, but presumably they're only exported via pointers rather than by symbol name, and isn't that exactly what static is for?
On Thu, Oct 10, 2013 at 06:07:38PM +0200, Stephen Warren wrote: > On 10/10/2013 05:13 AM, Peter De Schrijver wrote: > > On Tue, Oct 08, 2013 at 06:09:24PM +0200, Stephen Warren wrote: > >> On 10/08/2013 05:17 AM, Sachin Kamat wrote: > >>> Local variables used only in this file are made static. > >> > > > > Conceptually they are still exported. So I think it's counterintuitive to > > declare them static. Unless you expect namespace problems, I would rather > > leave it as is. > > I forget exactly which symbols this patch changed, but presumably > they're only exported via pointers rather than by symbol name, and isn't > that exactly what static is for? They are indeed exported using a pointer. depends on how you look at it I guess, but I see static as 'local to this file only' which isn't really true if you hand out pointers to others. Cheers, Peter.
On 10/14/2013 02:15 AM, Peter De Schrijver wrote: > On Thu, Oct 10, 2013 at 06:07:38PM +0200, Stephen Warren wrote: >> On 10/10/2013 05:13 AM, Peter De Schrijver wrote: >>> On Tue, Oct 08, 2013 at 06:09:24PM +0200, Stephen Warren wrote: >>>> On 10/08/2013 05:17 AM, Sachin Kamat wrote: >>>>> Local variables used only in this file are made static. >>>> >>> >>> Conceptually they are still exported. So I think it's counterintuitive to >>> declare them static. Unless you expect namespace problems, I would rather >>> leave it as is. >> >> I forget exactly which symbols this patch changed, but presumably >> they're only exported via pointers rather than by symbol name, and isn't >> that exactly what static is for? > > They are indeed exported using a pointer. depends on how you look at it I > guess, but I see static as 'local to this file only' which isn't really > true if you hand out pointers to others. Yes, static specifically means "the symbol name is local to this file". It says nothing about the data behind the symbol name.
On Mon, Oct 14, 2013 at 06:35:58PM +0200, Stephen Warren wrote: > On 10/14/2013 02:15 AM, Peter De Schrijver wrote: > > On Thu, Oct 10, 2013 at 06:07:38PM +0200, Stephen Warren wrote: > >> On 10/10/2013 05:13 AM, Peter De Schrijver wrote: > >>> On Tue, Oct 08, 2013 at 06:09:24PM +0200, Stephen Warren wrote: > >>>> On 10/08/2013 05:17 AM, Sachin Kamat wrote: > >>>>> Local variables used only in this file are made static. > >>>> > >>> > >>> Conceptually they are still exported. So I think it's counterintuitive to > >>> declare them static. Unless you expect namespace problems, I would rather > >>> leave it as is. > >> > >> I forget exactly which symbols this patch changed, but presumably > >> they're only exported via pointers rather than by symbol name, and isn't > >> that exactly what static is for? > > > > They are indeed exported using a pointer. depends on how you look at it I > > guess, but I see static as 'local to this file only' which isn't really > > true if you hand out pointers to others. > > Yes, static specifically means "the symbol name is local to this file". > It says nothing about the data behind the symbol name. I would still prefer the symbol visibility to have some relation with how the data is used.
On 10/15/2013 02:29 AM, Peter De Schrijver wrote: > On Mon, Oct 14, 2013 at 06:35:58PM +0200, Stephen Warren wrote: >> On 10/14/2013 02:15 AM, Peter De Schrijver wrote: >>> On Thu, Oct 10, 2013 at 06:07:38PM +0200, Stephen Warren wrote: >>>> On 10/10/2013 05:13 AM, Peter De Schrijver wrote: >>>>> On Tue, Oct 08, 2013 at 06:09:24PM +0200, Stephen Warren wrote: >>>>>> On 10/08/2013 05:17 AM, Sachin Kamat wrote: >>>>>>> Local variables used only in this file are made static. >>>>>> >>>>> >>>>> Conceptually they are still exported. So I think it's counterintuitive to >>>>> declare them static. Unless you expect namespace problems, I would rather >>>>> leave it as is. >>>> >>>> I forget exactly which symbols this patch changed, but presumably >>>> they're only exported via pointers rather than by symbol name, and isn't >>>> that exactly what static is for? >>> >>> They are indeed exported using a pointer. depends on how you look at it I >>> guess, but I see static as 'local to this file only' which isn't really >>> true if you hand out pointers to others. >> >> Yes, static specifically means "the symbol name is local to this file". >> It says nothing about the data behind the symbol name. > > I would still prefer the symbol visibility to have some relation with how > the data is used. No, if the *symbol* is not used, it should be static. There's absolutely zero benefit from having a visible symbol if it isn't used.
diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c index 197074a..76ed452 100644 --- a/drivers/clk/tegra/clk-pll.c +++ b/drivers/clk/tegra/clk-pll.c @@ -1340,7 +1340,7 @@ struct clk *tegra_clk_register_plle(const char *name, const char *parent_name, } #ifdef CONFIG_ARCH_TEGRA_114_SOC -const struct clk_ops tegra_clk_pllxc_ops = { +static const struct clk_ops tegra_clk_pllxc_ops = { .is_enabled = clk_pll_is_enabled, .enable = clk_pll_iddq_enable, .disable = clk_pll_iddq_disable, @@ -1349,7 +1349,7 @@ const struct clk_ops tegra_clk_pllxc_ops = { .set_rate = clk_pllxc_set_rate, }; -const struct clk_ops tegra_clk_pllm_ops = { +static const struct clk_ops tegra_clk_pllm_ops = { .is_enabled = clk_pll_is_enabled, .enable = clk_pll_iddq_enable, .disable = clk_pll_iddq_disable, @@ -1358,7 +1358,7 @@ const struct clk_ops tegra_clk_pllm_ops = { .set_rate = clk_pllm_set_rate, }; -const struct clk_ops tegra_clk_pllc_ops = { +static const struct clk_ops tegra_clk_pllc_ops = { .is_enabled = clk_pll_is_enabled, .enable = clk_pllc_enable, .disable = clk_pllc_disable, @@ -1367,7 +1367,7 @@ const struct clk_ops tegra_clk_pllc_ops = { .set_rate = clk_pllc_set_rate, }; -const struct clk_ops tegra_clk_pllre_ops = { +static const struct clk_ops tegra_clk_pllre_ops = { .is_enabled = clk_pll_is_enabled, .enable = clk_pll_iddq_enable, .disable = clk_pll_iddq_disable, @@ -1376,7 +1376,7 @@ const struct clk_ops tegra_clk_pllre_ops = { .set_rate = clk_pllre_set_rate, }; -const struct clk_ops tegra_clk_plle_tegra114_ops = { +static const struct clk_ops tegra_clk_plle_tegra114_ops = { .is_enabled = clk_pll_is_enabled, .enable = clk_plle_tegra114_enable, .disable = clk_plle_tegra114_disable,
Local variables used only in this file are made static. Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org> Cc: Stephen Warren <swarren@nvidia.com> --- drivers/clk/tegra/clk-pll.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)