Re: [PATCH v3 1/2] ASoC: codecs: Add support for AK5558 ADC driver
On Lu, 2018-02-12 at 12:02 +, Mark Brown wrote: > On Mon, Feb 05, 2018 at 07:01:54PM +0200, Daniel Baluta wrote: > > > > AK5558 is a 32-bit, 768 kHZ sampling, differential input ADC > > for digital audio systems. > > > > --- /dev/null > > +++ b/sound/soc/codecs/ak5558.c > > @@ -0,0 +1,618 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * Audio driver for AK5558 ADC > Please don't mix C++ and C style comments - just make the entire comment > C++. > Sure. Will use: // SPDX-License-Identifier: GPL-2.0 > > > > +static const char * const tdm_texts[] = { > > + "Off", "TDM128", "TDM256", "TDM512", > > +}; > This looks like it should be a set_tdm_slot() operation, and indeed > set_tdm_slot() appears to be implemented and duplicate this. > Yup, will remove this. At first there was no set_tdm_slot. > > > > +static const char * const dsdon_texts[] = { > > + "PCM", "DSD", > > +}; > This looks like it's setting the DAI format? Ditto. Will remove. > > > > > + SND_SOC_DAPM_MUX("AK5558 Ch1 Enable", SND_SOC_NOPM, 0, 0, > > + &ak5558_channel1_mux_control), > On/off controls should be switches not muxes, though if this is just > selecting which channels are active (rather than a mute) I'd not expect > it to be a control at all - the board can say if inputs are disabled. OK, agree that if we want to have a way to select which channels are active we should use a switch. Will remove this control for now. > > > > > +static int ak5558_set_mcki(struct snd_soc_codec *codec, int fs, int rclk) > > +{ > > + u8 mode; > > + > > + mode = snd_soc_read(codec, AK5558_02_CONTROL1); > > + mode &= ~AK5558_CKS; > > + mode |= AK5558_CKS_AUTO; > > + > > + snd_soc_update_bits(codec, AK5558_02_CONTROL1, AK5558_CKS, mode); > > + > > + return 0; > > +} > This appears to just ignore the parameters? These are left overs from a v1 cleanup. Will fix. thanks Mark! Will send v4 asap. thanks, Daniel.
Re: [PATCH v3 1/2] ASoC: codecs: Add support for AK5558 ADC driver
On Mon, Feb 05, 2018 at 07:01:54PM +0200, Daniel Baluta wrote: > AK5558 is a 32-bit, 768 kHZ sampling, differential input ADC > for digital audio systems. > --- /dev/null > +++ b/sound/soc/codecs/ak5558.c > @@ -0,0 +1,618 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Audio driver for AK5558 ADC Please don't mix C++ and C style comments - just make the entire comment C++. > +static const char * const tdm_texts[] = { > + "Off", "TDM128", "TDM256", "TDM512", > +}; This looks like it should be a set_tdm_slot() operation, and indeed set_tdm_slot() appears to be implemented and duplicate this. > +static const char * const dsdon_texts[] = { > + "PCM", "DSD", > +}; This looks like it's setting the DAI format? > + SND_SOC_DAPM_MUX("AK5558 Ch1 Enable", SND_SOC_NOPM, 0, 0, > + &ak5558_channel1_mux_control), On/off controls should be switches not muxes, though if this is just selecting which channels are active (rather than a mute) I'd not expect it to be a control at all - the board can say if inputs are disabled. > +static int ak5558_set_mcki(struct snd_soc_codec *codec, int fs, int rclk) > +{ > + u8 mode; > + > + mode = snd_soc_read(codec, AK5558_02_CONTROL1); > + mode &= ~AK5558_CKS; > + mode |= AK5558_CKS_AUTO; > + > + snd_soc_update_bits(codec, AK5558_02_CONTROL1, AK5558_CKS, mode); > + > + return 0; > +} This appears to just ignore the parameters? > +static int ak5558_set_dai_mute(struct snd_soc_dai *dai, int mute) > +{ > + struct snd_soc_codec *codec = dai->codec; > + struct ak5558_priv *ak5558 = snd_soc_codec_get_drvdata(codec); > + int ndt = 0; > + > + if (!mute) > + return 0; > + > + if (ak5558->fs != 0) > + ndt = 583000 / ak5558->fs; > + > + msleep(max(ndt, 5)); > + > + return 0; > +} This doesn't appear to interact with the device at all. > +static int ak5558_set_bias_level(struct snd_soc_codec *codec, > + enum snd_soc_bias_level level) > +{ > + switch (level) { > + case SND_SOC_BIAS_ON: > + case SND_SOC_BIAS_PREPARE: > + case SND_SOC_BIAS_STANDBY: > + break; > + case SND_SOC_BIAS_OFF: > + snd_soc_write(codec, AK5558_00_POWER_MANAGEMENT1, 0x00); > + break; > + } I'd expect there to be symmetry here - if we disable things when transitioning to _OFF we should enable them when transitioning out. > + reg = snd_soc_read(codec, AK5558_03_CONTROL2); > + reg &= ~AK5558_MODE_BITS; > + reg |= tdm_mode; > + snd_soc_write(codec, AK5558_03_CONTROL2, reg); snd_soc_update_bits(). signature.asc Description: PGP signature
Re: [PATCH v3 1/2] ASoC: codecs: Add support for AK5558 ADC driver
On Mon, Feb 5, 2018 at 7:01 PM, Daniel Baluta wrote: > AK5558 is a 32-bit, 768 kHZ sampling, differential input ADC > for digital audio systems. > > Datasheet is available at: > > https://www.akm.com/akm/en/file/datasheet/AK5558VN.pdf > > Initial patch includes support for normal and TDM modes. > > Signed-off-by: Junichi Wakasugi > [initial coding for 3.18 kernel] > Signed-off-by: Mihai Serban > [cleanups and porting to 4.9 kernel] > Signed-off-by: Shengjiu Wang > [tdm support] > Signed-off-by: Daniel Baluta > [pm support, cleanups and porting to latest kernel] Reviewed-by: Andy Shevchenko > --- > sound/soc/codecs/Kconfig | 6 + > sound/soc/codecs/Makefile | 2 + > sound/soc/codecs/ak5558.c | 618 > ++ > sound/soc/codecs/ak5558.h | 52 > 4 files changed, 678 insertions(+) > create mode 100644 sound/soc/codecs/ak5558.c > create mode 100644 sound/soc/codecs/ak5558.h > > diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig > index 2b331f7..c29728c 100644 > --- a/sound/soc/codecs/Kconfig > +++ b/sound/soc/codecs/Kconfig > @@ -42,6 +42,7 @@ config SND_SOC_ALL_CODECS > select SND_SOC_AK4642 if I2C > select SND_SOC_AK4671 if I2C > select SND_SOC_AK5386 > + select SND_SOC_AK5558 if I2C > select SND_SOC_ALC5623 if I2C > select SND_SOC_ALC5632 if I2C > select SND_SOC_BT_SCO > @@ -398,6 +399,11 @@ config SND_SOC_AK4671 > config SND_SOC_AK5386 > tristate "AKM AK5638 CODEC" > > +config SND_SOC_AK5558 > + tristate "AKM AK5558 CODEC" > + depends on I2C > + select REGMAP_I2C > + > config SND_SOC_ALC5623 > tristate "Realtek ALC5623 CODEC" > depends on I2C > diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile > index da15713..3e71d40 100644 > --- a/sound/soc/codecs/Makefile > +++ b/sound/soc/codecs/Makefile > @@ -34,6 +34,7 @@ snd-soc-ak4641-objs := ak4641.o > snd-soc-ak4642-objs := ak4642.o > snd-soc-ak4671-objs := ak4671.o > snd-soc-ak5386-objs := ak5386.o > +snd-soc-ak5558-objs := ak5558.o > snd-soc-arizona-objs := arizona.o > snd-soc-bt-sco-objs := bt-sco.o > snd-soc-cq93vc-objs := cq93vc.o > @@ -277,6 +278,7 @@ obj-$(CONFIG_SND_SOC_AK4641)+= snd-soc-ak4641.o > obj-$(CONFIG_SND_SOC_AK4642) += snd-soc-ak4642.o > obj-$(CONFIG_SND_SOC_AK4671) += snd-soc-ak4671.o > obj-$(CONFIG_SND_SOC_AK5386) += snd-soc-ak5386.o > +obj-$(CONFIG_SND_SOC_AK5558) += snd-soc-ak5558.o > obj-$(CONFIG_SND_SOC_ALC5623)+= snd-soc-alc5623.o > obj-$(CONFIG_SND_SOC_ALC5632) += snd-soc-alc5632.o > obj-$(CONFIG_SND_SOC_ARIZONA) += snd-soc-arizona.o > diff --git a/sound/soc/codecs/ak5558.c b/sound/soc/codecs/ak5558.c > new file mode 100644 > index 000..c1eed82 > --- /dev/null > +++ b/sound/soc/codecs/ak5558.c > @@ -0,0 +1,618 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Audio driver for AK5558 ADC > + * > + * Copyright (C) 2015 Asahi Kasei Microdevices Corporation > + * Copyright 2018 NXP > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "ak5558.h" > + > +/* AK5558 Codec Private Data */ > +struct ak5558_priv { > + struct snd_soc_codec codec; > + struct regmap *regmap; > + struct i2c_client *i2c; > + int fs; /* Sampling Frequency */ > + int rclk; /* Master Clock */ > + struct gpio_desc *reset_gpiod; /* Reset & Power down GPIO */ > + int slots; > + int slot_width; > +}; > + > +/* ak5558 register cache & default register settings */ > +static const struct reg_default ak5558_reg[] = { > + { 0x0, 0xFF }, /* 0x00AK5558_00_POWER_MANAGEMENT1 */ > + { 0x1, 0x01 }, /* 0x01AK5558_01_POWER_MANAGEMENT2 */ > + { 0x2, 0x01 }, /* 0x02AK5558_02_CONTROL1 */ > + { 0x3, 0x00 }, /* 0x03AK5558_03_CONTROL2 */ > + { 0x4, 0x00 }, /* 0x04AK5558_04_CONTROL3 */ > + { 0x5, 0x00 } /* 0x05AK5558_05_DSD */ > +}; > + > +static const char * const mono_texts[] = { > + "8 Slot", "2 Slot", "4 Slot", "1 Slot", > +}; > + > +static const struct soc_enum ak5558_mono_enum[] = { > + SOC_ENUM_SINGLE(AK5558_01_POWER_MANAGEMENT2, 1, > + ARRAY_SIZE(mono_texts), mono_texts) > +}; > + > +static const char * const tdm_texts[] = { > + "Off", "TDM128", "TDM256", "TDM512", > +}; > + > +static const char * const digfil_texts[] = { > + "Sharp Roll-Off", "Show Roll-Off", > + "Short Delay Sharp Roll-Off", "Short Delay Show Roll-Off", > +}; > + > +static const struct soc_enum ak5558_adcset_enum[] = { > + SOC_ENUM_SINGLE(AK5558_03_CONTROL2, 5, > + ARRAY_SIZE(tdm_texts), tdm_texts), > + SOC_ENUM_SINGLE(AK5558_04_CONTROL3, 0
Re: [PATCH v3 1/2] ASoC: codecs: Add support for AK5558 ADC driver
On Mon, Feb 5, 2018 at 3:01 PM, Daniel Baluta wrote: > AK5558 is a 32-bit, 768 kHZ sampling, differential input ADC > for digital audio systems. > > Datasheet is available at: > > https://www.akm.com/akm/en/file/datasheet/AK5558VN.pdf > > Initial patch includes support for normal and TDM modes. > > Signed-off-by: Junichi Wakasugi > [initial coding for 3.18 kernel] > Signed-off-by: Mihai Serban > [cleanups and porting to 4.9 kernel] > Signed-off-by: Shengjiu Wang > [tdm support] > Signed-off-by: Daniel Baluta > [pm support, cleanups and porting to latest kernel] Reviewed-by: Fabio Estevam
[PATCH v3 1/2] ASoC: codecs: Add support for AK5558 ADC driver
AK5558 is a 32-bit, 768 kHZ sampling, differential input ADC for digital audio systems. Datasheet is available at: https://www.akm.com/akm/en/file/datasheet/AK5558VN.pdf Initial patch includes support for normal and TDM modes. Signed-off-by: Junichi Wakasugi [initial coding for 3.18 kernel] Signed-off-by: Mihai Serban [cleanups and porting to 4.9 kernel] Signed-off-by: Shengjiu Wang [tdm support] Signed-off-by: Daniel Baluta [pm support, cleanups and porting to latest kernel] --- sound/soc/codecs/Kconfig | 6 + sound/soc/codecs/Makefile | 2 + sound/soc/codecs/ak5558.c | 618 ++ sound/soc/codecs/ak5558.h | 52 4 files changed, 678 insertions(+) create mode 100644 sound/soc/codecs/ak5558.c create mode 100644 sound/soc/codecs/ak5558.h diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 2b331f7..c29728c 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -42,6 +42,7 @@ config SND_SOC_ALL_CODECS select SND_SOC_AK4642 if I2C select SND_SOC_AK4671 if I2C select SND_SOC_AK5386 + select SND_SOC_AK5558 if I2C select SND_SOC_ALC5623 if I2C select SND_SOC_ALC5632 if I2C select SND_SOC_BT_SCO @@ -398,6 +399,11 @@ config SND_SOC_AK4671 config SND_SOC_AK5386 tristate "AKM AK5638 CODEC" +config SND_SOC_AK5558 + tristate "AKM AK5558 CODEC" + depends on I2C + select REGMAP_I2C + config SND_SOC_ALC5623 tristate "Realtek ALC5623 CODEC" depends on I2C diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index da15713..3e71d40 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -34,6 +34,7 @@ snd-soc-ak4641-objs := ak4641.o snd-soc-ak4642-objs := ak4642.o snd-soc-ak4671-objs := ak4671.o snd-soc-ak5386-objs := ak5386.o +snd-soc-ak5558-objs := ak5558.o snd-soc-arizona-objs := arizona.o snd-soc-bt-sco-objs := bt-sco.o snd-soc-cq93vc-objs := cq93vc.o @@ -277,6 +278,7 @@ obj-$(CONFIG_SND_SOC_AK4641)+= snd-soc-ak4641.o obj-$(CONFIG_SND_SOC_AK4642) += snd-soc-ak4642.o obj-$(CONFIG_SND_SOC_AK4671) += snd-soc-ak4671.o obj-$(CONFIG_SND_SOC_AK5386) += snd-soc-ak5386.o +obj-$(CONFIG_SND_SOC_AK5558) += snd-soc-ak5558.o obj-$(CONFIG_SND_SOC_ALC5623)+= snd-soc-alc5623.o obj-$(CONFIG_SND_SOC_ALC5632) += snd-soc-alc5632.o obj-$(CONFIG_SND_SOC_ARIZONA) += snd-soc-arizona.o diff --git a/sound/soc/codecs/ak5558.c b/sound/soc/codecs/ak5558.c new file mode 100644 index 000..c1eed82 --- /dev/null +++ b/sound/soc/codecs/ak5558.c @@ -0,0 +1,618 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Audio driver for AK5558 ADC + * + * Copyright (C) 2015 Asahi Kasei Microdevices Corporation + * Copyright 2018 NXP + */ + +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +#include "ak5558.h" + +/* AK5558 Codec Private Data */ +struct ak5558_priv { + struct snd_soc_codec codec; + struct regmap *regmap; + struct i2c_client *i2c; + int fs; /* Sampling Frequency */ + int rclk; /* Master Clock */ + struct gpio_desc *reset_gpiod; /* Reset & Power down GPIO */ + int slots; + int slot_width; +}; + +/* ak5558 register cache & default register settings */ +static const struct reg_default ak5558_reg[] = { + { 0x0, 0xFF }, /* 0x00AK5558_00_POWER_MANAGEMENT1 */ + { 0x1, 0x01 }, /* 0x01AK5558_01_POWER_MANAGEMENT2 */ + { 0x2, 0x01 }, /* 0x02AK5558_02_CONTROL1 */ + { 0x3, 0x00 }, /* 0x03AK5558_03_CONTROL2 */ + { 0x4, 0x00 }, /* 0x04AK5558_04_CONTROL3 */ + { 0x5, 0x00 } /* 0x05AK5558_05_DSD */ +}; + +static const char * const mono_texts[] = { + "8 Slot", "2 Slot", "4 Slot", "1 Slot", +}; + +static const struct soc_enum ak5558_mono_enum[] = { + SOC_ENUM_SINGLE(AK5558_01_POWER_MANAGEMENT2, 1, + ARRAY_SIZE(mono_texts), mono_texts) +}; + +static const char * const tdm_texts[] = { + "Off", "TDM128", "TDM256", "TDM512", +}; + +static const char * const digfil_texts[] = { + "Sharp Roll-Off", "Show Roll-Off", + "Short Delay Sharp Roll-Off", "Short Delay Show Roll-Off", +}; + +static const struct soc_enum ak5558_adcset_enum[] = { + SOC_ENUM_SINGLE(AK5558_03_CONTROL2, 5, + ARRAY_SIZE(tdm_texts), tdm_texts), + SOC_ENUM_SINGLE(AK5558_04_CONTROL3, 0, + ARRAY_SIZE(digfil_texts), digfil_texts), +}; + +static const char * const dsdon_texts[] = { + "PCM", "DSD", +}; + +static const char * const dsdsel_texts[] = { + "64fs", "128fs", "256fs" +}; + +static const char * const dckb_texts[] = { + "Falling", "Rising", +}; + +static const char * const dcks_texts[] = { + "512fs", "