lkml.org 
[lkml]   [2019]   [Apr]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 2/2] ARM: dts: sun50i: h6: Introduce Beelink GS1 board
Hi,

On Fri, 5 Apr 2019 at 11:08, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> Hi,
>
> On Thu, Apr 04, 2019 at 02:45:04PM +0200, Clément Péron wrote:
> > Beelink GS1 is an Allwinner H6 based TV box,
> > which support:
> > - Allwinner H6 Quad-core 64-bit ARM Cortex-A53
> > - GPU Mali-T720
> > - 2GB LPDDR3 RAM
> > - AXP805 PMIC
> > - 1Gbps GMAC via RTL8211E
> > - FN-Link 6222B-SRB Wifi/BT
> > - 1x USB 2.0 Host and 1x USB 3.0 Host
> > - HDMI port
> > - S/PDIF Tx
> > - IR receiver
> > - 5V/2A DC power supply
> >
> > Signed-off-by: Clément Péron <peron.clem@gmail.com>
> > ---
> >
> > Thanks to Jagan and Icenowy, most of this devicetree is taken from their works on
> > the OrangePi One Plus and Pine H64.
> >
> > arch/arm64/boot/dts/allwinner/Makefile | 1 +
> > .../dts/allwinner/sun50i-h6-beelink-gs1.dts | 252 ++++++++++++++++++
> > 2 files changed, 253 insertions(+)
> > create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
> > index e4dce2f6fa3a..2dd806a3d5c9 100644
> > --- a/arch/arm64/boot/dts/allwinner/Makefile
> > +++ b/arch/arm64/boot/dts/allwinner/Makefile
> > @@ -20,6 +20,7 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-pc2.dtb
> > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-prime.dtb
> > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-zero-plus.dtb
> > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-orangepi-zero-plus2.dtb
> > +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-beelink-gs1.dtb
> > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-lite2.dtb
> > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-one-plus.dtb
> > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-pine-h64.dtb
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
> > new file mode 100644
> > index 000000000000..d927e7d33bf3
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
> > @@ -0,0 +1,252 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ or MIT)
> > +/*
> > + * Copyright (C) 2019 Clément Péron <peron.clem@gmail.com>
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include "sun50i-h6.dtsi"
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +
> > +/ {
> > + model = "Beelink GS1";
> > + compatible = "azw,beelink-gs1", "allwinner,sun50i-h6";
>
> Please add that board to
> Documentation/devicetree/bindings/arm/sunxi.yaml, it's new and in
> linux-next.

OK didn't know about that will do it.

>
> > +&mmc0 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&mmc0_pins>;
> > + vmmc-supply = <&reg_cldo1>;
> > + cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>;
> > + bus-width = <4>;
> > + status = "okay";
> > +};
> > +
> > +&mmc2 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&mmc2_pins>;
>
> If those (mmc0 and mmc2_pins) are the only muxing options as their
> names suggests, it should be set in the DTSI.
Yes, it's the only muxing. I will add a patch to move these in the SOC dtsi.

>
> If not, the name of the pin group should be changed.
>
> > + reg_cldo2: cldo2 {
> > + regulator-always-on;
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > + regulator-name = "vcc-wifi-1";
> > + };
> > +
> > + reg_cldo3: cldo3 {
> > + regulator-always-on;
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > + regulator-name = "vcc-wifi-2";
> > + };
>
> I don't see any SDIO bus declared, why do you need those regulators on?
You're right forget to remove. Will do.

>
> Can you also declare the PIO regulators?
Ok,

Thanks for the review.


>
> Thanks!
> Maxmie
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

\
 
 \ /
  Last update: 2019-04-05 14:00    [W:0.048 / U:5.280 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site