RP1 clocks

[6by9]

For discussion over critical clocks, and those that shouldn't be disabled.

[6by9]

Updated.

No clocks now have CLK_IGNORE_UNUSED, and only the main PLLs and SYS_SEC are marked as CRITICAL.

Added the tx_clock config for the ethernet MAC. Ethernet is now working again.

I2C, DSI, and CSI/CFE also tested and working. I'll find an I2S audio device in a moment. Anything else that needs testing?

@P33M @naushir @pelwell comments please.

Complete clock summary is now

pi@bookworm64-1:~ $ cat /sys/kernel/debug/clk/clk_summary 
                                 enable  prepare  protect                                duty  hardware                            connection
   clock                          count    count    count        rate   accuracy phase  cycle    enable   consumer                         id
---------------------------------------------------------------------------------------------------------------------------------------------
 cam1_clk                            0       0        0        24000000    0          0     50000      Y   11-0060                         no_connection_id         
                                                                                                           deviceless                      no_connection_id         
 fw-clk-disp                         1       1        0        0           0          0     50000      Y   107c580000.hvs                  disp                     
                                                                                                           deviceless                      no_connection_id         
 fw-clk-vec                          0       0        0        0           0          0     50000      Y   deviceless                      no_connection_id         
 fw-clk-pixel-bvb                    1       1        0        324000000   0          0     50000      Y   107c706400.hdmi                 bvb                      
                                                                                                           107c701400.hdmi                 bvb                      
                                                                                                           deviceless                      no_connection_id         
 fw-clk-m2mc                         3       3        0        149985000   0          0     50000      Y   107c706400.hdmi                 hdmi                     
                                                                                                           107c701400.hdmi                 hdmi                     
                                                                                                           deviceless                      no_connection_id         
 fw-clk-hevc                         0       0        0        500000000   0          0     50000      Y   1000800000.codec                hevc                     
                                                                                                           deviceless                      no_connection_id         
 fw-clk-pixel                        0       0        0        0           0          0     50000      Y   deviceless                      no_connection_id         
 fw-clk-isp                          1       1        0        500000000   0          0     50000      Y   1000880000.pisp_be              o_connection_id         
                                                                                                           deviceless                      no_connection_id         
 fw-clk-v3d                          0       0        0        500000000   0          0     50000      Y   1002000000.v3d                  o_connection_id         
                                                                                                           deviceless                      no_connection_id         
 fw-clk-core                         1       1        0        500000000   0          0     50000      Y   107c580000.hvs                  core                     
                                                                                                           deviceless                      no_connection_id         
 fw-clk-arm                          0       0        0        1500000000  0          0     50000      Y   cpu0                            no_connection_id         
                                                                                                           cpu0                            no_connection_id         
                                                                                                           deviceless                      no_connection_id         
 core                                1       1        0        50000000    0          0     50000      Y   1f00188000.dma                  core-clk                 
                                                                                                           deviceless                      no_connection_id         
 src                                 0       0        0        1000000000  0          0     50000      Y   deviceless                      no_connection_id         
 hclk                                1       1        0        200000000   0          0     50000      Y   1f00100000.ethernet             hclk                     
                                                                                                           deviceless                      no_connection_id         
 pclk                                1       1        0        200000000   0          0     50000      Y   1f00100000.ethernet             pclk                     
                                                                                                           deviceless                      no_connection_id         
 xosc                                5       5        0        50000000    0          0     50000      Y   deviceless                      no_connection_id         
    clksrc_mipi1_dsi_byteclk         0       0        0        0           0          0     50000      Y      deviceless                      no_connection_id         
    clksrc_mipi0_dsi_byteclk         0       0        0        0           0          0     50000      Y      deviceless                      no_connection_id         
    clk_gp5                          0       0        0        50000000    0          0     50000      Y      deviceless                      no_connection_id         
    clk_gp4                          0       0        0        50000000    0          0     50000      Y      deviceless                      no_connection_id         
    clk_gp3                          0       0        0        50000000    0          0     50000      Y      deviceless                      no_connection_id         
    clk_gp0                          0       0        0        50000000    0          0     50000      Y      deviceless                      no_connection_id         
    clk_sdio_timer                   0       0        0        1000000     0          0     50000      Y      deviceless                      no_connection_id         
       clk_gp1                       0       0        0        1000000     0          0     50000      Y         deviceless                      no_connection_id         
    clk_adc                          1       1        0        50000000    0          0     50000      Y      1f000c8000.adc                  no_connection_id         
                                                                                                              deviceless                      no_connection_id         
    clk_eth_tsu                      1       1        0        50000000    0          0     50000      Y      1f00100000.ethernet             tsu_clk                  
                                                                                                              1f00100000.ethernet             tsu_clk                  
                                                                                                              deviceless                      no_connection_id         
    clk_mipi1_cfg                    0       0        0        25000000    0          0     50000      Y      1f00128000.csi                  o_connection_id         
                                                                                                              deviceless                      no_connection_id         
    clk_mipi0_cfg                    0       0        0        50000000    0          0     50000      Y      deviceless                      no_connection_id         
    clk_i2s                          0       0        0        50000000    0          0     50000      Y      deviceless                      no_connection_id         
    clk_uart                         0       0        0        50000000    0          0     50000      Y      deviceless                      no_connection_id         
    clk_slow_sys                     0       0        0        50000000    0          0     50000      Y      deviceless                      no_connection_id         
    pll_video_core                   1       1        0        0           0          0     50000      Y      deviceless                      no_connection_id         
       pll_video_sec                 0       0        0        0           0          0     50000      Y         deviceless                      no_connection_id         
       pll_video                     0       0        0        0           0          0     50000      Y         deviceless                      no_connection_id         
          pll_video_pri_ph           0       0        0        0           0          0     50000      Y            deviceless                      no_connection_id         
    pll_audio_core                   1       1        0        1536000001  0          0     50000      Y      deviceless                      no_connection_id         
       pll_audio_tern                0       0        0        80842106    0          0     50000      Y         deviceless                      no_connection_id         
       pll_audio_sec                 0       0        0        192000001   0          0     50000      Y         deviceless                      no_connection_id         
       pll_audio                     0       0        0        61440000    0          0     50000      Y         deviceless                      no_connection_id         
          pll_audio_pri_ph           0       0        0        30720000    0          0     50000      Y            deviceless                      no_connection_id         
    pll_sys_core                     3       3        0        1000000000  0          0     50000      Y      deviceless                      no_connection_id         
       pll_sys_sec                   2       2        0        125000000   0          0     50000      Y         deviceless                      no_connection_id         
          clk_eth                    1       1        0        125000000   0          0     50000      Y            1f00100000.ethernet             tx_clk                   
                                                                                                                    deviceless                      no_connection_id         
       pll_sys                       1       1        0        200000000   0          0     50000      Y         deviceless                      no_connection_id         
          clk_mipi1_dpi              0       0        0        200000000   0          0     50000      Y            deviceless                      no_connection_id         
          clk_mipi0_dpi              0       0        0        200000000   0          0     50000      Y            deviceless                      no_connection_id         
          clk_dpi                    0       0        0        200000000   0          0     50000      Y            deviceless                      no_connection_id         
          clk_sdio_alt_src           0       0        0        200000000   0          0     50000      Y            deviceless                      no_connection_id         
             clk_gp2                 0       0        0        200000000   0          0     50000      Y               deviceless                      no_connection_id         
          clk_sys                    1       1        0        200000000   0          0     50000      Y            1f00080000.i2c                  no_connection_id         
                                                                                                                    1f00188000.dma                  cfgr-clk                 
                                                                                                                    deviceless                      no_connection_id         
          pll_sys_pri_ph             0       0        0        100000000   0          0     50000      Y            deviceless                      no_connection_id         
             clk_vec                 0       0        0        100000000   0          0     50000      Y               deviceless                      no_connection_id         
 emmc2-clock                         2       2        0        200000000   0          0     50000      Y   1001100000.mmc                  no_connection_id         
                                                                                                           1000fff000.mmc                  no_connection_id         
                                                                                                           deviceless                      no_connection_id         
 uart-clock                          1       2        0        44236800    0          0     50000      Y   107d001000.serial               no_connection_id         
                                                                                                           deviceless                      no_connection_id         
 vpu-clock                           2       2        0        750000000   0          0     50000      Y   107d004000.spi                  no_connection_id         
                                                                                                           107d001000.serial               apb_pclk                 
                                                                                                           deviceless                      no_connection_id         
 otg                                 1       1        0        480000000   0          0     50000      Y   1000480000.usb                  otg                      
                                                                                                           deviceless                      no_connection_id         
 osc                                 0       0        0        54000000    0          0     50000      Y   deviceless                      no_connection_id         
 108MHz-clock                        1       1        0        108000000   0          0     50000      Y   deviceless                      no_connection_id         
    hdmi1-108MHz                     0       0        0        108000000   0          0     50000      N      107c706400.hdmi                 audio                    
                                                                                                              deviceless                      no_connection_id         
    hdmi0-108MHz                     1       1        0        108000000   0          0     50000      Y      107c701400.hdmi                 audio                    
                                                                                                              deviceless                      no_connection_id         
 27MHz-clock                         1       1        0        27000000    0          0     50000      Y   107c706400.hdmi                 cec                      
                                                                                                           107c701400.hdmi                 cec                      
                                                                                                           107d517a80.pwm                  no_connection_id         
                                                                                                           deviceless                      no_connection_id         
 clk_audio_out                       0       0        0        0           0          0     50000      Y   deviceless                      no_connection_id         
 clk_audio_in                        0       0        0        0           0          0     50000      Y   deviceless                      no_connection_id         
 clk_pwm1                            0       0        0        0           0          0     50000      Y   deviceless                      no_connection_id         
 clk_pwm0                            0       0        0        0           0          0     50000      Y   deviceless                      no_connection_id   

[P33M]

What happens if you force the ethernet link to 100Base-TX? The kernel shouldn't try to reconfigure clk_eth.

Edit: hclk and pclk are directly connected to clk_sys, but are still orphaned in that list above. Do you just declare those two as having clk_sys as the source?

[6by9]

What happens if you force the ethernet link to 100Base-TX? The kernel shouldn't try to reconfigure clk_eth.

Just following through on that one. Looks like it fails, so needs addressing.

Edit: hclk and pclk are directly connected to clk_sys, but are still orphaned in that list above. Do you just declare those two as having clk_sys as the source?

I've not changed that. They're base clocks in https://github.com/raspberrypi/linux/blob/rpi-6.6.y/arch/arm64/boot/dts/broadcom/rp1.dtsi#L1196-L1207, hence being orphaned. If they're actually off clk_sys, then the driver needs updating to add them.

[P33M]

This is scary: clk_slow_sys 0 0 0 50000000 0 0 50000 Y deviceless no_connection_id Which exists on downstream kernels too. Sure enough, its control register is zeroed. This should never be turned off.

[6by9]

This is scary: clk_slow_sys 0 0 0 50000000 0 0 50000 Y deviceless no_connection_id Which exists on downstream kernels too. Sure enough, its control register is zeroed. This should never be turned off.

This is running on a downstream 6.6.60 kernel. It'll get rejigged for mainline afterwards.

Easy to add clk_slow_sys as a critical clock, but what does it feed?!

[P33M]

Lots of important stuff that Linux doesn't know about. E.g. the downstream parts of the pad control registers (!) - which begs the question, how is pad control working at all?

[P33M]

Ah. Someone's thought of that. Neither clk_sys nor clk_slow_sys have enable bits implemented in their control registers.

[6by9]

I'll make clk_sys and clk_slow_sys critical to make it more obvious what's going on.

I'm just observing that RP1_ETH_CLK does not have an assigned rate in rp1.dtsi. Presumably the 125MHz I'm seeing is therefore coming from the firmware config of it, but really ought to be configured by the kernel.

The simplest approach for ethernet looks to be looks to have the MACB_CAPS_CLK_HW_CHG flag set so macb_set_tx_clk aborts immediately, as used on sama7g5_gem_config. The description for daafa1d33cc9286d4b17ad87a18df55687fd0ab6 looks fairly close to our situation. That does mean a custom compatible string though, so a bit of a faff. C'est la vie.

[P33M]

The design intent is for clk_eth to run at 125MHz for all modes of RGMII operation. A tx_clk at the appropriate rate is generated by selection of SPEED_MODE bits in the network control register, so we behave identically to the sama7g5 implementation.

[6by9]

Updated again.

New compatible on the ethernet MAC to set the MACB_CAPS_CLK_HW_CHG flag, so we can now change speed happily.

Any further comments?

[6by9]

Oh, we've still to resolve the question over hclk and pclk. Time to read the docs for them.

[6by9]

As discussed with Jonathan, hclk and pclk are connected to clk_sys, so redefined in DT to match that.

[6by9]

RP1 DMA (used by I2S) is doing funny things.

With "rp1: clk: Remove CLK_IGNORE_UNUSED flags", when dma_request_chan from snd_dmaengine_pcm_register results in a call to dma_chan_alloc_chan_resources and axi_chan_is_hw_enable, the return value of register DMAC_CHEN is 0xffffffff, when previously it returned 0x0.

rp1_dma is defined in DT as using CLK_SYS and sdhci_core clocks, but CLK_SYS is already marked as critical, and sdhci_core is a fixed clock. So is that correct, or is it using some other clock which we're now disabling as the kernel believes it is idle? (I'll got for a process of elimination to try and find which clock is required).

[6by9]

So the answer appears to be RP1_PLL_SYS_PRI_PH is the clock that is required for DMA to be able to read registers Point DT for rp1_dma at that instead of RP1_SYS_CLK, and I2S works again.

The binding for dw-axi-dmac says core-clk should be the Bus Clock (currently pointing at sdhci_core), and cfgr-clk should be the Module Clock (was RP1_CLK_SYS). @P33M @naushir Is this correct?

[P33M]

The DMAC has its own core clock divider, and I think "core" in the clk-summary list should be it

[P33M]

The binding description is a bit mangled. There are no APB registers - at least not in our version of the hardware. There's just a flat set of configuration registers, clocked from clk_sys ("Bus clock" isn't a core clock). The internal state machines are/can be clocked from a different source - in our implementation, clk_dma ("Module clock" isn't a configuration clock).

[6by9]

So RP1's DMA block does have clock control, but it wasn't defined as a clock or used. That lead to the parent (RP1_PLL_SYS_PRI_PH) being disabled, which broke DMA.

Additional patches added to define rp1_clk_dma, and actually use it. I2S now works again.

[P33M]

There are two remaining warts relating to SD/MMC - sdhci_core and sdio_src. There's "phy" in the sense that the PLL_SYS VCO frequency is directly used to generate sdio_src via rp1-clk-sdio, but sdhci_core is bogus.

However these interfaces are not used on Pi 5 so don't hold upstreaming.

[6by9]

Any further comments on this?

I do need to investigate why PLL_SYS_SEC needs to be critical, but we lose ethernet if it isn't, even though CLK_ETH is registering to use it. I haven't done a register comparison yet to figure out why, but that doesn't need to hold this up.

Fixing sdhci_core and sdio_src is a lower priority as the sdhci block isn't used (perhaps they should just be removed).

[P33M]

Are there any other cases where a PLL output may go through an enabled -> Linux disables -> Linux attempts to enable cycle? Could be a bug in setup there, as that won't have been rigorously tested (PLLs are switched on by default).

[P33M]

There's probably a bug here -

CLK_IS_CRITICAL:
root@raspberrypi:/sys/kernel/debug/clk# cat pll_sys_pri_ph/regdump
ph_reg = 0x00051010
root@raspberrypi:/sys/kernel/debug/clk# cat pll_sys_sec/regdump
sec = 0x80000800
root@raspberrypi:/sys/kernel/debug/clk# cat pll_sys/regdump
prim = 0x00051010
root@raspberrypi:/sys/kernel/debug/clk# cat pll_sys_core/regdump
cs = 0x80000001
pwr = 0x00000004
fbdiv_int = 0x00000014
fbdiv_frac = 0x00000000

Clock isn't critical:
root@raspberrypi:/sys/kernel/debug/clk# cat pll_sys_pri_ph/regdump
ph_reg = 0x00051010
root@raspberrypi:/sys/kernel/debug/clk# cat pll_sys_sec/regdump
sec = 0x80000000
root@raspberrypi:/sys/kernel/debug/clk# cat pll_sys/regdump
prim = 0x00051010
root@raspberrypi:/sys/kernel/debug/clk# cat pll_sys_core/regdump
cs = 0x80000001
pwr = 0x00000004
fbdiv_int = 0x00000014
fbdiv_frac = 0x00000000

The secondary divider gets a bogus setup (0 = 19, if I remember).

[P33M]

With no IS_CRITICAL flag, the divider gets turned on/off during boot. This tramples the divisor - and I never see a corresponding _set_rate call which is the only thing that could restore these bits. Surely if you're setting up a downstream clock, you demand that the parent produces the correct rate?

[P33M]

This noddy patch that preserves the existing divisor keeps Ethernet working -

diff --git a/drivers/clk/clk-rp1.c b/drivers/clk/clk-rp1.c
index 69ca9309b3b1..401356a236dd 100644
--- a/drivers/clk/clk-rp1.c
+++ b/drivers/clk/clk-rp1.c
@@ -927,7 +927,8 @@ static void rp1_pll_divider_off(struct clk_hw *hw)
        const struct rp1_pll_data *data = divider->data;

        spin_lock(&clockman->regs_lock);
-       clockman_write(clockman, data->ctrl_reg, PLL_SEC_RST);
+       clockman_write(clockman, data->ctrl_reg,
+                      clockman_read(clockman, data->ctrl_reg) | PLL_SEC_RST);
        spin_unlock(&clockman->regs_lock);
 }

but that glosses over the fact that PLL postdividers other than the ones downstream of pll_audio never get a set_rate call, so this just inherits the firmware clock setup.

[6by9]

I think the right solution is that there should be a load of assigned-clocks and assigned-clock-rates defined in DT, as we have at https://github.com/raspberrypi/linux/blob/rpi-6.6.y/arch/arm64/boot/dts/broadcom/rp1.dtsi#L30-L57

CLK_ETH isn't in that list, so we have no frequency defined. I'll try that fix quickly, but otherwise would like to get this merged.

[6by9]

No it doesn't solve the problem.

RP1_PLL_SYS_SEC was already in the list, so I was expecting that to have set the rate anyway, but it seems not. More research required on assigned-clocks at some point. #6479 to track that as it isn't critical.

Are all happy enough for now?