Linaro-mm-sig February 2024

linaro-mm-sig@lists.linaro.org

17 participants
45 discussions

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.

7 months, 3 weeks

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>; > + }; > + }; > + }; >

7 months, 3 weeks

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), >

7 months, 3 weeks

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

7 months, 3 weeks

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

7 months, 3 weeks

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,

7 months, 3 weeks

Re: [PATCH 11/11] drm/mediatek: Rename "pending_needs_vblank" to "needs_vblank"

by AngeloGioacchino Del Regno

Il 26/02/24 09:50, Shawn Sung ha scritto: > From: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> > > Rename "pending_needs_vblank" to "needs_vblank" to reduce the code size. > This decreases human readability though, so I would prefer that we omit this one. Regards, Angelo > Signed-off-by: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> > --- > drivers/gpu/drm/mediatek/mtk_crtc.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_crtc.c b/drivers/gpu/drm/mediatek/mtk_crtc.c > index 7fe234de83a3..a1fb6c67681d 100644 > --- a/drivers/gpu/drm/mediatek/mtk_crtc.c > +++ b/drivers/gpu/drm/mediatek/mtk_crtc.c > @@ -42,7 +42,7 @@ struct mtk_crtc { > struct drm_crtc base; > bool enabled; > > - bool pending_needs_vblank; > + bool needs_vblank; > struct drm_pending_vblank_event *event; > > struct drm_plane *planes; > @@ -105,9 +105,9 @@ static void mtk_crtc_finish_page_flip(struct mtk_crtc *mtk_crtc) > static void mtk_drm_finish_page_flip(struct mtk_crtc *mtk_crtc) > { > drm_crtc_handle_vblank(&mtk_crtc->base); > - if (!mtk_crtc->config_updating && mtk_crtc->pending_needs_vblank) { > + if (!mtk_crtc->config_updating && mtk_crtc->needs_vblank) { > mtk_crtc_finish_page_flip(mtk_crtc); > - mtk_crtc->pending_needs_vblank = false; > + mtk_crtc->needs_vblank = false; > } > } > > @@ -571,7 +571,7 @@ static void mtk_crtc_update_config(struct mtk_crtc *mtk_crtc, bool needs_vblank) > mutex_lock(&mtk_crtc->hw_lock); > mtk_crtc->config_updating = true; > if (needs_vblank) > - mtk_crtc->pending_needs_vblank = true; > + mtk_crtc->needs_vblank = true; > > for (i = 0; i < mtk_crtc->layer_nr; i++) { > struct drm_plane *plane = &mtk_crtc->planes[i];

7 months, 3 weeks

Re: [PATCH 05/11] drm/mediatek: Rename "mtk_drm_hdmi" to "mtk_hdmi"

by AngeloGioacchino Del Regno

Il 26/02/24 09:50, Shawn Sung ha scritto: > From: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> > > Rename all "mtk_drm_hdmi" to "mtk_hdmi": > - To align the naming rule > - To reduce the code size > > Signed-off-by: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> Reviewed-by: AngeloGiaocchino Del Regno <angelogioacchino.delregno(a)collabora.com>

7 months, 3 weeks

Re: [PATCH 01/11] drm/mediatek: Rename "mtk_drm_crtc" to "mtk_crtc"

by AngeloGioacchino Del Regno

Il 26/02/24 09:50, Shawn Sung ha scritto: > From: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> > > Rename all "mtk_drm_crtc" to "mtk_crtc" due to the following benefits: > - Lower the matches when searching the native drm_crtc* codes > - Reduce the code size > > Signed-off-by: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> Reviewed-by: AngeloGiaocchino Del Regno <angelogioacchino.delregno(a)collabora.com>

7 months, 3 weeks

Re: [PATCH 02/11] drm/mediatek: Rename "mtk_drm_ddp_comp" to "mtk_ddp_comp"

by AngeloGioacchino Del Regno

Il 26/02/24 09:50, Shawn Sung ha scritto: > From: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> > > Rename all "mtk_drm_ddp_comp" to "mtk_ddp_comp": > - To align the naming rule > - To reduce the code size > > Signed-off-by: Hsiao Chien Sung <shawn.sung(a)mediatek.corp-partner.google.com> Reviewed-by: AngeloGiaocchino Del Regno <angelogioacchino.delregno(a)collabora.com>

7 months, 3 weeks

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig February 2024