Re: [alsa-devel] [PATCH - [hdspm] RayDAT/AIO 1/2] ALSA: Add support for RME RayDAT and AIO
At Wed, 26 Jan 2011 19:32:14 +0100, Adrian Knoth wrote:
diff --git a/include/sound/hdspm.h b/include/sound/hdspm.h index 81990b2..c3f1819 100644 --- a/include/sound/hdspm.h +++ b/include/sound/hdspm.h @@ -131,4 +225,175 @@ typedef struct hdspm_version hdspm_version_t; typedef struct hdspm_channelfader snd_hdspm_channelfader_t; typedef struct hdspm_mixer hdspm_mixer_t;
-#endif /* __SOUND_HDSPM_H */ +/* These tables map the ALSA channels 1..N to the channels that we
- need to use in order to find the relevant channel buffer. RME
- refers to this kind of mapping as between "the ADAT channel and
- the DMA channel." We index it using the logical audio channel,
- and the value is the DMA channel (i.e. channel buffer number)
- where the data for that channel can be read/written from/to.
+*/
+char channel_map_unity_ss[HDSPM_MAX_CHANNELS] = {
Err, no, array definitions should be never in a header file. It doesn't matter whether it's static or not. The definition should be in *.c file where it's used.
So, simply move these array definitions to hdspm.c.
thanks,
Takashi
On Thu, 27 Jan 2011, Takashi Iwai wrote:
At Wed, 26 Jan 2011 19:32:14 +0100, Adrian Knoth wrote:
diff --git a/include/sound/hdspm.h b/include/sound/hdspm.h index 81990b2..c3f1819 100644 --- a/include/sound/hdspm.h +++ b/include/sound/hdspm.h @@ -131,4 +225,175 @@ typedef struct hdspm_version hdspm_version_t; typedef struct hdspm_channelfader snd_hdspm_channelfader_t; typedef struct hdspm_mixer hdspm_mixer_t;
-#endif /* __SOUND_HDSPM_H */ +/* These tables map the ALSA channels 1..N to the channels that we
- need to use in order to find the relevant channel buffer. RME
- refers to this kind of mapping as between "the ADAT channel and
- the DMA channel." We index it using the logical audio channel,
- and the value is the DMA channel (i.e. channel buffer number)
- where the data for that channel can be read/written from/to.
+*/
+char channel_map_unity_ss[HDSPM_MAX_CHANNELS] = {
Err, no, array definitions should be never in a header file. It doesn't matter whether it's static or not. The definition should be in *.c file where it's used.
So, simply move these array definitions to hdspm.c.
I think that the reason is to share these arrays with hdspmixer. But I agree, that it's better to move these static values to .c files.
Note that hdsm/hdspmixer issues are last ones to resolve before 1.0.24 release. I am waiting for the driver update.
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
As requested by Takashi and Jaroslav, these arrays should not be in the header file.
Signed-off-by: Adrian Knoth adi@drcomp.erfurt.thur.de
diff --git a/include/sound/hdspm.h b/include/sound/hdspm.h index c3f1819..1774ff5 100644 --- a/include/sound/hdspm.h +++ b/include/sound/hdspm.h @@ -225,175 +225,5 @@ typedef struct hdspm_version hdspm_version_t; typedef struct hdspm_channelfader snd_hdspm_channelfader_t; typedef struct hdspm_mixer hdspm_mixer_t;
-/* These tables map the ALSA channels 1..N to the channels that we - need to use in order to find the relevant channel buffer. RME - refers to this kind of mapping as between "the ADAT channel and - the DMA channel." We index it using the logical audio channel, - and the value is the DMA channel (i.e. channel buffer number) - where the data for that channel can be read/written from/to. -*/ - -char channel_map_unity_ss[HDSPM_MAX_CHANNELS] = { - 0, 1, 2, 3, 4, 5, 6, 7, - 8, 9, 10, 11, 12, 13, 14, 15, - 16, 17, 18, 19, 20, 21, 22, 23, - 24, 25, 26, 27, 28, 29, 30, 31, - 32, 33, 34, 35, 36, 37, 38, 39, - 40, 41, 42, 43, 44, 45, 46, 47, - 48, 49, 50, 51, 52, 53, 54, 55, - 56, 57, 58, 59, 60, 61, 62, 63 -}; - -char channel_map_unity_ds[HDSPM_MAX_CHANNELS] = { - 0, 2, 4, 6, 8, 10, 12, 14, - 16, 18, 20, 22, 24, 26, 28, 30, - 32, 34, 36, 38, 40, 42, 44, 46, - 48, 50, 52, 54, 56, 58, 60, 62, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, -}; - -char channel_map_unity_qs[HDSPM_MAX_CHANNELS] = { - 0, 4, 8, 12, 16, 20, 24, 28, - 32, 36, 40, 44, 48, 52, 56, 60, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, -}; - -char channel_map_raydat_ss[HDSPM_MAX_CHANNELS] = { - 4, 5, 6, 7, 8, 9, 10, 11, /* ADAT 1 */ - 12, 13, 14, 15, 16, 17, 18, 19, /* ADAT 2 */ - 20, 21, 22, 23, 24, 25, 26, 27, /* ADAT 3 */ - 28, 29, 30, 31, 32, 33, 34, 35, /* ADAT 4 */ - 0, 1, /* AES */ - 2, 3, /* SPDIF */ - -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, -}; - -char channel_map_raydat_ds[HDSPM_MAX_CHANNELS] = { - 4, 5, 6, 7, /* ADAT 1 */ - 8, 9, 10, 11, /* ADAT 2 */ - 12, 13, 14, 15, /* ADAT 3 */ - 16, 17, 18, 19, /* ADAT 4 */ - 0, 1, /* AES */ - 2, 3, /* SPDIF */ - -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, -}; - -char channel_map_raydat_qs[HDSPM_MAX_CHANNELS] = { - 4, 5, /* ADAT 1 */ - 6, 7, /* ADAT 2 */ - 8, 9, /* ADAT 3 */ - 10, 11, /* ADAT 4 */ - 0, 1, /* AES */ - 2, 3, /* SPDIF */ - -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, -}; - -char channel_map_aio_in_ss[HDSPM_MAX_CHANNELS] = { - 0, 1, /* line in */ - 8, 9, /* aes in, */ - 10, 11, /* spdif in */ - 12, 13, 14, 15, 16, 17, 18, 19, /* ADAT in */ - -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, -}; - -char channel_map_aio_out_ss[HDSPM_MAX_CHANNELS] = { - 0, 1, /* line out */ - 8, 9, /* aes out */ - 10, 11, /* spdif out */ - 12, 13, 14, 15, 16, 17, 18, 19, /* ADAT out */ - 6, 7, /* phone out */ - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, -}; - -char channel_map_aio_in_ds[HDSPM_MAX_CHANNELS] = { - 0, 1, /* line in */ - 8, 9, /* aes in */ - 10, 11, /* spdif in */ - 12, 14, 16, 18, /* adat in */ - -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1 -}; - -char channel_map_aio_out_ds[HDSPM_MAX_CHANNELS] = { - 0, 1, /* line out */ - 8, 9, /* aes out */ - 10, 11, /* spdif out */ - 12, 14, 16, 18, /* adat out */ - 6, 7, /* phone out */ - -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1 -}; - -char channel_map_aio_in_qs[HDSPM_MAX_CHANNELS] = { - 0, 1, /* line in */ - 8, 9, /* aes in */ - 10, 11, /* spdif in */ - 12, 16, /* adat in */ - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1 -}; - -char channel_map_aio_out_qs[HDSPM_MAX_CHANNELS] = { - 0, 1, /* line out */ - 8, 9, /* aes out */ - 10, 11, /* spdif out */ - 12, 16, /* adat out */ - 6, 7, /* phone out */ - -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, -1, -1, -1, -1 -};
#endif diff --git a/sound/pci/rme9652/hdspm.c b/sound/pci/rme9652/hdspm.c index 2db871d..28a1eb3 100644 --- a/sound/pci/rme9652/hdspm.c +++ b/sound/pci/rme9652/hdspm.c @@ -673,6 +673,177 @@ static char *texts_ports_aio_out_qs[] = { "Phone.L", "Phone.R" };
+/* These tables map the ALSA channels 1..N to the channels that we + need to use in order to find the relevant channel buffer. RME + refers to this kind of mapping as between "the ADAT channel and + the DMA channel." We index it using the logical audio channel, + and the value is the DMA channel (i.e. channel buffer number) + where the data for that channel can be read/written from/to. +*/ + +static char channel_map_unity_ss[HDSPM_MAX_CHANNELS] = { + 0, 1, 2, 3, 4, 5, 6, 7, + 8, 9, 10, 11, 12, 13, 14, 15, + 16, 17, 18, 19, 20, 21, 22, 23, + 24, 25, 26, 27, 28, 29, 30, 31, + 32, 33, 34, 35, 36, 37, 38, 39, + 40, 41, 42, 43, 44, 45, 46, 47, + 48, 49, 50, 51, 52, 53, 54, 55, + 56, 57, 58, 59, 60, 61, 62, 63 +}; + +static char channel_map_unity_ds[HDSPM_MAX_CHANNELS] = { + 0, 2, 4, 6, 8, 10, 12, 14, + 16, 18, 20, 22, 24, 26, 28, 30, + 32, 34, 36, 38, 40, 42, 44, 46, + 48, 50, 52, 54, 56, 58, 60, 62, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, +}; + +static char channel_map_unity_qs[HDSPM_MAX_CHANNELS] = { + 0, 4, 8, 12, 16, 20, 24, 28, + 32, 36, 40, 44, 48, 52, 56, 60, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, +}; + +static char channel_map_raydat_ss[HDSPM_MAX_CHANNELS] = { + 4, 5, 6, 7, 8, 9, 10, 11, /* ADAT 1 */ + 12, 13, 14, 15, 16, 17, 18, 19, /* ADAT 2 */ + 20, 21, 22, 23, 24, 25, 26, 27, /* ADAT 3 */ + 28, 29, 30, 31, 32, 33, 34, 35, /* ADAT 4 */ + 0, 1, /* AES */ + 2, 3, /* SPDIF */ + -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, +}; + +static char channel_map_raydat_ds[HDSPM_MAX_CHANNELS] = { + 4, 5, 6, 7, /* ADAT 1 */ + 8, 9, 10, 11, /* ADAT 2 */ + 12, 13, 14, 15, /* ADAT 3 */ + 16, 17, 18, 19, /* ADAT 4 */ + 0, 1, /* AES */ + 2, 3, /* SPDIF */ + -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, +}; + +static char channel_map_raydat_qs[HDSPM_MAX_CHANNELS] = { + 4, 5, /* ADAT 1 */ + 6, 7, /* ADAT 2 */ + 8, 9, /* ADAT 3 */ + 10, 11, /* ADAT 4 */ + 0, 1, /* AES */ + 2, 3, /* SPDIF */ + -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, +}; + +static char channel_map_aio_in_ss[HDSPM_MAX_CHANNELS] = { + 0, 1, /* line in */ + 8, 9, /* aes in, */ + 10, 11, /* spdif in */ + 12, 13, 14, 15, 16, 17, 18, 19, /* ADAT in */ + -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, +}; + +static char channel_map_aio_out_ss[HDSPM_MAX_CHANNELS] = { + 0, 1, /* line out */ + 8, 9, /* aes out */ + 10, 11, /* spdif out */ + 12, 13, 14, 15, 16, 17, 18, 19, /* ADAT out */ + 6, 7, /* phone out */ + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, +}; + +static char channel_map_aio_in_ds[HDSPM_MAX_CHANNELS] = { + 0, 1, /* line in */ + 8, 9, /* aes in */ + 10, 11, /* spdif in */ + 12, 14, 16, 18, /* adat in */ + -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1 +}; + +static char channel_map_aio_out_ds[HDSPM_MAX_CHANNELS] = { + 0, 1, /* line out */ + 8, 9, /* aes out */ + 10, 11, /* spdif out */ + 12, 14, 16, 18, /* adat out */ + 6, 7, /* phone out */ + -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1 +}; + +static char channel_map_aio_in_qs[HDSPM_MAX_CHANNELS] = { + 0, 1, /* line in */ + 8, 9, /* aes in */ + 10, 11, /* spdif in */ + 12, 16, /* adat in */ + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1 +}; + +static char channel_map_aio_out_qs[HDSPM_MAX_CHANNELS] = { + 0, 1, /* line out */ + 8, 9, /* aes out */ + 10, 11, /* spdif out */ + 12, 16, /* adat out */ + 6, 7, /* phone out */ + -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1 +}; + struct hdspm_midi { struct hdspm *hdspm; int id;
On Thu, Jan 27, 2011 at 09:09:05AM +0100, Jaroslav Kysela wrote:
+char channel_map_unity_ss[HDSPM_MAX_CHANNELS] = {
Err, no, array definitions should be never in a header file. It doesn't matter whether it's static or not. The definition should be in *.c file where it's used.
We have more than one C file using this information...
So, simply move these array definitions to hdspm.c.
I think that the reason is to share these arrays with hdspmixer.
Exactly. hdspmixer needs this information, too. If we move it to the C file, we would need to duplicate the information in hdspmixer. And then we forget about it next time hdspm.h needs to be updated.
But I agree, that it's better to move these static values to .c files.
Ok, I've sent a follow-up patch to avoid to send the whole 180k patch for the 4th time.
I'll now duplicate this information and send a patch against hdspmixer. I feel that there should be a better solution like getting the channel mapping from the driver (sysfs, proc, ioctl or whatever), but that's for some other time.
HTH
On Thu, 27 Jan 2011, Adrian Knoth wrote:
On Thu, Jan 27, 2011 at 09:09:05AM +0100, Jaroslav Kysela wrote:
+char channel_map_unity_ss[HDSPM_MAX_CHANNELS] = {
Err, no, array definitions should be never in a header file. It doesn't matter whether it's static or not. The definition should be in *.c file where it's used.
We have more than one C file using this information...
So, simply move these array definitions to hdspm.c.
I think that the reason is to share these arrays with hdspmixer.
Exactly. hdspmixer needs this information, too. If we move it to the C file, we would need to duplicate the information in hdspmixer. And then we forget about it next time hdspm.h needs to be updated.
But I agree, that it's better to move these static values to .c files.
Ok, I've sent a follow-up patch to avoid to send the whole 180k patch for the 4th time.
I'll now duplicate this information and send a patch against hdspmixer. I feel that there should be a better solution like getting the channel mapping from the driver (sysfs, proc, ioctl or whatever), but that's for some other time.
Thanks. I applied your patches to my git tree with some slight subject modifications. Please, Takashi, do a cherry-pick. Thanks.
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
At Thu, 27 Jan 2011 11:44:34 +0100 (CET), Jaroslav Kysela wrote:
On Thu, 27 Jan 2011, Adrian Knoth wrote:
On Thu, Jan 27, 2011 at 09:09:05AM +0100, Jaroslav Kysela wrote:
+char channel_map_unity_ss[HDSPM_MAX_CHANNELS] = {
Err, no, array definitions should be never in a header file. It doesn't matter whether it's static or not. The definition should be in *.c file where it's used.
We have more than one C file using this information...
So, simply move these array definitions to hdspm.c.
I think that the reason is to share these arrays with hdspmixer.
Exactly. hdspmixer needs this information, too. If we move it to the C file, we would need to duplicate the information in hdspmixer. And then we forget about it next time hdspm.h needs to be updated.
But I agree, that it's better to move these static values to .c files.
Ok, I've sent a follow-up patch to avoid to send the whole 180k patch for the 4th time.
I'll now duplicate this information and send a patch against hdspmixer. I feel that there should be a better solution like getting the channel mapping from the driver (sysfs, proc, ioctl or whatever), but that's for some other time.
Thanks. I applied your patches to my git tree with some slight subject modifications. Please, Takashi, do a cherry-pick. Thanks.
Done. But there are compile warnings. They must be fixed before your release.
Also, sync again hdspm.h in alsa-lib and etc.
thanks,
Takashi
On Thu, 27 Jan 2011, Takashi Iwai wrote:
At Thu, 27 Jan 2011 11:44:34 +0100 (CET), Jaroslav Kysela wrote:
On Thu, 27 Jan 2011, Adrian Knoth wrote:
On Thu, Jan 27, 2011 at 09:09:05AM +0100, Jaroslav Kysela wrote:
+char channel_map_unity_ss[HDSPM_MAX_CHANNELS] = {
Err, no, array definitions should be never in a header file. It doesn't matter whether it's static or not. The definition should be in *.c file where it's used.
We have more than one C file using this information...
So, simply move these array definitions to hdspm.c.
I think that the reason is to share these arrays with hdspmixer.
Exactly. hdspmixer needs this information, too. If we move it to the C file, we would need to duplicate the information in hdspmixer. And then we forget about it next time hdspm.h needs to be updated.
But I agree, that it's better to move these static values to .c files.
Ok, I've sent a follow-up patch to avoid to send the whole 180k patch for the 4th time.
I'll now duplicate this information and send a patch against hdspmixer. I feel that there should be a better solution like getting the channel mapping from the driver (sysfs, proc, ioctl or whatever), but that's for some other time.
Thanks. I applied your patches to my git tree with some slight subject modifications. Please, Takashi, do a cherry-pick. Thanks.
Done. But there are compile warnings. They must be fixed before your release.
I fixed all warning in commit "ALSA: hdspm - remove unused arrays, reduce stack usage in hwdep_ioctl" now.
Also, sync again hdspm.h in alsa-lib and etc.
Sure.
Jaroslav
----- Jaroslav Kysela perex@perex.cz Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc.
At Thu, 27 Jan 2011 11:27:57 +0100, Adrian Knoth wrote:
On Thu, Jan 27, 2011 at 09:09:05AM +0100, Jaroslav Kysela wrote:
+char channel_map_unity_ss[HDSPM_MAX_CHANNELS] = {
Err, no, array definitions should be never in a header file. It doesn't matter whether it's static or not. The definition should be in *.c file where it's used.
We have more than one C file using this information...
Still it doesn't rectify.
So, simply move these array definitions to hdspm.c.
I think that the reason is to share these arrays with hdspmixer.
Exactly. hdspmixer needs this information, too. If we move it to the C file, we would need to duplicate the information in hdspmixer. And then we forget about it next time hdspm.h needs to be updated.
Then such info isn't appropriate to be static. If it's dynamic, this has to be provided via other interface.
But I agree, that it's better to move these static values to .c files.
Ok, I've sent a follow-up patch to avoid to send the whole 180k patch for the 4th time.
I'll now duplicate this information and send a patch against hdspmixer. I feel that there should be a better solution like getting the channel mapping from the driver (sysfs, proc, ioctl or whatever), but that's for some other time.
Exactly.
BTW, now I get the following warnings: sound/pci/rme9652/hdspm.c:578:14: warning: ‘texts_sync_status’ defined but not used sound/pci/rme9652/hdspm.c:695:13: warning: ‘channel_map_unity_ds’ defined but not used sound/pci/rme9652/hdspm.c:706:13: warning: ‘channel_map_unity_qs’ defined but not used sound/pci/rme9652/hdspm.c: In function ‘snd_hdspm_hwdep_ioctl’: sound/pci/rme9652/hdspm.c:6141:1: warning: the frame size of 2464 bytes is larger than 1024 bytes
Care to fix it?
thanks,
Takashi
On 01/27/11 09:09, Jaroslav Kysela wrote:
So, simply move these array definitions to hdspm.c.
I think that the reason is to share these arrays with hdspmixer.
There's more than hdspmixer out there that needs this data.
But I agree, that it's better to move these static values to .c files.
The solution is to make them non-static (which was an error on my side). Replicating information is not a bright idea.
Flo
participants (4)
-
Adrian Knoth -
Florian Faber -
Jaroslav Kysela -
Takashi Iwai