[PATCH 1/2] ad1848: separate header for registers definition

[PATCH 1/2] ad1848: separate header for registers definition

[Krzysztof Helt]

From: Krzysztof Helt <krzysztof.h1@wp.pl>

Separate AD1848 registers definition into a new header.
The new header can be used by compatible CS4231 chip
driver.

Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>

---

This patch is a start combining ad1848_lib and cs4231_lib
functionality. My approach is to make the cs4231_lib module
use the ad1848_lib module. This should allow removing
most of identical functions from the cs4231_lib and use
the functions from the ad1848_lib directly.

If this approach is not a good idea, please drop the patch.

My current plan is to reuse the ad1848 register constants
in the cs423x code and remove duplicated cs4231 constants.
This should make the ad1848_lib and cs4231_lib easier to
compare for differences.

The next step is to use the snd_cs4231 structure instead of 
the snd_ad1848 structure inside the ad1848_lib. This will make
functions from the ad1848_lib taking the same arguments
as the functions in the cs4231_lib.

The last step would be to export needed functions from the
ad1848_lib and use them directly in the cs4231_lib.

This should reduce the size of the cs4231_lib while preserving
smaller the ad1848_lib for driver which uses only ad1848
functionality.

Kind regards,
Krzysztof

diff -urpN linux-ref/include/sound/ad1848-regs.h linux-mm/include/sound/ad1848-regs.h
--- linux-ref/include/sound/ad1848-regs.h	1970-01-01 01:00:00.000000000 +0100
+++ linux-mm/include/sound/ad1848-regs.h	2008-06-15 07:00:08.041351281 +0200
@@ -0,0 +1,104 @@
+#ifndef __SOUND_AD1848_REGS_H
+#define __SOUND_AD1848_REGS_H
+
+/*
+ *  Copyright (c) by Jaroslav Kysela <perex@perex.cz>
+ *  Definitions for AD1847/AD1848/CS4248 chips registers
+ *
+ *
+ *   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
+ *
+ */
+
+/* IO ports */
+
+#define AD1848P(chip, x) ((chip)->port + c_d_c_AD1848##x)
+
+#define c_d_c_AD1848REGSEL	0
+#define c_d_c_AD1848REG		1
+#define c_d_c_AD1848STATUS	2
+#define c_d_c_AD1848PIO		3
+
+/* codec registers */
+
+#define AD1848_LEFT_INPUT	0x00	/* left input control */
+#define AD1848_RIGHT_INPUT	0x01	/* right input control */
+#define AD1848_AUX1_LEFT_INPUT	0x02	/* left AUX1 input control */
+#define AD1848_AUX1_RIGHT_INPUT	0x03	/* right AUX1 input control */
+#define AD1848_AUX2_LEFT_INPUT	0x04	/* left AUX2 input control */
+#define AD1848_AUX2_RIGHT_INPUT	0x05	/* right AUX2 input control */
+#define AD1848_LEFT_OUTPUT	0x06	/* left output control register */
+#define AD1848_RIGHT_OUTPUT	0x07	/* right output control register */
+#define AD1848_DATA_FORMAT	0x08	/* clock and data format - playback/capture - bits 7-0 MCE */
+#define AD1848_IFACE_CTRL	0x09	/* interface control - bits 7-2 MCE */
+#define AD1848_PIN_CTRL		0x0a	/* pin control */
+#define AD1848_TEST_INIT	0x0b	/* test and initialization */
+#define AD1848_MISC_INFO	0x0c	/* miscellaneous information */
+#define AD1848_LOOPBACK		0x0d	/* loopback control */
+#define AD1848_DATA_UPR_CNT	0x0e	/* playback/capture upper base count */
+#define AD1848_DATA_LWR_CNT	0x0f	/* playback/capture lower base count */
+
+/* definitions for codec register select port - CODECP( REGSEL ) */
+
+#define AD1848_INIT		0x80	/* CODEC is initializing */
+#define AD1848_MCE		0x40	/* mode change enable */
+#define AD1848_TRD		0x20	/* transfer request disable */
+
+/* definitions for codec status register - CODECP( STATUS ) */
+
+#define AD1848_GLOBALIRQ	0x01	/* IRQ is active */
+
+/* definitions for AD1848_LEFT_INPUT and AD1848_RIGHT_INPUT registers */
+
+#define AD1848_ENABLE_MIC_GAIN	0x20
+
+#define AD1848_MIXS_LINE1	0x00
+#define AD1848_MIXS_AUX1	0x40
+#define AD1848_MIXS_LINE2	0x80
+#define AD1848_MIXS_ALL		0xc0
+
+/* definitions for clock and data format register - AD1848_PLAYBK_FORMAT */
+
+#define AD1848_LINEAR_8		0x00	/* 8-bit unsigned data */
+#define AD1848_ALAW_8		0x60	/* 8-bit A-law companded */
+#define AD1848_ULAW_8		0x20	/* 8-bit U-law companded */
+#define AD1848_LINEAR_16	0x40	/* 16-bit twos complement data - little endian */
+#define AD1848_STEREO		0x10	/* stereo mode */
+/* bits 3-1 define frequency divisor */
+#define AD1848_XTAL1		0x00	/* 24.576 crystal */
+#define AD1848_XTAL2		0x01	/* 16.9344 crystal */
+
+/* definitions for interface control register - AD1848_IFACE_CTRL */
+
+#define AD1848_CAPTURE_PIO	0x80	/* capture PIO enable */
+#define AD1848_PLAYBACK_PIO	0x40	/* playback PIO enable */
+#define AD1848_CALIB_MODE	0x18	/* calibration mode bits */
+#define AD1848_AUTOCALIB	0x08	/* auto calibrate */
+#define AD1848_SINGLE_DMA	0x04	/* use single DMA channel */
+#define AD1848_CAPTURE_ENABLE	0x02	/* capture enable */
+#define AD1848_PLAYBACK_ENABLE	0x01	/* playback enable */
+
+/* definitions for pin control register - AD1848_PIN_CTRL */
+
+#define AD1848_IRQ_ENABLE	0x02	/* enable IRQ */
+#define AD1848_XCTL1		0x40	/* external control #1 */
+#define AD1848_XCTL0		0x80	/* external control #0 */
+
+/* definitions for test and init register - AD1848_TEST_INIT */
+
+#define AD1848_CALIB_IN_PROGRESS 0x20	/* auto calibrate in progress */
+#define AD1848_DMA_REQUEST	0x10	/* DMA request in progress */
+
+#endif /* __SOUND_AD1848_REGS_H */
diff -urpN linux-ref/include/sound/ad1848.h linux-mm/include/sound/ad1848.h
--- linux-ref/include/sound/ad1848.h	2008-06-15 06:58:15.293013657 +0200
+++ linux-mm/include/sound/ad1848.h	2008-06-15 00:14:36.728628406 +0200
@@ -25,84 +25,7 @@
 #include "pcm.h"
 #include <linux/interrupt.h>
 
-/* IO ports */
-
-#define AD1848P( chip, x ) ( (chip) -> port + c_d_c_AD1848##x )
-
-#define c_d_c_AD1848REGSEL	0
-#define c_d_c_AD1848REG		1
-#define c_d_c_AD1848STATUS	2
-#define c_d_c_AD1848PIO		3
-
-/* codec registers */
-
-#define AD1848_LEFT_INPUT	0x00	/* left input control */
-#define AD1848_RIGHT_INPUT	0x01	/* right input control */
-#define AD1848_AUX1_LEFT_INPUT	0x02	/* left AUX1 input control */
-#define AD1848_AUX1_RIGHT_INPUT	0x03	/* right AUX1 input control */
-#define AD1848_AUX2_LEFT_INPUT	0x04	/* left AUX2 input control */
-#define AD1848_AUX2_RIGHT_INPUT	0x05	/* right AUX2 input control */
-#define AD1848_LEFT_OUTPUT	0x06	/* left output control register */
-#define AD1848_RIGHT_OUTPUT	0x07	/* right output control register */
-#define AD1848_DATA_FORMAT	0x08	/* clock and data format - playback/capture - bits 7-0 MCE */
-#define AD1848_IFACE_CTRL	0x09	/* interface control - bits 7-2 MCE */
-#define AD1848_PIN_CTRL		0x0a	/* pin control */
-#define AD1848_TEST_INIT	0x0b	/* test and initialization */
-#define AD1848_MISC_INFO	0x0c	/* miscellaneous information */
-#define AD1848_LOOPBACK		0x0d	/* loopback control */
-#define AD1848_DATA_UPR_CNT	0x0e	/* playback/capture upper base count */
-#define AD1848_DATA_LWR_CNT	0x0f	/* playback/capture lower base count */
-
-/* definitions for codec register select port - CODECP( REGSEL ) */
-
-#define AD1848_INIT		0x80	/* CODEC is initializing */
-#define AD1848_MCE		0x40	/* mode change enable */
-#define AD1848_TRD		0x20	/* transfer request disable */
-
-/* definitions for codec status register - CODECP( STATUS ) */
-
-#define AD1848_GLOBALIRQ	0x01	/* IRQ is active */
-
-/* definitions for AD1848_LEFT_INPUT and AD1848_RIGHT_INPUT registers */
-
-#define AD1848_ENABLE_MIC_GAIN	0x20
-
-#define AD1848_MIXS_LINE1	0x00
-#define AD1848_MIXS_AUX1	0x40
-#define AD1848_MIXS_LINE2	0x80
-#define AD1848_MIXS_ALL		0xc0
-
-/* definitions for clock and data format register - AD1848_PLAYBK_FORMAT */
-
-#define AD1848_LINEAR_8		0x00	/* 8-bit unsigned data */
-#define AD1848_ALAW_8		0x60	/* 8-bit A-law companded */
-#define AD1848_ULAW_8		0x20	/* 8-bit U-law companded */
-#define AD1848_LINEAR_16	0x40	/* 16-bit twos complement data - little endian */
-#define AD1848_STEREO		0x10	/* stereo mode */
-/* bits 3-1 define frequency divisor */
-#define AD1848_XTAL1		0x00	/* 24.576 crystal */
-#define AD1848_XTAL2		0x01	/* 16.9344 crystal */
-
-/* definitions for interface control register - AD1848_IFACE_CTRL */
-
-#define AD1848_CAPTURE_PIO	0x80	/* capture PIO enable */
-#define AD1848_PLAYBACK_PIO	0x40	/* playback PIO enable */
-#define AD1848_CALIB_MODE	0x18	/* calibration mode bits */
-#define AD1848_AUTOCALIB	0x08	/* auto calibrate */
-#define AD1848_SINGLE_DMA	0x04	/* use single DMA channel */
-#define AD1848_CAPTURE_ENABLE	0x02	/* capture enable */
-#define AD1848_PLAYBACK_ENABLE	0x01	/* playback enable */
-
-/* definitions for pin control register - AD1848_PIN_CTRL */
-
-#define AD1848_IRQ_ENABLE	0x02	/* enable IRQ */
-#define AD1848_XCTL1		0x40	/* external control #1 */
-#define AD1848_XCTL0		0x80	/* external control #0 */
-
-/* definitions for test and init register - AD1848_TEST_INIT */
-
-#define AD1848_CALIB_IN_PROGRESS 0x20	/* auto calibrate in progress */
-#define AD1848_DMA_REQUEST	0x10	/* DMA request in progress */
+#include "ad1848-regs.h"
 
 /* defines for codec.mode */
 

----------------------------------------------------------------------
Tania telefonia internetowa!
Sprawdz >>>  http://link.interia.pl/f1e2e

Re: [PATCH 1/2] ad1848: separate header for registers definition

[Rene Herman]

On 15-06-08 12:41, Krzysztof Helt wrote:

> This patch is a start combining ad1848_lib and cs4231_lib
> functionality. My approach is to make the cs4231_lib module
> use the ad1848_lib module. This should allow removing
> most of identical functions from the cs4231_lib and use
> the functions from the ad1848_lib directly.
> 
> If this approach is not a good idea, please drop the patch.
> 
> My current plan is to reuse the ad1848 register constants
> in the cs423x code and remove duplicated cs4231 constants.
> This should make the ad1848_lib and cs4231_lib easier to
> compare for differences.
> 
> The next step is to use the snd_cs4231 structure instead of 
> the snd_ad1848 structure inside the ad1848_lib. This will make
> functions from the ad1848_lib taking the same arguments
> as the functions in the cs4231_lib.
> 
> The last step would be to export needed functions from the
> ad1848_lib and use them directly in the cs4231_lib.
> 
> This should reduce the size of the cs4231_lib while preserving
> smaller the ad1848_lib for driver which uses only ad1848
> functionality.

I believe it would be going a bit overboard to actually make these two 
dependent libs. Unification would seem a good plan probably but as far 
as I'm concerned one lib to drive all these chips should do really.

cs4321_lib isn't all that much bigger. The seperation does make sense at 
the source level, but I'd personally go for a single library module (*) 
built from multiple source files. These chips aren't used by embedded 
these days and even if they were the sizes we are talking about are 
laughable in today's terms.

At a later stage, cs4236_lib could be folded in as well...

(*) perhaps under sound/drivers/wss, or sound/isa/lib/wss (or if you 
don't care for the wss name, something else; the location is more the 
point).

Rene.

Re: [PATCH 1/2] ad1848: separate header for registers definition

[Krzysztof Helt]

On Sun, 15 Jun 2008 18:07:02 +0200
Rene Herman <rene.herman@keyaccess.nl> wrote:

> On 15-06-08 12:41, Krzysztof Helt wrote:
> 
> > This patch is a start combining ad1848_lib and cs4231_lib
> > functionality. My approach is to make the cs4231_lib module
> > use the ad1848_lib module. This should allow removing
> > most of identical functions from the cs4231_lib and use
> > the functions from the ad1848_lib directly.
> > 
> > If this approach is not a good idea, please drop the patch.
> > 
> > My current plan is to reuse the ad1848 register constants
> > in the cs423x code and remove duplicated cs4231 constants.
> > This should make the ad1848_lib and cs4231_lib easier to
> > compare for differences.
> > 
> > The next step is to use the snd_cs4231 structure instead of 
> > the snd_ad1848 structure inside the ad1848_lib. This will make
> > functions from the ad1848_lib taking the same arguments
> > as the functions in the cs4231_lib.
> > 
> > The last step would be to export needed functions from the
> > ad1848_lib and use them directly in the cs4231_lib.
> > 
> > This should reduce the size of the cs4231_lib while preserving
> > smaller the ad1848_lib for driver which uses only ad1848
> > functionality.
> 
> I believe it would be going a bit overboard to actually make these two 
> dependent libs. Unification would seem a good plan probably but as far 
> as I'm concerned one lib to drive all these chips should do really.
> 

It can be done this way as well.

> (*) perhaps under sound/drivers/wss, or sound/isa/lib/wss (or if you 
> don't care for the wss name, something else; the location is more the 
> point).
> 

I like the wss name.

Takashi, please drop my patches. If the ad1848 and cs4231 should be unified
a correct way is to use cs4231 structures and constants in the ad1848 library.
Then it will be easy to merge them.

Regards,
Krzysztof

----------------------------------------------------------------------
Mieszkasz w USA? Zobacz portal dla Polonii!
kliknij >>> http://link.interia.pl/f1e36

Re: [PATCH 1/2] ad1848: separate header for registers definition

[Takashi Iwai]

At Sun, 15 Jun 2008 19:26:53 +0200,
Krzysztof Helt wrote:
> 
> On Sun, 15 Jun 2008 18:07:02 +0200
> Rene Herman <rene.herman@keyaccess.nl> wrote:
> 
> > On 15-06-08 12:41, Krzysztof Helt wrote:
> > 
> > > This patch is a start combining ad1848_lib and cs4231_lib
> > > functionality. My approach is to make the cs4231_lib module
> > > use the ad1848_lib module. This should allow removing
> > > most of identical functions from the cs4231_lib and use
> > > the functions from the ad1848_lib directly.
> > > 
> > > If this approach is not a good idea, please drop the patch.
> > > 
> > > My current plan is to reuse the ad1848 register constants
> > > in the cs423x code and remove duplicated cs4231 constants.
> > > This should make the ad1848_lib and cs4231_lib easier to
> > > compare for differences.
> > > 
> > > The next step is to use the snd_cs4231 structure instead of 
> > > the snd_ad1848 structure inside the ad1848_lib. This will make
> > > functions from the ad1848_lib taking the same arguments
> > > as the functions in the cs4231_lib.
> > > 
> > > The last step would be to export needed functions from the
> > > ad1848_lib and use them directly in the cs4231_lib.
> > > 
> > > This should reduce the size of the cs4231_lib while preserving
> > > smaller the ad1848_lib for driver which uses only ad1848
> > > functionality.
> > 
> > I believe it would be going a bit overboard to actually make these two 
> > dependent libs. Unification would seem a good plan probably but as far 
> > as I'm concerned one lib to drive all these chips should do really.
> > 
> 
> It can be done this way as well.
> 
> > (*) perhaps under sound/drivers/wss, or sound/isa/lib/wss (or if you 
> > don't care for the wss name, something else; the location is more the 
> > point).
> > 
> 
> I like the wss name.
> 
> Takashi, please drop my patches. If the ad1848 and cs4231 should be unified
> a correct way is to use cs4231 structures and constants in the ad1848 library.
> Then it will be easy to merge them.

OK, it makes sense.

thanks,

Takashi