Re: [PATCH] cpufreq: do not mark s3c2410_plls_add as __init

[Krzysztof Kozlowski]

On 17.11.2015 18:01, Arnd Bergmann wrote:
> On Tuesday 17 November 2015 10:57:48 Krzysztof Kozlowski wrote:
>> On 17.11.2015 07:17, Arnd Bergmann wrote:
>>> On Monday 16 November 2015 23:36:42 Rafael J. Wysocki wrote:

 This should go in through the Samsung tree, so I'll leave it for them to 
 pick
 it up (at least for the time being).
>>>
>>> Ok, fair enough. Kukjin or Krzysztof, can you pick this up?
>>
>> Sure.
>>
>> As for the patch I think everything can be converted to init/initdata
>> (as in attachment).
>>
> 
> I don't think so:
> 
> static struct subsys_interface s3c2442_plls169344_interface __initdata = {
> .name   = "s3c2442_plls169344",
> .subsys = _subsys,
> .add_dev= s3c2440_plls169344_add,
> };
> 
> This gets passed into subsys_interface_register(), which is not __init
> in turn. subsys_interface_register() then goes on to add
> s3c2442_plls169344_interface into a linked list that is traversed
> at runtime, but the __initdata section gets discarded so you now
> have a broken list.

Ah, yes, so the patch is fine:

Reviewed-by: Krzysztof Kozlowski 

Best regards,
Krzysztof
--
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] cpufreq: do not mark s3c2410_plls_add as __init

[Arnd Bergmann]

On Tuesday 17 November 2015 10:57:48 Krzysztof Kozlowski wrote:
> On 17.11.2015 07:17, Arnd Bergmann wrote:
> > On Monday 16 November 2015 23:36:42 Rafael J. Wysocki wrote:
> >>
> >> This should go in through the Samsung tree, so I'll leave it for them to 
> >> pick
> >> it up (at least for the time being).
> > 
> > Ok, fair enough. Kukjin or Krzysztof, can you pick this up?
> 
> Sure.
> 
> As for the patch I think everything can be converted to init/initdata
> (as in attachment).
> 

I don't think so:

static struct subsys_interface s3c2442_plls169344_interface __initdata = {
.name   = "s3c2442_plls169344",
.subsys = _subsys,
.add_dev= s3c2440_plls169344_add,
};

This gets passed into subsys_interface_register(), which is not __init
in turn. subsys_interface_register() then goes on to add
s3c2442_plls169344_interface into a linked list that is traversed
at runtime, but the __initdata section gets discarded so you now
have a broken list.

Arnd

--
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] cpufreq: do not mark s3c2410_plls_add as __init

[Rafael J. Wysocki]

On Monday, November 16, 2015 10:26:43 PM Arnd Bergmann wrote:
> s3c2410_plls_add is a device notifier that may be called at runtime and
> is correctly not marked __init. However it calls s3c_plltab_register()
> which is marked __init, and that triggers a build error when we are
> checking for section mismatches:
> 
> WARNING: vmlinux.o(.text+0x195e0): Section mismatch in reference from the 
> function s3c2410_plls_add() to the function .init.text:s3c_plltab_register()
> The function s3c2410_plls_add() references
> the function __init s3c_plltab_register().
> This is often because s3c2410_plls_add lacks a __init
> annotation or the annotation of s3c_plltab_register is wrong.
> 
> This removes the __init annotation from s3c2410_plls_add as well as the
> __initdata section annotations from s3c2440_plls_12 and s3c2440_plls_169344,
> which in turn are referenced from s3c2410_plls_add.
> 
> Signed-off-by: Arnd Bergmann 
> 
> It would be nice to get this merged through the cpufreq tree. Most likely
> this has never caused problems because the device notifiers in practice
> only get called at boot time, so I assume it's enough to have this fixed
> in 4.4 or 4.5 but not backported.
> 
> diff --git a/arch/arm/mach-s3c24xx/pll-s3c2440-1200.c 
> b/arch/arm/mach-s3c24xx/pll-s3c2440-1200.c
> index a19460e6e7b0..b355fca6cc2e 100644
> --- a/arch/arm/mach-s3c24xx/pll-s3c2440-1200.c
> +++ b/arch/arm/mach-s3c24xx/pll-s3c2440-1200.c
> @@ -20,7 +20,7 @@
>  #include 
>  #include 
>  
> -static struct cpufreq_frequency_table s3c2440_plls_12[] __initdata = {
> +static struct cpufreq_frequency_table s3c2440_plls_12[] = {
>   { .frequency = 7500,.driver_data = PLLVAL(0x75, 3, 3),  },  
> /* FVco 600.00 */
>   { .frequency = 8000,.driver_data = PLLVAL(0x98, 4, 3),  },  
> /* FVco 640.00 */
>   { .frequency = 9000,.driver_data = PLLVAL(0x70, 2, 3),  },  
> /* FVco 720.00 */
> diff --git a/arch/arm/mach-s3c24xx/pll-s3c2440-16934400.c 
> b/arch/arm/mach-s3c24xx/pll-s3c2440-16934400.c
> index 1191b2905625..be9a248b5ce9 100644
> --- a/arch/arm/mach-s3c24xx/pll-s3c2440-16934400.c
> +++ b/arch/arm/mach-s3c24xx/pll-s3c2440-16934400.c
> @@ -20,7 +20,7 @@
>  #include 
>  #include 
>  
> -static struct cpufreq_frequency_table s3c2440_plls_169344[] __initdata = {
> +static struct cpufreq_frequency_table s3c2440_plls_169344[] = {
>   { .frequency = 78019200,.driver_data = PLLVAL(121, 5, 3),   
> },  /* FVco 624.153600 */
>   { .frequency = 84067200,.driver_data = PLLVAL(131, 5, 3),   
> },  /* FVco 672.537600 */
>   { .frequency = 90115200,.driver_data = PLLVAL(141, 5, 3),   
> },  /* FVco 720.921600 */
> diff --git a/drivers/cpufreq/s3c24xx-cpufreq.c 
> b/drivers/cpufreq/s3c24xx-cpufreq.c
> index 733aa5153e74..68ef8fd9482f 100644
> --- a/drivers/cpufreq/s3c24xx-cpufreq.c
> +++ b/drivers/cpufreq/s3c24xx-cpufreq.c
> @@ -648,7 +648,7 @@ late_initcall(s3c_cpufreq_initcall);
>   *
>   * Register the given set of PLLs with the system.
>   */
> -int __init s3c_plltab_register(struct cpufreq_frequency_table *plls,
> +int s3c_plltab_register(struct cpufreq_frequency_table *plls,
>  unsigned int plls_no)
>  {
>   struct cpufreq_frequency_table *vals;
> 

This should go in through the Samsung tree, so I'll leave it for them to pick
it up (at least for the time being).

Thanks,
Rafael

--
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] cpufreq: do not mark s3c2410_plls_add as __init

[Arnd Bergmann]

On Monday 16 November 2015 23:36:42 Rafael J. Wysocki wrote:
> 
> This should go in through the Samsung tree, so I'll leave it for them to pick
> it up (at least for the time being).

Ok, fair enough. Kukjin or Krzysztof, can you pick this up?

Arnd
--
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] cpufreq: do not mark s3c2410_plls_add as __init

[Krzysztof Kozlowski]

On 17.11.2015 07:17, Arnd Bergmann wrote:
> On Monday 16 November 2015 23:36:42 Rafael J. Wysocki wrote:
>>
>> This should go in through the Samsung tree, so I'll leave it for them to pick
>> it up (at least for the time being).
> 
> Ok, fair enough. Kukjin or Krzysztof, can you pick this up?

Sure.

As for the patch I think everything can be converted to init/initdata
(as in attachment).

Best regards,
Krzysztof



diff --git a/arch/arm/mach-s3c24xx/pll-s3c2410.c b/arch/arm/mach-s3c24xx/pll-s3c2410.c
index 5e37d368594b..d9fd5319379b 100644
--- a/arch/arm/mach-s3c24xx/pll-s3c2410.c
+++ b/arch/arm/mach-s3c24xx/pll-s3c2410.c
@@ -65,12 +65,12 @@ static struct cpufreq_frequency_table pll_vals_12MHz[] = {
 { .frequency = 27000, .driver_data = PLLVAL(127, 1, 1),  },
 };
 
-static int s3c2410_plls_add(struct device *dev, struct subsys_interface *sif)
+static int __init s3c2410_plls_add(struct device *dev, struct subsys_interface *sif)
 {
 	return s3c_plltab_register(pll_vals_12MHz, ARRAY_SIZE(pll_vals_12MHz));
 }
 
-static struct subsys_interface s3c2410_plls_interface = {
+static struct subsys_interface s3c2410_plls_interface __initdata = {
 	.name		= "s3c2410_plls",
 	.subsys		= _subsys,
 	.add_dev	= s3c2410_plls_add,
@@ -83,7 +83,7 @@ static int __init s3c2410_pll_init(void)
 }
 arch_initcall(s3c2410_pll_init);
 
-static struct subsys_interface s3c2410a_plls_interface = {
+static struct subsys_interface s3c2410a_plls_interface __initdata = {
 	.name		= "s3c2410a_plls",
 	.subsys		= _subsys,
 	.add_dev	= s3c2410_plls_add,
diff --git a/arch/arm/mach-s3c24xx/pll-s3c2440-1200.c b/arch/arm/mach-s3c24xx/pll-s3c2440-1200.c
index a19460e6e7b0..01958230cf2f 100644
--- a/arch/arm/mach-s3c24xx/pll-s3c2440-1200.c
+++ b/arch/arm/mach-s3c24xx/pll-s3c2440-1200.c
@@ -50,7 +50,7 @@ static struct cpufreq_frequency_table s3c2440_plls_12[] __initdata = {
 	{ .frequency = 4,	.driver_data = PLLVAL(0x5c, 1, 1),  }, 	/* FVco 800.00 */
 };
 
-static int s3c2440_plls12_add(struct device *dev, struct subsys_interface *sif)
+static int __init s3c2440_plls12_add(struct device *dev, struct subsys_interface *sif)
 {
 	struct clk *xtal_clk;
 	unsigned long xtal;
@@ -71,7 +71,7 @@ static int s3c2440_plls12_add(struct device *dev, struct subsys_interface *sif)
 	return 0;
 }
 
-static struct subsys_interface s3c2440_plls12_interface = {
+static struct subsys_interface s3c2440_plls12_interface __initdata = {
 	.name		= "s3c2440_plls12",
 	.subsys		= _subsys,
 	.add_dev	= s3c2440_plls12_add,
@@ -84,7 +84,7 @@ static int __init s3c2440_pll_12mhz(void)
 }
 arch_initcall(s3c2440_pll_12mhz);
 
-static struct subsys_interface s3c2442_plls12_interface = {
+static struct subsys_interface s3c2442_plls12_interface __initdata = {
 	.name		= "s3c2442_plls12",
 	.subsys		= _subsys,
 	.add_dev	= s3c2440_plls12_add,
diff --git a/arch/arm/mach-s3c24xx/pll-s3c2440-16934400.c b/arch/arm/mach-s3c24xx/pll-s3c2440-16934400.c
index 1191b2905625..db9f476ab581 100644
--- a/arch/arm/mach-s3c24xx/pll-s3c2440-16934400.c
+++ b/arch/arm/mach-s3c24xx/pll-s3c2440-16934400.c
@@ -78,7 +78,7 @@ static struct cpufreq_frequency_table s3c2440_plls_169344[] __initdata = {
 	{ .frequency = 402192000,	.driver_data = PLLVAL(87, 2, 1), 	}, 	/* FVco 804.384000 */
 };
 
-static int s3c2440_plls169344_add(struct device *dev,
+static int __init s3c2440_plls169344_add(struct device *dev,
   struct subsys_interface *sif)
 {
 	struct clk *xtal_clk;
@@ -100,7 +100,7 @@ static int s3c2440_plls169344_add(struct device *dev,
 	return 0;
 }
 
-static struct subsys_interface s3c2440_plls169344_interface = {
+static struct subsys_interface s3c2440_plls169344_interface __initdata  = {
 	.name		= "s3c2440_plls169344",
 	.subsys		= _subsys,
 	.add_dev	= s3c2440_plls169344_add,
@@ -112,7 +112,7 @@ static int __init s3c2440_pll_16934400(void)
 }
 arch_initcall(s3c2440_pll_16934400);
 
-static struct subsys_interface s3c2442_plls169344_interface = {
+static struct subsys_interface s3c2442_plls169344_interface __initdata = {
 	.name		= "s3c2442_plls169344",
 	.subsys		= _subsys,
 	.add_dev	= s3c2440_plls169344_add,