diff mbox series

[v2,3/8] riscv: dts: thead: Add TH1520 pin control nodes

Message ID 20240103132852.298964-4-emil.renner.berthing@canonical.com
State New
Headers show
Series Add T-Head TH1520 SoC pin control | expand

Commit Message

Emil Renner Berthing Jan. 3, 2024, 1:28 p.m. UTC
Add nodes for pin controllers on the T-Head TH1520 RISC-V SoC.

Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
---
 .../boot/dts/thead/th1520-beaglev-ahead.dts   |  4 ++++
 .../dts/thead/th1520-lichee-module-4a.dtsi    |  4 ++++
 arch/riscv/boot/dts/thead/th1520.dtsi         | 24 +++++++++++++++++++
 3 files changed, 32 insertions(+)

Comments

Conor Dooley Jan. 8, 2024, 5:34 p.m. UTC | #1
On Wed, Jan 03, 2024 at 02:28:40PM +0100, Emil Renner Berthing wrote:
> Add nodes for pin controllers on the T-Head TH1520 RISC-V SoC.
> 
> Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> ---
>  .../boot/dts/thead/th1520-beaglev-ahead.dts   |  4 ++++
>  .../dts/thead/th1520-lichee-module-4a.dtsi    |  4 ++++
>  arch/riscv/boot/dts/thead/th1520.dtsi         | 24 +++++++++++++++++++
>  3 files changed, 32 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> index 70e8042c8304..6c56318a8705 100644
> --- a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> +++ b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> @@ -44,6 +44,10 @@ &osc_32k {
>  	clock-frequency = <32768>;
>  };
>  
> +&aonsys_clk {
> +	clock-frequency = <73728000>;
> +};
> +
>  &apb_clk {
>  	clock-frequency = <62500000>;
>  };
> diff --git a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> index a802ab110429..9865925be372 100644
> --- a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> +++ b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> @@ -25,6 +25,10 @@ &osc_32k {
>  	clock-frequency = <32768>;
>  };
>  
> +&aonsys_clk {
> +	clock-frequency = <73728000>;
> +};
> +
>  &apb_clk {
>  	clock-frequency = <62500000>;
>  };
> diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
> index ba4d2c673ac8..e65a306ff575 100644
> --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> @@ -134,6 +134,12 @@ osc_32k: 32k-oscillator {
>  		#clock-cells = <0>;
>  	};
>  
> +	aonsys_clk: aonsys-clk {
> +		compatible = "fixed-clock";
> +		clock-output-names = "aonsys_clk";
> +		#clock-cells = <0>;
> +	};

Did this stuff sneak into this commit accidentally?
Emil Renner Berthing Jan. 9, 2024, 12:02 p.m. UTC | #2
Conor Dooley wrote:
> On Wed, Jan 03, 2024 at 02:28:40PM +0100, Emil Renner Berthing wrote:
> > Add nodes for pin controllers on the T-Head TH1520 RISC-V SoC.
> >
> > Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> > ---
> >  .../boot/dts/thead/th1520-beaglev-ahead.dts   |  4 ++++
> >  .../dts/thead/th1520-lichee-module-4a.dtsi    |  4 ++++
> >  arch/riscv/boot/dts/thead/th1520.dtsi         | 24 +++++++++++++++++++
> >  3 files changed, 32 insertions(+)
> >
> > diff --git a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > index 70e8042c8304..6c56318a8705 100644
> > --- a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > +++ b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > @@ -44,6 +44,10 @@ &osc_32k {
> >  	clock-frequency = <32768>;
> >  };
> >
> > +&aonsys_clk {
> > +	clock-frequency = <73728000>;
> > +};
> > +
> >  &apb_clk {
> >  	clock-frequency = <62500000>;
> >  };
> > diff --git a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> > index a802ab110429..9865925be372 100644
> > --- a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> > +++ b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> > @@ -25,6 +25,10 @@ &osc_32k {
> >  	clock-frequency = <32768>;
> >  };
> >
> > +&aonsys_clk {
> > +	clock-frequency = <73728000>;
> > +};
> > +
> >  &apb_clk {
> >  	clock-frequency = <62500000>;
> >  };
> > diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
> > index ba4d2c673ac8..e65a306ff575 100644
> > --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> > +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> > @@ -134,6 +134,12 @@ osc_32k: 32k-oscillator {
> >  		#clock-cells = <0>;
> >  	};
> >
> > +	aonsys_clk: aonsys-clk {
> > +		compatible = "fixed-clock";
> > +		clock-output-names = "aonsys_clk";
> > +		#clock-cells = <0>;
> > +	};
>
> Did this stuff sneak into this commit accidentally?

Not really by accident no. It turns out the clock tree has gates for the bus
clock of each pinctrl block and I think it's better to add this clock
dependency to the bindings and driver up front.

Since there is not yet any clock driver the initial device tree for the TH1520
included the dummy apb_clk that two of the pinctrl blocks derive their clock
from, but not the "aonsys" clock needed by the "always-on" pinctrl. I thought
it was better to add this dummy clock with the only (so far) user of it, but if
you have a better idea, let me know.

/Emil
Conor Dooley Jan. 9, 2024, 1:04 p.m. UTC | #3
On Tue, Jan 09, 2024 at 04:02:01AM -0800, Emil Renner Berthing wrote:
> Conor Dooley wrote:
> > On Wed, Jan 03, 2024 at 02:28:40PM +0100, Emil Renner Berthing wrote:
> > > Add nodes for pin controllers on the T-Head TH1520 RISC-V SoC.
> > >
> > > Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> > > ---
> > >  .../boot/dts/thead/th1520-beaglev-ahead.dts   |  4 ++++
> > >  .../dts/thead/th1520-lichee-module-4a.dtsi    |  4 ++++
> > >  arch/riscv/boot/dts/thead/th1520.dtsi         | 24 +++++++++++++++++++
> > >  3 files changed, 32 insertions(+)
> > >
> > > diff --git a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > > index 70e8042c8304..6c56318a8705 100644
> > > --- a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > > +++ b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > > @@ -44,6 +44,10 @@ &osc_32k {
> > >  	clock-frequency = <32768>;
> > >  };
> > >
> > > +&aonsys_clk {
> > > +	clock-frequency = <73728000>;
> > > +};
> > > +
> > >  &apb_clk {
> > >  	clock-frequency = <62500000>;
> > >  };
> > > diff --git a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> > > index a802ab110429..9865925be372 100644
> > > --- a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> > > +++ b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> > > @@ -25,6 +25,10 @@ &osc_32k {
> > >  	clock-frequency = <32768>;
> > >  };
> > >
> > > +&aonsys_clk {
> > > +	clock-frequency = <73728000>;
> > > +};
> > > +
> > >  &apb_clk {
> > >  	clock-frequency = <62500000>;
> > >  };
> > > diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
> > > index ba4d2c673ac8..e65a306ff575 100644
> > > --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> > > +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> > > @@ -134,6 +134,12 @@ osc_32k: 32k-oscillator {
> > >  		#clock-cells = <0>;
> > >  	};
> > >
> > > +	aonsys_clk: aonsys-clk {
> > > +		compatible = "fixed-clock";
> > > +		clock-output-names = "aonsys_clk";
> > > +		#clock-cells = <0>;
> > > +	};
> >
> > Did this stuff sneak into this commit accidentally?
> 
> Not really by accident no. It turns out the clock tree has gates for the bus
> clock of each pinctrl block and I think it's better to add this clock
> dependency to the bindings and driver up front.

Maybe if I had looked a wee bit more deeply I would've noticed that it
was used there, but it's always good to mention the rationale in the
commit message so that it's more obvious why you're doin it.

> Since there is not yet any clock driver the initial device tree for the TH1520
> included the dummy apb_clk that two of the pinctrl blocks derive their clock
> from, but not the "aonsys" clock needed by the "always-on" pinctrl. I thought
> it was better to add this dummy clock with the only (so far) user of it, but if
> you have a better idea, let me know.

No, that's fine. I was just wondering why there was an unmentioned set
of clocks being added. If they're stubbed fixed clocks I dunno if it
makes sense to add them to the board.dts/module.dtsi files though. Where
do the initial values come from for the rates? Out of reset values or
set by firmware that may vary from board to board?

Cheers,
Conor.
Emil Renner Berthing Jan. 9, 2024, 2:28 p.m. UTC | #4
Conor Dooley wrote:
> On Tue, Jan 09, 2024 at 04:02:01AM -0800, Emil Renner Berthing wrote:
> > Conor Dooley wrote:
> > > On Wed, Jan 03, 2024 at 02:28:40PM +0100, Emil Renner Berthing wrote:
> > > > Add nodes for pin controllers on the T-Head TH1520 RISC-V SoC.
> > > >
> > > > Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> > > > ---
> > > >  .../boot/dts/thead/th1520-beaglev-ahead.dts   |  4 ++++
> > > >  .../dts/thead/th1520-lichee-module-4a.dtsi    |  4 ++++
> > > >  arch/riscv/boot/dts/thead/th1520.dtsi         | 24 +++++++++++++++++++
> > > >  3 files changed, 32 insertions(+)
> > > >
> > > > diff --git a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > > > index 70e8042c8304..6c56318a8705 100644
> > > > --- a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > > > +++ b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > > > @@ -44,6 +44,10 @@ &osc_32k {
> > > >  	clock-frequency = <32768>;
> > > >  };
> > > >
> > > > +&aonsys_clk {
> > > > +	clock-frequency = <73728000>;
> > > > +};
> > > > +
> > > >  &apb_clk {
> > > >  	clock-frequency = <62500000>;
> > > >  };
> > > > diff --git a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> > > > index a802ab110429..9865925be372 100644
> > > > --- a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> > > > +++ b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> > > > @@ -25,6 +25,10 @@ &osc_32k {
> > > >  	clock-frequency = <32768>;
> > > >  };
> > > >
> > > > +&aonsys_clk {
> > > > +	clock-frequency = <73728000>;
> > > > +};
> > > > +
> > > >  &apb_clk {
> > > >  	clock-frequency = <62500000>;
> > > >  };
> > > > diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
> > > > index ba4d2c673ac8..e65a306ff575 100644
> > > > --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> > > > +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> > > > @@ -134,6 +134,12 @@ osc_32k: 32k-oscillator {
> > > >  		#clock-cells = <0>;
> > > >  	};
> > > >
> > > > +	aonsys_clk: aonsys-clk {
> > > > +		compatible = "fixed-clock";
> > > > +		clock-output-names = "aonsys_clk";
> > > > +		#clock-cells = <0>;
> > > > +	};
> > >
> > > Did this stuff sneak into this commit accidentally?
> >
> > Not really by accident no. It turns out the clock tree has gates for the bus
> > clock of each pinctrl block and I think it's better to add this clock
> > dependency to the bindings and driver up front.
>
> Maybe if I had looked a wee bit more deeply I would've noticed that it
> was used there, but it's always good to mention the rationale in the
> commit message so that it's more obvious why you're doin it.

You absolutely right. I forgot to update the commit message.

> > Since there is not yet any clock driver the initial device tree for the TH1520
> > included the dummy apb_clk that two of the pinctrl blocks derive their clock
> > from, but not the "aonsys" clock needed by the "always-on" pinctrl. I thought
> > it was better to add this dummy clock with the only (so far) user of it, but if
> > you have a better idea, let me know.
>
> No, that's fine. I was just wondering why there was an unmentioned set
> of clocks being added. If they're stubbed fixed clocks I dunno if it
> makes sense to add them to the board.dts/module.dtsi files though. Where
> do the initial values come from for the rates? Out of reset values or
> set by firmware that may vary from board to board?

The vendor u-boot sets the PLLs different from the reset values. For now I
think it's the same code for every board using the Lichee Pi 4A module (and
probably also for the BeagleV Ahead), but it might still make sense to move the
freqency to the board instead of the module device tree.

/Emil
Conor Dooley Jan. 9, 2024, 5:34 p.m. UTC | #5
On Tue, Jan 09, 2024 at 06:28:19AM -0800, Emil Renner Berthing wrote:
> Conor Dooley wrote:
> > On Tue, Jan 09, 2024 at 04:02:01AM -0800, Emil Renner Berthing wrote:
> > > Conor Dooley wrote:
> > > > On Wed, Jan 03, 2024 at 02:28:40PM +0100, Emil Renner Berthing wrote:
> > > > > Add nodes for pin controllers on the T-Head TH1520 RISC-V SoC.
> > > > >
> > > > > Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> > > > > ---
> > > > >  .../boot/dts/thead/th1520-beaglev-ahead.dts   |  4 ++++
> > > > >  .../dts/thead/th1520-lichee-module-4a.dtsi    |  4 ++++
> > > > >  arch/riscv/boot/dts/thead/th1520.dtsi         | 24 +++++++++++++++++++
> > > > >  3 files changed, 32 insertions(+)
> > > > >
> > > > > diff --git a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > > > > index 70e8042c8304..6c56318a8705 100644
> > > > > --- a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > > > > +++ b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > > > > @@ -44,6 +44,10 @@ &osc_32k {
> > > > >  	clock-frequency = <32768>;
> > > > >  };
> > > > >
> > > > > +&aonsys_clk {
> > > > > +	clock-frequency = <73728000>;
> > > > > +};
> > > > > +
> > > > >  &apb_clk {
> > > > >  	clock-frequency = <62500000>;
> > > > >  };
> > > > > diff --git a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> > > > > index a802ab110429..9865925be372 100644
> > > > > --- a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> > > > > +++ b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> > > > > @@ -25,6 +25,10 @@ &osc_32k {
> > > > >  	clock-frequency = <32768>;
> > > > >  };
> > > > >
> > > > > +&aonsys_clk {
> > > > > +	clock-frequency = <73728000>;
> > > > > +};
> > > > > +
> > > > >  &apb_clk {
> > > > >  	clock-frequency = <62500000>;
> > > > >  };
> > > > > diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
> > > > > index ba4d2c673ac8..e65a306ff575 100644
> > > > > --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> > > > > +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> > > > > @@ -134,6 +134,12 @@ osc_32k: 32k-oscillator {
> > > > >  		#clock-cells = <0>;
> > > > >  	};
> > > > >
> > > > > +	aonsys_clk: aonsys-clk {
> > > > > +		compatible = "fixed-clock";
> > > > > +		clock-output-names = "aonsys_clk";
> > > > > +		#clock-cells = <0>;
> > > > > +	};
> > > >
> > > > Did this stuff sneak into this commit accidentally?
> > >
> > > Not really by accident no. It turns out the clock tree has gates for the bus
> > > clock of each pinctrl block and I think it's better to add this clock
> > > dependency to the bindings and driver up front.
> >
> > Maybe if I had looked a wee bit more deeply I would've noticed that it
> > was used there, but it's always good to mention the rationale in the
> > commit message so that it's more obvious why you're doin it.
> 
> You absolutely right. I forgot to update the commit message.
> 
> > > Since there is not yet any clock driver the initial device tree for the TH1520
> > > included the dummy apb_clk that two of the pinctrl blocks derive their clock
> > > from, but not the "aonsys" clock needed by the "always-on" pinctrl. I thought
> > > it was better to add this dummy clock with the only (so far) user of it, but if
> > > you have a better idea, let me know.
> >
> > No, that's fine. I was just wondering why there was an unmentioned set
> > of clocks being added. If they're stubbed fixed clocks I dunno if it
> > makes sense to add them to the board.dts/module.dtsi files though. Where
> > do the initial values come from for the rates? Out of reset values or
> > set by firmware that may vary from board to board?
> 
> The vendor u-boot sets the PLLs different from the reset values. For now I
> think it's the same code for every board using the Lichee Pi 4A module (and
> probably also for the BeagleV Ahead), but it might still make sense to move the
> freqency to the board instead of the module device tree.

Yeah, think so. Only temporarily though, do you have a clue if anyone is
working on the actual clock driver stuff? Seems pretty Deadge?
https://lore.kernel.org/linux-clk/?q=th1520

Cheers,
Conor.
Drew Fustini Jan. 9, 2024, 6:30 p.m. UTC | #6
On Tue, Jan 09, 2024 at 05:34:11PM +0000, Conor Dooley wrote:
> On Tue, Jan 09, 2024 at 06:28:19AM -0800, Emil Renner Berthing wrote:
> > Conor Dooley wrote:
> > > On Tue, Jan 09, 2024 at 04:02:01AM -0800, Emil Renner Berthing wrote:
> > > > Conor Dooley wrote:
> > > > > On Wed, Jan 03, 2024 at 02:28:40PM +0100, Emil Renner Berthing wrote:
> > > > > > Add nodes for pin controllers on the T-Head TH1520 RISC-V SoC.
> > > > > >
> > > > > > Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> > > > > > ---
> > > > > >  .../boot/dts/thead/th1520-beaglev-ahead.dts   |  4 ++++
> > > > > >  .../dts/thead/th1520-lichee-module-4a.dtsi    |  4 ++++
> > > > > >  arch/riscv/boot/dts/thead/th1520.dtsi         | 24 +++++++++++++++++++
> > > > > >  3 files changed, 32 insertions(+)
> > > > > >
> > > > > > diff --git a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > > > > > index 70e8042c8304..6c56318a8705 100644
> > > > > > --- a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > > > > > +++ b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
> > > > > > @@ -44,6 +44,10 @@ &osc_32k {
> > > > > >  	clock-frequency = <32768>;
> > > > > >  };
> > > > > >
> > > > > > +&aonsys_clk {
> > > > > > +	clock-frequency = <73728000>;
> > > > > > +};
> > > > > > +
> > > > > >  &apb_clk {
> > > > > >  	clock-frequency = <62500000>;
> > > > > >  };
> > > > > > diff --git a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> > > > > > index a802ab110429..9865925be372 100644
> > > > > > --- a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> > > > > > +++ b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
> > > > > > @@ -25,6 +25,10 @@ &osc_32k {
> > > > > >  	clock-frequency = <32768>;
> > > > > >  };
> > > > > >
> > > > > > +&aonsys_clk {
> > > > > > +	clock-frequency = <73728000>;
> > > > > > +};
> > > > > > +
> > > > > >  &apb_clk {
> > > > > >  	clock-frequency = <62500000>;
> > > > > >  };
> > > > > > diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
> > > > > > index ba4d2c673ac8..e65a306ff575 100644
> > > > > > --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> > > > > > +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> > > > > > @@ -134,6 +134,12 @@ osc_32k: 32k-oscillator {
> > > > > >  		#clock-cells = <0>;
> > > > > >  	};
> > > > > >
> > > > > > +	aonsys_clk: aonsys-clk {
> > > > > > +		compatible = "fixed-clock";
> > > > > > +		clock-output-names = "aonsys_clk";
> > > > > > +		#clock-cells = <0>;
> > > > > > +	};
> > > > >
> > > > > Did this stuff sneak into this commit accidentally?
> > > >
> > > > Not really by accident no. It turns out the clock tree has gates for the bus
> > > > clock of each pinctrl block and I think it's better to add this clock
> > > > dependency to the bindings and driver up front.
> > >
> > > Maybe if I had looked a wee bit more deeply I would've noticed that it
> > > was used there, but it's always good to mention the rationale in the
> > > commit message so that it's more obvious why you're doin it.
> > 
> > You absolutely right. I forgot to update the commit message.
> > 
> > > > Since there is not yet any clock driver the initial device tree for the TH1520
> > > > included the dummy apb_clk that two of the pinctrl blocks derive their clock
> > > > from, but not the "aonsys" clock needed by the "always-on" pinctrl. I thought
> > > > it was better to add this dummy clock with the only (so far) user of it, but if
> > > > you have a better idea, let me know.
> > >
> > > No, that's fine. I was just wondering why there was an unmentioned set
> > > of clocks being added. If they're stubbed fixed clocks I dunno if it
> > > makes sense to add them to the board.dts/module.dtsi files though. Where
> > > do the initial values come from for the rates? Out of reset values or
> > > set by firmware that may vary from board to board?
> > 
> > The vendor u-boot sets the PLLs different from the reset values. For now I
> > think it's the same code for every board using the Lichee Pi 4A module (and
> > probably also for the BeagleV Ahead), but it might still make sense to move the
> > freqency to the board instead of the module device tree.
> 
> Yeah, think so. Only temporarily though, do you have a clue if anyone is
> working on the actual clock driver stuff? Seems pretty Deadge?
> https://lore.kernel.org/linux-clk/?q=th1520

Yes, I am working on it. Jisheng passed me his work-in-progress based on
that original patch you linked to. I've been trying to work out an issue
with the emmc clock but it seems timely to share what I currently have.
I will post an RFC today.

Thanks,
Drew
diff mbox series

Patch

diff --git a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
index 70e8042c8304..6c56318a8705 100644
--- a/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
+++ b/arch/riscv/boot/dts/thead/th1520-beaglev-ahead.dts
@@ -44,6 +44,10 @@  &osc_32k {
 	clock-frequency = <32768>;
 };
 
+&aonsys_clk {
+	clock-frequency = <73728000>;
+};
+
 &apb_clk {
 	clock-frequency = <62500000>;
 };
diff --git a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
index a802ab110429..9865925be372 100644
--- a/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
+++ b/arch/riscv/boot/dts/thead/th1520-lichee-module-4a.dtsi
@@ -25,6 +25,10 @@  &osc_32k {
 	clock-frequency = <32768>;
 };
 
+&aonsys_clk {
+	clock-frequency = <73728000>;
+};
+
 &apb_clk {
 	clock-frequency = <62500000>;
 };
diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
index ba4d2c673ac8..e65a306ff575 100644
--- a/arch/riscv/boot/dts/thead/th1520.dtsi
+++ b/arch/riscv/boot/dts/thead/th1520.dtsi
@@ -134,6 +134,12 @@  osc_32k: 32k-oscillator {
 		#clock-cells = <0>;
 	};
 
+	aonsys_clk: aonsys-clk {
+		compatible = "fixed-clock";
+		clock-output-names = "aonsys_clk";
+		#clock-cells = <0>;
+	};
+
 	apb_clk: apb-clk-clock {
 		compatible = "fixed-clock";
 		clock-output-names = "apb_clk";
@@ -242,6 +248,12 @@  portd: gpio-controller@0 {
 			};
 		};
 
+		padctrl1_apsys: pinctrl@ffe7f3c000 {
+			compatible = "thead,th1520-group2-pinctrl";
+			reg = <0xff 0xe7f3c000 0x0 0x1000>;
+			clocks = <&apb_clk>;
+		};
+
 		gpio0: gpio@ffec005000 {
 			compatible = "snps,dw-apb-gpio";
 			reg = <0xff 0xec005000 0x0 0x1000>;
@@ -278,6 +290,12 @@  portb: gpio-controller@0 {
 			};
 		};
 
+		padctrl0_apsys: pinctrl@ffec007000 {
+			compatible = "thead,th1520-group3-pinctrl";
+			reg = <0xff 0xec007000 0x0 0x1000>;
+			clocks = <&apb_clk>;
+		};
+
 		uart2: serial@ffec010000 {
 			compatible = "snps,dw-apb-uart";
 			reg = <0xff 0xec010000 0x0 0x4000>;
@@ -414,6 +432,12 @@  porte: gpio-controller@0 {
 			};
 		};
 
+		padctrl_aosys: pinctrl@fffff4a000 {
+			compatible = "thead,th1520-group1-pinctrl";
+			reg = <0xff 0xfff4a000 0x0 0x2000>;
+			clocks = <&aonsys_clk>;
+		};
+
 		ao_gpio1: gpio@fffff52000 {
 			compatible = "snps,dw-apb-gpio";
 			reg = <0xff 0xfff52000 0x0 0x1000>;