Skip to contentSkip to conversation

clk: exynos4: Make exynos4_plls static

PATCH New Sachin Kamat 4 participants 8 messages Aug 6, 2013 – Aug 7, 2013

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/3 clk: exynos4: Make exynos4_plls static

'exynos4_plls' is used only in this file. Make it static.

---
 drivers/clk/samsung/clk-exynos4.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/clk/samsung/clk-exynos4.c +1 -1
index 68f9a4a..fec319d 100644
@@ -984,7 +984,7 @@ static __initdata struct of_device_id ext_clk_match[] = {
 	{},
 };
 
-struct __initdata samsung_pll_clock exynos4_plls[nr_plls] = {
+static struct __initdata samsung_pll_clock exynos4_plls[nr_plls] = {
 	[apll] = PLL_A(pll_35xx, fout_apll, "fout_apll", "fin_pll", APLL_LOCK,
 		APLL_CON0, "fout_apll", NULL),
 	[mpll] = PLL_A(pll_35xx, fout_mpll, "fout_mpll", "fin_pll",
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH 2/3 clk: exynos5250: Make exynos5250_plls static

exynos5250_plls is used only in this file. Make it static.

---
 drivers/clk/samsung/clk-exynos5250.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/clk/samsung/clk-exynos5250.c +1 -1
index df3628c..5865cef 100644
@@ -517,7 +517,7 @@ static __initdata struct samsung_pll_rate_table epll_24mhz_tbl[] = {
 	{ },
 };
 
-struct __initdata samsung_pll_clock exynos5250_plls[nr_plls] = {
+static struct __initdata samsung_pll_clock exynos5250_plls[nr_plls] = {
 	[apll] = PLL_A(pll_35xx, fout_apll, "fout_apll", "fin_pll", APLL_LOCK,
 		APLL_CON0, "fout_apll", NULL),
 	[mpll] = PLL_A(pll_35xx, fout_mpll, "fout_mpll", "fin_pll", MPLL_LOCK,
Signature · 1 line
1.7.9.5
#2 lore raw

PATCH 3/3 clk: exynos5420: Make exynos5420_plls static

In reply to Sachin Kamat

'exynos5420_plls' is used only in this file. Make is static.

---
 drivers/clk/samsung/clk-exynos5420.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/clk/samsung/clk-exynos5420.c +1 -1
index ca352695..872e13d 100644
@@ -727,7 +727,7 @@ static struct samsung_gate_clock exynos5420_gate_clks[] __initdata = {
 	GATE(smmu_mscl2, "smmu_mscl2", "aclk400_mscl", GATE_IP_MSCL, 10, 0, 0),
 };
 
-struct __initdata samsung_pll_clock exynos5420_plls[nr_plls] = {
+static struct __initdata samsung_pll_clock exynos5420_plls[nr_plls] = {
 	[apll] = PLL(pll_2550, fout_apll, "fout_apll", "fin_pll", APLL_LOCK,
 		APLL_CON0, NULL),
 	[cpll] = PLL(pll_2550, fout_mpll, "fout_mpll", "fin_pll", MPLL_LOCK,
Signature · 1 line
1.7.9.5
#3 lore raw

PATCH 1/3 clk: exynos4: Make exynos4_plls static

In reply to Sachin Kamat

Quoting Sachin Kamat (2013-08-06 04:31:13)

Quoted 3 lines

'exynos4_plls' is used only in this file. Make it static.

Taken all three into clk-next. Seems like a lot of static-ization patches for Exynos lately.

Regards,
Mike

Quoted 19 lines
---
 drivers/clk/samsung/clk-exynos4.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/clk/samsung/clk-exynos4.c +1 -1
index 68f9a4a..fec319d 100644
@@ -984,7 +984,7 @@ static __initdata struct of_device_id ext_clk_match[] = {
        {},
 };
 
-struct __initdata samsung_pll_clock exynos4_plls[nr_plls] = {
+static struct __initdata samsung_pll_clock exynos4_plls[nr_plls] = {
        [apll] = PLL_A(pll_35xx, fout_apll, "fout_apll", "fin_pll", APLL_LOCK,
                APLL_CON0, "fout_apll", NULL),
        [mpll] = PLL_A(pll_35xx, fout_mpll, "fout_mpll", "fin_pll",
Signature · 1 line
1.7.9.5
#4 lore raw

PATCH 1/3 clk: exynos4: Make exynos4_plls static

In reply to Sachin Kamat

Also note:

Review static __initdata struct of_device_id ext_clk_match[] = {

Quoted 1 line · Sachin Kamat
(quoted diff) +0 -0
@@ -984,7 +984,7 @@ static __initdata struct of_device_id ext_clk_match[] = {

For the declaration above...

drivers/clk/samsung/clk-exynos4.c static __initdata struct of_device_id ext_clk_match[] = { · line 987

Quoted 2 lines
drivers/clk/samsung/clk-exynos4.c +1 -1
-struct __initdata samsung_pll_clock exynos4_plls[nr_plls] = {
+static struct __initdata samsung_pll_clock exynos4_plls[nr_plls] = {

And this one... __initdata should come just before the '=', not at the start, not in the middle and not before the variable.

The reasoning is that with how you have it above, the attributes are applied to the structure. You want to apply the attributes to the declaration instead, so it should come after the variable name.

So, for example:

	struct foo *foo __attribute__((section(".foo"))) = (void *)1;

will place the "foo" variable into a section called ".foo", but:

	struct __attribute__((section(".foo"))) foo *foo = (void *)1;

will place "foo" into the normal .data section.

So, the rule with variable declarations is that the __ specifiers we have as macros in the kernel always come after the variable name being declared and nowhere else. We consider anywhere else buggy.

#5 lore raw

PATCH 1/3 clk: exynos4: Make exynos4_plls static

+CC Joe Perches

Quoted 23 lines · Russell King - ARM Linux

Also note:

Review static __initdata struct of_device_id ext_clk_match[] = {

Quoted 1 line · Sachin Kamat
(quoted diff) +0 -0
@@ -984,7 +984,7 @@ static __initdata struct of_device_id ext_clk_match[] = {

For the declaration above...

drivers/clk/samsung/clk-exynos4.c static __initdata struct of_device_id ext_clk_match[] = { · line 987

Quoted 2 lines
drivers/clk/samsung/clk-exynos4.c +1 -1
-struct __initdata samsung_pll_clock exynos4_plls[nr_plls] = {
+static struct __initdata samsung_pll_clock exynos4_plls[nr_plls] = {

And this one... __initdata should come just before the '=', not at the start, not in the middle and not before the variable.

The reasoning is that with how you have it above, the attributes are applied to the structure. You want to apply the attributes to the declaration instead, so it should come after the variable name.

So, for example:

        struct foo *foo __attribute__((section(".foo"))) = (void *)1;

will place the "foo" variable into a section called ".foo", but:

Quoted 7 lines
        struct __attribute__((section(".foo"))) foo *foo = (void *)1;

will place "foo" into the normal .data section.

So, the rule with variable declarations is that the __ specifiers we have as macros in the kernel always come after the variable name being declared and nowhere else. We consider anywhere else buggy.

Thanks for this useful tip, Russell. There are several instances in the kernel where these attributes are used at the beginning of the variable declaration.

Probably it would be useful to add this to checkpatch. Joe?

Signature · 2 lines
With warm regards,
Sachin
#6 lore raw

PATCH 1/3 clk: exynos4: Make exynos4_plls static

Quoted 34 lines · Sachin Kamat

+CC Joe Perches

Quoted 23 lines · Russell King - ARM Linux

Also note:

Review static __initdata struct of_device_id ext_clk_match[] = {

Quoted 1 line · Sachin Kamat
(quoted diff) +0 -0
@@ -984,7 +984,7 @@ static __initdata struct of_device_id ext_clk_match[] = {

For the declaration above...

drivers/clk/samsung/clk-exynos4.c static __initdata struct of_device_id ext_clk_match[] = { · line 987

Quoted 2 lines
drivers/clk/samsung/clk-exynos4.c +1 -1
-struct __initdata samsung_pll_clock exynos4_plls[nr_plls] = {
+static struct __initdata samsung_pll_clock exynos4_plls[nr_plls] = {

And this one... __initdata should come just before the '=', not at the start, not in the middle and not before the variable.

The reasoning is that with how you have it above, the attributes are applied to the structure. You want to apply the attributes to the declaration instead, so it should come after the variable name.

So, for example:

        struct foo *foo __attribute__((section(".foo"))) = (void *)1;

will place the "foo" variable into a section called ".foo", but:

Quoted 7 lines
        struct __attribute__((section(".foo"))) foo *foo = (void *)1;

will place "foo" into the normal .data section.

So, the rule with variable declarations is that the __ specifiers we have as macros in the kernel always come after the variable name being declared and nowhere else. We consider anywhere else buggy.

Quoted 5 lines

Thanks for this useful tip, Russell. There are several instances in the kernel where these attributes are used at the beginning of the variable declaration.

Probably it would be useful to add this to checkpatch. Joe?

I think Russell is using the royal "We".

http://en.wikipedia.org/wiki/Majestic_plural

There's no way for checkpatch to look at an __<foo> use and determine it should be before or after a variable.

__scanf, __printf, __cold, etc are often place before declarations.

#7 lore raw

PATCH 1/3 clk: exynos4: Make exynos4_plls static

Quoted 35 lines · Joe Perches
Quoted 34 lines · Sachin Kamat

+CC Joe Perches

Quoted 30 lines · Russell King - ARM Linux

Also note:

Review static __initdata struct of_device_id ext_clk_match[] = {

Quoted 1 line · Sachin Kamat
(quoted diff) +0 -0
@@ -984,7 +984,7 @@ static __initdata struct of_device_id ext_clk_match[] = {

For the declaration above...

drivers/clk/samsung/clk-exynos4.c static __initdata struct of_device_id ext_clk_match[] = { · line 987

Quoted 2 lines
drivers/clk/samsung/clk-exynos4.c +1 -1
-struct __initdata samsung_pll_clock exynos4_plls[nr_plls] = {
+static struct __initdata samsung_pll_clock exynos4_plls[nr_plls] = {

And this one... __initdata should come just before the '=', not at the start, not in the middle and not before the variable.

The reasoning is that with how you have it above, the attributes are applied to the structure. You want to apply the attributes to the declaration instead, so it should come after the variable name.

So, for example:

        struct foo *foo __attribute__((section(".foo"))) = (void *)1;

will place the "foo" variable into a section called ".foo", but:

        struct __attribute__((section(".foo"))) foo *foo = (void *)1;

will place "foo" into the normal .data section.

So, the rule with variable declarations is that the __ specifiers we have as macros in the kernel always come after the variable name being declared and nowhere else. We consider anywhere else buggy.

Quoted 7 lines
Quoted 5 lines

Thanks for this useful tip, Russell. There are several instances in the kernel where these attributes are used at the beginning of the variable declaration.

Probably it would be useful to add this to checkpatch. Joe?

I think Russell is using the royal "We".

No I'm not. There have been many patches to fix errors like the above in the past. If it's not considered a bug by the community as a whole, it damn well should be.