mbox series

[v3,0/2] Add mediatek codec mt6359 driver

Message ID 1597401954-28388-1-git-send-email-jiaxin.yu@mediatek.com (mailing list archive)
Headers show
Series Add mediatek codec mt6359 driver | expand

Message

Jiaxin Yu Aug. 14, 2020, 10:45 a.m. UTC 
Add mediatek codec (MT6359) driver

MT6359 support playback and capture feature.

On downlink path, it includes three DACs for handset, headset,
and lineout path. On unlink path, it includeds three ADCs for
main mic, second mic, 3rd mic, and headset mic.

By scenario, select *_MUX widget to create damp path.
And by select mic_type_mux to choose DMIC/AMIC/....

For example, select these MUX widget to create headset path
(1) DAC In Mux --> "Normal Path"
(2) HPL Mux --> "Audio Playback"
(3) HPR Mux --> "Audio Playback"

v3 changes:
	1. Remove calibration related functions.
	2. Conbined 'HPL Mux' and 'HPR Mux' to 'HP Mux'.
	3. Move the regulator_enable of 'vaud18' to the component probe.
	4. Fix some incorrect coding style.

v2 changes:
	1. patchwork link:
		https://patchwork.kernel.org/cover/11706935/
		https://patchwork.kernel.org/patch/11708865/
		https://patchwork.kernel.org/patch/11706937/

v1 changes:
	1.lkml link:
		https://lkml.org/lkml/2020/3/5/1257

Jiaxin Yu (2):
  WIP: ASoC: mediatek: mt6359: add codec driver
  WIP: dt-bindings: mediatek: mt6359: add codec document

 .../devicetree/bindings/sound/mt6359.yaml     |   68 +
 sound/soc/codecs/Kconfig                      |    8 +
 sound/soc/codecs/Makefile                     |    2 +
 sound/soc/codecs/mt6359.c                     | 2737 +++++++++++++++++
 sound/soc/codecs/mt6359.h                     | 2640 ++++++++++++++++
 5 files changed, 5455 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/mt6359.yaml
 create mode 100644 sound/soc/codecs/mt6359.c
 create mode 100644 sound/soc/codecs/mt6359.h

Comments

Tzung-Bi Shih Aug. 14, 2020, 1:18 p.m. UTC | #1 
On Fri, Aug 14, 2020 at 6:47 PM Jiaxin Yu <jiaxin.yu@mediatek.com> wrote:
> Jiaxin Yu (2):
>   WIP: ASoC: mediatek: mt6359: add codec driver
>   WIP: dt-bindings: mediatek: mt6359: add codec document

Please remove the "WIP: " prefixes and resend again.
Mark Brown Aug. 14, 2020, 4:01 p.m. UTC | #2 
On Fri, Aug 14, 2020 at 06:45:53PM +0800, Jiaxin Yu wrote:

This looks mostly good, a couple of very small things:

> +	ret = regulator_enable(priv->avdd_reg);
> +	if (ret) {
> +		dev_err(&pdev->dev, "%s(), failed to enable regulator!\n",
> +			__func__);
> +		return ret;
> +	}

You need a remove() function to undo this enable.

> +	dev_info(&pdev->dev, "%s(), dev name %s\n",
> +		 __func__, dev_name(&pdev->dev));

This isn't really adding anything, just remove it - it's not reading
info from the hardware or anything.
Jiaxin Yu Aug. 15, 2020, 5:27 p.m. UTC | #3 
On Fri, 2020-08-14 at 17:01 +0100, Mark Brown wrote:
> On Fri, Aug 14, 2020 at 06:45:53PM +0800, Jiaxin Yu wrote:
> 
> This looks mostly good, a couple of very small things:
> 
> > +	ret = regulator_enable(priv->avdd_reg);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "%s(), failed to enable regulator!\n",
> > +			__func__);
> > +		return ret;
> > +	}
> 
> You need a remove() function to undo this enable.
> 

Ok, I will add a remove() function to do regulator_disable() and
snd_soc_unregister_component().

> > +	dev_info(&pdev->dev, "%s(), dev name %s\n",
> > +		 __func__, dev_name(&pdev->dev));
> 
> This isn't really adding anything, just remove it - it's not reading
> info from the hardware or anything.

Yes, it was unnecessary, removed it in PATCH v4.