mbox series

[00/15] ASoC: mediatek: Add support for MT8186 SoC

Message ID 20220211103818.8266-1-jiaxin.yu@mediatek.com
Headers show
Series ASoC: mediatek: Add support for MT8186 SoC | expand

Message

Jiaxin Yu (俞家鑫) Feb. 11, 2022, 10:38 a.m. UTC 
This series of patches adds support for Mediatek AFE of MT8186 Soc.
Patches are based on broonie tree "for-next" branch.

Jiaxin Yu (15):
  ASoC: mediatek: mt6366: add codec driver
  ASoC: mediatek: mt8186: support audsys clock control
  ASoC: mediatek: mt8186: support adda in platform driver
  ASoC: mediatek: mt8186: support hostless in platform driver
  ASoC: mediatek: mt8186: support hw gain in platform driver
  ASoC: mediatek: mt8186: support i2s in platform driver
  ASoC: mediatek: mt8186: support pcm in platform driver
  ASoC: mediatek: mt8186: support src in platform driver
  ASoC: mediatek: mt8186: support tdm in platform driver
  ASoC: mediatek: mt8186: add platform driver
  dt-bindings: mediatek: mt8186: add audio afe document
  ASoC: mediatek: mt8186: add machine driver with mt6366, da7219 and
    max98357
  dt-bindings: mediatek: mt8186: add mt8186-mt6366-da7219-max98357
    document
  ASoC: mediatek: mt8186: add machine driver with mt6366, rt1019 and
    rt5682
  dt-bindings: mediatek: mt8186: add mt8186-mt6366-rt1019-rt5682
    document

 .../bindings/sound/mt8186-afe-pcm.yaml        |  175 +
 .../sound/mt8186-mt6366-da7219-max98357.yaml  |   47 +
 .../sound/mt8186-mt6366-rt1019-rt5682.yaml    |   47 +
 sound/soc/codecs/Kconfig                      |    8 +
 sound/soc/codecs/Makefile                     |    2 +
 sound/soc/mediatek/Kconfig                    |   44 +
 sound/soc/mediatek/Makefile                   |    1 +
 sound/soc/mediatek/mt8186/Makefile            |   21 +
 sound/soc/mediatek/mt8186/mt8186-afe-clk.c    |  719 ++++
 sound/soc/mediatek/mt8186/mt8186-afe-clk.h    |  210 +
 sound/soc/mediatek/mt8186/mt8186-afe-common.h |  245 ++
 .../soc/mediatek/mt8186/mt8186-afe-control.c  |  262 ++
 sound/soc/mediatek/mt8186/mt8186-afe-gpio.c   |  211 +
 sound/soc/mediatek/mt8186/mt8186-afe-gpio.h   |   19 +
 sound/soc/mediatek/mt8186/mt8186-afe-pcm.c    | 3030 +++++++++++++++
 sound/soc/mediatek/mt8186/mt8186-audsys-clk.c |  151 +
 sound/soc/mediatek/mt8186/mt8186-audsys-clk.h |   15 +
 .../soc/mediatek/mt8186/mt8186-audsys-clkid.h |   45 +
 sound/soc/mediatek/mt8186/mt8186-dai-adda.c   |  897 +++++
 .../soc/mediatek/mt8186/mt8186-dai-hostless.c |  296 ++
 .../soc/mediatek/mt8186/mt8186-dai-hw-gain.c  |  246 ++
 sound/soc/mediatek/mt8186/mt8186-dai-i2s.c    | 1352 +++++++
 sound/soc/mediatek/mt8186/mt8186-dai-pcm.c    |  433 +++
 sound/soc/mediatek/mt8186/mt8186-dai-src.c    |  758 ++++
 sound/soc/mediatek/mt8186/mt8186-dai-tdm.c    |  723 ++++
 .../mediatek/mt8186/mt8186-interconnection.h  |   69 +
 .../soc/mediatek/mt8186/mt8186-misc-control.c | 1729 +++++++++
 .../mt8186/mt8186-mt6366-da7219-max98357.c    |  903 +++++
 .../mt8186/mt8186-mt6366-rt1019-rt5682.c      |  887 +++++
 sound/soc/mediatek/mt8186/mt8186-reg.h        | 3433 +++++++++++++++++
 30 files changed, 16978 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/mt8186-afe-pcm.yaml
 create mode 100644 Documentation/devicetree/bindings/sound/mt8186-mt6366-da7219-max98357.yaml
 create mode 100644 Documentation/devicetree/bindings/sound/mt8186-mt6366-rt1019-rt5682.yaml
 create mode 100644 sound/soc/mediatek/mt8186/Makefile
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-afe-clk.c
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-afe-clk.h
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-afe-common.h
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-afe-control.c
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-afe-gpio.c
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-afe-gpio.h
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-afe-pcm.c
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-audsys-clk.c
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-audsys-clk.h
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-audsys-clkid.h
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-dai-adda.c
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-dai-hostless.c
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-dai-hw-gain.c
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-dai-i2s.c
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-dai-pcm.c
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-dai-src.c
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-dai-tdm.c
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-interconnection.h
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-misc-control.c
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-mt6366-da7219-max98357.c
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-mt6366-rt1019-rt5682.c
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-reg.h

Comments

Mark Brown Feb. 11, 2022, 3:10 p.m. UTC | #1 
On Fri, Feb 11, 2022 at 06:38:06PM +0800, Jiaxin Yu wrote:

This looks pretty good, there's some issues below but they're all very
minor, mostly stylistic things rather than anything substantial.

> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + *  MediaTek ALSA SoC Audio DAI ADDA Control
> + *
> + *  Copyright (c) 2022 MediaTek Inc.
> + *  Author: Jiaxin Yu <jiaxin.yu@mediatek.com>
> + */

Please make the entire comment a C++ one so things look more
intentional.

> +static int mtk_adda_ul_event(struct snd_soc_dapm_widget *w,
> +			     struct snd_kcontrol *kcontrol,
> +			     int event)
> +{
> +	struct snd_soc_component *cmpnt = snd_soc_dapm_to_component(w->dapm);
> +	struct mtk_base_afe *afe = snd_soc_component_get_drvdata(cmpnt);
> +	struct mt8186_afe_private *afe_priv = afe->platform_priv;
> +	int mtkaif_dmic = afe_priv->mtkaif_dmic;
> +
> +	dev_info(afe->dev, "%s(), name %s, event 0x%x, mtkaif_dmic %d\n",
> +		 __func__, w->name, event, mtkaif_dmic);

This should be dev_dbg() at most, otherwise the logs will get very noisy
(but note that there are trace points in the core which cover this).
There's a bunch of other dev_info() calls like this on DAPM events.

> +		if (afe_priv->mtkaif_protocol == MTKAIF_PROTOCOL_2_CLK_P2)
> +			regmap_write(afe->regmap, AFE_AUD_PAD_TOP, 0x38);
> +		else if (afe_priv->mtkaif_protocol == MTKAIF_PROTOCOL_2)
> +			regmap_write(afe->regmap, AFE_AUD_PAD_TOP, 0x30);
> +		else
> +			regmap_write(afe->regmap, AFE_AUD_PAD_TOP, 0x30);

This could be more clearly written as a switch statement.

> +			if (strcmp(w->name, "ADDA_MTKAIF_CFG") == 0) {
> +				if (afe_priv->mtkaif_chosen_phase[0] < 0 &&
> +				    afe_priv->mtkaif_chosen_phase[1] < 0) {
> +					dev_info(afe->dev,
> +						 "%s(), calib fail mtkaif_chosen_phase[0/1]:%d/%d\n",
> +						 __func__,

Should this be a dev_err() given that the calibration failed?

> +/* mtkaif dmic */
> +static const char * const mt8186_adda_off_on_str[] = {
> +	"Off", "On"
> +};
> +
> +static const struct soc_enum mt8186_adda_enum[] = {
> +	SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(mt8186_adda_off_on_str),
> +			    mt8186_adda_off_on_str),
> +};

This is a simple on/off control so should be a standard numeric control
with a name ending in Switch to help UIs handle it properly.

> +static int mt8186_adda_dmic_set(struct snd_kcontrol *kcontrol,
> +				struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *cmpnt = snd_soc_kcontrol_component(kcontrol);
> +	struct mtk_base_afe *afe = snd_soc_component_get_drvdata(cmpnt);
> +	struct mt8186_afe_private *afe_priv = afe->platform_priv;
> +	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
> +	int dmic_on;
> +
> +	if (ucontrol->value.enumerated.item[0] >= e->items)
> +		return -EINVAL;
> +
> +	dmic_on = ucontrol->value.integer.value[0];
> +
> +	dev_info(afe->dev, "%s(), kcontrol name %s, dmic_on %d\n",
> +		 __func__, kcontrol->id.name, dmic_on);
> +
> +	afe_priv->mtkaif_dmic = dmic_on;
> +	return 0;

This should return 1 if the value changed so an event is generated for
userspace.  You might want to run the mixer-test kselftest (ideally the
version that's in -next as there were a few bits added very recently),
it should detect issues like this.
Mark Brown Feb. 11, 2022, 4:12 p.m. UTC | #2 
On Fri, Feb 11, 2022 at 06:38:13PM +0800, Jiaxin Yu wrote:

>  sound/soc/mediatek/Kconfig                    |   44 +
>  sound/soc/mediatek/Makefile                   |    1 +
>  sound/soc/mediatek/mt8186/Makefile            |   21 +
>  sound/soc/mediatek/mt8186/mt8186-afe-clk.c    |  719 ++++
>  sound/soc/mediatek/mt8186/mt8186-afe-clk.h    |  210 +
>  sound/soc/mediatek/mt8186/mt8186-afe-common.h |  245 ++
>  .../soc/mediatek/mt8186/mt8186-afe-control.c  |  262 ++
>  sound/soc/mediatek/mt8186/mt8186-afe-gpio.c   |  211 +
>  sound/soc/mediatek/mt8186/mt8186-afe-gpio.h   |   19 +
>  sound/soc/mediatek/mt8186/mt8186-afe-pcm.c    | 3030 +++++++++++++++
>  .../mediatek/mt8186/mt8186-interconnection.h  |   69 +
>  .../soc/mediatek/mt8186/mt8186-misc-control.c | 1729 +++++++++
>  sound/soc/mediatek/mt8186/mt8186-reg.h        | 3433 +++++++++++++++++

I know it's already a long series but perhaps the clock and GPIO bits
could be split out into separate patches?  This one patch is over 300K
which is a bit much in one go, especially when it's not just all big
tables.
Jiaxin Yu (俞家鑫) Feb. 12, 2022, 7:18 a.m. UTC | #3 
On Fri, 2022-02-11 at 16:14 +0000, Mark Brown wrote:
> On Fri, Feb 11, 2022 at 06:38:03PM +0800, Jiaxin Yu wrote:
> > This series of patches adds support for Mediatek AFE of MT8186 Soc.
> > Patches are based on broonie tree "for-next" branch.
> 
> This all looks basically good, there are some issues which I've
> highlighted on the patches but like I kept on saying they're all
> fairly
> small and hopefully easy to address - there's no big structural
> problems
> or anything that I noticed.

Thank you for helping to review my submitted patches.
I will go through your comments carefully and fix them.
Jiaxin Yu (俞家鑫) Feb. 17, 2022, 1:51 p.m. UTC | #4 
On Fri, 2022-02-11 at 16:12 +0000, Mark Brown wrote:
> On Fri, Feb 11, 2022 at 06:38:13PM +0800, Jiaxin Yu wrote:
> 
> >  sound/soc/mediatek/Kconfig                    |   44 +
> >  sound/soc/mediatek/Makefile                   |    1 +
> >  sound/soc/mediatek/mt8186/Makefile            |   21 +
> >  sound/soc/mediatek/mt8186/mt8186-afe-clk.c    |  719 ++++
> >  sound/soc/mediatek/mt8186/mt8186-afe-clk.h    |  210 +
> >  sound/soc/mediatek/mt8186/mt8186-afe-common.h |  245 ++
> >  .../soc/mediatek/mt8186/mt8186-afe-control.c  |  262 ++
> >  sound/soc/mediatek/mt8186/mt8186-afe-gpio.c   |  211 +
> >  sound/soc/mediatek/mt8186/mt8186-afe-gpio.h   |   19 +
> >  sound/soc/mediatek/mt8186/mt8186-afe-pcm.c    | 3030
> > +++++++++++++++
> >  .../mediatek/mt8186/mt8186-interconnection.h  |   69 +
> >  .../soc/mediatek/mt8186/mt8186-misc-control.c | 1729 +++++++++
> >  sound/soc/mediatek/mt8186/mt8186-reg.h        | 3433
> > +++++++++++++++++
> 
> I know it's already a long series but perhaps the clock and GPIO bits
> could be split out into separate patches?  This one patch is over
> 300K
> which is a bit much in one go, especially when it's not just all big
> tables.

Ok, I see. I've split them out into separate patches in v2 version.