[PATCH 1/2] clk: pistachio: Fix initconst confusion

[PATCH 1/2] clk: pistachio: Fix initconst confusion

[Jiri Slaby]

From: Andi Kleen <ak@linux.intel.com>

A variable pointing to const isn't const itself. It'd have to contain
"const" keyword after "*" too. Therefore, PNAME() cannot put the strings
to "rodata".  Hence use __initdata instead of __initconst to fix this.

[js] more explanatory commit message.

Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: linux-clk@vger.kernel.org
Signed-off-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/clk/pistachio/clk.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/pistachio/clk.h b/drivers/clk/pistachio/clk.h
index f9c31e3a0e47..742e5fab00c0 100644
--- a/drivers/clk/pistachio/clk.h
+++ b/drivers/clk/pistachio/clk.h
@@ -34,7 +34,7 @@ struct pistachio_mux {
 	const char **parents;
 };
 
-#define PNAME(x) static const char *x[] __initconst
+#define PNAME(x) static const char *x[] __initdata
 
 #define MUX(_id, _name, _pnames, _reg, _shift)			\
 	{							\
-- 
2.36.1

[PATCH 2/2] clk: renesas: rcar-gen4: Fix initconst confusion for cpg_pll_config

[Jiri Slaby]

From: Andi Kleen <ak@linux.intel.com>

A variable pointing to const isn't const itself. It'd have to contain
"const" keyword after "*" too. Therefore, cpg_pll_config cannot be put
to "rodata".  Hence use __initdata instead of __initconst to fix this.

[js] more explanatory commit message.

Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: linux-renesas-soc@vger.kernel.org
Cc: linux-clk@vger.kernel.org
Signed-off-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/clk/renesas/rcar-gen4-cpg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/renesas/rcar-gen4-cpg.c b/drivers/clk/renesas/rcar-gen4-cpg.c
index c7ed43d6aa67..e27832e5114f 100644
--- a/drivers/clk/renesas/rcar-gen4-cpg.c
+++ b/drivers/clk/renesas/rcar-gen4-cpg.c
@@ -23,7 +23,7 @@
 #include "rcar-gen4-cpg.h"
 #include "rcar-cpg-lib.h"
 
-static const struct rcar_gen4_cpg_pll_config *cpg_pll_config __initconst;
+static const struct rcar_gen4_cpg_pll_config *cpg_pll_config __initdata;
 static unsigned int cpg_clk_extalr __initdata;
 static u32 cpg_mode __initdata;
 
-- 
2.36.1

Re: [PATCH 2/2] clk: renesas: rcar-gen4: Fix initconst confusion for cpg_pll_config

[Stephen Boyd]

Quoting Jiri Slaby (2022-06-23 01:32:17)
> From: Andi Kleen <ak@linux.intel.com>
> 
> A variable pointing to const isn't const itself. It'd have to contain
> "const" keyword after "*" too. Therefore, cpg_pll_config cannot be put
> to "rodata".  Hence use __initdata instead of __initconst to fix this.
> 
> [js] more explanatory commit message.
> 
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: linux-clk@vger.kernel.org
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> ---

Acked-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH 2/2] clk: renesas: rcar-gen4: Fix initconst confusion for cpg_pll_config

[Geert Uytterhoeven]

On Thu, Jun 23, 2022 at 10:32 AM Jiri Slaby <jslaby@suse.cz> wrote:
> From: Andi Kleen <ak@linux.intel.com>
>
> A variable pointing to const isn't const itself. It'd have to contain
> "const" keyword after "*" too. Therefore, cpg_pll_config cannot be put
> to "rodata".  Hence use __initdata instead of __initconst to fix this.
>
> [js] more explanatory commit message.
>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: linux-clk@vger.kernel.org
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-clk-for-v5.20.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/2] clk: pistachio: Fix initconst confusion

[Stephen Boyd]

Quoting Jiri Slaby (2022-06-23 01:32:16)
> From: Andi Kleen <ak@linux.intel.com>
> 
> A variable pointing to const isn't const itself. It'd have to contain
> "const" keyword after "*" too. Therefore, PNAME() cannot put the strings
> to "rodata".  Hence use __initdata instead of __initconst to fix this.
> 
> [js] more explanatory commit message.
> 
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: linux-clk@vger.kernel.org
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> ---
>  drivers/clk/pistachio/clk.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/pistachio/clk.h b/drivers/clk/pistachio/clk.h
> index f9c31e3a0e47..742e5fab00c0 100644
> --- a/drivers/clk/pistachio/clk.h
> +++ b/drivers/clk/pistachio/clk.h
> @@ -34,7 +34,7 @@ struct pistachio_mux {
>         const char **parents;
>  };
>  
> -#define PNAME(x) static const char *x[] __initconst
> +#define PNAME(x) static const char *x[] __initdata

Can it be const char * const and left as __initconst?

Re: [PATCH 1/2] clk: pistachio: Fix initconst confusion

[Jiri Slaby]

On 24. 06. 22, 2:42, Stephen Boyd wrote:
> Quoting Jiri Slaby (2022-06-23 01:32:16)
>> From: Andi Kleen <ak@linux.intel.com>
>>
>> A variable pointing to const isn't const itself. It'd have to contain
>> "const" keyword after "*" too. Therefore, PNAME() cannot put the strings
>> to "rodata".  Hence use __initdata instead of __initconst to fix this.
>>
>> [js] more explanatory commit message.
>>
>> Cc: Michael Turquette <mturquette@baylibre.com>
>> Cc: Stephen Boyd <sboyd@kernel.org>
>> Cc: linux-clk@vger.kernel.org
>> Signed-off-by: Andi Kleen <ak@linux.intel.com>
>> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
>> ---
>>   drivers/clk/pistachio/clk.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/pistachio/clk.h b/drivers/clk/pistachio/clk.h
>> index f9c31e3a0e47..742e5fab00c0 100644
>> --- a/drivers/clk/pistachio/clk.h
>> +++ b/drivers/clk/pistachio/clk.h
>> @@ -34,7 +34,7 @@ struct pistachio_mux {
>>          const char **parents;
>>   };
>>   
>> -#define PNAME(x) static const char *x[] __initconst
>> +#define PNAME(x) static const char *x[] __initdata
> 
> Can it be const char * const and left as __initconst?

Let me check, IIRC the struct where this is assigned would need to be 
updated too.

I will get into it only some time next week.

thanks,
-- 
js
suse labs

Re: [PATCH 1/2] clk: pistachio: Fix initconst confusion

[Stephen Boyd]

Quoting Jiri Slaby (2022-06-27 00:46:15)
> On 24. 06. 22, 2:42, Stephen Boyd wrote:
> > Quoting Jiri Slaby (2022-06-23 01:32:16)
> >> From: Andi Kleen <ak@linux.intel.com>
> >>
> >> A variable pointing to const isn't const itself. It'd have to contain
> >> "const" keyword after "*" too. Therefore, PNAME() cannot put the strings
> >> to "rodata".  Hence use __initdata instead of __initconst to fix this.
> >>
> >> [js] more explanatory commit message.
> >>
> >> Cc: Michael Turquette <mturquette@baylibre.com>
> >> Cc: Stephen Boyd <sboyd@kernel.org>
> >> Cc: linux-clk@vger.kernel.org
> >> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> >> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> >> ---
> >>   drivers/clk/pistachio/clk.h | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/clk/pistachio/clk.h b/drivers/clk/pistachio/clk.h
> >> index f9c31e3a0e47..742e5fab00c0 100644
> >> --- a/drivers/clk/pistachio/clk.h
> >> +++ b/drivers/clk/pistachio/clk.h
> >> @@ -34,7 +34,7 @@ struct pistachio_mux {
> >>          const char **parents;
> >>   };
> >>   
> >> -#define PNAME(x) static const char *x[] __initconst
> >> +#define PNAME(x) static const char *x[] __initdata
> > 
> > Can it be const char * const and left as __initconst?
> 
> Let me check, IIRC the struct where this is assigned would need to be 
> updated too.
> 
> I will get into it only some time next week.
> 

Ok, sounds good. This seems to at least compile locally.

----8<---

diff --git a/drivers/clk/pistachio/clk.h b/drivers/clk/pistachio/clk.h
index f9c31e3a0e47..8be02ac2d909 100644
--- a/drivers/clk/pistachio/clk.h
+++ b/drivers/clk/pistachio/clk.h
@@ -31,10 +31,10 @@ struct pistachio_mux {
 	unsigned int shift;
 	unsigned int num_parents;
 	const char *name;
-	const char **parents;
+	const char * const *parents;
 };
 
-#define PNAME(x) static const char *x[] __initconst
+#define PNAME(x) static const char * const x[] __initconst
 
 #define MUX(_id, _name, _pnames, _reg, _shift)			\
 	{							\

Re: [PATCH 1/2] clk: pistachio: Fix initconst confusion

[Jiri Slaby]

On 29. 06. 22, 10:21, Stephen Boyd wrote:
> Quoting Jiri Slaby (2022-06-27 00:46:15)
>> On 24. 06. 22, 2:42, Stephen Boyd wrote:
>>> Quoting Jiri Slaby (2022-06-23 01:32:16)
>>>> From: Andi Kleen <ak@linux.intel.com>
>>>>
>>>> A variable pointing to const isn't const itself. It'd have to contain
>>>> "const" keyword after "*" too. Therefore, PNAME() cannot put the strings
>>>> to "rodata".  Hence use __initdata instead of __initconst to fix this.
>>>>
>>>> [js] more explanatory commit message.
>>>>
>>>> Cc: Michael Turquette <mturquette@baylibre.com>
>>>> Cc: Stephen Boyd <sboyd@kernel.org>
>>>> Cc: linux-clk@vger.kernel.org
>>>> Signed-off-by: Andi Kleen <ak@linux.intel.com>
>>>> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
>>>> ---
>>>>    drivers/clk/pistachio/clk.h | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/clk/pistachio/clk.h b/drivers/clk/pistachio/clk.h
>>>> index f9c31e3a0e47..742e5fab00c0 100644
>>>> --- a/drivers/clk/pistachio/clk.h
>>>> +++ b/drivers/clk/pistachio/clk.h
>>>> @@ -34,7 +34,7 @@ struct pistachio_mux {
>>>>           const char **parents;
>>>>    };
>>>>    
>>>> -#define PNAME(x) static const char *x[] __initconst
>>>> +#define PNAME(x) static const char *x[] __initdata
>>>
>>> Can it be const char * const and left as __initconst?
>>
>> Let me check, IIRC the struct where this is assigned would need to be
>> updated too.
>>
>> I will get into it only some time next week.
>>
> 
> Ok, sounds good. This seems to at least compile locally.

Yeah, that works. I've sent a v2.

BTW is the code intended to put the actual strings to .init.rodata? As 
that was never the case. Only those PNAME defined arrays (pointers to 
strings) end up in .init.rodata now and the strings are in .rodata.

> ----8<---
> 
> diff --git a/drivers/clk/pistachio/clk.h b/drivers/clk/pistachio/clk.h
> index f9c31e3a0e47..8be02ac2d909 100644
> --- a/drivers/clk/pistachio/clk.h
> +++ b/drivers/clk/pistachio/clk.h
> @@ -31,10 +31,10 @@ struct pistachio_mux {
>   	unsigned int shift;
>   	unsigned int num_parents;
>   	const char *name;
> -	const char **parents;
> +	const char * const *parents;
>   };
>   
> -#define PNAME(x) static const char *x[] __initconst
> +#define PNAME(x) static const char * const x[] __initconst
>   
>   #define MUX(_id, _name, _pnames, _reg, _shift)			\
>   	{							\


-- 
js
suse labs

Re: [PATCH 1/2] clk: pistachio: Fix initconst confusion

[Stephen Boyd]

Quoting Jiri Slaby (2022-07-08 00:16:12)
> On 29. 06. 22, 10:21, Stephen Boyd wrote:
> > Quoting Jiri Slaby (2022-06-27 00:46:15)
> >> On 24. 06. 22, 2:42, Stephen Boyd wrote:
> >>> Quoting Jiri Slaby (2022-06-23 01:32:16)
> >>>> From: Andi Kleen <ak@linux.intel.com>
> >>>>
> >>>> A variable pointing to const isn't const itself. It'd have to contain
> >>>> "const" keyword after "*" too. Therefore, PNAME() cannot put the strings
> >>>> to "rodata".  Hence use __initdata instead of __initconst to fix this.
> >>>>
> >>>> [js] more explanatory commit message.
> >>>>
> >>>> Cc: Michael Turquette <mturquette@baylibre.com>
> >>>> Cc: Stephen Boyd <sboyd@kernel.org>
> >>>> Cc: linux-clk@vger.kernel.org
> >>>> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> >>>> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> >>>> ---
> >>>>    drivers/clk/pistachio/clk.h | 2 +-
> >>>>    1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/clk/pistachio/clk.h b/drivers/clk/pistachio/clk.h
> >>>> index f9c31e3a0e47..742e5fab00c0 100644
> >>>> --- a/drivers/clk/pistachio/clk.h
> >>>> +++ b/drivers/clk/pistachio/clk.h
> >>>> @@ -34,7 +34,7 @@ struct pistachio_mux {
> >>>>           const char **parents;
> >>>>    };
> >>>>    
> >>>> -#define PNAME(x) static const char *x[] __initconst
> >>>> +#define PNAME(x) static const char *x[] __initdata
> >>>
> >>> Can it be const char * const and left as __initconst?
> >>
> >> Let me check, IIRC the struct where this is assigned would need to be
> >> updated too.
> >>
> >> I will get into it only some time next week.
> >>
> > 
> > Ok, sounds good. This seems to at least compile locally.
> 
> Yeah, that works. I've sent a v2.
> 
> BTW is the code intended to put the actual strings to .init.rodata? As 
> that was never the case. Only those PNAME defined arrays (pointers to 
> strings) end up in .init.rodata now and the strings are in .rodata.

I think both the strings and the array should be in .init.rodata. The
clk framework deep copies data like parent names.