Received-SPF: pass (google.com: domain of linux-kernel+bounces-67323-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <09b0768a-4534-420f-9774-50e250795dd7@amd.com>
Date: Thu, 15 Feb 2024 10:45:21 -0600
User-Agent: Mozilla Thunderbird Beta
Subject: Re: [PATCH v10 2/4] dt-bindings: remoteproc: add Tightly Coupled
 Memory (TCM) bindings
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
 Rob Herring <robh@kernel.org>
Cc: andersson@kernel.org, robh+dt@kernel.org, devicetree@vger.kernel.org,
 krzysztof.kozlowski+dt@linaro.org, linux-remoteproc@vger.kernel.org,
 michal.simek@amd.com, Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>,
 ben.levinsky@amd.com, linux-kernel@vger.kernel.org,
 mathieu.poirier@linaro.org, conor+dt@kernel.org,
 linux-arm-kernel@lists.infradead.org
References: <20240213175450.3097308-1-tanmay.shah@amd.com>
 <20240213175450.3097308-3-tanmay.shah@amd.com>
 <170785205177.2155555.1311787541370066483.robh@kernel.org>
 <b931a24c-f676-4ddb-bb7c-e7a509d5dd4b@amd.com>
 <135e3154-2a55-40ac-9ba9-2de00833b903@linaro.org>
Content-Language: en-US
From: Tanmay Shah <tanmay.shah@amd.com>
In-Reply-To: <135e3154-2a55-40ac-9ba9-2de00833b903@linaro.org>
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?enBTc2pyb3dqcjVZL015MklvMHlFbDFrQTZjZmhDRlZtYmtSV1BLbUZPeXBq?=
 =?utf-8?B?clZ4bVh4NlVIcGQ4QU1XT0VaNXE5Z3p4OVFHRkpiK0FpM1dDZUQ3UDZ3b08v?=
 =?utf-8?B?Y3RtUlVhdE1aOENUZVBIREdUOEtUT0VGZnVVaWRNZVZGQlQxSFM5OVlHdytV?=
 =?utf-8?B?MEhRTWNWZDE1WFZoTWl2VGRNNUlJT3ZJNGE2VXJudW5NQmlJcDdVVThCbVNP?=
 =?utf-8?B?dlM3QWhka2VTbkI5ZHUyYnYrQ3RTY1ZDSUk4UE5GNEwyNTZueWF3Rm5yMk5i?=
 =?utf-8?B?dGJsd2tPNTlXdm9DTzIvZ2s1OUdFdGRUV2NFaHRWTEwwY21vdGlGR1BPeU5S?=
 =?utf-8?B?V054cEg1bVFsdy82QkowQlBpLy9VRHNkSlBHK1hyYWlIN2hqQ1NwRmRFUWIz?=
 =?utf-8?B?S1RubjhvRTcyazRFa2NmbmdZdDdXVmJOTWh2UU5EeWozOXNFYVBaRm5Ja01K?=
 =?utf-8?B?bkxUeU1waWgvQ1JzOVNUWStCZHRTVjJWTDdVaktzYWhjeGpiMXBNbThPVE4z?=
 =?utf-8?B?b0VpcXNoZ1hXRzBlSWlRM05jclRHSjlUazV6N3g5d1NSWHdPMVhtWEUyaGFu?=
 =?utf-8?B?RWhXWkZKNzRraFZGcWxUMk1pRitRREdNZTcybE9mOUlCbVRsa1Y0SVNBdkxZ?=
 =?utf-8?B?SDBwSGdxQkhlMUxORytDa0x3bkNhaG9nUSsyZkU5UHEwVXhpR0hoZHY2amxC?=
 =?utf-8?B?d0ZYQm5SaWN2NVVxdEQxaWg4cWlIZUZ6a3RrSlg2Y2wzbDUzbjNHS2ZjMjgv?=
 =?utf-8?B?OTR5b3ovbEhLMnppUUtsdDhtVEVGS3I3OWkyS3Z2U0xudXlrQU9sMERGNmVP?=
 =?utf-8?B?bDJtV1g0M3ZhSGdDb3IwbFI4RGdrSUY0eis4MVZlRi9JNGx2ZWgwWUQ0RW1w?=
 =?utf-8?B?bExRMmN2UTNkbHd2R21hQzZzVjkrMmZKNHZnNkc1Yy96ZUROQzRPZEJlcE1C?=
 =?utf-8?B?MGxDL3J2b2dJWXVrVnVkUTB1d2hyaHo3QjMrbitmN0MvcTMza0RTSmd1TEl3?=
 =?utf-8?B?dGFGbmw4Vm9ZSnlETmhlcXhZbFNtM2FEV3NnK05wSW9oU2x1WFhsU0NTYlFD?=
 =?utf-8?B?NzJqWlBrdFdrQWtnRjl4WENnUEFsNkNBekt4c0Q4SmtVcTJ2em81cTVNV2ZF?=
 =?utf-8?B?UHJzbVR5cWwzNHNBUGZraTBWcmVScUV3eVdob283czVnYktUSFJMNzV2YnNq?=
 =?utf-8?B?VEdiMmtDS09IU1JSR2dpeSsweU5ZWHNRY2llZERqVnNQWnd0K0NKYmcxN3kr?=
 =?utf-8?B?ZUwvb0ljTVhiMUhzMFNVOS9oTHVJRFNOVHBoNnJFTW5zZkVvMUlzV2UxUzRY?=
 =?utf-8?B?RldPL1JJSkozL25MR2FlN3RiaUpWVnl3VDVGaXBKcityM2l6RnpDNVMyNHEr?=
 =?utf-8?B?VGtTUnRoemxxbFcxeU81THR4MXJIc2xXQUtycHRQQnpqUkk2NHlSUlVrT0Yy?=
 =?utf-8?B?UkV6UTVHaHVCTmJPVm9QMHdZSUZURU5JZnpGemJYZWpreU10R0ZnOW5kaTNU?=
 =?utf-8?B?RFF5ZFFKL3RFNkNVaGJHYkRuOXptdUU3Wnk5eVhlRGp0bEk5OEh5aEJrS2pH?=
 =?utf-8?B?SEZMOHRqbWVxVDliRitoRHBNb0toYmYwY0VZeDZGVXpIZGFta0t3OFFEWm1C?=
 =?utf-8?B?VFQ5Ti9UWmZreWl0eUZCYWRVVVpoRXlSVkR0RGU4UU5KUndFbHRYZ2V2T2o0?=
 =?utf-8?B?TzBVRWxLcFVibVVQVjVZWVBUYnQzM081UWtNN2hWVXI1em91a3R3QUpERHR6?=
 =?utf-8?B?c1NSL0k2MDZGaDVGbVdEZ3hmZmtNZGoxWWFONDYvQ3VyL1NCMnJhbWVHZHlv?=
 =?utf-8?B?NHNlZjJLc3BTQThEaXQwNFVQcFp4NnV4WXBLS3BDS1lKQS9kY1JxNytFWkE1?=
 =?utf-8?B?WG5mRTcxV0lyQk0rTVdhMVlncGx6QTRDS04yZDQva2h0V0ErTmUzNHVWMVVF?=
 =?utf-8?B?aks4WlJkUUVCK2V3WFIvejg4YlQway85QlhtTlVHbzVjOXNpUXN5bmJhOXA2?=
 =?utf-8?B?OCt2dkF2OUM4MVNwWVlEKzAzUjNpc0pEZ2llQTBIQ0J3UFhMVWptN2ZuNFlB?=
 =?utf-8?B?YU4zNU5rSGhSeUV5T3Y5eFlMN0ZLUjJFQ0drUnRwQm9GMFFhRk5ZdS91RW9E?=
 =?utf-8?Q?3OT8If1lmzmFJ+SmokIbG+pen?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c9c20ad0-4609-47ac-4c6f-08dc2e45819b
X-MS-Exchange-CrossTenant-AuthSource: BL1PR12MB5874.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Feb 2024 16:45:24.3205
 (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: 22jkE02zne0OPnzasJNbD1zx8/wsUKmZErSrIBcD+3WiTn3rYbGE+z3sukmXxX3T
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CYYPR12MB8870


On 2/15/24 3:06 AM, Krzysztof Kozlowski wrote:
> On 13/02/2024 21:37, Tanmay Shah wrote:
> > Hello,
> > 
> > Thanks for reviews please find my comments below.
> > 
> > On 2/13/24 1:20 PM, Rob Herring wrote:
> >> On Tue, 13 Feb 2024 09:54:48 -0800, Tanmay Shah wrote:
> >>> From: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> >>>
> >>> Introduce bindings for TCM memory address space on AMD-xilinx Zynq
> >>> UltraScale+ platform. It will help in defining TCM in device-tree
> >>> and make it's access platform agnostic and data-driven.
> >>>
> >>> Tightly-coupled memories(TCMs) are low-latency memory that provides
> >>> predictable instruction execution and predictable data load/store
> >>> timing. Each Cortex-R5F processor contains two 64-bit wide 64 KB memory
> >>> banks on the ATCM and BTCM ports, for a total of 128 KB of memory.
> >>>
> >>> The TCM resources(reg, reg-names and power-domain) are documented for
> >>> each TCM in the R5 node. The reg and reg-names are made as required
> >>> properties as we don't want to hardcode TCM addresses for future
> >>> platforms and for zu+ legacy implementation will ensure that the
> >>> old dts w/o reg/reg-names works and stable ABI is maintained.
> >>>
> >>> It also extends the examples for TCM split and lockstep modes.
> >>>
> >>> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> >>> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
> >>> ---
> >>>
> >>> Changes in v10:
> >>>   - modify number of "reg", "reg-names" and "power-domains" entries
> >>>     based on cluster mode
> >>>   - Add extra optional atcm and btcm in "reg" property for lockstep mode
> >>>   - Add "reg-names" for extra optional atcm and btcm for lockstep mode
> >>>   - Drop previous Ack as bindings has new change
> >>>
> >>> Changes in v9:
> >>>   - None
> >>> Changes in v8:
> >>>   - None
> >>> Changes in v7:
> >>>   - None
> >>> Changes in v6:
> >>>   - None
> >>> Changes in v5:
> >>>   - None
> >>>
> >>> Changes in v4:
> >>>   - Use address-cells and size-cells value 2
> >>>   - Modify ranges property as per new value of address-cells
> >>>     and size-cells
> >>>   - Modify child node "reg" property accordingly
> >>>   - Remove previous ack for further review
> >>>
> >>> v4 link: https://lore.kernel.org/all/20230829181900.2561194-2-tanmay.shah@amd.com/
> >>>
> >>>  .../remoteproc/xlnx,zynqmp-r5fss.yaml         | 192 ++++++++++++++++--
> >>>  1 file changed, 170 insertions(+), 22 deletions(-)
> >>>
> >>
> >> 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:
> >> ./Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml:118:13: [warning] wrong indentation: expected 10 but found 12 (indentation)
> > Ack. I will fix this.
> > 
> > However, can I still get reviews on patch itself so if something else needs to be fixed I can fix in next revision as well.
>
> Sorry, I have too many patches to review to provide feedback on work
> which does not build/compile/test. First use automated tooling, like
> building a C code, to detect as many issues as possible then ask for
> reviewing. Not the other way around.

Ack. I will send new revision fixing the warning.

Thanks.


>
> Best regards,
> Krzysztof