mbox series

[v1,0/5] arm64: dts: rockchip: add basic dtsi/dts files for RK3568 SoC

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

Message

陈亮 April 21, 2021, 6:59 a.m. UTC
From: Liang Chen <cl@rock-chips.com>

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

Liang Chen (5):
  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
  arm64: dts: rockchip: add core dtsi for RK3568 SoC
  arm64: dts: rockchip: add basic dts for RK3568 EVB

 .../devicetree/bindings/arm/rockchip.yaml     |    5 +
 .../devicetree/bindings/i2c/i2c-rk3x.yaml     |    1 +
 .../bindings/mmc/rockchip-dw-mshc.yaml        |    4 +
 .../bindings/serial/snps-dw-apb-uart.yaml     |    1 +
 arch/arm64/boot/dts/rockchip/Makefile         |    1 +
 .../boot/dts/rockchip/rk3568-evb1-v10.dts     |  382 +++
 .../boot/dts/rockchip/rk3568-pinctrl.dtsi     | 2789 +++++++++++++++++
 arch/arm64/boot/dts/rockchip/rk3568.dtsi      |  795 +++++
 .../boot/dts/rockchip/rockchip-pinconf.dtsi   |  346 ++
 9 files changed, 4324 insertions(+)
 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 21, 2021, 9:26 a.m. UTC | #1
On 4/21/21 8:59 AM, cl@rock-chips.com wrote:
> From: Liang Chen <cl@rock-chips.com>
> 
> add "rockchip,rk3568-dwcmshc", "snps,dwcmshc-sdhci" for mmc nodes on
> a rk3568 platform to rockchip-dw-mshc.yaml.
> 
> Signed-off-by: Liang Chen <cl@rock-chips.com>
> ---
>  Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml
> index 3762f1c8de96..2a6c1cee4887 100644
> --- a/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml
> +++ b/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.yaml
> @@ -27,6 +27,8 @@ properties:
>        - const: rockchip,rk2928-dw-mshc
>        # for Rockchip RK3288
>        - const: rockchip,rk3288-dw-mshc

> +      # for Rockchip RK3568
> +      - const: rockchip,rk3568-dwcmshc

Remove.
This would create two descriptions.
"rockchip,rk3568-dwcmshc"
"rockchip,rk3568-dwcmshc", "snps,dwcmshc-sdhci"

>        - items:
>            - enum:

>              # for Rockchip PX30

Remove comments.

> @@ -41,6 +43,8 @@ properties:
>                - rockchip,rk3328-dw-mshc

>              # for Rockchip RK3368

Remove comments.

>                - rockchip,rk3368-dw-mshc

> +            # for Rockchip RK3568

Maybe remove the "#" comments in this enum part too.
This was one of the first Rockchip documents that was converted, but is
not really needed, as it both mentions the soc name.

> +              - rockchip,rk3568-dw-mshc

Sort this below rockchip,rk3399-dw-mshc

>              # for Rockchip RK3399

Remove comments.
>                - rockchip,rk3399-dw-mshc

>              # for Rockchip RV1108

Remove comments.

>
Marc Zyngier April 21, 2021, 1:36 p.m. UTC | #2
On Wed, 21 Apr 2021 07:59:20 +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 equipments.
> 
> This patch add basic core dtsi file for it.
> 
> Signed-off-by: Liang Chen <cl@rock-chips.com>
> ---
>  .../boot/dts/rockchip/rk3568-pinctrl.dtsi     | 2789 +++++++++++++++++
>  arch/arm64/boot/dts/rockchip/rk3568.dtsi      |  795 +++++
>  .../boot/dts/rockchip/rockchip-pinconf.dtsi   |  346 ++
>  3 files changed, 3930 insertions(+)
>  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
>

[...]

> diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> new file mode 100644
> index 000000000000..ac8db2f54f2b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
> @@ -0,0 +1,795 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2021 Rockchip Electronics Co., Ltd.
> + */
> +
> +#include <dt-bindings/clock/rk3568-cru.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/pinctrl/rockchip.h>
> +#include <dt-bindings/soc/rockchip,boot-mode.h>
> +#include <dt-bindings/phy/phy.h>
> +#include <dt-bindings/thermal/thermal.h>
> +
> +/ {
> +	compatible = "rockchip,rk3568";
> +
> +	interrupt-parent = <&gic>;
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +
> +	aliases {
> +		serial2 = &uart2;
> +	};
> +
> +	cpus {
> +		#address-cells = <2>;
> +		#size-cells = <0>;
> +
> +		cpu0: cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a55";
> +			reg = <0x0 0x0>;
> +			enable-method = "psci";
> +			clocks = <&scmi_clk 0>;
> +			operating-points-v2 = <&cpu0_opp_table>;
> +			#cooling-cells = <2>;
> +		};
> +		cpu1: cpu@100 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a55";
> +			reg = <0x0 0x100>;
> +			enable-method = "psci";
> +			operating-points-v2 = <&cpu0_opp_table>;
> +		};
> +		cpu2: cpu@200 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a55";
> +			reg = <0x0 0x200>;
> +			enable-method = "psci";
> +			operating-points-v2 = <&cpu0_opp_table>;
> +		};
> +		cpu3: cpu@300 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a55";
> +			reg = <0x0 0x300>;
> +			enable-method = "psci";
> +			operating-points-v2 = <&cpu0_opp_table>;
> +		};
> +	};
> +
> +	cpu0_opp_table: cpu0-opp-table {
> +		compatible = "operating-points-v2";
> +		opp-shared;
> +
> +		opp-408000000 {
> +			opp-hz = /bits/ 64 <408000000>;
> +			opp-microvolt = <825000 825000 1150000>;
> +			clock-latency-ns = <40000>;
> +		};
> +		opp-600000000 {
> +			opp-hz = /bits/ 64 <600000000>;
> +			opp-microvolt = <825000 825000 1150000>;
> +		};
> +		opp-816000000 {
> +			opp-hz = /bits/ 64 <816000000>;
> +			opp-microvolt = <825000 825000 1150000>;
> +			opp-suspend;
> +		};
> +		opp-1104000000 {
> +			opp-hz = /bits/ 64 <1104000000>;
> +			opp-microvolt = <825000 825000 1150000>;
> +		};
> +		opp-1416000000 {
> +			opp-hz = /bits/ 64 <1416000000>;
> +			opp-microvolt = <900000 900000 1150000>;
> +		};
> +		opp-1608000000 {
> +			opp-hz = /bits/ 64 <1608000000>;
> +			opp-microvolt = <975000 975000 1150000>;
> +		};
> +		opp-1800000000 {
> +			opp-hz = /bits/ 64 <1800000000>;
> +			opp-microvolt = <1050000 1050000 1150000>;
> +		};
> +		opp-1992000000 {
> +			opp-hz = /bits/ 64 <1992000000>;
> +			opp-microvolt = <1150000 1150000 1150000>;
> +		};
> +	};
> +
> +	arm-pmu {
> +		compatible = "arm,cortex-a55-pmu", "arm,armv8-pmuv3";
> +		interrupts = <GIC_SPI 228 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 229 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 230 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 231 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-affinity = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>;
> +	};
> +
> +	firmware {
> +		scmi: scmi {
> +			compatible = "arm,scmi-smc";
> +			shmem = <&scmi_shmem>;
> +			arm,smc-id = <0x82000010>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			scmi_clk: protocol@14 {
> +				reg = <0x14>;
> +				#clock-cells = <1>;
> +			};
> +		};
> +
> +	};
> +
> +	psci {
> +		compatible = "arm,psci-1.0";
> +		method = "smc";
> +	};
> +
> +	timer {
> +		compatible = "arm,armv8-timer";
> +		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
> +			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
> +			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
> +			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;

This doesn't match the GICv3 binding for PPIs.

> +		arm,no-tick-in-suspend;

Oh, really? :-(

> +	};
> +
> +	xin24m: xin24m {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <24000000>;
> +		clock-output-names = "xin24m";
> +	};
> +
> +	xin32k: xin32k {
> +		compatible = "fixed-clock";
> +		clock-frequency = <32768>;
> +		clock-output-names = "xin32k";
> +		#clock-cells = <0>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&clk32k_out0>;
> +	};
> +
> +	scmi_shmem: scmi-shmem@10f000 {
> +		compatible = "arm,scmi-shmem";
> +		reg = <0x0 0x0010f000 0x0 0x100>;
> +	};
> +
> +	gic: interrupt-controller@fd400000 {
> +		compatible = "arm,gic-v3";
> +		#interrupt-cells = <3>;
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +		interrupt-controller;
> +
> +		reg = <0x0 0xfd400000 0 0x10000>, /* GICD */
> +		      <0x0 0xfd460000 0 0xc0000>; /* GICR */
> +		interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;

Please add the 'mbi-alias' property, which should map onto the GICA
range that GIC600 provides. At least this could be useful to have MSIs
despite the lack of a working ITS. We can work out the usable ranges
on a per-board basis.

Thanks,

	M.
Ezequiel Garcia April 22, 2021, 5:23 p.m. UTC | #3
Hi Liang,

I'm very impressed Rockchip is pushing patches so early, thanks a lot!

See below.

On Wed, 2021-04-21 at 11:13 +0200, Heiko Stübner wrote:
> Hi Liang,
> 
> Am Mittwoch, 21. April 2021, 08:59:20 CEST schrieb cl@rock-chips.com:
> > 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 equipments.
> > 
> > This patch add basic core dtsi file for it.
> > 
> > Signed-off-by: Liang Chen <cl@rock-chips.com>
> 
> this is a first round of basic stuff :-) .
> 
> First of all, I really like the move of moving the pretty standardized
> pinconfig entries to the rockchip-pinconf.dtsi .
> 
> (1) But please move this into a separate patch to make that more visible
> and maybe even convert _some_ or all arm64 Rockchip socs to use that
> as well
> 
> "arm64: dts: rockchip: add generic pinconfig settings used by most Rockchip socs
> 
> The pinconfig settings for Rockchip SoCs are pretty similar on all socs,
> so move them to a shared dtsi to be included, instead of redefining them
> for each soc"
> 
> (2) I also like the external rk3568-pinctrl approach with the dtsi getting
> auto-generated. This will probably help us in keeping pinctrl settings
> synchronous between mainline and the vendor kernel.
> 
> (3) From my basic understanding the rk3568 is basically a rk3566 + more
> peripherals, so ideally they would share the basic ones in a rk3566.dtsi
> which the rk3568.dtsi then could include and extend with its additional
> peripherals.
> 
> With at least the pine64 boards being based on the rk3566, there probably
> will be quite a mainline use of it as well.
> 
> Or is there something that would prevent this?
> 

I agree with having a rk3566.dtsi, and rk3568.dtsi on top, instead of the
other way around. We have some RK3566 boards here, so we can surely test
the RK3566.dtsi patches very quickly.

Also, it's fine if you want to send v2 with just these minimal peripherals.
However, I think you could include GMAC and TS-ADC:

https://lore.kernel.org/linux-rockchip/31c2e531-96d0-a1c1-644c-28c60eb40cf4@gmail.com/T/#t
https://lore.kernel.org/linux-rockchip/20210421203409.40717-1-ezequiel@collabora.com/T/#t

These should work right out of the box!

Thanks!
Ezequiel
陈亮 April 25, 2021, 9:16 a.m. UTC | #4
Hi Marc,

     Thanks for reply.

     See below.

在 2021/4/21 下午9:36, Marc Zyngier 写道:
> On Wed, 21 Apr 2021 07:59:20 +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 equipments.
>>
>> This patch add basic core dtsi file for it.
>>
>> Signed-off-by: Liang Chen <cl@rock-chips.com>
>> ---
>>   .../boot/dts/rockchip/rk3568-pinctrl.dtsi     | 2789 +++++++++++++++++
>>   arch/arm64/boot/dts/rockchip/rk3568.dtsi      |  795 +++++
>>   .../boot/dts/rockchip/rockchip-pinconf.dtsi   |  346 ++
>>   3 files changed, 3930 insertions(+)
>>   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
>>
> [...]
>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3568.dtsi b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>> new file mode 100644
>> index 000000000000..ac8db2f54f2b
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/rockchip/rk3568.dtsi
>> @@ -0,0 +1,795 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Copyright (c) 2021 Rockchip Electronics Co., Ltd.
>> + */
>> +
>> +#include <dt-bindings/clock/rk3568-cru.h>
>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> +#include <dt-bindings/interrupt-controller/irq.h>
>> +#include <dt-bindings/pinctrl/rockchip.h>
>> +#include <dt-bindings/soc/rockchip,boot-mode.h>
>> +#include <dt-bindings/phy/phy.h>
>> +#include <dt-bindings/thermal/thermal.h>
>> +
>> +/ {
>> +	compatible = "rockchip,rk3568";
>> +
>> +	interrupt-parent = <&gic>;
>> +	#address-cells = <2>;
>> +	#size-cells = <2>;
>> +
>> +	aliases {
>> +		serial2 = &uart2;
>> +	};
>> +
>> +	cpus {
>> +		#address-cells = <2>;
>> +		#size-cells = <0>;
>> +
>> +		cpu0: cpu@0 {
>> +			device_type = "cpu";
>> +			compatible = "arm,cortex-a55";
>> +			reg = <0x0 0x0>;
>> +			enable-method = "psci";
>> +			clocks = <&scmi_clk 0>;
>> +			operating-points-v2 = <&cpu0_opp_table>;
>> +			#cooling-cells = <2>;
>> +		};
>> +		cpu1: cpu@100 {
>> +			device_type = "cpu";
>> +			compatible = "arm,cortex-a55";
>> +			reg = <0x0 0x100>;
>> +			enable-method = "psci";
>> +			operating-points-v2 = <&cpu0_opp_table>;
>> +		};
>> +		cpu2: cpu@200 {
>> +			device_type = "cpu";
>> +			compatible = "arm,cortex-a55";
>> +			reg = <0x0 0x200>;
>> +			enable-method = "psci";
>> +			operating-points-v2 = <&cpu0_opp_table>;
>> +		};
>> +		cpu3: cpu@300 {
>> +			device_type = "cpu";
>> +			compatible = "arm,cortex-a55";
>> +			reg = <0x0 0x300>;
>> +			enable-method = "psci";
>> +			operating-points-v2 = <&cpu0_opp_table>;
>> +		};
>> +	};
>> +
>> +	cpu0_opp_table: cpu0-opp-table {
>> +		compatible = "operating-points-v2";
>> +		opp-shared;
>> +
>> +		opp-408000000 {
>> +			opp-hz = /bits/ 64 <408000000>;
>> +			opp-microvolt = <825000 825000 1150000>;
>> +			clock-latency-ns = <40000>;
>> +		};
>> +		opp-600000000 {
>> +			opp-hz = /bits/ 64 <600000000>;
>> +			opp-microvolt = <825000 825000 1150000>;
>> +		};
>> +		opp-816000000 {
>> +			opp-hz = /bits/ 64 <816000000>;
>> +			opp-microvolt = <825000 825000 1150000>;
>> +			opp-suspend;
>> +		};
>> +		opp-1104000000 {
>> +			opp-hz = /bits/ 64 <1104000000>;
>> +			opp-microvolt = <825000 825000 1150000>;
>> +		};
>> +		opp-1416000000 {
>> +			opp-hz = /bits/ 64 <1416000000>;
>> +			opp-microvolt = <900000 900000 1150000>;
>> +		};
>> +		opp-1608000000 {
>> +			opp-hz = /bits/ 64 <1608000000>;
>> +			opp-microvolt = <975000 975000 1150000>;
>> +		};
>> +		opp-1800000000 {
>> +			opp-hz = /bits/ 64 <1800000000>;
>> +			opp-microvolt = <1050000 1050000 1150000>;
>> +		};
>> +		opp-1992000000 {
>> +			opp-hz = /bits/ 64 <1992000000>;
>> +			opp-microvolt = <1150000 1150000 1150000>;
>> +		};
>> +	};
>> +
>> +	arm-pmu {
>> +		compatible = "arm,cortex-a55-pmu", "arm,armv8-pmuv3";
>> +		interrupts = <GIC_SPI 228 IRQ_TYPE_LEVEL_HIGH>,
>> +			     <GIC_SPI 229 IRQ_TYPE_LEVEL_HIGH>,
>> +			     <GIC_SPI 230 IRQ_TYPE_LEVEL_HIGH>,
>> +			     <GIC_SPI 231 IRQ_TYPE_LEVEL_HIGH>;
>> +		interrupt-affinity = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>;
>> +	};
>> +
>> +	firmware {
>> +		scmi: scmi {
>> +			compatible = "arm,scmi-smc";
>> +			shmem = <&scmi_shmem>;
>> +			arm,smc-id = <0x82000010>;
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			scmi_clk: protocol@14 {
>> +				reg = <0x14>;
>> +				#clock-cells = <1>;
>> +			};
>> +		};
>> +
>> +	};
>> +
>> +	psci {
>> +		compatible = "arm,psci-1.0";
>> +		method = "smc";
>> +	};
>> +
>> +	timer {
>> +		compatible = "arm,armv8-timer";
>> +		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
>> +			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
>> +			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
>> +			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
> This doesn't match the GICv3 binding for PPIs.
fixed in V2.
>
>> +		arm,no-tick-in-suspend;
> Oh, really? :-(
yes, arm arch timer will stop in suspend mode on rk3568.
>
>> +	};
>> +
>> +	xin24m: xin24m {
>> +		compatible = "fixed-clock";
>> +		#clock-cells = <0>;
>> +		clock-frequency = <24000000>;
>> +		clock-output-names = "xin24m";
>> +	};
>> +
>> +	xin32k: xin32k {
>> +		compatible = "fixed-clock";
>> +		clock-frequency = <32768>;
>> +		clock-output-names = "xin32k";
>> +		#clock-cells = <0>;
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&clk32k_out0>;
>> +	};
>> +
>> +	scmi_shmem: scmi-shmem@10f000 {
>> +		compatible = "arm,scmi-shmem";
>> +		reg = <0x0 0x0010f000 0x0 0x100>;
>> +	};
>> +
>> +	gic: interrupt-controller@fd400000 {
>> +		compatible = "arm,gic-v3";
>> +		#interrupt-cells = <3>;
>> +		#address-cells = <2>;
>> +		#size-cells = <2>;
>> +		ranges;
>> +		interrupt-controller;
>> +
>> +		reg = <0x0 0xfd400000 0 0x10000>, /* GICD */
>> +		      <0x0 0xfd460000 0 0xc0000>; /* GICR */
>> +		interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> Please add the 'mbi-alias' property, which should map onto the GICA
> range that GIC600 provides. At least this could be useful to have MSIs
> despite the lack of a working ITS. We can work out the usable ranges
> on a per-board basis.

Thanks, we will try mbi-alias later, but we are afraid that the number 
of SPI is not enough.


>
> Thanks,
>
> 	M.
>
Marc Zyngier April 25, 2021, 10:20 a.m. UTC | #5
On Sun, 25 Apr 2021 10:16:50 +0100,
陈亮 <cl@rock-chips.com> wrote:
> 

> Hi Marc,

> 

>     Thanks for reply.

> 

>     See below.

> 

> 在 2021/4/21 下午9:36, Marc Zyngier 写道:

> > On Wed, 21 Apr 2021 07:59:20 +0100,

> > <cl@rock-chips.com> wrote:

> >> From: Liang Chen <cl@rock-chips.com>


[...]

> >> +	timer {

> >> +		compatible = "arm,armv8-timer";

> >> +		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,

> >> +			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,

> >> +			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,

> >> +			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;

> > This doesn't match the GICv3 binding for PPIs.

> fixed in V2.

> > 

> >> +		arm,no-tick-in-suspend;

> > Oh, really? :-(

> yes, arm arch timer will stop in suspend mode on rk3568.


That the comparator stops is completely expected, as it lives in the
CPU power domain.

But losing the system counter on suspend is a complete violation of
the architecture, which clearly states that "The system counter must
be implemented in an always-on power domain." (ARMv8 ARM G_a, D11.1.2
The system counter).

Are you actually losing the system counter?

> >

> >> +	};

> >> +

> >> +	xin24m: xin24m {

> >> +		compatible = "fixed-clock";

> >> +		#clock-cells = <0>;

> >> +		clock-frequency = <24000000>;

> >> +		clock-output-names = "xin24m";

> >> +	};

> >> +

> >> +	xin32k: xin32k {

> >> +		compatible = "fixed-clock";

> >> +		clock-frequency = <32768>;

> >> +		clock-output-names = "xin32k";

> >> +		#clock-cells = <0>;

> >> +		pinctrl-names = "default";

> >> +		pinctrl-0 = <&clk32k_out0>;

> >> +	};

> >> +

> >> +	scmi_shmem: scmi-shmem@10f000 {

> >> +		compatible = "arm,scmi-shmem";

> >> +		reg = <0x0 0x0010f000 0x0 0x100>;

> >> +	};

> >> +

> >> +	gic: interrupt-controller@fd400000 {

> >> +		compatible = "arm,gic-v3";

> >> +		#interrupt-cells = <3>;

> >> +		#address-cells = <2>;

> >> +		#size-cells = <2>;

> >> +		ranges;

> >> +		interrupt-controller;

> >> +

> >> +		reg = <0x0 0xfd400000 0 0x10000>, /* GICD */

> >> +		      <0x0 0xfd460000 0 0xc0000>; /* GICR */

> >> +		interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;

> > Please add the 'mbi-alias' property, which should map onto the GICA

> > range that GIC600 provides. At least this could be useful to have MSIs

> > despite the lack of a working ITS. We can work out the usable ranges

> > on a per-board basis.

> 

> Thanks, we will try mbi-alias later, but we are afraid that the number

> of SPI is not enough.


Not enough for what? People might want to trade the use of in-SoC
devices for MSIs. Also, the DT should describe the whole of the HW,
including features that you don't find useful.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.