| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c5c0849d-a95a-25a0-11f8-9156770afc10@gmail.com>
Date: Tue, 16 Nov 2021 12:18:13 +0100
From: Matthias Brugger <matthias.bgg@...il.com>
To: Sam Shih <sam.shih@...iatek.com>, Rob Herring <robh+dt@...nel.org>,
Hsin-Yi Wang <hsinyi@...omium.org>,
Enric Balletbo i Serra <enric.balletbo@...labora.com>,
Fabien Parent <fparent@...libre.com>,
Seiya Wang <seiya.wang@...iatek.com>,
Sean Wang <sean.wang@...iatek.com>, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org
Cc: John Crispin <john@...ozen.org>, Ryder Lee <Ryder.Lee@...iatek.com>
Subject: Re: [PATCH v7 2/3] arm64: dts: mediatek: add basic mt7986a support
On 16/11/2021 02:39, Sam Shih wrote:
> Hi,
>
> On Mon, 2021-11-15 at 17:27 +0100, Matthias Brugger wrote:
>> Hi,
>>
>> On 18/10/2021 13:40, Sam Shih wrote:
>>> Add basic chip support for Mediatek mt7986a, include
>>> basic uart nodes, rng node and watchdog node.
>>>
>>> Add cpu node, timer node, gic node, psci and reserved-memory node
>>> for ARM Trusted Firmware.
>>>
>>
>> What is the exact difference between mt7986a and mt7986b? Right now,
>> it's only
>> the compatible, for that it makes no sense to split them.
>>
>
> The difference between mt7986a and mt7986b is pinout which described
> in our pinctrl patch series
> https://lore.kernel.org/all/20211022124036.5291-3-sam.shih@mediatek.com/
>
> You are right, in this "basic SoC support" patch series, only show
> compatible differences
>
>> It would be good to see what the exact differences are, so that we
>> can see if it
>> makes sense to have one of the alternatives:
>> 1) use a common mt7986.dtsi which get included by mt7986[a,b].dtsi
>> 2) Use on mt7986.dtsi and only add one mt7986a.dtsi or mt7986b.dtsi
>> which has
>> add-ons.
>>
>
> In this case, can we use solution (1) to create a generic mt7986.dtsi
> in this patch series, and add mt7986[a,b].dtsi to the dts part of the
> pinctrl patch series to separate the difference nodes?
>
If the only difference is the GPIO controller then why not go with solution 2.
Create a mt7986.dtsi which holds e.g. the node for pincontroller mt7986a and
then create a mt7986b.dtsi that just changes compatible and gpio-ranges:
&pio {
compatible = "mediatek,mt7986b-pinctrl";
gpio-ranges = <&pio 0 0 41>, <&pio 66 66 35>;
}
What do you think?
Regards,
Matthias
>>> Signed-off-by: Sam Shih <sam.shih@...iatek.com>
>>>
>>> ---
>>> v7: added memory node back to dts
>>> v6: removed clock and pinctrl node, to separate basic part into a
>>> single
>>> patch series
>>>
>>> Original thread:
>>>
> https://urldefense.com/v3/__https://lore.kernel.org/all/20211004124155.1404-1-sam.shih@mediatek.com/__;!!CTRNKA9wMg0ARbw!xCEW0lwTKx4k272sWASoi90y_yRyQdAx0oNJC-jSAIWnIEkprJD-gAc1ugCvo0ex$
>>>
>>>
>>> v5: follow reviewr's comment: removed clock freqency node in timer
>>> due to
>>> we have set CNTFRQ_EL0 in ATF firmware, and also corrected
>>> GICD range
>>> v4: added missing gic register bases, and fixed range of GICR
>>> v3: used the stdout-path instead of console=ttyS0
>>> v2: modified clock and uart node due to clock driver updated
>>> ---
>>> arch/arm64/boot/dts/mediatek/Makefile | 1 +
>>> arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts | 38 +++++
>>> arch/arm64/boot/dts/mediatek/mt7986a.dtsi | 149
>>> +++++++++++++++++++
>>> 3 files changed, 188 insertions(+)
>>> create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
>>> create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a.dtsi
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/Makefile
>>> b/arch/arm64/boot/dts/mediatek/Makefile
>>> index 4f68ebed2e31..e6c3a73b9e4a 100644
>>> --- a/arch/arm64/boot/dts/mediatek/Makefile
>>> +++ b/arch/arm64/boot/dts/mediatek/Makefile
>>> @@ -7,6 +7,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
>>> dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-x20-dev.dtb
>>> dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
>>> dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
>>> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-rfb.dtb
>>> dtb-$(CONFIG_ARCH_MEDIATEK) += mt8167-pumpkin.dtb
>>> dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-elm.dtb
>>> dtb-$(CONFIG_ARCH_MEDIATEK) += mt8173-elm-hana.dtb
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
>>> b/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
>>> new file mode 100644
>>> index 000000000000..ca074cf8e578
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/mediatek/mt7986a-rfb.dts
>>> @@ -0,0 +1,38 @@
>>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>>> +/*
>>> + * Copyright (C) 2021 MediaTek Inc.
>>> + * Author: Sam.Shih <sam.shih@...iatek.com>
>>> + */
>>> +
>>> +/dts-v1/;
>>> +#include "mt7986a.dtsi"
>>> +
>>> +/ {
>>> + model = "MediaTek MT7986a RFB";
>>> + compatible = "mediatek,mt7986a-rfb";
>>> +
>>> + aliases {
>>> + serial0 = &uart0;
>>> + };
>>> +
>>> + chosen {
>>> + stdout-path = "serial0:115200n8";
>>> + bootargs = "earlycon=uart8250,mmio32,0x11002000
>>> swiotlb=512";
>>
>> We normally don't add earlycon parameter to the normal bootargs, as
>> it's only
>> for debugging. Also what do we need the swiotlb? Are there any
>> limitation in the
>> HW that makes us need it?
>>
>
> Thank you for your suggestion, as far as I know, it should not have
> hardware limitations. This bootargs is just inherent from mt7622-
> rfb.dts, I will delete it and test again on our development board.
>
>> Regards,
>> Matthias
>>
>>> + };
>>> +
>>> + memory {
>>> + reg = <0 0x40000000 0 0x40000000>;
>>> + };
>>> +};
>>> +
>>> +&uart0 {
>>> + status = "okay";
>>> +};
>>> +
>>> +&uart1 {
>>> + status = "okay";
>>> +};
>>> +
>>> +&uart2 {
>>> + status = "okay";
>>> +};
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt7986a.dtsi
>>> b/arch/arm64/boot/dts/mediatek/mt7986a.dtsi
>>> new file mode 100644
>>> index 000000000000..75912bcf6c9c
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/mediatek/mt7986a.dtsi
>>> @@ -0,0 +1,149 @@
>>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>>> +/*
>>> + * Copyright (C) 2021 MediaTek Inc.
>>> + * Author: Sam.Shih <sam.shih@...iatek.com>
>>> + */
>>> +
>>> +#include <dt-bindings/interrupt-controller/irq.h>
>>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +
>>> +/ {
>>> + compatible = "mediatek,mt7986a";
>>> + interrupt-parent = <&gic>;
>>> + #address-cells = <2>;
>>> + #size-cells = <2>;
>>> +
>>> + system_clk: dummy40m {
>>> + compatible = "fixed-clock";
>>> + clock-frequency = <40000000>;
>>> + #clock-cells = <0>;
>>> + };
>>> +
>>> + cpus {
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> + cpu0: cpu@0 {
>>> + device_type = "cpu";
>>> + compatible = "arm,cortex-a53";
>>> + enable-method = "psci";
>>> + reg = <0x0>;
>>> + #cooling-cells = <2>;
>>> + };
>>> +
>>> + cpu1: cpu@1 {
>>> + device_type = "cpu";
>>> + compatible = "arm,cortex-a53";
>>> + enable-method = "psci";
>>> + reg = <0x1>;
>>> + #cooling-cells = <2>;
>>> + };
>>> +
>>> + cpu2: cpu@2 {
>>> + device_type = "cpu";
>>> + compatible = "arm,cortex-a53";
>>> + enable-method = "psci";
>>> + reg = <0x2>;
>>> + #cooling-cells = <2>;
>>> + };
>>> +
>>> + cpu3: cpu@3 {
>>> + device_type = "cpu";
>>> + enable-method = "psci";
>>> + compatible = "arm,cortex-a53";
>>> + reg = <0x3>;
>>> + #cooling-cells = <2>;
>>> + };
>>> + };
>>> +
>>> + psci {
>>> + compatible = "arm,psci-0.2";
>>> + method = "smc";
>>> + };
>>> +
>>> + reserved-memory {
>>> + #address-cells = <2>;
>>> + #size-cells = <2>;
>>> + ranges;
>>> + /* 192 KiB reserved for ARM Trusted Firmware (BL31) */
>>> + secmon_reserved: secmon@...00000 {
>>> + reg = <0 0x43000000 0 0x30000>;
>>> + no-map;
>>> + };
>>> + };
>>> +
>>> + 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@...0000 {
>>> + compatible = "arm,gic-v3";
>>> + #interrupt-cells = <3>;
>>> + interrupt-parent = <&gic>;
>>> + interrupt-controller;
>>> + reg = <0 0x0c000000 0 0x10000>, /* GICD */
>>> + <0 0x0c080000 0 0x80000>, /* GICR */
>>> + <0 0x0c400000 0 0x2000>, /* GICC */
>>> + <0 0x0c410000 0 0x1000>, /* GICH */
>>> + <0 0x0c420000 0 0x2000>; /* GICV */
>>> + interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
>>> + };
>>> +
>>> + watchdog: watchdog@...1c000 {
>>> + compatible = "mediatek,mt7986-wdt",
>>> + "mediatek,mt6589-wdt";
>>> + reg = <0 0x1001c000 0 0x1000>;
>>> + interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
>>> + #reset-cells = <1>;
>>> + status = "disabled";
>>> + };
>>> +
>>> + trng: trng@...0f000 {
>>> + compatible = "mediatek,mt7986-rng",
>>> + "mediatek,mt7623-rng";
>>> + reg = <0 0x1020f000 0 0x100>;
>>> + clocks = <&system_clk>;
>>> + clock-names = "rng";
>>> + status = "disabled";
>>> + };
>>> +
>>> + uart0: serial@...02000 {
>>> + compatible = "mediatek,mt7986-uart",
>>> + "mediatek,mt6577-uart";
>>> + reg = <0 0x11002000 0 0x400>;
>>> + interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>;
>>> + clocks = <&system_clk>;
>>> + status = "disabled";
>>> + };
>>> +
>>> + uart1: serial@...03000 {
>>> + compatible = "mediatek,mt7986-uart",
>>> + "mediatek,mt6577-uart";
>>> + reg = <0 0x11003000 0 0x400>;
>>> + interrupts = <GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>;
>>> + clocks = <&system_clk>;
>>> + status = "disabled";
>>> + };
>>> +
>>> + uart2: serial@...04000 {
>>> + compatible = "mediatek,mt7986-uart",
>>> + "mediatek,mt6577-uart";
>>> + reg = <0 0x11004000 0 0x400>;
>>> + interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
>>> + clocks = <&system_clk>;
>>> + status = "disabled";
>>> + };
>>> +
>>> + };
>>> +
>>> +};
>>>
>
> Regards,
> Sam
>
Powered by blists - more mailing lists