From: "Marty E. Plummer" <hanetzer@startmail.com>
To: Rob Herring <robh@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
xuejiancheng@hisilicon.com, leo.yan@linaro.org,
linux-clk@vger.kernel.org,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
mark.rutland@arm.com, mturquette@baylibre.com,
wenpan@hisilicon.com, linux@armlinux.org.uk,
sboyd@codeaurora.org, xuwei5@hisilicon.com,
zhangfei.gao@linaro.org, gregkh@linuxfoundation.org,
arnd@arndb.de
Subject: Re: [RFC RESEND 3/3] arm: dts: add Hi3521A dts
Date: Wed, 20 Sep 2017 21:15:16 -0500 [thread overview]
Message-ID: <20170921021511.q6cbew5d5ec5cxxd@proprietary-killer.fossland> (raw)
In-Reply-To: <CAL_JsqLfRN=CjcDbwANE7gwCQS-_s99f6UANPLAooiaWB1mvfA@mail.gmail.com>
On Thu, Sep 21, 2017 at 01:08:39AM +0000, Rob Herring wrote:
> On Wed, Sep 20, 2017 at 6:04 PM, Marty E. Plummer
> <hanetzer@startmail.com> wrote:
> > On Wed, Sep 20, 2017 at 08:53:03PM +0000, Rob Herring wrote:
> >> On Sun, Sep 17, 2017 at 03:23:27AM -0500, Marty E. Plummer wrote:
> >> > Add hi3521a.dtsi and hi3521a-rs-dm290e.dts for RaySharp CCTV systems,
> >> > marketed under the name Samsung SDR-B74301N
> >> >
> >> > Signed-off-by: Marty E. Plummer <hanetzer@startmail.com>
> >> > ---
> >> > arch/arm/boot/dts/Makefile | 2 +
> >> > arch/arm/boot/dts/hi3521a-rs-dm290e.dts | 52 ++++++
> >> > arch/arm/boot/dts/hi3521a.dtsi | 310 ++++++++++++++++++++++++++++++++
> >> > 3 files changed, 364 insertions(+)
> >> > create mode 100644 arch/arm/boot/dts/hi3521a-rs-dm290e.dts
> >> > create mode 100644 arch/arm/boot/dts/hi3521a.dtsi
> >> >
> >> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> >> > index faf46abaa4a2..e7b9b5dde20f 100644
> >> > --- a/arch/arm/boot/dts/Makefile
> >> > +++ b/arch/arm/boot/dts/Makefile
> >> > @@ -189,6 +189,8 @@ dtb-$(CONFIG_ARCH_GEMINI) += \
> >> > gemini-sq201.dtb \
> >> > gemini-wbd111.dtb \
> >> > gemini-wbd222.dtb
> >> > +dtb-$(CONFIG_ARCH_HI3521A) += \
> >> > + hi3521a-rs-dm290e.dtb
> >> > dtb-$(CONFIG_ARCH_HI3xxx) += \
> >> > hi3620-hi4511.dtb
> >> > dtb-$(CONFIG_ARCH_HIGHBANK) += \
> >> > diff --git a/arch/arm/boot/dts/hi3521a-rs-dm290e.dts b/arch/arm/boot/dts/hi3521a-rs-dm290e.dts
> >> > new file mode 100644
> >> > index 000000000000..b32c8392c93f
> >> > --- /dev/null
> >> > +++ b/arch/arm/boot/dts/hi3521a-rs-dm290e.dts
> >> > @@ -0,0 +1,52 @@
> >> > +/*
> >> > + * Copyright (C) 2017 Marty Plummer <hanetzer@startmail.com>
> >> > + *
> >> > + * This program is free software: you can redistribute it and/or modify
> >> > + * it under the terms of the GNU General Public License as published by
> >> > + * the Free Software Foundation, either version 3 of the License, or
> >> > + * (at your option) any later version.
> >>
> >> Should be version 2 or later? Doesn't really matter to me from a DT
> >> perspective, but it is in the kernel tree.
> >>
> >> You can use SPDX tags if you want.
> >>
> > Oh, that's a good idea. I hadn't seen any SPDX tags in the tree that I
> > noticed before. I ended up just using the :Gpl command from neovim.
> >> > + *
> >> > + * This program is distributed in the hope that it will be useful,
> >> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >> > + * GNU General Public License for more details.
> >> > + *
> >> > + * You should have received a copy of the GNU General Public License
> >> > + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> >> > + */
> >> > +
> >> > +/dts-v1/;
> >> > +#include "hi3521a.dtsi"
> >> > +
> >> > +/ {
> >> > + model = "RaySharp RS-DM-290E DVR Board";
> >> > + compatible = "hisilicon,hi3521a";
> >>
> >> Needs a board compatible too.
> >>
> > Something like `compatible = "hisilicon,hi3521a", "raysharp,rs-dm-290e";` ?
>
> Yes, but flip the order. Most specific compatible first.
>
> >> > +
> >> > + aliases {
> >> > + serial0 = &uart0;
> >> > + serial1 = &uart1;
> >> > + serial2 = &uart2;
> >> > + };
> >> > +
> >> > + memory {
> >>
> >> Needs a unit-address.
> >>
> > Could you explain what you mean here? As in, memory@someaddr? What would
> > I use here?
>
> "memory@80000000". Building with W=2 will tell you.
>
Ah, nice trick. Suppose that makes sense, as every other thing was the
same on that sort of thing. Not sure if I've ever seen memory@addr
before.
> >> > + device_type = "memory";
> >> > + reg = <0x80000000 0xf00000>;
> >> > + };
> >> > +};
> >> > +
> >> > +&hi_sfc {
> >> > + status = "okay";
> >> > + spi-nor@0 {
> >> > + compatible = "jedec,spi-nor";
> >>
> >> I don't remember offhand, but I think this should have a device specific
> >> compatible too.
> >>
> > Instead of "jedec,spi-nor" ? Specific to the SPI chip?
>
> No, both with jedec,spi-nor 2nd.
>
Gotcha, will fix it up.
> >> > + reg = <0>;
> >> > + spi-max-frequency = <104000000>;
> >> > + };
> >> > +};
> >> > +
> >> > +&uart0 {
> >> > + status = "okay";
> >> > +};
> >> > +
> >> > +&dual_timer0 {
> >> > + status = "okay";
> >> > +};
> >> > diff --git a/arch/arm/boot/dts/hi3521a.dtsi b/arch/arm/boot/dts/hi3521a.dtsi
> >> > new file mode 100644
> >> > index 000000000000..2af746fdec46
> >> > --- /dev/null
> >> > +++ b/arch/arm/boot/dts/hi3521a.dtsi
> >> > @@ -0,0 +1,310 @@
> >> > +/*
> >> > + * Copyright (C) 2017 Marty Plummer <hanetzer@startmail.com>
> >> > + *
> >> > + * This program is free software: you can redistribute it and/or modify
> >> > + * it under the terms of the GNU General Public License as published by
> >> > + * the Free Software Foundation, either version 3 of the License, or
> >> > + * (at your option) any later version.
> >> > + *
> >> > + * This program is distributed in the hope that it will be useful,
> >> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >> > + * GNU General Public License for more details.
> >> > + *
> >> > + * You should have received a copy of the GNU General Public License
> >> > + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> >> > + */
> >> > +
> >> > +#include <dt-bindings/clock/hi3521a-clock.h>
> >> > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> >> > +/ {
> >> > + #address-cells = <1>;
> >> > + #size-cells = <1>;
> >> > + chosen { };
> >> > +
> >> > + cpus {
> >> > + #address-cells = <1>;
> >> > + #size-cells = <0>;
> >> > +
> >> > + cpu0: cpu@0 {
> >> > + device_type = "cpu";
> >> > + compatible = "arm,cortex-a7";
> >> > + reg = <0>;
> >> > + };
> >> > + };
> >> > +
> >> > + hi_sfc: spi-nor-controller@10000000 {
> >> > + compatible = "hisilicon,hi3521a-spi-nor", "hisilicon,fmc-spi-nor";
> >> > + #address-cells = <1>;
> >> > + #size-cells = <0>;
> >> > + reg = <0x10000000 0x10000>, <0x14000000 0x1000000>;
> >> > + reg-names = "control", "memory";
> >> > + clocks = <&crg HI3521A_FMC_CLK>;
> >> > + status = "disabled";
> >> > + };
> >> > +
> >> > + gic: interrupt-controller@10300000 {
> >> > + compatible = "arm,pl390";
> >> > + #interrupt-cells = <3>;
> >> > + interrupt-controller;
> >> > + reg = <0x10301000 0x1000>, <0x10302000 0x1000>;
> >> > + };
> >> > +
> >> > + clk_3m: clk_3m {
> >> > + compatible = "fixed-clock";
> >> > + #clock-cells = <0>;
> >> > + clock-frequency = <3000000>;
> >> > + };
> >> > +
> >> > + crg: clock-reset-controller@12040000 {
> >> > + compatible = "hisilicon,hi3521a-crg";
> >> > + #clock-cells = <1>;
> >> > + #reset-cells = <2>;
> >> > + reg = <0x12040000 0x10000>;
> >> > + };
> >>
> >> These memory mapped peripherals should be under a bus node.
> >>
> > Crap, will fix.
> >> > +
> >> > + soc {
> >> > + #address-cells = <1>;
> >> > + #size-cells = <1>;
> >> > + compatible = "simple-bus";
> >> > + interrupt-parent = <&gic>;
> >> > + ranges;
> >>
> >> It is preferred to have a value here and limit the range of the bus
> >> addresses.
> >>
> > Yeah, I think I've seen that before, I don't quite grok how that works.
> >> > +
> >> > + dmac: dma@10060000 {
> >>
> >> dma-controller@...
> >>
> > Will fix.
> >> > + compatible = "arm,pl080", "arm,primecell";
> >> > + reg = <0x10060000 0x1000>;
> >> > + interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
> >> > + status = "disabled";
> >>
> >> I wouldn't think enabling dma would be a per board decision.
> >>
> > I've just noticed that in general dtsi files just lay it all out and are
> > mostly "disabled", though if you think this should be explicitly enabled
> > thats fine by me.
> >> > + };
> >> > +
> >> > + dual_timer0: timer@12000000 {
> >> > + compatible = "arm,sp804", "arm,primecell";
> >> > + interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
> >> > + <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> >> > + reg = <0x12000000 0x1000>;
> >> > + clocks = <&clk_3m>;
> >> > + clock-names = "apb_pclk";
> >>
> >> IIRC, it is deprecated to have a single clock here. The h/w has 2 clock
> >> inputs.
> >>
> > Are you meaning for the 0x0 index and 0x20 index clocks?
>
> Yes. Maybe it's 3 clocks. Anyway, should all be in the sp804 binding doc.
>
> >> Where's the ARM architected timer?
> >>
> > Unsure tbqf, just doing my best to translate a datasheet into code. Do
> > all ARM soc's have one?
>
> All A7's should I think.
>
Gotcha.
> Rob
next prev parent reply other threads:[~2017-09-21 2:15 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-17 8:23 [RFC RESEND 0/3] Add support for Hisilicon Hi3521A SoC Marty E. Plummer
2017-09-17 8:23 ` [RFC RESEND 1/3] clk: hisilicon: add CRG driver " Marty E. Plummer
2017-09-17 8:23 ` [RFC RESEND 2/3] arm: hisi: enable " Marty E. Plummer
2017-09-17 8:23 ` [RFC RESEND 3/3] arm: dts: add Hi3521A dts Marty E. Plummer
[not found] ` <20170917082327.10058-4-hanetzer-zVALfsKDk1AS+FvcfC7Uqw@public.gmane.org>
2017-09-20 20:53 ` Rob Herring
2017-09-20 23:04 ` Marty E. Plummer
[not found] ` <20170920230405.tz3vzoys2vn72rgv-1hXBbFsFebEOoWXT+9nehYcPyU6MP8rFhTcysPvq43Q@public.gmane.org>
2017-09-21 1:08 ` Rob Herring
2017-09-21 2:15 ` Marty E. Plummer [this message]
2017-09-18 10:55 ` [RFC RESEND 0/3] Add support for Hisilicon Hi3521A SoC Greg KH
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=20170921021511.q6cbew5d5ec5cxxd@proprietary-killer.fossland \
--to=hanetzer@startmail.com \
--cc=arnd@arndb.de \
--cc=devicetree@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=leo.yan@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=mark.rutland@arm.com \
--cc=mturquette@baylibre.com \
--cc=robh@kernel.org \
--cc=sboyd@codeaurora.org \
--cc=wenpan@hisilicon.com \
--cc=xuejiancheng@hisilicon.com \
--cc=xuwei5@hisilicon.com \
--cc=zhangfei.gao@linaro.org \
/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).