[alsa-devel] [PATCH] topology: Include <linux/types.h> in API header file
From: Mengdong Lin mengdong.lin@linux.intel.com
Include <linux/types.h> in the API header file topology.h
The struct snd_tplg_stream_caps_template has a member "formats" of type __le64. So <linux/types.h> is needed.
Signed-off-by: Mengdong Lin mengdong.lin@linux.intel.com
diff --git a/include/topology.h b/include/topology.h index b631871..5ee67e2 100644 --- a/include/topology.h +++ b/include/topology.h @@ -21,6 +21,8 @@ #ifndef __ALSA_TOPOLOGY_H #define __ALSA_TOPOLOGY_H
+#include <linux/types.h> + #ifdef __cplusplus extern "C" { #endif
On Fri, 06 Nov 2015 13:39:36 +0100, mengdong.lin@linux.intel.com wrote:
From: Mengdong Lin mengdong.lin@linux.intel.com
Include <linux/types.h> in the API header file topology.h
The struct snd_tplg_stream_caps_template has a member "formats" of type __le64. So <linux/types.h> is needed.
We shouldn't use __le64 for user-space API. I couldn't find it in include/topology.h. Where is it used?
Of course, it's fine to use it for kernel ABI. But then linux/types.h is already included in local.h, so it shouldn't be a problem.
Takashi
Signed-off-by: Mengdong Lin mengdong.lin@linux.intel.com
diff --git a/include/topology.h b/include/topology.h index b631871..5ee67e2 100644 --- a/include/topology.h +++ b/include/topology.h @@ -21,6 +21,8 @@ #ifndef __ALSA_TOPOLOGY_H #define __ALSA_TOPOLOGY_H
+#include <linux/types.h>
#ifdef __cplusplus extern "C" {
#endif
1.9.1
On Fri, Nov 06, 2015 at 01:59:41PM +0100, Takashi Iwai wrote:
On Fri, 06 Nov 2015 13:39:36 +0100, mengdong.lin@linux.intel.com wrote:
From: Mengdong Lin mengdong.lin@linux.intel.com
Include <linux/types.h> in the API header file topology.h
The struct snd_tplg_stream_caps_template has a member "formats" of type __le64. So <linux/types.h> is needed.
We shouldn't use __le64 for user-space API. I couldn't find it in include/topology.h. Where is it used?
Of course, it's fine to use it for kernel ABI. But then linux/types.h is already included in local.h, so it shouldn't be a problem.
Our tool threw this error
./local-alsa/include/alsa/topology.h:668:2: error: â__le64â does not name a type __le64 formats; /*!< supported formats SNDRV_PCM_FMTBIT_* */
It is coming from new ABI update
/* * Stream Capabilities */ struct snd_soc_tplg_stream_caps { __le32 size; /* in bytes of this structure */ char name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN]; __le64 formats; /* supported formats SNDRV_PCM_FMTBIT_* */ ^^^^^^^^^^^^^^^^^^^^^
__le32 rates; /* supported rates SNDRV_PCM_RATE_* */ __le32 rate_min; /* min rate */ __le32 rate_max; /* max rate */ __le32 channels_min; /* min channels */ __le32 channels_max; /* max channels */ __le32 periods_min; /* min number of periods */ __le32 periods_max; /* max number of periods */ __le32 period_size_min; /* min period size bytes */ __le32 period_size_max; /* max period size bytes */ __le32 buffer_size_min; /* min buffer size bytes */ __le32 buffer_size_max; /* max buffer size bytes */ } __attribute__((packed));
The problem is the users of alsa-lib and not the alsa-lib compile itself..
Thanks
On Fri, 06 Nov 2015 14:31:18 +0100, Vinod Koul wrote:
On Fri, Nov 06, 2015 at 01:59:41PM +0100, Takashi Iwai wrote:
On Fri, 06 Nov 2015 13:39:36 +0100, mengdong.lin@linux.intel.com wrote:
From: Mengdong Lin mengdong.lin@linux.intel.com
Include <linux/types.h> in the API header file topology.h
The struct snd_tplg_stream_caps_template has a member "formats" of type __le64. So <linux/types.h> is needed.
We shouldn't use __le64 for user-space API. I couldn't find it in include/topology.h. Where is it used?
Of course, it's fine to use it for kernel ABI. But then linux/types.h is already included in local.h, so it shouldn't be a problem.
Our tool threw this error
./local-alsa/include/alsa/topology.h:668:2: error: â__le64â does not name a type __le64 formats; /*!< supported formats SNDRV_PCM_FMTBIT_* */
It is coming from new ABI update
/*
- Stream Capabilities
*/ struct snd_soc_tplg_stream_caps { __le32 size; /* in bytes of this structure */ char name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN]; __le64 formats; /* supported formats SNDRV_PCM_FMTBIT_* */ ^^^^^^^^^^^^^^^^^^^^^
__le32 rates; /* supported rates SNDRV_PCM_RATE_* */ __le32 rate_min; /* min rate */ __le32 rate_max; /* max rate */ __le32 channels_min; /* min channels */ __le32 channels_max; /* max channels */ __le32 periods_min; /* min number of periods */ __le32 periods_max; /* max number of periods */ __le32 period_size_min; /* min period size bytes */ __le32 period_size_max; /* max period size bytes */ __le32 buffer_size_min; /* min buffer size bytes */ __le32 buffer_size_max; /* max buffer size bytes */} __attribute__((packed));
The problem is the users of alsa-lib and not the alsa-lib compile itself..
The structure above shouldn't be exposed to user-space as is. If it's a part of alsa-lib API, it must be generic.
Takashi
On Fri, 2015-11-06 at 14:33 +0100, Takashi Iwai wrote:
On Fri, 06 Nov 2015 14:31:18 +0100, Vinod Koul wrote:
On Fri, Nov 06, 2015 at 01:59:41PM +0100, Takashi Iwai wrote:
On Fri, 06 Nov 2015 13:39:36 +0100, mengdong.lin@linux.intel.com wrote:
From: Mengdong Lin mengdong.lin@linux.intel.com
Include <linux/types.h> in the API header file topology.h
The struct snd_tplg_stream_caps_template has a member "formats" of type __le64. So <linux/types.h> is needed.
We shouldn't use __le64 for user-space API. I couldn't find it in include/topology.h. Where is it used?
Of course, it's fine to use it for kernel ABI. But then linux/types.h is already included in local.h, so it shouldn't be a problem.
Our tool threw this error
./local-alsa/include/alsa/topology.h:668:2: error: â__le64â does not name a type __le64 formats; /*!< supported formats SNDRV_PCM_FMTBIT_* */
Can you try the following patch for the userspace header.
From 568f769269122e90fceee605a98a1a526db2b3d4 Mon Sep 17 00:00:00 2001
From: Liam Girdwood liam.r.girdwood@linux.intel.com Date: Fri, 6 Nov 2015 14:30:36 +0000 Subject: [PATCH] topology: remove little endian type from userspace header.
Use a generic unsigned long long for formats instead of an ABI endian specific __le64 type.
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com --- include/topology.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/topology.h b/include/topology.h index b631871..1a110a2 100644 --- a/include/topology.h +++ b/include/topology.h @@ -665,7 +665,7 @@ struct snd_tplg_stream_template { */ struct snd_tplg_stream_caps_template { const char *name; /*!< name of the stream caps */ - __le64 formats; /*!< supported formats SNDRV_PCM_FMTBIT_* */ + unsigned long long formats; /*!< supported formats SNDRV_PCM_FMTBIT_* */ unsigned int rates; /*!< supported rates SNDRV_PCM_RATE_* */ unsigned int rate_min; /*!< min rate */ unsigned int rate_max; /*!< max rate */
On Fri, 06 Nov 2015 15:37:27 +0100, Liam Girdwood wrote:
On Fri, 2015-11-06 at 14:33 +0100, Takashi Iwai wrote:
On Fri, 06 Nov 2015 14:31:18 +0100, Vinod Koul wrote:
On Fri, Nov 06, 2015 at 01:59:41PM +0100, Takashi Iwai wrote:
On Fri, 06 Nov 2015 13:39:36 +0100, mengdong.lin@linux.intel.com wrote:
From: Mengdong Lin mengdong.lin@linux.intel.com
Include <linux/types.h> in the API header file topology.h
The struct snd_tplg_stream_caps_template has a member "formats" of type __le64. So <linux/types.h> is needed.
We shouldn't use __le64 for user-space API. I couldn't find it in include/topology.h. Where is it used?
Of course, it's fine to use it for kernel ABI. But then linux/types.h is already included in local.h, so it shouldn't be a problem.
Our tool threw this error
./local-alsa/include/alsa/topology.h:668:2: error: â__le64â does not name a type __le64 formats; /*!< supported formats SNDRV_PCM_FMTBIT_* */
Can you try the following patch for the userspace header.
From 568f769269122e90fceee605a98a1a526db2b3d4 Mon Sep 17 00:00:00 2001
From: Liam Girdwood liam.r.girdwood@linux.intel.com Date: Fri, 6 Nov 2015 14:30:36 +0000 Subject: [PATCH] topology: remove little endian type from userspace header.
Use a generic unsigned long long for formats instead of an ABI endian specific __le64 type.
Or use uint64_t instead. This is the standard definition of 64bit integer. I thought we already include stdint.h somewhere, but you can included it there again to be sure, too.
Takashi
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com
include/topology.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/topology.h b/include/topology.h index b631871..1a110a2 100644 --- a/include/topology.h +++ b/include/topology.h @@ -665,7 +665,7 @@ struct snd_tplg_stream_template { */ struct snd_tplg_stream_caps_template { const char *name; /*!< name of the stream caps */
- __le64 formats; /*!< supported formats SNDRV_PCM_FMTBIT_* */
- unsigned long long formats; /*!< supported formats SNDRV_PCM_FMTBIT_* */ unsigned int rates; /*!< supported rates SNDRV_PCM_RATE_* */ unsigned int rate_min; /*!< min rate */ unsigned int rate_max; /*!< max rate */
-- 2.5.0
On Fri, 2015-11-06 at 15:40 +0100, Takashi Iwai wrote:
On Fri, 06 Nov 2015 15:37:27 +0100, Liam Girdwood wrote:
From 568f769269122e90fceee605a98a1a526db2b3d4 Mon Sep 17 00:00:00 2001
From: Liam Girdwood liam.r.girdwood@linux.intel.com Date: Fri, 6 Nov 2015 14:30:36 +0000 Subject: [PATCH] topology: remove little endian type from userspace header.
Use a generic unsigned long long for formats instead of an ABI endian specific __le64 type.
Or use uint64_t instead. This is the standard definition of 64bit integer. I thought we already include stdint.h somewhere, but you can included it there again to be sure, too.
I didn't see stdint.h so used unsigned long long on the first patch. The patch below fixes this though :-
From 52d3023e482ac856692bf27819d97d9a037a055b Mon Sep 17 00:00:00 2001
From: Liam Girdwood liam.r.girdwood@linux.intel.com Date: Fri, 6 Nov 2015 14:30:36 +0000 Subject: [PATCH] topology: remove little endian type from userspace header.
Use a generic uint64_t for formats instead of an ABI endian specific __le64 type.
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com --- include/topology.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/topology.h b/include/topology.h index b631871..993537c 100644 --- a/include/topology.h +++ b/include/topology.h @@ -21,6 +21,8 @@ #ifndef __ALSA_TOPOLOGY_H #define __ALSA_TOPOLOGY_H
+#include <stdint.h> + #ifdef __cplusplus extern "C" { #endif @@ -665,7 +667,7 @@ struct snd_tplg_stream_template { */ struct snd_tplg_stream_caps_template { const char *name; /*!< name of the stream caps */ - __le64 formats; /*!< supported formats SNDRV_PCM_FMTBIT_* */ + uint64_t formats; /*!< supported formats SNDRV_PCM_FMTBIT_* */ unsigned int rates; /*!< supported rates SNDRV_PCM_RATE_* */ unsigned int rate_min; /*!< min rate */ unsigned int rate_max; /*!< max rate */
On Fri, Nov 06, 2015 at 02:57:17PM +0000, Liam Girdwood wrote:
On Fri, 2015-11-06 at 15:40 +0100, Takashi Iwai wrote:
On Fri, 06 Nov 2015 15:37:27 +0100, Liam Girdwood wrote:
From 568f769269122e90fceee605a98a1a526db2b3d4 Mon Sep 17 00:00:00 2001
From: Liam Girdwood liam.r.girdwood@linux.intel.com Date: Fri, 6 Nov 2015 14:30:36 +0000 Subject: [PATCH] topology: remove little endian type from userspace header.
Use a generic unsigned long long for formats instead of an ABI endian specific __le64 type.
Or use uint64_t instead. This is the standard definition of 64bit integer. I thought we already include stdint.h somewhere, but you can included it there again to be sure, too.
I didn't see stdint.h so used unsigned long long on the first patch. The patch below fixes this though :-
From 52d3023e482ac856692bf27819d97d9a037a055b Mon Sep 17 00:00:00 2001 From: Liam Girdwood liam.r.girdwood@linux.intel.com Date: Fri, 6 Nov 2015 14:30:36 +0000 Subject: [PATCH] topology: remove little endian type from userspace header.
Use a generic uint64_t for formats instead of an ABI endian specific __le64 type.
Tested and it works fine.
Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com
include/topology.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/topology.h b/include/topology.h index b631871..993537c 100644 --- a/include/topology.h +++ b/include/topology.h @@ -21,6 +21,8 @@ #ifndef __ALSA_TOPOLOGY_H #define __ALSA_TOPOLOGY_H
+#include <stdint.h>
#ifdef __cplusplus extern "C" { #endif @@ -665,7 +667,7 @@ struct snd_tplg_stream_template { */ struct snd_tplg_stream_caps_template { const char *name; /*!< name of the stream caps */
- __le64 formats; /*!< supported formats SNDRV_PCM_FMTBIT_* */
- uint64_t formats; /*!< supported formats SNDRV_PCM_FMTBIT_* */ unsigned int rates; /*!< supported rates SNDRV_PCM_RATE_* */ unsigned int rate_min; /*!< min rate */ unsigned int rate_max; /*!< max rate */
-- 2.5.0
--
On Mon, Nov 09, 2015 at 07:34:29PM +0530, Subhransu S. Prusty wrote:
On Fri, Nov 06, 2015 at 02:57:17PM +0000, Liam Girdwood wrote:
On Fri, 2015-11-06 at 15:40 +0100, Takashi Iwai wrote:
On Fri, 06 Nov 2015 15:37:27 +0100, Liam Girdwood wrote:
From 568f769269122e90fceee605a98a1a526db2b3d4 Mon Sep 17 00:00:00 2001
From: Liam Girdwood liam.r.girdwood@linux.intel.com Date: Fri, 6 Nov 2015 14:30:36 +0000 Subject: [PATCH] topology: remove little endian type from userspace header.
Use a generic unsigned long long for formats instead of an ABI endian specific __le64 type.
Or use uint64_t instead. This is the standard definition of 64bit integer. I thought we already include stdint.h somewhere, but you can included it there again to be sure, too.
I didn't see stdint.h so used unsigned long long on the first patch. The patch below fixes this though :-
From 52d3023e482ac856692bf27819d97d9a037a055b Mon Sep 17 00:00:00 2001 From: Liam Girdwood liam.r.girdwood@linux.intel.com Date: Fri, 6 Nov 2015 14:30:36 +0000 Subject: [PATCH] topology: remove little endian type from userspace header.
Use a generic uint64_t for formats instead of an ABI endian specific __le64 type.
Tested and it works fine.
Hi Takashi,
Is this fix okay, was this applied ?
Dne 10.11.2015 v 13:29 Vinod Koul napsal(a):
On Mon, Nov 09, 2015 at 07:34:29PM +0530, Subhransu S. Prusty wrote:
On Fri, Nov 06, 2015 at 02:57:17PM +0000, Liam Girdwood wrote:
On Fri, 2015-11-06 at 15:40 +0100, Takashi Iwai wrote:
On Fri, 06 Nov 2015 15:37:27 +0100, Liam Girdwood wrote:
From 568f769269122e90fceee605a98a1a526db2b3d4 Mon Sep 17 00:00:00 2001
From: Liam Girdwood liam.r.girdwood@linux.intel.com Date: Fri, 6 Nov 2015 14:30:36 +0000 Subject: [PATCH] topology: remove little endian type from userspace header.
Use a generic unsigned long long for formats instead of an ABI endian specific __le64 type.
Or use uint64_t instead. This is the standard definition of 64bit integer. I thought we already include stdint.h somewhere, but you can included it there again to be sure, too.
I didn't see stdint.h so used unsigned long long on the first patch. The patch below fixes this though :-
From 52d3023e482ac856692bf27819d97d9a037a055b Mon Sep 17 00:00:00 2001 From: Liam Girdwood liam.r.girdwood@linux.intel.com Date: Fri, 6 Nov 2015 14:30:36 +0000 Subject: [PATCH] topology: remove little endian type from userspace header.
Use a generic uint64_t for formats instead of an ABI endian specific __le64 type.
Tested and it works fine.
Hi Takashi,
Is this fix okay, was this applied ?
Yes, it's in alsa-lib-1.1.0, too:
http://git.alsa-project.org/?p=alsa-lib.git;a=summary
Jaroslav
participants (6)
-
Jaroslav Kysela -
Liam Girdwood -
mengdong.lin＠linux.intel.com -
Subhransu S. Prusty -
Takashi Iwai -
Vinod Koul