| Message ID | 20211125153131.163533-7-geert@linux-m68k.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | riscv: dts: Miscellaneous fixes | expand |
On 25/11/2021 16:31, Geert Uytterhoeven wrote: > "make dtbs_check" reports: > > arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dt.yaml: soc: refclk: {'compatible': ['fixed-clock'], '#clock-cells': [[0]], 'clock-frequency': [[600000000]], 'clock-output-names': ['msspllclk'], 'phandle': [[7]]} should not be valid under {'type': 'object'} > From schema: dtschema/schemas/simple-bus.yaml > > Fix this by moving the node out of the "soc" subnode. > While at it, rename it to "msspllclk", and drop the now superfluous > "clock-output-names" property. > > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> > --- > arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > It is also logical because refclk usually is not a property of the SoC. It actually might be a property of board... Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> Best regards, Krzysztof
On 26/11/2021 09:48, Krzysztof Kozlowski wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 25/11/2021 16:31, Geert Uytterhoeven wrote: >> "make dtbs_check" reports: >> >> arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dt.yaml: soc: refclk: {'compatible': ['fixed-clock'], '#clock-cells': [[0]], 'clock-frequency': [[600000000]], 'clock-output-names': ['msspllclk'], 'phandle': [[7]]} should not be valid under {'type': 'object'} >> From schema: dtschema/schemas/simple-bus.yaml >> >> Fix this by moving the node out of the "soc" subnode. >> While at it, rename it to "msspllclk", and drop the now superfluous >> "clock-output-names" property. >> >> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> >> --- >> arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi | 13 ++++++------- >> 1 file changed, 6 insertions(+), 7 deletions(-) >> > > It is also logical because refclk usually is not a property of the SoC. > It actually might be a property of board... This is one of the fun FPGAisms like the GPIO interrupt configuration. This clock setting is determined by what design has been loaded onto the FPGA - the msspll outputs are configurable, I could redo my FPGA design and change this to 500 MHz etc. In turn the msspll clock is set by another clock source that is actually on the board of either 100 or 125 MHz. Since it's not set at bitstream programming time, I would agree that that property should be moved to out of mpfs.dtsi. > > > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> > > > Best regards, > Krzysztof >
On 26/11/2021 10:16, conor wrote: > On 26/11/2021 09:48, Krzysztof Kozlowski wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know >> the content is safe >> >> On 25/11/2021 16:31, Geert Uytterhoeven wrote: >>> "make dtbs_check" reports: >>> >>> arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dt.yaml: >>> soc: refclk: {'compatible': ['fixed-clock'], '#clock-cells': [[0]], >>> 'clock-frequency': [[600000000]], 'clock-output-names': >>> ['msspllclk'], 'phandle': [[7]]} should not be valid under {'type': >>> 'object'} >>> From schema: dtschema/schemas/simple-bus.yaml >>> >>> Fix this by moving the node out of the "soc" subnode. >>> While at it, rename it to "msspllclk", and drop the now superfluous >>> "clock-output-names" property. >>> >>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> >>> --- >>> arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi | 13 ++++++------- >>> 1 file changed, 6 insertions(+), 7 deletions(-) >>> >> >> It is also logical because refclk usually is not a property of the SoC. >> It actually might be a property of board... > This is one of the fun FPGAisms like the GPIO interrupt configuration. > This clock setting is determined by what design has been loaded onto the > FPGA - the msspll outputs are configurable, I could redo my FPGA design > and change this to 500 MHz etc. In turn the msspll clock is set by > another clock source that is actually on the board of either 100 or 125 > MHz. > > Since it's not set at bitstream programming time, I would agree that > that property should be moved to out of mpfs.dtsi. Since it's not set **UNTIL** bitstream programming time >> >> >> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> >> >> >> Best regards, >> Krzysztof >> >
On 26/11/2021 10:16, conor wrote: > On 26/11/2021 09:48, Krzysztof Kozlowski wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know >> the content is safe >> >> On 25/11/2021 16:31, Geert Uytterhoeven wrote: >>> "make dtbs_check" reports: >>> >>> arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dt.yaml: >>> soc: refclk: {'compatible': ['fixed-clock'], '#clock-cells': [[0]], >>> 'clock-frequency': [[600000000]], 'clock-output-names': >>> ['msspllclk'], 'phandle': [[7]]} should not be valid under {'type': >>> 'object'} >>> From schema: dtschema/schemas/simple-bus.yaml >>> >>> Fix this by moving the node out of the "soc" subnode. >>> While at it, rename it to "msspllclk", and drop the now superfluous >>> "clock-output-names" property. >>> >>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> >>> --- >>> arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi | 13 ++++++------- >>> 1 file changed, 6 insertions(+), 7 deletions(-) >>> >> >> It is also logical because refclk usually is not a property of the SoC. >> It actually might be a property of board... > This is one of the fun FPGAisms like the GPIO interrupt configuration. > This clock setting is determined by what design has been loaded onto the > FPGA - the msspll outputs are configurable, I could redo my FPGA design > and change this to 500 MHz etc. In turn the msspll clock is set by > another clock source that is actually on the board of either 100 or 125 > MHz. > > Since it's not set until bitstream programming time, I would agree that > that property should be moved to out of mpfs.dtsi. (typo fixed) Geert/Krzysztof, Would the following make sense: - Since the refclk hardware is a part of the chip, move the refclk out of the soc node but leave it in mfps.dtsi - The clk freq itself is set by the fpga bitstream, so move the clock-frequency property to mpfs-icicle-kit.dts? >> >> >> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> >> >> >> Best regards, >> Krzysztof >> >
On 03/12/2021 16:29, Conor.Dooley@microchip.com wrote: > On 26/11/2021 10:16, conor wrote: >> On 26/11/2021 09:48, Krzysztof Kozlowski wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know >>> the content is safe >>> >>> On 25/11/2021 16:31, Geert Uytterhoeven wrote: >>>> "make dtbs_check" reports: >>>> >>>> arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dt.yaml: >>>> soc: refclk: {'compatible': ['fixed-clock'], '#clock-cells': [[0]], >>>> 'clock-frequency': [[600000000]], 'clock-output-names': >>>> ['msspllclk'], 'phandle': [[7]]} should not be valid under {'type': >>>> 'object'} >>>> From schema: dtschema/schemas/simple-bus.yaml >>>> >>>> Fix this by moving the node out of the "soc" subnode. >>>> While at it, rename it to "msspllclk", and drop the now superfluous >>>> "clock-output-names" property. >>>> >>>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> >>>> --- >>>> arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi | 13 ++++++------- >>>> 1 file changed, 6 insertions(+), 7 deletions(-) >>>> >>> >>> It is also logical because refclk usually is not a property of the SoC. >>> It actually might be a property of board... >> This is one of the fun FPGAisms like the GPIO interrupt configuration. >> This clock setting is determined by what design has been loaded onto the >> FPGA - the msspll outputs are configurable, I could redo my FPGA design >> and change this to 500 MHz etc. In turn the msspll clock is set by >> another clock source that is actually on the board of either 100 or 125 >> MHz. >> >> Since it's not set until bitstream programming time, I would agree that >> that property should be moved to out of mpfs.dtsi. (typo fixed) > Geert/Krzysztof, > Would the following make sense: > - Since the refclk hardware is a part of the chip, move the refclk out > of the soc node but leave it in mfps.dtsi > - The clk freq itself is set by the fpga bitstream, so move the > clock-frequency property to mpfs-icicle-kit.dts? Yes, makes sense to me. Best regards, Krzysztof
Hi Conor, On Fri, Dec 3, 2021 at 4:30 PM <Conor.Dooley@microchip.com> wrote: > On 26/11/2021 10:16, conor wrote: > > On 26/11/2021 09:48, Krzysztof Kozlowski wrote: > >> EXTERNAL EMAIL: Do not click links or open attachments unless you know > >> the content is safe > >> On 25/11/2021 16:31, Geert Uytterhoeven wrote: > >>> "make dtbs_check" reports: > >>> > >>> arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dt.yaml: > >>> soc: refclk: {'compatible': ['fixed-clock'], '#clock-cells': [[0]], > >>> 'clock-frequency': [[600000000]], 'clock-output-names': > >>> ['msspllclk'], 'phandle': [[7]]} should not be valid under {'type': > >>> 'object'} > >>> From schema: dtschema/schemas/simple-bus.yaml > >>> > >>> Fix this by moving the node out of the "soc" subnode. > >>> While at it, rename it to "msspllclk", and drop the now superfluous > >>> "clock-output-names" property. > >>> > >>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> > >>> --- > >>> arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi | 13 ++++++------- > >>> 1 file changed, 6 insertions(+), 7 deletions(-) > >>> > >> > >> It is also logical because refclk usually is not a property of the SoC. > >> It actually might be a property of board... > > This is one of the fun FPGAisms like the GPIO interrupt configuration. > > This clock setting is determined by what design has been loaded onto the > > FPGA - the msspll outputs are configurable, I could redo my FPGA design > > and change this to 500 MHz etc. In turn the msspll clock is set by > > another clock source that is actually on the board of either 100 or 125 > > MHz. > > > > Since it's not set until bitstream programming time, I would agree that > > that property should be moved to out of mpfs.dtsi. (typo fixed) > > Geert/Krzysztof, > Would the following make sense: > - Since the refclk hardware is a part of the chip, move the refclk out > of the soc node but leave it in mfps.dtsi > - The clk freq itself is set by the fpga bitstream, so move the > clock-frequency property to mpfs-icicle-kit.dts? That was exactly what I had in mind when I read your previous email. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
diff --git a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi index c71d2d682fc0a0e7..893864cf2447a9d4 100644 --- a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi +++ b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi @@ -139,6 +139,12 @@ cpu4_intc: interrupt-controller { }; }; + refclk: msspllclk { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <600000000>; + }; + soc { #address-cells = <2>; #size-cells = <2>; @@ -190,13 +196,6 @@ dma@3000000 { #dma-cells = <1>; }; - refclk: refclk { - compatible = "fixed-clock"; - #clock-cells = <0>; - clock-frequency = <600000000>; - clock-output-names = "msspllclk"; - }; - clkcfg: clkcfg@20002000 { compatible = "microchip,mpfs-clkcfg"; reg = <0x0 0x20002000 0x0 0x1000>;
"make dtbs_check" reports: arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dt.yaml: soc: refclk: {'compatible': ['fixed-clock'], '#clock-cells': [[0]], 'clock-frequency': [[600000000]], 'clock-output-names': ['msspllclk'], 'phandle': [[7]]} should not be valid under {'type': 'object'} From schema: dtschema/schemas/simple-bus.yaml Fix this by moving the node out of the "soc" subnode. While at it, rename it to "msspllclk", and drop the now superfluous "clock-output-names" property. Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> --- arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)