Re: [PATCH 2/2] firmware: arm_scmi: Add qcom hvc/shmem transport

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 7/18/2023 12:30 PM, Bjorn Andersson wrote:
On Tue, Jul 18, 2023 at 12:10:16PM -0700, Nikunj Kela wrote:
On 7/18/2023 12:07 PM, Bjorn Andersson wrote:
On Tue, Jul 18, 2023 at 11:53:24AM -0700, Nikunj Kela wrote:
On 7/18/2023 11:29 AM, Bjorn Andersson wrote:
On Tue, Jul 18, 2023 at 09:08:33AM -0700, Nikunj Kela wrote:
diff --git a/drivers/firmware/arm_scmi/qcom_hvc.c b/drivers/firmware/arm_scmi/qcom_hvc.c
[..]
[..]
+#ifdef CONFIG_ARM64
+	cap_id = readq((void *)(scmi_info->shmem) + size +
+		       sizeof(unsigned long));
+#else
+	cap_id = readl((void *)(scmi_info->shmem) + size +
+		       sizeof(unsigned long));
+#endif
Please don't make the in-memory representation depend on architecture
specific data types. Quite likely you didn't compile test one of these
variants?

Just define the in-memory representation as u32 + u64.
I tested this for ARM64, I didn't test it for 32bit since Hypervisor doesn't
support it currently. In future, it may add 32 bit support too.
I'd not be surprised if the capability id is 64 bit on a 32-bit machine
as well, it's not really a property of the architecture.
on 32bit machine, you will have to use SMC32 convention. lt will mean that
the parameters can only be 32 bit long. If you keep cap-id 64 bit in 32bit
machines, then it has to be passed in two parameters. Are you suggesting, I
make this driver dependent on ARM64 and only care about 64 bit for now?

I'm suggesting that the calling convention is one thing and the
in-memory format for passing the information to the driver is a
different thing.

Keep the arguments passed in memory architecture-independent (i.e. make
it u64).

If you're saying that the calling convention would be different on a
32-bit system, then you are also saying that your driver _is_ 64-bit
specific. Please confirm what the size of your capability id would be in
such a system and make sure the Kconfig and/or the code, reflects
reality.

Thanks,
Bjorn
It is confirmed that cap-id will be 32 bit long on 32bit machines. That said, I will make changes to use last 16bytes irrespective of the architecture.



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux