[PATCH 0/4] arm64: dts: mediatek: Fix systimer clock description

[PATCH 0/4] arm64: dts: mediatek: Fix systimer clock description

[Chen-Yu Tsai]

Hi,

This series fixes the clock description for the systimer block. The
systimer is fed by the main 26 MHz oscillator, and internally divides
the clock, normally by 2.

However this ended up being modeled in various incorrect ways, such as
the clock divider being in the TOPCKGEN block, or as a standalone 13 MHz
oscillator.

This series fixes the description of the systimer clock input in an ABI
compatible way, i.e. the clock rate of the input clock remains the same
at 13 MHz. The clock is now modeled as a divide-by-2 fixed factor clock
being fed by the main oscillator.

An added benefit is that in Linux the systimer no longer requires the
main SoC clk driver to do an early init dance.

Please have a look.

The next step would be to fix up the systimer driver in a backward
compatible way and have it read the divider value from hardware.


Regards
ChenYu

Chen-Yu Tsai (4):
  arm64: dts: mediatek: mt8183: Fix systimer 13 MHz clock description
  arm64: dts: mediatek: mt8192: Fix systimer 13 MHz clock description
  arm64: dts: mediatek: mt8195: Fix systimer 13 MHz clock description
  arm64: dts: mediatek: mt8186: Fix systimer 13 MHz clock description

 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 12 ++++++++++--
 arch/arm64/boot/dts/mediatek/mt8186.dtsi |  8 +++++---
 arch/arm64/boot/dts/mediatek/mt8192.dtsi | 12 ++++++++++--
 arch/arm64/boot/dts/mediatek/mt8195.dtsi | 11 ++++++++++-
 4 files changed, 35 insertions(+), 8 deletions(-)

-- 
2.38.1.584.g0f3c55d4c2-goog

[PATCH 0/4] arm64: dts: mediatek: Fix systimer clock description

[Chen-Yu Tsai]

Hi,

This series fixes the clock description for the systimer block. The
systimer is fed by the main 26 MHz oscillator, and internally divides
the clock, normally by 2.

However this ended up being modeled in various incorrect ways, such as
the clock divider being in the TOPCKGEN block, or as a standalone 13 MHz
oscillator.

This series fixes the description of the systimer clock input in an ABI
compatible way, i.e. the clock rate of the input clock remains the same
at 13 MHz. The clock is now modeled as a divide-by-2 fixed factor clock
being fed by the main oscillator.

An added benefit is that in Linux the systimer no longer requires the
main SoC clk driver to do an early init dance.

Please have a look.

The next step would be to fix up the systimer driver in a backward
compatible way and have it read the divider value from hardware.


Regards
ChenYu

Chen-Yu Tsai (4):
  arm64: dts: mediatek: mt8183: Fix systimer 13 MHz clock description
  arm64: dts: mediatek: mt8192: Fix systimer 13 MHz clock description
  arm64: dts: mediatek: mt8195: Fix systimer 13 MHz clock description
  arm64: dts: mediatek: mt8186: Fix systimer 13 MHz clock description

 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 12 ++++++++++--
 arch/arm64/boot/dts/mediatek/mt8186.dtsi |  8 +++++---
 arch/arm64/boot/dts/mediatek/mt8192.dtsi | 12 ++++++++++--
 arch/arm64/boot/dts/mediatek/mt8195.dtsi | 11 ++++++++++-
 4 files changed, 35 insertions(+), 8 deletions(-)

-- 
2.38.1.584.g0f3c55d4c2-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/4] arm64: dts: mediatek: mt8183: Fix systimer 13 MHz clock description

[Chen-Yu Tsai]

The systimer block derives its 13 MHz clock by dividing the main 26 MHz
oscillator clock by 2 internally, not through the TOPCKGEN clock
controller.

On the MT8183 this divider is set either by power-on-reset or by the
bootloader. The bootloader may then make the divider unconfigurable to,
but can be read out by, the operating system.

Making the systimer block take the 26 MHz clock directly requires
changing the implementations. As an ABI compatible fix, change the
input clock of the systimer block a fixed factor divide-by-2 clock
that takes the 26 MHz oscillator as its input.

Fixes: 5bc8e2875ffb ("arm64: dts: mt8183: add systimer0 device node")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 19ff1babc359..0cbbaebe1213 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -585,6 +585,15 @@ psci {
 		method = "smc";
 	};
 
+	clk13m: fixed-factor-clock-13m {
+		compatible = "fixed-factor-clock";
+		#clock-cells = <0>;
+		clocks = <&clk26m>;
+		clock-div = <2>;
+		clock-mult = <1>;
+		clock-output-names = "clk13m";
+	};
+
 	clk26m: oscillator {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
@@ -968,8 +977,7 @@ systimer: timer@10017000 {
 				     "mediatek,mt6765-timer";
 			reg = <0 0x10017000 0 0x1000>;
 			interrupts = <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&topckgen CLK_TOP_CLK13M>;
-			clock-names = "clk13m";
+			clocks = <&clk13m>;
 		};
 
 		iommu: iommu@10205000 {
-- 
2.38.1.584.g0f3c55d4c2-goog

[PATCH 1/4] arm64: dts: mediatek: mt8183: Fix systimer 13 MHz clock description

[Chen-Yu Tsai]

The systimer block derives its 13 MHz clock by dividing the main 26 MHz
oscillator clock by 2 internally, not through the TOPCKGEN clock
controller.

On the MT8183 this divider is set either by power-on-reset or by the
bootloader. The bootloader may then make the divider unconfigurable to,
but can be read out by, the operating system.

Making the systimer block take the 26 MHz clock directly requires
changing the implementations. As an ABI compatible fix, change the
input clock of the systimer block a fixed factor divide-by-2 clock
that takes the 26 MHz oscillator as its input.

Fixes: 5bc8e2875ffb ("arm64: dts: mt8183: add systimer0 device node")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 19ff1babc359..0cbbaebe1213 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -585,6 +585,15 @@ psci {
 		method = "smc";
 	};
 
+	clk13m: fixed-factor-clock-13m {
+		compatible = "fixed-factor-clock";
+		#clock-cells = <0>;
+		clocks = <&clk26m>;
+		clock-div = <2>;
+		clock-mult = <1>;
+		clock-output-names = "clk13m";
+	};
+
 	clk26m: oscillator {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
@@ -968,8 +977,7 @@ systimer: timer@10017000 {
 				     "mediatek,mt6765-timer";
 			reg = <0 0x10017000 0 0x1000>;
 			interrupts = <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&topckgen CLK_TOP_CLK13M>;
-			clock-names = "clk13m";
+			clocks = <&clk13m>;
 		};
 
 		iommu: iommu@10205000 {
-- 
2.38.1.584.g0f3c55d4c2-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] arm64: dts: mediatek: mt8183: Fix systimer 13 MHz clock description

[AngeloGioacchino Del Regno]

Il 01/12/22 09:42, Chen-Yu Tsai ha scritto:
> The systimer block derives its 13 MHz clock by dividing the main 26 MHz
> oscillator clock by 2 internally, not through the TOPCKGEN clock
> controller.
> 
> On the MT8183 this divider is set either by power-on-reset or by the
> bootloader. The bootloader may then make the divider unconfigurable to,
> but can be read out by, the operating system.
> 
> Making the systimer block take the 26 MHz clock directly requires
> changing the implementations. As an ABI compatible fix, change the
> input clock of the systimer block a fixed factor divide-by-2 clock
> that takes the 26 MHz oscillator as its input.
> 
> Fixes: 5bc8e2875ffb ("arm64: dts: mt8183: add systimer0 device node")
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

I generally not just like - but *love* - this change, I had that in my mind
for a couple of months now and forgot about it because reasons.

There's just one thing that, since we're doing this now, we can clarify (and
that's important to avoid questions like "why isn't this board-specific"):
the 26MHz clock "clk26m" oscillator that we're using for the system timers
is a SoC-provided clock, and its name is "SYSCLK" as in "System bus clock".

I know that your target is to describe how we get from 26M to 13M, but at
this point it may be worth it to use the right names to help preventing
confusion about that clock not being an external crystal on the board but
something internal to the SoC.

So, I propose:
1. Change `clk26m: oscillator` to `clk26m: sysclk` or `clk26m: sysclk-26m`;
2. Add the divider as `clk13m: sysclk-div2`.

What do you think?

Cheers,
Angelo

> ---
>   arch/arm64/boot/dts/mediatek/mt8183.dtsi | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index 19ff1babc359..0cbbaebe1213 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -585,6 +585,15 @@ psci {
>   		method = "smc";
>   	};
>   
> +	clk13m: fixed-factor-clock-13m {
> +		compatible = "fixed-factor-clock";
> +		#clock-cells = <0>;
> +		clocks = <&clk26m>;
> +		clock-div = <2>;
> +		clock-mult = <1>;
> +		clock-output-names = "clk13m";
> +	};
> +
>   	clk26m: oscillator {
>   		compatible = "fixed-clock";
>   		#clock-cells = <0>;
> @@ -968,8 +977,7 @@ systimer: timer@10017000 {
>   				     "mediatek,mt6765-timer";
>   			reg = <0 0x10017000 0 0x1000>;
>   			interrupts = <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>;
> -			clocks = <&topckgen CLK_TOP_CLK13M>;
> -			clock-names = "clk13m";
> +			clocks = <&clk13m>;
>   		};
>   
>   		iommu: iommu@10205000 {

Re: [PATCH 1/4] arm64: dts: mediatek: mt8183: Fix systimer 13 MHz clock description

[AngeloGioacchino Del Regno]

Il 01/12/22 09:42, Chen-Yu Tsai ha scritto:
> The systimer block derives its 13 MHz clock by dividing the main 26 MHz
> oscillator clock by 2 internally, not through the TOPCKGEN clock
> controller.
> 
> On the MT8183 this divider is set either by power-on-reset or by the
> bootloader. The bootloader may then make the divider unconfigurable to,
> but can be read out by, the operating system.
> 
> Making the systimer block take the 26 MHz clock directly requires
> changing the implementations. As an ABI compatible fix, change the
> input clock of the systimer block a fixed factor divide-by-2 clock
> that takes the 26 MHz oscillator as its input.
> 
> Fixes: 5bc8e2875ffb ("arm64: dts: mt8183: add systimer0 device node")
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

I generally not just like - but *love* - this change, I had that in my mind
for a couple of months now and forgot about it because reasons.

There's just one thing that, since we're doing this now, we can clarify (and
that's important to avoid questions like "why isn't this board-specific"):
the 26MHz clock "clk26m" oscillator that we're using for the system timers
is a SoC-provided clock, and its name is "SYSCLK" as in "System bus clock".

I know that your target is to describe how we get from 26M to 13M, but at
this point it may be worth it to use the right names to help preventing
confusion about that clock not being an external crystal on the board but
something internal to the SoC.

So, I propose:
1. Change `clk26m: oscillator` to `clk26m: sysclk` or `clk26m: sysclk-26m`;
2. Add the divider as `clk13m: sysclk-div2`.

What do you think?

Cheers,
Angelo

> ---
>   arch/arm64/boot/dts/mediatek/mt8183.dtsi | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index 19ff1babc359..0cbbaebe1213 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -585,6 +585,15 @@ psci {
>   		method = "smc";
>   	};
>   
> +	clk13m: fixed-factor-clock-13m {
> +		compatible = "fixed-factor-clock";
> +		#clock-cells = <0>;
> +		clocks = <&clk26m>;
> +		clock-div = <2>;
> +		clock-mult = <1>;
> +		clock-output-names = "clk13m";
> +	};
> +
>   	clk26m: oscillator {
>   		compatible = "fixed-clock";
>   		#clock-cells = <0>;
> @@ -968,8 +977,7 @@ systimer: timer@10017000 {
>   				     "mediatek,mt6765-timer";
>   			reg = <0 0x10017000 0 0x1000>;
>   			interrupts = <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>;
> -			clocks = <&topckgen CLK_TOP_CLK13M>;
> -			clock-names = "clk13m";
> +			clocks = <&clk13m>;
>   		};
>   
>   		iommu: iommu@10205000 {



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] arm64: dts: mediatek: mt8183: Fix systimer 13 MHz clock description

[Chen-Yu Tsai]

On Thu, Dec 1, 2022 at 5:31 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 01/12/22 09:42, Chen-Yu Tsai ha scritto:
> > The systimer block derives its 13 MHz clock by dividing the main 26 MHz
> > oscillator clock by 2 internally, not through the TOPCKGEN clock
> > controller.
> >
> > On the MT8183 this divider is set either by power-on-reset or by the
> > bootloader. The bootloader may then make the divider unconfigurable to,
> > but can be read out by, the operating system.
> >
> > Making the systimer block take the 26 MHz clock directly requires
> > changing the implementations. As an ABI compatible fix, change the
> > input clock of the systimer block a fixed factor divide-by-2 clock
> > that takes the 26 MHz oscillator as its input.
> >
> > Fixes: 5bc8e2875ffb ("arm64: dts: mt8183: add systimer0 device node")
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
>
> I generally not just like - but *love* - this change, I had that in my mind
> for a couple of months now and forgot about it because reasons.
>
> There's just one thing that, since we're doing this now, we can clarify (and
> that's important to avoid questions like "why isn't this board-specific"):
> the 26MHz clock "clk26m" oscillator that we're using for the system timers
> is a SoC-provided clock, and its name is "SYSCLK" as in "System bus clock".

Looking at the schematics it is terribly more complicated. :(

The crystal feeds the DCXO in the PMIC, which also acts as a buffer.
The PMIC then feeds the SoC and any other chips, such as a modem.

On other platforms we describe the oscillator at the dtsi level as well.
The reason why it isn't board-specific is that the requirements and
properties of the crystal are specified in the platform's datasheet,
i.e. it is a design requirement that every board use the same crystal.

I don't see the datasheet spelling out SYSCLK though. The TOPCKGEN part
mostly just refers to it as CLK26M, or some variant of it, which likely
denotes some fan-out branch. The system timer part also just says "26M"
or "26 MHz clock source".

Also, we can't change the clock name, as "clk26m" is hard-coded into
the clk drivers.

> I know that your target is to describe how we get from 26M to 13M, but at
> this point it may be worth it to use the right names to help preventing
> confusion about that clock not being an external crystal on the board but
> something internal to the SoC.

But as I described above, it is an actual crystal on the board. We are
simply omitting parts of the signal path. Notably the PMIC needs to be
excluded due to circular dependency reasons. And also we most definitely
don't want the system to be touching it.

ChenYu

> So, I propose:
> 1. Change `clk26m: oscillator` to `clk26m: sysclk` or `clk26m: sysclk-26m`;
> 2. Add the divider as `clk13m: sysclk-div2`.
>
> What do you think?
>
> Cheers,
> Angelo
>
> > ---
> >   arch/arm64/boot/dts/mediatek/mt8183.dtsi | 12 ++++++++++--
> >   1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > index 19ff1babc359..0cbbaebe1213 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > @@ -585,6 +585,15 @@ psci {
> >               method = "smc";
> >       };
> >
> > +     clk13m: fixed-factor-clock-13m {
> > +             compatible = "fixed-factor-clock";
> > +             #clock-cells = <0>;
> > +             clocks = <&clk26m>;
> > +             clock-div = <2>;
> > +             clock-mult = <1>;
> > +             clock-output-names = "clk13m";
> > +     };
> > +
> >       clk26m: oscillator {
> >               compatible = "fixed-clock";
> >               #clock-cells = <0>;
> > @@ -968,8 +977,7 @@ systimer: timer@10017000 {
> >                                    "mediatek,mt6765-timer";
> >                       reg = <0 0x10017000 0 0x1000>;
> >                       interrupts = <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>;
> > -                     clocks = <&topckgen CLK_TOP_CLK13M>;
> > -                     clock-names = "clk13m";
> > +                     clocks = <&clk13m>;
> >               };
> >
> >               iommu: iommu@10205000 {
>
>

Re: [PATCH 1/4] arm64: dts: mediatek: mt8183: Fix systimer 13 MHz clock description

[Chen-Yu Tsai]

On Thu, Dec 1, 2022 at 5:31 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 01/12/22 09:42, Chen-Yu Tsai ha scritto:
> > The systimer block derives its 13 MHz clock by dividing the main 26 MHz
> > oscillator clock by 2 internally, not through the TOPCKGEN clock
> > controller.
> >
> > On the MT8183 this divider is set either by power-on-reset or by the
> > bootloader. The bootloader may then make the divider unconfigurable to,
> > but can be read out by, the operating system.
> >
> > Making the systimer block take the 26 MHz clock directly requires
> > changing the implementations. As an ABI compatible fix, change the
> > input clock of the systimer block a fixed factor divide-by-2 clock
> > that takes the 26 MHz oscillator as its input.
> >
> > Fixes: 5bc8e2875ffb ("arm64: dts: mt8183: add systimer0 device node")
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
>
> I generally not just like - but *love* - this change, I had that in my mind
> for a couple of months now and forgot about it because reasons.
>
> There's just one thing that, since we're doing this now, we can clarify (and
> that's important to avoid questions like "why isn't this board-specific"):
> the 26MHz clock "clk26m" oscillator that we're using for the system timers
> is a SoC-provided clock, and its name is "SYSCLK" as in "System bus clock".

Looking at the schematics it is terribly more complicated. :(

The crystal feeds the DCXO in the PMIC, which also acts as a buffer.
The PMIC then feeds the SoC and any other chips, such as a modem.

On other platforms we describe the oscillator at the dtsi level as well.
The reason why it isn't board-specific is that the requirements and
properties of the crystal are specified in the platform's datasheet,
i.e. it is a design requirement that every board use the same crystal.

I don't see the datasheet spelling out SYSCLK though. The TOPCKGEN part
mostly just refers to it as CLK26M, or some variant of it, which likely
denotes some fan-out branch. The system timer part also just says "26M"
or "26 MHz clock source".

Also, we can't change the clock name, as "clk26m" is hard-coded into
the clk drivers.

> I know that your target is to describe how we get from 26M to 13M, but at
> this point it may be worth it to use the right names to help preventing
> confusion about that clock not being an external crystal on the board but
> something internal to the SoC.

But as I described above, it is an actual crystal on the board. We are
simply omitting parts of the signal path. Notably the PMIC needs to be
excluded due to circular dependency reasons. And also we most definitely
don't want the system to be touching it.

ChenYu

> So, I propose:
> 1. Change `clk26m: oscillator` to `clk26m: sysclk` or `clk26m: sysclk-26m`;
> 2. Add the divider as `clk13m: sysclk-div2`.
>
> What do you think?
>
> Cheers,
> Angelo
>
> > ---
> >   arch/arm64/boot/dts/mediatek/mt8183.dtsi | 12 ++++++++++--
> >   1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > index 19ff1babc359..0cbbaebe1213 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > @@ -585,6 +585,15 @@ psci {
> >               method = "smc";
> >       };
> >
> > +     clk13m: fixed-factor-clock-13m {
> > +             compatible = "fixed-factor-clock";
> > +             #clock-cells = <0>;
> > +             clocks = <&clk26m>;
> > +             clock-div = <2>;
> > +             clock-mult = <1>;
> > +             clock-output-names = "clk13m";
> > +     };
> > +
> >       clk26m: oscillator {
> >               compatible = "fixed-clock";
> >               #clock-cells = <0>;
> > @@ -968,8 +977,7 @@ systimer: timer@10017000 {
> >                                    "mediatek,mt6765-timer";
> >                       reg = <0 0x10017000 0 0x1000>;
> >                       interrupts = <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>;
> > -                     clocks = <&topckgen CLK_TOP_CLK13M>;
> > -                     clock-names = "clk13m";
> > +                     clocks = <&clk13m>;
> >               };
> >
> >               iommu: iommu@10205000 {
>
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] arm64: dts: mediatek: mt8183: Fix systimer 13 MHz clock description

[AngeloGioacchino Del Regno]

Il 01/12/22 11:33, Chen-Yu Tsai ha scritto:
> On Thu, Dec 1, 2022 at 5:31 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Il 01/12/22 09:42, Chen-Yu Tsai ha scritto:
>>> The systimer block derives its 13 MHz clock by dividing the main 26 MHz
>>> oscillator clock by 2 internally, not through the TOPCKGEN clock
>>> controller.
>>>
>>> On the MT8183 this divider is set either by power-on-reset or by the
>>> bootloader. The bootloader may then make the divider unconfigurable to,
>>> but can be read out by, the operating system.
>>>
>>> Making the systimer block take the 26 MHz clock directly requires
>>> changing the implementations. As an ABI compatible fix, change the
>>> input clock of the systimer block a fixed factor divide-by-2 clock
>>> that takes the 26 MHz oscillator as its input.
>>>
>>> Fixes: 5bc8e2875ffb ("arm64: dts: mt8183: add systimer0 device node")
>>> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
>>
>> I generally not just like - but *love* - this change, I had that in my mind
>> for a couple of months now and forgot about it because reasons.
>>
>> There's just one thing that, since we're doing this now, we can clarify (and
>> that's important to avoid questions like "why isn't this board-specific"):
>> the 26MHz clock "clk26m" oscillator that we're using for the system timers
>> is a SoC-provided clock, and its name is "SYSCLK" as in "System bus clock".
> 
> Looking at the schematics it is terribly more complicated. :(
> 
> The crystal feeds the DCXO in the PMIC, which also acts as a buffer.
> The PMIC then feeds the SoC and any other chips, such as a modem.
> 
> On other platforms we describe the oscillator at the dtsi level as well.
> The reason why it isn't board-specific is that the requirements and
> properties of the crystal are specified in the platform's datasheet,
> i.e. it is a design requirement that every board use the same crystal.
> 
> I don't see the datasheet spelling out SYSCLK though. The TOPCKGEN part

I think I got confused with the MCU Debug System (DEM) part. :\

> mostly just refers to it as CLK26M, or some variant of it, which likely
> denotes some fan-out branch. The system timer part also just says "26M"
> or "26 MHz clock source".
> 
> Also, we can't change the clock name, as "clk26m" is hard-coded into
> the clk drivers.
> 

I wasn't proposing to change the clock-output-names, but nevermind anyway,
on an afterthought, it's probably a good idea to just go with clk26m,
regardless of whether I'm wrong or right about sysclk, as the first one is
the name that we can also find in many (all?) downstream code for other
SoC models.

>> I know that your target is to describe how we get from 26M to 13M, but at
>> this point it may be worth it to use the right names to help preventing
>> confusion about that clock not being an external crystal on the board but
>> something internal to the SoC.
> 
> But as I described above, it is an actual crystal on the board. We are
> simply omitting parts of the signal path. Notably the PMIC needs to be
> excluded due to circular dependency reasons. And also we most definitely
> don't want the system to be touching it.
> 

Let's go with this one.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH 1/4] arm64: dts: mediatek: mt8183: Fix systimer 13 MHz clock description

[AngeloGioacchino Del Regno]

Il 01/12/22 11:33, Chen-Yu Tsai ha scritto:
> On Thu, Dec 1, 2022 at 5:31 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Il 01/12/22 09:42, Chen-Yu Tsai ha scritto:
>>> The systimer block derives its 13 MHz clock by dividing the main 26 MHz
>>> oscillator clock by 2 internally, not through the TOPCKGEN clock
>>> controller.
>>>
>>> On the MT8183 this divider is set either by power-on-reset or by the
>>> bootloader. The bootloader may then make the divider unconfigurable to,
>>> but can be read out by, the operating system.
>>>
>>> Making the systimer block take the 26 MHz clock directly requires
>>> changing the implementations. As an ABI compatible fix, change the
>>> input clock of the systimer block a fixed factor divide-by-2 clock
>>> that takes the 26 MHz oscillator as its input.
>>>
>>> Fixes: 5bc8e2875ffb ("arm64: dts: mt8183: add systimer0 device node")
>>> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
>>
>> I generally not just like - but *love* - this change, I had that in my mind
>> for a couple of months now and forgot about it because reasons.
>>
>> There's just one thing that, since we're doing this now, we can clarify (and
>> that's important to avoid questions like "why isn't this board-specific"):
>> the 26MHz clock "clk26m" oscillator that we're using for the system timers
>> is a SoC-provided clock, and its name is "SYSCLK" as in "System bus clock".
> 
> Looking at the schematics it is terribly more complicated. :(
> 
> The crystal feeds the DCXO in the PMIC, which also acts as a buffer.
> The PMIC then feeds the SoC and any other chips, such as a modem.
> 
> On other platforms we describe the oscillator at the dtsi level as well.
> The reason why it isn't board-specific is that the requirements and
> properties of the crystal are specified in the platform's datasheet,
> i.e. it is a design requirement that every board use the same crystal.
> 
> I don't see the datasheet spelling out SYSCLK though. The TOPCKGEN part

I think I got confused with the MCU Debug System (DEM) part. :\

> mostly just refers to it as CLK26M, or some variant of it, which likely
> denotes some fan-out branch. The system timer part also just says "26M"
> or "26 MHz clock source".
> 
> Also, we can't change the clock name, as "clk26m" is hard-coded into
> the clk drivers.
> 

I wasn't proposing to change the clock-output-names, but nevermind anyway,
on an afterthought, it's probably a good idea to just go with clk26m,
regardless of whether I'm wrong or right about sysclk, as the first one is
the name that we can also find in many (all?) downstream code for other
SoC models.

>> I know that your target is to describe how we get from 26M to 13M, but at
>> this point it may be worth it to use the right names to help preventing
>> confusion about that clock not being an external crystal on the board but
>> something internal to the SoC.
> 
> But as I described above, it is an actual crystal on the board. We are
> simply omitting parts of the signal path. Notably the PMIC needs to be
> excluded due to circular dependency reasons. And also we most definitely
> don't want the system to be touching it.
> 

Let's go with this one.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/4] arm64: dts: mediatek: mt8192: Fix systimer 13 MHz clock description

[Chen-Yu Tsai]

The systimer block derives its 13 MHz clock by dividing the main 26 MHz
oscillator clock by 2 internally, not through the TOPCKGEN clock
controller.

On the MT8192 this divider is fixed to /2 and is not configurable.

Making the systimer block take the 26 MHz clock directly requires
changing the implementations. As an ABI compatible fix, change the
input clock of the systimer block a fixed factor divide-by-2 clock
that takes the 26 MHz oscillator as its input.

Fixes: 48489980e27e ("arm64: dts: Add Mediatek SoC MT8192 and evaluation board dts and Makefile")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 arch/arm64/boot/dts/mediatek/mt8192.dtsi | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
index fc39ccc0d4bf..ab4d4f605493 100644
--- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
@@ -29,6 +29,15 @@ aliases {
 		rdma4 = &rdma4;
 	};
 
+	clk13m: fixed-factor-clock-13m {
+		compatible = "fixed-factor-clock";
+		#clock-cells = <0>;
+		clocks = <&clk26m>;
+		clock-div = <2>;
+		clock-mult = <1>;
+		clock-output-names = "clk13m";
+	};
+
 	clk26m: oscillator0 {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
@@ -534,8 +543,7 @@ systimer: timer@10017000 {
 				     "mediatek,mt6765-timer";
 			reg = <0 0x10017000 0 0x1000>;
 			interrupts = <GIC_SPI 233 IRQ_TYPE_LEVEL_HIGH 0>;
-			clocks = <&topckgen CLK_TOP_CSW_F26M_D2>;
-			clock-names = "clk13m";
+			clocks = <&clk13m>;
 		};
 
 		pwrap: pwrap@10026000 {
-- 
2.38.1.584.g0f3c55d4c2-goog

[PATCH 2/4] arm64: dts: mediatek: mt8192: Fix systimer 13 MHz clock description

[Chen-Yu Tsai]

The systimer block derives its 13 MHz clock by dividing the main 26 MHz
oscillator clock by 2 internally, not through the TOPCKGEN clock
controller.

On the MT8192 this divider is fixed to /2 and is not configurable.

Making the systimer block take the 26 MHz clock directly requires
changing the implementations. As an ABI compatible fix, change the
input clock of the systimer block a fixed factor divide-by-2 clock
that takes the 26 MHz oscillator as its input.

Fixes: 48489980e27e ("arm64: dts: Add Mediatek SoC MT8192 and evaluation board dts and Makefile")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 arch/arm64/boot/dts/mediatek/mt8192.dtsi | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
index fc39ccc0d4bf..ab4d4f605493 100644
--- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
@@ -29,6 +29,15 @@ aliases {
 		rdma4 = &rdma4;
 	};
 
+	clk13m: fixed-factor-clock-13m {
+		compatible = "fixed-factor-clock";
+		#clock-cells = <0>;
+		clocks = <&clk26m>;
+		clock-div = <2>;
+		clock-mult = <1>;
+		clock-output-names = "clk13m";
+	};
+
 	clk26m: oscillator0 {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
@@ -534,8 +543,7 @@ systimer: timer@10017000 {
 				     "mediatek,mt6765-timer";
 			reg = <0 0x10017000 0 0x1000>;
 			interrupts = <GIC_SPI 233 IRQ_TYPE_LEVEL_HIGH 0>;
-			clocks = <&topckgen CLK_TOP_CSW_F26M_D2>;
-			clock-names = "clk13m";
+			clocks = <&clk13m>;
 		};
 
 		pwrap: pwrap@10026000 {
-- 
2.38.1.584.g0f3c55d4c2-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/4] arm64: dts: mediatek: mt8192: Fix systimer 13 MHz clock description

[AngeloGioacchino Del Regno]

Il 01/12/22 09:42, Chen-Yu Tsai ha scritto:
> The systimer block derives its 13 MHz clock by dividing the main 26 MHz
> oscillator clock by 2 internally, not through the TOPCKGEN clock
> controller.
> 
> On the MT8192 this divider is fixed to /2 and is not configurable.
> 
> Making the systimer block take the 26 MHz clock directly requires
> changing the implementations. As an ABI compatible fix, change the
> input clock of the systimer block a fixed factor divide-by-2 clock
> that takes the 26 MHz oscillator as its input.
> 
> Fixes: 48489980e27e ("arm64: dts: Add Mediatek SoC MT8192 and evaluation board dts and Makefile")
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH 2/4] arm64: dts: mediatek: mt8192: Fix systimer 13 MHz clock description

[AngeloGioacchino Del Regno]

Il 01/12/22 09:42, Chen-Yu Tsai ha scritto:
> The systimer block derives its 13 MHz clock by dividing the main 26 MHz
> oscillator clock by 2 internally, not through the TOPCKGEN clock
> controller.
> 
> On the MT8192 this divider is fixed to /2 and is not configurable.
> 
> Making the systimer block take the 26 MHz clock directly requires
> changing the implementations. As an ABI compatible fix, change the
> input clock of the systimer block a fixed factor divide-by-2 clock
> that takes the 26 MHz oscillator as its input.
> 
> Fixes: 48489980e27e ("arm64: dts: Add Mediatek SoC MT8192 and evaluation board dts and Makefile")
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/4] arm64: dts: mediatek: mt8195: Fix systimer 13 MHz clock description

[Chen-Yu Tsai]

The systimer block derives its 13 MHz clock by dividing the main 26 MHz
oscillator clock by 2 internally, not through the TOPCKGEN clock
controller.

On the MT8195 this divider is set either by power-on-reset or by the
bootloader. The bootloader may then make the divider unconfigurable to,
but can be read out by, the operating system.

Making the systimer block take the 26 MHz clock directly requires
changing the implementations. As an ABI compatible fix, change the
input clock of the systimer block a fixed factor divide-by-2 clock
that takes the 26 MHz oscillator as its input.

Fixes: 37f2582883be ("arm64: dts: Add mediatek SoC mt8195 and evaluation board")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 arch/arm64/boot/dts/mediatek/mt8195.dtsi | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 5d31536f4c48..60e15833956e 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -248,6 +248,15 @@ sound: mt8195-sound {
 		status = "disabled";
 	};
 
+	clk13m: fixed-factor-clock-13m {
+		compatible = "fixed-factor-clock";
+		#clock-cells = <0>;
+		clocks = <&clk26m>;
+		clock-div = <2>;
+		clock-mult = <1>;
+		clock-output-names = "clk13m";
+	};
+
 	clk26m: oscillator-26m {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
@@ -705,7 +714,7 @@ systimer: timer@10017000 {
 				     "mediatek,mt6765-timer";
 			reg = <0 0x10017000 0 0x1000>;
 			interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH 0>;
-			clocks = <&topckgen CLK_TOP_CLK26M_D2>;
+			clocks = <&clk13m>;
 		};
 
 		pwrap: pwrap@10024000 {
-- 
2.38.1.584.g0f3c55d4c2-goog

[PATCH 3/4] arm64: dts: mediatek: mt8195: Fix systimer 13 MHz clock description

[Chen-Yu Tsai]

The systimer block derives its 13 MHz clock by dividing the main 26 MHz
oscillator clock by 2 internally, not through the TOPCKGEN clock
controller.

On the MT8195 this divider is set either by power-on-reset or by the
bootloader. The bootloader may then make the divider unconfigurable to,
but can be read out by, the operating system.

Making the systimer block take the 26 MHz clock directly requires
changing the implementations. As an ABI compatible fix, change the
input clock of the systimer block a fixed factor divide-by-2 clock
that takes the 26 MHz oscillator as its input.

Fixes: 37f2582883be ("arm64: dts: Add mediatek SoC mt8195 and evaluation board")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 arch/arm64/boot/dts/mediatek/mt8195.dtsi | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 5d31536f4c48..60e15833956e 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -248,6 +248,15 @@ sound: mt8195-sound {
 		status = "disabled";
 	};
 
+	clk13m: fixed-factor-clock-13m {
+		compatible = "fixed-factor-clock";
+		#clock-cells = <0>;
+		clocks = <&clk26m>;
+		clock-div = <2>;
+		clock-mult = <1>;
+		clock-output-names = "clk13m";
+	};
+
 	clk26m: oscillator-26m {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
@@ -705,7 +714,7 @@ systimer: timer@10017000 {
 				     "mediatek,mt6765-timer";
 			reg = <0 0x10017000 0 0x1000>;
 			interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH 0>;
-			clocks = <&topckgen CLK_TOP_CLK26M_D2>;
+			clocks = <&clk13m>;
 		};
 
 		pwrap: pwrap@10024000 {
-- 
2.38.1.584.g0f3c55d4c2-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/4] arm64: dts: mediatek: mt8195: Fix systimer 13 MHz clock description

[AngeloGioacchino Del Regno]

Il 01/12/22 09:42, Chen-Yu Tsai ha scritto:
> The systimer block derives its 13 MHz clock by dividing the main 26 MHz
> oscillator clock by 2 internally, not through the TOPCKGEN clock
> controller.
> 
> On the MT8195 this divider is set either by power-on-reset or by the
> bootloader. The bootloader may then make the divider unconfigurable to,
> but can be read out by, the operating system.
> 
> Making the systimer block take the 26 MHz clock directly requires
> changing the implementations. As an ABI compatible fix, change the
> input clock of the systimer block a fixed factor divide-by-2 clock
> that takes the 26 MHz oscillator as its input.
> 
> Fixes: 37f2582883be ("arm64: dts: Add mediatek SoC mt8195 and evaluation board")
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH 3/4] arm64: dts: mediatek: mt8195: Fix systimer 13 MHz clock description

[AngeloGioacchino Del Regno]

Il 01/12/22 09:42, Chen-Yu Tsai ha scritto:
> The systimer block derives its 13 MHz clock by dividing the main 26 MHz
> oscillator clock by 2 internally, not through the TOPCKGEN clock
> controller.
> 
> On the MT8195 this divider is set either by power-on-reset or by the
> bootloader. The bootloader may then make the divider unconfigurable to,
> but can be read out by, the operating system.
> 
> Making the systimer block take the 26 MHz clock directly requires
> changing the implementations. As an ABI compatible fix, change the
> input clock of the systimer block a fixed factor divide-by-2 clock
> that takes the 26 MHz oscillator as its input.
> 
> Fixes: 37f2582883be ("arm64: dts: Add mediatek SoC mt8195 and evaluation board")
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/4] arm64: dts: mediatek: mt8186: Fix systimer 13 MHz clock description

[Chen-Yu Tsai]

The systimer block derives its 13 MHz clock by dividing the main 26 MHz
oscillator clock by 2 internally. The 13 MHz clock is not a separate
oscillator.

Fix this by making the 13 MHz clock a divide-by-2 fixed factor clock,
taking its input from the main 26 MHz oscillator.

Fixes: 2e78620b1350 ("arm64: dts: Add MediaTek MT8186 dts and evaluation board and Makefile")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 arch/arm64/boot/dts/mediatek/mt8186.dtsi | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8186.dtsi b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
index 4a2f7ad3c6f0..209f26f12dbc 100644
--- a/arch/arm64/boot/dts/mediatek/mt8186.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
@@ -215,10 +215,12 @@ l3_0: l3-cache {
 		};
 	};
 
-	clk13m: oscillator-13m {
-		compatible = "fixed-clock";
+	clk13m: fixed-factor-clock-13m {
+		compatible = "fixed-factor-clock";
 		#clock-cells = <0>;
-		clock-frequency = <13000000>;
+		clocks = <&clk26m>;
+		clock-div = <2>;
+		clock-mult = <1>;
 		clock-output-names = "clk13m";
 	};
 
-- 
2.38.1.584.g0f3c55d4c2-goog

[PATCH 4/4] arm64: dts: mediatek: mt8186: Fix systimer 13 MHz clock description

[Chen-Yu Tsai]

The systimer block derives its 13 MHz clock by dividing the main 26 MHz
oscillator clock by 2 internally. The 13 MHz clock is not a separate
oscillator.

Fix this by making the 13 MHz clock a divide-by-2 fixed factor clock,
taking its input from the main 26 MHz oscillator.

Fixes: 2e78620b1350 ("arm64: dts: Add MediaTek MT8186 dts and evaluation board and Makefile")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 arch/arm64/boot/dts/mediatek/mt8186.dtsi | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8186.dtsi b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
index 4a2f7ad3c6f0..209f26f12dbc 100644
--- a/arch/arm64/boot/dts/mediatek/mt8186.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
@@ -215,10 +215,12 @@ l3_0: l3-cache {
 		};
 	};
 
-	clk13m: oscillator-13m {
-		compatible = "fixed-clock";
+	clk13m: fixed-factor-clock-13m {
+		compatible = "fixed-factor-clock";
 		#clock-cells = <0>;
-		clock-frequency = <13000000>;
+		clocks = <&clk26m>;
+		clock-div = <2>;
+		clock-mult = <1>;
 		clock-output-names = "clk13m";
 	};
 
-- 
2.38.1.584.g0f3c55d4c2-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] arm64: dts: mediatek: mt8186: Fix systimer 13 MHz clock description

[AngeloGioacchino Del Regno]

Il 01/12/22 09:42, Chen-Yu Tsai ha scritto:
> The systimer block derives its 13 MHz clock by dividing the main 26 MHz
> oscillator clock by 2 internally. The 13 MHz clock is not a separate
> oscillator.
> 
> Fix this by making the 13 MHz clock a divide-by-2 fixed factor clock,
> taking its input from the main 26 MHz oscillator.
> 
> Fixes: 2e78620b1350 ("arm64: dts: Add MediaTek MT8186 dts and evaluation board and Makefile")
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH 4/4] arm64: dts: mediatek: mt8186: Fix systimer 13 MHz clock description

[AngeloGioacchino Del Regno]

Il 01/12/22 09:42, Chen-Yu Tsai ha scritto:
> The systimer block derives its 13 MHz clock by dividing the main 26 MHz
> oscillator clock by 2 internally. The 13 MHz clock is not a separate
> oscillator.
> 
> Fix this by making the 13 MHz clock a divide-by-2 fixed factor clock,
> taking its input from the main 26 MHz oscillator.
> 
> Fixes: 2e78620b1350 ("arm64: dts: Add MediaTek MT8186 dts and evaluation board and Makefile")
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/4] arm64: dts: mediatek: Fix systimer clock description

[Matthias Brugger]

On 01/12/2022 09:42, Chen-Yu Tsai wrote:
> Hi,
> 
> This series fixes the clock description for the systimer block. The
> systimer is fed by the main 26 MHz oscillator, and internally divides
> the clock, normally by 2.
> 
> However this ended up being modeled in various incorrect ways, such as
> the clock divider being in the TOPCKGEN block, or as a standalone 13 MHz
> oscillator.
> 
> This series fixes the description of the systimer clock input in an ABI
> compatible way, i.e. the clock rate of the input clock remains the same
> at 13 MHz. The clock is now modeled as a divide-by-2 fixed factor clock
> being fed by the main oscillator.
> 
> An added benefit is that in Linux the systimer no longer requires the
> main SoC clk driver to do an early init dance.
> 
> Please have a look.
> 
> The next step would be to fix up the systimer driver in a backward
> compatible way and have it read the divider value from hardware.
> 

Whole series applied, thanks!

> 
> Regards
> ChenYu
> 
> Chen-Yu Tsai (4):
>    arm64: dts: mediatek: mt8183: Fix systimer 13 MHz clock description
>    arm64: dts: mediatek: mt8192: Fix systimer 13 MHz clock description
>    arm64: dts: mediatek: mt8195: Fix systimer 13 MHz clock description
>    arm64: dts: mediatek: mt8186: Fix systimer 13 MHz clock description
> 
>   arch/arm64/boot/dts/mediatek/mt8183.dtsi | 12 ++++++++++--
>   arch/arm64/boot/dts/mediatek/mt8186.dtsi |  8 +++++---
>   arch/arm64/boot/dts/mediatek/mt8192.dtsi | 12 ++++++++++--
>   arch/arm64/boot/dts/mediatek/mt8195.dtsi | 11 ++++++++++-
>   4 files changed, 35 insertions(+), 8 deletions(-)
> 

Re: [PATCH 0/4] arm64: dts: mediatek: Fix systimer clock description

[Matthias Brugger]

On 01/12/2022 09:42, Chen-Yu Tsai wrote:
> Hi,
> 
> This series fixes the clock description for the systimer block. The
> systimer is fed by the main 26 MHz oscillator, and internally divides
> the clock, normally by 2.
> 
> However this ended up being modeled in various incorrect ways, such as
> the clock divider being in the TOPCKGEN block, or as a standalone 13 MHz
> oscillator.
> 
> This series fixes the description of the systimer clock input in an ABI
> compatible way, i.e. the clock rate of the input clock remains the same
> at 13 MHz. The clock is now modeled as a divide-by-2 fixed factor clock
> being fed by the main oscillator.
> 
> An added benefit is that in Linux the systimer no longer requires the
> main SoC clk driver to do an early init dance.
> 
> Please have a look.
> 
> The next step would be to fix up the systimer driver in a backward
> compatible way and have it read the divider value from hardware.
> 

Whole series applied, thanks!

> 
> Regards
> ChenYu
> 
> Chen-Yu Tsai (4):
>    arm64: dts: mediatek: mt8183: Fix systimer 13 MHz clock description
>    arm64: dts: mediatek: mt8192: Fix systimer 13 MHz clock description
>    arm64: dts: mediatek: mt8195: Fix systimer 13 MHz clock description
>    arm64: dts: mediatek: mt8186: Fix systimer 13 MHz clock description
> 
>   arch/arm64/boot/dts/mediatek/mt8183.dtsi | 12 ++++++++++--
>   arch/arm64/boot/dts/mediatek/mt8186.dtsi |  8 +++++---
>   arch/arm64/boot/dts/mediatek/mt8192.dtsi | 12 ++++++++++--
>   arch/arm64/boot/dts/mediatek/mt8195.dtsi | 11 ++++++++++-
>   4 files changed, 35 insertions(+), 8 deletions(-)
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel