From: Georgii Staroselskii <georgii.staroselskii@emlid.com>
To: Maxime Ripard <mripard@kernel.org>
Cc: mark.rutland@arm.com, linux-sunxi@googlegroups.com,
georgii.staroselskii@emlid.com, wens@csie.org,
robh+dt@kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 2/2] arm: dts: sunxi: Add Neutis N5H3 support
Date: Tue, 12 Nov 2019 17:52:55 +0300 [thread overview]
Message-ID: <20191112145255.GA3812@softcrasher> (raw)
In-Reply-To: <20191112121558.GZ4345@gilmour.lan>
On Tue, Nov 12, 2019 at 01:15:58PM +0100, Maxime Ripard wrote:
> Hi,
>
Hi!
> On Wed, Nov 06, 2019 at 05:03:18PM +0300, Georgii Staroselskii wrote:
> > Emlid Neutis N5H3 is a version of Emlid Neutis SoM with H3 instead of H5
> > inside.
> >
> > 6eeb4180d4b9 ("ARM: dts: sunxi: h3-h5: Add Bananapi M2+ v1.2 device")
> > was used as reference.
> >
> > Signed-off-by: Georgii Staroselskii <georgii.staroselskii@emlid.com>
> > ---
> > arch/arm/boot/dts/Makefile | 1 +
> > .../dts/sun8i-h3-emlid-neutis-n5h3-devboard.dts | 61 ++++++++++++++++++++++
> > arch/arm/boot/dts/sun8i-h3-emlid-neutis-n5h3.dtsi | 11 ++++
> > 3 files changed, 73 insertions(+)
> > create mode 100644 arch/arm/boot/dts/sun8i-h3-emlid-neutis-n5h3-devboard.dts
> > create mode 100644 arch/arm/boot/dts/sun8i-h3-emlid-neutis-n5h3.dtsi
> >
> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> > index 3f13b88..c997b0c 100644
> > --- a/arch/arm/boot/dts/Makefile
> > +++ b/arch/arm/boot/dts/Makefile
> > @@ -1119,6 +1119,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
> > sun8i-h3-orangepi-plus2e.dtb \
> > sun8i-h3-orangepi-zero-plus2.dtb \
> > sun8i-h3-rervision-dvk.dtb \
> > + sun8i-h3-emlid-neutis-n5h3-devboard.dtb \
>
> There's no need to duplicate the H3 in the name, we can just call it
>
> sun8i-h3-emlid-neutis-n5-devboard.dts
>
> Unless you expect some other boards named in a similar matter?
>
The thing is that Neutis N5H3 is the name of the board. So I guess it
makes sense to let this name be the part of the dts as well.
> > sun8i-r16-bananapi-m2m.dtb \
> > sun8i-r16-nintendo-nes-classic.dtb \
> > sun8i-r16-nintendo-super-nes-classic.dtb \
> > diff --git a/arch/arm/boot/dts/sun8i-h3-emlid-neutis-n5h3-devboard.dts b/arch/arm/boot/dts/sun8i-h3-emlid-neutis-n5h3-devboard.dts
> > new file mode 100644
> > index 00000000..3b68750
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/sun8i-h3-emlid-neutis-n5h3-devboard.dts
> > @@ -0,0 +1,61 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> > +/*
> > + * DTS for Emlid Neutis N5 Dev board.
> > + *
> > + * Copyright (C) 2019 Georgii Staroselskii <georgiii.staroselskii@emlid.com>
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include "sun8i-h3-emlid-neutis-n5h3.dtsi"
> > +
> > +/ {
> > + model = "Emlid Neutis N5H3 Developer board";
> > + compatible = "emlid,neutis-n5h3-devboard",
> > + "emlid,neutis-n5h3",
> > + "allwinner,sun8i-h3";
>
> Same remarks for the compatible, we have the h3 compatible here to
> differentiate between the two.
>
> You should also document this combination to
> Documentation/devicetree/bindings/arm/sunxi.yaml.
>
Thanks, will do.
> > +
> > + vdd_cpux: gpio-regulator {
> > + compatible = "regulator-gpio";
> > + regulator-name = "vdd-cpux";
> > + regulator-type = "voltage";
> > + regulator-boot-on;
> > + regulator-always-on;
> > + regulator-min-microvolt = <1100000>;
> > + regulator-max-microvolt = <1300000>;
> > + regulator-ramp-delay = <50>; /* 4ms */
> > + gpios = <&r_pio 0 6 GPIO_ACTIVE_HIGH>; /* PL6 */
> > + gpios-states = <0x1>;
> > + states = <1100000 0x0
> > + 1300000 0x1>;
>
> While DTC outputs the same thing, and it works, you should make this
> an array of 2 items of 2 cells, instead of a array of 1 item of 4
> cells.
>
> Like this: states = <1100000 0x0>, <1300000 0x1>;
>
> While this doesn't change anything with DTC, other cases (like DT
> validation) care about this.
>
Thank you!
> Look good otherwise.
I have spotted a couple of issues with the DTSI that I'm going to
address in V2.
Thanks for the review.
> Maxime
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
prev parent reply other threads:[~2019-11-12 14:53 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-06 14:03 [PATCH 0/2] Neutis N5H3 support Georgii Staroselskii
2019-11-06 14:03 ` Georgii Staroselskii
2019-11-06 14:03 ` [PATCH 1/2] arm: dts: allwinner: Split out non-SoC specific parts of Neutis N5 Georgii Staroselskii
2019-11-06 14:03 ` Georgii Staroselskii
2019-11-06 14:03 ` [PATCH 2/2] arm: dts: sunxi: Add Neutis N5H3 support Georgii Staroselskii
2019-11-06 14:03 ` Georgii Staroselskii
2019-11-12 12:15 ` Maxime Ripard
2019-11-12 12:15 ` Maxime Ripard
2019-11-12 14:52 ` Georgii Staroselskii [this message]
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=20191112145255.GA3812@softcrasher \
--to=georgii.staroselskii@emlid.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-sunxi@googlegroups.com \
--cc=mark.rutland@arm.com \
--cc=mripard@kernel.org \
--cc=robh+dt@kernel.org \
--cc=wens@csie.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.