Re: [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Aug 09, 2022 at 07:40:28AM +1000, Matt Flax wrote:

>     * Documented the devicetree binding

The DT binding needs to be a separate patch CCed to the DT
maintainers...

> index 000000000000..f8d845d42f42
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/src4xxx.txt
> @@ -0,0 +1,20 @@
> +Texas Instruments src4392 DT bindings

...and new DT bindings need to be in YAML format.

> +++ b/sound/soc/codecs/src4xxx-i2c.c
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Driver for SRC4XXX codecs

Please make the entire comment a *C++* one - the SPDX bit needs to be
C++ so the rest of it should be.

> +/* SRC attenuation */
> +static const DECLARE_TLV_DB_SCALE(src_tlv, -12750, 50, 0);
> +
> +static const struct snd_kcontrol_new src4xxx_controls[] = {
> +	SOC_DOUBLE_R_TLV("SRC vol",
> +		SRC4XXX_SCR_CTL_30, SRC4XXX_SCR_CTL_31, 0, 255, 1, src_tlv),
> +};

Volume controls need to end in Volume.  You should run the mixer-test
kselftest against a card with this driver, it should spot problems like
this.

> +	switch (dai->id) {
> +	case SRC4XXX_PORTB:
> +		reg = SRC4XXX_PORTB_CTL_06;
> +		break;
> +	default:
> +		reg = SRC4XXX_PORTA_CTL_04;
> +	}

Missing break in the default case.

> +					SRC4XXX_MCLK_DIV_MASK, val);
> +	} else
> +		dev_info(dai->dev, "not setting up MCLK as not master\n");
> +

{ } on both sides or neither of the else please.

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux