linux-kernel - Re: [PATCH v4 16/17] arm64: dts: rockchip: Add vop on rk3588

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d45b1a26-3ea3-4699-b86f-cc7d38718721@rock-chips.com>
Date:   Sat, 9 Dec 2023 09:28:55 +0800
From:   Andy Yan <andy.yan@...k-chips.com>
To:     Heiko Stübner <heiko@...ech.de>,
        Andy Yan <andyshrk@....com>
Cc:     hjc@...k-chips.com, dri-devel@...ts.freedesktop.org,
        linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org,
        krzysztof.kozlowski+dt@...aro.org, robh+dt@...nel.org,
        devicetree@...r.kernel.org, sebastian.reichel@...labora.com,
        kever.yang@...k-chips.com, chris.obbard@...labora.com
Subject: Re: [PATCH v4 16/17] arm64: dts: rockchip: Add vop on rk3588

Hi Heiko:

On 12/9/23 00:26, Heiko Stübner wrote:
> Hi Andy,
> 
> Am Donnerstag, 7. Dezember 2023, 09:02:47 CET schrieb Andy Yan:
>> From: Andy Yan <andy.yan@...k-chips.com>
>>
>> Add vop dt node for rk3588.
>>
>> Signed-off-by: Andy Yan <andy.yan@...k-chips.com>
>> ---
>>
>> (no changes since v1)
>>
>>   arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 96 +++++++++++++++++++++++
>>   1 file changed, 96 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
>> index 7064c0e9179f..a9810ca78dc4 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
>> @@ -593,6 +608,87 @@ i2c0: i2c@...80000 {
>>   		status = "disabled";
>>   	};
>>   
>> +	vop: vop@...90000 {
>> +		compatible = "rockchip,rk3588-vop";
>> +		reg = <0x0 0xfdd90000 0x0 0x4200>, <0x0 0xfdd95000 0x0 0x1000>;
>> +		reg-names = "vop", "gamma_lut";
>> +		interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH 0>;
>> +		clocks = <&cru ACLK_VOP>,
>> +			 <&cru HCLK_VOP>,
>> +			 <&cru DCLK_VOP0>,
>> +			 <&cru DCLK_VOP1>,
>> +			 <&cru DCLK_VOP2>,
>> +			 <&cru DCLK_VOP3>,
>> +			 <&cru PCLK_VOP_ROOT>;
>> +		clock-names = "aclk",
>> +			      "hclk",
>> +			      "dclk_vp0",
>> +			      "dclk_vp1",
>> +			      "dclk_vp2",
>> +			      "dclk_vp3",
>> +			      "pclk_vop";
>> +		resets = <&cru SRST_A_VOP>,
>> +			 <&cru SRST_H_VOP>,
>> +			 <&cru SRST_D_VOP0>,
>> +			 <&cru SRST_D_VOP1>,
>> +			 <&cru SRST_D_VOP2>,
>> +			 <&cru SRST_D_VOP3>;
>> +		reset-names = "axi",
>> +			      "ahb",
>> +			      "dclk_vp0",
>> +			      "dclk_vp1",
>> +			      "dclk_vp2",
>> +			      "dclk_vp3";
> 
> resets and reset-names do not seem to be part of the binding, so
> should probably be added there.

I will drop resets and reset-names in next version.
> 
> 
>> +		iommus = <&vop_mmu>;
>> +		power-domains = <&power RK3588_PD_VOP>;
>> +		rockchip,grf = <&sys_grf>;
>> +		rockchip,vop-grf = <&vop_grf>;
>> +		rockchip,vo1-grf = <&vo1_grf>;
>> +		rockchip,pmu = <&pmu>;
>> +
> 
> move this blank line _below_ the status=disabled please.

ok, will do.
> 
>> +		status = "disabled";
>> +		vop_out: ports {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			vp0: port@0 {
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +				reg = <0>;
>> +			};
>> +
>> +			vp1: port@1 {
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +				reg = <1>;
>> +			};
>> +
>> +			vp2: port@2 {
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +				reg = <2>;
>> +			};
>> +
>> +			vp3: port@3 {
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +				reg = <3>;
>> +			};
>> +		};
>> +	};
>> +
>> +	vop_mmu: iommu@...97e00 {
>> +		compatible = "rockchip,rk3588-iommu", "rockchip,rk3568-iommu";
>> +		reg = <0x0 0xfdd97e00 0x0 0x100>, <0x0 0xfdd97f00 0x0 0x100>;
>> +		interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH 0>;
>> +		interrupt-names = "vop_mmu";
> 
> interrupt-names is not part of the mainline iommu binding, so should be dropped

will do.
> 
> 
> Thanks
> Heiko
> 
>