devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Heiko Stuebner <heiko@sntech.de>
To: Andy Yan <andyshrk@gmail.com>
Cc: devicetree@vger.kernel.org, robh+dt@kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	"open list:ARM/Rockchip SoC..."
	<linux-rockchip@lists.infradead.org>
Subject: Re: [PATCH] arm64: dts: rockchip: Add dts for Leez RK3399 P710 SBC
Date: Sun, 04 Aug 2019 11:59:16 +0200	[thread overview]
Message-ID: <2357897.ziZui2JcdL@phil> (raw)
In-Reply-To: <CANbgqATNLO=wJfKNDY74qmQUqAP_9Xv29nGhLj+Y0Cc7CAMQyg@mail.gmail.com>

Hi Andy,

Am Sonntag, 4. August 2019, 10:38:26 CEST schrieb Andy Yan:
> Heiko Stuebner <heiko@sntech.de> 于2019年8月4日周日 上午8:34写道:
> > Am Samstag, 3. August 2019, 13:46:12 CEST schrieb Andy Yan:
> > > Leez P710 is a RK3399 based SBC, designed by Leez team
> > > from lenovo [0].
> > >
> > > Specification
> > > - Rockchip RK3399
> > > - 4/2GB LPDDR4
> > > - TF sd scard slot
> > > - eMMC
> > > - M.2 B-Key for 4G LTE
> > > - AP6256 for WiFi + BT
> > > - Gigabit ethernet
> > > - HDMI out
> > > - 40 pin header
> > > - TYPE-C Power supply
> > >
> > > [0] https://leez.lenovo.com
> > >
> > > Signed-off-by: Andy Yan <andyshrk@gmail.com>
> > > ---
> > >  .../devicetree/bindings/arm/rockchip.yaml     |   5 +
> > >  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
> > >  .../boot/dts/rockchip/rk3399-leez-p710.dts    | 635 ++++++++++++++++++
> > >  3 files changed, 641 insertions(+)
> > >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts
> > >
> > > diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml
> > b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > index 34865042f4e4..da9cd947abfa 100644
> > > --- a/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > +++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > @@ -329,6 +329,11 @@ properties:
> > >                - khadas,edge-v
> > >            - const: rockchip,rk3399
> > >
> > > +      - description: Leez RK3399 P710
> > > +        items:
> > > +          - const: leez,p710
> >
> > Is "leez" really the vendor?
> > Part of me would assume something like
> >         lenovo,leez-p710
> >
> >
> According to Leez team, they want to treat Leez an independent vendor here。

ok, that should be fine then. You'll just have to also add
an entry for them into the vendor-prefixes.yaml file then.

(And of course rework the regulator tree)

Thanks
Heiko

> > So please clarify :-)
> > And also please make sure the decided vendor is part of the vendor-prefixes
> > binding in Documentation/devicestree/bindings/vendor-prefixes.yaml
> >
> > > +          - const: rockchip,rk3399
> > > +
> > >        - description: mqmaker MiQi
> > >          items:
> > >            - const: mqmaker,miqi
> > > diff --git a/arch/arm64/boot/dts/rockchip/Makefile
> > b/arch/arm64/boot/dts/rockchip/Makefile
> > > index daa2c78e22c3..1f18a9392d15 100644
> > > --- a/arch/arm64/boot/dts/rockchip/Makefile
> > > +++ b/arch/arm64/boot/dts/rockchip/Makefile
> > > @@ -20,6 +20,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-hugsun-x99.dtb
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-khadas-edge.dtb
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-khadas-edge-captain.dtb
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-khadas-edge-v.dtb
> > > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-leez-p710.dtb
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopc-t4.dtb
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopi-m4.dtb
> > >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-nanopi-neo4.dtb
> > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts
> > b/arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts
> > > new file mode 100644
> > > index 000000000000..b342f5e8692b
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/rockchip/rk3399-leez-p710.dts
> > > @@ -0,0 +1,635 @@
> > > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > > +/*
> > > + * Copyright (c) 2019 Andy Yan <andy.yan@gmail.com>
> > > + */
> > > +
> > > +/dts-v1/;
> > > +#include <dt-bindings/input/linux-event-codes.h>
> > > +#include <dt-bindings/pwm/pwm.h>
> > > +#include "rk3399.dtsi"
> > > +#include "rk3399-opp.dtsi"
> > > +
> > > +/ {
> > > +     model = "Leez RK3399 P710";
> > > +     compatible = "leez,p710", "rockchip,rk3399";
> >
> > same comment as above, so maybe:
> >         model = "Lenovo Leez RK3399 P710";
> >         compatible = "lenovo,leez-p710", "rockchip,rk3399";
> >
> >
> >
> > > +
> > > +     chosen {
> > > +             stdout-path = "serial2:1500000n8";
> > > +     };
> > > +
> > > +     clkin_gmac: external-gmac-clock {
> > > +             compatible = "fixed-clock";
> > > +             clock-frequency = <125000000>;
> > > +             clock-output-names = "clkin_gmac";
> > > +             #clock-cells = <0>;
> > > +     };
> > > +
> > > +     sdio_pwrseq: sdio-pwrseq {
> > > +             compatible = "mmc-pwrseq-simple";
> > > +             clocks = <&rk808 1>;
> > > +             clock-names = "ext_clock";
> > > +             pinctrl-names = "default";
> > > +             pinctrl-0 = <&wifi_enable_h>;
> > > +             reset-gpios = <&gpio0 RK_PB2 GPIO_ACTIVE_LOW>;
> > > +     };
> > > +
> > > +     dc5v_adp: dc-5v {
> > > +             compatible = "regulator-fixed";
> > > +             regulator-name = "dc5v_adapter";
> > > +             regulator-always-on;
> > > +             regulator-boot-on;
> > > +             regulator-min-microvolt = <5000000>;
> > > +             regulator-max-microvolt = <5000000>;
> > > +     };
> > > +
> > > +     vcc5v0_sys: vcc-sys {
> >
> > vcc5v0_sys: vcc5v0-sys ?
> >
> > > +             compatible = "regulator-fixed";
> > > +             regulator-name = "vcc5v0_sys";
> > > +             regulator-always-on;
> > > +             regulator-boot-on;
> > > +             regulator-min-microvolt = <5000000>;
> > > +             regulator-max-microvolt = <5000000>;
> > > +             vin-supply = <&dc5v_adp>;
> > > +     };
> > > +
> > > +     vcc3v3_sys: vcc3v3-sys {
> > > +             compatible = "regulator-fixed";
> > > +             regulator-name = "vcc3v3_sys";
> > > +             regulator-always-on;
> > > +             regulator-boot-on;
> > > +             regulator-min-microvolt = <3300000>;
> > > +             regulator-max-microvolt = <3300000>;
> > > +             vin-supply = <&vcc5v0_sys>;
> > > +     };
> > > +
> > > +     vcc5v0_host: vcc5v0-host-regulator {
> > > +             compatible = "regulator-fixed";
> > > +             enable-active-high;
> > > +             gpio = <&gpio2 RK_PA2 GPIO_ACTIVE_HIGH>;
> > > +             pinctrl-names = "default";
> > > +             pinctrl-0 = <&vcc5v0_host_en>;
> > > +             regulator-name = "vcc5v0_host";
> > > +             regulator-always-on;
> > > +             vin-supply = <&vcc5v0_sys>;
> > > +     };
> > > +
> > > +     vcc_lan: vcc3v3-phy-regulator {
> > > +             compatible = "regulator-fixed";
> > > +             regulator-name = "vcc_lan";
> > > +             regulator-always-on;
> > > +             regulator-boot-on;
> > > +             regulator-min-microvolt = <3300000>;
> > > +             regulator-max-microvolt = <3300000>;
> > > +
> > > +             regulator-state-mem {
> > > +                     regulator-off-in-suspend;
> > > +             };
> > > +     };
> >
> > In general, please model an actual regulator-tree and do not copy the
> > unspecific Rockchip vendor tree. These unconnected regulators are a very
> > good indicator that the real power-tree got ignored (missing vin-supply
> > here)
> >
> > I found schematics on https://github.com/leezsbc/resources/wiki/Leez-P710:
> > 链接: https://pan.baidu.com/s/1NPWbuI5csT4zftKUCnRs7g
> > 提取码: rvrh
> >
> > and there the power-tree is described in a complete way.
> >
> > regulator/regulator_summaray in the kernels debugfs should
> > show a nice tree structure starting from the dc-adapter input.
> >
> > Also please use names matching the supply names from the schematics.
> >
> > Same for pinctrl names, please use names as used in the board schematics.
> >
> >
> > > +     vdd_log: vdd-log {
> > > +             compatible = "pwm-regulator";
> > > +             pwms = <&pwm2 0 25000 1>;
> > > +             regulator-name = "vdd_log";
> > > +             regulator-always-on;
> > > +             regulator-boot-on;
> > > +             regulator-min-microvolt = <800000>;
> > > +             regulator-max-microvolt = <1400000>;
> > > +             vin-supply = <&vcc5v0_sys>;
> > > +     };
> > > +};
> > > +
> > > +&cpu_l0 {
> > > +     cpu-supply = <&vdd_cpu_l>;
> > > +};
> > > +
> > > +&cpu_l1 {
> > > +     cpu-supply = <&vdd_cpu_l>;
> > > +};
> > > +
> > > +&cpu_l2 {
> > > +     cpu-supply = <&vdd_cpu_l>;
> > > +};
> > > +
> > > +&cpu_l3 {
> > > +     cpu-supply = <&vdd_cpu_l>;
> > > +};
> > > +
> > > +&cpu_b0 {
> > > +     cpu-supply = <&vdd_cpu_b>;
> > > +};
> > > +
> > > +&cpu_b1 {
> > > +     cpu-supply = <&vdd_cpu_b>;
> > > +};
> > > +
> > > +&emmc_phy {
> > > +     status = "okay";
> > > +};
> > > +
> > > +&gmac {
> > > +     assigned-clocks = <&cru SCLK_RMII_SRC>;
> > > +     assigned-clock-parents = <&clkin_gmac>;
> > > +     clock_in_out = "input";
> > > +     phy-supply = <&vcc_lan>;
> > > +     phy-mode = "rgmii";
> > > +     pinctrl-names = "default";
> > > +     pinctrl-0 = <&rgmii_pins>;
> > > +     snps,reset-gpio = <&gpio3 RK_PB7 GPIO_ACTIVE_LOW>;
> > > +     snps,reset-active-low;
> > > +     snps,reset-delays-us = <0 10000 50000>;
> > > +     tx_delay = <0x28>;
> > > +     rx_delay = <0x11>;
> > > +     status = "okay";
> > > +};
> > > +
> > > +&gpu {
> > > +     mali-supply = <&vdd_gpu>;
> > > +     status = "okay";
> > > +};
> > > +
> > > +&hdmi {
> > > +     ddc-i2c-bus = <&i2c3>;
> >
> > can this also use the internal i2c inside the dw-hdmi?
> >
> >
> > > +     pinctrl-names = "default";
> > > +     pinctrl-0 = <&hdmi_cec>;
> > > +     status = "okay";
> > > +};
> > > +
> > > +&hdmi_sound {
> > > +     status = "okay";
> > > +};
> > > +
> > > +&i2c0 {
> > > +     clock-frequency = <400000>;
> > > +     i2c-scl-rising-time-ns = <168>;
> > > +     i2c-scl-falling-time-ns = <4>;
> > > +     status = "okay";
> > > +
> > > +     rk808: pmic@1b {
> > > +             compatible = "rockchip,rk808";
> > > +             reg = <0x1b>;
> > > +             interrupt-parent = <&gpio1>;
> > > +             interrupts = <21 IRQ_TYPE_LEVEL_LOW>;
> > > +             #clock-cells = <1>;
> > > +             clock-output-names = "xin32k", "rk808-clkout2";
> > > +             pinctrl-names = "default";
> > > +             pinctrl-0 = <&pmic_int_l>;
> > > +             rockchip,system-power-controller;
> > > +             wakeup-source;
> > > +
> > > +             vcc1-supply = <&vcc5v0_sys>;
> > > +             vcc2-supply = <&vcc5v0_sys>;
> > > +             vcc3-supply = <&vcc5v0_sys>;
> > > +             vcc4-supply = <&vcc5v0_sys>;
> > > +             vcc6-supply = <&vcc5v0_sys>;
> > > +             vcc7-supply = <&vcc5v0_sys>;
> > > +             vcc8-supply = <&vcc3v3_sys>;
> > > +             vcc9-supply = <&vcc5v0_sys>;
> > > +             vcc10-supply = <&vcc5v0_sys>;
> > > +             vcc11-supply = <&vcc5v0_sys>;
> > > +             vcc12-supply = <&vcc3v3_sys>;
> > > +             vddio-supply = <&vcc_1v8>;
> > > +
> > > +             regulators {
> > > +                     vdd_center: DCDC_REG1 {
> > > +                             regulator-name = "vdd_center";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-min-microvolt = <750000>;
> > > +                             regulator-max-microvolt = <1350000>;
> > > +                             regulator-ramp-delay = <6001>;
> > > +                             regulator-state-mem {
> > > +                                     regulator-off-in-suspend;
> > > +                             };
> > > +                     };
> > > +
> > > +                     vdd_cpu_l: DCDC_REG2 {
> > > +                             regulator-name = "vdd_cpu_l";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-min-microvolt = <750000>;
> > > +                             regulator-max-microvolt = <1350000>;
> > > +                             regulator-ramp-delay = <6001>;
> > > +                             regulator-state-mem {
> > > +                                     regulator-off-in-suspend;
> > > +                             };
> > > +                     };
> > > +
> > > +                     vcc_ddr: DCDC_REG3 {
> > > +                             regulator-name = "vcc_ddr";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-state-mem {
> > > +                                     regulator-on-in-suspend;
> > > +                             };
> > > +                     };
> > > +
> > > +                     vcc_1v8: DCDC_REG4 {
> > > +                             regulator-name = "vcc_1v8";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-min-microvolt = <1800000>;
> > > +                             regulator-max-microvolt = <1800000>;
> > > +                             regulator-state-mem {
> > > +                                     regulator-on-in-suspend;
> > > +                                     regulator-suspend-microvolt =
> > <1800000>;
> > > +                             };
> > > +                     };
> > > +
> > > +                     vcc1v8_dvp: LDO_REG1 {
> > > +                             regulator-name = "vcc1v8_dvp";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-min-microvolt = <1800000>;
> > > +                             regulator-max-microvolt = <1800000>;
> > > +                             regulator-state-mem {
> > > +                                     regulator-off-in-suspend;
> > > +                             };
> > > +                     };
> > > +
> > > +                     vcc1v8_hdmi: LDO_REG2 {
> > > +                             regulator-name = "vcc1v8_hdmi";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-min-microvolt = <1800000>;
> > > +                             regulator-max-microvolt = <1800000>;
> > > +                             regulator-state-mem {
> > > +                                     regulator-off-in-suspend;
> > > +                             };
> > > +                     };
> > > +
> > > +                     vcca_1v8: LDO_REG3 {
> > > +                             regulator-name = "vcca_1v8";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-min-microvolt = <1800000>;
> > > +                             regulator-max-microvolt = <1800000>;
> > > +                             regulator-state-mem {
> > > +                                     regulator-on-in-suspend;
> > > +                                     regulator-suspend-microvolt =
> > <1800000>;
> > > +                             };
> > > +                     };
> > > +
> > > +                     vccio_sd: LDO_REG4 {
> > > +                             regulator-name = "vccio_sd";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-min-microvolt = <3000000>;
> > > +                             regulator-max-microvolt = <3000000>;
> > > +                             regulator-state-mem {
> > > +                                     regulator-on-in-suspend;
> > > +                                     regulator-suspend-microvolt =
> > <3000000>;
> > > +                             };
> > > +                     };
> > > +
> > > +                     vcca3v0_codec: LDO_REG5 {
> > > +                             regulator-name = "vcca3v0_codec";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-min-microvolt = <3000000>;
> > > +                             regulator-max-microvolt = <3000000>;
> > > +                             regulator-state-mem {
> > > +                                     regulator-off-in-suspend;
> > > +                             };
> > > +                     };
> > > +
> > > +                     vcc_1v5: LDO_REG6 {
> > > +                             regulator-name = "vcc_1v5";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-min-microvolt = <1500000>;
> > > +                             regulator-max-microvolt = <1500000>;
> > > +                             regulator-state-mem {
> > > +                                     regulator-on-in-suspend;
> > > +                                     regulator-suspend-microvolt =
> > <1500000>;
> > > +                             };
> > > +                     };
> > > +
> > > +                     vcc0v9_hdmi: LDO_REG7 {
> > > +                             regulator-name = "vcc0v9_hdmi";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-min-microvolt = <900000>;
> > > +                             regulator-max-microvolt = <900000>;
> > > +                             regulator-state-mem {
> > > +                                     regulator-off-in-suspend;
> > > +                             };
> > > +                     };
> > > +
> > > +                     vcc_3v0: LDO_REG8 {
> > > +                             regulator-name = "vcc_3v0";
> > > +                             regulator-always-on;
> > > +                             regulator-boot-on;
> > > +                             regulator-min-microvolt = <3000000>;
> > > +                             regulator-max-microvolt = <3000000>;
> > > +                             regulator-state-mem {
> > > +                                     regulator-on-in-suspend;
> > > +                                     regulator-suspend-microvolt =
> > <3000000>;
> > > +                             };
> > > +                     };
> > > +
> >
> > unneeded blank line
> >
> > > +             };
> > > +     };
> > > +
> >
> >
> > Heiko
> >
> >
> >





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

      reply	other threads:[~2019-08-04  9:59 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-03 11:46 [PATCH] arm64: dts: rockchip: Add dts for Leez RK3399 P710 SBC Andy Yan
2019-08-04  0:34 ` Heiko Stuebner
2019-08-04  8:38   ` Andy Yan
2019-08-04  9:59     ` Heiko Stuebner [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=2357897.ziZui2JcdL@phil \
    --to=heiko@sntech.de \
    --cc=andyshrk@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=robh+dt@kernel.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).