Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753789AbdHQVLs (ORCPT ); Thu, 17 Aug 2017 17:11:48 -0400 Received: from mail-oi0-f68.google.com ([209.85.218.68]:36652 "EHLO mail-oi0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753597AbdHQVLq (ORCPT ); Thu, 17 Aug 2017 17:11:46 -0400 MIME-Version: 1.0 In-Reply-To: <5a53e449-b269-89da-9b69-e560132502fc@xilinx.com> References: <98038734d73c604dee6ac0d34740d5bc2034e87d.1501854302.git.michal.simek@xilinx.com> <247a5660-6c53-ccc3-7b9f-bfd2a11f6f54@xilinx.com> <3809b6a0-1cb9-37af-a524-8bc1986268ec@xilinx.com> <5a53e449-b269-89da-9b69-e560132502fc@xilinx.com> From: Arnd Bergmann Date: Thu, 17 Aug 2017 23:11:45 +0200 X-Google-Sender-Auth: 2q0yi3AuqqFQVtCWgCutmyHJeec Message-ID: Subject: Re: [PATCH 3/3] soc: xilinx: zynqmp: Add firmware interface To: Michal Simek Cc: Linux ARM , =?UTF-8?Q?S=C3=B6ren_Brinkmann?= , Lucas Stach , Michal Simek , yangbo lu , =?UTF-8?Q?Andreas_F=C3=A4rber?= , Linux Kernel Mailing List , Alexandre Belloni , Baoyou Xie , Shawn Guo , Geert Uytterhoeven , Nicolas Ferre , Simon Horman Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1465 Lines: 39 On Thu, Aug 17, 2017 at 12:48 PM, Michal Simek wrote: > On 16.8.2017 17:05, Arnd Bergmann wrote: >> On Wed, Aug 16, 2017 at 4:34 PM, Michal Simek wrote: >> >> Looks good, just make sure you also check with sparse (make C=1) >> to ensure you have the right __le64/__le32 types everywhere. > > Are you aware about any doc where it is written that data should be > passed as little endian? Looking at http://infocenter.arm.com/help/topic/com.arm.doc.den0028b/ARM_DEN0028B_SMC_Calling_Convention.pdf now, I think that the structure above is endian-neutral as the arguments get passed in registers rather than memory. However, if you pass pointers to data structures in memory, those data structures would have to be defined with __le32/__le64 types. > I was playing with it a little bit and this means that these 2(3 with > hvc) needs to be changed. > > asmlinkage void __arm_smccc_smc(__le64 a0, __le64 a1, __le64 a2, > __le64 a3,__le64 a4, __le64 a5, __le64 a6, __le64 a7, > struct arm_smccc_res *res, struct arm_smccc_quirk *quirk); > > struct arm_smccc_res { > - unsigned long a0; > - unsigned long a1; > - unsigned long a2; > - unsigned long a3; > + __le64 a0; > + __le64 a1; > + __le64 a2; > + __le64 a3; > }; This is clearly wrong on 32-bit machines, I think this is intentionally defined as 'unsigned long' to have register sized arguments. Arnd