[PATCH v6 4/4] arm64: dts: Add MediaTek MT8188 dts and evaluation board and Makefile
Chen-Yu Tsai
wenst at chromium.org
Tue Oct 24 02:36:53 PDT 2023
Hi,
On Mon, Oct 23, 2023 at 6:55 PM Alexandre Mergnat <amergnat at baylibre.com> wrote:
>
>
>
> On 23/10/2023 10:38, Jason-ch Chen wrote:
> > From: jason-ch chen <Jason-ch.Chen at mediatek.com>
> >
> > MT8188 is a SoC based on 64bit ARMv8 architecture. It contains 6 CA55
> > and 2 CA78 cores. MT8188 share many HW IP with MT65xx series.
> >
> > We add basic chip support for MediaTek MT8188 on evaluation board.
> >
> > Signed-off-by: jason-ch chen <Jason-ch.Chen at mediatek.com>
> > ---
> > arch/arm64/boot/dts/mediatek/Makefile | 1 +
> > arch/arm64/boot/dts/mediatek/mt8188-evb.dts | 387 ++++++++
> > arch/arm64/boot/dts/mediatek/mt8188.dtsi | 956 ++++++++++++++++++++
> > 3 files changed, 1344 insertions(+)
> > create mode 100644 arch/arm64/boot/dts/mediatek/mt8188-evb.dts
> > create mode 100644 arch/arm64/boot/dts/mediatek/mt8188.dtsi
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
> > index e6e7592a3645..8900b939ed52 100644
> > --- a/arch/arm64/boot/dts/mediatek/Makefile
> > +++ b/arch/arm64/boot/dts/mediatek/Makefile
> > @@ -44,6 +44,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt8183-kukui-krane-sku0.dtb
> > dtb-$(CONFIG_ARCH_MEDIATEK) += mt8183-kukui-krane-sku176.dtb
> > dtb-$(CONFIG_ARCH_MEDIATEK) += mt8183-pumpkin.dtb
> > dtb-$(CONFIG_ARCH_MEDIATEK) += mt8186-evb.dtb
> > +dtb-$(CONFIG_ARCH_MEDIATEK) += mt8188-evb.dtb
> > dtb-$(CONFIG_ARCH_MEDIATEK) += mt8192-asurada-hayato-r1.dtb
> > dtb-$(CONFIG_ARCH_MEDIATEK) += mt8192-asurada-hayato-r5-sku2.dtb
> > dtb-$(CONFIG_ARCH_MEDIATEK) += mt8192-asurada-spherion-r0.dtb
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8188-evb.dts b/arch/arm64/boot/dts/mediatek/mt8188-evb.dts
> > new file mode 100644
> > index 000000000000..68a82b49f7a3
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/mediatek/mt8188-evb.dts
> > @@ -0,0 +1,387 @@
> > +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> > +/*
> > + * Copyright (C) 2023 MediaTek Inc.
> > + */
> > +/dts-v1/;
> > +#include "mt8188.dtsi"
> > +#include "mt6359.dtsi"
> > +
> > +/ {
> > + model = "MediaTek MT8188 evaluation board";
> > + compatible = "mediatek,mt8188-evb", "mediatek,mt8188";
> > +
> > + aliases {
> > + serial0 = &uart0;
> > + i2c0 = &i2c0;
> > + i2c1 = &i2c1;
> > + i2c2 = &i2c2;
> > + i2c3 = &i2c3;
> > + i2c4 = &i2c4;
> > + i2c5 = &i2c5;
> > + i2c6 = &i2c6;
> > + mmc0 = &mmc0;
> > + };
> > +
> > + chosen: chosen {
> > + stdout-path = "serial0:115200n8";
> > + };
> > +
> > + memory at 40000000 {
> > + device_type = "memory";
> > + reg = <0 0x40000000 0 0x80000000>;
> > + };
> > +
> > + reserved_memory: reserved-memory {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > + ranges;
> > +
> > + scp_mem_reserved: memory at 50000000 {
> > + compatible = "shared-dma-pool";
> > + reg = <0 0x50000000 0 0x2900000>;
> > + no-map;
> > + };
> > + };
> > +};
> > +
> > +&auxadc {
> > + status = "okay";
> > +};
> > +
> > +&i2c0 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&i2c0_pins>;
> > + clock-frequency = <400000>;
> > + status = "okay";
>
> IMO, the order should be
>
> clock-frequency = <400000>;
> pinctrl-0 = <&i2c0_pins>;
> pinctrl-names = "default";
> status = "okay";
>
> Please apply this to other nodes
Within the MediaTek platform device trees, this ordering seems to be all
over. The one here seems to be the ordering MediaTek likes. The device
trees upstreamed by Collabora folks have:
{
status = "okay";
clock-frequency = <400000>;
pinctrl-names = "default";
pinctrl-0 = <&i2c3_pins>;
}
As long as the ordering sort of makes sense (external bus properties come
after internal properties), I probably wouldn't argue either way.
> > +};
> > +
> > +&i2c1 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&i2c1_pins>;
> > + clock-frequency = <400000>;
> > + status = "okay";
> > +};
> > +
> > +&i2c2 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&i2c2_pins>;
> > + clock-frequency = <400000>;
> > + status = "okay";
> > +};
> > +
> > +&i2c3 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&i2c3_pins>;
> > + clock-frequency = <400000>;
> > + status = "okay";
> > +};
> > +
> > +&i2c4 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&i2c4_pins>;
> > + clock-frequency = <400000>;
> > + status = "okay";
> > +};
> > +
> > +&i2c5 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&i2c5_pins>;
> > + clock-frequency = <400000>;
> > + status = "okay";
> > +};
> > +
> > +&i2c6 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&i2c6_pins>;
> > + clock-frequency = <400000>;
> > + status = "okay";
> > +};
> > +
> > +&mmc0 {
> > + bus-width = <8>;
> > + hs400-ds-delay = <0x1481b>;
> > + max-frequency = <200000000>;
> > +
> > + cap-mmc-highspeed;
> > + mmc-hs200-1_8v;
> > + mmc-hs400-1_8v;
> > + supports-cqe;
> > + cap-mmc-hw-reset;
> > + no-sdio;
> > + no-sd;
> > + non-removable;
> > +
> > + vmmc-supply = <&mt6359_vemc_1_ldo_reg>;
> > + vqmmc-supply = <&mt6359_vufs_ldo_reg>;
> > +
> > + pinctrl-names = "default", "state_uhs";
> > + pinctrl-0 = <&mmc0_default_pins>;
> > + pinctrl-1 = <&mmc0_uhs_pins>;
> > +
> > + status = "okay";
> > +};
> > +
> > +&mt6359_vcore_buck_reg {
> > + regulator-always-on;
> > +};
> > +
> > +&mt6359_vgpu11_buck_reg {
> > + regulator-always-on;
> > +};
> > +
> > +&mt6359_vpu_buck_reg {
> > + regulator-always-on;
> > +};
> > +
> > +&mt6359_vrf12_ldo_reg {
> > + regulator-always-on;
> > +};
> > +
> > +&nor_flash {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&nor_pins_default>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + status = "okay";
>
> Order:
>
>
> #address-cells = <1>;
> #size-cells = <0>;
>
> pinctrl-0 = <&nor_pins_default>;
> pinctrl-names = "default";
I think pinctrl-names before pinctrl-* makes more sense. We declare the
names and by extension how many pinctrl-N entries are needed first. The
vast majority of the arm64 device tree files have pinctrl-names before
pinctrl-N. The only platform that exclusively has pinctrl-N before
pinctrl-names is amlogic.
If there's a preference for a particular order platform-wide or tree-wide
then it should probably be documented somewhere?
> status = "okay";
I think #address-cells and #size-cells belong at the end of the list,
even after "status", just before any child nodes. They describe
properties or requirements for the child nodes, not for the node they
sit in.
> > +
> > + flash at 0 {
> > + compatible = "jedec,spi-nor";
> > + reg = <0>;
> > + spi-max-frequency = <52000000>;
> > + };
> > +};
> > +
>
> ..snip..
>
> > +
> > +&pmic {
> > + interrupts-extended = <&pio 222 IRQ_TYPE_LEVEL_HIGH>;
> > +};
> > +
> > +&scp {
> > + memory-region = <&scp_mem_reserved>;
> > + status = "okay";
> > +};
> > +
> > +&spi0 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&spi0_pins>;
>
> Order:
>
> pinctrl-0 = <&spi0_pins>;
> pinctrl-names = "default";
>
> Please apply this to other nodes
See above.
ChenYu
> > + status = "okay";
> > +};
> > +
> > +&spi1 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&spi1_pins>;
> > + status = "okay";
> > +};
> > +
> > +&spi2 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&spi2_pins>;
> > + status = "okay";
> > +};
> > +
> > +&u3phy0 {
> > + status = "okay";
> > +};
> > +
> > +&u3phy1 {
> > + status = "okay";
> > +};
> > +
> > +&u3phy2 {
> > + status = "okay";
> > +};
> > +
> > +&uart0 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&uart0_pins>;
> > + status = "okay";
> > +};
> > +
>
> ..snip..
>
> > + };
> > + };
> > +};
>
> After that:
> Reviewed-by: Alexandre Mergnat <amergnat at baylibre.com>
>
> --
> Regards,
> Alexandre
More information about the Linux-mediatek
mailing list