Re: [alsa-devel] [PATCH] ASoC: SOF: use __u32 instead of uint32_t in uapi headers

From: Pierre-Louis Bossart
Date: Mon Jul 22 2019 - 11:22:08 EST




On 7/22/19 8:34 AM, Arnd Bergmann wrote:
On Mon, Jul 22, 2019 at 3:16 PM Pierre-Louis Bossart
<pierre-louis.bossart@xxxxxxxxxxxxxxx> wrote:
On 7/22/19 7:56 AM, Takashi Iwai wrote:
On Mon, 22 Jul 2019 14:49:34 +0200,
Pierre-Louis Bossart wrote:



On 7/21/19 9:23 AM, Masahiro Yamada wrote:
When CONFIG_UAPI_HEADER_TEST=y, exported headers are compile-tested to
make sure they can be included from user-space.

Currently, header.h and fw.h are excluded from the test coverage.
To make them join the compile-test, we need to fix the build errors
attached below.

For a case like this, we decided to use __u{8,16,32,64} variable types
in this discussion:

https://lkml.org/lkml/2019/6/5/18

these files are shared with the SOF project and used as is (with minor
formatting) for the firmware compilation. I am not sure I understand
the ask here, are you really asking SOF to use linux-specific type
definitions?

Actually this is linux-kernel UAPI header files, so yes, we should
follow the convention there as much as possible.

So far we haven't been strict about these types. But now we have a
unit test for checking it, so it's a good opportunity to address the
issues.

Maybe a bit of background. For SOF we split the includes in 4 directories

https://github.com/thesofproject/sof/tree/master/src/include

- sof: internal includes for firmware only
- ipc: definitions of the structures for information exchanged over the
IPC channel. This directory is used as is by the Linux kernel and
mirrored in include/sound/sof
- user: definitions needed for firmware tools, e.g. to generate the
image or parse the trace. this directory is not used by the Linux kernel.
- kernel: definitions for the firmware format, needed for the loader to
parse the firmware files. This is not directly used by applications
running on the target, it really defines the content passed to the
kernel with request_firmware. This directory is mirrored in the Linux
include/uapi/sound/sof directory.

Our goal is to minimize the differences and allow deltas e.g. for
license or comments. We could add a definition for __u32 when linux is
not used, I am just not sure if these two files really fall in the UAPI
category and if the checks make sense.

If you can build all the SOF user space without these headers being
installed to /usr/include/sound/sof/, you can move them from
include/uapi/sound/sof to include/sounds/sof and leave the types
unchanged.

yes we don't need those files to build userspace stuff. The idea was that these format definitions establish a contract between userspace (more specifically the files stored in /lib/firmware) and the kernel. IIRC we wanted to make sure that any changes would be tracked as breaking userspace. If the consensus is that the uapi directory is strictly used for builds then we should indeed move those files