linux-clk.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: Mars Cheng <mars.cheng@mediatek.com>
Cc: Matthias Brugger <matthias.bgg@gmail.com>,
	Rob Herring <robh@kernel.org>, Stephen Boyd <sboyd@kernel.org>,
	Sean Wang <sean.wang@kernel.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	devicetree@vger.kernel.org, CC Hwang <cc.hwang@mediatek.com>,
	wsd_upstream@mediatek.com, Loda Chou <loda.chou@mediatek.com>,
	linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	mtk01761 <wendell.lin@mediatek.com>,
	linux-clk@vger.kernel.org
Subject: Re: [PATCH v2 11/11] arm64: dts: add dts nodes for MT6779
Date: Mon, 19 Aug 2019 13:07:00 +0100	[thread overview]
Message-ID: <c533371d-efcd-59dc-0172-3f5775221302@kernel.org> (raw)
In-Reply-To: <1566214950.17081.3.camel@mtkswgap22>

Hi Mars,

On 19/08/2019 12:42, Mars Cheng wrote:
> Hi Marc
> 
> On Mon, 2019-08-19 at 10:40 +0100, Marc Zyngier wrote:
>> On 19/08/2019 10:21, Mars Cheng wrote:
>>> this adds initial MT6779 dts settings fo board support,
>>> including cpu, gic, timer, ccf, pinctrl, uart...etc.
>>>
>>> Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
>>> ---
>>>  arch/arm64/boot/dts/mediatek/Makefile        |    1 +
>>>  arch/arm64/boot/dts/mediatek/mt6779-evb.dtsi |   31 ++++
>>>  arch/arm64/boot/dts/mediatek/mt6779.dts      |  229 ++++++++++++++++++++++++++
>>>  3 files changed, 261 insertions(+)
>>>  create mode 100644 arch/arm64/boot/dts/mediatek/mt6779-evb.dtsi
>>>  create mode 100644 arch/arm64/boot/dts/mediatek/mt6779.dts
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
>>> index 458bbc4..53f1c61 100644
>>> --- a/arch/arm64/boot/dts/mediatek/Makefile
>>> +++ b/arch/arm64/boot/dts/mediatek/Makefile
>>> @@ -1,6 +1,7 @@
>>>  # SPDX-License-Identifier: GPL-2.0
>>>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt2712-evb.dtb
>>>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt6755-evb.dtb
>>> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt6779-evb.dtb
>>>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt6795-evb.dtb
>>>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
>>>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-x20-dev.dtb
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt6779-evb.dtsi b/arch/arm64/boot/dts/mediatek/mt6779-evb.dtsi
>>> new file mode 100644
>>> index 0000000..164f5cb
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/mediatek/mt6779-evb.dtsi
>>> @@ -0,0 +1,31 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * Copyright (c) 2019 MediaTek Inc.
>>> + * Author: Mars.C <mars.cheng@mediatek.com>
>>> + *
>>> + */
>>> +
>>> +/dts-v1/;
>>> +#include "mt6779.dtsi"
>>> +
>>> +/ {
>>> +	model = "MediaTek MT6779 EVB";
>>> +	compatible = "mediatek,mt6779-evb", "mediatek,mt6779";
>>> +
>>> +	aliases {
>>> +		serial0 = &uart0;
>>> +	};
>>> +
>>> +	memory@40000000 {
>>> +		device_type = "memory";
>>> +		reg = <0 0x40000000 0 0x1e800000>;
>>> +	};
>>> +
>>> +	chosen {
>>> +		stdout-path = "serial0:921600n8";
>>> +	};
>>> +};
>>> +
>>> +&uart0 {
>>> +	status = "okay";
>>> +};
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt6779.dts b/arch/arm64/boot/dts/mediatek/mt6779.dts
>>> new file mode 100644
>>> index 0000000..daa25b7
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/mediatek/mt6779.dts
>>> @@ -0,0 +1,229 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * Copyright (c) 2019 MediaTek Inc.
>>> + * Author: Mars.C <mars.cheng@mediatek.com>
>>> + *
>>> + */
>>> +
>>> +#include <dt-bindings/clock/mt6779-clk.h>
>>> +#include <dt-bindings/interrupt-controller/irq.h>
>>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +
>>> +/ {
>>> +	compatible = "mediatek,mt6779";
>>> +	interrupt-parent = <&sysirq>;
>>> +	#address-cells = <2>;
>>> +	#size-cells = <2>;
>>> +
>>> +	psci {
>>> +		compatible = "arm,psci-0.2";
>>> +		method = "smc";
>>> +	};
>>> +
>>> +	cpus {
>>> +		#address-cells = <1>;
>>> +		#size-cells = <0>;
>>> +
>>> +		cpu0: cpu@0 {
>>> +			device_type = "cpu";
>>> +			compatible = "arm,cortex-a55";
>>> +			enable-method = "psci";
>>> +			reg = <0x000>;
>>> +		};
>>> +
>>> +		cpu1: cpu@1 {
>>> +			device_type = "cpu";
>>> +			compatible = "arm,cortex-a55";
>>> +			enable-method = "psci";
>>> +			reg = <0x100>;
>>> +		};
>>> +
>>> +		cpu2: cpu@2 {
>>> +			device_type = "cpu";
>>> +			compatible = "arm,cortex-a55";
>>> +			enable-method = "psci";
>>> +			reg = <0x200>;
>>> +		};
>>> +
>>> +		cpu3: cpu@3 {
>>> +			device_type = "cpu";
>>> +			compatible = "arm,cortex-a55";
>>> +			enable-method = "psci";
>>> +			reg = <0x300>;
>>> +		};
>>> +
>>> +		cpu4: cpu@4 {
>>> +			device_type = "cpu";
>>> +			compatible = "arm,cortex-a55";
>>> +			enable-method = "psci";
>>> +			reg = <0x400>;
>>> +		};
>>> +
>>> +		cpu5: cpu@5 {
>>> +			device_type = "cpu";
>>> +			compatible = "arm,cortex-a55";
>>> +			enable-method = "psci";
>>> +			reg = <0x500>;
>>> +		};
>>> +
>>> +		cpu6: cpu@6 {
>>> +			device_type = "cpu";
>>> +			compatible = "arm,cortex-a75";
>>> +			enable-method = "psci";
>>> +			reg = <0x600>;
>>> +		};
>>> +
>>> +		cpu7: cpu@7 {
>>> +			device_type = "cpu";
>>> +			compatible = "arm,cortex-a75";
>>> +			enable-method = "psci";
>>> +			reg = <0x700>;
>>> +		};
>>> +	};
>>> +
>>> +	clk26m: oscillator@0 {
>>> +		compatible = "fixed-clock";
>>> +		#clock-cells = <0>;
>>> +		clock-frequency = <26000000>;
>>> +		clock-output-names = "clk26m";
>>> +	};
>>> +
>>> +	clk32k: oscillator@1 {
>>> +		compatible = "fixed-clock";
>>> +		#clock-cells = <0>;
>>> +		clock-frequency = <32768>;
>>> +		clock-output-names = "clk32k";
>>> +	};
>>> +
>>> +	uart_clk: dummy26m {
>>> +		compatible = "fixed-clock";
>>> +		clock-frequency = <26000000>;
>>> +		#clock-cells = <0>;
>>> +	};
>>> +
>>> +	timer {
>>> +		compatible = "arm,armv8-timer";
>>> +		interrupt-parent = <&gic>;
>>> +		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
>>> +			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
>>> +			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
>>> +			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
>>> +	};
>>> +
>>> +	soc {
>>> +		#address-cells = <2>;
>>> +		#size-cells = <2>;
>>> +		compatible = "simple-bus";
>>> +		ranges;
>>> +
>>> +		gic: interrupt-controller@0c000000 {
>>> +			compatible = "arm,gic-v3";
>>> +			#interrupt-cells = <3>;
>>
>> You also haven't described the CPU PMUs. Depending on how they are wired
>> (SPIs or PPIs), you may have to change the interrupt-cells property to
>> include a cell for the PPI partitioning.
>>
> 
> pmu nodes would be:
> 
>         pmu {
>                 compatible = "arm,armv8-pmuv3";
>                 interrupt-parent = <&gic>;
>                 interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW>;
>         };
> 
>         dsu-pmu-0 {
>                 compatible = "arm,dsu-pmu";
>                 interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
>                 cpus = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>,
>                         <&cpu4>, <&cpu5>, <&cpu6>, <&cpu7>;
>         };
> 
> so I think interrupt-cells could be <3>, will add pmu nodes in v3.

No, that's wrong, at least for the CPU pmu node.

First, you need two of them (one for the A55s, one for the A75s).
Then you need to partition the corresponding PPI so that they can be
described as separate affinity sets.
Finally, this implies that #interrupt-cells goes up to 4, and all the
interrupts directly routed to the GIC must be updated.

You should have something like this:

	&gic {
		ppi-partitions {
			cluster0: interrupt-partition-0 {
				affinity = <&cpu0 &cpu1 &cpu2
                                            &cpu3 &cpu4 &cpu5>;
			};

			cluster1: interrupt-partition-1 {
				affinity = <&cpu6 &cpu7>;
			};
	};

	pmu_a55 {
		compatible = "arm,cortex-a55-pmu", "arm,armv8-pmuv3";
		interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW &cluster0>;
	};

	pmu_a75 {
		compatible = "arm,cortex-a75-pmu", "arm,armv8-pmuv3";
		interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW &cluster1>;
	};

Please see the rk3399 usage of the binding, as it is the canonical example.

> 
>>
>>> +			#address-cells = <2>;
>>> +			#size-cells = <2>;
>>> +			#redistributor-regions = <1>;
>>
>> This is the default, so this can be omitted.
>>
> 
> Got it, will remove it in v3.
> 
>>> +			interrupt-parent = <&gic>;
>>> +			interrupt-controller;
>>> +			reg = <0 0x0c000000 0 0x40000>,  /* GICD */
>>> +			      <0 0x0c040000 0 0x200000>, /* GICR */
>>> +			      <0 0x0c400000 0 0x2000>,   /* GICC */
>>> +			      <0 0x0c410000 0 0x1000>,   /* GICH */
>>> +			      <0 0x0c420000 0 0x2000>;   /* GICV */
>>
>> Where do the last 3 ranges come from? Neither Cortex-A55 nor A75 have
>> the memory-mapped CPU interface. It looks like a copy/paste from another
>> SoC...
>>
> 
> My bad, should remove the last 3 interfaces. will do in v3.
> 
>>> +			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
>>> +		};
>>
>> And no ITS?
>>
> 
> No, we don't have ITS support in HW.

That's pretty sad.

	M.
-- 
Jazz is not dead, it just smells funny...

  reply	other threads:[~2019-08-19 12:07 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-19  9:21 [PATCHv2 00/11] Add basic SoC Support for Mediatek MT6779 SoC Mars Cheng
2019-08-19  9:21 ` [PATCH v2 01/11] dt-bindings: mediatek: add support for mt6779 reference board Mars Cheng
2019-08-23 15:50   ` Matthias Brugger
2019-08-19  9:21 ` [PATCH v2 02/11] dt-bindings: mtk-uart: add mt6779 uart bindings Mars Cheng
2019-08-23 15:47   ` Matthias Brugger
2019-08-19  9:21 ` [PATCH v2 03/11] dt-bindings: irq: mtk,sysirq: add support for mt6779 Mars Cheng
2019-08-23  8:51   ` Linus Walleij
2019-08-23 15:51     ` Matthias Brugger
2019-08-23 15:44   ` Matthias Brugger
2019-08-27 16:50   ` Rob Herring
2019-08-19  9:21 ` [PATCH v2 04/11] pinctrl: mediatek: update pinmux definitions " Mars Cheng
2019-08-23 15:53   ` Matthias Brugger
2020-01-02  4:04     ` Hanks Chen
2019-08-19  9:21 ` [PATCH v2 05/11] pinctrl: mediatek: avoid virtual gpio trying to set reg Mars Cheng
2019-08-23  8:57   ` Linus Walleij
2019-12-22 13:52     ` Hanks Chen
2020-01-07 10:20       ` Linus Walleij
2020-01-08 11:27         ` Hanks Chen
2019-08-19  9:21 ` [PATCH v2 06/11] pinctrl: mediatek: add pinctrl support for MT6779 SoC Mars Cheng
     [not found]   ` <CAGp9LzoVwNxY8Q3G4hxpa7=orsEox+J0mNamag70wyjrGvDiZw@mail.gmail.com>
2019-08-23  8:59     ` Linus Walleij
2019-08-19  9:21 ` [PATCH v2 07/11] pinctrl: mediatek: add mt6779 eint support Mars Cheng
2019-08-22 18:13   ` Sean Wang
2019-08-19  9:21 ` [PATCH v2 08/11] dt-bindings: mediatek: bindings for MT6779 clk Mars Cheng
2019-08-27 16:52   ` Rob Herring
2019-09-10 14:53   ` Stephen Boyd
2019-08-19  9:21 ` [PATCH v2 09/11] clk: mediatek: Add dt-bindings for MT6779 clocks Mars Cheng
2019-08-27 16:53   ` Rob Herring
2019-09-10 14:53   ` Stephen Boyd
2019-08-19  9:21 ` [PATCH v2 10/11] clk: mediatek: Add MT6779 clock support Mars Cheng
2019-09-10 14:53   ` Stephen Boyd
2019-08-19  9:21 ` [PATCH v2 11/11] arm64: dts: add dts nodes for MT6779 Mars Cheng
2019-08-19  9:40   ` Marc Zyngier
2019-08-19 11:42     ` Mars Cheng
2019-08-19 12:07       ` Marc Zyngier [this message]
2019-08-22  0:46         ` Mars Cheng
2019-08-23 16:13   ` Matthias Brugger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c533371d-efcd-59dc-0172-3f5775221302@kernel.org \
    --to=maz@kernel.org \
    --cc=cc.hwang@mediatek.com \
    --cc=devicetree@vger.kernel.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=loda.chou@mediatek.com \
    --cc=mars.cheng@mediatek.com \
    --cc=matthias.bgg@gmail.com \
    --cc=robh@kernel.org \
    --cc=sboyd@kernel.org \
    --cc=sean.wang@kernel.org \
    --cc=wendell.lin@mediatek.com \
    --cc=wsd_upstream@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).