On Fri, Apr 22, 2022 at 2:59 PM Conor Dooley <mail@xxxxxxxxxxx> wrote: > > On 22/04/2022 20:39, Palmer Dabbelt wrote: > > On Wed, 13 Apr 2022 00:58:27 PDT (-0700), conor.dooley@xxxxxxxxxxxxx wrote: > >> Hey all, > >> After the clock driver for the PolarFire SoC was accepted I started work > >> on the onboard RTC & found out that the reference clock for the rtc was > >> actually missing from the clock driver. > >> > >> While restructuring the clock driver to add support for the rtc > >> reference, I also noticed that there were some problems with how the FIC > >> clocks were being used. The FIC clocks are the cpu side inputs to the > >> AXI fabric interconnections & are not the clocks for any peripherals. > >> > >> This first three patches in this series fixes the problems with the FICs: > >> - the fic clocks incorrectly had the AHB clock as their parents > >> - the last fic, named differently to the others, had not been set as > >> a critical clock > >> - some peripherals on the fabric side were incorrectly using the cpu > >> side fic clocks, resulting in incorrect rates. > >> > >> The second part of the series fixes the missing rtc reference clock. > >> There are three main changes: > >> - Changing the reference clock in the dt to the external 125 MHz > >> oscillator rather than using the output of an internal pll. This has > >> the added benefit of not requiring changes to the device tree if this > >> part of the bitstream changes. > >> - Adding a new clock into the driver that sits above the existing > >> configurable clocks & has the external reference as a parent. The new > >> clock provides the parent for the AHB/AXI clocks which formerly came > >> from the device tree. > >> - Adding the rtc reference clock to the dt bindings, device tree and > >> clock driver at the configurable clock level, alongside AXI and AHB. > >> > >> I kept series separate from [0] since that's tied to the CONFIG_PM stuff > >> & fixes a specific problem. > >> > >> Changes since v1: > >> After speaking with Krzysztof, I have merged the rtc reference changes > >> [1] with these fixes for 5.18. This was done since the relevant drivers > >> and bindings only arrived in v5.18 & there'll now be no issue with > >> breaking the ABI. > >> Backwards compatiblity with the device tree from before 5.18 will be > >> broken by these changes, but the board did not boot then anyway... If > >> that is not okay, please lmk. > >> > >> The patch renaming sys_base was dropped since that's not a fix. > >> > >> Version 1 would not apply without [0] & that should be fixed too. > >> > >> Thanks, > >> Conor. > >> > >> Changes since v2: > >> - Wrapped text in dt-binding changes at 80 cols > >> - Ordered the clock defines numerically in the binding header > >> - Fixed the Fixes tag on the last patch and added the second tag > >> > >> [0] https://lore.kernel.org/linux-riscv/20220408143646.3693104-1-conor.dooley@xxxxxxxxxxxxx > >> [1] https://lore.kernel.org/linux-riscv/20220411072340.740981-1-conor.dooley@xxxxxxxxxxxxx > >> > >> Conor Dooley (9): > >> clk: microchip: mpfs: fix parents for FIC clocks > >> clk: microchip: mpfs: mark CLK_ATHENA as critical > >> riscv: dts: microchip: fix usage of fic clocks on mpfs > >> dt-bindings: clk: mpfs document msspll dri registers > >> dt-bindings: clk: mpfs: add defines for two new clocks > >> dt-bindings: rtc: add refclk to mpfs-rtc > >> clk: microchip: mpfs: re-parent the configurable clocks > >> clk: microchip: mpfs: add RTCREF clock control > >> riscv: dts: microchip: reparent mpfs clocks > >> > >> .../bindings/clock/microchip,mpfs.yaml | 13 +- > >> .../bindings/rtc/microchip,mfps-rtc.yaml | 15 +- > >> .../dts/microchip/microchip-mpfs-fabric.dtsi | 16 +- > >> .../microchip/microchip-mpfs-icicle-kit.dts | 2 +- > >> .../boot/dts/microchip/microchip-mpfs.dtsi | 10 +- > >> drivers/clk/microchip/clk-mpfs.c | 191 +++++++++++++++--- > >> .../dt-bindings/clock/microchip,mpfs-clock.h | 5 +- > >> 7 files changed, 211 insertions(+), 41 deletions(-) > > > > Thanks. These generally look good to me, but I don't see acks from everyone. I'm perfectly fine treating these as fixes and taking them through the RISC-V tree, but looks like it's mostly clk stuff so > > > > Acked-by: Palmer Dabbelt <palmer@xxxxxxxxxxxx> > > Ye, hopefully they go via clk. Stephen replied to v1 or 2 so > I figure they're in his queue :) > > > > > in case someone else wants to take it. I've put these over at palmer/riscv-pfsoc-clk but haven't merged that anywhere, I'll hold off until at least next week to give everyone time to chime in. > > > > On a somewhat related note, I'm getting some DT schema failures > > /scratch/riscv-systems-ci-fixes/check/dt_check/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dtb: /: memory@80000000: 'clocks' does not match any of the regexes: 'pinctrl-[0-9]+' > > From schema: /home/palmer/.local/lib/python3.8/site-packages/dtschema-2022.3.2-py3.8.egg/dtschema/schemas/memory.yaml > > /scratch/riscv-systems-ci-fixes/check/dt_check/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dtb: /: memory@1000000000: 'clocks' does not match any of the regexes: 'pinctrl-[0-9]+' > > From schema: /home/palmer/.local/lib/python3.8/site-packages/dtschema-2022.3.2-py3.8.egg/dtschema/schemas/memory.yaml > > /scratch/riscv-systems-ci-fixes/check/dt_check/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dtb: soc: syscontroller: {'compatible': ['microchip,mpfs-sys-controller'], 'mboxes': [[15, 0]], 'status': ['okay']} should not be valid under {'type': 'object'} > > From schema: /home/palmer/.local/lib/python3.8/site-packages/dtschema-2022.3.2-py3.8.egg/dtschema/schemas/simple-bus.yaml > > Looks like none of them are new from this patch set, though. Atul's been chasing down various DT schema failures so they might be fixed already. > > Ye, I do know about those. I meant to try deleting the clocks > property but didn't get a chance, just been busy. It's not > related to this series nor does it matter if it makes it prior > to 5.18 so I was going to submit it on its own. > The other is in my list-of-things-to-ask-Rob/Krzk-when-I-know- > for-sure-what-my-question-actually-is... microchip,mpfs-sys-controller doesn't have MMIO registers, so it's not on a MMIO bus (aka simple-bus). Move it to an appropriate location (top level if not part of something else). Rob