RP1 clocks #6459
RP1 clocks #6459
Conversation
6by9
force-pushed
the
rpi-6.6.y-rp1-clks
branch
from
1061d14
to
6bf48c4
Compare
|
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 |
|
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? |
Just following through on that one. Looks like it fails, so needs addressing.
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. |
|
This is scary: |
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?! |
|
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? |
|
Ah. Someone's thought of that. Neither clk_sys nor clk_slow_sys have enable bits implemented in their control registers. |
|
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 |
|
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
force-pushed
the
rpi-6.6.y-rp1-clks
branch
from
6bf48c4
to
5da90e6
Compare
|
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? |
|
Oh, we've still to resolve the question over hclk and pclk. Time to read the docs for them. |
|
As discussed with Jonathan, hclk and pclk are connected to clk_sys, so redefined in DT to match that. |
|
RP1 DMA (used by I2S) is doing funny things. With "rp1: clk: Remove CLK_IGNORE_UNUSED flags", when 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 the answer appears to be The binding for dw-axi-dmac says core-clk should be the Bus Clock (currently pointing at |
|
The DMAC has its own core clock divider, and I think "core" in the clk-summary list should be it |
|
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
force-pushed
the
rpi-6.6.y-rp1-clks
branch
from
e596f5b
to
33a5416
Compare
|
So RP1's DMA block does have clock control, but it wasn't defined as a clock or used. That lead to the parent ( Additional patches added to define rp1_clk_dma, and actually use it. I2S now works again. |
6by9
force-pushed
the
rpi-6.6.y-rp1-clks
branch
from
33a5416
to
c5ef134
Compare
|
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. |
|
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). |
|
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). |
|
There's probably a bug here - The secondary divider gets a bogus setup (0 = 19, if I remember). |
|
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? |
|
This noddy patch that preserves the existing divisor keeps Ethernet working - 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. |
|
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. |
|
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? |
The Raspberry Pi RP1 chip has the Cadence GEM ethernet controller, so add a compatible string for it. Signed-off-by: Dave Stevenson <[email protected]>
The RP1 chip has the Cadence GEM block, but wants the tx_clock to always run at 125MHz, in the same way as sama7g5. Add the relevant configuration. Signed-off-by: Dave Stevenson <[email protected]>
Rather than clearing all the bits in rp1_pll_divider_off and setting PLL_SEC_RST, retain the status of all the other bits. Signed-off-by: Dave Stevenson <[email protected]>
The clock setup had been copied from clk-bcm2835 which had to cope with the firmware having configured clocks, so there were flags of CLK_IS_CRITICAL and CLK_IGNORE_UNUSED dotted around. That isn't the situation with RP1 where only the main PLLs, CLK_SYS, and CLK_SLOW_SYS are critical, so update the configuration to match. Signed-off-by: Dave Stevenson <[email protected]>
Configure RP1's ethernet block to do the correct thing. clk_eth is intended to be fixed at 125MHz, so use a new compatible, and use assigned-clocks to configure the clock appropriately. Signed-off-by: Dave Stevenson <[email protected]>
The DMA block has a clock, but wasn't defined in the driver. This resulted in the parent being disabled as unused, and then DMA stopped working. Signed-off-by: Dave Stevenson <[email protected]>
There should be no issue in disabling the RP1 clocks as long as the kernel knows about all consumers. Signed-off-by: Dave Stevenson <[email protected]>
hclk and pclk of the MAC are connected to clk_sys, so define them as being connected accordingly, rather than having fake fixed clocks for them. Signed-off-by: Dave Stevenson <[email protected]>
This makes the kernel representation of the clock structure match reality. Signed-off-by: Dave Stevenson <[email protected]>
6by9
force-pushed
the
rpi-6.6.y-rp1-clks
branch
from
c5ef134
to
56a949d
Compare
|
RP1_PLL_SYS_SEC fixed up by not clearing register bits in rp1_pll_divider_off. Branch rebased whilst at it. That should close off all issues (I hope). |
For discussion over critical clocks, and those that shouldn't be disabled.