From: zhengxing <[email protected]>
Hi,
The simple-card is not common at present, soc maybe need own machine
driver for jack detection.
Add drivers for two families of rockchip-bases chromebooks. These
machine drives don't use simplecard because we need custom jack
detection plumbing.
- use ts3a227e for ext jack detection with max98090
- call rt5645_set_jack_detect function via rt5645 codec driver
Thanks.
zhengxing (2):
ASoC: rockchip: Add machine driver for max98090 codec
ASoC: rockchip: Add machine driver for rt5645/rt5650 codec
sound/soc/rockchip/Kconfig | 19 +++
sound/soc/rockchip/Makefile | 6 +
sound/soc/rockchip/rockchip_max98090.c | 246 ++++++++++++++++++++++++++++++++
sound/soc/rockchip/rockchip_rt5645.c | 235 ++++++++++++++++++++++++++++++
4 files changed, 506 insertions(+)
create mode 100644 sound/soc/rockchip/rockchip_max98090.c
create mode 100644 sound/soc/rockchip/rockchip_rt5645.c
--
1.7.9.5
From: zhengxing <[email protected]>
The driver is used for rockchip board using a max98090.
Reviewed-by: Dylan Reid <[email protected]>
Signed-off-by: zhengxing <[email protected]>
---
sound/soc/rockchip/Kconfig | 10 ++
sound/soc/rockchip/Makefile | 4 +
sound/soc/rockchip/rockchip_max98090.c | 246 ++++++++++++++++++++++++++++++++
3 files changed, 260 insertions(+)
create mode 100644 sound/soc/rockchip/rockchip_max98090.c
diff --git a/sound/soc/rockchip/Kconfig b/sound/soc/rockchip/Kconfig
index e181826..d123566 100644
--- a/sound/soc/rockchip/Kconfig
+++ b/sound/soc/rockchip/Kconfig
@@ -14,3 +14,13 @@ config SND_SOC_ROCKCHIP_I2S
Say Y or M if you want to add support for I2S driver for
Rockchip I2S device. The device supports upto maximum of
8 channels each for play and record.
+
+config SND_SOC_ROCKCHIP_MAX98090
+ tristate "ASoC support for Rockchip boards using a MAX98090 codec"
+ depends on SND_SOC_ROCKCHIP && I2C && GPIOLIB
+ select SND_SOC_ROCKCHIP_I2S
+ select SND_SOC_MAX98090
+ select SND_SOC_TS3A227E
+ help
+ Say Y or M here if you want to add support for SoC audio on Rockchip
+ boards using the MAX98090 codec, such as Veyron.
diff --git a/sound/soc/rockchip/Makefile b/sound/soc/rockchip/Makefile
index b921909..df3445b 100644
--- a/sound/soc/rockchip/Makefile
+++ b/sound/soc/rockchip/Makefile
@@ -2,3 +2,7 @@
snd-soc-i2s-objs := rockchip_i2s.o
obj-$(CONFIG_SND_SOC_ROCKCHIP_I2S) += snd-soc-i2s.o
+
+snd-soc-rockchip-max98090-objs := rockchip_max98090.o
+
+obj-$(CONFIG_SND_SOC_ROCKCHIP_MAX98090) += snd-soc-rockchip-max98090.o
diff --git a/sound/soc/rockchip/rockchip_max98090.c b/sound/soc/rockchip/rockchip_max98090.c
new file mode 100644
index 0000000..dcdce06
--- /dev/null
+++ b/sound/soc/rockchip/rockchip_max98090.c
@@ -0,0 +1,246 @@
+/*
+ * Rockchip machine ASoC driver for boards using a MAX90809 CODEC.
+ *
+ * Copyright (c) 2014, ROCKCHIP CORPORATION. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <sound/core.h>
+#include <sound/jack.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+
+#include "rockchip_i2s.h"
+#include "../codecs/ts3a227e.h"
+
+#define DRV_NAME "rockchip-snd-max98090"
+
+static struct snd_soc_jack headset_jack;
+static struct snd_soc_jack_pin headset_jack_pins[] = {
+ {
+ .pin = "Headset Jack",
+ .mask = SND_JACK_HEADPHONE | SND_JACK_MICROPHONE |
+ SND_JACK_BTN_0 | SND_JACK_BTN_1 |
+ SND_JACK_BTN_2 | SND_JACK_BTN_3,
+ },
+};
+
+static const struct snd_soc_dapm_widget rk_dapm_widgets[] = {
+ SND_SOC_DAPM_HP("Headphone", NULL),
+ SND_SOC_DAPM_MIC("Headset Mic", NULL),
+ SND_SOC_DAPM_MIC("Int Mic", NULL),
+ SND_SOC_DAPM_SPK("Speaker", NULL),
+};
+
+static const struct snd_soc_dapm_route rk_audio_map[] = {
+ {"IN34", NULL, "Headset Mic"},
+ {"IN34", NULL, "MICBIAS"},
+ {"MICBIAS", NULL, "Headset Mic"},
+ {"DMICL", NULL, "Int Mic"},
+ {"Headphone", NULL, "HPL"},
+ {"Headphone", NULL, "HPR"},
+ {"Speaker", NULL, "SPKL"},
+ {"Speaker", NULL, "SPKR"},
+};
+
+static const struct snd_kcontrol_new rk_mc_controls[] = {
+ SOC_DAPM_PIN_SWITCH("Headphone"),
+ SOC_DAPM_PIN_SWITCH("Headset Mic"),
+ SOC_DAPM_PIN_SWITCH("Int Mic"),
+ SOC_DAPM_PIN_SWITCH("Speaker"),
+};
+
+static int rk_aif1_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params)
+{
+ int ret = 0;
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+ struct snd_soc_dai *codec_dai = rtd->codec_dai;
+ int mclk;
+
+ switch (params_rate(params)) {
+ case 8000:
+ case 16000:
+ case 48000:
+ case 96000:
+ mclk = 12288000;
+ break;
+ case 44100:
+ mclk = 11289600;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ ret = snd_soc_dai_set_sysclk(cpu_dai, 0, mclk,
+ SND_SOC_CLOCK_OUT);
+ if (ret < 0) {
+ dev_err(codec_dai->dev, "Can't set codec clock %d\n", ret);
+ return ret;
+ }
+
+ ret = snd_soc_dai_set_sysclk(codec_dai, 0, mclk,
+ SND_SOC_CLOCK_IN);
+ if (ret < 0) {
+ dev_err(codec_dai->dev, "Can't set codec clock %d\n", ret);
+ return ret;
+ }
+
+ return ret;
+}
+
+static int rk_init(struct snd_soc_pcm_runtime *runtime)
+{
+ struct snd_soc_card *card = runtime->card;
+
+ card->dapm.idle_bias_off = true;
+
+ /* Enable Headset and 4 Buttons Jack detection */
+ return snd_soc_card_jack_new(card, "Headset Jack",
+ SND_JACK_HEADSET |
+ SND_JACK_BTN_0 | SND_JACK_BTN_1 |
+ SND_JACK_BTN_2 | SND_JACK_BTN_3,
+ &headset_jack,
+ headset_jack_pins,
+ ARRAY_SIZE(headset_jack_pins));
+}
+
+static int rk_98090_headset_init(struct snd_soc_component *component)
+{
+ return ts3a227e_enable_jack_detect(component, &headset_jack);
+}
+
+static struct snd_soc_ops rk_aif1_ops = {
+ .hw_params = rk_aif1_hw_params,
+};
+
+static struct snd_soc_aux_dev rk_98090_headset_dev = {
+ .name = "Headset Chip",
+ .init = rk_98090_headset_init,
+};
+
+static struct snd_soc_dai_link rk_dailink = {
+ .name = "max98090",
+ .stream_name = "Audio",
+ .codec_dai_name = "HiFi",
+ .init = rk_init,
+ .ops = &rk_aif1_ops,
+ /* set max98090 as slave */
+ .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
+ SND_SOC_DAIFMT_CBS_CFS,
+};
+
+static struct snd_soc_card snd_soc_card_rk = {
+ .name = "ROCKCHIP-I2S",
+ .dai_link = &rk_dailink,
+ .num_links = 1,
+ .aux_dev = &rk_98090_headset_dev,
+ .num_aux_devs = 1,
+ .dapm_widgets = rk_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(rk_dapm_widgets),
+ .dapm_routes = rk_audio_map,
+ .num_dapm_routes = ARRAY_SIZE(rk_audio_map),
+ .controls = rk_mc_controls,
+ .num_controls = ARRAY_SIZE(rk_mc_controls),
+};
+
+static int snd_rk_mc_probe(struct platform_device *pdev)
+{
+ int ret = 0;
+ struct snd_soc_card *card = &snd_soc_card_rk;
+ struct device_node *np = pdev->dev.of_node;
+
+ /* register the soc card */
+ card->dev = &pdev->dev;
+ platform_set_drvdata(pdev, card);
+
+ rk_dailink.codec_of_node = of_parse_phandle(np,
+ "rockchip,audio-codec", 0);
+ if (!rk_dailink.codec_of_node) {
+ dev_err(&pdev->dev,
+ "Property 'rockchip,audio-codec' missing or invalid\n");
+ return -EINVAL;
+ }
+
+ rk_dailink.cpu_of_node = of_parse_phandle(np,
+ "rockchip,i2s-controller", 0);
+ if (!rk_dailink.cpu_of_node) {
+ dev_err(&pdev->dev,
+ "Property 'rockchip,i2s-controller' missing or invalid\n");
+ return -EINVAL;
+ }
+
+ rk_dailink.platform_of_node = rk_dailink.cpu_of_node;
+
+ rk_98090_headset_dev.codec_of_node = of_parse_phandle(np,
+ "rockchip,headset-codec", 0);
+ if (!rk_98090_headset_dev.codec_of_node) {
+ dev_err(&pdev->dev,
+ "Property 'rockchip,headset-codec' missing/invalid\n");
+ return -EINVAL;
+ }
+
+ ret = snd_soc_register_card(card);
+ if (ret) {
+ pr_err("snd_soc_register_card failed %d\n", ret);
+ return ret;
+ }
+
+ ret = snd_soc_of_parse_card_name(card, "rockchip,model");
+ if (ret)
+ return ret;
+
+ return ret;
+}
+
+static int snd_rk_mc_remove(struct platform_device *pdev)
+{
+ struct snd_soc_card *soc_card = platform_get_drvdata(pdev);
+
+ snd_soc_card_set_drvdata(soc_card, NULL);
+ snd_soc_unregister_card(soc_card);
+ platform_set_drvdata(pdev, NULL);
+ return 0;
+}
+
+static const struct of_device_id rockchip_max98090_of_match[] = {
+ { .compatible = "rockchip,rockchip-audio-max98090", },
+ {},
+};
+
+static struct platform_driver snd_rk_mc_driver = {
+ .probe = snd_rk_mc_probe,
+ .remove = snd_rk_mc_remove,
+ .driver = {
+ .name = DRV_NAME,
+ .owner = THIS_MODULE,
+ .pm = &snd_soc_pm_ops,
+ .of_match_table = rockchip_max98090_of_match,
+ },
+};
+
+module_platform_driver(snd_rk_mc_driver);
+MODULE_AUTHOR("jianqun <[email protected]>");
+MODULE_DESCRIPTION("Rockchip max98090 machine ASoC driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRV_NAME);
+MODULE_DEVICE_TABLE(of, rockchip_max98090_of_match);
--
1.7.9.5
From: zhengxing <[email protected]>
The driver is used for rockchip board using a rt5645/rt5650.
Reviewed-by: Dylan Reid <[email protected]>
Signed-off-by: zhengxing <[email protected]>
---
sound/soc/rockchip/Kconfig | 9 ++
sound/soc/rockchip/Makefile | 2 +
sound/soc/rockchip/rockchip_rt5645.c | 235 ++++++++++++++++++++++++++++++++++
3 files changed, 246 insertions(+)
create mode 100644 sound/soc/rockchip/rockchip_rt5645.c
diff --git a/sound/soc/rockchip/Kconfig b/sound/soc/rockchip/Kconfig
index d123566..58bae8e 100644
--- a/sound/soc/rockchip/Kconfig
+++ b/sound/soc/rockchip/Kconfig
@@ -24,3 +24,12 @@ config SND_SOC_ROCKCHIP_MAX98090
help
Say Y or M here if you want to add support for SoC audio on Rockchip
boards using the MAX98090 codec, such as Veyron.
+
+config SND_SOC_ROCKCHIP_RT5645
+ tristate "ASoC support for Rockchip boards using a RT5645/RT5650 codec"
+ depends on SND_SOC_ROCKCHIP && I2C && GPIOLIB
+ select SND_SOC_ROCKCHIP_I2S
+ select SND_SOC_RT5645
+ help
+ Say Y or M here if you want to add support for SoC audio on Rockchip
+ boards using the RT5645/RT5650 codec, such as Veyron.
diff --git a/sound/soc/rockchip/Makefile b/sound/soc/rockchip/Makefile
index df3445b..1bc1dc3 100644
--- a/sound/soc/rockchip/Makefile
+++ b/sound/soc/rockchip/Makefile
@@ -4,5 +4,7 @@ snd-soc-i2s-objs := rockchip_i2s.o
obj-$(CONFIG_SND_SOC_ROCKCHIP_I2S) += snd-soc-i2s.o
snd-soc-rockchip-max98090-objs := rockchip_max98090.o
+snd-soc-rockchip-rt5645-objs := rockchip_rt5645.o
obj-$(CONFIG_SND_SOC_ROCKCHIP_MAX98090) += snd-soc-rockchip-max98090.o
+obj-$(CONFIG_SND_SOC_ROCKCHIP_RT5645) += snd-soc-rockchip-rt5645.o
diff --git a/sound/soc/rockchip/rockchip_rt5645.c b/sound/soc/rockchip/rockchip_rt5645.c
new file mode 100644
index 0000000..1a9a3f2
--- /dev/null
+++ b/sound/soc/rockchip/rockchip_rt5645.c
@@ -0,0 +1,235 @@
+/*
+ * Rockchip machine ASoC driver for boards using a RT5645/RT5650 CODEC.
+ *
+ * Copyright (c) 2015, ROCKCHIP CORPORATION. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <linux/delay.h>
+#include <sound/core.h>
+#include <sound/jack.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include "rockchip_i2s.h"
+
+#define DRV_NAME "rockchip-snd-rt5645"
+
+static struct snd_soc_jack headset_jack;
+
+/* Jack detect via rt5645 driver. */
+extern int rt5645_set_jack_detect(struct snd_soc_codec *codec,
+ struct snd_soc_jack *hp_jack, struct snd_soc_jack *mic_jack,
+ struct snd_soc_jack *btn_jack);
+
+static const struct snd_soc_dapm_widget rk_dapm_widgets[] = {
+ SND_SOC_DAPM_HP("Headphones", NULL),
+ SND_SOC_DAPM_SPK("Speakers", NULL),
+ SND_SOC_DAPM_MIC("Headset Mic", NULL),
+ SND_SOC_DAPM_MIC("Int Mic", NULL),
+};
+
+static const struct snd_soc_dapm_route rk_audio_map[] = {
+ /* Input Lines */
+ {"DMIC L2", NULL, "Int Mic"},
+ {"DMIC R2", NULL, "Int Mic"},
+ {"RECMIXL", NULL, "Headset Mic"},
+ {"RECMIXR", NULL, "Headset Mic"},
+
+ /* Output Lines */
+ {"Headphones", NULL, "HPOR"},
+ {"Headphones", NULL, "HPOL"},
+ {"Speakers", NULL, "SPOL"},
+ {"Speakers", NULL, "SPOR"},
+};
+
+static const struct snd_kcontrol_new rk_mc_controls[] = {
+ SOC_DAPM_PIN_SWITCH("Headphones"),
+ SOC_DAPM_PIN_SWITCH("Speakers"),
+ SOC_DAPM_PIN_SWITCH("Headset Mic"),
+ SOC_DAPM_PIN_SWITCH("Int Mic"),
+};
+
+static int rk_aif1_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params)
+{
+ int ret = 0;
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+ struct snd_soc_dai *codec_dai = rtd->codec_dai;
+ int mclk;
+
+ switch (params_rate(params)) {
+ case 8000:
+ case 16000:
+ case 48000:
+ case 96000:
+ mclk = 12288000;
+ break;
+ case 44100:
+ mclk = 11289600;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ ret = snd_soc_dai_set_sysclk(cpu_dai, 0, mclk,
+ SND_SOC_CLOCK_OUT);
+ if (ret < 0) {
+ dev_err(codec_dai->dev, "Can't set codec clock %d\n", ret);
+ return ret;
+ }
+
+ ret = snd_soc_dai_set_sysclk(codec_dai, 0, mclk,
+ SND_SOC_CLOCK_IN);
+ if (ret < 0) {
+ dev_err(codec_dai->dev, "Can't set codec clock %d\n", ret);
+ return ret;
+ }
+
+ return ret;
+}
+
+static int rk_init(struct snd_soc_pcm_runtime *runtime)
+{
+ struct snd_soc_card *card = runtime->card;
+ int ret;
+
+ card->dapm.idle_bias_off = true;
+
+ /* Enable Headset and 4 Buttons Jack detection */
+ ret = snd_soc_card_jack_new(card, "Headset Jack",
+ SND_JACK_HEADPHONE | SND_JACK_MICROPHONE |
+ SND_JACK_BTN_0 | SND_JACK_BTN_1 |
+ SND_JACK_BTN_2 | SND_JACK_BTN_3,
+ &headset_jack, NULL, 0);
+ if (!ret) {
+ dev_err(card->dev, "New Headset Jack failed! (%d)\n", ret);
+ return ret;
+ }
+
+ return rt5645_set_jack_detect(runtime->codec,
+ &headset_jack,
+ &headset_jack,
+ &headset_jack);
+}
+
+static struct snd_soc_ops rk_aif1_ops = {
+ .hw_params = rk_aif1_hw_params,
+};
+
+static struct snd_soc_dai_link rk_dailink = {
+ .name = "rt5645",
+ .stream_name = "rt5645 PCM",
+ .codec_dai_name = "rt5645-aif1",
+ .init = rk_init,
+ .ops = &rk_aif1_ops,
+ /* set rt5645 as slave */
+ .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
+ SND_SOC_DAIFMT_CBS_CFS,
+};
+
+static struct snd_soc_card snd_soc_card_rk = {
+ .name = "I2S-RT5650",
+ .dai_link = &rk_dailink,
+ .num_links = 1,
+ .dapm_widgets = rk_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(rk_dapm_widgets),
+ .dapm_routes = rk_audio_map,
+ .num_dapm_routes = ARRAY_SIZE(rk_audio_map),
+ .controls = rk_mc_controls,
+ .num_controls = ARRAY_SIZE(rk_mc_controls),
+};
+
+static int snd_rk_mc_probe(struct platform_device *pdev)
+{
+ int ret = 0;
+ struct snd_soc_card *card = &snd_soc_card_rk;
+ struct device_node *np = pdev->dev.of_node;
+
+ /* register the soc card */
+ card->dev = &pdev->dev;
+ platform_set_drvdata(pdev, card);
+
+ rk_dailink.codec_of_node = of_parse_phandle(np,
+ "rockchip,audio-codec", 0);
+ if (!rk_dailink.codec_of_node) {
+ dev_err(&pdev->dev,
+ "Property 'rockchip,audio-codec' missing or invalid\n");
+ return -EINVAL;
+ }
+
+ rk_dailink.cpu_of_node = of_parse_phandle(np,
+ "rockchip,i2s-controller", 0);
+ if (!rk_dailink.cpu_of_node) {
+ dev_err(&pdev->dev,
+ "Property 'rockchip,i2s-controller' missing or invalid\n");
+ return -EINVAL;
+ }
+
+ rk_dailink.platform_of_node = rk_dailink.cpu_of_node;
+
+ ret = snd_soc_of_parse_card_name(card, "rockchip,model");
+ if (ret)
+ return ret;
+
+ ret = snd_soc_register_card(card);
+ if (ret) {
+ pr_err("snd_soc_register_card failed %d\n", ret);
+ return ret;
+ }
+
+ return ret;
+}
+
+static int snd_rk_mc_remove(struct platform_device *pdev)
+{
+ struct snd_soc_card *soc_card = platform_get_drvdata(pdev);
+
+ snd_soc_card_set_drvdata(soc_card, NULL);
+ snd_soc_unregister_card(soc_card);
+ platform_set_drvdata(pdev, NULL);
+
+ return 0;
+}
+
+static const struct of_device_id rockchip_rt5645_of_match[] = {
+ { .compatible = "rockchip,rockchip-audio-rt5645", },
+ {},
+};
+
+static struct platform_driver snd_rk_mc_driver = {
+ .probe = snd_rk_mc_probe,
+ .remove = snd_rk_mc_remove,
+ .driver = {
+ .name = DRV_NAME,
+ .owner = THIS_MODULE,
+ .pm = &snd_soc_pm_ops,
+ .of_match_table = rockchip_rt5645_of_match,
+ },
+};
+
+module_platform_driver(snd_rk_mc_driver);
+
+MODULE_AUTHOR("Xing Zheng <[email protected]>");
+MODULE_DESCRIPTION("Rockchip rt5645 machine ASoC driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRV_NAME);
+MODULE_DEVICE_TABLE(of, rockchip_rt5645_of_match);
--
1.7.9.5
On wo, 2015-07-15 at 11:15 +0800, Xing Zheng wrote:
> --- /dev/null
> +++ b/sound/soc/rockchip/rockchip_max98090.c
> +#define DRV_NAME "rockchip-snd-max98090"
> +static const struct of_device_id rockchip_max98090_of_match[] = {
> + { .compatible = "rockchip,rockchip-audio-max98090", },
> + {},
> +};
> +
> +static struct platform_driver snd_rk_mc_driver = {
> + .probe = snd_rk_mc_probe,
> + .remove = snd_rk_mc_remove,
> + .driver = {
> + .name = DRV_NAME,
> + .owner = THIS_MODULE,
> + .pm = &snd_soc_pm_ops,
> + .of_match_table = rockchip_max98090_of_match,
> + },
> +};
> +
> +module_platform_driver(snd_rk_mc_driver);
Nit: empty line here.
> +MODULE_AUTHOR("jianqun <[email protected]>");
> +MODULE_DESCRIPTION("Rockchip max98090 machine ASoC driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:" DRV_NAME);
This seems only useful if there's a corresponding struct
platform_device. Ie, a platform_device with a "rockchip-snd-max98090"
.name, which will trigger a "MODALIAS=platform:rockchip-snd-max98090"
uevent when it's created. But I couldn't find where such a
platform_device is created.
Did I miss something? Or is there another way this alias is useful here?
> +MODULE_DEVICE_TABLE(of, rockchip_max98090_of_match);
The common pattern is to put MODULE_DEVICE_TABLE() directly after the
table it exports.
Likewise for 2/2 (except the empty line nit, that is).
Thanks,
Paul Bolle
On Thu, Jul 16, 2015 at 10:05:44AM +0200, Paul Bolle wrote:
> > +static struct platform_driver snd_rk_mc_driver = {
> > + .probe = snd_rk_mc_probe,
> > + .remove = snd_rk_mc_remove,
> > + .driver = {
> > + .name = DRV_NAME,
> > +MODULE_ALIAS("platform:" DRV_NAME);
> This seems only useful if there's a corresponding struct
> platform_device. Ie, a platform_device with a "rockchip-snd-max98090"
> .name, which will trigger a "MODALIAS=platform:rockchip-snd-max98090"
> uevent when it's created. But I couldn't find where such a
> platform_device is created.
> Did I miss something? Or is there another way this alias is useful here?
You've got platform_device and platform_driver confused I think.
Hi Paul,
Thank you for your reply.
On 2015年07月16日 16:05, Paul Bolle wrote:
> On wo, 2015-07-15 at 11:15 +0800, Xing Zheng wrote:
>> +static const struct of_device_id rockchip_max98090_of_match[] = {
>> + { .compatible = "rockchip,rockchip-audio-max98090", },
>> + {},
>> +};
>> +
>> +static struct platform_driver snd_rk_mc_driver = {
>> + .probe = snd_rk_mc_probe,
>> + .remove = snd_rk_mc_remove,
>> + .driver = {
>> + .name = DRV_NAME,
>> + .owner = THIS_MODULE,
>> + .pm =&snd_soc_pm_ops,
>> + .of_match_table = rockchip_max98090_of_match,
>> + },
>> +};
>> +
>> +module_platform_driver(snd_rk_mc_driver);
> Nit: empty line here.
Done.
>
>> +MODULE_AUTHOR("jianqun<[email protected]>");
>> +MODULE_DESCRIPTION("Rockchip max98090 machine ASoC driver");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_ALIAS("platform:" DRV_NAME);
> This seems only useful if there's a corresponding struct
> platform_device. Ie, a platform_device with a "rockchip-snd-max98090"
> .name, which will trigger a "MODALIAS=platform:rockchip-snd-max98090"
> uevent when it's created. But I couldn't find where such a
> platform_device is created.
>
> Did I miss something? Or is there another way this alias is useful here?
Yes, I didn't care about this but I think it maybe correct.
>> +MODULE_DEVICE_TABLE(of, rockchip_max98090_of_match);
> The common pattern is to put MODULE_DEVICE_TABLE() directly after the
> table it exports.
Done.
> Likewise for 2/2 (except the empty line nit, that is).
OK, done.
> Thanks,
>
>
> Paul Bolle
>
>
>
On 2015年07月16日 19:00, Mark Brown wrote:
> On Thu, Jul 16, 2015 at 10:05:44AM +0200, Paul Bolle wrote:
>
>>> +static struct platform_driver snd_rk_mc_driver = {
>>> + .probe = snd_rk_mc_probe,
>>> + .remove = snd_rk_mc_remove,
>>> + .driver = {
>>> + .name = DRV_NAME,
>>> +MODULE_ALIAS("platform:" DRV_NAME);
>> This seems only useful if there's a corresponding struct
>> platform_device. Ie, a platform_device with a "rockchip-snd-max98090"
>> .name, which will trigger a "MODALIAS=platform:rockchip-snd-max98090"
>> uevent when it's created. But I couldn't find where such a
>> platform_device is created.
>> Did I miss something? Or is there another way this alias is useful here?
> You've got platform_device and platform_driver confused I think.
Mark, Thank you.
On do, 2015-07-16 at 12:00 +0100, Mark Brown wrote:
> You've got platform_device and platform_driver confused I think.
I did? You mean that creating a platform_DRIVER triggers that
MODALIAS=platform:{...] uevent?
Paul Bolle
Hi,
On do, 2015-07-16 at 19:20 +0800, zhengxing wrote:
> > Did I miss something? Or is there another way this alias is useful
> > here?
> Yes, I didn't care about this but I think it maybe correct.
What is correct: my comment or the use of MODALIAS() in this patch?
Thanks,
Paul Bolle
Hi Paul,
I mean that the use of MODALIAS() in this patch, and I refered to
tegra_max98090.c(have been upstreamed) that used it like this also. So I
didn't care the using.
Thanks.
On 2015年07月16日 19:49, Paul Bolle wrote:
> Hi,
>
> On do, 2015-07-16 at 19:20 +0800, zhengxing wrote:
>>> Did I miss something? Or is there another way this alias is useful
>>> here?
>> Yes, I didn't care about this but I think it maybe correct.
> What is correct: my comment or the use of MODALIAS() in this patch?
>
> Thanks,
>
>
> Paul Bolle
>
>
>
On Thu, Jul 16, 2015 at 01:47:41PM +0200, Paul Bolle wrote:
> On do, 2015-07-16 at 12:00 +0100, Mark Brown wrote:
> > You've got platform_device and platform_driver confused I think.
> I did? You mean that creating a platform_DRIVER triggers that
> MODALIAS=platform:{...] uevent?
No, in that case you've not understood what the MODULE_ALIAS is
for. It's there so that userspace knows which module to load if it gets
a device with no driver, it goes in the driver not in the code
registering the device.
On do, 2015-07-16 at 13:55 +0100, Mark Brown wrote:
> No, in that case you've not understood what the MODULE_ALIAS is
> for. It's there so that userspace knows which module to load if it
> gets
> a device with no driver, it goes in the driver not in the code
> registering the device.
That's exactly how I understood MODULE_ALIAS() to work.
And it works, in short, because a platform device fires a
"MODALIAS=platform:[...]" uevent when it's created. And userspace uses
that uevent to load the module carrying that alias.
Let's put it this was. If one does
sudo find /sys -perm -o=r -name uevent -exec grep -H MODALIAS=platform: {} \;
or
sudo find /sys -perm -o=r -name modalias -exec grep -H platform: {} \;
(both lists should be similar)
on the systems this patch is targeting, will
platform:rockchip-snd-max98090
then show up?
Thanks,
Paul Bolle
Hi,
On do, 2015-07-16 at 19:59 +0800, zhengxing wrote:
> I mean that the use of MODALIAS() in this patch, and I refered to
> tegra_max98090.c(have been upstreamed) that used it like this also. So
> I didn't care the using.
And I think the same problem with MODULE_ALIAS() can be found in that
driver. Ie, where does the platform device that has a "tegra-snd
-max98090" .name hide? (See the reply I just sent to mark for more
details.)
Thanks,
Paul Bolle
On Thu, Jul 16, 2015 at 04:15:16PM +0200, Paul Bolle wrote:
> Let's put it this was. If one does
> sudo find /sys -perm -o=r -name uevent -exec grep -H MODALIAS=platform: {} \;
>
> or
> sudo find /sys -perm -o=r -name modalias -exec grep -H platform: {} \;
> (both lists should be similar)
> on the systems this patch is targeting, will
> platform:rockchip-snd-max98090
> then show up?
Why would this not be the case - what is the difference you beleive this
driver has to other platform drivers?
On 2015/7/16 22:19, Paul Bolle wrote:
> Hi,
>
> On do, 2015-07-16 at 19:59 +0800, zhengxing wrote:
>> I mean that the use of MODALIAS() in this patch, and I refered to
>> tegra_max98090.c(have been upstreamed) that used it like this also. So
>> I didn't care the using.
>
> And I think the same problem with MODULE_ALIAS() can be found in that
> driver. Ie, where does the platform device that has a "tegra-snd
> -max98090" .name hide? (See the reply I just sent to mark for more
> details.)
May be that is hidden in some device tree files.
MODULE_ALIAS() is used by a driver to announce that it supports
such types of devices. And bus enumerator will create those
devices by probing hardware or parsing some configuration files.
Thanks!
Gerry
Hi Gerry,
On do, 2015-07-16 at 23:25 +0800, Jiang Liu wrote:
> May be that is hidden in some device tree files.
> MODULE_ALIAS() is used by a driver to announce that it supports
> such types of devices. And bus enumerator will create those
> devices by probing hardware or parsing some configuration files.
Then someone could simply point me to the device tree file where that
"platform:" alias comes from. (Note that I, of course, do check the tree
for the substrings involved before tossing questions like these onto
lkml.)
Besides, as far as I can tell, for device tree support the magic
actually hides in
MODULE_DEVICE_TABLE(of, rockchip_max98090_of_match);
Which, I think, implies that any MODULE_ALIAS("platform:[...]") is
pointless for systems booting with device tree support.
Thanks,
Paul Bolle
On do, 2015-07-16 at 16:05 +0100, Mark Brown wrote:
> Why would this not be the case - what is the difference you beleive
> this driver has to other platform drivers?
It's my believe that for MODULE_ALIAS("platform:[...]") to be useful
there needs to be corresponding struct platform_device. For this patch
that would be a platform device named "rockchip-snd-max98090". (This is
something that I try to check rather carefully, because these devices
can be generated on the fly.)
I'm happy to drop this believe if someone shows me another way that
MODULE_ALIAS("platform:[...]") can actually be used.
So, in short, the difference between this driver and other platform
drivers is that, as far as I'm aware, this platform driver lacks a
corresponding platform device. Probably because OF support suffices to
get this module autoloaded.
Thanks,
Paul Bolle
On Thu, Jul 16, 2015 at 06:22:45PM +0200, Paul Bolle wrote:
> On do, 2015-07-16 at 16:05 +0100, Mark Brown wrote:
> > Why would this not be the case - what is the difference you beleive
> > this driver has to other platform drivers?
> It's my believe that for MODULE_ALIAS("platform:[...]") to be useful
> there needs to be corresponding struct platform_device. For this patch
> that would be a platform device named "rockchip-snd-max98090". (This is
> something that I try to check rather carefully, because these devices
> can be generated on the fly.)
> I'm happy to drop this believe if someone shows me another way that
> MODULE_ALIAS("platform:[...]") can actually be used.
> So, in short, the difference between this driver and other platform
> drivers is that, as far as I'm aware, this platform driver lacks a
> corresponding platform device. Probably because OF support suffices to
> get this module autoloaded.
This is a patch adding a device driver. We do not require that patches
adding device drivers also add architecture code to load the driver or
even be part of the same patch series since that isn't really helpful
for anything. We don't even require that board code be part of mainline
at all, though obviously we do encourage it.
If you want to make a tree wide effort to remove MODULE_ALIAS()s that
do not have any in tree users please do that separately. Right now it's
perfectly OK to do it.
Hi Paul,
On 2015年07月16日 22:15, Paul Bolle wrote:
>
> That's exactly how I understood MODULE_ALIAS() to work.
>
> And it works, in short, because a platform device fires a
> "MODALIAS=platform:[...]" uevent when it's created. And userspace uses
> that uevent to load the module carrying that alias.
>
> Let's put it this was. If one does
> sudo find /sys -perm -o=r -name uevent -exec grep -H MODALIAS=platform: {} \;
>
> or
> sudo find /sys -perm -o=r -name modalias -exec grep -H platform: {} \;
>
> (both lists should be similar)
>
> on the systems this patch is targeting, will
> platform:rockchip-snd-max98090
>
> then show up?
>
> Thanks,
>
>
> Paul Bolle
>
Thank you for your patience and detailed explanation.
I tested your two ways and didn't find the
"platform:rockchip-snd-max98090" in my device(kernel v3.14), and the
driver path is:
localhost rockchip-snd-max98090 # pwd
/sys/bus/platform/drivers/rockchip-snd-max98090
Thanks.
On Wed, Jul 15, 2015 at 11:15:42AM +0800, Xing Zheng wrote:
This looks pretty good, a couple of minor points below which should be
quick to fix.
> +static int rk_init(struct snd_soc_pcm_runtime *runtime)
> +{
> + struct snd_soc_card *card = runtime->card;
> +
> + card->dapm.idle_bias_off = true;
You shouldn't need to do this? If you do need to do it we should make
it possible to do it from the card struct.
> + ret = snd_soc_register_card(card);
> + if (ret) {
> + pr_err("snd_soc_register_card failed %d\n", ret);
> + return ret;
> + }
> +
> + ret = snd_soc_of_parse_card_name(card, "rockchip,model");
> + if (ret)
> + return ret;
This should be devm_snd_soc_register_card() and you need to parse the
card name before registering it, otherwise the card might instantiate
before the name is set.
> +static int snd_rk_mc_remove(struct platform_device *pdev)
> +{
> + struct snd_soc_card *soc_card = platform_get_drvdata(pdev);
> +
> + snd_soc_card_set_drvdata(soc_card, NULL);
> + snd_soc_unregister_card(soc_card);
> + platform_set_drvdata(pdev, NULL);
No need for any of the _set_drvdata() calls, the core does them and they
shouldn't make any difference anyway.
On Wed, Jul 15, 2015 at 11:15:43AM +0800, Xing Zheng wrote:
> From: zhengxing <[email protected]>
>
> The driver is used for rockchip board using a rt5645/rt5650.
Same comments here, mostly good but some minor issues. The other thing
with both of these is that you need to have a binding document for them.
On 2015年07月18日 02:04, Mark Brown wrote:
> On Wed, Jul 15, 2015 at 11:15:42AM +0800, Xing Zheng wrote:
>
> This looks pretty good, a couple of minor points below which should be
> quick to fix.
>
>> +static int rk_init(struct snd_soc_pcm_runtime *runtime)
>> +{
>> + struct snd_soc_card *card = runtime->card;
>> +
>> + card->dapm.idle_bias_off = true;
> You shouldn't need to do this? If you do need to do it we should make
> it possible to do it from the card struct.
Done, we don't need it in the machine driver.
>> + ret = snd_soc_register_card(card);
>> + if (ret) {
>> + pr_err("snd_soc_register_card failed %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ret = snd_soc_of_parse_card_name(card, "rockchip,model");
>> + if (ret)
>> + return ret;
> This should be devm_snd_soc_register_card() and you need to parse the
> card name before registering it, otherwise the card might instantiate
> before the name is set.
Done.
>> +static int snd_rk_mc_remove(struct platform_device *pdev)
>> +{
>> + struct snd_soc_card *soc_card = platform_get_drvdata(pdev);
>> +
>> + snd_soc_card_set_drvdata(soc_card, NULL);
>> + snd_soc_unregister_card(soc_card);
>> + platform_set_drvdata(pdev, NULL);
> No need for any of the _set_drvdata() calls, the core does them and they
> shouldn't make any difference anyway.
Done. I will remove *_set_drvdata and *get_drvdata because we don't need
them any more.
Thanks.
Hi Mark,
On 2015年07月18日 02:11, Mark Brown wrote:
> On Wed, Jul 15, 2015 at 11:15:43AM +0800, Xing Zheng wrote:
>> From: zhengxing<[email protected]>
>>
>> The driver is used for rockchip board using a rt5645/rt5650.
> Same comments here, mostly good but some minor issues. The other thing
> with both of these is that you need to have a binding document for them.
OK. Thank you.
>>> +static int snd_rk_mc_remove(struct platform_device *pdev)
>>> +{
>>> + struct snd_soc_card *soc_card = platform_get_drvdata(pdev);
>>> +
>>> + snd_soc_card_set_drvdata(soc_card, NULL);
>>> + snd_soc_unregister_card(soc_card);
>>> + platform_set_drvdata(pdev, NULL);
>> No need for any of the _set_drvdata() calls, the core does them and they
>> shouldn't make any difference anyway.
> Done. I will remove *_set_drvdata and *get_drvdata because we don't
> need them any more.
>
Sorry, I mean just remove platform_set_drvdata(pdev, NULL);