| Message ID | 25fcbab4c04bcbbdc4577dc58822540829f91dc9.1699879741.git.unicorn_wang@outlook.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | riscv: sophgo: add clock support for sg2042 | expand |
On Mon, Nov 13, 2023 at 09:20:11PM +0800, Chen Wang wrote: > From: Chen Wang <unicorn_wang@outlook.com> > > Add clock generator node to device tree for SG2042, and enable clock for > uart0. > > Signed-off-by: Chen Wang <unicorn_wang@outlook.com> > --- > arch/riscv/boot/dts/sophgo/sg2042-clock.dtsi | 76 ++++++++++++++++++++ There's no need to create an entirely new file for this. > arch/riscv/boot/dts/sophgo/sg2042.dtsi | 10 +++ > 2 files changed, 86 insertions(+) > create mode 100644 arch/riscv/boot/dts/sophgo/sg2042-clock.dtsi > > diff --git a/arch/riscv/boot/dts/sophgo/sg2042-clock.dtsi b/arch/riscv/boot/dts/sophgo/sg2042-clock.dtsi > new file mode 100644 > index 000000000000..66d2723fab35 > --- /dev/null > +++ b/arch/riscv/boot/dts/sophgo/sg2042-clock.dtsi > @@ -0,0 +1,76 @@ > +// SPDX-License-Identifier: (GPL-2.0 OR MIT) > +/* > + * Copyright (C) 2023 Sophgo Technology Inc. All rights reserved. > + */ > + > +/ { > + cgi: oscillator { > + compatible = "fixed-clock"; > + clock-frequency = <25000000>; > + clock-output-names = "cgi"; > + #clock-cells = <0>; > + }; What actually is this oscillator? Is it provided by another clock controller on the SoC, or is it provided by an oscillator on the board? > + > + clkgen: clock-controller { > + compatible = "sophgo,sg2042-clkgen"; > + #clock-cells = <1>; > + system-ctrl = <&sys_ctrl>; Why is this node not a child of the system controller? Cheers, Conor.
On 2023/11/15 1:31, Conor Dooley wrote: > On Mon, Nov 13, 2023 at 09:20:11PM +0800, Chen Wang wrote: >> From: Chen Wang <unicorn_wang@outlook.com> >> >> Add clock generator node to device tree for SG2042, and enable clock for >> uart0. >> >> Signed-off-by: Chen Wang <unicorn_wang@outlook.com> >> --- >> arch/riscv/boot/dts/sophgo/sg2042-clock.dtsi | 76 ++++++++++++++++++++ > There's no need to create an entirely new file for this. Agree, I will merge this into sg2042.dtsi in next revision. > >> arch/riscv/boot/dts/sophgo/sg2042.dtsi | 10 +++ >> 2 files changed, 86 insertions(+) >> create mode 100644 arch/riscv/boot/dts/sophgo/sg2042-clock.dtsi >> >> diff --git a/arch/riscv/boot/dts/sophgo/sg2042-clock.dtsi b/arch/riscv/boot/dts/sophgo/sg2042-clock.dtsi >> new file mode 100644 >> index 000000000000..66d2723fab35 >> --- /dev/null >> +++ b/arch/riscv/boot/dts/sophgo/sg2042-clock.dtsi >> @@ -0,0 +1,76 @@ >> +// SPDX-License-Identifier: (GPL-2.0 OR MIT) >> +/* >> + * Copyright (C) 2023 Sophgo Technology Inc. All rights reserved. >> + */ >> + >> +/ { >> + cgi: oscillator { >> + compatible = "fixed-clock"; >> + clock-frequency = <25000000>; >> + clock-output-names = "cgi"; >> + #clock-cells = <0>; >> + }; > What actually is this oscillator? > Is it provided by another clock controller on the SoC, or is it provided > by an oscillator on the board? This oscillator is an individual ic chip outside the SoC on the board, that's why I list it outside soc node. Actually the "cgi" is abbrevation for "Clock Generation IC chip". >> + >> + clkgen: clock-controller { >> + compatible = "sophgo,sg2042-clkgen"; >> + #clock-cells = <1>; >> + system-ctrl = <&sys_ctrl>; > Why is this node not a child of the system controller? I will move this node to child of syscon in next revision, thanks for your reminder. > > Cheers, > Conor.
On 2023-11-14 7:34 PM, Chen Wang wrote: > On 2023/11/15 1:31, Conor Dooley wrote: >> On Mon, Nov 13, 2023 at 09:20:11PM +0800, Chen Wang wrote: >>> From: Chen Wang <unicorn_wang@outlook.com> >>> >>> Add clock generator node to device tree for SG2042, and enable clock for >>> uart0. >>> >>> Signed-off-by: Chen Wang <unicorn_wang@outlook.com> >>> --- >>> arch/riscv/boot/dts/sophgo/sg2042-clock.dtsi | 76 ++++++++++++++++++++ >> There's no need to create an entirely new file for this. > Agree, I will merge this into sg2042.dtsi in next revision. >> >>> arch/riscv/boot/dts/sophgo/sg2042.dtsi | 10 +++ >>> 2 files changed, 86 insertions(+) >>> create mode 100644 arch/riscv/boot/dts/sophgo/sg2042-clock.dtsi >>> >>> diff --git a/arch/riscv/boot/dts/sophgo/sg2042-clock.dtsi >>> b/arch/riscv/boot/dts/sophgo/sg2042-clock.dtsi >>> new file mode 100644 >>> index 000000000000..66d2723fab35 >>> --- /dev/null >>> +++ b/arch/riscv/boot/dts/sophgo/sg2042-clock.dtsi >>> @@ -0,0 +1,76 @@ >>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT) >>> +/* >>> + * Copyright (C) 2023 Sophgo Technology Inc. All rights reserved. >>> + */ >>> + >>> +/ { >>> + cgi: oscillator { >>> + compatible = "fixed-clock"; >>> + clock-frequency = <25000000>; >>> + clock-output-names = "cgi"; >>> + #clock-cells = <0>; >>> + }; >> What actually is this oscillator? >> Is it provided by another clock controller on the SoC, or is it provided >> by an oscillator on the board? > > This oscillator is an individual ic chip outside the SoC on the board, that's > why I list it outside soc node. > > Actually the "cgi" is abbrevation for "Clock Generation IC chip". Since the oscillator is outside the SoC, this node (or at least its clock-frequency property) belongs in the board devicetree, not the SoC .dtsi. See [1]. Regards, Samuel [1]: https://lore.kernel.org/linux-riscv/b5401052-e803-9788-64d6-82b2737533ce@linaro.org/
On 2023/11/15 10:15, Samuel Holland wrote: > On 2023-11-14 7:34 PM, Chen Wang wrote: >> On 2023/11/15 1:31, Conor Dooley wrote: >>> On Mon, Nov 13, 2023 at 09:20:11PM +0800, Chen Wang wrote: >>>> From: Chen Wang <unicorn_wang@outlook.com> >>>> >>>> Add clock generator node to device tree for SG2042, and enable clock for >>>> uart0. >>>> >>>> Signed-off-by: Chen Wang <unicorn_wang@outlook.com> >>>> --- >>>> arch/riscv/boot/dts/sophgo/sg2042-clock.dtsi | 76 ++++++++++++++++++++ >>> There's no need to create an entirely new file for this. >> Agree, I will merge this into sg2042.dtsi in next revision. >>>> arch/riscv/boot/dts/sophgo/sg2042.dtsi | 10 +++ >>>> 2 files changed, 86 insertions(+) >>>> create mode 100644 arch/riscv/boot/dts/sophgo/sg2042-clock.dtsi >>>> >>>> diff --git a/arch/riscv/boot/dts/sophgo/sg2042-clock.dtsi >>>> b/arch/riscv/boot/dts/sophgo/sg2042-clock.dtsi >>>> new file mode 100644 >>>> index 000000000000..66d2723fab35 >>>> --- /dev/null >>>> +++ b/arch/riscv/boot/dts/sophgo/sg2042-clock.dtsi >>>> @@ -0,0 +1,76 @@ >>>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT) >>>> +/* >>>> + * Copyright (C) 2023 Sophgo Technology Inc. All rights reserved. >>>> + */ >>>> + >>>> +/ { >>>> + cgi: oscillator { >>>> + compatible = "fixed-clock"; >>>> + clock-frequency = <25000000>; >>>> + clock-output-names = "cgi"; >>>> + #clock-cells = <0>; >>>> + }; >>> What actually is this oscillator? >>> Is it provided by another clock controller on the SoC, or is it provided >>> by an oscillator on the board? >> This oscillator is an individual ic chip outside the SoC on the board, that's >> why I list it outside soc node. >> >> Actually the "cgi" is abbrevation for "Clock Generation IC chip". > Since the oscillator is outside the SoC, this node (or at least its > clock-frequency property) belongs in the board devicetree, not the SoC .dtsi. > See [1]. > > Regards, > Samuel > > [1]: > https://lore.kernel.org/linux-riscv/b5401052-e803-9788-64d6-82b2737533ce@linaro.org/ Thank you Samuel, I have learned this and will correct this in next revision.
diff --git a/arch/riscv/boot/dts/sophgo/sg2042-clock.dtsi b/arch/riscv/boot/dts/sophgo/sg2042-clock.dtsi new file mode 100644 index 000000000000..66d2723fab35 --- /dev/null +++ b/arch/riscv/boot/dts/sophgo/sg2042-clock.dtsi @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: (GPL-2.0 OR MIT) +/* + * Copyright (C) 2023 Sophgo Technology Inc. All rights reserved. + */ + +/ { + cgi: oscillator { + compatible = "fixed-clock"; + clock-frequency = <25000000>; + clock-output-names = "cgi"; + #clock-cells = <0>; + }; + + clkgen: clock-controller { + compatible = "sophgo,sg2042-clkgen"; + #clock-cells = <1>; + system-ctrl = <&sys_ctrl>; + clocks = <&cgi>; + assigned-clocks = \ + <&clkgen DIV_CLK_FPLL_RP_CPU_NORMAL_1>, + <&clkgen DIV_CLK_FPLL_50M_A53>, + <&clkgen DIV_CLK_FPLL_TOP_RP_CMN_DIV2>, + <&clkgen DIV_CLK_FPLL_UART_500M>, + <&clkgen DIV_CLK_FPLL_AHB_LPC>, + <&clkgen DIV_CLK_FPLL_EFUSE>, + <&clkgen DIV_CLK_FPLL_TX_ETH0>, + <&clkgen DIV_CLK_FPLL_PTP_REF_I_ETH0>, + <&clkgen DIV_CLK_FPLL_REF_ETH0>, + <&clkgen DIV_CLK_FPLL_EMMC>, + <&clkgen DIV_CLK_FPLL_SD>, + <&clkgen DIV_CLK_FPLL_TOP_AXI0>, + <&clkgen DIV_CLK_FPLL_TOP_AXI_HSPERI>, + <&clkgen DIV_CLK_FPLL_AXI_DDR_1>, + <&clkgen DIV_CLK_FPLL_DIV_TIMER1>, + <&clkgen DIV_CLK_FPLL_DIV_TIMER2>, + <&clkgen DIV_CLK_FPLL_DIV_TIMER3>, + <&clkgen DIV_CLK_FPLL_DIV_TIMER4>, + <&clkgen DIV_CLK_FPLL_DIV_TIMER5>, + <&clkgen DIV_CLK_FPLL_DIV_TIMER6>, + <&clkgen DIV_CLK_FPLL_DIV_TIMER7>, + <&clkgen DIV_CLK_FPLL_DIV_TIMER8>, + <&clkgen DIV_CLK_FPLL_100K_EMMC>, + <&clkgen DIV_CLK_FPLL_100K_SD>, + <&clkgen DIV_CLK_FPLL_GPIO_DB>, + <&clkgen DIV_CLK_MPLL_RP_CPU_NORMAL_0>, + <&clkgen DIV_CLK_MPLL_AXI_DDR_0>; + assigned-clock-rates = \ + <2000000000>, + <50000000>, + <1000000000>, + <500000000>, + <200000000>, + <25000000>, + <125000000>, + <50000000>, + <25000000>, + <100000000>, + <100000000>, + <100000000>, + <250000000>, + <1000000000>, + <50000000>, + <50000000>, + <50000000>, + <50000000>, + <50000000>, + <50000000>, + <50000000>, + <50000000>, + <100000>, + <100000>, + <100000>, + <2000000000>, + <1000000000>; + }; +}; diff --git a/arch/riscv/boot/dts/sophgo/sg2042.dtsi b/arch/riscv/boot/dts/sophgo/sg2042.dtsi index 93256540d078..de79c0cdb4c1 100644 --- a/arch/riscv/boot/dts/sophgo/sg2042.dtsi +++ b/arch/riscv/boot/dts/sophgo/sg2042.dtsi @@ -5,8 +5,10 @@ /dts-v1/; #include <dt-bindings/interrupt-controller/irq.h> +#include <dt-bindings/clock/sophgo-sg2042-clk.h> #include "sg2042-cpus.dtsi" +#include "sg2042-clock.dtsi" / { compatible = "sophgo,sg2042"; @@ -311,12 +313,20 @@ intc: interrupt-controller@7090000000 { riscv,ndev = <224>; }; + sys_ctrl: syscon@7030010000 { + compatible = "sophgo,sg2042-syscon", "syscon"; + reg = <0x70 0x30010000 0x0 0x8000>; + }; + uart0: serial@7040000000 { compatible = "snps,dw-apb-uart"; reg = <0x00000070 0x40000000 0x00000000 0x00001000>; interrupt-parent = <&intc>; interrupts = <112 IRQ_TYPE_LEVEL_HIGH>; clock-frequency = <500000000>; + clocks = <&clkgen GATE_CLK_UART_500M>, + <&clkgen GATE_CLK_APB_UART>; + clock-names = "baudclk", "apb_pclk"; reg-shift = <2>; reg-io-width = <4>; status = "disabled";