Received-SPF: pass (google.com: domain of linux-kernel+bounces-15848-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <ceacb4a6-7768-43b4-a3e7-ed8e8665f5f6@amd.com>
Date: Wed, 3 Jan 2024 12:19:22 -0600
User-Agent: Mozilla Thunderbird Beta
Subject: Re: [PATCH v8 2/3] dts: zynqmp: add properties for TCM in remoteproc
Content-Language: en-US
To: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: "andersson@kernel.org" <andersson@kernel.org>,
 "robh+dt@kernel.org" <robh+dt@kernel.org>,
 "krzysztof.kozlowski+dt@linaro.org" <krzysztof.kozlowski+dt@linaro.org>,
 "conor+dt@kernel.org" <conor+dt@kernel.org>,
 "Simek, Michal" <michal.simek@amd.com>, "Levinsky, Ben"
 <ben.levinsky@amd.com>,
 "linux-remoteproc@vger.kernel.org" <linux-remoteproc@vger.kernel.org>,
 "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
 "linux-arm-kernel@lists.infradead.org"
 <linux-arm-kernel@lists.infradead.org>,
 "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
References: <20231215235725.1247350-1-tanmay.shah@amd.com>
 <20231215235725.1247350-3-tanmay.shah@amd.com> <ZZWfUkrQc58+GATN@p14s>
From: Tanmay Shah <tanmay.shah@amd.com>
In-Reply-To: <ZZWfUkrQc58+GATN@p14s>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?WW5SWGFvcUxYbEt6UzZyRitGMjVIZU9CclRKTFd2ZzBhQVJWdXRPT1hQZXZS?=
 =?utf-8?B?Nmxya2M3WnFZYkV6c3dPTWFpOGIwVGdjYzBkcWdaQVdGZTYyc2NXR2ZqbCta?=
 =?utf-8?B?RGpEMXVOVldIWTZpc2hNdkZqamtWZHJZMVJPUk9XcCtTcUlhS0NLOThOa2lq?=
 =?utf-8?B?eU9IL0o1RHNiaTJVbTZJWTR1ZEt5eG5veERrdVNXZTZZMVpqZGVpYWM1VTdR?=
 =?utf-8?B?Wmhoa0lEYXlFV3YvdEJPYVo1K2lITXpIYlNSTDN1djZGakoyZ3p2VGVwc2Zy?=
 =?utf-8?B?RzVmN1hEamRyRE0yaisvdTB2S2lXWUJpZHBJRDQzZTNkZTU2Tk9mSmFsNEVI?=
 =?utf-8?B?b2RxaHVpK29HREViN1pQUk82aytUMkRjQlV1alp1cXg3aXFkaXgxV3BzRFRG?=
 =?utf-8?B?emFJNVVFOFFVNHh2S1BnZUdaOVk5OFdxd0VVNmlJK3pYSjhyVEFsOEhPNGpx?=
 =?utf-8?B?UjRsSGhqSitGeUw3bVdneW1BWHZNTGpQZVh4SnZvbmlKR2pwYVB4d1FMbkUw?=
 =?utf-8?B?YzJpdVNXWGpjRkpHSTlsaGNWd3NzUjlCbTRDWUdERW12a2NMSGRUY3B5U0Jl?=
 =?utf-8?B?UG9zUWl0ZjRJUnBaZmdnZ2tjSUlSdHNmVWxSU1FCdXVUVHgyMjFwNUx5SzRZ?=
 =?utf-8?B?L1g0b1JOTTdjNkpQaHFsYTkycS9mVG1qeXBqNlI3aE1PNC9DS3VXWHpSRXNa?=
 =?utf-8?B?ejlLOW5VU2UxeUtCVlVyUDNhOTduRzZFYlk0ZU5ZQ1FJS0V5eTlVZ1hWVk16?=
 =?utf-8?B?MTJnaGxuWi90dmVBS2tXQThsZXh5Z2dkdWRjQmtlY3g5UG9UYTRrOEJPd29X?=
 =?utf-8?B?aWtoTE5zQ0h5MVZLM1pKK0VSM1FXSUVrOENaSHBiN2s5SmJCZjR6czZEMDJu?=
 =?utf-8?B?d3FNa2dtY0xzVzRFYUQ5anQ1RWlEZVY3SXptOXFiSW1IVU50b0dlNnBqR3NP?=
 =?utf-8?B?STJpZnVkVk9Xcko4MkwxTEFhQ1g0M1R5UGpneVBNdktYbkprdG40SVF0T1J4?=
 =?utf-8?B?M093SXVnMDZ2NjMvRkszWEFvVFlIOElmcXRsRytKQ0Npd2VUUUQ1Z0xaNDl3?=
 =?utf-8?B?djFWNXdmRTZZQU9WajVESG85SVdwYmtUajQwVHMxdFRUbDcxa2lIcWFGNUV4?=
 =?utf-8?B?ZS9wZm4vTCtqQS9OTzN4Wjh3ZkVTb0dsY1Q4SlBMODZKR0ZlY1pHOHNRZkh3?=
 =?utf-8?B?SHM3a0FUcktTVnkvWVhvS0hNRWYxNy9RRVczblpNbkxHc1luNFFRUXVwVGpQ?=
 =?utf-8?B?SkZoeVB6V3llZVlVTXFGWExoRnpEbGF2ck92RDRaQnRrNi84cW85VUxqeGMx?=
 =?utf-8?B?djBNSTRKZGdnNWpNQThwUTZTWitKelBLUjVQZXpodmMzK3puSHE5c1M3ckhD?=
 =?utf-8?B?MEJIcG91eTRNZHJiNFBLVUdnQldTVHhSM01xdmJCNzBjbmpXcXN3dkdVbjUv?=
 =?utf-8?B?V0lRT3JJQ0dUbjk0UklEZ3hpMHd2eG1hNFVISGx1b1BxalV6MmlLNVJ5bHNl?=
 =?utf-8?B?dGQ2di9ReEhwc3VFUHVoM0J4Zk5tbGhleFkyTEJBMnZzVjJuQjkwbWpMMFVJ?=
 =?utf-8?B?Q2hMWklHRFRYOElFaTRJUWZCNjBXWkg2dGVlSW44NUdjeXFoOWd3L1B1T3J3?=
 =?utf-8?B?eUpxb0wxOEc2NTRrZlJ5cHdvZDdpL3llQjZPNVVoTTRQdkp5SUVsSkt2b0hD?=
 =?utf-8?B?ZEJGa1dqNWx0MkhoQU5GWDdFMkU1WnMrQkxWa1RSSG5SUU5RQ1NwZXJ3T3Bx?=
 =?utf-8?B?YnMzWjFoS3RVTHI1Sm9ZMHIzMXZQTUFDdVdZRk1NZ3k5VmVpd2xhWWdEeGFo?=
 =?utf-8?B?eS9WU2xma202SEpyMWd3em9HVjVBb3hrd0lBYkZsaEtpMlRMMlBmaFVVQjEy?=
 =?utf-8?B?OUpLaGZyVjNuUlF4SmluaXpBdC81aTBMRTBMWEo1ZjRSSGZkbkJnemg4dE12?=
 =?utf-8?B?UDBZbTVlbmVIRjlTYmcybGdldUhBcCtIOUx6QnN0S215Z1VoQTlyMnNETW9P?=
 =?utf-8?B?Z3crSlc4MzM0SWx5c29FQnU1dnVnVmZhTjFNRnRzMS8rbmRhSVhZcEoxNk1F?=
 =?utf-8?B?YVRZa1VSQjI2RWo0aGw1aGVvUU1ySWRpUW9lakRuRGF5Y3lINjF2ekRVR1pJ?=
 =?utf-8?Q?hF0Ah4j71utK3t9f8wyeTSde+?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 823068f0-c5f9-41d2-f7cc-08dc0c888428
X-MS-Exchange-CrossTenant-AuthSource: BL1PR12MB5874.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Jan 2024 18:19:25.3447
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: +LAv94oHvXnChqcvQY2fWGbTAGQcyKEWPN/aFZhDSZl13J25OJ9kHeYnyeK3o/oh
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CYXPR12MB9317


On 1/3/24 11:54 AM, Mathieu Poirier wrote:
> Hi Tanmay,
>
> On Fri, Dec 15, 2023 at 03:57:24PM -0800, Tanmay Shah wrote:
> > Add properties as per new bindings in zynqmp remoteproc node
> > to represent TCM address and size.
> > 
> > This patch also adds alternative remoteproc node to represent
> > remoteproc cluster in split mode. By default lockstep mode is
> > enabled and users should disable it before using split mode
> > dts. Both device-tree nodes can't be used simultaneously one
> > of them must be disabled. For zcu102-1.0 and zcu102-1.1 board
> > remoteproc split mode dts node is enabled and lockstep mode
> > dts is disabled.
> > 
> > Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
> > ---
> >  .../boot/dts/xilinx/zynqmp-zcu102-rev1.0.dts  |  8 +++
> >  arch/arm64/boot/dts/xilinx/zynqmp.dtsi        | 60 +++++++++++++++++--
> >  2 files changed, 63 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-rev1.0.dts b/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-rev1.0.dts
> > index c8f71a1aec89..495ca94b45db 100644
> > --- a/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-rev1.0.dts
> > +++ b/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-rev1.0.dts
> > @@ -14,6 +14,14 @@ / {
> >  	compatible = "xlnx,zynqmp-zcu102-rev1.0", "xlnx,zynqmp-zcu102", "xlnx,zynqmp";
> >  };
> >  
> > +&rproc_split {
> > +	status = "okay";
> > +};
> > +
> > +&rproc_lockstep {
> > +	status = "disabled";
> > +};
> > +
> >  &eeprom {
> >  	#address-cells = <1>;
> >  	#size-cells = <1>;
> > diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> > index b61fc99cd911..602e6aba7ac5 100644
> > --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> > +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> > @@ -247,19 +247,69 @@ fpga_full: fpga-full {
> >  		ranges;
> >  	};
> >  
> > -	remoteproc {
> > +	rproc_lockstep: remoteproc@ffe00000 {
> >  		compatible = "xlnx,zynqmp-r5fss";
> >  		xlnx,cluster-mode = <1>;
> >  
> > -		r5f-0 {
> > +		#address-cells = <2>;
> > +		#size-cells = <2>;
> > +
> > +		ranges = <0x0 0x0 0x0 0xffe00000 0x0 0x20000>,
> > +			 <0x0 0x20000 0x0 0xffe20000 0x0 0x20000>,
> > +			 <0x1 0x0 0x0 0xffe90000 0x0 0x10000>,
> > +			 <0x1 0x20000 0x0 0xffeb0000 0x0 0x10000>;
>
> As far as I can tell, in lockstep mode the last two entries are not needed.
> This is also contrasting with that is in the bindings, which is quite confusing.

Hi Mathieu,

Yes I agree. I think it should be same as of bindings example.

However, I will have to test and make sure no impact on driver. I will fix it in next revision.

Meanwhile, are you going to review driver changes as well or do you want to wait for next revision ?

Tanmay


> > +
> > +		r5f@0 {
> > +			compatible = "xlnx,zynqmp-r5f";
> > +			reg = <0x0 0x0 0x0 0x20000>, <0x0 0x20000 0x0 0x20000>;
> > +			reg-names = "atcm", "btcm";
> > +			power-domains = <&zynqmp_firmware PD_RPU_0>,
> > +					<&zynqmp_firmware PD_R5_0_ATCM>,
> > +					<&zynqmp_firmware PD_R5_0_BTCM>;
> > +			memory-region = <&rproc_0_fw_image>;
> > +		};
> > +
> > +		r5f@1 {
> > +			compatible = "xlnx,zynqmp-r5f";
> > +			reg = <0x1 0x0 0x0 0x10000>, <0x1 0x20000 0x0 0x10000>;
> > +			reg-names = "atcm", "btcm";
> > +			power-domains = <&zynqmp_firmware PD_RPU_1>,
> > +					<&zynqmp_firmware PD_R5_1_ATCM>,
> > +					<&zynqmp_firmware PD_R5_1_BTCM>;
> > +			memory-region = <&rproc_1_fw_image>;
> > +		};
> > +	};
> > +
> > +	rproc_split: remoteproc-split@ffe00000 {
> > +		status = "disabled";
> > +		compatible = "xlnx,zynqmp-r5fss";
> > +		xlnx,cluster-mode = <0>;
> > +
> > +		#address-cells = <2>;
> > +		#size-cells = <2>;
> > +
> > +		ranges = <0x0 0x0 0x0 0xffe00000 0x0 0x10000>,
> > +			 <0x0 0x20000 0x0 0xffe20000 0x0 0x10000>,
> > +			 <0x1 0x0 0x0 0xffe90000 0x0 0x10000>,
> > +			 <0x1 0x20000 0x0 0xffeb0000 0x0 0x10000>;
> > +
> > +		r5f@0 {
> >  			compatible = "xlnx,zynqmp-r5f";
> > -			power-domains = <&zynqmp_firmware PD_RPU_0>;
> > +			reg = <0x0 0x0 0x0 0x10000>, <0x0 0x20000 0x0 0x10000>;
> > +			reg-names = "atcm", "btcm";
> > +			power-domains = <&zynqmp_firmware PD_RPU_0>,
> > +					<&zynqmp_firmware PD_R5_0_ATCM>,
> > +					<&zynqmp_firmware PD_R5_0_BTCM>;
> >  			memory-region = <&rproc_0_fw_image>;
> >  		};
> >  
> > -		r5f-1 {
> > +		r5f@1 {
> >  			compatible = "xlnx,zynqmp-r5f";
> > -			power-domains = <&zynqmp_firmware PD_RPU_1>;
> > +			reg = <0x1 0x0 0x0 0x10000>, <0x1 0x20000 0x0 0x10000>;
> > +			reg-names = "atcm", "btcm";
> > +			power-domains = <&zynqmp_firmware PD_RPU_1>,
> > +					<&zynqmp_firmware PD_R5_1_ATCM>,
> > +					<&zynqmp_firmware PD_R5_1_BTCM>;
> >  			memory-region = <&rproc_1_fw_image>;
> >  		};
> >  	};
> > -- 
> > 2.25.1
> >