* [PATCH - [hdspm] 0/3] Support for RME RayDAT and AIO
@ 2011-01-18 14:12 Adrian Knoth
2011-01-18 14:12 ` [PATCH - [hdspm] 1/3] Add headers for RayDAT and AIO support Adrian Knoth
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Adrian Knoth @ 2011-01-18 14:12 UTC (permalink / raw)
To: patch; +Cc: Adrian Knoth, alsa-devel
Hi!
As announced a few days ago, I've integrated the offspring hdspm driver
by Florian Faber. The new code now also supports the 36in/36out RME
RayDAT cards and RME AIOs.
I tried to follow the kernel's coding style as much as possible,
however, there are still a few overlong lines left. For example, I
didn't want to break "if read_reg(...) & 0x3f" at "& 0x3f", as it
worsens readability. If it's required to comply, feel free to add the
missing line breaks.
Right now, RayDATs and AIOs can only operate on a single period size
of 1024 samples, because these cards always use a fixed 16k buffer per
channel and only vary interrupt frequency. I'm going to try to fix this
after I've learned more about the PCM interface. ;) Any suggestions
welcome.
Cheers
Adrian Knoth (3):
Add headers for RayDAT and AIO support.
Enable RME RayDAT and AIO
Add RayDAT and AIO strings to Kconfig
include/hdspm.h | 442 +++++-
pci/Kconfig | 6 +-
pci/rme9652/hdspm.c | 4135 +++++++++++++++++++++++++++++++++++++--------------
3 files changed, 3442 insertions(+), 1141 deletions(-)
--
1.7.2.3
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH - [hdspm] 1/3] Add headers for RayDAT and AIO support. 2011-01-18 14:12 [PATCH - [hdspm] 0/3] Support for RME RayDAT and AIO Adrian Knoth @ 2011-01-18 14:12 ` Adrian Knoth 2011-01-18 14:40 ` Takashi Iwai 2011-01-18 14:12 ` [PATCH - [hdspm] 3/3] Add RayDAT and AIO strings to Kconfig Adrian Knoth ` (2 subsequent siblings) 3 siblings, 1 reply; 15+ messages in thread From: Adrian Knoth @ 2011-01-18 14:12 UTC (permalink / raw) To: patch; +Cc: Adrian Knoth, alsa-devel Merged the changes by Florian Faber to the hdspm driver. Code taken from http://wiki.linuxproaudio.org/index.php/Driver:hdspe Whitespace fixes, also added missing commas to the texts_ports_* arrays. Changed card_clock to unsigned long long in struct hdspm_status. Signed-off-by: Adrian Knoth <adi@drcomp.erfurt.thur.de> diff --git a/include/hdspm.h b/include/hdspm.h index 81990b2..5b69694 100644 --- a/include/hdspm.h +++ b/include/hdspm.h @@ -3,8 +3,8 @@ /* * Copyright (C) 2003 Winfried Ritsch (IEM) * based on hdsp.h from Thomas Charbonnel (thomas@undata.org) - * - * + * + * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation; either version 2 of the License, or @@ -23,50 +23,37 @@ /* Maximum channels is 64 even on 56Mode you have 64playbacks to matrix */ #define HDSPM_MAX_CHANNELS 64 -/* -------------------- IOCTL Peak/RMS Meters -------------------- */ - -/* peam rms level structure like we get from hardware - - maybe in future we can memory map it so I just copy it - to user on ioctl call now an dont change anything - rms are made out of low and high values - where (long) ????_rms = (????_rms_l >> 8) + ((????_rms_h & 0xFFFFFF00)<<24) - (i asume so from the code) -*/ - -struct hdspm_peak_rms { +typedef enum { + MADI, + MADIface, + AIO, + AES32, + RayDAT +} hdspm_io_type; - unsigned int level_offset[1024]; - unsigned int input_peak[64]; - unsigned int playback_peak[64]; - unsigned int output_peak[64]; - unsigned int xxx_peak[64]; /* not used */ +/* -------------------- IOCTL Peak/RMS Meters -------------------- */ - unsigned int reserved[256]; /* not used */ +struct hdspm_peak_rms { + uint32_t input_peaks[64]; + uint32_t playback_peaks[64]; + uint32_t output_peaks[64]; - unsigned int input_rms_l[64]; - unsigned int playback_rms_l[64]; - unsigned int output_rms_l[64]; - unsigned int xxx_rms_l[64]; /* not used */ + uint64_t input_rms[64]; + uint64_t playback_rms[64]; + uint64_t output_rms[64]; - unsigned int input_rms_h[64]; - unsigned int playback_rms_h[64]; - unsigned int output_rms_h[64]; - unsigned int xxx_rms_h[64]; /* not used */ + enum {ss, ds, qs} speed; + int status2; }; -struct hdspm_peak_rms_ioctl { - struct hdspm_peak_rms *peak; -}; -/* use indirect access due to the limit of ioctl bit size */ #define SNDRV_HDSPM_IOCTL_GET_PEAK_RMS \ - _IOR('H', 0x40, struct hdspm_peak_rms_ioctl) + _IOR('H', 0x42, struct hdspm_peak_rms) /* ------------ CONFIG block IOCTL ---------------------- */ -struct hdspm_config_info { +struct hdspm_config { unsigned char pref_sync_ref; unsigned char wordclock_sync_check; unsigned char madi_sync_check; @@ -80,14 +67,118 @@ struct hdspm_config_info { unsigned int analog_out; }; -#define SNDRV_HDSPM_IOCTL_GET_CONFIG_INFO \ - _IOR('H', 0x41, struct hdspm_config_info) -/* get Soundcard Version */ +#define SNDRV_HDSPM_IOCTL_GET_CONFIG \ + _IOR('H', 0x41, struct hdspm_config) + + + +/** + * If there's a TCO (TimeCode Option) board installed, + * there are further options and status data available. + * The hdspm_ltc structure contains the current SMPTE + * timecode and some status information and can be + * optained via SNDRV_HDSPM_IOCTL_GET_LTC or in the + * hdspm_status struct. + **/ + +typedef enum { + format_invalid, + fps_24, + fps_25, + fps_2997, + fps_30 +} hdspm_ltc_format; + +typedef enum { + frame_invalid, + drop_frame, + full_frame +} hdspm_ltc_frame; + +typedef enum { + ntsc, + pal, + no_video +} hdspm_ltc_input_format; + +struct hdspm_ltc { + unsigned int ltc; + + hdspm_ltc_format format; + hdspm_ltc_frame frame; + hdspm_ltc_input_format input_format; +}; + +#define SNDRV_HDSPM_IOCTL_GET_LTC _IOR('H', 0x46, struct hdspm_mixer_ioctl) + + +/** + * The status data reflects the device's current state + * as determined by the card's configuration and + * connection status. + **/ + +typedef enum { + hdspm_sync_no_lock = 0, + hdspm_sync_lock = 1, + hdspm_sync_sync = 2 +} hdspm_sync; + +typedef enum {hdspm_input_optical = 0, hdspm_input_coax = 1} hdspm_madi_input; + +typedef enum {hdspm_format_ch_64 = 0, + hdspm_format_ch_56 = 1 +} hdspm_madi_channel_format; + +typedef enum {hdspm_frame_48 = 0, + hdspm_frame_96 = 1 +} hdspm_madi_frame_format; + +typedef enum {syncsource_wc = 0, + syncsource_madi = 1, + syncsource_tco = 2, + syncsource_sync = 3, + syncsource_none = 4 +} hdspm_syncsource; + +struct hdspm_status { + hdspm_io_type card_type; + hdspm_syncsource autosync_source; + + unsigned long long card_clock; + unsigned int master_period; + + union { + struct { + hdspm_sync sync_wc; + hdspm_sync sync_madi; + hdspm_sync sync_tco; + hdspm_sync sync_in; + hdspm_madi_input madi_input; + hdspm_madi_channel_format channel_format; + hdspm_madi_frame_format frame_format; + } madi; + } card_specific; +}; + +#define SNDRV_HDSPM_IOCTL_GET_STATUS \ + _IOR('H', 0x47, struct hdspm_status) + + +/** + * Get information about the card and its add-ons. + **/ + +#define HDSPM_ADDON_TCO 1 struct hdspm_version { + hdspm_io_type card_type; + char cardname[20]; + unsigned int serial; unsigned short firmware_rev; + int addons; }; #define SNDRV_HDSPM_IOCTL_GET_VERSION _IOR('H', 0x43, struct hdspm_version) @@ -103,7 +194,7 @@ struct hdspm_version { /* equivalent to hardware definition, maybe for future feature of mmap of * them */ -/* each of 64 outputs has 64 infader and 64 outfader: +/* each of 64 outputs has 64 infader and 64 outfader: Ins to Outs mixer[out].in[in], Outstreams to Outs mixer[out].pb[pb] */ #define HDSPM_MIXER_CHANNELS HDSPM_MAX_CHANNELS @@ -131,4 +222,277 @@ 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 */ + +static char *texts_sync_status[] = { + "no lock", + "lock", + "sync" +}; + +static char *texts_ports_madi[] = { + "MADI.1", "MADI.2", "MADI.3", "MADI.4", "MADI.5", "MADI.6", + "MADI.7", "MADI.8", "MADI.9", "MADI.10", "MADI.11", "MADI.12", + "MADI.13", "MADI.14", "MADI.15", "MADI.16", "MADI.17", "MADI.18", + "MADI.19", "MADI.20", "MADI.21", "MADI.22", "MADI.23", "MADI.24", + "MADI.25", "MADI.26", "MADI.27", "MADI.28", "MADI.29", "MADI.30", + "MADI.31", "MADI.32", "MADI.33", "MADI.34", "MADI.35", "MADI.36", + "MADI.37", "MADI.38", "MADI.39", "MADI.40", "MADI.41", "MADI.42", + "MADI.43", "MADI.44", "MADI.45", "MADI.46", "MADI.47", "MADI.48", + "MADI.49", "MADI.50", "MADI.51", "MADI.52", "MADI.53", "MADI.54", + "MADI.55", "MADI.56", "MADI.57", "MADI.58", "MADI.59", "MADI.60", + "MADI.61", "MADI.62", "MADI.63", "MADI.64", +}; + + +static char *texts_ports_raydat_ss[] = { + "ADAT1.1", "ADAT1.2", "ADAT1.3", "ADAT1.4", "ADAT1.5", "ADAT1.6", + "ADAT1.7", "ADAT1.8", "ADAT2.1", "ADAT2.2", "ADAT2.3", "ADAT2.4", + "ADAT2.5", "ADAT2.6", "ADAT2.7", "ADAT2.8", "ADAT3.1", "ADAT3.2", + "ADAT3.3", "ADAT3.4", "ADAT3.5", "ADAT3.6", "ADAT3.7", "ADAT3.8", + "ADAT4.1", "ADAT4.2", "ADAT4.3", "ADAT4.4", "ADAT4.5", "ADAT4.6", + "ADAT4.7", "ADAT4.8", + "AES.L", "AES.R", + "SPDIF.L", "SPDIF.R" +}; + +static char *texts_ports_raydat_ds[] = { + "ADAT1.1", "ADAT1.2", "ADAT1.3", "ADAT1.4", + "ADAT2.1", "ADAT2.2", "ADAT2.3", "ADAT2.4", + "ADAT3.1", "ADAT3.2", "ADAT3.3", "ADAT3.4", + "ADAT4.1", "ADAT4.2", "ADAT4.3", "ADAT4.4", + "AES.L", "AES.R", + "SPDIF.L", "SPDIF.R" +}; + +static char *texts_ports_raydat_qs[] = { + "ADAT1.1", "ADAT1.2", + "ADAT2.1", "ADAT2.2", + "ADAT3.1", "ADAT3.2", + "ADAT4.1", "ADAT4.2", + "AES.L", "AES.R", + "SPDIF.L", "SPDIF.R" +}; + + +static char *texts_ports_aio_in_ss[] = { + "Analogue.L", "Analogue.R", + "AES.L", "AES.R", + "SPDIF.L", "SPDIF.R", + "ADAT.1", "ADAT.2", "ADAT.3", "ADAT.4", "ADAT.5", "ADAT.6", + "ADAT.7", "ADAT.8" +}; + +static char *texts_ports_aio_out_ss[] = { + "Analogue.L", "Analogue.R", + "AES.L", "AES.R", + "SPDIF.L", "SPDIF.R", + "ADAT.1", "ADAT.2", "ADAT.3", "ADAT.4", "ADAT.5", "ADAT.6", + "ADAT.7", "ADAT.8", + "Phone.L", "Phone.R" +}; + +static char *texts_ports_aio_in_ds[] = { + "Analogue.L", "Analogue.R", + "AES.L", "AES.R", + "SPDIF.L", "SPDIF.R", + "ADAT.1", "ADAT.2", "ADAT.3", "ADAT.4" +}; + +static char *texts_ports_aio_out_ds[] = { + "Analogue.L", "Analogue.R", + "AES.L", "AES.R", + "SPDIF.L", "SPDIF.R", + "ADAT.1", "ADAT.2", "ADAT.3", "ADAT.4", + "Phone.L", "Phone.R" +}; + +static char *texts_ports_aio_in_qs[] = { + "Analogue.L", "Analogue.R", + "AES.L", "AES.R", + "SPDIF.L", "SPDIF.R", + "ADAT.1", "ADAT.2", "ADAT.3", "ADAT.4" +}; + +static char *texts_ports_aio_out_qs[] = { + "Analogue.L", "Analogue.R", + "AES.L", "AES.R", + "SPDIF.L", "SPDIF.R", + "ADAT.1", "ADAT.2", "ADAT.3", "ADAT.4", + "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 +}; + + +#endif -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH - [hdspm] 1/3] Add headers for RayDAT and AIO support. 2011-01-18 14:12 ` [PATCH - [hdspm] 1/3] Add headers for RayDAT and AIO support Adrian Knoth @ 2011-01-18 14:40 ` Takashi Iwai 2011-01-18 15:00 ` Adrian Knoth [not found] ` <290463D19D2E064191F1F96ECA480A89432873706D@EXMAIL02.scwf.nsc.com> 0 siblings, 2 replies; 15+ messages in thread From: Takashi Iwai @ 2011-01-18 14:40 UTC (permalink / raw) To: Adrian Knoth; +Cc: alsa-devel, patch At Tue, 18 Jan 2011 15:12:17 +0100, Adrian Knoth wrote: > > +struct hdspm_peak_rms { > + uint32_t input_peaks[64]; > + uint32_t playback_peaks[64]; > + uint32_t output_peaks[64]; Strange indentation level? > +struct hdspm_status { > + hdspm_io_type card_type; > + hdspm_syncsource autosync_source; > + > + unsigned long long card_clock; Use more explicit int type with bit width. > +static char *texts_sync_status[] = { > + "no lock", > + "lock", > + "sync" > +}; Put these to *.c. Takashi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH - [hdspm] 1/3] Add headers for RayDAT and AIO support. 2011-01-18 14:40 ` Takashi Iwai @ 2011-01-18 15:00 ` Adrian Knoth [not found] ` <290463D19D2E064191F1F96ECA480A89432873706D@EXMAIL02.scwf.nsc.com> 1 sibling, 0 replies; 15+ messages in thread From: Adrian Knoth @ 2011-01-18 15:00 UTC (permalink / raw) To: alsa-devel On Tue, Jan 18, 2011 at 03:40:21PM +0100, Takashi Iwai wrote: > Adrian Knoth wrote: > > > > +struct hdspm_peak_rms { > > + uint32_t input_peaks[64]; > > + uint32_t playback_peaks[64]; > > + uint32_t output_peaks[64]; > Strange indentation level? Indeed. Fixed. > > +struct hdspm_status { > > + hdspm_io_type card_type; > > + hdspm_syncsource autosync_source; > > + > > + unsigned long long card_clock; > Use more explicit int type with bit width. Done. > > +static char *texts_sync_status[] = { > > + "no lock", > > + "lock", > > + "sync" > > +}; > > Put these to *.c. Looks like this array is actually unused. Let's see which of them can be dropped or moved to *.c. I'll have to cross-check with hdspmixer, some parts are referenced there. Cheers -- mail: adi@thur.de http://adi.thur.de PGP/GPG: key via keyserver ^ permalink raw reply [flat|nested] 15+ messages in thread
[parent not found: <290463D19D2E064191F1F96ECA480A89432873706D@EXMAIL02.scwf.nsc.com>]
[parent not found: <s5hwrm1l68q.wl%tiwai@suse.de>]
* Re: Process of sumit the Audio driver code to ALSA [not found] ` <s5hwrm1l68q.wl%tiwai@suse.de> @ 2011-01-19 7:20 ` Reddy, MR Swami 2011-01-19 7:36 ` Takashi Iwai 2011-01-19 22:55 ` Seungwhan Youn 0 siblings, 2 replies; 15+ messages in thread From: Reddy, MR Swami @ 2011-01-19 7:20 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel@alsa-project.org Thanks for quick response! Yes, it ASOC based and currently, the audio drivers supported for Samsung 6410 target. Thanks Swami -----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, January 19, 2011 12:39 PM To: Reddy, MR Swami Subject: Re: Process of sumit the Audio driver code to ALSA At Tue, 18 Jan 2011 20:58:28 -0800, Reddy, MR Swami wrote: > > Hello, > We (National Semi) have developed ALSA based audio drivers for our Audio subsystem. Could you please let me know the process to submit and include as part of ALSA project. > > The ALSA audio driver version info as follows: > - Linux - 2.6.24 base > - Andorid- 2.1 base Is this based on ASoC? If not, I recommend you to port the code to ASoC, thus it's the standard for embedded devices. If you have any questions, just post to alsa-devel ML. thanks, Takashi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Process of sumit the Audio driver code to ALSA 2011-01-19 7:20 ` Process of sumit the Audio driver code to ALSA Reddy, MR Swami @ 2011-01-19 7:36 ` Takashi Iwai 2011-03-07 11:26 ` Reddy, MR Swami 2011-01-19 22:55 ` Seungwhan Youn 1 sibling, 1 reply; 15+ messages in thread From: Takashi Iwai @ 2011-01-19 7:36 UTC (permalink / raw) To: Reddy, MR Swami; +Cc: alsa-devel@alsa-project.org At Tue, 18 Jan 2011 23:20:15 -0800, Reddy, MR Swami wrote: > > Thanks for quick response! > Yes, it ASOC based and currently, the audio drivers supported for Samsung 6410 target. OK, then just post your patches to alsa-devel ML, Cc'ed to ASoC maintainers (Liam Girdwood <lrg@slimlogic.co.uk> and Mark Brown <broonie@opensource.wolfsonmicro.com>). But I guess there have been quite many changes in ASoC core (e.g. multi-core support) that are no longer compatible with 2.6.24-based code. These have to be fixed, of course, before merging. It's up to you -- you can first pot the original patch and ask some hints for forward-porting, or first start reading the latest ASoC API changes, do port your driver then post the revised patch. thanks, Takashi > > Thanks > Swami > > -----Original Message----- > From: Takashi Iwai [mailto:tiwai@suse.de] > Sent: Wednesday, January 19, 2011 12:39 PM > To: Reddy, MR Swami > Subject: Re: Process of sumit the Audio driver code to ALSA > > At Tue, 18 Jan 2011 20:58:28 -0800, > Reddy, MR Swami wrote: > > > > Hello, > > We (National Semi) have developed ALSA based audio drivers for our Audio subsystem. Could you please let me know the process to submit and include as part of ALSA project. > > > > The ALSA audio driver version info as follows: > > - Linux - 2.6.24 base > > - Andorid- 2.1 base > > Is this based on ASoC? If not, I recommend you to port the code to ASoC, thus it's the standard for embedded devices. > If you have any questions, just post to alsa-devel ML. > > > thanks, > > Takashi > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Process of sumit the Audio driver code to ALSA 2011-01-19 7:36 ` Takashi Iwai @ 2011-03-07 11:26 ` Reddy, MR Swami 0 siblings, 0 replies; 15+ messages in thread From: Reddy, MR Swami @ 2011-03-07 11:26 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel@alsa-project.org Hi Takashi, OK. I will submit the patches for National's (NSC) LM49352 and LM49350 audio cards (with older version of ALSA and Linux 2.6.24 tested). And request for the forward porting guidelines. Thanks Swami -----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, January 19, 2011 1:07 PM To: Reddy, MR Swami Cc: alsa-devel@alsa-project.org Subject: Re: Process of sumit the Audio driver code to ALSA At Tue, 18 Jan 2011 23:20:15 -0800, Reddy, MR Swami wrote: > > Thanks for quick response! > Yes, it ASOC based and currently, the audio drivers supported for Samsung 6410 target. OK, then just post your patches to alsa-devel ML, Cc'ed to ASoC maintainers (Liam Girdwood <lrg@slimlogic.co.uk> and Mark Brown <broonie@opensource.wolfsonmicro.com>). But I guess there have been quite many changes in ASoC core (e.g. multi-core support) that are no longer compatible with 2.6.24-based code. These have to be fixed, of course, before merging. It's up to you -- you can first pot the original patch and ask some hints for forward-porting, or first start reading the latest ASoC API changes, do port your driver then post the revised patch. thanks, Takashi > > Thanks > Swami > > -----Original Message----- > From: Takashi Iwai [mailto:tiwai@suse.de] > Sent: Wednesday, January 19, 2011 12:39 PM > To: Reddy, MR Swami > Subject: Re: Process of sumit the Audio driver code to ALSA > > At Tue, 18 Jan 2011 20:58:28 -0800, > Reddy, MR Swami wrote: > > > > Hello, > > We (National Semi) have developed ALSA based audio drivers for our Audio subsystem. Could you please let me know the process to submit and include as part of ALSA project. > > > > The ALSA audio driver version info as follows: > > - Linux - 2.6.24 base > > - Andorid- 2.1 base > > Is this based on ASoC? If not, I recommend you to port the code to ASoC, thus it's the standard for embedded devices. > If you have any questions, just post to alsa-devel ML. > > > thanks, > > Takashi > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Process of sumit the Audio driver code to ALSA 2011-01-19 7:20 ` Process of sumit the Audio driver code to ALSA Reddy, MR Swami 2011-01-19 7:36 ` Takashi Iwai @ 2011-01-19 22:55 ` Seungwhan Youn 1 sibling, 0 replies; 15+ messages in thread From: Seungwhan Youn @ 2011-01-19 22:55 UTC (permalink / raw) To: Reddy, MR Swami; +Cc: Takashi Iwai, alsa-devel@alsa-project.org Hi, On Wed, Jan 19, 2011 at 4:20 PM, Reddy, MR Swami <MR.Swami.Reddy@nsc.com> wrote: > Thanks for quick response! > Yes, it ASOC based and currently, the audio drivers supported for Samsung 6410 target. If you are trying to submit your new ASoC driver, you may work based on Mr. Mark Brown's ASoC git repository (git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound-2.6.git). And also, if your driver works on Samsung 6410 target, 'sound/soc/samsung/smdk_wm8580.c' and 'sound/soc/samsung/smdk_wm9713.c' on 'for-2.6.39' (or 'for-next') branch will give you some hints. ;) Have fun, claude _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH - [hdspm] 3/3] Add RayDAT and AIO strings to Kconfig 2011-01-18 14:12 [PATCH - [hdspm] 0/3] Support for RME RayDAT and AIO Adrian Knoth 2011-01-18 14:12 ` [PATCH - [hdspm] 1/3] Add headers for RayDAT and AIO support Adrian Knoth @ 2011-01-18 14:12 ` Adrian Knoth 2011-01-18 14:36 ` [PATCH - [hdspm] 0/3] Support for RME RayDAT and AIO Takashi Iwai [not found] ` <1295359939-14673-3-git-send-email-adi@drcomp.erfurt.thur.de> 3 siblings, 0 replies; 15+ messages in thread From: Adrian Knoth @ 2011-01-18 14:12 UTC (permalink / raw) To: patch; +Cc: Adrian Knoth, alsa-devel Inform users about the newly added capabilities. Signed-off-by: Adrian Knoth <adi@drcomp.erfurt.thur.de> diff --git a/pci/Kconfig b/pci/Kconfig index 7b2678a..5df9844 100644 --- a/pci/Kconfig +++ b/pci/Kconfig @@ -570,13 +570,13 @@ comment "Don't forget to add built-in firmwares for HDSP driver" depends on SND_HDSP=y config SND_HDSPM - tristate "RME Hammerfall DSP MADI" + tristate "RME Hammerfall DSP MADI/RayDAT/AIO" select SND_HWDEP select SND_RAWMIDI select SND_PCM help - Say Y here to include support for RME Hammerfall DSP MADI - soundcards. + Say Y here to include support for RME Hammerfall DSP MADI, + RayDAT and AIO soundcards. To compile this driver as a module, choose M here: the module will be called snd-hdspm. -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH - [hdspm] 0/3] Support for RME RayDAT and AIO 2011-01-18 14:12 [PATCH - [hdspm] 0/3] Support for RME RayDAT and AIO Adrian Knoth 2011-01-18 14:12 ` [PATCH - [hdspm] 1/3] Add headers for RayDAT and AIO support Adrian Knoth 2011-01-18 14:12 ` [PATCH - [hdspm] 3/3] Add RayDAT and AIO strings to Kconfig Adrian Knoth @ 2011-01-18 14:36 ` Takashi Iwai 2011-01-18 17:06 ` Adrian Knoth [not found] ` <1295359939-14673-3-git-send-email-adi@drcomp.erfurt.thur.de> 3 siblings, 1 reply; 15+ messages in thread From: Takashi Iwai @ 2011-01-18 14:36 UTC (permalink / raw) To: Adrian Knoth; +Cc: alsa-devel At Tue, 18 Jan 2011 15:12:16 +0100, Adrian Knoth wrote: > > Hi! > > As announced a few days ago, I've integrated the offspring hdspm driver > by Florian Faber. The new code now also supports the 36in/36out RME > RayDAT cards and RME AIOs. Thanks for the patch! > I tried to follow the kernel's coding style as much as possible, > however, there are still a few overlong lines left. For example, I > didn't want to break "if read_reg(...) & 0x3f" at "& 0x3f", as it > worsens readability. If it's required to comply, feel free to add the > missing line breaks. This is no big issue. Would be better if the line fits in 80 chars, but it's no must. > Right now, RayDATs and AIOs can only operate on a single period size > of 1024 samples, because these cards always use a fixed 16k buffer per > channel and only vary interrupt frequency. I'm going to try to fix this > after I've learned more about the PCM interface. ;) Any suggestions > welcome. Basically one patch should contain the changes that are build-clean, i.e. you can build cleanly after patching. Thus changing a header and *.c in separate patches doesn't work well. Also, please use the generic variable types as much as possible for ioctl structs. Avoid use of enum or such, especially ones that are typedef'ed. In general, avoid typedefs in the kernel code as much as possible. Defining each enum type is no-go. Second, don't define arrays in the header file. They must be in the body, not included headers. Also, remember checkpatch.pl is your friend. thanks, Takashi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH - [hdspm] 0/3] Support for RME RayDAT and AIO 2011-01-18 14:36 ` [PATCH - [hdspm] 0/3] Support for RME RayDAT and AIO Takashi Iwai @ 2011-01-18 17:06 ` Adrian Knoth 2011-01-19 6:41 ` Takashi Iwai 0 siblings, 1 reply; 15+ messages in thread From: Adrian Knoth @ 2011-01-18 17:06 UTC (permalink / raw) To: alsa-devel On 01/18/11 15:36, Takashi Iwai wrote: Hi! > Basically one patch should contain the changes that are build-clean, > i.e. you can build cleanly after patching. Thus changing a header > and *.c in separate patches doesn't work well. Ok. Better luck next time. ;) Speaking of which: should I resend everything as a whole-in-one patch? I could also provide a download link (given we're talking 170+kb here) > Also, please use the generic variable types as much as possible > for ioctl structs. Avoid use of enum or such, especially ones that > are typedef'ed. In general, avoid typedefs in the kernel code as much > as possible. Defining each enum type is no-go. If've dropped all the new typedefs and only kept the existing ones now. However, it still reads "enum foo something" in the structs for some ioctls. Is this acceptable or do I have to change this to int? If so, would I manually add a couple of #defines? I somehow feel losing the semantic connection to the enum is bad, but this might OTOH be intended, e.g. for the sake of a stable interface. > Second, don't define arrays in the header file. They must be in the > body, not included headers. Cross-checked with hdspmixer and moved to .c, except for the channel mappings as they're included from userspace. (channel_map*) > Also, remember checkpatch.pl is your friend. It already was for the very last days. But I'm more excited about my new friend Lindent. ;) Cheers ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH - [hdspm] 0/3] Support for RME RayDAT and AIO 2011-01-18 17:06 ` Adrian Knoth @ 2011-01-19 6:41 ` Takashi Iwai 0 siblings, 0 replies; 15+ messages in thread From: Takashi Iwai @ 2011-01-19 6:41 UTC (permalink / raw) To: Adrian Knoth; +Cc: alsa-devel At Tue, 18 Jan 2011 18:06:00 +0100, Adrian Knoth wrote: > > On 01/18/11 15:36, Takashi Iwai wrote: > > Hi! > > > Basically one patch should contain the changes that are build-clean, > > i.e. you can build cleanly after patching. Thus changing a header > > and *.c in separate patches doesn't work well. > > Ok. Better luck next time. ;) Speaking of which: should I resend > everything as a whole-in-one patch? That's OK. At best, split a patch into some logical pieces (i.e. MIDI handling changes, addition of RayDAT/AIO support, code-refactoring, etc.) But if this is hard to achieve, just "the big one" is enough. > I could also provide a download link (given we're talking 170+kb here) This should be avoided. The link content isn't archived, so we'll loose the code history. And it makes difficult to review. > > Also, please use the generic variable types as much as possible > > for ioctl structs. Avoid use of enum or such, especially ones that > > are typedef'ed. In general, avoid typedefs in the kernel code as much > > as possible. Defining each enum type is no-go. > > If've dropped all the new typedefs and only kept the existing ones now. > However, it still reads "enum foo something" in the structs for some > ioctls. > > Is this acceptable or do I have to change this to int? If so, would I > manually add a couple of #defines? You can use int (short or whatever) and keep enum as its values. > I somehow feel losing the semantic > connection to the enum is bad, but this might OTOH be intended, e.g. for > the sake of a stable interface. Exactly. enum itself is nice to use, but the problem is that its size isn't defined strictly, rather depending on its contents. So, for ioctl, types with fixed size is better. > > Second, don't define arrays in the header file. They must be in the > > body, not included headers. > > Cross-checked with hdspmixer and moved to .c, except for the channel > mappings as they're included from userspace. (channel_map*) OK. > > Also, remember checkpatch.pl is your friend. > > It already was for the very last days. But I'm more excited about my new > friend Lindent. ;) Heh, but sometimes the sense of beauty conflicts between Lindent and human being ;) I recommend you to re-read the code after running it. thanks, Takashi ^ permalink raw reply [flat|nested] 15+ messages in thread
[parent not found: <1295359939-14673-3-git-send-email-adi@drcomp.erfurt.thur.de>]
* Re: [PATCH - [hdspm] 2/3] Enable RME RayDAT and AIO [not found] ` <1295359939-14673-3-git-send-email-adi@drcomp.erfurt.thur.de> @ 2011-01-18 14:55 ` Takashi Iwai 2011-01-18 15:24 ` Adrian Knoth 0 siblings, 1 reply; 15+ messages in thread From: Takashi Iwai @ 2011-01-18 14:55 UTC (permalink / raw) To: Adrian Knoth; +Cc: alsa-devel At Tue, 18 Jan 2011 15:12:18 +0100, Adrian Knoth wrote: > > @@ -29,12 +42,13 @@ > #include <linux/moduleparam.h> > #include <linux/slab.h> > #include <linux/pci.h> > -#include <linux/math64.h> No math64 stuff is necessary? > +#include <linux/sysfs.h> It's not implemented yet, so don't include it. > -static DEFINE_PCI_DEVICE_TABLE(snd_hdspm_ids) = { > +static struct pci_device_id snd_hdspm_ids[] __devinitdata = { Why open-coded again? > @@ -796,6 +1009,8 @@ static void hdspm_silence_playback(struct hdspm *hdspm) > int n = hdspm->period_bytes; > void *buf = hdspm->playback_buffer; > > + snd_printk(KERN_INFO "hdspm_silence_playback\n"); Don't put such a verbose debug message in frequently called callbacks. > static void hdspm_set_dds_value(struct hdspm *hdspm, int rate) ... > n = div_u64(n, rate); > /* n should be less than 2^32 for being written to FREQ register */ > - snd_BUG_ON(n >> 32); > + snd_BUG_ON((n >> 32) == 0); This check is consistent with the comment above... The rest is too long to review. Would be nice if you can split the whole patch in logical orders... thanks, Takashi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH - [hdspm] 2/3] Enable RME RayDAT and AIO 2011-01-18 14:55 ` [PATCH - [hdspm] 2/3] Enable " Takashi Iwai @ 2011-01-18 15:24 ` Adrian Knoth 2011-01-21 7:07 ` Takashi Iwai 0 siblings, 1 reply; 15+ messages in thread From: Adrian Knoth @ 2011-01-18 15:24 UTC (permalink / raw) To: alsa-devel On Tue, Jan 18, 2011 at 03:55:56PM +0100, Takashi Iwai wrote: > > -#include <linux/math64.h> > No math64 stuff is necessary? Probably not. At least it works and compiles without. Perhaps cross-included from somewhere else? I see div_u64 down there, so I guess linux/math64.h should be included. Fixed. > > +#include <linux/sysfs.h> > It's not implemented yet, so don't include it. ACK. > > -static DEFINE_PCI_DEVICE_TABLE(snd_hdspm_ids) = { > > +static struct pci_device_id snd_hdspm_ids[] __devinitdata = { > Why open-coded again? No particular reason. Now you mention it, I'll change it back. How did it end up there?: I took the code from Florian's tarball. Looks like there was some divergent development in the meantime (between his fork and mainline kernel) > > @@ -796,6 +1009,8 @@ static void hdspm_silence_playback(struct hdspm *hdspm) > > int n = hdspm->period_bytes; > > void *buf = hdspm->playback_buffer; > > > > + snd_printk(KERN_INFO "hdspm_silence_playback\n"); > Don't put such a verbose debug message in frequently called callbacks. ACK. Removed. > > n = div_u64(n, rate); > > /* n should be less than 2^32 for being written to FREQ register */ > > - snd_BUG_ON(n >> 32); > > + snd_BUG_ON((n >> 32) == 0); > This check is consistent with the comment above... Inconsistent I guess...? I suggest to remove the line. We know n, we control rate, this should never be an issue. > The rest is too long to review. Would be nice if you can split the > whole patch in logical orders... I'm afraid I can't, because there's no logical order. There's just this single hdspm.c (and .h) by Florian and my style fixes. No VCS. So whatever is wrong needs to be spotted by some experienced ALSA dev. I guess you'd be faster fixing it instead of mailing me all the things that are wrong, but I'm happy to change things you point out. Cheers -- mail: adi@thur.de http://adi.thur.de PGP/GPG: key via keyserver ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH - [hdspm] 2/3] Enable RME RayDAT and AIO 2011-01-18 15:24 ` Adrian Knoth @ 2011-01-21 7:07 ` Takashi Iwai 0 siblings, 0 replies; 15+ messages in thread From: Takashi Iwai @ 2011-01-21 7:07 UTC (permalink / raw) To: Adrian Knoth; +Cc: alsa-devel At Tue, 18 Jan 2011 16:24:08 +0100, Adrian Knoth wrote: > > > > -static DEFINE_PCI_DEVICE_TABLE(snd_hdspm_ids) = { > > > +static struct pci_device_id snd_hdspm_ids[] __devinitdata = { > > Why open-coded again? > > No particular reason. Now you mention it, I'll change it back. I guess this was changed in the upstream from the version Florian's code based on. > How did it end up there?: I took the code from Florian's tarball. Looks > like there was some divergent development in the meantime (between his > fork and mainline kernel) Yes, it's always bad to fork in such a way. If forking, it should track also the upstream changes (which can be better done via git). > > > n = div_u64(n, rate); > > > /* n should be less than 2^32 for being written to FREQ register */ > > > - snd_BUG_ON(n >> 32); > > > + snd_BUG_ON((n >> 32) == 0); > > This check is consistent with the comment above... > > Inconsistent I guess...? Yep. > I suggest to remove the line. We know n, we control rate, this should > never be an issue. Fair enough. > > The rest is too long to review. Would be nice if you can split the > > whole patch in logical orders... > > I'm afraid I can't, because there's no logical order. There's just this > single hdspm.c (and .h) by Florian and my style fixes. No VCS. Too bad. > So whatever is wrong needs to be spotted by some experienced ALSA dev. I > guess you'd be faster fixing it instead of mailing me all the things > that are wrong, but I'm happy to change things you point out. Thanks. Could you repost the fixed patches? Don't forget to put me to Cc (or send me and put alsa-devel ML to Cc), so that it reaches to me directly. I don't look through ML so regularly, and alsa-devel ML blocks the posts from non-subscribers or ones with too large contents for approvals. Takashi ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2011-03-07 11:26 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-18 14:12 [PATCH - [hdspm] 0/3] Support for RME RayDAT and AIO Adrian Knoth
2011-01-18 14:12 ` [PATCH - [hdspm] 1/3] Add headers for RayDAT and AIO support Adrian Knoth
2011-01-18 14:40 ` Takashi Iwai
2011-01-18 15:00 ` Adrian Knoth
[not found] ` <290463D19D2E064191F1F96ECA480A89432873706D@EXMAIL02.scwf.nsc.com>
[not found] ` <s5hwrm1l68q.wl%tiwai@suse.de>
2011-01-19 7:20 ` Process of sumit the Audio driver code to ALSA Reddy, MR Swami
2011-01-19 7:36 ` Takashi Iwai
2011-03-07 11:26 ` Reddy, MR Swami
2011-01-19 22:55 ` Seungwhan Youn
2011-01-18 14:12 ` [PATCH - [hdspm] 3/3] Add RayDAT and AIO strings to Kconfig Adrian Knoth
2011-01-18 14:36 ` [PATCH - [hdspm] 0/3] Support for RME RayDAT and AIO Takashi Iwai
2011-01-18 17:06 ` Adrian Knoth
2011-01-19 6:41 ` Takashi Iwai
[not found] ` <1295359939-14673-3-git-send-email-adi@drcomp.erfurt.thur.de>
2011-01-18 14:55 ` [PATCH - [hdspm] 2/3] Enable " Takashi Iwai
2011-01-18 15:24 ` Adrian Knoth
2011-01-21 7:07 ` Takashi Iwai
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).