On Mon, Jul 26, 2021 at 05:34:37PM -0500, David Rhodes wrote:
This looks mostly good, a few pretty small things below:
> +++ b/sound/soc/codecs/cs35l41-i2c.c
> @@ -0,0 +1,116 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
C files need to have a C++ style SPDX header, please make the entire
comment a C++ one so things look more consistent.
> +static const struct snd_kcontrol_new cs35l41_aud_controls[] = {
> + SOC_SINGLE_SX_TLV("Digital PCM Volume", CS35L41_AMP_DIG_VOL_CTRL,
> + 3, 0x4CF, 0x391, dig_vol_tlv),
> + SOC_SINGLE_TLV("AMP PCM Gain", CS35L41_AMP_GAIN_CTRL, 5, 0x14, 0,
> + amp_gain_tlv),
Volume controls need to end in Volume like the one immediately above,
see control-names.rst.
> + /* returning NULL can be an option if in stereo mode */
> + cs35l41->reset_gpio = devm_gpiod_get_optional(cs35l41->dev, "reset",
> + GPIOD_OUT_LOW);
Was this included in the DT binding?
> + ret = devm_snd_soc_register_component(cs35l41->dev,
> + &soc_component_dev_cs35l41,
> + cs35l41_dai, ARRAY_SIZE(cs35l41_dai));
> + if (ret < 0) {
> + dev_err(cs35l41->dev, "%s: Register codec failed\n", __func__);
> + goto err;
> + }
> +
> + ret = cs35l41_set_pdata(cs35l41);
> + if (ret < 0) {
> + dev_err(cs35l41->dev, "%s: Set pdata failed\n", __func__);
> + goto err;
> + }
Are you sure it's safe to register the device before applying the
platform data configuration - as soon as the device is registered the
sound card might become visible?
Attachment:
signature.asc
Description: PGP signature
