mbox series

[v3,00/10] arm64: dts: rockchip: add basic dtsi/dts files for RK3568 SoC

Message ID 20210428134759.22076-1-cl@rock-chips.com
Headers show
Series arm64: dts: rockchip: add basic dtsi/dts files for RK3568 SoC | expand

Message

cl April 28, 2021, 1:47 p.m. UTC
From: Liang Chen <cl@rock-chips.com>

v1:
1. add some dt-bindings for RK3568 devices.
2. add core dtsi for RK3568 SoC.
3. add basic dts for RK3568 EVB

v2:
1. sort device nodes by some rules.

v3:
1. make ARCH=arm64 dtbs_check, then fix some errors and add some documents.

Liang Chen (10):
  dt-bindings: i2c: i2c-rk3x: add description for rk3568
  dt-bindings: serial: snps-dw-apb-uart: add description for rk3568
  dt-bindings: mmc: rockchip-dw-mshc: add description for rk3568
  dt-bindings: watchdog: dw-wdt: add description for rk3568
  dt-bindings: pwm: rockchip: add description for rk3568
  dt-bindings: gpio: change items restriction of clock for
    rockchip,gpio-bank
  dt-bindings: soc: rockchip: Convert grf.txt to YAML
  arm64: dts: rockchip: add generic pinconfig settings used by most
    Rockchip socs
  arm64: dts: rockchip: add core dtsi for RK3568 SoC
  arm64: dts: rockchip: add basic dts for RK3568 EVB

 .../devicetree/bindings/arm/rockchip.yaml     |    5 +
 .../bindings/gpio/rockchip,gpio-bank.yaml     |    3 +-
 .../devicetree/bindings/i2c/i2c-rk3x.yaml     |    1 +
 .../bindings/mmc/rockchip-dw-mshc.yaml        |    9 +-
 .../devicetree/bindings/pwm/pwm-rockchip.yaml |    1 +
 .../bindings/serial/snps-dw-apb-uart.yaml     |    1 +
 .../devicetree/bindings/soc/rockchip/grf.txt  |   61 -
 .../devicetree/bindings/soc/rockchip/grf.yaml |   58 +
 .../bindings/watchdog/snps,dw-wdt.yaml        |    1 +
 arch/arm64/boot/dts/rockchip/Makefile         |    1 +
 .../boot/dts/rockchip/rk3568-evb1-v10.dts     |   80 +
 .../boot/dts/rockchip/rk3568-pinctrl.dtsi     | 3111 +++++++++++++++++
 arch/arm64/boot/dts/rockchip/rk3568.dtsi      |  789 +++++
 .../boot/dts/rockchip/rockchip-pinconf.dtsi   |  344 ++
 14 files changed, 4395 insertions(+), 70 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/soc/rockchip/grf.txt
 create mode 100644 Documentation/devicetree/bindings/soc/rockchip/grf.yaml
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3568-pinctrl.dtsi
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3568.dtsi
 create mode 100644 arch/arm64/boot/dts/rockchip/rockchip-pinconf.dtsi

Comments

Johan Jonker April 28, 2021, 4:56 p.m. UTC | #1
Hi Liang,

Check example with:
make ARCH=arm64 dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/soc/rockchip/grf.yaml

Build log:
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20210428134938.22383-1-cl@rock-chips.com/

On 4/28/21 3:49 PM, cl@rock-chips.com wrote:
> From: Liang Chen <cl@rock-chips.com>
> 
> Current dts files with 'grf' nodes are manually verified. In order to
> automate this process grf.txt has to be converted to YAML.

Add new descriptions for:
"rockchip,rk3399-pmugrf", "syscon", "simple-mfd"
"rockchip,rk3399-grf", "syscon", "simple-mfd"

> 
> Signed-off-by: Liang Chen <cl@rock-chips.com>
> ---
>  .../devicetree/bindings/soc/rockchip/grf.txt  | 61 -------------------
>  .../devicetree/bindings/soc/rockchip/grf.yaml | 61 +++++++++++++++++++
>  2 files changed, 61 insertions(+), 61 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/soc/rockchip/grf.txt
>  create mode 100644 Documentation/devicetree/bindings/soc/rockchip/grf.yaml
> 
> diff --git a/Documentation/devicetree/bindings/soc/rockchip/grf.txt b/Documentation/devicetree/bindings/soc/rockchip/grf.txt
> deleted file mode 100644
> index f96511aa3897..000000000000
> --- a/Documentation/devicetree/bindings/soc/rockchip/grf.txt
> +++ /dev/null
> @@ -1,61 +0,0 @@
> -* Rockchip General Register Files (GRF)
> -
> -The general register file will be used to do static set by software, which
> -is composed of many registers for system control.
> -
> -From RK3368 SoCs, the GRF is divided into two sections,
> -- GRF, used for general non-secure system,
> -- SGRF, used for general secure system,
> -- PMUGRF, used for always on system
> -
> -On RK3328 SoCs, the GRF adds a section for USB2PHYGRF,
> -
> -ON RK3308 SoC, the GRF is divided into four sections:
> -- GRF, used for general non-secure system,
> -- SGRF, used for general secure system,
> -- DETECTGRF, used for audio codec system,
> -- COREGRF, used for pvtm,
> -
> -Required Properties:
> -
> -- compatible: GRF should be one of the following:
> -   - "rockchip,px30-grf", "syscon": for px30
> -   - "rockchip,rk3036-grf", "syscon": for rk3036
> -   - "rockchip,rk3066-grf", "syscon": for rk3066
> -   - "rockchip,rk3188-grf", "syscon": for rk3188
> -   - "rockchip,rk3228-grf", "syscon": for rk3228
> -   - "rockchip,rk3288-grf", "syscon": for rk3288
> -   - "rockchip,rk3308-grf", "syscon": for rk3308
> -   - "rockchip,rk3328-grf", "syscon": for rk3328
> -   - "rockchip,rk3368-grf", "syscon": for rk3368
> -   - "rockchip,rk3399-grf", "syscon": for rk3399
> -   - "rockchip,rv1108-grf", "syscon": for rv1108
> -- compatible: DETECTGRF should be one of the following:
> -   - "rockchip,rk3308-detect-grf", "syscon": for rk3308
> -- compatilbe: COREGRF should be one of the following:
> -   - "rockchip,rk3308-core-grf", "syscon": for rk3308
> -- compatible: PMUGRF should be one of the following:
> -   - "rockchip,px30-pmugrf", "syscon": for px30
> -   - "rockchip,rk3368-pmugrf", "syscon": for rk3368
> -   - "rockchip,rk3399-pmugrf", "syscon": for rk3399
> -- compatible: SGRF should be one of the following:
> -   - "rockchip,rk3288-sgrf", "syscon": for rk3288
> -- compatible: USB2PHYGRF should be one of the following:
> -   - "rockchip,px30-usb2phy-grf", "syscon": for px30
> -   - "rockchip,rk3328-usb2phy-grf", "syscon": for rk3328
> -- compatible: USBGRF should be one of the following:
> -   - "rockchip,rv1108-usbgrf", "syscon": for rv1108
> -- reg: physical base address of the controller and length of memory mapped
> -  region.
> -
> -Example: GRF and PMUGRF of RK3399 SoCs
> -
> -	pmugrf: syscon@ff320000 {
> -		compatible = "rockchip,rk3399-pmugrf", "syscon";
> -		reg = <0x0 0xff320000 0x0 0x1000>;
> -	};
> -
> -	grf: syscon@ff770000 {
> -		compatible = "rockchip,rk3399-grf", "syscon";
> -		reg = <0x0 0xff770000 0x0 0x10000>;
> -	};
> diff --git a/Documentation/devicetree/bindings/soc/rockchip/grf.yaml b/Documentation/devicetree/bindings/soc/rockchip/grf.yaml
> new file mode 100644
> index 000000000000..61ce5b4c9ed0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/rockchip/grf.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/soc/rockchip/grf.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip General Register Files
> +
> +maintainers:
> +  - Heiko Stuebner <heiko@sntech.de>
> +
> +properties:
> +  compatible:

> +    oneOf:
> +      - items:

items:

When there are no other combinations then with syscon and simple-mfd
then there's no need for "oneOf".


> +          - enum:
> +              - rockchip,px30-grf
> +              - rockchip,px30-pmugrf
> +              - rockchip,px30-usb2phy-grf
> +              - rockchip,rk3036-grf
> +              - rockchip,rk3066-grf
> +              - rockchip,rk3188-grf
> +              - rockchip,rk3228-grf
> +              - rockchip,rk3288-grf
> +              - rockchip,rk3288-sgrf
> +              - rockchip,rk3308-core-grf
> +              - rockchip,rk3308-detect-grf
> +              - rockchip,rk3308-grf
> +              - rockchip,rk3328-grf
> +              - rockchip,rk3328-usb2phy-grf
> +              - rockchip,rk3368-grf
> +              - rockchip,rk3368-pmugrf
> +              - rockchip,rk3399-grf
> +              - rockchip,rk3399-pmugrf

> +              - rockchip,rk3568-grf
> +              - rockchip,rk3568-pmugrf

Conversion and new properties are two separate actions.
Must add that to the commit message.

> +              - rockchip,rv1108-grf
> +              - rockchip,rv1108-usbgrf
> +          - const: syscon
> +          - const: simple-mfd
> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    pmugrf: syscon@ff320000 {

> +       compatible = "rockchip,rk3399-pmugrf", "syscon";

compatible = "rockchip,rk3399-pmugrf", "syscon", "simple-mfd";

> +       reg = <0x0 0xff320000 0x0 0x1000>;

This is for 64 bit, normal examples use 32 bit.

> +    };
> +
> +    grf: syscon@ff770000 {

> +       compatible = "rockchip,rk3399-grf", "syscon";

compatible = "rockchip,rk3399-grf", "syscon", "simple-mfd";

> +       reg = <0x0 0xff770000 0x0 0x10000>;

use 32 bit

> +    };
>
Rob Herring (Arm) April 28, 2021, 10:15 p.m. UTC | #2
On Wed, 28 Apr 2021 21:49:38 +0800, cl@rock-chips.com wrote:
> From: Liang Chen <cl@rock-chips.com>
> 
> Current dts files with 'grf' nodes are manually verified. In order to
> automate this process grf.txt has to be converted to YAML.
> 
> Signed-off-by: Liang Chen <cl@rock-chips.com>
> ---
>  .../devicetree/bindings/soc/rockchip/grf.txt  | 61 -------------------
>  .../devicetree/bindings/soc/rockchip/grf.yaml | 61 +++++++++++++++++++
>  2 files changed, 61 insertions(+), 61 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/soc/rockchip/grf.txt
>  create mode 100644 Documentation/devicetree/bindings/soc/rockchip/grf.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/rockchip/grf.example.dt.yaml: example-0: syscon@ff320000:reg:0: [0, 4281466880, 0, 4096] is too long
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/rockchip/grf.example.dt.yaml: example-0: syscon@ff770000:reg:0: [0, 4285988864, 0, 65536] is too long
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/rockchip/grf.example.dt.yaml: syscon@ff320000: compatible: 'oneOf' conditional failed, one must be fixed:
	['rockchip,rk3399-pmugrf', 'syscon'] is too short
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/rockchip/grf.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/rockchip/grf.example.dt.yaml: syscon@ff770000: compatible: 'oneOf' conditional failed, one must be fixed:
	['rockchip,rk3399-grf', 'syscon'] is too short
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/rockchip/grf.yaml

See https://patchwork.ozlabs.org/patch/1471171

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
Marc Zyngier April 29, 2021, 7:40 a.m. UTC | #3
On Thu, 29 Apr 2021 02:13:35 +0100,
陈亮 <cl@rock-chips.com> wrote:
> 
> Hi Marc,
> 
> 在 2021/4/28 下午11:06, Marc Zyngier 写道:
> > On Wed, 28 Apr 2021 14:50:02 +0100,
> > <cl@rock-chips.com> wrote:
> >> From: Liang Chen <cl@rock-chips.com>
> >> 
> >> RK3568 is a high-performance and low power quad-core application processor
> >> designed for personal mobile internet device and AIoT equipment. This patch
> >> add basic core dtsi file for it.
> >> 
> >> We use scmi_clk for cortex-a55 instead of standard ARMCLK, so that
> >> kernel/uboot/rtos can change cpu clk with the same code in ATF, and we will
> >> enalbe a special high-performance PLL when high frequency is required. The
> >> smci_clk code is in ATF, and clkid for cpu is 0, as below:
> >> 
> >>      cpu0: cpu@0 {
> >>          device_type = "cpu";
> >>          compatible = "arm,cortex-a55";
> >>          reg = <0x0 0x0>;
> >>          clocks = <&scmi_clk 0>;
> >>      };
> >> 
> >> Signed-off-by: Liang Chen <cl@rock-chips.com>
> >> ---
> >>   .../boot/dts/rockchip/rk3568-pinctrl.dtsi     | 3111 +++++++++++++++++
> >>   arch/arm64/boot/dts/rockchip/rk3568.dtsi      |  779 +++++
> >>   2 files changed, 3890 insertions(+)
> >>   create mode 100644 arch/arm64/boot/dts/rockchip/rk3568-pinctrl.dtsi
> >>   create mode 100644 arch/arm64/boot/dts/rockchip/rk3568.dtsi
> > [...]
> > 
> >> +	gic: interrupt-controller@fd400000 {
> >> +		compatible = "arm,gic-v3";
> >> +		reg = <0x0 0xfd400000 0 0x10000>, /* GICD */
> >> +		      <0x0 0xfd460000 0 0xc0000>; /* GICR */
> > If this SoC has 4 CPUs, that's 4 redistributors. Given that GIC600
> > doesn't implement VLPIs, that's 128kB per redistributors. Why is GICR
> > large enough for 6 CPUs here? Is that copy-pasted from another SoC?
> Copy from rk3399, sorry.
> >> +		interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> >> +		interrupt-controller;
> >> +		#interrupt-cells = <3>;
> >> +		mbi-alias = <0x0 0xfd400000>;
> >> +		mbi-ranges = <296 24>;
> >> +		msi-controller;
> >> +	};
> > Glad to see that you found some spare SPIs to get MSIs going
> > 
> > However, the whole point of mbi-alias (aka GICA in GIC600) is to be
> > different from GICD and provide some isolation via an IOMMU.  If I
> > trust the TRM, if should be at 0xfd10000 in your implementation.
> 
> But in the ./devicetree/bindings/interrupt-controller/arm,gic-v3.yaml, say:
> 
>   mbi-alias:
>     description:
>       Address property. Base address of an alias of the *GICD* region
> containing
>       only the {SET,CLR}SPI registers to be used if isolation is required,
>       and if supported by the HW.


[recurring theme: I happen to know about this section of the binding,
having written the original myself]

How does that contradict my comment? GIC600's GICA page only contains
the four {SET,CLR}_SPI registers, as expected (see section 4.3 in the
TRM[1]), and the address is computed using table 4-1 "Register map
pages" of the same document.

Please either fix the DT or explain why the GICA distributor alias
isn't usable.

	M.

[1] https://documentation-service.arm.com/static/5e7ddddacbfe76649ba53034