Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

3 participants
2908 discussions

Re: [PATCH 01/18] ASoC: dt-bindings: mediatek,mt8365-afe: Add audio afe document

by Krzysztof Kozlowski

On 26/02/2024 15:01, Alexandre Mergnat wrote: > Add MT8365 audio front-end bindings > > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> > --- > .../bindings/sound/mediatek,mt8365-afe.yaml | 164 +++++++++++++++++++++ > 1 file changed, 164 insertions(+) > > diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8365-afe.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8365-afe.yaml > new file mode 100644 > index 000000000000..1d7eb2532ad2 > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8365-afe.yaml > @@ -0,0 +1,164 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/sound/mediatek,mt8365-afe.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: MediaTek AFE PCM controller for MT8365 > + > +maintainers: > + - Alexandre Mergnat <amergnat(a)baylibre.com> > + > +properties: > + compatible: > + const: mediatek,mt8365-afe-pcm > + > + reg: > + maxItems: 2 You must describe the items. > + > + interrupts: > + maxItems: 1 > + > + mediatek,topckgen: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: The phandle of the mediatek topckgen controller What for? Don't repeat the property name. Say something useful. > + > + power-domains: > + maxItems: 1 > + > + "#sound-dai-cells": > + const: 0 > + > + clocks: > + items: > + - description: 26M clock > + - description: mux for audio clock > + - description: audio i2s0 mck > + - description: audio i2s1 mck > + - description: audio i2s2 mck > + - description: audio i2s3 mck > + - description: engen 1 clock > + - description: engen 2 clock > + - description: audio 1 clock > + - description: audio 2 clock > + - description: mux for i2s0 > + - description: mux for i2s1 > + - description: mux for i2s2 > + - description: mux for i2s3 > + > + clock-names: > + items: > + - const: top_clk26m_clk > + - const: top_audio_sel > + - const: audio_i2s0_m > + - const: audio_i2s1_m > + - const: audio_i2s2_m > + - const: audio_i2s3_m > + - const: engen1 > + - const: engen2 > + - const: aud1 > + - const: aud2 > + - const: i2s0_m_sel > + - const: i2s1_m_sel > + - const: i2s2_m_sel > + - const: i2s3_m_sel > + > + mediatek,i2s-shared-clock: Why do you need this property? Linux (and other systems) handle sharing clocks properly. > + description: > + i2s modules can share the same external clock pin. > + If this property is not present the clock mode is separrate. Typo > + type: boolean > + > + mediatek,dmic-iir-on: > + description: > + Boolean which specifies whether the DMIC IIR is enabled. > + If this property is not present the IIR is disabled. "is enabled" or "enable it"? You described the desired Linux feature or behavior, not the actual hardware. The bindings are about the latter, so instead you need to rephrase the property and its description to match actual hardware capabilities/features/configuration etc. > + type: boolean > + > + mediatek,dmic-irr-mode: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + Selects stop band of IIR DC-removal filter. > + 0 = Software programmable custom coeff loaded by the driver. Bindings are for hardware, not drivers. Why is this a property of board DTS? > + 1 = 5Hz if 48KHz mode. > + 2 = 10Hz if 48KHz mode. > + 3 = 25Hz if 48KHz mode. > + 4 = 50Hz if 48KHz mode. > + 5 = 65Hz if 48KHz mode. Use proper unit suffixes - hz. > + enum: > + - 0 > + - 1 > + - 2 > + - 3 > + - 4 > + - 5 > + > + mediatek,dmic-two-wire-mode: > + description: > + Boolean which turns on digital microphone for two wire mode. > + If this property is not present the two wire mode is disabled. This looks like hardware property, but the naming looks like SW. Again you instruct what driver should do. Standard disclaimer: You described the desired Linux feature or behavior, not the actual hardware. The bindings are about the latter, so instead you need to rephrase the property and its description to match actual hardware capabilities/features/configuration etc. > + type: boolean > + > + Just one blank line. > +required: > + - compatible > + - reg > + - interrupts > + - power-domains > + - mediatek,topckgen > + - clocks > + - clock-names > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/mediatek,mt8365-clk.h> > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + #include <dt-bindings/power/mediatek,mt8365-power.h> > + > + soc { > + #address-cells = <2>; > + #size-cells = <2>; > + > + afe@11220000 { What is AFE? https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetr… > + compatible = "mediatek,mt8365-afe-pcm"; Best regards, Krzysztof

1 year, 4 months

Re: [PATCH 11/18] ASoC: mediatek: mt8365: Add platform driver

by Krzysztof Kozlowski

On 26/02/2024 15:01, Alexandre Mergnat wrote: > Add mt8365 platform driver. > > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> ... > + memif->substream = substream; > + > + snd_pcm_hw_constraint_step(substream->runtime, 0, > + SNDRV_PCM_HW_PARAM_BUFFER_BYTES, 16); > + > + snd_soc_set_runtime_hwparams(substream, mtk_afe_hardware); > + > + ret = snd_pcm_hw_constraint_integer(runtime, > + SNDRV_PCM_HW_PARAM_PERIODS); > + if (ret < 0) > + dev_err(afe->dev, "snd_pcm_hw_constraint_integer failed\n"); > + > + mt8365_afe_enable_main_clk(afe); > + return ret; > +} > + > +static void mt8365_afe_fe_shutdown(struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); > + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai); > + int memif_num = snd_soc_rtd_to_cpu(rtd, 0)->id; > + struct mtk_base_afe_memif *memif = &afe->memif[memif_num]; > + > + dev_dbg(afe->dev, "%s %s\n", __func__, memif->data->name); Drop trace-like prints. Tracing is for this. Before upstreaming driver from vendor repository, such stuff should be cleaned. > + > + memif->substream = NULL; > + > + mt8365_afe_disable_main_clk(afe); > +} > + > +static int mt8365_afe_fe_hw_params(struct snd_pcm_substream *substream, > + struct snd_pcm_hw_params *params, struct snd_soc_dai *dai) > +{ > + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); > + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai); > + struct mt8365_afe_private *afe_priv = afe->platform_priv; > + int dai_id = snd_soc_rtd_to_cpu(rtd, 0)->id; > + struct mt8365_control_data *ctrl_data = &afe_priv->ctrl_data; > + struct mtk_base_afe_memif *memif = &afe->memif[dai_id]; > + struct mt8365_fe_dai_data *fe_data = &afe_priv->fe_data[dai_id]; > + const size_t request_size = params_buffer_bytes(params); > + unsigned int channels = params_channels(params); > + unsigned int rate = params_rate(params); > + int ret, fs = 0; > + unsigned int base_end_offset = 8; > + > + dev_info(afe->dev, > + "%s %s period = %d rate = %d channels = %d\n", > + __func__, memif->data->name, params_period_size(params), > + rate, channels); > + > + if (dai_id == MT8365_AFE_MEMIF_VUL2) { > + if (!ctrl_data->bypass_cm1) > + /* configure cm1 */ > + mt8365_afe_configure_cm(afe, > + MT8365_CM1, channels, rate); > + else > + regmap_update_bits(afe->regmap, AFE_CM1_CON0, > + CM_AFE_CM1_VUL_SEL, CM_AFE_CM1_VUL_SEL); > + } else if (dai_id == MT8365_AFE_MEMIF_TDM_IN) { > + if (!ctrl_data->bypass_cm2) > + /* configure cm2 */ > + mt8365_afe_configure_cm(afe, > + MT8365_CM2, channels, rate); > + else > + regmap_update_bits(afe->regmap, AFE_CM2_CON0, > + CM_AFE_CM2_TDM_SEL, ~CM_AFE_CM2_TDM_SEL); > + > + base_end_offset = 4; > + } > + > + if (request_size > fe_data->sram_size) { > + ret = snd_pcm_lib_malloc_pages(substream, request_size); > + if (ret < 0) { > + dev_err(afe->dev, > + "%s %s malloc pages %zu bytes failed %d\n", > + __func__, memif->data->name, request_size, ret); > + return ret; > + } > + > + fe_data->use_sram = false; > + > + mt8365_afe_emi_clk_on(afe); > + } else { > + struct snd_dma_buffer *dma_buf = &substream->dma_buffer; > + > + dma_buf->dev.type = SNDRV_DMA_TYPE_DEV; > + dma_buf->dev.dev = substream->pcm->card->dev; > + dma_buf->area = (unsigned char *)fe_data->sram_vir_addr; > + dma_buf->addr = fe_data->sram_phy_addr; > + dma_buf->bytes = request_size; > + snd_pcm_set_runtime_buffer(substream, dma_buf); > + > + fe_data->use_sram = true; > + } > + > + memif->phys_buf_addr = lower_32_bits(substream->runtime->dma_addr); > + memif->buffer_size = substream->runtime->dma_bytes; > + > + /* start */ > + regmap_write(afe->regmap, memif->data->reg_ofs_base, > + memif->phys_buf_addr); > + /* end */ > + regmap_write(afe->regmap, > + memif->data->reg_ofs_base + base_end_offset, > + memif->phys_buf_addr + memif->buffer_size - 1); > + > + /* set channel */ > + if (memif->data->mono_shift >= 0) { > + unsigned int mono = (params_channels(params) == 1) ? 1 : 0; > + > + if (memif->data->mono_reg < 0) > + dev_info(afe->dev, "%s mono_reg is NULL\n", __func__); > + else > + regmap_update_bits(afe->regmap, memif->data->mono_reg, > + 1 << memif->data->mono_shift, > + mono << memif->data->mono_shift); > + } > + > + /* set rate */ > + if (memif->data->fs_shift < 0) > + return 0; > + > + fs = afe->memif_fs(substream, params_rate(params)); > + > + if (fs < 0) > + return -EINVAL; > + > + if (memif->data->fs_reg < 0) > + dev_info(afe->dev, "%s fs_reg is NULL\n", __func__); > + else > + regmap_update_bits(afe->regmap, memif->data->fs_reg, > + memif->data->fs_maskbit << memif->data->fs_shift, > + fs << memif->data->fs_shift); > + > + return 0; > +} > + > +static int mt8365_afe_fe_hw_free(struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); > + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai); > + struct mt8365_afe_private *afe_priv = afe->platform_priv; > + int dai_id = snd_soc_rtd_to_cpu(rtd, 0)->id; > + struct mtk_base_afe_memif *memif = &afe->memif[dai_id]; > + struct mt8365_fe_dai_data *fe_data = &afe_priv->fe_data[dai_id]; > + int ret = 0; > + > + dev_dbg(afe->dev, "%s %s\n", __func__, memif->data->name); Drop > + > + if (fe_data->use_sram) { > + snd_pcm_set_runtime_buffer(substream, NULL); > + } else { > + ret = snd_pcm_lib_free_pages(substream); > + > + mt8365_afe_emi_clk_off(afe); > + } > + > + return ret; > +} > + > +static int mt8365_afe_fe_prepare(struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); > + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai); > + int dai_id = snd_soc_rtd_to_cpu(rtd, 0)->id; > + struct mtk_base_afe_memif *memif = &afe->memif[dai_id]; > + > + /* set format */ > + if (memif->data->hd_reg >= 0) { > + switch (substream->runtime->format) { > + case SNDRV_PCM_FORMAT_S16_LE: > + regmap_update_bits(afe->regmap, memif->data->hd_reg, > + 3 << memif->data->hd_shift, > + 0 << memif->data->hd_shift); > + break; > + case SNDRV_PCM_FORMAT_S32_LE: > + regmap_update_bits(afe->regmap, memif->data->hd_reg, > + 3 << memif->data->hd_shift, > + 3 << memif->data->hd_shift); > + > + if (dai_id == MT8365_AFE_MEMIF_TDM_IN) { > + regmap_update_bits(afe->regmap, > + memif->data->hd_reg, > + 3 << memif->data->hd_shift, > + 1 << memif->data->hd_shift); > + regmap_update_bits(afe->regmap, > + memif->data->hd_reg, > + 1 << memif->data->hd_align_mshift, > + 1 << memif->data->hd_align_mshift); > + } > + break; > + case SNDRV_PCM_FORMAT_S24_LE: > + regmap_update_bits(afe->regmap, memif->data->hd_reg, > + 3 << memif->data->hd_shift, > + 1 << memif->data->hd_shift); > + break; > + default: > + return -EINVAL; > + } > + } > + > + mt8365_afe_irq_direction_enable(afe, > + memif->irq_usage, > + MT8365_AFE_IRQ_DIR_MCU); > + > + return 0; > +} > + > +int mt8365_afe_fe_trigger(struct snd_pcm_substream *substream, int cmd, > + struct snd_soc_dai *dai) > +{ > + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); > + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai); > + struct mt8365_afe_private *afe_priv = afe->platform_priv; > + int dai_id = snd_soc_rtd_to_cpu(rtd, 0)->id; > + struct mt8365_control_data *ctrl_data = &afe_priv->ctrl_data; > + > + switch (cmd) { > + case SNDRV_PCM_TRIGGER_START: > + case SNDRV_PCM_TRIGGER_RESUME: > + /* enable channel merge */ > + if (dai_id == MT8365_AFE_MEMIF_VUL2 && > + !ctrl_data->bypass_cm1) { > + regmap_update_bits(afe->regmap, > + AFE_CM1_CON0, > + CM_AFE_CM_ON, CM_AFE_CM_ON); > + } else if (dai_id == MT8365_AFE_MEMIF_TDM_IN && > + !ctrl_data->bypass_cm2) { > + regmap_update_bits(afe->regmap, > + AFE_CM2_CON0, > + CM_AFE_CM_ON, CM_AFE_CM_ON); > + } > + break; > + case SNDRV_PCM_TRIGGER_STOP: > + case SNDRV_PCM_TRIGGER_SUSPEND: > + /* disable channel merge */ > + if (dai_id == MT8365_AFE_MEMIF_VUL2 && > + !ctrl_data->bypass_cm1) { > + regmap_update_bits(afe->regmap, > + AFE_CM1_CON0, > + CM_AFE_CM_ON, ~CM_AFE_CM_ON); > + } else if (dai_id == MT8365_AFE_MEMIF_TDM_IN && > + !ctrl_data->bypass_cm2) { > + regmap_update_bits(afe->regmap, > + AFE_CM2_CON0, > + CM_AFE_CM_ON, ~CM_AFE_CM_ON); > + } > + break; > + default: > + break; > + } > + > + return mtk_afe_fe_trigger(substream, cmd, dai); > +} > + > +static int mt8365_afe_hw_gain1_startup(struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai); > + > + mt8365_afe_enable_main_clk(afe); > + return 0; > +} > + > +static void mt8365_afe_hw_gain1_shutdown(struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai); > + struct mt8365_afe_private *afe_priv = afe->platform_priv; > + struct mt8365_be_dai_data *be = > + &afe_priv->be_data[dai->id - MT8365_AFE_BACKEND_BASE]; > + > + const unsigned int stream = substream->stream; Why do you const local scalars? What is the point of this variable anyway? > + > + if (be->prepared[stream]) { > + regmap_update_bits(afe->regmap, AFE_GAIN1_CON0, > + AFE_GAIN1_CON0_EN_MASK, 0); > + be->prepared[stream] = false; > + } > + mt8365_afe_disable_main_clk(afe); > +} > + > +static int mt8365_afe_hw_gain1_prepare(struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct snd_pcm_runtime * const runtime = substream->runtime; > + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai); > + struct mt8365_afe_private *afe_priv = afe->platform_priv; > + struct mt8365_be_dai_data *be = > + &afe_priv->be_data[dai->id - MT8365_AFE_BACKEND_BASE]; > + > + const unsigned int rate = runtime->rate; > + const unsigned int stream = substream->stream; So this const is everywhere? How does it change it from code safety? No one expects to overwrite such variables anyway, so there is no increase in readability, either. > + int fs; > + unsigned int val1 = 0, val2 = 0; > + > + if (be->prepared[stream]) { > + dev_info(afe->dev, "%s prepared already\n", __func__); > + return 0; > + } > + > + fs = mt8365_afe_fs_timing(rate); > + regmap_update_bits(afe->regmap, AFE_GAIN1_CON0, > + AFE_GAIN1_CON0_MODE_MASK, (unsigned int)fs<<4); > + > + regmap_read(afe->regmap, AFE_GAIN1_CON1, &val1); > + regmap_read(afe->regmap, AFE_GAIN1_CUR, &val2); > + if ((val1 & AFE_GAIN1_CON1_MASK) != (val2 & AFE_GAIN1_CUR_MASK)) > + regmap_update_bits(afe->regmap, AFE_GAIN1_CUR, > + AFE_GAIN1_CUR_MASK, val1); > + > + regmap_update_bits(afe->regmap, AFE_GAIN1_CON0, > + AFE_GAIN1_CON0_EN_MASK, 1); > + be->prepared[stream] = true; > + > + return 0; > +} > + > +static const struct snd_pcm_hardware mt8365_hostless_hardware = { > + .info = (SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_INTERLEAVED | > + SNDRV_PCM_INFO_MMAP_VALID), > + .period_bytes_min = 256, > + .period_bytes_max = 4 * 48 * 1024, > + .periods_min = 2, > + .periods_max = 256, > + .buffer_bytes_max = 8 * 48 * 1024, > + .fifo_size = 0, > +}; > + > +/* dai ops */ > +static int mtk_dai_hostless_startup(struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct mtk_base_afe *afe = snd_soc_dai_get_drvdata(dai); > + struct snd_pcm_runtime *runtime = substream->runtime; > + int ret; > + > + snd_soc_set_runtime_hwparams(substream, &mt8365_hostless_hardware); > + > + ret = snd_pcm_hw_constraint_integer(runtime, > + SNDRV_PCM_HW_PARAM_PERIODS); > + if (ret < 0) > + dev_info(afe->dev, "snd_pcm_hw_constraint_integer failed\n"); > + return ret; > +} > + > +/* FE DAIs */ > +static const struct snd_soc_dai_ops mt8365_afe_fe_dai_ops = { > + .startup = mt8365_afe_fe_startup, > + .shutdown = mt8365_afe_fe_shutdown, > + .hw_params = mt8365_afe_fe_hw_params, > + .hw_free = mt8365_afe_fe_hw_free, > + .prepare = mt8365_afe_fe_prepare, > + .trigger = mt8365_afe_fe_trigger, > +}; > + > +static const struct snd_soc_dai_ops mt8365_dai_hostless_ops = { > + .startup = mtk_dai_hostless_startup, > +}; > + ... > +static int mt8365_afe_cm2_io_input_mux_get(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) > +{ > + ucontrol->value.integer.value[0] = mCM2Input; > + > + return 0; > +} > + > +static int mt8365_afe_cm2_io_input_mux_put(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) Run checkpatch --strict > +{ > + struct snd_soc_dapm_context *dapm = > + snd_soc_dapm_kcontrol_dapm(kcontrol); > + struct snd_soc_component *comp = snd_soc_dapm_to_component(dapm); > + struct mtk_base_afe *afe = snd_soc_component_get_drvdata(comp); > + struct mt8365_afe_private *afe_priv = afe->platform_priv; > + int ret; > + > + mCM2Input = ucontrol->value.enumerated.item[0]; > + > + afe_priv->cm2_mux_input = mCM2Input; > + ret = snd_soc_dapm_put_enum_double(kcontrol, ucontrol); > + > + return ret; > +} > + ... > + > +static const struct regmap_config mt8365_afe_regmap_config = { > + .reg_bits = 32, > + .reg_stride = 4, > + .val_bits = 32, > + .max_register = MAX_REGISTER, > + .cache_type = REGCACHE_NONE, > +}; > + > +static irqreturn_t mt8365_afe_irq_handler(int irq, void *dev_id) > +{ > + struct mtk_base_afe *afe = dev_id; > + unsigned int reg_value; > + unsigned int mcu_irq_mask; > + int i, ret; > + > + ret = regmap_read(afe->regmap, AFE_IRQ_MCU_STATUS, &reg_value); > + if (ret) { > + dev_err(afe->dev, "%s irq status err\n", __func__); I can imagine happy user seeing flooded dmesg on each IRQ with these messages... > + reg_value = AFE_IRQ_STATUS_BITS; > + goto err_irq; > + } > + > + ret = regmap_read(afe->regmap, AFE_IRQ_MCU_EN, &mcu_irq_mask); > + if (ret) { > + dev_err(afe->dev, "%s irq mcu_en err\n", __func__); > + reg_value = AFE_IRQ_STATUS_BITS; > + goto err_irq; > + } > + > + /* only clr cpu irq */ > + reg_value &= mcu_irq_mask; > + > + for (i = 0; i < MT8365_AFE_MEMIF_NUM; i++) { > + struct mtk_base_afe_memif *memif = &afe->memif[i]; > + struct mtk_base_afe_irq *mcu_irq; > + > + if (memif->irq_usage < 0) > + continue; > + > + mcu_irq = &afe->irqs[memif->irq_usage]; > + > + if (!(reg_value & (1 << mcu_irq->irq_data->irq_clr_shift))) > + continue; > + > + snd_pcm_period_elapsed(memif->substream); > + } > + > +err_irq: > + /* clear irq */ > + regmap_write(afe->regmap, AFE_IRQ_MCU_CLR, > + reg_value & AFE_IRQ_STATUS_BITS); > + > + return IRQ_HANDLED; > +} > + > +static int __maybe_unused mt8365_afe_runtime_suspend(struct device *dev) > +{ > + return 0; > +} > + > +static int mt8365_afe_runtime_resume(struct device *dev) > +{ > + return 0; > +} > + > +static int __maybe_unused mt8365_afe_suspend(struct device *dev) > +{ > + struct mtk_base_afe *afe = dev_get_drvdata(dev); > + struct regmap *regmap = afe->regmap; > + int i; > + > + mt8365_afe_enable_main_clk(afe); > + > + if (!afe->reg_back_up) > + afe->reg_back_up = > + devm_kcalloc(dev, afe->reg_back_up_list_num, > + sizeof(unsigned int), GFP_KERNEL); > + > + for (i = 0; i < afe->reg_back_up_list_num; i++) > + regmap_read(regmap, afe->reg_back_up_list[i], > + &afe->reg_back_up[i]); > + > + mt8365_afe_disable_main_clk(afe); > + > + return 0; > +} > + > +static int __maybe_unused mt8365_afe_resume(struct device *dev) > +{ > + struct mtk_base_afe *afe = dev_get_drvdata(dev); > + struct regmap *regmap = afe->regmap; > + int i = 0; > + > + if (!afe->reg_back_up) { > + dev_dbg(dev, "%s no reg_backup\n", __func__); > + return 0; > + } > + > + mt8365_afe_enable_main_clk(afe); > + > + for (i = 0; i < afe->reg_back_up_list_num; i++) > + regmap_write(regmap, afe->reg_back_up_list[i], > + afe->reg_back_up[i]); > + > + mt8365_afe_disable_main_clk(afe); > + > + return 0; > +} > + > +static int __maybe_unused mt8365_afe_dev_runtime_suspend(struct device *dev) > +{ > + struct mtk_base_afe *afe = dev_get_drvdata(dev); > + > + dev_dbg(afe->dev, "%s suspend %d %d >>\n", __func__, > + pm_runtime_status_suspended(dev), afe->suspended); Drop silly trace-like prints. Tracing is for this. > + > + if (pm_runtime_status_suspended(dev) || afe->suspended) > + return 0; > + > + mt8365_afe_suspend(dev); > + > + afe->suspended = true; > + > + dev_dbg(afe->dev, "%s <<\n", __func__); Drop silly trace-like prints. Tracing is for this. > + > + return 0; > +} > + > +static int __maybe_unused mt8365_afe_dev_runtime_resume(struct device *dev) > +{ > + struct mtk_base_afe *afe = dev_get_drvdata(dev); > + > + dev_dbg(afe->dev, "%s suspend %d %d >>\n", __func__, > + pm_runtime_status_suspended(dev), afe->suspended); Drop silly trace-like prints. Tracing is for this. > + > + if (pm_runtime_status_suspended(dev) || !afe->suspended) > + return 0; > + > + mt8365_afe_resume(dev); > + > + afe->suspended = false; > + > + dev_dbg(afe->dev, "%s <<\n", __func__); Drop silly trace-like prints. Tracing is for this. > + > + return 0; > +} > + > +static int mt8365_afe_init_registers(struct mtk_base_afe *afe) > +{ > + size_t i; > + > + static struct { > + unsigned int reg; > + unsigned int mask; > + unsigned int val; > + } init_regs[] = { > + { AFE_CONN_24BIT, GENMASK(31, 0), GENMASK(31, 0) }, > + { AFE_CONN_24BIT_1, GENMASK(21, 0), GENMASK(21, 0) }, > + }; > + > + mt8365_afe_enable_main_clk(afe); > + > + for (i = 0; i < ARRAY_SIZE(init_regs); i++) > + regmap_update_bits(afe->regmap, init_regs[i].reg, > + init_regs[i].mask, init_regs[i].val); > + > + mt8365_afe_disable_main_clk(afe); > + > + return 0; > +} > + > +static int mt8365_afe_component_probe(struct snd_soc_component *component) > +{ > + return mtk_afe_add_sub_dai_control(component); > +} > + > +static const struct snd_soc_component_driver mt8365_afe_component = { > + .name = AFE_PCM_NAME, > + .probe = mt8365_afe_component_probe, > + .pointer = mtk_afe_pcm_pointer, > + .pcm_construct = mtk_afe_pcm_new, > +}; > + > +static const struct snd_soc_component_driver mt8365_afe_pcm_component = { > + .name = "mt8365-afe-pcm-dai", > +}; > + > +static int mt8365_dai_memif_register(struct mtk_base_afe *afe) > +{ > + struct mtk_base_afe_dai *dai; > + > + dai = devm_kzalloc(afe->dev, sizeof(*dai), GFP_KERNEL); > + if (!dai) > + return -ENOMEM; > + > + list_add(&dai->list, &afe->sub_dais); > + > + dai->dai_drivers = mt8365_memif_dai_driver; > + dai->num_dai_drivers = ARRAY_SIZE(mt8365_memif_dai_driver); > + > + dai->dapm_widgets = mt8365_memif_widgets; > + dai->num_dapm_widgets = ARRAY_SIZE(mt8365_memif_widgets); > + dai->dapm_routes = mt8365_memif_routes; > + dai->num_dapm_routes = ARRAY_SIZE(mt8365_memif_routes); > + return 0; > +} > + > +typedef int (*dai_register_cb)(struct mtk_base_afe *); > +static const dai_register_cb dai_register_cbs[] = { > + mt8365_dai_pcm_register, > + mt8365_dai_i2s_register, > + mt8365_dai_adda_register, > + mt8365_dai_dmic_register, > + mt8365_dai_memif_register, > +}; > + > +static int mt8365_afe_pcm_dev_probe(struct platform_device *pdev) > +{ > + struct mtk_base_afe *afe; > + struct mt8365_afe_private *afe_priv; > + struct device *dev; > + int ret, i, sel_irq; > + unsigned int irq_id; > + struct resource *res; > + > + afe = devm_kzalloc(&pdev->dev, sizeof(*afe), GFP_KERNEL); > + if (!afe) > + return -ENOMEM; > + platform_set_drvdata(pdev, afe); > + > + afe->platform_priv = devm_kzalloc(&pdev->dev, sizeof(*afe_priv), > + GFP_KERNEL); > + if (!afe->platform_priv) > + return -ENOMEM; > + > + afe_priv = afe->platform_priv; > + afe->dev = &pdev->dev; > + dev = afe->dev; > + > + spin_lock_init(&afe_priv->afe_ctrl_lock); > + mutex_init(&afe_priv->afe_clk_mutex); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + afe->base_addr = devm_ioremap_resource(&pdev->dev, res); Why not using helper combining these two? > + if (IS_ERR(afe->base_addr)) > + return PTR_ERR(afe->base_addr); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); > + if (res) { > + afe_priv->afe_sram_vir_addr = > + devm_ioremap_resource(&pdev->dev, res); > + if (!IS_ERR(afe_priv->afe_sram_vir_addr)) { > + afe_priv->afe_sram_phy_addr = res->start; > + afe_priv->afe_sram_size = resource_size(res); > + } > + } > + > + /* initial audio related clock */ > + ret = mt8365_afe_init_audio_clk(afe); > + if (ret) { > + dev_err(afe->dev, "mt8365_afe_init_audio_clk fail\n"); return dev_err_probe() > + return ret; > + } > + > + afe->regmap = devm_regmap_init_mmio_clk(&pdev->dev, "top_audio_sel", > + afe->base_addr, &mt8365_afe_regmap_config); > + if (IS_ERR(afe->regmap)) > + return PTR_ERR(afe->regmap); > + > + /* memif % irq initialize*/ > + afe->memif_size = MT8365_AFE_MEMIF_NUM; > + afe->memif = devm_kcalloc(afe->dev, afe->memif_size, > + sizeof(*afe->memif), GFP_KERNEL); > + if (!afe->memif) > + return -ENOMEM; > + > + afe->irqs_size = MT8365_AFE_IRQ_NUM; > + afe->irqs = devm_kcalloc(afe->dev, afe->irqs_size, > + sizeof(*afe->irqs), GFP_KERNEL); > + if (!afe->irqs) > + return -ENOMEM; > + > + for (i = 0; i < afe->irqs_size; i++) > + afe->irqs[i].irq_data = &irq_data[i]; > + > + irq_id = platform_get_irq(pdev, 0); > + if (!irq_id) { That does not look correct. 0 is valid. Look at help of this function. > + dev_err(afe->dev, "np %s no irq\n", afe->dev->of_node->name); return dev_err_probe() > + return -ENXIO; > + } > + ret = devm_request_irq(afe->dev, irq_id, mt8365_afe_irq_handler, > + 0, "Afe_ISR_Handle", (void *)afe); > + if (ret) { > + dev_err(afe->dev, "could not request_irq\n"); return dev_err_probe() > + return ret; > + } > + > + /* init sub_dais */ > + INIT_LIST_HEAD(&afe->sub_dais); > + > + for (i = 0; i < ARRAY_SIZE(dai_register_cbs); i++) { > + ret = dai_register_cbs[i](afe); > + if (ret) { > + dev_warn(afe->dev, "dai register i %d fail, ret %d\n", > + i, ret); > + return ret; > + } > + } > + > + /* init dai_driver and component_driver */ > + ret = mtk_afe_combine_sub_dai(afe); > + if (ret) { > + dev_warn(afe->dev, "mtk_afe_combine_sub_dai fail, ret %d\n", > + ret); > + return ret; > + } > + > + for (i = 0; i < afe->memif_size; i++) { > + afe->memif[i].data = &memif_data[i]; > + sel_irq = memif_specified_irqs[i]; > + if (sel_irq >= 0) { > + afe->memif[i].irq_usage = sel_irq; > + afe->memif[i].const_irq = 1; > + afe->irqs[sel_irq].irq_occupyed = true; > + } else { > + afe->memif[i].irq_usage = -1; > + } > + } > + > + afe->mtk_afe_hardware = &mt8365_afe_hardware; > + afe->memif_fs = mt8365_memif_fs; > + afe->irq_fs = mt8365_irq_fs; > + > + pm_runtime_enable(&pdev->dev); > + > + pm_runtime_get_sync(&pdev->dev); > + afe->reg_back_up_list = mt8365_afe_backup_list; > + afe->reg_back_up_list_num = ARRAY_SIZE(mt8365_afe_backup_list); > + afe->runtime_resume = mt8365_afe_runtime_resume; > + afe->runtime_suspend = mt8365_afe_runtime_suspend; > + > + /* open afe pdn for dapm read/write audio register */ > + mt8365_afe_enable_top_cg(afe, MT8365_TOP_CG_AFE); > + > + /* Set 26m parent clk */ > + mt8365_afe_set_clk_parent(afe, > + afe_priv->clocks[MT8365_CLK_TOP_AUD_SEL], > + afe_priv->clocks[MT8365_CLK_CLK26M]); > + > + ret = devm_snd_soc_register_component(&pdev->dev, > + &mt8365_afe_component, NULL, 0); > + if (ret) { > + dev_warn(dev, "err_platform\n"); > + return ret; > + } > + > + ret = devm_snd_soc_register_component(&pdev->dev, > + &mt8365_afe_pcm_component, > + afe->dai_drivers, > + afe->num_dai_drivers); > + if (ret) { > + dev_warn(dev, "err_dai_component\n"); > + return ret; > + } > + > + mt8365_afe_init_registers(afe); > + > + dev_info(&pdev->dev, "MT8365 AFE driver initialized.\n"); Drop simple probe success messages everywhere. They are not needed and they don't bring useful information. Best regards, Krzysztof

1 year, 4 months

Re: [PATCH 10/18] ASoc: mediatek: mt8365: Add a specific soundcard for EVK

by Krzysztof Kozlowski

On 26/02/2024 15:01, amergnat(a)baylibre.com wrote: > From: Nicolas Belin <nbelin(a)baylibre.com> > > Add a specific soundcard for mt8365-evk. It supports audio jack > in/out, dmics, the amic and lineout. > > Signed-off-by: Nicolas Belin <nbelin(a)baylibre.com> > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> ... > +static int mt8365_mt6357_dev_probe(struct platform_device *pdev) > +{ > + struct snd_soc_card *card = &mt8365_mt6357_card; > + struct device *dev = &pdev->dev; > + struct device_node *platform_node; > + struct mt8365_mt6357_priv *priv; > + int i, ret; > + > + card->dev = dev; > + ret = parse_dai_link_info(card); > + if (ret) > + goto err; > + > + platform_node = of_parse_phandle(dev->of_node, "mediatek,platform", 0); > + if (!platform_node) { > + dev_err(dev, "Property 'platform' missing or invalid\n"); > + return -EINVAL; > + } > + > + for (i = 0; i < card->num_links; i++) { > + if (mt8365_mt6357_dais[i].platforms->name) > + continue; > + mt8365_mt6357_dais[i].platforms->of_node = platform_node; > + } > + > + priv = devm_kzalloc(dev, sizeof(struct mt8365_mt6357_priv), > + GFP_KERNEL); > + if (!priv) { > + ret = -ENOMEM; > + dev_err(dev, "%s allocate card private data fail %d\n", > + __func__, ret); Don't print anything on memory allocations failures. Run coccinelle/coccicheck to detect such trivial issues. > + return ret; > + } > + > + snd_soc_card_set_drvdata(card, priv); > + > + mt8365_mt6357_gpio_probe(card); > + > + ret = devm_snd_soc_register_card(dev, card); > + if (ret) > + dev_err(dev, "%s snd_soc_register_card fail %d\n", > + __func__, ret); > +err: > + of_node_put(platform_node); > + clean_card_reference(card); > + return ret; > +} > + > +static const struct of_device_id mt8365_mt6357_dt_match[] = { > + { .compatible = "mediatek,mt8365-mt6357", }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, mt8365_mt6357_dt_match); > + > +static struct platform_driver mt8365_mt6357_driver = { > + .driver = { > + .name = "mt8365_mt6357", > + .of_match_table = mt8365_mt6357_dt_match, > + .pm = &snd_soc_pm_ops, > + }, > + .probe = mt8365_mt6357_dev_probe, > +}; > + > +module_platform_driver(mt8365_mt6357_driver); > + > +/* Module information */ > +MODULE_DESCRIPTION("MT8365 EVK SoC machine driver"); > +MODULE_AUTHOR("Nicolas Belin <nbelin(a)baylibre.com>"); > +MODULE_LICENSE("GPL"); > +MODULE_ALIAS("mt8365 mt6357 soc card"); This does not look like correct alias. Drop. > Best regards, Krzysztof

1 year, 4 months

Re: [PATCH] net: ethernet: ti: am65-cpsw: Add minimal XDP support

by Andrew Lunn

> +static struct sk_buff *am65_cpsw_alloc_skb(struct net_device *ndev, unsigned int len) > +{ > + struct page *page; > + struct sk_buff *skb; > + > + page = dev_alloc_pages(0); You are likely to get better performance if you use the page_pool. When FEC added XDP support, the first set of changes was to make use of page_pool. That improved the drivers performance. Then XDP was added on top. Maybe you can follow that pattern. Andrew

1 year, 4 months

Re: [PATCH 12/18] ASoC: codecs: mt6357: add MT6357 codec

by Mark Brown

On Mon, Feb 26, 2024 at 03:01:50PM +0100, amergnat(a)baylibre.com wrote: > index 000000000000..13e95c227114 > --- /dev/null > +++ b/sound/soc/codecs/mt6357.c > @@ -0,0 +1,1805 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * MT6357 ALSA SoC audio codec driver > + * Please use a C++ comment for the whole comment to make it clearer that this is intentional. > +static void set_playback_gpio(struct mt6357_priv *priv, bool enable) > +{ > + if (enable) { > + /* set gpio mosi mode */ > + regmap_write(priv->regmap, MT6357_GPIO_MODE2_CLR, GPIO_MODE2_CLEAR_ALL); > + regmap_write(priv->regmap, MT6357_GPIO_MODE2_SET, GPIO8_MODE_SET_AUD_CLK_MOSI | > + GPIO9_MODE_SET_AUD_DAT_MOSI0 | > + GPIO10_MODE_SET_AUD_DAT_MOSI1 | > + GPIO11_MODE_SET_AUD_SYNC_MOSI); This would be a lot more legible if you worked out the values to set and then had a single set of writes, currently the indentation makes it very hard to read. Similarly for other similar functions. > +static int mt6357_put_volsw(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) > +{ > + struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol); > + struct mt6357_priv *priv = snd_soc_component_get_drvdata(component); > + struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; > + unsigned int reg; > + int ret; > + > + ret = snd_soc_put_volsw(kcontrol, ucontrol); > + if (ret < 0) > + return ret; > + > + switch (mc->reg) { > + case MT6357_ZCD_CON2: > + regmap_read(priv->regmap, MT6357_ZCD_CON2, &reg); > + priv->ana_gain[ANALOG_VOLUME_HPOUTL] = > + (reg & AUD_HPL_GAIN_MASK) >> AUD_HPL_GAIN_SFT; > + priv->ana_gain[ANALOG_VOLUME_HPOUTR] = > + (reg & AUD_HPR_GAIN_MASK) >> AUD_HPR_GAIN_SFT; > + break; It would probably be less code and would definitely be clearer and simpler to just read the values when we need them rather than constatly keeping a cache separate to the register cache. > + /* ul channel swap */ > + SOC_SINGLE("UL LR Swap", MT6357_AFE_UL_DL_CON0, AFE_UL_LR_SWAP_SFT, 1, 0), On/off controls should end in Switch. > +static const char * const hslo_mux_map[] = { > + "Open", "DACR", "Playback", "Test mode" > +}; > + > +static int hslo_mux_map_value[] = { > + 0x0, 0x1, 0x2, 0x3, > +}; Why not just use a normal mux here, there's no missing values or reordering? Similarly for other muxes. > +static unsigned int mt6357_read(struct snd_soc_component *codec, unsigned int reg) > +{ > + struct mt6357_priv *priv = snd_soc_component_get_drvdata(codec); > + unsigned int val; > + > + pr_debug("%s() reg = 0x%x", __func__, reg); > + regmap_read(priv->regmap, reg, &val); > + return val; > +} Remove these, there are vastly more logging facilities as standard in the regmap core. > +/* Reg bit defines */ > +/* MT6357_GPIO_DIR0 */ > +#define GPIO8_DIR_MASK BIT(8) > +#define GPIO8_DIR_INPUT 0 Please namespace your defines, these look very generic.

1 year, 4 months

Re: [PATCH 02/18] ASoC: dt-bindings: mediatek,mt8365-mt6357: Add audio sound card document

by AngeloGioacchino Del Regno

Il 26/02/24 15:01, Alexandre Mergnat ha scritto: > Add soundcard bindings for the MT8365 SoC with the MT6357 audio codec. > > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> > --- > .../bindings/sound/mediatek,mt8365-mt6357.yaml | 127 +++++++++++++++++++++ > 1 file changed, 127 insertions(+) > > diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8365-mt6357.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8365-mt6357.yaml > new file mode 100644 > index 000000000000..f469611ec6b6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8365-mt6357.yaml > @@ -0,0 +1,127 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/sound/mediatek,mt8365-mt6357.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Mediatek MT8365 sound card with MT6357 sound codec. > + > +maintainers: > + - Alexandre Mergnat <amergnat(a)baylibre.com> > + > +description: > + This binding describes the MT8365 sound card. > + > +properties: > + compatible: > + const: mediatek,mt8365-mt6357 > + > + mediatek,hp-pull-down: > + description: > + Earphone driver positive output stage short to the > + audio reference ground. > + Default value is false. > + type: boolean > + > + mediatek,micbias0-microvolt: > + description: | description: Selects MIC Bias 0 output voltage > + Selects MIC Bias 0 output voltage. > + [1.7v, 1.8v, 1.9v, 2.0v, 2.1v, 2.5v, 2.6v, 2.7v] > + enum: [0, 1, 2, 3, 4, 5, 6, 7] No, you don't say 0 1 2 3 4 to a property that says "microvolt", that's simply wrong. mediatek,micbias0-microvolt = <2100000>; ...so you want a binding that says enum: [ 1700000, 1800000, this, that, 2700000] > + > + mediatek,micbias1-microvolt: > + description: | > + Selects MIC Bias 1 output voltage. > + [1.7v, 1.8v, 1.9v, 2.0v, 2.1v, 2.5v, 2.6v, 2.7v] > + enum: [0, 1, 2, 3, 4, 5, 6, 7] same here. > + > + mediatek,platform: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: The phandle of MT8365 ASoC platform. > + > + pinctrl-names: > + minItems: 1 > + items: > + - const: aud_default > + - const: aud_dmic > + - const: aud_miso_off > + - const: aud_miso_on > + - const: aud_mosi_off > + - const: aud_mosi_on > + > + vaud28-supply: > + description: > + 2.8 volt supply for the audio codec > + > +patternProperties: > + "^dai-link-[0-9]+$": > + type: object > + description: > + Container for dai-link level properties and CODEC sub-nodes. > + > + properties: > + codec: > + type: object > + description: Holds subnode which indicates codec dai. > + > + properties: > + sound-dai: > + maxItems: 1 > + description: phandle of the codec DAI > + > + additionalProperties: false > + > + link-name: > + description: > + This property corresponds to the name of the BE dai-link to which > + we are going to update parameters in this node. > + items: > + const: 2ND I2S BE > + > + sound-dai: > + maxItems: 1 > + description: phandle of the CPU DAI > + > + additionalProperties: false > + > + required: > + - link-name > + - sound-dai > + > +additionalProperties: false > + > +required: > + - compatible > + - mediatek,platform > + - pinctrl-names > + - vaud28-supply > + > +examples: > + - | > + sound { > + compatible = "mediatek,mt8365-mt6357"; > + mediatek,platform = <&afe>; Please: https://docs.kernel.org/devicetree/bindings/dts-coding-style.html Regards, Angelo > + pinctrl-names = "aud_default", > + "aud_dmic", > + "aud_miso_off", > + "aud_miso_on", > + "aud_mosi_off", > + "aud_mosi_on"; > + pinctrl-0 = <&aud_default_pins>; > + pinctrl-1 = <&aud_dmic_pins>; > + pinctrl-2 = <&aud_miso_off_pins>; > + pinctrl-3 = <&aud_miso_on_pins>; > + pinctrl-4 = <&aud_mosi_off_pins>; > + pinctrl-5 = <&aud_mosi_on_pins>; > + vaud28-supply = <&mt6357_vaud28_reg>; > + > + /* hdmi interface */ > + dai-link-0 { > + sound-dai = <&afe>; > + link-name = "2ND I2S BE"; > + > + codec { > + sound-dai = <&it66121hdmitx>; > + }; > + }; > + }; >

1 year, 4 months

Re: [PATCH 13/18] mfd: mt6397-core: register mt6357 sound codec

by AngeloGioacchino Del Regno

Il 26/02/24 15:01, amergnat(a)baylibre.com ha scritto: > From: Fabien Parent <fparent(a)baylibre.com> > > Add MT6357 codec entry in the MFD driver. > > Signed-off-by: Fabien Parent <fparent(a)baylibre.com> > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno(a)collabora.com> > --- > drivers/mfd/mt6397-core.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c > index 4449dde05021..4fd4a2da5ad7 100644 > --- a/drivers/mfd/mt6397-core.c > +++ b/drivers/mfd/mt6397-core.c > @@ -141,6 +141,9 @@ static const struct mfd_cell mt6357_devs[] = { > .num_resources = ARRAY_SIZE(mt6357_rtc_resources), > .resources = mt6357_rtc_resources, > .of_compatible = "mediatek,mt6357-rtc", > + }, { > + .name = "mt6357-sound", > + .of_compatible = "mediatek,mt6357-sound" > }, { > .name = "mtk-pmic-keys", > .num_resources = ARRAY_SIZE(mt6357_keys_resources), >

1 year, 4 months

Re: [PATCH 12/18] ASoC: codecs: mt6357: add MT6357 codec

by AngeloGioacchino Del Regno

Il 26/02/24 15:01, amergnat(a)baylibre.com ha scritto: > From: Nicolas Belin <nbelin(a)baylibre.com> > > Add the support of MT6357 PMIC audio codec. > > Signed-off-by: Nicolas Belin <nbelin(a)baylibre.com> > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> > --- > sound/soc/codecs/Kconfig | 7 + > sound/soc/codecs/Makefile | 2 + > sound/soc/codecs/mt6357.c | 1805 +++++++++++++++++++++++++++++++++++++++++++++ > sound/soc/codecs/mt6357.h | 674 +++++++++++++++++ > 4 files changed, 2488 insertions(+) > > diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig > index 59f9742e9ff4..9cf2b9b70472 100644 > --- a/sound/soc/codecs/Kconfig > +++ b/sound/soc/codecs/Kconfig > @@ -153,6 +153,7 @@ config SND_SOC_ALL_CODECS > imply SND_SOC_MC13783 > imply SND_SOC_ML26124 > imply SND_SOC_MT6351 > + imply SND_SOC_MT6357 > imply SND_SOC_MT6358 > imply SND_SOC_MT6359 > imply SND_SOC_MT6660 > @@ -2361,6 +2362,12 @@ config SND_SOC_ML26124 > config SND_SOC_MT6351 > tristate "MediaTek MT6351 Codec" > > +config SND_SOC_MT6357 > + tristate "MediaTek MT6357 Codec" > + help > + Enable support for the platform which uses MT6357 as > + external codec device. > + > config SND_SOC_MT6358 > tristate "MediaTek MT6358 Codec" > help > diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile > index f53baa2b9565..33e27612867e 100644 > --- a/sound/soc/codecs/Makefile > +++ b/sound/soc/codecs/Makefile > @@ -169,6 +169,7 @@ snd-soc-ml26124-objs := ml26124.o > snd-soc-msm8916-analog-objs := msm8916-wcd-analog.o > snd-soc-msm8916-digital-objs := msm8916-wcd-digital.o > snd-soc-mt6351-objs := mt6351.o > +snd-soc-mt6357-objs := mt6357.o > snd-soc-mt6358-objs := mt6358.o > snd-soc-mt6359-objs := mt6359.o > snd-soc-mt6359-accdet-objs := mt6359-accdet.o > @@ -554,6 +555,7 @@ obj-$(CONFIG_SND_SOC_ML26124) += snd-soc-ml26124.o > obj-$(CONFIG_SND_SOC_MSM8916_WCD_ANALOG) +=snd-soc-msm8916-analog.o > obj-$(CONFIG_SND_SOC_MSM8916_WCD_DIGITAL) +=snd-soc-msm8916-digital.o > obj-$(CONFIG_SND_SOC_MT6351) += snd-soc-mt6351.o > +obj-$(CONFIG_SND_SOC_MT6357) += snd-soc-mt6357.o > obj-$(CONFIG_SND_SOC_MT6358) += snd-soc-mt6358.o > obj-$(CONFIG_SND_SOC_MT6359) += snd-soc-mt6359.o > obj-$(CONFIG_SND_SOC_MT6359_ACCDET) += mt6359-accdet.o > diff --git a/sound/soc/codecs/mt6357.c b/sound/soc/codecs/mt6357.c > new file mode 100644 > index 000000000000..13e95c227114 > --- /dev/null > +++ b/sound/soc/codecs/mt6357.c > @@ -0,0 +1,1805 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * MT6357 ALSA SoC audio codec driver > + * > + * Copyright (c) 2024 Baylibre > + * Author: Nicolas Belin <nbelin(a)baylibre.com> > + */ > + > +#include <linux/dma-mapping.h> > +#include <sound/soc.h> > +#include <sound/tlv.h> > +#include <linux/mfd/mt6397/core.h> > +#include <linux/regulator/consumer.h> > + > +#include "mt6357.h" > + > +static void set_playback_gpio(struct mt6357_priv *priv, bool enable) > +{ you're anyway always doing CLEAR_ALL, so you can do it outside of the if branch. regmap_write( ... CLEAR_ALL); if (enable) { ... } else { ... } > + if (enable) { > + /* set gpio mosi mode */ > + regmap_write(priv->regmap, MT6357_GPIO_MODE2_CLR, GPIO_MODE2_CLEAR_ALL); > + regmap_write(priv->regmap, MT6357_GPIO_MODE2_SET, GPIO8_MODE_SET_AUD_CLK_MOSI | > + GPIO9_MODE_SET_AUD_DAT_MOSI0 | > + GPIO10_MODE_SET_AUD_DAT_MOSI1 | > + GPIO11_MODE_SET_AUD_SYNC_MOSI); Are you sure that you need to write to MODE2_SET *and* to MODE2?! > + regmap_write(priv->regmap, MT6357_GPIO_MODE2, GPIO8_MODE_AUD_CLK_MOSI | > + GPIO9_MODE_AUD_DAT_MOSI0 | > + GPIO10_MODE_AUD_DAT_MOSI1 | > + GPIO11_MODE_AUD_SYNC_MOSI); > + } else { > + /* set pad_aud_*_mosi to GPIO mode and dir input > + * reason: > + * pad_aud_dat_mosi*, because the pin is used as boot strap > + */ > + regmap_write(priv->regmap, MT6357_GPIO_MODE2_CLR, GPIO_MODE2_CLEAR_ALL); > + regmap_write(priv->regmap, MT6357_GPIO_MODE2, GPIO8_MODE_GPIO | > + GPIO9_MODE_GPIO | > + GPIO10_MODE_GPIO | > + GPIO11_MODE_GPIO); > + regmap_update_bits(priv->regmap, MT6357_GPIO_DIR0, GPIO8_DIR_MASK | > + GPIO9_DIR_MASK | > + GPIO10_DIR_MASK | > + GPIO11_DIR_MASK, > + GPIO8_DIR_INPUT | > + GPIO9_DIR_INPUT | > + GPIO10_DIR_INPUT | > + GPIO11_DIR_INPUT); > + } > +} > + > +static void set_capture_gpio(struct mt6357_priv *priv, bool enable) > +{ same comment applies here. > + if (enable) { > + /* set gpio miso mode */ > + regmap_write(priv->regmap, MT6357_GPIO_MODE3_CLR, GPIO_MODE3_CLEAR_ALL); > + regmap_write(priv->regmap, MT6357_GPIO_MODE3_SET, GPIO12_MODE_SET_AUD_CLK_MISO | > + GPIO13_MODE_SET_AUD_DAT_MISO0 | > + GPIO14_MODE_SET_AUD_DAT_MISO1 | > + GPIO15_MODE_SET_AUD_SYNC_MISO); > + regmap_write(priv->regmap, MT6357_GPIO_MODE3, GPIO12_MODE_AUD_CLK_MISO | > + GPIO13_MODE_AUD_DAT_MISO0 | > + GPIO14_MODE_AUD_DAT_MISO1 | > + GPIO15_MODE_AUD_SYNC_MISO); > + } else { > + /* set pad_aud_*_miso to GPIO mode and dir input > + * reason: > + * pad_aud_clk_miso, because when playback only the miso_clk > + * will also have 26m, so will have power leak > + * pad_aud_dat_miso*, because the pin is used as boot strap > + */ > + regmap_write(priv->regmap, MT6357_GPIO_MODE3_CLR, GPIO_MODE3_CLEAR_ALL); > + regmap_write(priv->regmap, MT6357_GPIO_MODE3, GPIO12_MODE_GPIO | > + GPIO13_MODE_GPIO | > + GPIO14_MODE_GPIO | > + GPIO15_MODE_GPIO); > + regmap_update_bits(priv->regmap, MT6357_GPIO_DIR0, GPIO12_DIR_MASK | > + GPIO13_DIR_MASK | > + GPIO14_DIR_MASK | > + GPIO15_DIR_MASK, > + GPIO12_DIR_INPUT | > + GPIO13_DIR_INPUT | > + GPIO14_DIR_INPUT | > + GPIO15_DIR_INPUT); > + } > +} > + > +static void hp_main_output_ramp(struct mt6357_priv *priv, bool up) > +{ > + int i = 0, stage = 0; > + int target = 7; > + /* Enable/Reduce HPL/R main output stage step by step */ > + for (i = 0; i <= target; i++) { double 'i` initialization. > + stage = up ? i : target - i; > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON1, > + HPLOUT_STG_CTRL_VAUDP15_MASK, > + stage << HPLOUT_STG_CTRL_VAUDP15_SFT); > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON1, > + HPROUT_STG_CTRL_VAUDP15_MASK, > + stage << HPROUT_STG_CTRL_VAUDP15_SFT); > + usleep_range(600, 700); > + } > +} > + > +static void hp_aux_feedback_loop_gain_ramp(struct mt6357_priv *priv, bool up) > +{ > + int i = 0, stage = 0; > + /* Reduce HP aux feedback loop gain step by step */ > + for (i = 0; i <= 0xf; i++) { same > + stage = up ? i : 0xf - i; > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON6, > + HP_AUX_LOOP_GAIN_MASK, stage << HP_AUX_LOOP_GAIN_SFT); > + usleep_range(600, 700); > + } > +} > + > +static void hp_pull_down(struct mt6357_priv *priv, bool enable) > +{ > + if (enable) > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON2, > + HPP_SHORT_2VCM_VAUDP15_MASK, HPP_SHORT_2VCM_VAUDP15_ENABLE); > + else > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON2, > + HPP_SHORT_2VCM_VAUDP15_MASK, HPP_SHORT_2VCM_VAUDP15_DISABLE); > +} > + > +static bool is_valid_hp_pga_idx(int reg_idx) > +{ > + return (reg_idx >= DL_GAIN_8DB && reg_idx <= DL_GAIN_N_12DB) || reg_idx == DL_GAIN_N_40DB; > +} > + > +static void volume_ramp(struct mt6357_priv *priv, int lfrom, int lto, > + int rfrom, int rto, unsigned int reg_addr) > +{ > + int lcount, rcount, sleep = 0; > + > + if (!is_valid_hp_pga_idx(lfrom) || !is_valid_hp_pga_idx(lto)) > + pr_debug("%s(), invalid left volume index, from %d, to %d\n", > + __func__, lfrom, lto); > + > + if (!is_valid_hp_pga_idx(rfrom) || !is_valid_hp_pga_idx(rto)) > + pr_debug("%s(), invalid right volume index, from %d, to %d\n", > + __func__, rfrom, rto); > + > + if (lto > lfrom) > + lcount = 1; > + else > + lcount = -1; > + > + if (rto > rfrom) > + rcount = 1; > + else > + rcount = -1; > + > + while ((lto != lfrom) || (rto != rfrom)) { > + if (lto != lfrom) { > + lfrom += lcount; > + if (is_valid_hp_pga_idx(lfrom)) { > + regmap_update_bits(priv->regmap, reg_addr, DL_GAIN_REG_LEFT_MASK, > + lfrom << DL_GAIN_REG_LEFT_SHIFT); > + sleep = 1; > + } > + } > + if (rto != rfrom) { > + rfrom += rcount; > + if (is_valid_hp_pga_idx(rfrom)) { > + regmap_update_bits(priv->regmap, reg_addr, DL_GAIN_REG_RIGHT_MASK, > + rfrom << DL_GAIN_REG_RIGHT_SHIFT); > + sleep = 1; > + } > + } > + if (sleep) > + usleep_range(200, 300); > + } > +} > + > +static void lo_volume_ramp(struct mt6357_priv *priv, int lfrom, int lto, int rfrom, int rto) > +{ > + volume_ramp(priv, lfrom, lto, rfrom, rto, MT6357_ZCD_CON1); > +} > + > +static void hp_volume_ramp(struct mt6357_priv *priv, int lfrom, int lto, int rfrom, int rto) > +{ > + volume_ramp(priv, lfrom, lto, rfrom, rto, MT6357_ZCD_CON2); > +} > + > +static void hs_volume_ramp(struct mt6357_priv *priv, int from, int to) > +{ > + volume_ramp(priv, from, to, 0, 0, MT6357_ZCD_CON3); > +} > + > +static int mt6357_put_volsw(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) > +{ > + struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol); > + struct mt6357_priv *priv = snd_soc_component_get_drvdata(component); > + struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; > + unsigned int reg; > + int ret; > + > + ret = snd_soc_put_volsw(kcontrol, ucontrol); > + if (ret < 0) > + return ret; > + regmap_read(priv->regmap, mc->reg, &reg) switch (mc->reg) { ... }; return 0; > + switch (mc->reg) { > + case MT6357_ZCD_CON2: > + regmap_read(priv->regmap, MT6357_ZCD_CON2, &reg); > + priv->ana_gain[ANALOG_VOLUME_HPOUTL] = > + (reg & AUD_HPL_GAIN_MASK) >> AUD_HPL_GAIN_SFT; > + priv->ana_gain[ANALOG_VOLUME_HPOUTR] = > + (reg & AUD_HPR_GAIN_MASK) >> AUD_HPR_GAIN_SFT; > + break; > + case MT6357_ZCD_CON1: > + regmap_read(priv->regmap, MT6357_ZCD_CON1, &reg); > + priv->ana_gain[ANALOG_VOLUME_LINEOUTL] = > + (reg & AUD_LOL_GAIN_MASK) >> AUD_LOL_GAIN_SFT; > + priv->ana_gain[ANALOG_VOLUME_LINEOUTR] = > + (reg & AUD_LOR_GAIN_MASK) >> AUD_LOR_GAIN_SFT; > + break; > + case MT6357_ZCD_CON3: > + regmap_read(priv->regmap, MT6357_ZCD_CON3, &reg); > + priv->ana_gain[ANALOG_VOLUME_HSOUT] = > + (reg & AUD_HS_GAIN_MASK) >> AUD_HS_GAIN_SFT; > + break; > + case MT6357_AUDENC_ANA_CON0: > + case MT6357_AUDENC_ANA_CON1: > + regmap_read(priv->regmap, MT6357_AUDENC_ANA_CON0, &reg); > + priv->ana_gain[ANALOG_VOLUME_MIC1] = > + (reg & AUDPREAMPLGAIN_MASK) >> AUDPREAMPLGAIN_SFT; > + regmap_read(priv->regmap, MT6357_AUDENC_ANA_CON1, &reg); > + priv->ana_gain[ANALOG_VOLUME_MIC2] = > + (reg & AUDPREAMPRGAIN_MASK) >> AUDPREAMPRGAIN_SFT; > + break; > + } > + > + return ret; > +} > + ..snip.. > + > +static int adc_enable_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 mt6357_priv *priv = snd_soc_component_get_drvdata(cmpnt); > + > + dev_dbg(priv->dev, "%s(), event = 0x%x\n", __func__, event); > + switch (event) { > + case SND_SOC_DAPM_PRE_PMU: > + /* enable aud_pad TX fifos */ > + regmap_update_bits(priv->regmap, MT6357_AFE_AUD_PAD_TOP, > + AUD_PAD_TX_FIFO_NORMAL_PATH_MASK, > + AUD_PAD_TX_FIFO_NORMAL_PATH_ENABLE); > + /* UL turn on */ > + regmap_update_bits(priv->regmap, MT6357_AFE_UL_SRC_CON0_L, > + UL_SRC_ON_TMP_CTL_MASK, UL_SRC_ENABLE); > + /* Wait to avoid any pop noises */ > + msleep(100); > + //set the mic gains to the stored values Keep the comments style consistent please. > + regmap_update_bits(priv->regmap, MT6357_AUDENC_ANA_CON0, AUDPREAMPLGAIN_MASK, > + priv->ana_gain[ANALOG_VOLUME_MIC1] << AUDPREAMPLGAIN_SFT); > + regmap_update_bits(priv->regmap, MT6357_AUDENC_ANA_CON1, AUDPREAMPRGAIN_MASK, > + priv->ana_gain[ANALOG_VOLUME_MIC2] << AUDPREAMPRGAIN_SFT); > + break; > + case SND_SOC_DAPM_POST_PMD: > + /* UL turn off */ > + regmap_update_bits(priv->regmap, MT6357_AFE_UL_SRC_CON0_L, > + UL_SRC_ON_TMP_CTL_MASK, UL_SRC_DISABLE); > + /* disable aud_pad TX fifos */ > + regmap_update_bits(priv->regmap, MT6357_AFE_AUD_PAD_TOP, > + AUD_PAD_TX_FIFO_NORMAL_PATH_MASK, > + AUD_PAD_TX_FIFO_NORMAL_PATH_DISABLE); > + break; > + default: > + break; > + } > + > + return 0; > +} > + ..snip.. > + > +static int lol_mux_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 mt6357_priv *priv = snd_soc_component_get_drvdata(cmpnt); > + > + dev_dbg(priv->dev, "%s(), event 0x%x\n", __func__, event); > + switch (event) { > + case SND_SOC_DAPM_POST_PMU: > + /* Set LO STB enhance circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON4, > + AUD_LOLOUT_STB_ENH_VAUDP15_MASK, > + AUD_LOLOUT_STB_ENH_VAUDP15_ENABLE); > + /* Enable LO driver bias circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON4, > + AUD_LOL_PWRUP_BIAS_VAUDP15_MASK, > + AUD_LOL_PWRUP_BIAS_VAUDP15_ENABLE); > + /* Enable LO driver core circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON4, > + AUD_LOL_PWRUP_VAUDP15_MASK, AUD_LOL_PWRUP_VAUDP15_ENABLE); > + /* Set LOL gain to normal gain step by step */ > + lo_volume_ramp(priv, DL_GAIN_N_40DB, priv->ana_gain[ANALOG_VOLUME_LINEOUTL], > + DL_GAIN_N_40DB, priv->ana_gain[ANALOG_VOLUME_LINEOUTR]); > + break; > + case SND_SOC_DAPM_PRE_PMD: > + Drop extra blank line. > + /* decrease LOL gain to minimum gain step by step */ > + lo_volume_ramp(priv, priv->ana_gain[ANALOG_VOLUME_LINEOUTL], DL_GAIN_N_40DB, > + priv->ana_gain[ANALOG_VOLUME_LINEOUTR], DL_GAIN_N_40DB); > + /* Disable LO driver core circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON4, > + AUD_LOL_PWRUP_VAUDP15_MASK, AUD_LOL_PWRUP_VAUDP15_DISABLE); > + /* Disable LO driver bias circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON4, > + AUD_LOL_PWRUP_BIAS_VAUDP15_MASK, > + AUD_LOL_PWRUP_BIAS_VAUDP15_DISABLE); > + /* Clear LO STB enhance circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON4, > + AUD_LOLOUT_STB_ENH_VAUDP15_MASK, > + AUD_LOLOUT_STB_ENH_VAUDP15_DISABLE); > + break; > + default: > + break; > + } > + > + return 0; > +} > + > +static int hs_mux_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 mt6357_priv *priv = snd_soc_component_get_drvdata(cmpnt); > + > + dev_dbg(priv->dev, "%s(), event 0x%x\n", __func__, event); > + switch (event) { > + case SND_SOC_DAPM_POST_PMU: > + > + /* Set HS STB enhance circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON3, > + AUD_HSOUT_STB_ENH_VAUDP15_MASK, > + AUD_HSOUT_STB_ENH_VAUDP15_ENABLE); > + /* Enable HS driver bias circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON3, > + AUD_HS_PWRUP_BIAS_VAUDP15_MASK, > + AUD_HS_PWRUP_BIAS_VAUDP15_ENABLE); > + /* Enable HS driver core circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON3, > + AUD_HS_PWRUP_VAUDP15_MASK, AUD_HS_PWRUP_VAUDP15_ENABLE); > + /* Set HS gain to normal gain step by step */ > + hs_volume_ramp(priv, DL_GAIN_N_40DB, priv->ana_gain[ANALOG_VOLUME_HSOUT]); > + break; > + case SND_SOC_DAPM_PRE_PMD: > + same > + /* decrease HS gain to minimum gain step by step */ > + hs_volume_ramp(priv, priv->ana_gain[ANALOG_VOLUME_HSOUT], DL_GAIN_N_40DB); > + /* Disable HS driver core circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON3, > + AUD_HS_PWRUP_VAUDP15_MASK, AUD_HS_PWRUP_VAUDP15_DISABLE); > + /* Disable HS driver bias circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON3, > + AUD_HS_PWRUP_BIAS_VAUDP15_MASK, > + AUD_HS_PWRUP_BIAS_VAUDP15_ENABLE); > + /* Clear HS STB enhance circuits */ > + regmap_update_bits(priv->regmap, MT6357_AUDDEC_ANA_CON3, > + AUD_HSOUT_STB_ENH_VAUDP15_MASK, > + AUD_HSOUT_STB_ENH_VAUDP15_DISABLE); > + break; > + default: > + break; > + } > + > + return 0; > +} > + ..snip.. > + > +static unsigned int mt6357_read(struct snd_soc_component *codec, unsigned int reg) > +{ > + struct mt6357_priv *priv = snd_soc_component_get_drvdata(codec); > + unsigned int val; > + > + pr_debug("%s() reg = 0x%x", __func__, reg); I'm not sure that you really need that pr_debug().... > + regmap_read(priv->regmap, reg, &val); > + return val; > +} > + > +static int mt6357_write(struct snd_soc_component *codec, unsigned int reg, unsigned int value) > +{ > + struct mt6357_priv *priv = snd_soc_component_get_drvdata(codec); > + > + pr_debug("%s() reg = 0x%x value= 0x%x\n", __func__, reg, value); ...nor this one. > + regmap_update_bits(priv->regmap, reg, 0xffff, value); > + return 0; return regmap_update_bits(...); > +} > + > +static const struct snd_soc_component_driver mt6357_soc_component_driver = { > + .probe = mt6357_codec_probe, > + .read = mt6357_read, > + .write = mt6357_write, > + .controls = mt6357_controls, > + .num_controls = ARRAY_SIZE(mt6357_controls), > + .dapm_widgets = mt6357_dapm_widgets, > + .num_dapm_widgets = ARRAY_SIZE(mt6357_dapm_widgets), > + .dapm_routes = mt6357_dapm_routes, > + .num_dapm_routes = ARRAY_SIZE(mt6357_dapm_routes), > +}; > + > +static void mt6357_parse_dt(struct mt6357_priv *priv) > +{ > + int ret, voltage_index; > + u32 micbias_voltage_index = 0; > + struct device *dev = priv->dev; > + > + priv->pull_down_needed = false; > + if (of_property_read_bool(dev->of_node, "mediatek,hp-pull-down")) > + priv->pull_down_needed = true; > + > + micbias_voltage_index = MT6357_MICBIAS0_DEFAULT_VOLTAGE_INDEX; > + ret = of_property_read_u32(dev->of_node, "mediatek,micbias0-microvolt", &voltage_index); > + if (!ret) > + micbias_voltage_index = voltage_index; > + > + regmap_update_bits(priv->regmap, MT6357_AUDENC_ANA_CON8, AUD_MICBIAS0_VREF_MASK, > + micbias_voltage_index << AUD_MICBIAS0_VREF_SFT); > + > + micbias_voltage_index = MT6357_MICBIAS1_DEFAULT_VOLTAGE_INDEX; > + ret = of_property_read_u32(dev->of_node, "mediatek,micbias1-microvolt", &voltage_index); > + if (!ret) > + micbias_voltage_index = voltage_index; > + > + regmap_update_bits(priv->regmap, MT6357_AUDENC_ANA_CON9, AUD_MICBIAS1_VREF_MASK, > + micbias_voltage_index << AUD_MICBIAS1_VREF_SFT); > +} > + > +static int mt6357_platform_driver_probe(struct platform_device *pdev) > +{ > + struct mt6357_priv *priv; > + struct mt6397_chip *mt6397 = dev_get_drvdata(pdev->dev.parent); struct mt6397_chip *mt6397 = dev_get_drvdata(pdev->dev.parent); struct mt6357_priv *priv; int ret; looks better :-) > + int ret; > + > + ret = devm_regulator_get_enable(&pdev->dev, "vaud28"); > + if (ret) > + return dev_err_probe(&pdev->dev, ret, "failed to enable vaud28 regulator\n"); > + > + priv = devm_kzalloc(&pdev->dev, > + sizeof(struct mt6357_priv), > + GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + dev_set_drvdata(&pdev->dev, priv); > + priv->dev = &pdev->dev; > + > + priv->regmap = mt6397->regmap; > + if (IS_ERR(priv->regmap)) > + return PTR_ERR(priv->regmap); > + > + mt6357_parse_dt(priv); > + > + pdev->dev.coherent_dma_mask = DMA_BIT_MASK(64); > + if (!pdev->dev.dma_mask) > + pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask; > + > + return devm_snd_soc_register_component(&pdev->dev, > + &mt6357_soc_component_driver, > + mtk_6357_dai_codecs, > + ARRAY_SIZE(mtk_6357_dai_codecs)); > +} > + > +static const struct of_device_id mt6357_of_match[] = { > + {.compatible = "mediatek,mt6357-sound",}, > + {} { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, mt6357_of_match); > + > +static struct platform_driver mt6357_platform_driver = { > + .driver = { > + .name = "mt6357-sound", > + .of_match_table = mt6357_of_match, > + }, > + .probe = mt6357_platform_driver_probe, > +}; > + > +module_platform_driver(mt6357_platform_driver) > + > +MODULE_DESCRIPTION("MT6357 ALSA SoC codec driver"); > +MODULE_AUTHOR("Nicolas Belin <nbelin(a)baylibre.com>"); > +MODULE_LICENSE("GPL"); Cheers, Angelo

1 year, 4 months

Re: [PATCH 10/18] ASoc: mediatek: mt8365: Add a specific soundcard for EVK

by AngeloGioacchino Del Regno

Il 26/02/24 15:01, amergnat(a)baylibre.com ha scritto: > From: Nicolas Belin <nbelin(a)baylibre.com> > > Add a specific soundcard for mt8365-evk. It supports audio jack > in/out, dmics, the amic and lineout. > > Signed-off-by: Nicolas Belin <nbelin(a)baylibre.com> > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> > --- > sound/soc/mediatek/mt8365/mt8365-mt6357.c | 379 ++++++++++++++++++++++++++++++ > 1 file changed, 379 insertions(+) > > diff --git a/sound/soc/mediatek/mt8365/mt8365-mt6357.c b/sound/soc/mediatek/mt8365/mt8365-mt6357.c > new file mode 100644 > index 000000000000..5192b35458e6 > --- /dev/null > +++ b/sound/soc/mediatek/mt8365/mt8365-mt6357.c > @@ -0,0 +1,379 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Mediatek MT8365 Sound Card driver > + * > + * Copyright (c) 2024 MediaTek Inc. > + * Authors: Nicolas Belin <nbelin(a)baylibre.com> > + */ > + > +#include <linux/module.h> > +#include <linux/of_gpio.h> > +#include <sound/soc.h> > +#include <sound/pcm_params.h> > +#include "mt8365-afe-common.h" > +#include <linux/pinctrl/consumer.h> > +#include "../common/mtk-soundcard-driver.h" > + > +enum PINCTRL_PIN_STATE { Aren't those supposed to be AFE pin states? Anyway, lower case name for enums please, and no zero initializer for the first entry. enum mt8365_pin_state { PIN_STATE_DEFAULT, .... }; > + PIN_STATE_DEFAULT = 0, > + PIN_STATE_DMIC, > + PIN_STATE_MISO_OFF, > + PIN_STATE_MISO_ON, > + PIN_STATE_MOSI_OFF, > + PIN_STATE_MOSI_ON, > + PIN_STATE_MAX > +}; > + > +static const char * const mt8365_mt6357_pin_str[PIN_STATE_MAX] = { > + "aud_default", > + "aud_dmic", > + "aud_miso_off", > + "aud_miso_on", > + "aud_mosi_off", > + "aud_mosi_on", > +}; > + > +struct mt8365_mt6357_priv { > + struct pinctrl *pinctrl; > + struct pinctrl_state *pin_states[PIN_STATE_MAX]; > +}; > + > +struct mt8365_dai_link_prop { > + char *name; > + unsigned int link_id; > +}; This structure is unused. > + > +enum { > + /* FE */ > + DAI_LINK_DL1_PLAYBACK = 0, > + DAI_LINK_DL2_PLAYBACK, > + DAI_LINK_AWB_CAPTURE, > + DAI_LINK_VUL_CAPTURE, > + /* BE */ > + DAI_LINK_2ND_I2S_INTF, > + DAI_LINK_DMIC, > + DAI_LINK_INT_ADDA, > + DAI_LINK_NUM > +}; > + > +static const struct snd_soc_dapm_widget mt8365_mt6357_widgets[] = { > + SND_SOC_DAPM_OUTPUT("HDMI Out"), > +}; > + > +static const struct snd_soc_dapm_route mt8365_mt6357_routes[] = { > + {"HDMI Out", NULL, "2ND I2S Playback"}, > + {"DMIC In", NULL, "MICBIAS0"}, > +}; > + > +static int mt8365_mt6357_int_adda_startup(struct snd_pcm_substream *substream) > +{ > + struct snd_soc_pcm_runtime *rtd = substream->private_data; > + struct mt8365_mt6357_priv *priv = snd_soc_card_get_drvdata(rtd->card); > + int ret = 0; > + > + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { > + if (IS_ERR(priv->pin_states[PIN_STATE_MOSI_ON])) > + return ret; > + > + ret = pinctrl_select_state(priv->pinctrl, > + priv->pin_states[PIN_STATE_MOSI_ON]); > + if (ret) > + dev_err(rtd->card->dev, "%s failed to select state %d\n", > + __func__, ret); > + } > + > + if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) { > + if (IS_ERR(priv->pin_states[PIN_STATE_MISO_ON])) > + return ret; > + > + ret = pinctrl_select_state(priv->pinctrl, > + priv->pin_states[PIN_STATE_MISO_ON]); > + if (ret) > + dev_err(rtd->card->dev, "%s failed to select state %d\n", > + __func__, ret); > + } > + > + return 0; > +} > + > +static void mt8365_mt6357_int_adda_shutdown(struct snd_pcm_substream *substream) > +{ > + struct snd_soc_pcm_runtime *rtd = substream->private_data; > + struct mt8365_mt6357_priv *priv = snd_soc_card_get_drvdata(rtd->card); > + int ret = 0; > + > + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { > + if (IS_ERR(priv->pin_states[PIN_STATE_MOSI_OFF])) > + return; > + > + ret = pinctrl_select_state(priv->pinctrl, > + priv->pin_states[PIN_STATE_MOSI_OFF]); > + if (ret) > + dev_err(rtd->card->dev, "%s failed to select state %d\n", > + __func__, ret); > + } > + > + if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) { > + if (IS_ERR(priv->pin_states[PIN_STATE_MISO_OFF])) > + return; > + > + ret = pinctrl_select_state(priv->pinctrl, > + priv->pin_states[PIN_STATE_MISO_OFF]); > + if (ret) > + dev_err(rtd->card->dev, "%s failed to select state %d\n", > + __func__, ret); > + } > + > +} > + > +static const struct snd_soc_ops mt8365_mt6357_int_adda_ops = { > + .startup = mt8365_mt6357_int_adda_startup, > + .shutdown = mt8365_mt6357_int_adda_shutdown, > +}; > + > +SND_SOC_DAILINK_DEFS(playback1, > + DAILINK_COMP_ARRAY(COMP_CPU("DL1")), > + DAILINK_COMP_ARRAY(COMP_DUMMY()), > + DAILINK_COMP_ARRAY(COMP_EMPTY())); > +SND_SOC_DAILINK_DEFS(playback2, > + DAILINK_COMP_ARRAY(COMP_CPU("DL2")), > + DAILINK_COMP_ARRAY(COMP_DUMMY()), > + DAILINK_COMP_ARRAY(COMP_EMPTY())); > +SND_SOC_DAILINK_DEFS(awb_capture, > + DAILINK_COMP_ARRAY(COMP_CPU("AWB")), > + DAILINK_COMP_ARRAY(COMP_DUMMY()), > + DAILINK_COMP_ARRAY(COMP_EMPTY())); > +SND_SOC_DAILINK_DEFS(vul, > + DAILINK_COMP_ARRAY(COMP_CPU("VUL")), > + DAILINK_COMP_ARRAY(COMP_DUMMY()), > + DAILINK_COMP_ARRAY(COMP_EMPTY())); > + > +SND_SOC_DAILINK_DEFS(i2s3, > + DAILINK_COMP_ARRAY(COMP_CPU("2ND I2S")), > + DAILINK_COMP_ARRAY(COMP_DUMMY()), > + DAILINK_COMP_ARRAY(COMP_EMPTY())); > +SND_SOC_DAILINK_DEFS(dmic, > + DAILINK_COMP_ARRAY(COMP_CPU("DMIC")), > + DAILINK_COMP_ARRAY(COMP_DUMMY()), > + DAILINK_COMP_ARRAY(COMP_EMPTY())); > +SND_SOC_DAILINK_DEFS(primary_codec, > + DAILINK_COMP_ARRAY(COMP_CPU("INT ADDA")), > + DAILINK_COMP_ARRAY(COMP_CODEC("mt6357-sound", "mt6357-snd-codec-aif1")), > + DAILINK_COMP_ARRAY(COMP_EMPTY())); > + > +/* Digital audio interface glue - connects codec <---> CPU */ > +static struct snd_soc_dai_link mt8365_mt6357_dais[] = { > + /* Front End DAI links */ > + [DAI_LINK_DL1_PLAYBACK] = { > + .name = "DL1_FE", > + .stream_name = "MultiMedia1_PLayback", > + .id = DAI_LINK_DL1_PLAYBACK, > + .trigger = { > + SND_SOC_DPCM_TRIGGER_POST, > + SND_SOC_DPCM_TRIGGER_POST > + }, > + .dynamic = 1, > + .dpcm_playback = 1, > + .dpcm_merged_rate = 1, > + SND_SOC_DAILINK_REG(playback1), > + }, > + [DAI_LINK_DL2_PLAYBACK] = { > + .name = "DL2_FE", > + .stream_name = "MultiMedia2_PLayback", > + .id = DAI_LINK_DL2_PLAYBACK, > + .trigger = { > + SND_SOC_DPCM_TRIGGER_POST, > + SND_SOC_DPCM_TRIGGER_POST > + }, > + .dynamic = 1, > + .dpcm_playback = 1, > + .dpcm_merged_rate = 1, > + SND_SOC_DAILINK_REG(playback2), > + }, > + [DAI_LINK_AWB_CAPTURE] = { > + .name = "AWB_FE", > + .stream_name = "DL1_AWB_Record", > + .id = DAI_LINK_AWB_CAPTURE, > + .trigger = { > + SND_SOC_DPCM_TRIGGER_POST, > + SND_SOC_DPCM_TRIGGER_POST > + }, > + .dynamic = 1, > + .dpcm_capture = 1, > + .dpcm_merged_rate = 1, > + SND_SOC_DAILINK_REG(awb_capture), > + }, > + [DAI_LINK_VUL_CAPTURE] = { > + .name = "VUL_FE", > + .stream_name = "MultiMedia1_Capture", > + .id = DAI_LINK_VUL_CAPTURE, > + .trigger = { > + SND_SOC_DPCM_TRIGGER_POST, > + SND_SOC_DPCM_TRIGGER_POST > + }, > + .dynamic = 1, > + .dpcm_capture = 1, > + .dpcm_merged_rate = 1, > + SND_SOC_DAILINK_REG(vul), > + }, > + /* Back End DAI links */ > + [DAI_LINK_2ND_I2S_INTF] = { > + .name = "2ND I2S BE", > + .no_pcm = 1, > + .id = DAI_LINK_2ND_I2S_INTF, > + .dai_fmt = SND_SOC_DAIFMT_I2S | > + SND_SOC_DAIFMT_NB_NF | > + SND_SOC_DAIFMT_CBS_CFS, > + .dpcm_playback = 1, > + .dpcm_capture = 1, > + SND_SOC_DAILINK_REG(i2s3), > + }, > + [DAI_LINK_DMIC] = { > + .name = "DMIC BE", > + .no_pcm = 1, > + .id = DAI_LINK_DMIC, > + .dpcm_capture = 1, > + SND_SOC_DAILINK_REG(dmic), > + }, > + [DAI_LINK_INT_ADDA] = { > + .name = "MTK Codec", > + .no_pcm = 1, > + .id = DAI_LINK_INT_ADDA, > + .dpcm_playback = 1, > + .dpcm_capture = 1, > + .ops = &mt8365_mt6357_int_adda_ops, > + SND_SOC_DAILINK_REG(primary_codec), > + }, > +}; > + > +static int pinctrl_setup(struct snd_soc_card *card, > + struct mt8365_mt6357_priv *priv, > + unsigned int pin_id) > +{ > + int ret = 0; > + /* if pin exist */ > + if (!IS_ERR(priv->pin_states[pin_id])) { > + ret = pinctrl_select_state(priv->pinctrl, > + priv->pin_states[pin_id]); > + if (ret) > + dev_err(card->dev, > + "%s failed to select state %d\n", > + __func__, ret); > + } > + return ret; > +} > + > +static int mt8365_mt6357_gpio_probe(struct snd_soc_card *card) > +{ > + struct mt8365_mt6357_priv *priv = snd_soc_card_get_drvdata(card); > + int ret = 0; > + int i; > + > + priv->pinctrl = devm_pinctrl_get(card->dev); > + if (IS_ERR(priv->pinctrl)) { > + ret = PTR_ERR(priv->pinctrl); > + dev_err(card->dev, "%s devm_pinctrl_get failed %d\n", > + __func__, ret); > + return ret; > + } > + > + for (i = 0 ; i < PIN_STATE_MAX ; i++) { > + priv->pin_states[i] = pinctrl_lookup_state(priv->pinctrl, > + mt8365_mt6357_pin_str[i]); > + if (IS_ERR(priv->pin_states[i])) { > + ret = PTR_ERR(priv->pin_states[i]); > + dev_err(card->dev, "%s Can't find pin state %s %d\n", > + __func__, mt8365_mt6357_pin_str[i], ret); > + } > + } > + > + /* Setup pins */ for (i = PIN_STATE_DEFAULT; i < PIN_STATE_MAX; i++) { /* Are you sure about that?!?! */ if (IS_ERR(priv->pin_states[i])) continue; ret = pinctrl_select_state(...) if (ret) { /* Should you go back to default? */ pinctrl_select_state(card, priv, PIN_STATE_DEFAULT); return dev_err_probe(card->dev, ret, "ARGH!\n"); /* :-) */ } }; > + ret |= pinctrl_setup(card, priv, PIN_STATE_DEFAULT); ....because OR'in return values is a big no. > + ret |= pinctrl_setup(card, priv, PIN_STATE_DMIC); > + ret |= pinctrl_setup(card, priv, PIN_STATE_MISO_OFF); > + ret |= pinctrl_setup(card, priv, PIN_STATE_MOSI_OFF); > + return 0; > + return ret; > +} > + > +static struct snd_soc_card mt8365_mt6357_card = { > + .name = "mt8365-evk", > + .owner = THIS_MODULE, > + .dai_link = mt8365_mt6357_dais, > + .num_links = ARRAY_SIZE(mt8365_mt6357_dais), > + .dapm_widgets = mt8365_mt6357_widgets, > + .num_dapm_widgets = ARRAY_SIZE(mt8365_mt6357_widgets), > + .dapm_routes = mt8365_mt6357_routes, > + .num_dapm_routes = ARRAY_SIZE(mt8365_mt6357_routes), > +}; > + > +static int mt8365_mt6357_dev_probe(struct platform_device *pdev) > +{ > + struct snd_soc_card *card = &mt8365_mt6357_card; > + struct device *dev = &pdev->dev; > + struct device_node *platform_node; > + struct mt8365_mt6357_priv *priv; > + int i, ret; > + > + card->dev = dev; > + ret = parse_dai_link_info(card); > + if (ret) > + goto err; > + > + platform_node = of_parse_phandle(dev->of_node, "mediatek,platform", 0); > + if (!platform_node) { > + dev_err(dev, "Property 'platform' missing or invalid\n"); > + return -EINVAL; > + } > + > + for (i = 0; i < card->num_links; i++) { > + if (mt8365_mt6357_dais[i].platforms->name) > + continue; > + mt8365_mt6357_dais[i].platforms->of_node = platform_node; > + } > + > + priv = devm_kzalloc(dev, sizeof(struct mt8365_mt6357_priv), > + GFP_KERNEL); > + if (!priv) { > + ret = -ENOMEM; > + dev_err(dev, "%s allocate card private data fail %d\n", > + __func__, ret); > + return ret; > + } > + > + snd_soc_card_set_drvdata(card, priv); > + > + mt8365_mt6357_gpio_probe(card); P.S.: with my cleanup this function would not need any parse_dai_link_info() call, nor phandle parsing, nor platform_node assignment, nor any call to devm_snd_soc_register_card(), nor.... ...practically you'd need to only allocate your priv (granted that you are not moving the gpio_probe() to the AFE driver) and the actual gpio_probe() call. But I'm not pressing to wait, anyway. Cheers, Angelo

1 year, 4 months

Re: [PATCH 00/18] Add audio support for the MediaTek Genio 350-evk board

by AngeloGioacchino Del Regno

Il 26/02/24 15:01, Alexandre Mergnat ha scritto: > This serie aim to add the following audio support for the Genio 350-evk: > - Playback > - 2ch Headset Jack (Earphone) > - 1ch Line-out Jack (Speaker) > - 8ch HDMI Tx > - Capture > - 1ch DMIC (On-board Digital Microphone) > - 1ch AMIC (On-board Analogic Microphone) > - 1ch Headset Jack (External Analogic Microphone) > > Of course, HDMI playback need the MT8365 display patches [1] and a DTS > change documented in "mediatek,mt8365-mt6357.yaml". > > [1]: https://lore.kernel.org/all/20231023-display-support-v1-0-5c860ed5c33b@bayl… > > Signed-off-by: Alexandre Mergnat <amergnat(a)baylibre.com> Actually, I am cooking a series (I'm finishing the testing....) that brings quite a bit of cleanups in MTK ASoC, including the commonization of the machine driver probe, with the dai-link DT nodes, and which also modernizes most of the existing drivers to use that instead. If you wait for a day or two, your mt8365-mt6357.c driver's probe function can be shrunk to ~3 lines or something like that.. very easily :-) Cheers, Angelo > --- > Alexandre Mergnat (15): > ASoC: dt-bindings: mediatek,mt8365-afe: Add audio afe document > ASoC: dt-bindings: mediatek,mt8365-mt6357: Add audio sound card document > dt-bindings: mfd: mediatek: Add codec property for MT6357 PMIC > ASoC: mediatek: mt8365: Add common header > SoC: mediatek: mt8365: support audio clock control > ASoC: mediatek: mt8365: Add I2S DAI support > ASoC: mediatek: mt8365: Add ADDA DAI support > ASoC: mediatek: mt8365: Add DMIC DAI support > ASoC: mediatek: mt8365: Add PCM DAI support > ASoC: mediatek: mt8365: Add platform driver > ASoC: mediatek: Add MT8365 support > arm64: defconfig: enable mt8365 sound > arm64: dts: mediatek: add mt6357 audio codec support > arm64: dts: mediatek: add afe support for mt8365 SoC > arm64: dts: mediatek: add audio support for mt8365-evk > > Fabien Parent (1): > mfd: mt6397-core: register mt6357 sound codec > > Nicolas Belin (2): > ASoc: mediatek: mt8365: Add a specific soundcard for EVK > ASoC: codecs: mt6357: add MT6357 codec > > .../devicetree/bindings/mfd/mediatek,mt6357.yaml | 11 + > .../bindings/sound/mediatek,mt8365-afe.yaml | 164 ++ > .../bindings/sound/mediatek,mt8365-mt6357.yaml | 127 ++ > arch/arm64/boot/dts/mediatek/mt6357.dtsi | 6 +- > arch/arm64/boot/dts/mediatek/mt8365-evk.dts | 95 +- > arch/arm64/boot/dts/mediatek/mt8365.dtsi | 47 +- > arch/arm64/configs/defconfig | 2 + > drivers/mfd/mt6397-core.c | 3 + > sound/soc/codecs/Kconfig | 7 + > sound/soc/codecs/Makefile | 2 + > sound/soc/codecs/mt6357.c | 1805 +++++++++++++++ > sound/soc/codecs/mt6357.h | 674 ++++++ > sound/soc/mediatek/Kconfig | 20 + > sound/soc/mediatek/Makefile | 1 + > sound/soc/mediatek/mt8365/Makefile | 15 + > sound/soc/mediatek/mt8365/mt8365-afe-clk.c | 455 ++++ > sound/soc/mediatek/mt8365/mt8365-afe-clk.h | 55 + > sound/soc/mediatek/mt8365/mt8365-afe-common.h | 495 +++++ > sound/soc/mediatek/mt8365/mt8365-afe-pcm.c | 2347 ++++++++++++++++++++ > sound/soc/mediatek/mt8365/mt8365-dai-adda.c | 355 +++ > sound/soc/mediatek/mt8365/mt8365-dai-dmic.c | 475 ++++ > sound/soc/mediatek/mt8365/mt8365-dai-i2s.c | 901 ++++++++ > sound/soc/mediatek/mt8365/mt8365-dai-pcm.c | 298 +++ > sound/soc/mediatek/mt8365/mt8365-mt6357.c | 379 ++++ > sound/soc/mediatek/mt8365/mt8365-reg.h | 987 ++++++++ > 25 files changed, 9718 insertions(+), 8 deletions(-) > --- > base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d > change-id: 20240226-audio-i350-4e11da088e55 > > Best regards,

1 year, 4 months

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig