[alsa-devel] [PATCH] ASoC: ad74111: new codec driver
From: Cliff Cai cliff.cai@analog.com
Signed-off-by: Cliff Cai cliff.cai@analog.com Signed-off-by: Scott Jiang scott.jiang@analog.com Signed-off-by: Mike Frysinger vapier@gentoo.org --- sound/soc/codecs/Kconfig | 8 +++- sound/soc/codecs/Makefile | 4 ++- sound/soc/codecs/ad74111.c | 76 ++++++++++++++++++++++++++++++++++++++++++++ sound/soc/codecs/ad74111.h | 63 ++++++++++++++++++++++++++++++++++++ 4 files changed, 148 insertions(+), 3 deletions(-) create mode 100644 sound/soc/codecs/ad74111.c create mode 100644 sound/soc/codecs/ad74111.h
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index dc6d253..ee65b7c 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -16,8 +16,9 @@ config SND_SOC_ALL_CODECS select SND_SOC_AD183X if SPI_MASTER select SND_SOC_AD193X if SND_SOC_I2C_AND_SPI select SND_SOC_AD1980 if SND_SOC_AC97_BUS - select SND_SOC_ADS117X select SND_SOC_AD73311 if I2C + select SND_SOC_AD74111 + select SND_SOC_ADS117X select SND_SOC_AK4104 if SPI_MASTER select SND_SOC_AK4535 if I2C select SND_SOC_AK4642 if I2C @@ -126,7 +127,10 @@ config SND_SOC_AD1980
config SND_SOC_AD73311 tristate - + +config SND_SOC_AD74111 + tristate + config SND_SOC_ADS117X tristate
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index 44d3278..b834755 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -4,6 +4,7 @@ snd-soc-ad183x-objs := ad183x.o snd-soc-ad193x-objs := ad193x.o snd-soc-ad1980-objs := ad1980.o snd-soc-ad73311-objs := ad73311.o +snd-soc-ad74111-objs := ad74111.o snd-soc-ads117x-objs := ads117x.o snd-soc-ak4104-objs := ak4104.o snd-soc-ak4535-objs := ak4535.o @@ -90,7 +91,8 @@ obj-$(CONFIG_SND_SOC_AC97_CODEC) += snd-soc-ac97.o obj-$(CONFIG_SND_SOC_AD183X) += snd-soc-ad183x.o obj-$(CONFIG_SND_SOC_AD193X) += snd-soc-ad193x.o obj-$(CONFIG_SND_SOC_AD1980) += snd-soc-ad1980.o -obj-$(CONFIG_SND_SOC_AD73311) += snd-soc-ad73311.o +obj-$(CONFIG_SND_SOC_AD73311) += snd-soc-ad73311.o +obj-$(CONFIG_SND_SOC_AD74111) += snd-soc-ad74111.o obj-$(CONFIG_SND_SOC_ADS117X) += snd-soc-ads117x.o obj-$(CONFIG_SND_SOC_AK4104) += snd-soc-ak4104.o obj-$(CONFIG_SND_SOC_AK4535) += snd-soc-ak4535.o diff --git a/sound/soc/codecs/ad74111.c b/sound/soc/codecs/ad74111.c new file mode 100644 index 0000000..9d247ae --- /dev/null +++ b/sound/soc/codecs/ad74111.c @@ -0,0 +1,76 @@ +/* + * ALSA Soc AD74111 codec support + * + * Copyright 2009-2011 Analog Devices Inc. + * + * Licensed under the GPL-2 or later. + */ + +#include <linux/init.h> +#include <linux/slab.h> +#include <linux/module.h> +#include <linux/kernel.h> +#include <linux/device.h> +#include <sound/core.h> +#include <sound/pcm.h> +#include <sound/ac97_codec.h> +#include <sound/initval.h> +#include <sound/soc.h> + +#include "ad74111.h" + +static struct snd_soc_dai_driver ad74111_dai = { + .name = "ad74111-hifi", + .playback = { + .stream_name = "Playback", + .channels_min = 1, + .channels_max = 1, + .rates = SNDRV_PCM_RATE_8000, + .formats = SNDRV_PCM_FMTBIT_S16_LE, }, + .capture = { + .stream_name = "Capture", + .channels_min = 1, + .channels_max = 1, + .rates = SNDRV_PCM_RATE_8000, + .formats = SNDRV_PCM_FMTBIT_S16_LE, }, +}; + +static struct snd_soc_codec_driver soc_codec_dev_ad74111; + +static int ad74111_probe(struct platform_device *pdev) +{ + return snd_soc_register_codec(&pdev->dev, + &soc_codec_dev_ad74111, &ad74111_dai, 1); +} + +static int __devexit ad74111_remove(struct platform_device *pdev) +{ + snd_soc_unregister_codec(&pdev->dev); + return 0; +} + +static struct platform_driver ad74111_codec_driver = { + .driver = { + .name = "ad74111-codec", + .owner = THIS_MODULE, + }, + + .probe = ad74111_probe, + .remove = __devexit_p(ad74111_remove), +}; + +static int __init ad74111_init(void) +{ + return platform_driver_register(&ad74111_codec_driver); +} +module_init(ad74111_init); + +static void __exit ad74111_exit(void) +{ + platform_driver_unregister(&ad74111_codec_driver); +} +module_exit(ad74111_exit); + +MODULE_DESCRIPTION("ASoC AD74111 driver"); +MODULE_AUTHOR("Cliff Cai"); +MODULE_LICENSE("GPL"); diff --git a/sound/soc/codecs/ad74111.h b/sound/soc/codecs/ad74111.h new file mode 100644 index 0000000..19a8183 --- /dev/null +++ b/sound/soc/codecs/ad74111.h @@ -0,0 +1,63 @@ +/* + * definitions for AD74111 registers + * + * Copyright 2006-2011 Analog Devices Inc. + * + * Licensed under the GPL-2 or later. + */ + +#ifndef __AD74111_H__ +#define __AD74111_H__ + +#define AD_READ 0x0000 +#define AD_WRITE 0x8000 + +/* Control register A */ +#define CTRL_REG_A (0 << 11) + +#define REGA_REFAMP (1 << 2) +#define REGA_REF (1 << 3) +#define REGA_DAC (1 << 4) +#define REGA_ADC (1 << 5) +#define REGA_ADC_INPAMP (1 << 6) + +/* Control register B */ +#define CTRL_REG_B (1 << 11) + +#define REGB_FCLKDIV(x) ((x) & 0x3) +#define REGB_SCLKDIV(x) (((x) & 0x3) << 2) +#define REGB_TCLKDIV(x) (((x) & 0x3) << 4) + +/* Control register C */ +#define CTRL_REG_C (2 << 11) + +#define REGC_ADC_HP (1 << 0) +#define REGC_DAC_DEEMPH(x) (((x) & 0x3) << 1) +#define REGC_LG_DELAY (1 << 3) +#define REGC_WORD_WIDTH(x) (((x) & 0x3) << 4) + +/* Control register D */ +#define CTRL_REG_D (3 << 11) + +#define REGD_MASTER (1 << 0) +#define REGD_FDCLK (1 << 1) +#define REGD_DSP_MODE (1 << 2) +#define REGD_MIX_MODE (1 << 3) +#define REGD_MFS (1 << 9) + +/* Control register E */ +#define CTRL_REG_E (4 << 11) + +#define REGE_DAC_MUTE (1 << 0) +#define REGE_ADC_MUTE (1 << 1) +#define REGE_ADC_GAIN(x) (((x) & 0x7) << 2) +#define REGE_ADC_PEAKEN (1 << 5) + +/* Control register F */ +#define CTRL_REG_F (5 << 11) +#define REGF_DAC_VOL(x) ((x) & 0x3F) + +/* Control register G */ +#define CTRL_REG_G (6 << 11) + +#endif
On Sat, Mar 26, 2011 at 04:33:46AM -0400, Mike Frysinger wrote:
select SND_SOC_AD1980 if SND_SOC_AC97_BUS
- select SND_SOC_ADS117X select SND_SOC_AD73311 if I2C
- select SND_SOC_AD74111
- select SND_SOC_ADS117X
A few of your patches have this sort of additional change in them. While the cleanup is good it would be better to split it into a separate patch. Seeing the unrelated change slows down review and it'll also create extra merge issues when applying or cherry picking the driver back to older kernels.
+static struct platform_driver ad74111_codec_driver = {
- .driver = {
.name = "ad74111-codec",
Again, drop the -codec unless this is part of a MFD.
+/*
- definitions for AD74111 registers
Does this device really have registers? There's no reference at all to them in the driver.
+/* Control register F */ +#define CTRL_REG_F (5 << 11) +#define REGF_DAC_VOL(x) ((x) & 0x3F)
If the registers should be here they should be namespaced.
On Sat, Mar 26, 2011 at 08:21, Mark Brown wrote:
On Sat, Mar 26, 2011 at 04:33:46AM -0400, Mike Frysinger wrote:
select SND_SOC_AD1980 if SND_SOC_AC97_BUS
- select SND_SOC_ADS117X
select SND_SOC_AD73311 if I2C
- select SND_SOC_AD74111
- select SND_SOC_ADS117X
A few of your patches have this sort of additional change in them. While the cleanup is good it would be better to split it into a separate patch. Seeing the unrelated change slows down review and it'll also create extra merge issues when applying or cherry picking the driver back to older kernels.
in general i agree, but for moving literally one line i dont generally worry about it
+static struct platform_driver ad74111_codec_driver = {
- .driver = {
- .name = "ad74111-codec",
Again, drop the -codec unless this is part of a MFD.
when i looked at the history of things, it seemed like it's the ASoC maintainers sticking "-codec" on stuff. so should i go through existing drivers and revert that back ? -mike
On Sat, Mar 26, 2011 at 01:52:48PM -0400, Mike Frysinger wrote:
On Sat, Mar 26, 2011 at 08:21, Mark Brown wrote:
A few of your patches have this sort of additional change in them. While the cleanup is good it would be better to split it into a separate patch. Seeing the unrelated change slows down review and it'll also create extra merge issues when applying or cherry picking the driver back to older kernels.
in general i agree, but for moving literally one line i dont generally worry about it
It's a one line patch that totally changes the shape of the diff for that hunk. As I said above this slows down review, it's jarring as one has to stop and reverse engineer from the change if was intentional and if it is sensible. It could be some unrelated cleanup, it could be (and frequently is) context that got included in the diff by mistake.
Again, drop the -codec unless this is part of a MFD.
when i looked at the history of things, it seemed like it's the ASoC maintainers sticking "-codec" on stuff. so should i go through existing drivers and revert that back ?
Yes, they were included by mistake as a result of automated changes in the multi-component merges.
On Sat, Mar 26, 2011 at 14:09, Mark Brown wrote:
On Sat, Mar 26, 2011 at 01:52:48PM -0400, Mike Frysinger wrote:
On Sat, Mar 26, 2011 at 08:21, Mark Brown wrote:
A few of your patches have this sort of additional change in them. While the cleanup is good it would be better to split it into a separate patch. Seeing the unrelated change slows down review and it'll also create extra merge issues when applying or cherry picking the driver back to older kernels.
in general i agree, but for moving literally one line i dont generally worry about it
It's a one line patch that totally changes the shape of the diff for that hunk. As I said above this slows down review, it's jarring as one has to stop and reverse engineer from the change if was intentional and if it is sensible. It could be some unrelated cleanup, it could be (and frequently is) context that got included in the diff by mistake.
i still disagree, but considering you have the luxury of not accepting our patches, i guess it doesnt matter too much huh
Again, drop the -codec unless this is part of a MFD.
when i looked at the history of things, it seemed like it's the ASoC maintainers sticking "-codec" on stuff. so should i go through existing drivers and revert that back ?
Yes, they were included by mistake as a result of automated changes in the multi-component merges.
ok, should be easy to send out fixes for those -mike
On Sun, Mar 27, 2011 at 12:10:08AM -0400, Mike Frysinger wrote:
On Sat, Mar 26, 2011 at 14:09, Mark Brown wrote:
It's a one line patch that totally changes the shape of the diff for that hunk. As I said above this slows down review, it's jarring as one has to stop and reverse engineer from the change if was intentional and if it is sensible. It could be some unrelated cleanup, it could be (and frequently is) context that got included in the diff by mistake.
i still disagree, but considering you have the luxury of not accepting our patches, i guess it doesnt matter too much huh
It's not a luxury, believe me. Most of the issues you guys are having here seem to be process issues as much as anything else, there's a couple of things you could do which would really help make the whole thing run more smoothly:
- Submit against current -next. It looks like you're mostly submitting once a release or so, usually shortly after the release, with code based off either the release that just went out or the release before that. This means that you're going to be at least one kernel revision out of date on current APIs and best practices and sometimes means you're submitting code that won't even compile. ASoC moves pretty quickly so you really do need to check with current code.
- Follow up promptly on reviews - I'm rarely seeing updates to patches that involve any domain specific changes, and where there is followup it's often at the next release. This interacts badly with the submission against old code issues as that means you're likely to require some fairly straightforward API updates but waiting to resubmit means that you'll often end up requiring further updates next time around.
For example, the SSM2604 driver you're submitting now was originally sent last August and you sent patches for ADAV801/3, ADAU1361, ADAU1381 and ADAU1761 at the end of last year which mostly just needed fairly straightforward updates for old kernel issues but which haven't been resubmitted since. None of these devices look like they should be hard to get support integrated into mainline but the above issues are making that harder work than it needs to be.
On Sun, Mar 27, 2011 at 09:43, Mark Brown wrote:
- Follow up promptly on reviews - I'm rarely seeing updates to patches that involve any domain specific changes, and where there is followup it's often at the next release. This interacts badly with the submission against old code issues as that means you're likely to require some fairly straightforward API updates but waiting to resubmit means that you'll often end up requiring further updates next time around.
For example, the SSM2604 driver you're submitting now was originally sent last August and you sent patches for ADAV801/3, ADAU1361, ADAU1381 and ADAU1761 at the end of last year which mostly just needed fairly straightforward updates for old kernel issues but which haven't been resubmitted since. None of these devices look like they should be hard to get support integrated into mainline but the above issues are making that harder work than it needs to be.
atm, it's basically lack of developer time. technically, ADI codecs are no longer my job (they were marginally so in the past). i do any updates that i can slap together in my off time, but as soon as you start talking about real issues (such as exposing registers or programming the part differently), it's beyond my experience. most of the TLA's you throw around i have to look up ;).
so basically yes, i agree it's currently screwed up. -mike
On Mon, Mar 28, 2011 at 03:22:11AM -0400, Mike Frysinger wrote:
atm, it's basically lack of developer time. technically, ADI codecs are no longer my job (they were marginally so in the past). i do any updates that i can slap together in my off time, but as soon as you start talking about real issues (such as exposing registers or programming the part differently), it's beyond my experience. most of the TLA's you throw around i have to look up ;).
Sure, I understand the position you're in - I was more meaning ADI as a whole when I said "you" there.
participants (2)
-
Mark Brown -
Mike Frysinger