Re: [PATCH v3 1/2] ASoC: codecs: Add support for AK4458 DAC driver

2018-02-16 Thread Mark Brown
On Wed, Feb 14, 2018 at 03:21:06PM +0200, Cosmin-Gabriel Samoila wrote:

> --- /dev/null
> +++ b/sound/soc/codecs/ak4458.c
> @@ -0,0 +1,659 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Audio driver for AK4458 DAC
> + *

Please don't mix C and C++ comments line this, just make the entire
block a C++ comment.  Since this looks otherwise good I'll apply this,
please send a followup patch fixing the above.


signature.asc
Description: PGP signature


Re: [PATCH v3 1/2] ASoC: codecs: Add support for AK4458 DAC driver

2018-02-14 Thread Philippe Ombredanne
On Wed, Feb 14, 2018 at 2:21 PM, Cosmin-Gabriel Samoila
 wrote:
> The AK4458 is a 32-bit 8ch Premium DAC that corresponds
> to a 768kHz PCM input and an 11.2MHz DSD input at maximum.
> It supports I2S, DSD and TDM modes with 24 or 32 bit MSB
> or 16, 24, 32 LSB formats. Its datasheet is available here:
> https://www.akm.com/akm/en/file/datasheet/AK4458VN.pdf
>
> Signed-off-by: Junichi Wakasugi 
> Signed-off-by: Mihai Serban 
> Signed-off-by: Shengjiu Wang 
> Signed-off-by: Cosmin-Gabriel Samoila 
> ---

> --- /dev/null
> +++ b/sound/soc/codecs/ak4458.c
> @@ -0,0 +1,659 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Audio driver for AK4458 DAC
> + *
> + * Copyright (C) 2016 Asahi Kasei Microdevices Corporation
> + * Copyright 2018 NXP

Thanks for using the proper SPDX tag here yet see my comments below.



> +MODULE_AUTHOR("Junichi Wakasugi ");
> +MODULE_AUTHOR("Mihai Serban ");
> +MODULE_DESCRIPTION("ASoC AK4458 DAC driver");
> +MODULE_LICENSE("GPL");

This means GPL-2.0+ per module.h and therefore does not match your
GPL-2.0 SPDX tag above.
Go one way or the other, but not both way please.

> diff --git a/sound/soc/codecs/ak4458.h b/sound/soc/codecs/ak4458.h
> new file mode 100644
> index 000..16d9d22
> --- /dev/null
> +++ b/sound/soc/codecs/ak4458.h
> @@ -0,0 +1,86 @@
> +// SPDX-License-Identifier: GPL-2.0

As weird as it sounds the style should be plain /*
SPDX-License-Identifier: GPL-2.0 /* here per [1]

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/license-rules.rst

-- 
Cordially
Philippe Ombredanne


Re: [PATCH v3 1/2] ASoC: codecs: Add support for AK4458 DAC driver

2018-02-14 Thread Andy Shevchenko
On Wed, Feb 14, 2018 at 3:21 PM, Cosmin-Gabriel Samoila
 wrote:
> The AK4458 is a 32-bit 8ch Premium DAC that corresponds
> to a 768kHz PCM input and an 11.2MHz DSD input at maximum.
> It supports I2S, DSD and TDM modes with 24 or 32 bit MSB
> or 16, 24, 32 LSB formats. Its datasheet is available here:
> https://www.akm.com/akm/en/file/datasheet/AK4458VN.pdf
>

Looks fine to me

Reviewed-by: Andy Shevchenko 

One nit below.

> Signed-off-by: Junichi Wakasugi 
> Signed-off-by: Mihai Serban 
> Signed-off-by: Shengjiu Wang 
> Signed-off-by: Cosmin-Gabriel Samoila 
> ---
>  sound/soc/codecs/Kconfig  |   6 +
>  sound/soc/codecs/Makefile |   2 +
>  sound/soc/codecs/ak4458.c | 659 
> ++
>  sound/soc/codecs/ak4458.h |  86 ++
>  4 files changed, 753 insertions(+)
>  create mode 100644 sound/soc/codecs/ak4458.c
>  create mode 100644 sound/soc/codecs/ak4458.h
>
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index 2b331f7..7f6fb5e 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -35,6 +35,7 @@ config SND_SOC_ALL_CODECS
> select SND_SOC_ADAU7002
> select SND_SOC_ADS117X
> select SND_SOC_AK4104 if SPI_MASTER
> +   select SND_SOC_AK4458 if I2C
> select SND_SOC_AK4535 if I2C
> select SND_SOC_AK4554
> select SND_SOC_AK4613 if I2C
> @@ -375,6 +376,11 @@ config SND_SOC_AK4104
> tristate "AKM AK4104 CODEC"
> depends on SPI_MASTER
>
> +config SND_SOC_AK4458
> +   tristate "AKM AK4458 CODEC"
> +   depends on I2C
> +   select REGMAP_I2C
> +
>  config SND_SOC_AK4535
> tristate
>
> diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
> index 4053c72..339b674 100644
> --- a/sound/soc/codecs/Makefile
> +++ b/sound/soc/codecs/Makefile
> @@ -27,6 +27,7 @@ snd-soc-adav801-objs := adav801.o
>  snd-soc-adav803-objs := adav803.o
>  snd-soc-ads117x-objs := ads117x.o
>  snd-soc-ak4104-objs := ak4104.o
> +snd-soc-ak4458-objs := ak4458.o
>  snd-soc-ak4535-objs := ak4535.o
>  snd-soc-ak4554-objs := ak4554.o
>  snd-soc-ak4613-objs := ak4613.o
> @@ -270,6 +271,7 @@ obj-$(CONFIG_SND_SOC_ADAV801)  += snd-soc-adav801.o
>  obj-$(CONFIG_SND_SOC_ADAV803)  += snd-soc-adav803.o
>  obj-$(CONFIG_SND_SOC_ADS117X)  += snd-soc-ads117x.o
>  obj-$(CONFIG_SND_SOC_AK4104)   += snd-soc-ak4104.o
> +obj-$(CONFIG_SND_SOC_AK4458)   += snd-soc-ak4458.o
>  obj-$(CONFIG_SND_SOC_AK4535)   += snd-soc-ak4535.o
>  obj-$(CONFIG_SND_SOC_AK4554)   += snd-soc-ak4554.o
>  obj-$(CONFIG_SND_SOC_AK4613)   += snd-soc-ak4613.o
> diff --git a/sound/soc/codecs/ak4458.c b/sound/soc/codecs/ak4458.c
> new file mode 100644
> index 000..b579cda
> --- /dev/null
> +++ b/sound/soc/codecs/ak4458.c
> @@ -0,0 +1,659 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Audio driver for AK4458 DAC
> + *
> + * Copyright (C) 2016 Asahi Kasei Microdevices Corporation
> + * Copyright 2018 NXP
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Keep it sorted alphabetically.

> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Ditto.

> +
> +#include "ak4458.h"
> +
> +/* AK4458 Codec Private Data */
> +struct ak4458_priv {
> +   struct device *dev;
> +   struct regmap *regmap;
> +   struct gpio_desc *reset_gpiod;
> +   struct gpio_desc *mute_gpiod;
> +   int digfil; /* SSLOW, SD, SLOW bits */
> +   int fs; /* sampling rate */
> +   int fmt;
> +   int slots;
> +   int slot_width;
> +};
> +
> +static const struct reg_default ak4458_reg_defaults[] = {
> +   { 0x00, 0x0C }, /*  0x00AK4458_00_CONTROL1  */
> +   { 0x01, 0x22 }, /*  0x01AK4458_01_CONTROL2  */
> +   { 0x02, 0x00 }, /*  0x02AK4458_02_CONTROL3  */
> +   { 0x03, 0xFF }, /*  0x03AK4458_03_LCHATT*/
> +   { 0x04, 0xFF }, /*  0x04AK4458_04_RCHATT*/
> +   { 0x05, 0x00 }, /*  0x05AK4458_05_CONTROL4  */
> +   { 0x06, 0x00 }, /*  0x06AK4458_06_DSD1  */
> +   { 0x07, 0x03 }, /*  0x07AK4458_07_CONTROL5  */
> +   { 0x08, 0x00 }, /*  0x08AK4458_08_SOUND_CONTROL */
> +   { 0x09, 0x00 }, /*  0x09AK4458_09_DSD2  */
> +   { 0x0A, 0x0D }, /*  0x0AAK4458_0A_CONTROL6  */
> +   { 0x0B, 0x0C }, /*  0x0BAK4458_0B_CONTROL7  */
> +   { 0x0C, 0x00 }, /*  0x0CAK4458_0C_CONTROL8  */
> +   { 0x0D, 0x00 }, /*  0x0DAK4458_0D_CONTROL9  */
> +   { 0x0E, 0x50 }, /*  0x0EAK4458_0E_CONTROL10 */
> +   { 0x0F, 0xFF }, /*  0x0FAK4458_0F_L2CHATT   */
> +   { 0x10, 0xFF }, /*  0x10AK4458_10_R2CHATT   */
> +   { 0x11, 0xFF }, /*  0x11AK4458_11_L3CHATT   */
> +   { 0x12, 0xFF }, /*  0x12AK4458_12_R3CHATT   */
> +   { 0x13, 0xFF }, /*  0x13AK