All samsung MUX clocks that are defined with MUX() set the
CLK_SET_RATE_NO_REPARENT flag in __MUX(), which prevents MUXes to be
reparented during clk_set_rate().
Introduce nMUX() for MUX clocks that can be reparented. One user of
nMUX() will be GS101. GS101 defines MUX clocks that are dedicated for
each instance of an IP (see MUX USI). The reparenting of these MUX clocks
will not affect other instances of the same IP or different IPs
altogether.
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/clk/samsung/clk.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
index a70bd7cce39f..01f58b7686db 100644
--- a/drivers/clk/samsung/clk.h
+++ b/drivers/clk/samsung/clk.h
@@ -146,6 +146,26 @@ struct samsung_mux_clock {
#define MUX_F(_id, cname, pnames, o, s, w, f, mf) \
__MUX(_id, cname, pnames, o, s, w, f, mf)
+/* Used by MUX clocks where reparenting is allowed. */
+#define __nMUX(_id, cname, pnames, o, s, w, f, mf) \
+ { \
+ .id = _id, \
+ .name = cname, \
+ .parent_names = pnames, \
+ .num_parents = ARRAY_SIZE(pnames), \
+ .flags = f, \
+ .offset = o, \
+ .shift = s, \
+ .width = w, \
+ .mux_flags = mf, \
+ }
+
+#define nMUX(_id, cname, pnames, o, s, w) \
+ __nMUX(_id, cname, pnames, o, s, w, 0, 0)
+
+#define nMUX_F(_id, cname, pnames, o, s, w, f, mf) \
+ __nMUX(_id, cname, pnames, o, s, w, f, mf)
+
/**
* struct samsung_div_clock - information about div clock
* @id: platform specific id of the clock
--
2.44.0.396.g6e790dbe36-goog
Hi Tudor,
On Tue, 2024-03-26 at 17:28 +0000, Tudor Ambarus wrote:
> All samsung MUX clocks that are defined with MUX() set the
> CLK_SET_RATE_NO_REPARENT flag in __MUX(), which prevents MUXes to be
> reparented during clk_set_rate().
>
> Introduce nMUX() for MUX clocks that can be reparented.
What does n in nMUX stand for?
> [...]
>
> +/* Used by MUX clocks where reparenting is allowed. */
> +#define __nMUX(_id, cname, pnames, o, s, w, f, mf) \
> + { \
> + .id = _id, \
> + .name = cname, \
> + .parent_names = pnames, \
> + .num_parents = ARRAY_SIZE(pnames), \
> + .flags = f, \
> + .offset = o, \
> + .shift = s, \
> + .width = w, \
> + .mux_flags = mf, \
> + }
You've duplicated __MUX() and removed the CLK_SET_RATE_NO_REPARENT
from flags - I think it would make sense to instead drop the flag
from the existing __MUX(), and adjust the only two existing users
of the macro, i.e. to add it in MUX() and MUX_F().
Cheers,
Andre'
On 3/26/24 17:43, André Draszik wrote:
> Hi Tudor,
>
Hi, Andre'!
> On Tue, 2024-03-26 at 17:28 +0000, Tudor Ambarus wrote:
>> All samsung MUX clocks that are defined with MUX() set the
>> CLK_SET_RATE_NO_REPARENT flag in __MUX(), which prevents MUXes to be
>> reparented during clk_set_rate().
>>
>> Introduce nMUX() for MUX clocks that can be reparented.
>
> What does n in nMUX stand for?
I thought about using the common terminology, "n-to-1 multiplexer",
where n is the number of select lines. I'm open to other suggestions if
there are any. I should have specified the naming scheme in the commit
message, will do in the next version.
>
>> [...]
>>
>> +/* Used by MUX clocks where reparenting is allowed. */
>> +#define __nMUX(_id, cname, pnames, o, s, w, f, mf) \
>> + { \
>> + .id = _id, \
>> + .name = cname, \
>> + .parent_names = pnames, \
>> + .num_parents = ARRAY_SIZE(pnames), \
>> + .flags = f, \
>> + .offset = o, \
>> + .shift = s, \
>> + .width = w, \
>> + .mux_flags = mf, \
>> + }
>
> You've duplicated __MUX() and removed the CLK_SET_RATE_NO_REPARENT
> from flags - I think it would make sense to instead drop the flag
> from the existing __MUX(), and adjust the only two existing users
> of the macro, i.e. to add it in MUX() and MUX_F().
>
Yes, I find the suggestion good. Will do in v3.
Thanks,
ta
On 26/03/2024 18:28, Tudor Ambarus wrote:
> All samsung MUX clocks that are defined with MUX() set the
> CLK_SET_RATE_NO_REPARENT flag in __MUX(), which prevents MUXes to be
> reparented during clk_set_rate().
>
> Introduce nMUX() for MUX clocks that can be reparented. One user of
> nMUX() will be GS101. GS101 defines MUX clocks that are dedicated for
> each instance of an IP (see MUX USI). The reparenting of these MUX clocks
> will not affect other instances of the same IP or different IPs
> altogether.
>
> Signed-off-by: Tudor Ambarus <[email protected]>
> ---
> drivers/clk/samsung/clk.h | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
> index a70bd7cce39f..01f58b7686db 100644
> --- a/drivers/clk/samsung/clk.h
> +++ b/drivers/clk/samsung/clk.h
> @@ -146,6 +146,26 @@ struct samsung_mux_clock {
> #define MUX_F(_id, cname, pnames, o, s, w, f, mf) \
> __MUX(_id, cname, pnames, o, s, w, f, mf)
>
> +/* Used by MUX clocks where reparenting is allowed. */
..where reparenting on clock rate change is allowed
Because otherwise this suggest muxes cannot change :)
No need to resend just for this, I can fix it while applying. Still
waiting for some review, till EOD.
Best regards,
Krzysztof
On 3/28/24 09:56, Krzysztof Kozlowski wrote:
> On 26/03/2024 18:28, Tudor Ambarus wrote:
>> All samsung MUX clocks that are defined with MUX() set the
>> CLK_SET_RATE_NO_REPARENT flag in __MUX(), which prevents MUXes to be
>> reparented during clk_set_rate().
>>
>> Introduce nMUX() for MUX clocks that can be reparented. One user of
>> nMUX() will be GS101. GS101 defines MUX clocks that are dedicated for
>> each instance of an IP (see MUX USI). The reparenting of these MUX clocks
>> will not affect other instances of the same IP or different IPs
>> altogether.
>>
>> Signed-off-by: Tudor Ambarus <[email protected]>
>> ---
>> drivers/clk/samsung/clk.h | 20 ++++++++++++++++++++
>> 1 file changed, 20 insertions(+)
>>
>> diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
>> index a70bd7cce39f..01f58b7686db 100644
>> --- a/drivers/clk/samsung/clk.h
>> +++ b/drivers/clk/samsung/clk.h
>> @@ -146,6 +146,26 @@ struct samsung_mux_clock {
>> #define MUX_F(_id, cname, pnames, o, s, w, f, mf) \
>> __MUX(_id, cname, pnames, o, s, w, f, mf)
>>
>> +/* Used by MUX clocks where reparenting is allowed. */
>
> ...where reparenting on clock rate change is allowed
>
> Because otherwise this suggest muxes cannot change :)
Ok.
>
> No need to resend just for this, I can fix it while applying. Still
> waiting for some review, till EOD.
>
Andre' suggested I can avoid defining __nMUX() by removing the
CLK_SET_RATE_NO_REPARENT flag from __MUX() and instead add the flag in
the MUX() and MUX_F() definitions. Something like this:
diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
index a70bd7cce39f..fb06caa71f0a 100644
--- a/drivers/clk/samsung/clk.h
+++ b/drivers/clk/samsung/clk.h
@@ -133,7 +133,7 @@ struct samsung_mux_clock {
.name = cname, \
.parent_names = pnames, \
.num_parents = ARRAY_SIZE(pnames), \
- .flags = (f) | CLK_SET_RATE_NO_REPARENT, \
+ .flags = f, \
.offset = o, \
.shift = s, \
.width = w, \
@@ -141,9 +141,16 @@ struct samsung_mux_clock {
}
#define MUX(_id, cname, pnames, o, s, w) \
- __MUX(_id, cname, pnames, o, s, w, 0, 0)
+ __MUX(_id, cname, pnames, o, s, w, CLK_SET_RATE_NO_REPARENT, 0)
#define MUX_F(_id, cname, pnames, o, s, w, f, mf) \
+ __MUX(_id, cname, pnames, o, s, w, (f) |
CLK_SET_RATE_NO_REPARENT, mf)
+
+/* Used by MUX clocks where reparenting on clock rate change is allowed. */
+#define nMUX(_id, cname, pnames, o, s, w) \
+ __MUX(_id, cname, pnames, o, s, w, 0, 0)
+
+#define nMUX_F(_id, cname, pnames, o, s, w, f, mf) \
__MUX(_id, cname, pnames, o, s, w, f, mf)
I find the suggestion fine. Will submit v3 in a min, addressing all the
comments.
Thanks,
ta