On Mon, Feb 03, 2025 at 06:01:17PM +0100, Nikola Jelic wrote: This looks like a new version of: https://lore.kernel.org/linux-sound/20250121230903.89808-2-nikola.jelic83@xxxxxxxxx/ but it's not identified as v2 (nor does it have a changelog of any kind, inter-version or otherwise). > +static int cmx655_i2c_probe(struct i2c_client *client) > +{ > + int ret; > + > + ret = > + cmx655_common_register_component(&client->dev, > + devm_regmap_init_i2c(client, > + &cmx655_regmap), > + client->irq); This would be more legible if you used a temporary variable to store the regmap. > + cmx655_common_unregister_component(&client->dev); > + gpiod_set_value_cansleep(cmx655_data->reset_gpio, 1); Why isn't the GPIO set in the common unregister function? > + *ndiv = 0; > + *pll_ctrl = 0; > + switch (clk_id) { > + case (CMX655_SYSCLK_RCLK): > + case (CMX655_SYSCLK_LPO): > + case (CMX655_SYSCLK_LRCLK): The brackets around the constants here are weird. > + ret = cmx655_get_sys_clk_config(cmx655_dai_data->sys_clk, > + primary_mode, srate_setting, > + &clk_src, &rdiv, &ndiv, &pll_ctrl); > + if (ret < 0) { > + dev_err(component->dev, > + "Failed to get system clock settings %i\n", ret); > + } We then proceed to use the configuration? > + } else { > + cmx655_dai_data->best_clk_running = true; > + } > + if (snd_soc_component_test_bits(component, CMX655_CLKCTRL, Some blank lines between blocks would help a lot with legibility throughout the driver. > + /* Wait for filters to settle */ > + if (snd_soc_component_test_bits > + (component, CMX655_RVF, CMX655_VF_DCBLOCK, > + CMX655_VF_DCBLOCK) == 0) { Please try to follow the kernel coding style more, here just putting one parameter per line with normal indentation is probably better. > +static const unsigned int cmx655_rate_vals[] = { > + 8000, 16000, 32000, 48000 > +}; > + > +static const struct snd_pcm_hw_constraint_list cmx655_rate_constraint = { > + .count = ARRAY_SIZE(cmx655_rate_vals), > + .list = cmx655_rate_vals, > +}; > + > +static int cmx655_dai_startup(struct snd_pcm_substream *stream, > + struct snd_soc_dai *dai) > +{ > + int ret = 0; > + > + ret = snd_pcm_hw_constraint_list(stream->runtime, 0, > + SNDRV_PCM_HW_PARAM_RATE, > + &cmx655_rate_constraint); > + return ret; > +} If the driver just supports a constant set of constraints why set them dynamically - set them in the rates for the DAI. > + SOC_SINGLE_TLV("ALC Gain Playback Volume", CMX655_ALCGAIN, 0, 12, 0, > + cmx655_alc_gain), A gain is a volume. > + SOC_SINGLE("Companding Switch", CMX655_SAIMUX, 4, 1, 0), > + SOC_ENUM("Companding Type Enum", cmx655_companding_enum), No Enum, the control is for Companding Type. > + /* Custom widgets for Mics to get them to turn on before switches */ > + {.id = snd_soc_dapm_mic, > + .name = "Left Mic", 1 > + {.id = snd_soc_dapm_mic, > + .name = "Right Mic", Define a macro for the repated pattern if one is really needed.
Attachment:
signature.asc
Description: PGP signature
