Skip to contentSkip to conversation

saa7146: Emagic Audiowerk8 low-level ALSA driver

PATCH In review Matthias Nyffenegger 2 participants 2 messages Oct 22, 2008 – Oct 23, 2008

lore
Share & tools
lore

Matthias Nyffenegger

author
#0 lore raw

PATCH 1/3 saa7146: Emagic Audiowerk8 low-level ALSA driver

From: Matthias Nyffenegger <[email protected]>

Low-level ALSA driver for Emagic Audiowerk8 sound card. Project page: http://sourceforge.net/projects/aw8-alsa
Built and tested with Vanilla 2.6.25.16

Signed-off-by Matthias Nyffenegger <[email protected]>

---
This is a request for submission to ALSA-tree.

diff -uprN ../alsa-driver-1.0.17/alsa-kernel/pci/saa7146/log.h ../alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/log.h

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/log.h renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/log.h +43 -0
@@ -0,0 +1,43 @@
+/*
+ *  Kernel log convenience macros
+ *  Copyright (c) 2006- by M. Nyffenegger <matthias.nyffenegger[AT]bluewin.ch>
+ *
+ *
+ *   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
+ *   (at your option) any later version.
+ *
+ *   This program is distributed in the hope that it will be useful,
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *   GNU General Public License for more details.
+ *
+ *   You should have received a copy of the GNU General Public License
+ *   along with this program; if not, write to the Free Software
+ *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
+ *
+ */
+#ifndef LOG_H_
+#define LOG_H_
+#define LOG_ENABLE
+#ifndef SAA7146_SUBSYS_LOG_TAG
+#define SAA7146_SUBSYS_LOG_TAG "SAA7146"
+#endif
+#ifdef LOG_ENABLE
+# define LOG_ERROR(fmt, args...) \
+	printk(KERN_ERR SAA7146_SUBSYS_LOG_TAG ": %s:%d:" fmt "\n", \
+					__func__, __LINE__, ## args);
+# define LOG_WARN(fmt, args...) \
+	printk(KERN_WARNING SAA7146_SUBSYS_LOG_TAG ": %s:%d:" fmt "\n", \
+					__func__, __LINE__, ## args);
+# define LOG_INFO(fmt, args...) \
+	printk(KERN_INFO SAA7146_SUBSYS_LOG_TAG ": %s:%d:" fmt "\n", \
+					__func__, __LINE__, ## args);
+#else
+# define LOG_ERROR(fmt, args...)
+# define LOG_WARN(fmt, args...)
+# define LOG_INFO(fmt, args...)
+#endif /* LOG_ENABLE */
+
+#endif /*LOG_H_*/

diff -uprN ../alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146audio.c ../alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146audio.c +343 -0

343 lines not on this page · raw message

diff -uprN ../alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146audio.h ../alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.h

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.h renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146audio.h +120 -0

120 lines not on this page · raw message

diff -uprN ../alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146.h ../alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146.h

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146.h renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146.h +216 -0

216 lines not on this page · raw message

diff -uprN ../alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146i2c.c ../alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2c.c

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2c.c renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146i2c.c +270 -0

270 lines not on this page · raw message

diff -uprN ../alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146i2c.h ../alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2c.h

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2c.h renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146i2c.h +81 -0

81 lines not on this page · raw message

diff -uprN ../alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146i2s.c ../alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2s.c

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2s.c renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146i2s.c +437 -0

437 lines not on this page · raw message

diff -uprN ../alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146i2s.h ../alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2s.h

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2s.h renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146i2s.h +197 -0

197 lines not on this page · raw message

#1 lore raw

At Thu, 23 Oct 2008 00:04:50 +0200,

Quoted 5 lines · Matthias Nyffenegger

From: Matthias Nyffenegger <[email protected]>

Low-level ALSA driver for Emagic Audiowerk8 sound card. Project page: http://sourceforge.net/projects/aw8-alsa

Thanks for the patch.

Quoted 1 line

Built and tested with Vanilla 2.6.25.16

Any chance to test, at least, on 2.6.27?

Some brief review comments below.

Quoted 4 lines

diff -uprN ../alsa-driver-1.0.17/alsa-kernel/pci/saa7146/log.h ../alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/log.h

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/log.h renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/log.h +0 -0

(snip)

Quoted 7 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/log.h renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/log.h +7 -0
+#ifdef LOG_ENABLE
+# define LOG_ERROR(fmt, args...) \
+	printk(KERN_ERR SAA7146_SUBSYS_LOG_TAG ": %s:%d:" fmt "\n", \
+					__func__, __LINE__, ## args);
+# define LOG_WARN(fmt, args...) \
+	printk(KERN_WARNING SAA7146_SUBSYS_LOG_TAG ": %s:%d:" fmt "\n", \
+					__func__, __LINE__, ## args);

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/log.h line 43

Quoted 10 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/log.h renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/log.h +10 -0
+# define LOG_INFO(fmt, args...) \
+	printk(KERN_INFO SAA7146_SUBSYS_LOG_TAG ": %s:%d:" fmt "\n", \
+					__func__, __LINE__, ## args);
+#else
+# define LOG_ERROR(fmt, args...)
+# define LOG_WARN(fmt, args...)
+# define LOG_INFO(fmt, args...)
+#endif /* LOG_ENABLE */
+
+#endif /*LOG_H_*/

Use the standard log function instead of a home-made one. There are different functions, such as pr_err(), and the one tied with the device such as dev_err(). The latter one is preferred nowadays if possible.

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c line 33

Quoted 11 lines

diff -uprN ../alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146audio.c ../alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146audio.c +7 -0
+/**
+ * Address maps for Audio DMA Control PCI_ADP, BaseAx_x, ProtAx_x, PageAx_x and
+ * MC1 Audio Transfer Enable: 1st dim. index corresponds to
+ * enum audio_interfaces's, 2nd to enum directions's.
+ * TODO: decouple from saa7146i2s.h by providing a function that makes a
+ *       reliable mapping (independent of int values of enum members).
+ */

Don't use "/**" unless you give kernel-doc style comments.

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c line 35

Quoted 2 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146audio.c +2 -0
+static const int Base[2][2] = {{BaseA1_in, BaseA1_out},
+				{BaseA2_in, BaseA2_out} };

Remove the space before the closing brace.

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c line 43

Quoted 8 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146audio.c +8 -0
+static const int Prot[2][2] = {{ProtA1_in, ProtA1_out},
+				{ProtA2_in, ProtA2_out} };
+static const int Page[2][2] = {{PageA1_in, PageA1_out},
+				{PageA2_in, PageA2_out} };
+static const int TR_E[2][2] = {{TR_E_A1_IN, TR_E_A1_OUT},
+				{TR_E_A2_IN, TR_E_A2_OUT} };
+static const int PCI_ADP[2][2] = {{PCI_ADP2, PCI_ADP1},
+				{PCI_ADP4, PCI_ADP3} };

Ditto.

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c line 76

Quoted 4 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146audio.c +4 -0
+/**
+ * see saa7146audio.h
+ */
+struct audio_stream *saa7146_stream_prepare_capture(

saa7146 is used in other devices such as video.
Thus saa7146_ prefix may conflict. Please add snd_ prefix to be sure for global (even unexported) functions and variables.

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c line 109

Quoted 5 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146audio.c +5 -0
+/**
+ * see saa7146audio.h
+ */
+void saa7146_stream_unprepare(struct saa7146audio *chipaudio,
+				struct audio_stream *stream)

"unprepare" is no common word. Better to use "cleanup" or so.

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c line 112

Quoted 3 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146audio.c +3 -0
+{
+	int i = 0;
+	struct i2s_device *device = NULL;

No need to initialize.

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c

Quoted 2 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146audio.c +2 -0
+
+	if (stream != NULL) {

In the kernel code, more common style is

	if (stream) {
		...

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c line 201

Quoted 12 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146audio.c +12 -0
+static struct audio_stream *stream_prepare(struct saa7146audio *chipaudio,
+					unsigned int stream_nr,
+					unsigned long buffer_base_addr,
+					int buffer_size,
+					int channel_count,
+					int sample_length,
+					enum endian endian,
+					enum directions direction)
+{
+	struct audio_stream *stream = NULL;
+	int max_streams = (direction == in ?
+				MAX_IN_AUDIO_STREAMS : MAX_OUT_AUDIO_STREAMS);

Use unsigned int in this case. Or check negative value, too.

Quoted 26 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146audio.c +26 -0
+	struct audio_stream *streams = (direction == in ?
+				chipaudio->in_streams : chipaudio->out_streams);
+
+	if (stream_nr > max_streams || streams[stream_nr].state == enabled) {
+		LOG_WARN("Stream nr=%d not available", stream_nr);
+		return NULL;
+	}
+	stream = &streams[stream_nr];
+	if (stream_check_resources(chipaudio, stream_nr, channel_count,
+				sample_length, endian, direction) != 0)
+		return NULL;
+	if (stream_init(chipaudio, stream, stream_nr, buffer_base_addr,
+		buffer_size, channel_count, sample_length, endian, direction)
+									!= 0)
+		return NULL;
+	if (stream_setup_dma(chipaudio, stream) != 0)
+		return NULL;
+	return stream;
+}
+
+/**
+ * Check if the requested resources (channels, sample-length, endian) are
+ * available.
+ */
+static int stream_check_resources(struct saa7146audio *chipaudio,
+				unsigned int stream_nr,

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c line 237

Quoted 10 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146audio.c +10 -0
+				int channel_count,
+				int sample_length,
+				enum endian endian,
+				enum directions direction)
+{
+	int i = 0;
+	int available_channels = 0;
+	int available_frame_length = 0;
+	struct i2s_device *device = NULL;
+	struct audio_interface *ai = NULL;

Unnecessary initializations (can be seen in every place)...

Quoted 6 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146audio.c +6 -0
+static int stream_setup_dma(struct saa7146audio *chipaudio,
+				struct audio_stream *stream)
+{
+	int exp = 0;
+	int limit = 0;
+	enum audio_interfaces ai_nr = stream->audio_interface->id;

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c line 323

Quoted 10 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146audio.c +10 -0
+	enum directions dir = stream->direction;
+	struct saa7146reg *chip = &chipaudio->chipi2s.chip;
+
+	if (!is_valid_period(stream->buffer_size >> 1)) {
+		LOG_ERROR("Invalid buffer size %d", stream->buffer_size);
+		return -1;
+	}
+	/* find exponent for period, required to figure out the DMA IRQ limit */
+	limit = stream->buffer_size >> 1; /* period is buffer_size/2 */
+	for (exp = 0; limit > 0; limit >>= 1, exp++);

Put ";" in the next line.
But, this calculation can be replaced with ilog2() (+1) in linux/log2.h.

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c line 342

Quoted 9 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.c renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146audio.c +9 -0
+/**
+ * Period size must be >= 64bytes <= 1Mb and a power of 2.
+ * This is a restriction imposed by the SAA7146 DMA capabilities: the period at
+ * which DMA IRQs are generated is 64*2^n where n={1-14}.
+ * @return 1 if the given period is valid.
+ */
+static int is_valid_period(int period)
+{
+	return (period >= 64) && (period <= (1 << 20)) && is_pow_of_2(period);

Don't use magic numbers here.

Quoted 4 lines

diff -uprN ../alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146audio.h ../alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.h

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146audio.h renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146audio.h +0 -0

(snip)

Quoted 7 lines

+/**
+ * TODO: description
+ * @param chipaudio used for SAA7146 I2S ctrl and register access over PCI bus,
+ * must not be NULL.
+ * @return A reference to an audio_stream structure, or NULL in case the
+ * stream could not be opened.
+ */

Put the function description around the definition (i.e. *.c), not around the declaration in the header.

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146.h line 44

Quoted 13 lines

diff -uprN ../alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146.h ../alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146.h

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146.h renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146.h +9 -0
+/**
+ * Read an SAA7146 register, mind that PCI is little-endian.
+ * @param chip used for SAA7146 register access over PCI bus
+ * @param offset register address offset
+ * @return contents of register at the given address offset
+ */
+static inline uint32_t saa7146_read(struct saa7146reg *chip, int offset)
+{
+	return __le32_to_cpu(readl(chip->iobase_virt + offset));

No need to change the endian here. readl() does it already.

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146.h line 57

Quoted 13 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146.h renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146.h +13 -0
+}
+
+/**
+ * Write an SAA7146 register, mind that PCI is little-endian.
+ * @param chip used for SAA7146 register access over PCI bus
+ * @param offset register address offset
+ * @param data the data to be written at the given address offset
+ */
+static inline void saa7146_write(struct saa7146reg *chip,
+				int offset,
+				uint32_t data)
+{
+	writel(__cpu_to_le32(data), chip->iobase_virt + offset);

Ditto.

Quoted 4 lines

diff -uprN ../alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146i2c.c ../alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2c.c

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2c.c renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146i2c.c +0 -0

(snip)

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2c.c

Quoted 2 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2c.c renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146i2c.c +2 -0
+/* Convenience macros */
+#define MAX(a, b) ((a) > (b) ? (a) : (b))

Use the standard one.

Quoted 4 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2c.c renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146i2c.c +4 -0
+
+/**
+ * See SAA7146 tech. specification p.122 - 126 for details.
+ */

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2c.c line 42

Quoted 10 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2c.c renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146i2c.c +10 -0
+
+#define IICSTA_ABORT	0x80
+#define IICSTA_ERR_BSY	0x7f
+#define MC1_EI2C	0x01000000
+#define MC2_UPLD_IIC	0x00010000
+#define START		3
+#define CONT		2
+#define STOP		1
+#define NOP		0
+#define RW		1

I'm afraid these names are too simple and can be easily misused...

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2c.c line 139

Quoted 12 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2c.c renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146i2c.c +12 -0
+static int i2c_open(struct saa7146reg *chip,
+			enum saa7146_i2c_clock clk,
+			int read,
+			uint8_t dev_addr,
+			uint8_t *sub_addr,
+			unsigned int sub_addr_size)
+{
+	int sub_addr_bytes_transmitted = 0;
+	uint8_t addr = 0;
+
+	i2c_prepare(chip, clk);
+	addr = (dev_addr<<1)&~RW;

Fix spaces.

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2s.c

Quoted 4 lines

diff -uprN ../alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146i2s.c ../alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2s.c

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2s.c renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146i2s.c +0 -0

(snip0

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2s.c line 28

Quoted 3 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2s.c renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146i2s.c +3 -0
+/* Convenience macros */
+#define MAX(a, b) ((a) > (b) ? (a) : (b))
+#define MIN(a, b) ((a) < (b) ? (a) : (b))

Use standard macros.

Quoted 4 lines

diff -uprN ../alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146i2s.h ../alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2s.h

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2s.h renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146i2s.h +0 -0

(snip)

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2s.h line 37

Quoted 5 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2s.h renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146i2s.h +5 -0
+/**
+ * Enumeration of SAA7146 audio-interfaces A1 and A2.
+ * Order matters (see head of saa7146i2s.c: address and bit offset maps)
+ */
+enum audio_interfaces {a1, a2};

We are in the world of C where enum is always global.
So please use more distinctive words. "a1" and "a2" look very like variable names.

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2s.h line 43

Quoted 5 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2s.h renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146i2s.h +5 -0
+/**
+ * Enumeration of SAA7146 i2c WS lines.
+ * Order matters (see head of saa7146i2s.c: address and bit offset maps)
+ */
+enum ws_lines {ws0, ws1, ws2, ws3, ws4};

Ditto.

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2s.h line 54

Quoted 10 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2s.h renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146i2s.h +10 -0
+/**
+ * Enumeration of SAA7146 i2c SD lines.
+ * sd0_i_a2: SD0 is always input on SAA7146 audiointerface A2
+ * sd0_o_a1: SD0 is always output on SAA7146 audiointerface A1
+ * sd4_i_a1: SD4 is always input on SAA7146 audiointerface A1
+ * sd4_o_a2: SD4 is always output on SAA7146 audiointerface A2
+ * Order matters (see head of saa7146i2s.c: address and bit offset maps)
+ */
+enum sd_lines {sd0_i_a2, sd0_o_a1, sd1_io_ax, sd2_io_ax, sd3_io_ax, sd4_i_a1,
+	sd4_o_a2};

Ditto.

Quoted 1 line

+enum endian {le, be};

Ditto.
(The endian notion would be better rather as a bool flag such as
is_big_endian.)

Quoted 1 line

+enum directions {in, out};

Ditto.

Quoted 1 line

+enum states {disabled, enabled};

Ditto. This should be really a bool.

Quoted 1 line

+enum hw_mon {source, destination, notmonitored};

Ditto.

alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2s.h line 195

Quoted 7 lines
alsa-driver-1.0.17.mod/alsa-kernel/pci/saa7146/saa7146i2s.h renamed from alsa-driver-1.0.17/alsa-kernel/pci/saa7146/saa7146i2s.h +7 -0
+/**
+ * @return 1 if the given value is a power of 2.
+ */
+static inline int is_pow_of_2(int val)
+{
+	return ((val != 0) && ((val & (val - 1)) == 0));
+}

There is already is_power_of_2() in linux/log2.h.

thanks,

Takashi