The G12A/G12B Socs embeds a specific clock tree for each CPU cluster :
cpu_clk / cpub_clk
| \- cpu_clk_dyn
| | \- cpu_clk_premux0
| | |- cpu_clk_postmux0
| | | |- cpu_clk_dyn0_div
| | | \- xtal/fclk_div2/fclk_div3
| | \- xtal/fclk_div2/fclk_div3
| \- cpu_clk_premux1
| |- cpu_clk_postmux1
| | |- cpu_clk_dyn1_div
| | \- xtal/fclk_div2/fclk_div3
| \- xtal/fclk_div2/fclk_div3
\ sys_pll / sys1_pll
This patchset adds notifiers on cpu_clk / cpub_clk, cpu_clk_dyn,
cpu_clk_premux0 and sys_pll / sys1_pll to permit change frequency of
the CPU clock in a safe way as recommended by the vendor Documentation
and reference code.
This patchset :
- introduces needed core and meson clk changes
- adds the clock notifiers
Dependencies:
- None
This patchset is split from the v3 RFC/RFC patchset at [3]
Changes from v1 at [4]:
- Removed export of regmap_div ops functions
- Added standalone cpu dynamic divider driver
- Uses cpu dynamic divider driver in g12a clkc driver
- Added missing signed-off tags
- Fixed missing removal of CLKID_CPUB_CLK from internal g12a.h
Changes from RFT/RFC v3 at [3]:
- Rebased on clk-meson v5.4/drivers tree with Alexandre's patches
- Removed the eeclk setup() callback, moved to a toplevel g12a-data struct
Changes since RFT/RFC v2 at [2]:
- Rebased on clk-meson v5.3/drivers trees
- added Kevin's review tags
Changes since RFT/RFC v1 at [1]:
- Added EXPORT_SYMBOL_GPL() to clk_hw_set_parent
- Added missing static to g12b_cpub_clk_mux0_div_ops and g12a_cpu_clk_mux_nb
- Simplified g12a_cpu_clk_mux_notifier_cb() without switch/case
- Fixed typo in "this the current path" in g12a.c
- Fixed various checkpatch errors
[1] https://patchwork.kernel.org/cover/11006929/
[2] https://patchwork.kernel.org/cover/11017273/
[3] https://patchwork.kernel.org/cover/11025309/
[4] https://patchwork.kernel.org/cover/11063803/
Neil Armstrong (4):
clk: core: introduce clk_hw_set_parent()
clk: meson: add g12a cpu dynamic divider driver
clk: meson: g12a: add notifiers to handle cpu clock change
clk: meson: g12a: expose CPUB clock ID for G12B
drivers/clk/clk.c | 6 +
drivers/clk/meson/Kconfig | 5 +
drivers/clk/meson/Makefile | 1 +
drivers/clk/meson/clk-cpu-dyndiv.c | 73 ++++
drivers/clk/meson/clk-cpu-dyndiv.h | 20 +
drivers/clk/meson/g12a.c | 535 +++++++++++++++++++++++---
drivers/clk/meson/g12a.h | 1 -
include/dt-bindings/clock/g12a-clkc.h | 1 +
include/linux/clk-provider.h | 1 +
9 files changed, 588 insertions(+), 55 deletions(-)
create mode 100644 drivers/clk/meson/clk-cpu-dyndiv.c
create mode 100644 drivers/clk/meson/clk-cpu-dyndiv.h
--
2.22.0
Introduce the clk_hw_set_parent() provider call to change parent of
a clock by using the clk_hw pointers.
This eases the clock reparenting from clock rate notifiers and
implementing DVFS with simpler code avoiding the boilerplates
functions as __clk_lookup(clk_hw_get_name()) then clk_set_parent().
Signed-off-by: Neil Armstrong <[email protected]>
Acked-by: Martin Blumenstingl <[email protected]>
---
drivers/clk/clk.c | 6 ++++++
include/linux/clk-provider.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index c0990703ce54..c11b1781d24a 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2487,6 +2487,12 @@ static int clk_core_set_parent_nolock(struct clk_core *core,
return ret;
}
+int clk_hw_set_parent(struct clk_hw *hw, struct clk_hw *parent)
+{
+ return clk_core_set_parent_nolock(hw->core, parent->core);
+}
+EXPORT_SYMBOL_GPL(clk_hw_set_parent);
+
/**
* clk_set_parent - switch the parent of a mux clk
* @clk: the mux clk whose input we are switching
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 2ae7604783dd..dce5521a9bf6 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -817,6 +817,7 @@ unsigned int clk_hw_get_num_parents(const struct clk_hw *hw);
struct clk_hw *clk_hw_get_parent(const struct clk_hw *hw);
struct clk_hw *clk_hw_get_parent_by_index(const struct clk_hw *hw,
unsigned int index);
+int clk_hw_set_parent(struct clk_hw *hw, struct clk_hw *new_parent);
unsigned int __clk_get_enable_count(struct clk *clk);
unsigned long clk_hw_get_rate(const struct clk_hw *hw);
unsigned long __clk_get_flags(struct clk *clk);
--
2.22.0
Expose the CPUB clock id to add DVFS to the second CPU cluster of
the Amlogic G12B SoC.
Reviewed-by: Martin Blumenstingl <[email protected]>
Signed-off-by: Neil Armstrong <[email protected]>
---
drivers/clk/meson/g12a.h | 1 -
include/dt-bindings/clock/g12a-clkc.h | 1 +
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/meson/g12a.h b/drivers/clk/meson/g12a.h
index c8aed31fbe17..559a34cfdfeb 100644
--- a/drivers/clk/meson/g12a.h
+++ b/drivers/clk/meson/g12a.h
@@ -216,7 +216,6 @@
#define CLKID_CPUB_CLK_DYN1_DIV 221
#define CLKID_CPUB_CLK_DYN1 222
#define CLKID_CPUB_CLK_DYN 223
-#define CLKID_CPUB_CLK 224
#define CLKID_CPUB_CLK_DIV16_EN 225
#define CLKID_CPUB_CLK_DIV16 226
#define CLKID_CPUB_CLK_DIV2 227
diff --git a/include/dt-bindings/clock/g12a-clkc.h b/include/dt-bindings/clock/g12a-clkc.h
index b6b127e45634..8ccc29ac7a72 100644
--- a/include/dt-bindings/clock/g12a-clkc.h
+++ b/include/dt-bindings/clock/g12a-clkc.h
@@ -137,5 +137,6 @@
#define CLKID_VDEC_HEVC 207
#define CLKID_VDEC_HEVCF 210
#define CLKID_TS 212
+#define CLKID_CPUB_CLK 224
#endif /* __G12A_CLKC_H */
--
2.22.0
On Wed 31 Jul 2019 at 10:40, Neil Armstrong <[email protected]> wrote:
> Introduce the clk_hw_set_parent() provider call to change parent of
> a clock by using the clk_hw pointers.
>
> This eases the clock reparenting from clock rate notifiers and
> implementing DVFS with simpler code avoiding the boilerplates
> functions as __clk_lookup(clk_hw_get_name()) then clk_set_parent().
>
> Signed-off-by: Neil Armstrong <[email protected]>
> Acked-by: Martin Blumenstingl <[email protected]>
Looks ok to me but we will obviously need Stephen's ack to apply it
> ---
> drivers/clk/clk.c | 6 ++++++
> include/linux/clk-provider.h | 1 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index c0990703ce54..c11b1781d24a 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -2487,6 +2487,12 @@ static int clk_core_set_parent_nolock(struct clk_core *core,
> return ret;
> }
>
> +int clk_hw_set_parent(struct clk_hw *hw, struct clk_hw *parent)
> +{
> + return clk_core_set_parent_nolock(hw->core, parent->core);
> +}
> +EXPORT_SYMBOL_GPL(clk_hw_set_parent);
> +
> /**
> * clk_set_parent - switch the parent of a mux clk
> * @clk: the mux clk whose input we are switching
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 2ae7604783dd..dce5521a9bf6 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -817,6 +817,7 @@ unsigned int clk_hw_get_num_parents(const struct clk_hw *hw);
> struct clk_hw *clk_hw_get_parent(const struct clk_hw *hw);
> struct clk_hw *clk_hw_get_parent_by_index(const struct clk_hw *hw,
> unsigned int index);
> +int clk_hw_set_parent(struct clk_hw *hw, struct clk_hw *new_parent);
> unsigned int __clk_get_enable_count(struct clk *clk);
> unsigned long clk_hw_get_rate(const struct clk_hw *hw);
> unsigned long __clk_get_flags(struct clk *clk);
> --
> 2.22.0
On Wed 31 Jul 2019 at 10:40, Neil Armstrong <[email protected]> wrote:
>
> Neil Armstrong (4):
> clk: core: introduce clk_hw_set_parent()
> clk: meson: add g12a cpu dynamic divider driver
> clk: meson: g12a: add notifiers to handle cpu clock change
> clk: meson: g12a: expose CPUB clock ID for G12B
>
> drivers/clk/clk.c | 6 +
> drivers/clk/meson/Kconfig | 5 +
> drivers/clk/meson/Makefile | 1 +
> drivers/clk/meson/clk-cpu-dyndiv.c | 73 ++++
> drivers/clk/meson/clk-cpu-dyndiv.h | 20 +
> drivers/clk/meson/g12a.c | 535 +++++++++++++++++++++++---
> drivers/clk/meson/g12a.h | 1 -
> include/dt-bindings/clock/g12a-clkc.h | 1 +
> include/linux/clk-provider.h | 1 +
> 9 files changed, 588 insertions(+), 55 deletions(-)
> create mode 100644 drivers/clk/meson/clk-cpu-dyndiv.c
> create mode 100644 drivers/clk/meson/clk-cpu-dyndiv.h
Patchset looks good to me.
Waiting for Stephen's ack on patch #1 to apply it.
>
> --
> 2.22.0
Quoting Jerome Brunet (2019-08-06 01:28:19)
> On Wed 31 Jul 2019 at 10:40, Neil Armstrong <[email protected]> wrote:
>
> > Introduce the clk_hw_set_parent() provider call to change parent of
> > a clock by using the clk_hw pointers.
> >
> > This eases the clock reparenting from clock rate notifiers and
> > implementing DVFS with simpler code avoiding the boilerplates
> > functions as __clk_lookup(clk_hw_get_name()) then clk_set_parent().
> >
> > Signed-off-by: Neil Armstrong <[email protected]>
> > Acked-by: Martin Blumenstingl <[email protected]>
>
> Looks ok to me but we will obviously need Stephen's ack to apply it
Acked-by: Stephen Boyd <[email protected]>
>
> > ---
> > drivers/clk/clk.c | 6 ++++++
> > include/linux/clk-provider.h | 1 +
> > 2 files changed, 7 insertions(+)
> >
> > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > index c0990703ce54..c11b1781d24a 100644
> > --- a/drivers/clk/clk.c
> > +++ b/drivers/clk/clk.c
> > @@ -2487,6 +2487,12 @@ static int clk_core_set_parent_nolock(struct clk_core *core,
> > return ret;
> > }
> >
> > +int clk_hw_set_parent(struct clk_hw *hw, struct clk_hw *parent)
> > +{
> > + return clk_core_set_parent_nolock(hw->core, parent->core);
I wonder if you really want to call all those things in
clk_core_set_parent_nolock(). For example, do you want notifiers to run
again and for rates to be speculated? Maybe all you want to do is
overwrite some value for the clk's parent by calling into the ops for
the clk and generically parse the value that's passed as the "parent"
here.
I ask because it may be good to avoid doing all that work and updating
bookkeeping when we're deep in a notifier. If we can clearly understand
what the driver wants to do from the notifier then it's simpler to
change in the future to use things such as the coordinated clk rate
vaporware.
Neil Armstrong <[email protected]> writes:
> The G12A/G12B Socs embeds a specific clock tree for each CPU cluster :
> cpu_clk / cpub_clk
> | \- cpu_clk_dyn
> | | \- cpu_clk_premux0
> | | |- cpu_clk_postmux0
> | | | |- cpu_clk_dyn0_div
> | | | \- xtal/fclk_div2/fclk_div3
> | | \- xtal/fclk_div2/fclk_div3
> | \- cpu_clk_premux1
> | |- cpu_clk_postmux1
> | | |- cpu_clk_dyn1_div
> | | \- xtal/fclk_div2/fclk_div3
> | \- xtal/fclk_div2/fclk_div3
> \ sys_pll / sys1_pll
>
> This patchset adds notifiers on cpu_clk / cpub_clk, cpu_clk_dyn,
> cpu_clk_premux0 and sys_pll / sys1_pll to permit change frequency of
> the CPU clock in a safe way as recommended by the vendor Documentation
> and reference code.
>
> This patchset :
> - introduces needed core and meson clk changes
> - adds the clock notifiers
>
> Dependencies:
> - None
nit: this doesn't apply to v5.3-rc, but appears to apply on
clk-meson/v5.4/drivers, so it appears to be dependent on the cleanups
from Alex.
Kevin
On Thu 08 Aug 2019 at 14:18, Kevin Hilman <[email protected]> wrote:
> Neil Armstrong <[email protected]> writes:
>
>> The G12A/G12B Socs embeds a specific clock tree for each CPU cluster :
>> cpu_clk / cpub_clk
>> | \- cpu_clk_dyn
>> | | \- cpu_clk_premux0
>> | | |- cpu_clk_postmux0
>> | | | |- cpu_clk_dyn0_div
>> | | | \- xtal/fclk_div2/fclk_div3
>> | | \- xtal/fclk_div2/fclk_div3
>> | \- cpu_clk_premux1
>> | |- cpu_clk_postmux1
>> | | |- cpu_clk_dyn1_div
>> | | \- xtal/fclk_div2/fclk_div3
>> | \- xtal/fclk_div2/fclk_div3
>> \ sys_pll / sys1_pll
>>
>> This patchset adds notifiers on cpu_clk / cpub_clk, cpu_clk_dyn,
>> cpu_clk_premux0 and sys_pll / sys1_pll to permit change frequency of
>> the CPU clock in a safe way as recommended by the vendor Documentation
>> and reference code.
>>
>> This patchset :
>> - introduces needed core and meson clk changes
>> - adds the clock notifiers
>>
>> Dependencies:
>> - None
>
> nit: this doesn't apply to v5.3-rc, but appears to apply on
> clk-meson/v5.4/drivers, so it appears to be dependent on the cleanups
> from Alex.
Indeed, Applied on top of this.
>
> Kevin
Jerome Brunet <[email protected]> writes:
> On Thu 08 Aug 2019 at 14:18, Kevin Hilman <[email protected]> wrote:
>
>> Neil Armstrong <[email protected]> writes:
>>
>>> The G12A/G12B Socs embeds a specific clock tree for each CPU cluster :
>>> cpu_clk / cpub_clk
>>> | \- cpu_clk_dyn
>>> | | \- cpu_clk_premux0
>>> | | |- cpu_clk_postmux0
>>> | | | |- cpu_clk_dyn0_div
>>> | | | \- xtal/fclk_div2/fclk_div3
>>> | | \- xtal/fclk_div2/fclk_div3
>>> | \- cpu_clk_premux1
>>> | |- cpu_clk_postmux1
>>> | | |- cpu_clk_dyn1_div
>>> | | \- xtal/fclk_div2/fclk_div3
>>> | \- xtal/fclk_div2/fclk_div3
>>> \ sys_pll / sys1_pll
>>>
>>> This patchset adds notifiers on cpu_clk / cpub_clk, cpu_clk_dyn,
>>> cpu_clk_premux0 and sys_pll / sys1_pll to permit change frequency of
>>> the CPU clock in a safe way as recommended by the vendor Documentation
>>> and reference code.
>>>
>>> This patchset :
>>> - introduces needed core and meson clk changes
>>> - adds the clock notifiers
>>>
>>> Dependencies:
>>> - None
>>
>> nit: this doesn't apply to v5.3-rc, but appears to apply on
>> clk-meson/v5.4/drivers, so it appears to be dependent on the cleanups
>> from Alex.
>
> Indeed, Applied on top of this.
>
Please let me know when you have a stable tag for this. I wont' be able
to apply the odroid-n2 DVFS patch until I have this.
Kevin