On 2019-06-24 15:08, derek.fang@xxxxxxxxxxx wrote:
+static int rt1308_reg_init(struct snd_soc_component *component)
+{
+ struct rt1308_priv *rt1308 = snd_soc_component_get_drvdata(component);
+
+ regmap_multi_reg_write(rt1308->regmap, init_list, RT1308_INIT_REG_LEN);
+ return 0;
s/return 0/return regmap_multi_reg_write/ perhaps?
+static int rt1308_classd_event(struct snd_soc_dapm_widget *w,
+ struct snd_kcontrol *kcontrol, int event)
+{
+ struct snd_soc_component *component =
+ snd_soc_dapm_to_component(w->dapm);
+
+ switch (event) {
+ case SND_SOC_DAPM_POST_PMU:
+ msleep(30);
+ snd_soc_component_update_bits(component, RT1308_POWER_STATUS,
+ RT1308_POW_PDB_REG_BIT, RT1308_POW_PDB_REG_BIT);
+ msleep(40);
+ break;
+ case SND_SOC_DAPM_PRE_PMD:
+ snd_soc_component_update_bits(component, RT1308_POWER_STATUS,
+ RT1308_POW_PDB_REG_BIT, 0);
+ usleep_range(150000, 200000);
+ break;
+
+ default:
+ return 0;
+ }
+
+ return 0;
Redundant return.
+static int rt1308_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)
+{
+ struct snd_soc_component *component = dai->component;
+ struct rt1308_priv *rt1308 = snd_soc_component_get_drvdata(component);
+ unsigned int val_len = 0, val_clk, mask_clk;
+ int pre_div, bclk_ms, frame_size;
+
+ rt1308->lrck = params_rate(params);
+ pre_div = rt1308_get_clk_info(rt1308->sysclk, rt1308->lrck);
+ if (pre_div < 0) {
+ dev_err(component->dev,
+ "Unsupported clock setting %d\n", rt1308->lrck);
+ return -EINVAL;
+ }
+
+ frame_size = snd_soc_params_to_frame_size(params);
+ if (frame_size < 0) {
+ dev_err(component->dev, "Unsupported frame size: %d\n",
+ frame_size);
+ return -EINVAL;
+ }
+
+ bclk_ms = frame_size > 32;
+ rt1308->bclk = rt1308->lrck * (32 << bclk_ms);
+
+ dev_dbg(component->dev, "bclk_ms is %d and pre_div is %d for iis %d\n",
+ bclk_ms, pre_div, dai->id);
+
+ dev_dbg(component->dev, "lrck is %dHz and pre_div is %d for iis %d\n",
+ rt1308->lrck, pre_div, dai->id);
+
+ switch (params_width(params)) {
+ case 16:
+ val_len |= RT1308_I2S_DL_SEL_16B;
+ break;
+ case 20:
+ val_len |= RT1308_I2S_DL_SEL_20B;
+ break;
+ case 24:
+ val_len |= RT1308_I2S_DL_SEL_24B;
+ break;
+ case 8:
+ val_len |= RT1308_I2S_DL_SEL_8B;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ switch (dai->id) {
+ case RT1308_AIF1:
+ mask_clk = RT1308_DIV_FS_SYS_MASK;
+ val_clk = pre_div << RT1308_DIV_FS_SYS_SFT;
+ snd_soc_component_update_bits(component,
+ RT1308_I2S_SET_2, RT1308_I2S_DL_SEL_MASK,
+ val_len);
+ break;
+ default:
+ dev_err(component->dev, "Invalid dai->id: %d\n", dai->id);
+ return -EINVAL;
+ }
So, either id == RT1308_AIF1 -or- func collapses. I'd suggest a refactor:
if (dai->id != RT1308_AIF1)
// collapse
// do the stuff
Moreover, this block (if-statement) should probably be moved to the top
of the func. Why bother with any ops if dai->id does not match.
+
+ snd_soc_component_update_bits(component, RT1308_CLK_1,
+ mask_clk, val_clk);
is mask_clk local even needed? It could be simply inlined..
+
+ return 0;
+}
+static int rt1308_set_dai_fmt(struct snd_soc_dai *dai, unsigned int fmt)
+{
+ struct snd_soc_component *component = dai->component;
+ struct rt1308_priv *rt1308 = snd_soc_component_get_drvdata(component);
+ unsigned int reg_val = 0, reg1_val = 0;
+
+ switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+ case SND_SOC_DAIFMT_CBS_CFS:
+ rt1308->master = 0;
+ break;
+ default:
+ return -EINVAL;
+ } > +
+ switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+ case SND_SOC_DAIFMT_I2S:
+ break;
+ case SND_SOC_DAIFMT_LEFT_J:
+ reg_val |= RT1308_I2S_DF_SEL_LEFT;
+ break;
+ case SND_SOC_DAIFMT_DSP_A:
+ reg_val |= RT1308_I2S_DF_SEL_PCM_A;
+ break;
+ case SND_SOC_DAIFMT_DSP_B:
+ reg_val |= RT1308_I2S_DF_SEL_PCM_B;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+ case SND_SOC_DAIFMT_NB_NF:
+ break;
+ case SND_SOC_DAIFMT_IB_NF:
+ reg1_val |= RT1308_I2S_BCLK_INV;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ switch (dai->id) {
+ case RT1308_AIF1:
+ snd_soc_component_update_bits(component,
+ RT1308_I2S_SET_1, RT1308_I2S_DF_SEL_MASK,
+ reg_val);
+ snd_soc_component_update_bits(component,
+ RT1308_I2S_SET_2, RT1308_I2S_BCLK_MASK,
+ reg1_val);
+ break;
+ default:
+ dev_err(component->dev, "Invalid dai->id: %d\n", dai->id);
+ return -EINVAL;
+ }
Same treatment as for rt1308_hw_params could be applied.
+static int rt1308_probe(struct snd_soc_component *component)
+{
+ struct rt1308_priv *rt1308 = snd_soc_component_get_drvdata(component);
+
+ rt1308->component = component;
+
+ rt1308_reg_init(component);
+
+ return 0;
s/return 0/return rt1308_reg_init/ perhaps?
+#if defined(CONFIG_OF)
+static const struct of_device_id rt1308_of_match[] = {
+ { .compatible = "realtek,rt1308", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, rt1308_of_match);
+#endif
+
+#ifdef CONFIG_ACPI
+static struct acpi_device_id rt1308_acpi_match[] = {
+ {"10EC1308", 0,},
+ {},
+};
+MODULE_DEVICE_TABLE(acpi, rt1308_acpi_match);
+#endif
+
+static const struct i2c_device_id rt1308_i2c_id[] = {
+ { "rt1308", 0 },
+ { }
+};
Each of these 3 const arrays above has different spacing of their
elements. It would look better if same style was chosen for all of em.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
