Re: [PATCH 1/3] clk: exynos4: Make exynos4_plls static

2013-08-07 Thread Russell King - ARM Linux
On Tue, Aug 06, 2013 at 08:37:28PM -0700, Joe Perches wrote:
 On Wed, 2013-08-07 at 08:51 +0530, Sachin Kamat wrote:
  +CC Joe Perches
  
  On 7 August 2013 01:32, Russell King - ARM Linux li...@arm.linux.org.uk 
  wrote:
   Also note:
  
   On Tue, Aug 06, 2013 at 05:01:13PM +0530, Sachin Kamat wrote:
   @@ -984,7 +984,7 @@ static __initdata struct of_device_id 
   ext_clk_match[] = {
  
   For the declaration above...
  
   -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.
  
  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.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/3] clk: exynos4: Make exynos4_plls static

2013-08-06 Thread Sachin Kamat
'exynos4_plls' is used only in this file. Make it static.

Signed-off-by: Sachin Kamat sachin.ka...@linaro.org
---
 drivers/clk/samsung/clk-exynos4.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/samsung/clk-exynos4.c 
b/drivers/clk/samsung/clk-exynos4.c
index 68f9a4a..fec319d 100644
--- a/drivers/clk/samsung/clk-exynos4.c
+++ b/drivers/clk/samsung/clk-exynos4.c
@@ -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,
-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] clk: exynos4: Make exynos4_plls static

2013-08-06 Thread Mike Turquette
Quoting Sachin Kamat (2013-08-06 04:31:13)
 'exynos4_plls' is used only in this file. Make it static.
 
 Signed-off-by: Sachin Kamat sachin.ka...@linaro.org

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

Regards,
Mike

 ---
  drivers/clk/samsung/clk-exynos4.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/clk/samsung/clk-exynos4.c 
 b/drivers/clk/samsung/clk-exynos4.c
 index 68f9a4a..fec319d 100644
 --- a/drivers/clk/samsung/clk-exynos4.c
 +++ b/drivers/clk/samsung/clk-exynos4.c
 @@ -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,
 -- 
 1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] clk: exynos4: Make exynos4_plls static

2013-08-06 Thread Sachin Kamat
+CC Joe Perches

On 7 August 2013 01:32, Russell King - ARM Linux li...@arm.linux.org.uk wrote:
 Also note:

 On Tue, Aug 06, 2013 at 05:01:13PM +0530, Sachin Kamat wrote:
 @@ -984,7 +984,7 @@ static __initdata struct of_device_id ext_clk_match[] = {

 For the declaration above...

 -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.

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?

-- 
With warm regards,
Sachin
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] clk: exynos4: Make exynos4_plls static

2013-08-06 Thread Joe Perches
On Wed, 2013-08-07 at 08:51 +0530, Sachin Kamat wrote:
 +CC Joe Perches
 
 On 7 August 2013 01:32, Russell King - ARM Linux li...@arm.linux.org.uk 
 wrote:
  Also note:
 
  On Tue, Aug 06, 2013 at 05:01:13PM +0530, Sachin Kamat wrote:
  @@ -984,7 +984,7 @@ static __initdata struct of_device_id ext_clk_match[] 
  = {
 
  For the declaration above...
 
  -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.
 
 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.


--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html