On Tue, 2022-10-04 at 11:37 +0200, AngeloGioacchino Del Regno wrote:
Il 30/09/22 16:56, Trevor Wu ha scritto:
..snip..
+static int mt8188_adda_mtkaif_init(struct mtk_base_afe *afe) +{
- struct mt8188_afe_private *afe_priv = afe->platform_priv;
- struct mtkaif_param *param = &afe_priv->mtkaif_params;
- int delay_data;
- int delay_cycle;
- unsigned int mask = 0;
- unsigned int val = 0;
- /* set rx protocol 2 & mtkaif_rxif_clkinv_adc inverse */
- mask = (MTKAIF_RXIF_CLKINV_ADC | MTKAIF_RXIF_PROTOCOL2);
- val = (MTKAIF_RXIF_CLKINV_ADC | MTKAIF_RXIF_PROTOCOL2);
- regmap_update_bits(afe->regmap, AFE_ADDA_MTKAIF_CFG0, mask,
val);
This should be regmap_set_bits(afe->regmap, AFE_ADDA_MTKAIF_CFG0, MTKAIF_RXIF_CLKINV_ADC | MTKAIF_RXIF_PROTOCOL2);
OK. I will replace all similar usages in V2.
- mask = RG_RX_PROTOCOL2;
- val = RG_RX_PROTOCOL2;
- regmap_update_bits(afe->regmap, AFE_AUD_PAD_TOP, mask, val);
regmap_set_bits() again
- if (!param->mtkaif_calibration_ok) {
dev_info(afe->dev, "%s(), calibrationfail\n", __func__);
return 0;- }
- /* set delay for ch1, ch2 */
- if (param->mtkaif_phase_cycle[MT8188_MTKAIF_MISO_0] >=
param->mtkaif_phase_cycle[MT8188_MTKAIF_MISO_1]) {delay_data = DELAY_DATA_MISO1;delay_cycle =param->mtkaif_phase_cycle[MT8188_MTKAIF_MISO_0]
param-mtkaif_phase_cycle[MT8188_MTKAIF_MISO_1];
- } else {
delay_data = DELAY_DATA_MISO0;delay_cycle =param->mtkaif_phase_cycle[MT8188_MTKAIF_MISO_1]
param-mtkaif_phase_cycle[MT8188_MTKAIF_MISO_0];
- }
- val = 0;
- mask = (MTKAIF_RXIF_DELAY_DATA | MTKAIF_RXIF_DELAY_CYCLE_MASK);
- val |= MTKAIF_RXIF_DELAY_CYCLE(delay_cycle) &
MTKAIF_RXIF_DELAY_CYCLE_MASK;val = FIELD_PREP(MTKAIF_RXIF_DELAY_CYCLE_MASK, delay_cycle);
- val |= delay_data << MTKAIF_RXIF_DELAY_DATA_SHIFT;
val |= FIELD_PREP(MTKAIF_RXIF_DELAY_DATA, delay_data);
Can you please use bitfield access macros across the entire file (and the others)? This will both increase human readability and add compile-time checks on register fields.
Thanks for your suggestion. compile-time checks are helpful to find some unexpected errors. I will update it in V2.
- regmap_update_bits(afe->regmap, AFE_ADDA_MTKAIF_RX_CFG2, mask,
val);
- return 0;
+}
+static int mtk_adda_mtkaif_cfg_event(struct snd_soc_dapm_widget *w,
struct snd_kcontrol *kcontrol,int event)+{
- struct snd_soc_component *cmpnt = snd_soc_dapm_to_component(w-
dapm);
- struct mtk_base_afe *afe =
snd_soc_component_get_drvdata(cmpnt);
- dev_dbg(afe->dev, "%s(), name %s, event 0x%x\n",
__func__, w->name, event);- switch (event) {
- case SND_SOC_DAPM_PRE_PMU:
mt8188_adda_mtkaif_init(afe);break;- default:
break;- }
- return 0;
+}
+static int mtk_adda_dl_event(struct snd_soc_dapm_widget *w,
struct snd_kcontrol *kcontrol,int event)+{
- struct snd_soc_component *cmpnt = snd_soc_dapm_to_component(w-
dapm);
- struct mtk_base_afe *afe =
snd_soc_component_get_drvdata(cmpnt);
- dev_dbg(afe->dev, "%s(), name %s, event 0x%x\n",
__func__, w->name, event);- switch (event) {
- case SND_SOC_DAPM_POST_PMD:
/* should delayed 1/fs(smallest is 8k) = 125us beforeafe off */
usleep_range(125, 135);break;- default:
break;- }
- return 0;
+}
+static void mtk_adda_ul_mictype(struct mtk_base_afe *afe, bool dmic) +{
- unsigned int reg = AFE_ADDA_UL_SRC_CON0;
- unsigned int val = 0;
- unsigned int mask;
- mask = (UL_SDM3_LEVEL_CTL | UL_MODE_3P25M_CH1_CTL |
UL_MODE_3P25M_CH2_CTL);val = (UL_SDM3_LEVEL_CTL | UL_MODE_3P25M_CH1_CTL | UL_MODE_3P25M_CH2_CTL);
- /* turn on dmic, ch1, ch2 */
- if (dmic)
regmap_set_bits(afe->regmap, reg, val);else regmap_clear_bits(afe->regmap, reg, val);
OK. I will update this part in V2.
val = mask;- regmap_update_bits(afe->regmap, reg, mask, val);
+}
..snip..
+static int mtk_afe_adc_hires_connect(struct snd_soc_dapm_widget *source,
struct snd_soc_dapm_widget *sink)+{
- struct snd_soc_dapm_widget *w = source;
- struct snd_soc_component *cmpnt = snd_soc_dapm_to_component(w-
dapm);
- struct mtk_base_afe *afe =
snd_soc_component_get_drvdata(cmpnt);
- struct mt8188_afe_private *afe_priv = afe->platform_priv;
- struct mtk_dai_adda_priv *adda_priv;
- adda_priv = afe_priv->dai_priv[MT8188_AFE_IO_ADDA];
- if (!adda_priv) {
dev_err(afe->dev, "%s adda_priv == NULL", __func__);return 0;return -EINVAL?
dapm_supply_check_power doesn't handled error return value, so it seems to be better to keep return 0 here.
- }
- return (adda_priv->ul_rate > ADDA_HIRES_THRES) ? 1 : 0;
return !!(adda_priv->ul_rate > ADDA_HIRES_THRES);
OK. I will update it in V2.
+}
+static int mtk_afe_dac_hires_connect(struct snd_soc_dapm_widget *source,
struct snd_soc_dapm_widget *sink)+{
- struct snd_soc_dapm_widget *w = source;
- struct snd_soc_component *cmpnt = snd_soc_dapm_to_component(w-
dapm);
- struct mtk_base_afe *afe =
snd_soc_component_get_drvdata(cmpnt);
- struct mt8188_afe_private *afe_priv = afe->platform_priv;
- struct mtk_dai_adda_priv *adda_priv;
- adda_priv = afe_priv->dai_priv[MT8188_AFE_IO_ADDA];
- if (!adda_priv) {
dev_err(afe->dev, "%s adda_priv == NULL", __func__);return 0;same here
- }
- return (adda_priv->dl_rate > ADDA_HIRES_THRES) ? 1 : 0;
+}
..snip..
Regards, Angelo