On 03/23/2015 11:07 AM, Joakim Bech wrote:
On Fri, Mar 20, 2015 at 01:44:36PM +0100, Jens Wiklander wrote:
On Fri, Mar 20, 2015 at 12:00:21PM +0100, Pascal Brand wrote:
Hello Jens,
On 19 March 2015 at 14:48, Jérôme Forissier jerome.forissier@linaro.org wrote:
Hi Jens,
On 03/19/2015 11:23 AM, Jens Wiklander wrote:
Signed-off-by: Jens Wiklander jens.wiklander@linaro.org
[...]
- Identifies the generic TEE driver, and the specific TEE driver.
- */
+struct tee_version {
uint32_t gen_version;What about uint32_t vs __u32 an so on in things shared with user space? See Documentation/CodingStyle (and an interesting discussion here: http://yarchive.net/comp/linux/kernel_headers.html)
I also vote for __u32 etc. everywhere, as it is also recommended in http://www.kernel.org/doc/Documentation/ioctl/botching-up-ioctls.txt.
[...]
+/**
- struct tee_mem_share_data - share memory with Secure OS
- @buf: share user space memory
- @dma_buf: share foreign dma_buf memory
- @flags: Flags to/from sharing, unused bits set to zero by caller
- @pad: Padding, set to zero by caller
- If TEE_MEM_SHARE_FLAG_FOREIGN_BUFFER is set use the dma_buf field,
else
- the buf field in the union.
- */
+struct tee_mem_share_data {
union {struct tee_mem_buf buf;struct tee_mem_dma_buf dma_buf;};uint32_t flags;uint32_t pad;+};
Yet another Shared Memory structure ;) How could we make more explicit the name "tee_mem_share_data" (YASM will not be accepted ;) )
Perhaps we should tie the different structs close to the IOCTL defines in this file. These structs are only used to pass parameter to the ioctl in questsion. The subsystem will use different types internally.
I think that is a good idea. Another thing, I've touched it before, do we actually need to have the word "mem" everywhere? tee_shm (which clashes with name in the other h-file, so that exact name wouldn't work here), tee_buf and tee_dma_buf would be sufficient according to me.
+1 for tee_buf and tee_dma_buf but I think I would keep tee_mem_share_data (it seems the meaning is less obvious without it).