Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190312091520.8863-1-jiaxun.yang@flygoat.com> <20190312091520.8863-5-jiaxun.yang@flygoat.com>
In-Reply-To: <20190312091520.8863-5-jiaxun.yang@flygoat.com>
From:   Rob Herring <robh@kernel.org>
Date:   Tue, 12 Mar 2019 07:28:00 -0500
Message-ID: <CABGGiszYh0uE_ybrfhK2byz4XZVAm9wvL5tQg0R85nnLt4c1iw@mail.gmail.com>
Subject: Re: [PATCH 4/4] MIPS: Loongson32: dts: add ls1b & ls1c
To:     Jiaxun Yang <jiaxun.yang@flygoat.com>
Cc:     linux-mips@vger.kernel.org, paul.burton@mips.com,
        keguang.zhang@gmail.com,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Mar 12, 2019 at 4:16 AM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote=
:
>
> Add devicetree skeleton for ls1b and ls1c
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>  arch/mips/boot/dts/loongson/Makefile  |   6 ++
>  arch/mips/boot/dts/loongson/ls1b.dts  |  21 +++++
>  arch/mips/boot/dts/loongson/ls1c.dts  |  25 ++++++
>  arch/mips/boot/dts/loongson/ls1x.dtsi | 117 ++++++++++++++++++++++++++
>  4 files changed, 169 insertions(+)
>  create mode 100644 arch/mips/boot/dts/loongson/Makefile
>  create mode 100644 arch/mips/boot/dts/loongson/ls1b.dts
>  create mode 100644 arch/mips/boot/dts/loongson/ls1c.dts
>  create mode 100644 arch/mips/boot/dts/loongson/ls1x.dtsi
>
> diff --git a/arch/mips/boot/dts/loongson/Makefile b/arch/mips/boot/dts/lo=
ongson/Makefile
> new file mode 100644
> index 000000000000..447801568f33
> --- /dev/null
> +++ b/arch/mips/boot/dts/loongson/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0
> +dtb-$(CONFIG_LOONGSON1_LS1B)   +=3D ls1b.dtb
> +
> +dtb-$(CONFIG_LOONGSON1_LS1B)   +=3D ls1c.dtb
> +
> +obj-$(CONFIG_BUILTIN_DTB)      +=3D $(addsuffix .o, $(dtb-y))
> diff --git a/arch/mips/boot/dts/loongson/ls1b.dts b/arch/mips/boot/dts/lo=
ongson/ls1b.dts
> new file mode 100644
> index 000000000000..6d40dc502acf
> --- /dev/null
> +++ b/arch/mips/boot/dts/loongson/ls1b.dts
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019 Jiaxun Yang <jiaxun.yang@flygoat.com>
> + */
> +
> +/dts-v1/;
> +#include <ls1x.dtsi>
> +
> +/ {
> +       model =3D "Loongson LS1B";
> +       compatible =3D "loongson,ls1b";

Documented?

> +
> +};
> +
> +&ehci0 {
> +       status =3D "okay";
> +};
> +
> +&ohci0 {
> +       status =3D "okay";
> +};
> \ No newline at end of file

Fix this.

> diff --git a/arch/mips/boot/dts/loongson/ls1c.dts b/arch/mips/boot/dts/lo=
ongson/ls1c.dts
> new file mode 100644
> index 000000000000..778d205a586e
> --- /dev/null
> +++ b/arch/mips/boot/dts/loongson/ls1c.dts
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019 Jiaxun Yang <jiaxun.yang@flygoat.com>
> + */
> +
> +/dts-v1/;
> +#include <ls1x.dtsi>
> +
> +/ {
> +       model =3D "Loongson LS1C300A";
> +       compatible =3D "loongson,ls1c300a";
> +
> +};
> +
> +&platintc4 {
> +       status =3D "okay";
> +};
> +
> +&ehci0 {
> +       status =3D "okay";
> +};
> +
> +&ohci0 {
> +       status =3D "okay";
> +};
> \ No newline at end of file
> diff --git a/arch/mips/boot/dts/loongson/ls1x.dtsi b/arch/mips/boot/dts/l=
oongson/ls1x.dtsi
> new file mode 100644
> index 000000000000..f808e4328fd8
> --- /dev/null
> +++ b/arch/mips/boot/dts/loongson/ls1x.dtsi
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019 Jiaxun Yang <jiaxun.yang@flygoat.com>
> + */
> +
> +/dts-v1/;
> +#include <dt-bindings/interrupt-controller/irq.h>
> +
> +
> +/ {
> +    #address-cells =3D <1>;
> +       #size-cells =3D <1>;
> +
> +       cpus {
> +               #address-cells =3D <1>;
> +               #size-cells =3D <0>;
> +
> +               cpu@0 {
> +                       device_type =3D "cpu";
> +                       reg =3D <0>;

Needs a (documented) compatible string.

> +               };
> +       };
> +
> +       cpu_intc: interrupt-controller {
> +               #address-cells =3D <0>;
> +               compatible =3D "mti,cpu-interrupt-controller";
> +
> +               interrupt-controller;
> +               #interrupt-cells =3D <1>;
> +       };
> +
> +       soc {
> +               #address-cells =3D <1>;
> +               #size-cells =3D <1>;
> +
> +               compatible =3D "simple-bus";
> +               ranges;
> +
> +
> +               platintc0: interrupt-controller@1fd01040 {
> +                       compatible =3D "loongson,ls1x-intc";
> +                       reg =3D <0x1fd01040 0x18>;
> +
> +                       interrupt-controller;
> +                       #interrupt-cells =3D <2>;
> +
> +                       interrupt-parent =3D <&cpu_intc>;
> +                       interrupts =3D <2>;
> +               };
> +
> +               platintc1: interrupt-controller@1fd01058 {
> +                       compatible =3D "loongson,ls1x-intc";
> +                       reg =3D <0x1fd01058 0x18>;
> +
> +                       interrupt-controller;
> +                       #interrupt-cells =3D <2>;
> +
> +                       interrupt-parent =3D <&cpu_intc>;
> +                       interrupts =3D <3>;
> +               };
> +
> +               platintc2: interrupt-controller@1fd01070 {
> +                       compatible =3D "loongson,ls1x-intc";
> +                       reg =3D <0x1fd01070 0x18>;
> +
> +                       interrupt-controller;
> +                       #interrupt-cells =3D <2>;
> +
> +                       interrupt-parent =3D <&cpu_intc>;
> +                       interrupts =3D <4>;
> +               };
> +
> +               platintc3: interrupt-controller@1fd01088 {
> +                       compatible =3D "loongson,ls1x-intc";
> +                       reg =3D <0x1fd01088 0x18>;
> +
> +                       interrupt-controller;
> +                       #interrupt-cells =3D <2>;
> +
> +                       interrupt-parent =3D <&cpu_intc>;
> +                       interrupts =3D <5>;
> +               };
> +
> +               platintc4: interrupt-controller@1fd010a0 {
> +                       compatible =3D "loongson,ls1x-intc";
> +                       reg =3D <0x1fd010a0 0x18>;
> +
> +                       interrupt-controller;
> +                       #interrupt-cells =3D <2>;
> +
> +                       interrupt-parent =3D <&cpu_intc>;
> +                       interrupts =3D <6>;
> +
> +           status =3D "disabled";

Some indentation problem.

> +               };
> +
> +               ehci0: usb@1fe20000 {
> +                       compatible =3D "generic-ehci";

It would be better to add a chip specific compatible here. Most all
USB controllers have some quirks.

> +                       reg =3D <0x1fe20000 0x100>;
> +                       interrupt-parent =3D <&platintc1>;
> +                       interrupts =3D <0 IRQ_TYPE_LEVEL_HIGH>;
> +
> +           status =3D "disabled";
> +                       };
> +
> +               ohci0: usb@1fe28000 {
> +                       compatible =3D "generic-ohci";
> +                       reg =3D <0x1fe28000 0x100>;
> +                       interrupt-parent =3D <&platintc1>;
> +                       interrupts =3D <1 IRQ_TYPE_LEVEL_HIGH>;
> +
> +           status =3D "disabled";
> +                       };

Don't you need a serial port or something for a console?

> +
> +       };
> +};
> +\ =E6=96=87=E4=BB=B6=E5=B0=BE=E6=B2=A1=E6=9C=89=E6=8D=A2=E8=A1=8C=E7=AC=
=A6
> --
> 2.20.1
>