mbox series

[v4,0/4] rockchip: add GATE_LINK

Message ID 20231018070144.8512-1-zhangqing@rock-chips.com
Headers show
Series rockchip: add GATE_LINK | expand

Message

zhangqing Oct. 18, 2023, 7:01 a.m. UTC 
Recent Rockchip SoCs have a new hardware block called Native Interface
Unit (NIU), which gates clocks to devices behind them. These effectively
need two parent clocks.
Use GATE_LINK to handle this.

change in v4:
[PATCH v4 1/4]: No change
[PATCH v4 2/4]: No change
[PATCH v4 3/4]: dropping CLK_NR_CLKS,reword commit message
[PATCH v4 4/4]: No change

change in V3:
[PATCH v3 1/4]: new, export clk_gate_endisable for PATCH2.
[PATCH v3 2/4]: reuse clk_gate_endisable and clk_gate_is_enabled.
                add prepare and unprepare ops.
[PATCH v3 3/4]: No change
[PATCH v3 4/4]: reword commit message

change in V2:
[PATCH v2 1/3]: fix reported warnings
[PATCH v2 2/3]: Bindings submit independent patches
[PATCH v2 3/3]: fix reported warnings

Elaine Zhang (4):
  clk: gate: export clk_gate_endisable
  clk: rockchip: add support for gate link
  dt-bindings: clock: rk3588: export PCLK_VO1GRF clk id
  clk: rockchip: rk3588: Adjust the GATE_LINK parameter

 drivers/clk/clk-gate.c                        |   3 +-
 drivers/clk/rockchip/Makefile                 |   1 +
 drivers/clk/rockchip/clk-gate-link.c          | 120 ++++++++++++++++++
 drivers/clk/rockchip/clk-rk3588.c             | 110 ++++++++--------
 drivers/clk/rockchip/clk.c                    |   7 +
 drivers/clk/rockchip/clk.h                    |  22 ++++
 .../dt-bindings/clock/rockchip,rk3588-cru.h   |   2 +-
 include/linux/clk-provider.h                  |   1 +
 8 files changed, 213 insertions(+), 53 deletions(-)
 create mode 100644 drivers/clk/rockchip/clk-gate-link.c

Comments

Sebastian Reichel Oct. 20, 2023, 5:42 p.m. UTC | #1 
Hi,

On Wed, Oct 18, 2023 at 03:01:40PM +0800, Elaine Zhang wrote:
> Recent Rockchip SoCs have a new hardware block called Native Interface
> Unit (NIU), which gates clocks to devices behind them. These effectively
> need two parent clocks.
> Use GATE_LINK to handle this.
> 
> change in v4:
> [PATCH v4 1/4]: No change
> [PATCH v4 2/4]: No change
> [PATCH v4 3/4]: dropping CLK_NR_CLKS,reword commit message
> [PATCH v4 4/4]: No change
> 
> change in V3:
> [PATCH v3 1/4]: new, export clk_gate_endisable for PATCH2.
> [PATCH v3 2/4]: reuse clk_gate_endisable and clk_gate_is_enabled.
>                 add prepare and unprepare ops.
> [PATCH v3 3/4]: No change
> [PATCH v3 4/4]: reword commit message
> 
> change in V2:
> [PATCH v2 1/3]: fix reported warnings
> [PATCH v2 2/3]: Bindings submit independent patches
> [PATCH v2 3/3]: fix reported warnings
> 
> Elaine Zhang (4):
>   clk: gate: export clk_gate_endisable
>   clk: rockchip: add support for gate link
>   dt-bindings: clock: rk3588: export PCLK_VO1GRF clk id
>   clk: rockchip: rk3588: Adjust the GATE_LINK parameter
> 
>  drivers/clk/clk-gate.c                        |   3 +-
>  drivers/clk/rockchip/Makefile                 |   1 +
>  drivers/clk/rockchip/clk-gate-link.c          | 120 ++++++++++++++++++
>  drivers/clk/rockchip/clk-rk3588.c             | 110 ++++++++--------
>  drivers/clk/rockchip/clk.c                    |   7 +
>  drivers/clk/rockchip/clk.h                    |  22 ++++
>  .../dt-bindings/clock/rockchip,rk3588-cru.h   |   2 +-
>  include/linux/clk-provider.h                  |   1 +
>  8 files changed, 213 insertions(+), 53 deletions(-)
>  create mode 100644 drivers/clk/rockchip/clk-gate-link.c

I get this with the patchset applied:

Oct 20 18:25:04 rk3588-evb1 kernel: clk: Disabling unused clocks
Oct 20 18:25:04 rk3588-evb1 kernel: ------------[ cut here ]------------
Oct 20 18:25:04 rk3588-evb1 kernel: hclk_rkvenc0 already disabled
Oct 20 18:25:04 rk3588-evb1 kernel: WARNING: CPU: 4 PID: 1 at drivers/clk/clk.c:1090 clk_core_disable+0x1b0/0x1e0
Oct 20 18:25:04 rk3588-evb1 kernel: Modules linked in:
Oct 20 18:25:04 rk3588-evb1 kernel: CPU: 4 PID: 1 Comm: swapper/0 Not tainted 6.6.0-rc6-00044-g31c3dbd16ca1 #1120
Oct 20 18:25:04 rk3588-evb1 kernel: Hardware name: Rockchip RK3588 EVB1 V10 Board (DT)
Oct 20 18:25:04 rk3588-evb1 kernel: pstate: 604000c9 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
Oct 20 18:25:04 rk3588-evb1 kernel: pc : clk_core_disable+0x1b0/0x1e0
Oct 20 18:25:04 rk3588-evb1 kernel: lr : clk_core_disable+0x1b0/0x1e0
Oct 20 18:25:04 rk3588-evb1 kernel: sp : ffff8000846fbbf0
Oct 20 18:25:04 rk3588-evb1 kernel: x29: ffff8000846fbbf0 x28: ffff8000833a0008 x27: ffff8000830e0130
Oct 20 18:25:04 rk3588-evb1 kernel: x26: ffff800082ffcff0 x25: ffff8000830b9be8 x24: ffff800083236100
Oct 20 18:25:04 rk3588-evb1 kernel: x23: 000000000000104a x22: 0000000000000000 x21: ffff800084669000
Oct 20 18:25:04 rk3588-evb1 kernel: x20: ffff00040087ca00 x19: ffff00040087ca00 x18: ffffffffffffffff
Oct 20 18:25:04 rk3588-evb1 kernel: x17: ffff80008435d050 x16: ffff80008435cfe0 x15: 0720072007200720
Oct 20 18:25:04 rk3588-evb1 kernel: x14: 0720072007200720 x13: 0720072007200720 x12: 0720072007200720
Oct 20 18:25:04 rk3588-evb1 kernel: x11: 0720072007200720 x10: ffff800084079890 x9 : ffff800080128a78
Oct 20 18:25:04 rk3588-evb1 kernel: x8 : 00000000ffffefff x7 : ffff800084079890 x6 : 80000000fffff000
Oct 20 18:25:04 rk3588-evb1 kernel: x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
Oct 20 18:25:04 rk3588-evb1 kernel: x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000400a08000
Oct 20 18:25:04 rk3588-evb1 kernel: Call trace:
Oct 20 18:25:04 rk3588-evb1 kernel:  clk_core_disable+0x1b0/0x1e0
Oct 20 18:25:04 rk3588-evb1 kernel:  clk_disable+0x38/0x60
Oct 20 18:25:04 rk3588-evb1 kernel:  clk_gate_link_disable+0x2c/0x48
Oct 20 18:25:04 rk3588-evb1 kernel:  clk_disable_unused_subtree+0x104/0x270
Oct 20 18:25:04 rk3588-evb1 kernel:  clk_disable_unused_subtree+0x34/0x270
Oct 20 18:25:04 rk3588-evb1 kernel:  clk_disable_unused_subtree+0x34/0x270
Oct 20 18:25:04 rk3588-evb1 kernel:  clk_disable_unused_subtree+0x34/0x270
Oct 20 18:25:04 rk3588-evb1 kernel:  clk_disable_unused_subtree+0x34/0x270
Oct 20 18:25:04 rk3588-evb1 kernel:  clk_disable_unused_subtree+0x34/0x270
Oct 20 18:25:04 rk3588-evb1 kernel:  clk_disable_unused+0x54/0x148
Oct 20 18:25:04 rk3588-evb1 kernel:  do_one_initcall+0x48/0x2a8
Oct 20 18:25:04 rk3588-evb1 kernel:  kernel_init_freeable+0x1ec/0x3d8
Oct 20 18:25:04 rk3588-evb1 kernel:  kernel_init+0x2c/0x1f8
Oct 20 18:25:04 rk3588-evb1 kernel:  ret_from_fork+0x10/0x20
Oct 20 18:25:04 rk3588-evb1 kernel: ---[ end trace 0000000000000000 ]---
Oct 20 18:25:04 rk3588-evb1 kernel: ------------[ cut here ]------------
... (more clocks follow)

I managed to fix this by introducing some flags, that clk_disable /
clk_unprepare is only called on the linked clock if there was a
prior clk_enable/clk_prepare for it.

Apart from that this does not remove the existing GATE clocks for
pclk_vo0grf and pclk_vo1grf resulting in the following errors:

Oct 20 19:22:37 rk3588-evb1 kernel: pclk_vo0grf clk_register field
Oct 20 19:22:37 rk3588-evb1 kernel: rockchip_clk_register_branches: failed to register clock pclk_vo0grf: -17
Oct 20 19:22:37 rk3588-evb1 kernel: pclk_vo1grf clk_register field
Oct 20 19:22:37 rk3588-evb1 kernel: rockchip_clk_register_branches: failed to register clock pclk_vo1grf: -17

Last but not least I think it's better to restructure the patches a
bit:

Patch 1: Add CLK ID for PCLK_VO1GRF in the DT binding (add Fixes tag for 4f5ca304f202)
Patch 2: Fix clock"pclk_vo0grf" and "pclk_vo1grf" (add Fixes tag for f1c506d152ff)
Patch 3: Adjust GATE_LINK parameter from string to constant, but
         keep local GATE_LINK() define
Patch 4: Export clk_gate_endisable
Patch 5: Add gate-link support and remove local GATE_LINK() define from rk3588
Patch 6: Remove RK3588_LINKED_CLK

Greetings,

-- Sebastian
Stephen Boyd Oct. 25, 2023, 9:40 p.m. UTC | #2 
Quoting Sebastian Reichel (2023-10-25 12:48:49)
> Hello Stephen,
> 
> On Mon, Oct 23, 2023 at 06:47:17PM -0700, Stephen Boyd wrote:
> > Quoting Elaine Zhang (2023-10-18 00:01:40)
> > > Recent Rockchip SoCs have a new hardware block called Native Interface
> > > Unit (NIU), which gates clocks to devices behind them. These effectively
> > > need two parent clocks.
> > > Use GATE_LINK to handle this.
> > 
> > Why can't pm clks be used here? The qcom clk driver has been doing that
> > for some time now. 
> > 
> >  $ git grep pm_clk_add -- drivers/clk/qcom/
> 
> Maybe I'm mistaken, but as far as I can tell this is adding the
> dependency on controller level and only works because Qualcomm
> has multiple separate clock controllers. In the Rockchip design
> there is only one platform device.
> 
> Note, that the original downstream code from Rockchip actually used
> pm_clk infrastructure by moving these clocks to separate platform
> devices. I changed this when upstreaming the code, since that leaks
> into DT and from DT point of view there should be only one clock
> controller.
> 

Why can't the rockchip driver bind to a single device node and make
sub-devices for each clk domain and register clks for those? Maybe it
can use the auxiliary driver infrastructure to do that?