lkml.org 
[lkml]   [2020]   [Dec]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v5 23/24] arm64: dts: mediatek: Add mt8192 clock controllers
From
Date
Quoting Ikjoon Jang (2020-11-22 20:02:37)
> On Mon, Nov 09, 2020 at 10:03:48AM +0800, Weiyi Lu wrote:
> > Add clock controller nodes for SoC mt8192
> >
> > Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
> > ---
> > arch/arm64/boot/dts/mediatek/mt8192.dtsi | 163 +++++++++++++++++++++++++++++++
> > 1 file changed, 163 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> > index e12e024..92dcfbd 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> > @@ -5,6 +5,7 @@
> > */
> >
> > /dts-v1/;
> > +#include <dt-bindings/clock/mt8192-clk.h>
> > #include <dt-bindings/interrupt-controller/arm-gic.h>
> > #include <dt-bindings/interrupt-controller/irq.h>
> > #include <dt-bindings/pinctrl/mt8192-pinfunc.h>
> > @@ -213,6 +214,24 @@
> > };
> > };
> >
> > + topckgen: syscon@10000000 {
> > + compatible = "mediatek,mt8192-topckgen", "syscon";
> > + reg = <0 0x10000000 0 0x1000>;
> > + #clock-cells = <1>;
> > + };
> > +
> > + infracfg: syscon@10001000 {
> > + compatible = "mediatek,mt8192-infracfg", "syscon";
> > + reg = <0 0x10001000 0 0x1000>;
> > + #clock-cells = <1>;
> > + };
> > +
> > + pericfg: syscon@10003000 {
> > + compatible = "mediatek,mt8192-pericfg", "syscon";
> > + reg = <0 0x10003000 0 0x1000>;
> > + #clock-cells = <1>;
> > + };
> > +
>
> There are 26 new bindings for mt8192 clock providers, "mediatek,mt8192-*'.
> I guess the one reason of doing this is that those mmio blocks are
> just scattered all around over different memory regions.

Yeah it seems that Mediatek likes to scatter the clks into basically
every IP block. The alternate approach would be to create virtual
platform device children of those IP blocks to register the clks.

>
> I wonder if there could be a simpler way of merging them into one
> binding of "mediatek,mt8192-clocks" and converting all new bindings
> into generic syscon:
>
> mt8192-clocks: mt8192_clocks {
> compatible = "mediatek,mt8192-clocks";
> #clock-cells = <1>;
>
> infracfg: clk_infracfg {
> syscon = <&syscon_infracfg>;
> };
> pericfg: clk_pericfg {
> syscon = <&syscon_pericfg>:
> };
> };
>
> syscon_pericfg: syscon@10003000 {
> compatible = "syscon";
> reg = <0 0x10003000 0 0x1000>;
> };

Is the syscon used for anything besides a clk provider? Having a
mt8192_clocks node is wrong. The syscon is the clk provider and it
should have a #clock-cells property. Making up a node that doesn't have
a reg property is usually a sign that something is wrong.

\
 
 \ /
  Last update: 2020-12-17 09:57    [W:0.054 / U:1.252 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site