[v2] ASoC: mediatek: Add support for MT8186 SoC

[v2 11/17] ASoC: mediatek: mt8186: support gpio control in platform driver

Posted by Jiaxin Yu 3 years, 11 months ago

This patch add gpio control for all audio interface separately.

Signed-off-by: Jiaxin Yu <jiaxin.yu@mediatek.com>
---
 sound/soc/mediatek/mt8186/mt8186-afe-gpio.c | 210 ++++++++++++++++++++
 sound/soc/mediatek/mt8186/mt8186-afe-gpio.h |  19 ++
 2 files changed, 229 insertions(+)
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-afe-gpio.c
 create mode 100644 sound/soc/mediatek/mt8186/mt8186-afe-gpio.h

diff --git a/sound/soc/mediatek/mt8186/mt8186-afe-gpio.c b/sound/soc/mediatek/mt8186/mt8186-afe-gpio.c
new file mode 100644
index 000000000000..6faec5c95bf3
--- /dev/null
+++ b/sound/soc/mediatek/mt8186/mt8186-afe-gpio.c
@@ -0,0 +1,210 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// mt8186-afe-gpio.c  --  Mediatek 8186 afe gpio ctrl
+//
+// Copyright (c) 2022 MediaTek Inc.
+// Author: Jiaxin Yu <jiaxin.yu@mediatek.com>
+
+#include <linux/gpio.h>
+#include <linux/pinctrl/consumer.h>
+
+#include "mt8186-afe-common.h"
+#include "mt8186-afe-gpio.h"
+
+struct pinctrl *aud_pinctrl;
+
+enum mt8186_afe_gpio {
+	MT8186_AFE_GPIO_CLK_MOSI_OFF,
+	MT8186_AFE_GPIO_CLK_MOSI_ON,
+	MT8186_AFE_GPIO_CLK_MISO_OFF,
+	MT8186_AFE_GPIO_CLK_MISO_ON,
+	MT8186_AFE_GPIO_DAT_MISO_OFF,
+	MT8186_AFE_GPIO_DAT_MISO_ON,
+	MT8186_AFE_GPIO_DAT_MOSI_OFF,
+	MT8186_AFE_GPIO_DAT_MOSI_ON,
+	MT8186_AFE_GPIO_I2S0_OFF,
+	MT8186_AFE_GPIO_I2S0_ON,
+	MT8186_AFE_GPIO_I2S1_OFF,
+	MT8186_AFE_GPIO_I2S1_ON,
+	MT8186_AFE_GPIO_I2S2_OFF,
+	MT8186_AFE_GPIO_I2S2_ON,
+	MT8186_AFE_GPIO_I2S3_OFF,
+	MT8186_AFE_GPIO_I2S3_ON,
+	MT8186_AFE_GPIO_TDM_OFF,
+	MT8186_AFE_GPIO_TDM_ON,
+	MT8186_AFE_GPIO_PCM_OFF,
+	MT8186_AFE_GPIO_PCM_ON,
+	MT8186_AFE_GPIO_GPIO_NUM
+};
+
+struct audio_gpio_attr {
+	const char *name;
+	bool gpio_prepare;
+	struct pinctrl_state *gpioctrl;
+};
+
+static struct audio_gpio_attr aud_gpios[MT8186_AFE_GPIO_GPIO_NUM] = {
+	[MT8186_AFE_GPIO_CLK_MOSI_OFF] = {"aud_clk_mosi_off", false, NULL},
+	[MT8186_AFE_GPIO_CLK_MOSI_ON] = {"aud_clk_mosi_on", false, NULL},
+	[MT8186_AFE_GPIO_CLK_MISO_OFF] = {"aud_clk_miso_off", false, NULL},
+	[MT8186_AFE_GPIO_CLK_MISO_ON] = {"aud_clk_miso_on", false, NULL},
+	[MT8186_AFE_GPIO_DAT_MISO_OFF] = {"aud_dat_miso_off", false, NULL},
+	[MT8186_AFE_GPIO_DAT_MISO_ON] = {"aud_dat_miso_on", false, NULL},
+	[MT8186_AFE_GPIO_DAT_MOSI_OFF] = {"aud_dat_mosi_off", false, NULL},
+	[MT8186_AFE_GPIO_DAT_MOSI_ON] = {"aud_dat_mosi_on", false, NULL},
+	[MT8186_AFE_GPIO_I2S0_OFF] = {"aud_gpio_i2s0_off", false, NULL},
+	[MT8186_AFE_GPIO_I2S0_ON] = {"aud_gpio_i2s0_on", false, NULL},
+	[MT8186_AFE_GPIO_I2S1_OFF] = {"aud_gpio_i2s1_off", false, NULL},
+	[MT8186_AFE_GPIO_I2S1_ON] = {"aud_gpio_i2s1_on", false, NULL},
+	[MT8186_AFE_GPIO_I2S2_OFF] = {"aud_gpio_i2s2_off", false, NULL},
+	[MT8186_AFE_GPIO_I2S2_ON] = {"aud_gpio_i2s2_on", false, NULL},
+	[MT8186_AFE_GPIO_I2S3_OFF] = {"aud_gpio_i2s3_off", false, NULL},
+	[MT8186_AFE_GPIO_I2S3_ON] = {"aud_gpio_i2s3_on", false, NULL},
+	[MT8186_AFE_GPIO_TDM_OFF] = {"aud_gpio_tdm_off", false, NULL},
+	[MT8186_AFE_GPIO_TDM_ON] = {"aud_gpio_tdm_on", false, NULL},
+	[MT8186_AFE_GPIO_PCM_OFF] = {"aud_gpio_pcm_off", false, NULL},
+	[MT8186_AFE_GPIO_PCM_ON] = {"aud_gpio_pcm_on", false, NULL},
+};
+
+static DEFINE_MUTEX(gpio_request_mutex);
+
+int mt8186_afe_gpio_init(struct device *dev)
+{
+	int ret;
+	int i = 0;
+
+	aud_pinctrl = devm_pinctrl_get(dev);
+	if (IS_ERR(aud_pinctrl)) {
+		ret = PTR_ERR(aud_pinctrl);
+		dev_info(dev, "%s(), ret %d, cannot get aud_pinctrl!\n",
+			 __func__, ret);
+		return -ENODEV;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(aud_gpios); i++) {
+		aud_gpios[i].gpioctrl = pinctrl_lookup_state(aud_pinctrl,
+							     aud_gpios[i].name);
+		if (IS_ERR(aud_gpios[i].gpioctrl)) {
+			ret = PTR_ERR(aud_gpios[i].gpioctrl);
+			dev_info(dev, "%s(), pinctrl_lookup_state %s fail, ret %d\n",
+				 __func__, aud_gpios[i].name, ret);
+		} else {
+			aud_gpios[i].gpio_prepare = true;
+		}
+	}
+
+	/* gpio status init */
+	mt8186_afe_gpio_request(dev, false, MT8186_DAI_ADDA, 0);
+	mt8186_afe_gpio_request(dev, false, MT8186_DAI_ADDA, 1);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mt8186_afe_gpio_init);
+
+static int mt8186_afe_gpio_select(struct device *dev,
+				  enum mt8186_afe_gpio type)
+{
+	int ret = 0;
+
+	if (type < 0 || type >= MT8186_AFE_GPIO_GPIO_NUM) {
+		dev_info(dev, "%s(), error, invalid gpio type %d\n",
+			 __func__, type);
+		return -EINVAL;
+	}
+
+	if (!aud_gpios[type].gpio_prepare) {
+		dev_info(dev, "%s(), error, gpio type %d not prepared\n",
+			 __func__, type);
+		return -EIO;
+	}
+
+	ret = pinctrl_select_state(aud_pinctrl,
+				   aud_gpios[type].gpioctrl);
+	if (ret)
+		dev_info(dev, "%s(), error, can not set gpio type %d\n",
+			 __func__, type);
+
+	return ret;
+}
+
+static int mt8186_afe_gpio_adda_dl(struct device *dev, bool enable)
+{
+	if (enable) {
+		mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_CLK_MOSI_ON);
+		mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_DAT_MOSI_ON);
+	} else {
+		mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_DAT_MOSI_OFF);
+		mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_CLK_MOSI_OFF);
+	}
+
+	return 0;
+}
+
+static int mt8186_afe_gpio_adda_ul(struct device *dev, bool enable)
+{
+	if (enable) {
+		mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_CLK_MISO_ON);
+		mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_DAT_MISO_ON);
+	} else {
+		mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_DAT_MISO_OFF);
+		mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_CLK_MISO_OFF);
+	}
+
+	return 0;
+}
+
+int mt8186_afe_gpio_request(struct device *dev, bool enable,
+			    int dai, int uplink)
+{
+	mutex_lock(&gpio_request_mutex);
+	switch (dai) {
+	case MT8186_DAI_ADDA:
+		if (uplink)
+			mt8186_afe_gpio_adda_ul(dev, enable);
+		else
+			mt8186_afe_gpio_adda_dl(dev, enable);
+		break;
+	case MT8186_DAI_I2S_0:
+		if (enable)
+			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_I2S0_ON);
+		else
+			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_I2S0_OFF);
+		break;
+	case MT8186_DAI_I2S_1:
+		if (enable)
+			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_I2S1_ON);
+		else
+			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_I2S1_OFF);
+		break;
+	case MT8186_DAI_I2S_2:
+		if (enable)
+			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_I2S2_ON);
+		else
+			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_I2S2_OFF);
+		break;
+	case MT8186_DAI_I2S_3:
+		if (enable)
+			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_I2S3_ON);
+		else
+			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_I2S3_OFF);
+		break;
+	case MT8186_DAI_TDM_IN:
+		if (enable)
+			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_TDM_ON);
+		else
+			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_TDM_OFF);
+		break;
+	case MT8186_DAI_PCM:
+		if (enable)
+			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_PCM_ON);
+		else
+			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_PCM_OFF);
+		break;
+	default:
+		mutex_unlock(&gpio_request_mutex);
+		dev_info(dev, "%s(), invalid dai %d\n", __func__, dai);
+		return -EINVAL;
+	}
+	mutex_unlock(&gpio_request_mutex);
+	return 0;
+}
diff --git a/sound/soc/mediatek/mt8186/mt8186-afe-gpio.h b/sound/soc/mediatek/mt8186/mt8186-afe-gpio.h
new file mode 100644
index 000000000000..1ddc27838eb1
--- /dev/null
+++ b/sound/soc/mediatek/mt8186/mt8186-afe-gpio.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * mt6833-afe-gpio.h  --  Mediatek 6833 afe gpio ctrl definition
+ *
+ * Copyright (c) 2022 MediaTek Inc.
+ * Author: Jiaxin Yu <jiaxin.yu@mediatek.com>
+ */
+
+#ifndef _MT8186_AFE_GPIO_H_
+#define _MT8186_AFE_GPIO_H_
+
+struct mtk_base_afe;
+
+int mt8186_afe_gpio_init(struct device *dev);
+
+int mt8186_afe_gpio_request(struct device *dev, bool enable,
+			    int dai, int uplink);
+
+#endif
-- 
2.18.0

Re: [v2 11/17] ASoC: mediatek: mt8186: support gpio control in platform driver

Posted by AngeloGioacchino Del Regno 3 years, 11 months ago

Il 17/02/22 14:41, Jiaxin Yu ha scritto:
> This patch add gpio control for all audio interface separately.
> 
> Signed-off-by: Jiaxin Yu <jiaxin.yu@mediatek.com>
> ---
>   sound/soc/mediatek/mt8186/mt8186-afe-gpio.c | 210 ++++++++++++++++++++
>   sound/soc/mediatek/mt8186/mt8186-afe-gpio.h |  19 ++
>   2 files changed, 229 insertions(+)
>   create mode 100644 sound/soc/mediatek/mt8186/mt8186-afe-gpio.c
>   create mode 100644 sound/soc/mediatek/mt8186/mt8186-afe-gpio.h
> 
> diff --git a/sound/soc/mediatek/mt8186/mt8186-afe-gpio.c b/sound/soc/mediatek/mt8186/mt8186-afe-gpio.c
> new file mode 100644
> index 000000000000..6faec5c95bf3
> --- /dev/null
> +++ b/sound/soc/mediatek/mt8186/mt8186-afe-gpio.c
> @@ -0,0 +1,210 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// mt8186-afe-gpio.c  --  Mediatek 8186 afe gpio ctrl
> +//
> +// Copyright (c) 2022 MediaTek Inc.
> +// Author: Jiaxin Yu <jiaxin.yu@mediatek.com>
> +
> +#include <linux/gpio.h>
> +#include <linux/pinctrl/consumer.h>
> +
> +#include "mt8186-afe-common.h"
> +#include "mt8186-afe-gpio.h"
> +
> +struct pinctrl *aud_pinctrl;
> +
> +enum mt8186_afe_gpio {
> +	MT8186_AFE_GPIO_CLK_MOSI_OFF,
> +	MT8186_AFE_GPIO_CLK_MOSI_ON,
> +	MT8186_AFE_GPIO_CLK_MISO_OFF,
> +	MT8186_AFE_GPIO_CLK_MISO_ON,
> +	MT8186_AFE_GPIO_DAT_MISO_OFF,
> +	MT8186_AFE_GPIO_DAT_MISO_ON,
> +	MT8186_AFE_GPIO_DAT_MOSI_OFF,
> +	MT8186_AFE_GPIO_DAT_MOSI_ON,
> +	MT8186_AFE_GPIO_I2S0_OFF,
> +	MT8186_AFE_GPIO_I2S0_ON,
> +	MT8186_AFE_GPIO_I2S1_OFF,
> +	MT8186_AFE_GPIO_I2S1_ON,
> +	MT8186_AFE_GPIO_I2S2_OFF,
> +	MT8186_AFE_GPIO_I2S2_ON,
> +	MT8186_AFE_GPIO_I2S3_OFF,
> +	MT8186_AFE_GPIO_I2S3_ON,
> +	MT8186_AFE_GPIO_TDM_OFF,
> +	MT8186_AFE_GPIO_TDM_ON,
> +	MT8186_AFE_GPIO_PCM_OFF,
> +	MT8186_AFE_GPIO_PCM_ON,
> +	MT8186_AFE_GPIO_GPIO_NUM
> +};
> +
> +struct audio_gpio_attr {
> +	const char *name;
> +	bool gpio_prepare;
> +	struct pinctrl_state *gpioctrl;
> +};
> +
> +static struct audio_gpio_attr aud_gpios[MT8186_AFE_GPIO_GPIO_NUM] = {
> +	[MT8186_AFE_GPIO_CLK_MOSI_OFF] = {"aud_clk_mosi_off", false, NULL},
> +	[MT8186_AFE_GPIO_CLK_MOSI_ON] = {"aud_clk_mosi_on", false, NULL},
> +	[MT8186_AFE_GPIO_CLK_MISO_OFF] = {"aud_clk_miso_off", false, NULL},
> +	[MT8186_AFE_GPIO_CLK_MISO_ON] = {"aud_clk_miso_on", false, NULL},
> +	[MT8186_AFE_GPIO_DAT_MISO_OFF] = {"aud_dat_miso_off", false, NULL},
> +	[MT8186_AFE_GPIO_DAT_MISO_ON] = {"aud_dat_miso_on", false, NULL},
> +	[MT8186_AFE_GPIO_DAT_MOSI_OFF] = {"aud_dat_mosi_off", false, NULL},
> +	[MT8186_AFE_GPIO_DAT_MOSI_ON] = {"aud_dat_mosi_on", false, NULL},
> +	[MT8186_AFE_GPIO_I2S0_OFF] = {"aud_gpio_i2s0_off", false, NULL},
> +	[MT8186_AFE_GPIO_I2S0_ON] = {"aud_gpio_i2s0_on", false, NULL},
> +	[MT8186_AFE_GPIO_I2S1_OFF] = {"aud_gpio_i2s1_off", false, NULL},
> +	[MT8186_AFE_GPIO_I2S1_ON] = {"aud_gpio_i2s1_on", false, NULL},
> +	[MT8186_AFE_GPIO_I2S2_OFF] = {"aud_gpio_i2s2_off", false, NULL},
> +	[MT8186_AFE_GPIO_I2S2_ON] = {"aud_gpio_i2s2_on", false, NULL},
> +	[MT8186_AFE_GPIO_I2S3_OFF] = {"aud_gpio_i2s3_off", false, NULL},
> +	[MT8186_AFE_GPIO_I2S3_ON] = {"aud_gpio_i2s3_on", false, NULL},
> +	[MT8186_AFE_GPIO_TDM_OFF] = {"aud_gpio_tdm_off", false, NULL},
> +	[MT8186_AFE_GPIO_TDM_ON] = {"aud_gpio_tdm_on", false, NULL},
> +	[MT8186_AFE_GPIO_PCM_OFF] = {"aud_gpio_pcm_off", false, NULL},
> +	[MT8186_AFE_GPIO_PCM_ON] = {"aud_gpio_pcm_on", false, NULL},
> +};
> +
> +static DEFINE_MUTEX(gpio_request_mutex);
> +
> +int mt8186_afe_gpio_init(struct device *dev)
> +{
> +	int ret;
> +	int i = 0;
> +
> +	aud_pinctrl = devm_pinctrl_get(dev);
> +	if (IS_ERR(aud_pinctrl)) {
> +		ret = PTR_ERR(aud_pinctrl);
> +		dev_info(dev, "%s(), ret %d, cannot get aud_pinctrl!\n",
> +			 __func__, ret);

dev_err()
... and return ret.

> +		return -ENODEV;
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(aud_gpios); i++) {
> +		aud_gpios[i].gpioctrl = pinctrl_lookup_state(aud_pinctrl,
> +							     aud_gpios[i].name);
> +		if (IS_ERR(aud_gpios[i].gpioctrl)) {
> +			ret = PTR_ERR(aud_gpios[i].gpioctrl);
> +			dev_info(dev, "%s(), pinctrl_lookup_state %s fail, ret %d\n",
> +				 __func__, aud_gpios[i].name, ret);

dev_err()

P.S.: I think that this function should return ret, at this point, to avoid
       unexpected behavior.


> +		} else {
> +			aud_gpios[i].gpio_prepare = true;
> +		}
> +	}
> +
> +	/* gpio status init */
> +	mt8186_afe_gpio_request(dev, false, MT8186_DAI_ADDA, 0);
> +	mt8186_afe_gpio_request(dev, false, MT8186_DAI_ADDA, 1);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(mt8186_afe_gpio_init);
> +
> +static int mt8186_afe_gpio_select(struct device *dev,
> +				  enum mt8186_afe_gpio type)
> +{
> +	int ret = 0;
> +
> +	if (type < 0 || type >= MT8186_AFE_GPIO_GPIO_NUM) {
> +		dev_info(dev, "%s(), error, invalid gpio type %d\n",
> +			 __func__, type);
> +		return -EINVAL;
> +	}
> +
> +	if (!aud_gpios[type].gpio_prepare) {
> +		dev_info(dev, "%s(), error, gpio type %d not prepared\n",
> +			 __func__, type);
> +		return -EIO;
> +	}
> +
> +	ret = pinctrl_select_state(aud_pinctrl,
> +				   aud_gpios[type].gpioctrl);
> +	if (ret)
> +		dev_info(dev, "%s(), error, can not set gpio type %d\n",
> +			 __func__, type);
> +
> +	return ret;

Please change it like so:

	if (ret) {
		dev_err(dev, "Failed to select picntrl state for type %d\n", type);
		return ret;
	}

	return 0;

> +}
> +
> +static int mt8186_afe_gpio_adda_dl(struct device *dev, bool enable)
> +{
> +	if (enable) {
> +		mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_CLK_MOSI_ON);
> +		mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_DAT_MOSI_ON);
> +	} else {
> +		mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_DAT_MOSI_OFF);
> +		mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_CLK_MOSI_OFF);
> +	}
> +
> +	return 0;
> +}
> +
> +static int mt8186_afe_gpio_adda_ul(struct device *dev, bool enable)
> +{
> +	if (enable) {
> +		mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_CLK_MISO_ON);
> +		mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_DAT_MISO_ON);
> +	} else {
> +		mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_DAT_MISO_OFF);
> +		mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_CLK_MISO_OFF);
> +	}
> +
> +	return 0;
> +}
> +
> +int mt8186_afe_gpio_request(struct device *dev, bool enable,
> +			    int dai, int uplink)
> +{

I think that it's more readable and even shorter if you do:

	enum mt8186_afe_gpio sel;

	int ret = -EINVAL;



	mutex_lock(&gpio_request_mutex);



	switch (dai) {

	case MT8186_DAI_ADDA:
		if (uplink)
			ret = mt8186_afe_gpio_adda_ul(dev, enable);
		else
			ret = mt8186_afe_gpio_adda_dl(dev, enable);
		goto unlock;
	case MT8186_DAI_I2S_0:

		sel = enable ? MT8186_AFE_GPIO_I2S0_ON : MT8186_AFE_GPIO_I2S0_OFF;

		break;

	case MT8186_DAI_I2S_1:

		sel = enable ? MT8186_AFE_GPIO_I2S1_ON : MT8186_AFE_GPIO_I2S1_OFF;

		break;



	.................... all other cases ................

	default:

		dev_err(dev, "invalid dai %d\n", dai);

		goto unlock;

	}


	ret = mt8186_afe_gpio_select(dev, sel);


unlock:

	mutex_unlock(&gpio_request_mutex);

	return ret;
}

> +	mutex_lock(&gpio_request_mutex);
> +	switch (dai) {
> +	case MT8186_DAI_ADDA:
> +		if (uplink)
> +			mt8186_afe_gpio_adda_ul(dev, enable);
> +		else
> +			mt8186_afe_gpio_adda_dl(dev, enable);
> +		break;
> +	case MT8186_DAI_I2S_0:
> +		if (enable)
> +			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_I2S0_ON);
> +		else
> +			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_I2S0_OFF);
> +		break;
> +	case MT8186_DAI_I2S_1:
> +		if (enable)
> +			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_I2S1_ON);
> +		else
> +			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_I2S1_OFF);
> +		break;
> +	case MT8186_DAI_I2S_2:
> +		if (enable)
> +			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_I2S2_ON);
> +		else
> +			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_I2S2_OFF);
> +		break;
> +	case MT8186_DAI_I2S_3:
> +		if (enable)
> +			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_I2S3_ON);
> +		else
> +			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_I2S3_OFF);
> +		break;
> +	case MT8186_DAI_TDM_IN:
> +		if (enable)
> +			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_TDM_ON);
> +		else
> +			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_TDM_OFF);
> +		break;
> +	case MT8186_DAI_PCM:
> +		if (enable)
> +			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_PCM_ON);
> +		else
> +			mt8186_afe_gpio_select(dev, MT8186_AFE_GPIO_PCM_OFF);
> +		break;
> +	default:
> +		mutex_unlock(&gpio_request_mutex);
> +		dev_info(dev, "%s(), invalid dai %d\n", __func__, dai);
> +		return -EINVAL;
> +	}
> +	mutex_unlock(&gpio_request_mutex);
> +	return 0;
> +}
> diff --git a/sound/soc/mediatek/mt8186/mt8186-afe-gpio.h b/sound/soc/mediatek/mt8186/mt8186-afe-gpio.h
> new file mode 100644
> index 000000000000..1ddc27838eb1
> --- /dev/null
> +++ b/sound/soc/mediatek/mt8186/mt8186-afe-gpio.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * mt6833-afe-gpio.h  --  Mediatek 6833 afe gpio ctrl definition
> + *
> + * Copyright (c) 2022 MediaTek Inc.
> + * Author: Jiaxin Yu <jiaxin.yu@mediatek.com>
> + */
> +
> +#ifndef _MT8186_AFE_GPIO_H_
> +#define _MT8186_AFE_GPIO_H_
> +
> +struct mtk_base_afe;
> +
> +int mt8186_afe_gpio_init(struct device *dev);
> +
> +int mt8186_afe_gpio_request(struct device *dev, bool enable,
> +			    int dai, int uplink);
> +
> +#endif

Re: [v2 11/17] ASoC: mediatek: mt8186: support gpio control in platform driver

Posted by Jiaxin Yu 3 years, 11 months ago

On Fri, 2022-02-18 at 15:54 +0100, AngeloGioacchino Del Regno wrote:
> Il 17/02/22 14:41, Jiaxin Yu ha scritto:
> > This patch add gpio control for all audio interface separately.
> > 
> > Signed-off-by: Jiaxin Yu <jiaxin.yu@mediatek.com>
> > ---
> >   sound/soc/mediatek/mt8186/mt8186-afe-gpio.c | 210
> > ++++++++++++++++++++
> >   sound/soc/mediatek/mt8186/mt8186-afe-gpio.h |  19 ++
> >   2 files changed, 229 insertions(+)
> >   create mode 100644 sound/soc/mediatek/mt8186/mt8186-afe-gpio.c
> >   create mode 100644 sound/soc/mediatek/mt8186/mt8186-afe-gpio.h
> > 
> > diff --git a/sound/soc/mediatek/mt8186/mt8186-afe-gpio.c
> > b/sound/soc/mediatek/mt8186/mt8186-afe-gpio.c
> > new file mode 100644
> > index 000000000000..6faec5c95bf3
> > --- /dev/null
> > +++ b/sound/soc/mediatek/mt8186/mt8186-afe-gpio.c
> > @@ -0,0 +1,210 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +//
> > +// mt8186-afe-gpio.c  --  Mediatek 8186 afe gpio ctrl
> > +//
> > +// Copyright (c) 2022 MediaTek Inc.
> > +// Author: Jiaxin Yu <jiaxin.yu@mediatek.com>
> > +
> > +#include <linux/gpio.h>
> > +#include <linux/pinctrl/consumer.h>
> > +
> > +#include "mt8186-afe-common.h"
> > +#include "mt8186-afe-gpio.h"
> > +
> > +struct pinctrl *aud_pinctrl;
> > +
> > +enum mt8186_afe_gpio {
> > +	MT8186_AFE_GPIO_CLK_MOSI_OFF,
> > +	MT8186_AFE_GPIO_CLK_MOSI_ON,
> > +	MT8186_AFE_GPIO_CLK_MISO_OFF,
> > +	MT8186_AFE_GPIO_CLK_MISO_ON,
> > +	MT8186_AFE_GPIO_DAT_MISO_OFF,
> > +	MT8186_AFE_GPIO_DAT_MISO_ON,
> > +	MT8186_AFE_GPIO_DAT_MOSI_OFF,
> > +	MT8186_AFE_GPIO_DAT_MOSI_ON,
> > +	MT8186_AFE_GPIO_I2S0_OFF,
> > +	MT8186_AFE_GPIO_I2S0_ON,
> > +	MT8186_AFE_GPIO_I2S1_OFF,
> > +	MT8186_AFE_GPIO_I2S1_ON,
> > +	MT8186_AFE_GPIO_I2S2_OFF,
> > +	MT8186_AFE_GPIO_I2S2_ON,
> > +	MT8186_AFE_GPIO_I2S3_OFF,
> > +	MT8186_AFE_GPIO_I2S3_ON,
> > +	MT8186_AFE_GPIO_TDM_OFF,
> > +	MT8186_AFE_GPIO_TDM_ON,
> > +	MT8186_AFE_GPIO_PCM_OFF,
> > +	MT8186_AFE_GPIO_PCM_ON,
> > +	MT8186_AFE_GPIO_GPIO_NUM
> > +};
> > +
> > +struct audio_gpio_attr {
> > +	const char *name;
> > +	bool gpio_prepare;
> > +	struct pinctrl_state *gpioctrl;
> > +};
> > +
> > +static struct audio_gpio_attr aud_gpios[MT8186_AFE_GPIO_GPIO_NUM]
> > = {
> > +	[MT8186_AFE_GPIO_CLK_MOSI_OFF] = {"aud_clk_mosi_off", false,
> > NULL},
> > +	[MT8186_AFE_GPIO_CLK_MOSI_ON] = {"aud_clk_mosi_on", false,
> > NULL},
> > +	[MT8186_AFE_GPIO_CLK_MISO_OFF] = {"aud_clk_miso_off", false,
> > NULL},
> > +	[MT8186_AFE_GPIO_CLK_MISO_ON] = {"aud_clk_miso_on", false,
> > NULL},
> > +	[MT8186_AFE_GPIO_DAT_MISO_OFF] = {"aud_dat_miso_off", false,
> > NULL},
> > +	[MT8186_AFE_GPIO_DAT_MISO_ON] = {"aud_dat_miso_on", false,
> > NULL},
> > +	[MT8186_AFE_GPIO_DAT_MOSI_OFF] = {"aud_dat_mosi_off", false,
> > NULL},
> > +	[MT8186_AFE_GPIO_DAT_MOSI_ON] = {"aud_dat_mosi_on", false,
> > NULL},
> > +	[MT8186_AFE_GPIO_I2S0_OFF] = {"aud_gpio_i2s0_off", false,
> > NULL},
> > +	[MT8186_AFE_GPIO_I2S0_ON] = {"aud_gpio_i2s0_on", false, NULL},
> > +	[MT8186_AFE_GPIO_I2S1_OFF] = {"aud_gpio_i2s1_off", false,
> > NULL},
> > +	[MT8186_AFE_GPIO_I2S1_ON] = {"aud_gpio_i2s1_on", false, NULL},
> > +	[MT8186_AFE_GPIO_I2S2_OFF] = {"aud_gpio_i2s2_off", false,
> > NULL},
> > +	[MT8186_AFE_GPIO_I2S2_ON] = {"aud_gpio_i2s2_on", false, NULL},
> > +	[MT8186_AFE_GPIO_I2S3_OFF] = {"aud_gpio_i2s3_off", false,
> > NULL},
> > +	[MT8186_AFE_GPIO_I2S3_ON] = {"aud_gpio_i2s3_on", false, NULL},
> > +	[MT8186_AFE_GPIO_TDM_OFF] = {"aud_gpio_tdm_off", false, NULL},
> > +	[MT8186_AFE_GPIO_TDM_ON] = {"aud_gpio_tdm_on", false, NULL},
> > +	[MT8186_AFE_GPIO_PCM_OFF] = {"aud_gpio_pcm_off", false, NULL},
> > +	[MT8186_AFE_GPIO_PCM_ON] = {"aud_gpio_pcm_on", false, NULL},
> > +};
> > +
> > +static DEFINE_MUTEX(gpio_request_mutex);
> > +
> > +int mt8186_afe_gpio_init(struct device *dev)
> > +{
> > +	int ret;
> > +	int i = 0;
> > +
> > +	aud_pinctrl = devm_pinctrl_get(dev);
> > +	if (IS_ERR(aud_pinctrl)) {
> > +		ret = PTR_ERR(aud_pinctrl);
> > +		dev_info(dev, "%s(), ret %d, cannot get
> > aud_pinctrl!\n",
> > +			 __func__, ret);
> 
> dev_err()
> ... and return ret.
> 
> > +		return -ENODEV;
> > +	}
> > +
> > +	for (i = 0; i < ARRAY_SIZE(aud_gpios); i++) {
> > +		aud_gpios[i].gpioctrl =
> > pinctrl_lookup_state(aud_pinctrl,
> > +							     aud_gpios[
> > i].name);
> > +		if (IS_ERR(aud_gpios[i].gpioctrl)) {
> > +			ret = PTR_ERR(aud_gpios[i].gpioctrl);
> > +			dev_info(dev, "%s(), pinctrl_lookup_state %s
> > fail, ret %d\n",
> > +				 __func__, aud_gpios[i].name, ret);
> 
> dev_err()
> 
> P.S.: I think that this function should return ret, at this point, to
> avoid
>        unexpected behavior.

Because we maybe don't need to config all of audio interface gpio in
dtsi. for example, we may only use I2S0/I2S1 instead of I2S3/I2S4, so
there only config I2S0/I2S1's pinctrl in dtsi. So I think there only
need dev_info() as a reminder. How do you suggest for it?

> 
> 
> > +		} else {
> > +			aud_gpios[i].gpio_prepare = true;
> > +		}
> > +	}
> > +
> > +	/* gpio status init */
> > +	mt8186_afe_gpio_request(dev, false, MT8186_DAI_ADDA, 0);
> > +	mt8186_afe_gpio_request(dev, false, MT8186_DAI_ADDA, 1);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(mt8186_afe_gpio_init);
> > +
> > +static int mt8186_afe_gpio_select(struct device *dev,
> > +				  enum mt8186_afe_gpio type)
> > +{
> > +	int ret = 0;
> > +
> > +	if (type < 0 || type >= MT8186_AFE_GPIO_GPIO_NUM) {
> > +		dev_info(dev, "%s(), error, invalid gpio type %d\n",
> > +			 __func__, type);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (!aud_gpios[type].gpio_prepare) {
> > +		dev_info(dev, "%s(), error, gpio type %d not
> > prepared\n",
> > +			 __func__, type);
> > +		return -EIO;
> > +	}
> > +
> > +	ret = pinctrl_select_state(aud_pinctrl,
> > +				   aud_gpios[type].gpioctrl);
> > +	if (ret)
> > +		dev_info(dev, "%s(), error, can not set gpio type
> > %d\n",
> > +			 __func__, type);
> > +
> > +	return ret;
> 
> Please change it like so:
> 
> 	if (ret) {
> 		dev_err(dev, "Failed to select picntrl state for type
> %d\n", type);
> 		return ret;
> 	}
> 
> 	return 0;
> 
> > +}
> > +
> > +static int mt8186_afe_gpio_adda_dl(struct device *dev, bool
> > enable)
> > +{
> > +	if (enable) {
> > +		mt8186_afe_gpio_select(dev,
> > MT8186_AFE_GPIO_CLK_MOSI_ON);
> > +		mt8186_afe_gpio_select(dev,
> > MT8186_AFE_GPIO_DAT_MOSI_ON);
> > +	} else {
> > +		mt8186_afe_gpio_select(dev,
> > MT8186_AFE_GPIO_DAT_MOSI_OFF);
> > +		mt8186_afe_gpio_select(dev,
> > MT8186_AFE_GPIO_CLK_MOSI_OFF);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int mt8186_afe_gpio_adda_ul(struct device *dev, bool
> > enable)
> > +{
> > +	if (enable) {
> > +		mt8186_afe_gpio_select(dev,
> > MT8186_AFE_GPIO_CLK_MISO_ON);
> > +		mt8186_afe_gpio_select(dev,
> > MT8186_AFE_GPIO_DAT_MISO_ON);
> > +	} else {
> > +		mt8186_afe_gpio_select(dev,
> > MT8186_AFE_GPIO_DAT_MISO_OFF);
> > +		mt8186_afe_gpio_select(dev,
> > MT8186_AFE_GPIO_CLK_MISO_OFF);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +int mt8186_afe_gpio_request(struct device *dev, bool enable,
> > +			    int dai, int uplink)
> > +{
> 
> I think that it's more readable and even shorter if you do:
> 
> 	enum mt8186_afe_gpio sel;
> 
> 	int ret = -EINVAL;
> 
> 
> 
> 	mutex_lock(&gpio_request_mutex);
> 
> 
> 
> 	switch (dai) {
> 
> 	case MT8186_DAI_ADDA:
> 		if (uplink)
> 			ret = mt8186_afe_gpio_adda_ul(dev, enable);
> 		else
> 			ret = mt8186_afe_gpio_adda_dl(dev, enable);
> 		goto unlock;
> 	case MT8186_DAI_I2S_0:
> 
> 		sel = enable ? MT8186_AFE_GPIO_I2S0_ON :
> MT8186_AFE_GPIO_I2S0_OFF;
> 
> 		break;
> 
> 	case MT8186_DAI_I2S_1:
> 
> 		sel = enable ? MT8186_AFE_GPIO_I2S1_ON :
> MT8186_AFE_GPIO_I2S1_OFF;
> 
> 		break;
> 
> 
> 
> 	.................... all other cases ................
> 
> 	default:
> 
> 		dev_err(dev, "invalid dai %d\n", dai);
> 
> 		goto unlock;
> 
> 	}
> 
> 
> 	ret = mt8186_afe_gpio_select(dev, sel);
> 
> 
> unlock:
> 
> 	mutex_unlock(&gpio_request_mutex);
> 
> 	return ret;
> }
> 
> > +	mutex_lock(&gpio_request_mutex);
> > +	switch (dai) {
> > +	case MT8186_DAI_ADDA:
> > +		if (uplink)
> > +			mt8186_afe_gpio_adda_ul(dev, enable);
> > +		else
> > +			mt8186_afe_gpio_adda_dl(dev, enable);
> > +		break;
> > +	case MT8186_DAI_I2S_0:
> > +		if (enable)
> > +			mt8186_afe_gpio_select(dev,
> > MT8186_AFE_GPIO_I2S0_ON);
> > +		else
> > +			mt8186_afe_gpio_select(dev,
> > MT8186_AFE_GPIO_I2S0_OFF);
> > +		break;
> > +	case MT8186_DAI_I2S_1:
> > +		if (enable)
> > +			mt8186_afe_gpio_select(dev,
> > MT8186_AFE_GPIO_I2S1_ON);
> > +		else
> > +			mt8186_afe_gpio_select(dev,
> > MT8186_AFE_GPIO_I2S1_OFF);
> > +		break;
> > +	case MT8186_DAI_I2S_2:
> > +		if (enable)
> > +			mt8186_afe_gpio_select(dev,
> > MT8186_AFE_GPIO_I2S2_ON);
> > +		else
> > +			mt8186_afe_gpio_select(dev,
> > MT8186_AFE_GPIO_I2S2_OFF);
> > +		break;
> > +	case MT8186_DAI_I2S_3:
> > +		if (enable)
> > +			mt8186_afe_gpio_select(dev,
> > MT8186_AFE_GPIO_I2S3_ON);
> > +		else
> > +			mt8186_afe_gpio_select(dev,
> > MT8186_AFE_GPIO_I2S3_OFF);
> > +		break;
> > +	case MT8186_DAI_TDM_IN:
> > +		if (enable)
> > +			mt8186_afe_gpio_select(dev,
> > MT8186_AFE_GPIO_TDM_ON);
> > +		else
> > +			mt8186_afe_gpio_select(dev,
> > MT8186_AFE_GPIO_TDM_OFF);
> > +		break;
> > +	case MT8186_DAI_PCM:
> > +		if (enable)
> > +			mt8186_afe_gpio_select(dev,
> > MT8186_AFE_GPIO_PCM_ON);
> > +		else
> > +			mt8186_afe_gpio_select(dev,
> > MT8186_AFE_GPIO_PCM_OFF);
> > +		break;
> > +	default:
> > +		mutex_unlock(&gpio_request_mutex);
> > +		dev_info(dev, "%s(), invalid dai %d\n", __func__, dai);
> > +		return -EINVAL;
> > +	}
> > +	mutex_unlock(&gpio_request_mutex);
> > +	return 0;
> > +}
> > diff --git a/sound/soc/mediatek/mt8186/mt8186-afe-gpio.h
> > b/sound/soc/mediatek/mt8186/mt8186-afe-gpio.h
> > new file mode 100644
> > index 000000000000..1ddc27838eb1
> > --- /dev/null
> > +++ b/sound/soc/mediatek/mt8186/mt8186-afe-gpio.h
> > @@ -0,0 +1,19 @@
> > +/* SPDX-License-Identifier: GPL-2.0
> > + *
> > + * mt6833-afe-gpio.h  --  Mediatek 6833 afe gpio ctrl definition
> > + *
> > + * Copyright (c) 2022 MediaTek Inc.
> > + * Author: Jiaxin Yu <jiaxin.yu@mediatek.com>
> > + */
> > +
> > +#ifndef _MT8186_AFE_GPIO_H_
> > +#define _MT8186_AFE_GPIO_H_
> > +
> > +struct mtk_base_afe;
> > +
> > +int mt8186_afe_gpio_init(struct device *dev);
> > +
> > +int mt8186_afe_gpio_request(struct device *dev, bool enable,
> > +			    int dai, int uplink);
> > +
> > +#endif
> 
>