On Thu, Sep 19, 2019 at 11:08 PM Jernej Škrabec <jernej.skrabec@xxxxxxxx> wrote: > > Hi! > > Dne četrtek, 19. september 2019 ob 15:54:49 CEST je Cheng-Yi Chiang > napisal(a): > > Use two dai_links. One for HDMI and one for max98090. > > With this setup, audio can play to speaker and HDMI selectively. > > > > Signed-off-by: Cheng-Yi Chiang <cychiang@xxxxxxxxxxxx> > > --- > > .../boot/dts/rk3288-veyron-analog-audio.dtsi | 1 + > > sound/soc/rockchip/rockchip_max98090.c | 129 ++++++++++++++---- > > 2 files changed, 103 insertions(+), 27 deletions(-) > > > > diff --git a/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi > > b/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi index > > 445270aa136e..51208d161d65 100644 > > --- a/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi > > +++ b/arch/arm/boot/dts/rk3288-veyron-analog-audio.dtsi > > @@ -17,6 +17,7 @@ > > rockchip,hp-det-gpios = <&gpio6 RK_PA5 > GPIO_ACTIVE_HIGH>; > > rockchip,mic-det-gpios = <&gpio6 RK_PB3 > GPIO_ACTIVE_LOW>; > > rockchip,headset-codec = <&headsetcodec>; > > + rockchip,hdmi-codec = <&hdmi>; > > }; > > }; > > > > diff --git a/sound/soc/rockchip/rockchip_max98090.c > > b/sound/soc/rockchip/rockchip_max98090.c index c5fc24675a33..6c217492bb30 > > 100644 > > --- a/sound/soc/rockchip/rockchip_max98090.c > > +++ b/sound/soc/rockchip/rockchip_max98090.c > > @@ -11,6 +11,7 @@ > > #include <linux/gpio.h> > > #include <linux/of_gpio.h> > > #include <sound/core.h> > > +#include <sound/hdmi-codec.h> > > #include <sound/jack.h> > > #include <sound/pcm.h> > > #include <sound/pcm_params.h> > > @@ -41,6 +42,7 @@ static const struct snd_soc_dapm_widget rk_dapm_widgets[] > > = { SND_SOC_DAPM_MIC("Headset Mic", NULL), > > SND_SOC_DAPM_MIC("Int Mic", NULL), > > SND_SOC_DAPM_SPK("Speaker", NULL), > > + SND_SOC_DAPM_LINE("HDMI", NULL), > > }; > > > > static const struct snd_soc_dapm_route rk_audio_map[] = { > > @@ -52,6 +54,7 @@ static const struct snd_soc_dapm_route rk_audio_map[] = { > > {"Headphone", NULL, "HPR"}, > > {"Speaker", NULL, "SPKL"}, > > {"Speaker", NULL, "SPKR"}, > > + {"HDMI", NULL, "TX"}, > > }; > > > > static const struct snd_kcontrol_new rk_mc_controls[] = { > > @@ -59,6 +62,7 @@ static const struct snd_kcontrol_new rk_mc_controls[] = { > > SOC_DAPM_PIN_SWITCH("Headset Mic"), > > SOC_DAPM_PIN_SWITCH("Int Mic"), > > SOC_DAPM_PIN_SWITCH("Speaker"), > > + SOC_DAPM_PIN_SWITCH("HDMI"), > > }; > > > > static int rk_aif1_hw_params(struct snd_pcm_substream *substream, > > @@ -92,38 +96,63 @@ static int rk_aif1_hw_params(struct snd_pcm_substream > > *substream, > > > > 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); > > + if (ret) { > > + dev_err(cpu_dai->dev, "Can't set cpu dai clock %d\n", > ret); > > return ret; > > } > > > > + /* HDMI codec dai does not need to set sysclk. */ > > + if (!strcmp(rtd->dai_link->name, "HDMI")) > > + return 0; > > + > > 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); > > + if (ret) { > > + dev_err(codec_dai->dev, "Can't set codec dai clock > %d\n", ret); > > return ret; > > } > > > > - return ret; > > + return 0; > > } > > > > static const struct snd_soc_ops rk_aif1_ops = { > > .hw_params = rk_aif1_hw_params, > > }; > > > > -SND_SOC_DAILINK_DEFS(hifi, > > - DAILINK_COMP_ARRAY(COMP_EMPTY()), > > - DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "HiFi")), > > - DAILINK_COMP_ARRAY(COMP_EMPTY())); > > - > > -static struct snd_soc_dai_link rk_dailink = { > > - .name = "max98090", > > - .stream_name = "Audio", > > - .ops = &rk_aif1_ops, > > - /* set max98090 as slave */ > > - .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | > > - SND_SOC_DAIFMT_CBS_CFS, > > - SND_SOC_DAILINK_REG(hifi), > > +SND_SOC_DAILINK_DEFS(analog, > > + DAILINK_COMP_ARRAY(COMP_EMPTY()), > > + DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "HiFi")), > > + DAILINK_COMP_ARRAY(COMP_EMPTY())); > > + > > +SND_SOC_DAILINK_DEFS(hdmi, > > + DAILINK_COMP_ARRAY(COMP_EMPTY()), > > + DAILINK_COMP_ARRAY(COMP_CODEC(NULL, "i2s-hifi")), > > + DAILINK_COMP_ARRAY(COMP_EMPTY())); > > + > > +enum { > > + DAILINK_MAX98090, > > + DAILINK_HDMI, > > +}; > > + > > +/* max98090 and HDMI codec dai_link */ > > +static struct snd_soc_dai_link rk_dailinks[] = { > > + [DAILINK_MAX98090] = { > > + .name = "max98090", > > + .stream_name = "Analog", > > + .ops = &rk_aif1_ops, > > + /* set max98090 as slave */ > > + .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | > > + SND_SOC_DAIFMT_CBS_CFS, > > + SND_SOC_DAILINK_REG(analog), > > + }, > > + [DAILINK_HDMI] = { > > + .name = "HDMI", > > + .stream_name = "HDMI", > > + .ops = &rk_aif1_ops, > > + .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | > > + SND_SOC_DAIFMT_CBS_CFS, > > + SND_SOC_DAILINK_REG(hdmi), > > + } > > }; > > > > static int rk_98090_headset_init(struct snd_soc_component *component); > > @@ -136,8 +165,8 @@ static struct snd_soc_aux_dev rk_98090_headset_dev = { > > static struct snd_soc_card snd_soc_card_rk = { > > .name = "ROCKCHIP-I2S", > > .owner = THIS_MODULE, > > - .dai_link = &rk_dailink, > > - .num_links = 1, > > + .dai_link = rk_dailinks, > > + .num_links = ARRAY_SIZE(rk_dailinks), > > .aux_dev = &rk_98090_headset_dev, > > .num_aux_devs = 1, > > .dapm_widgets = rk_dapm_widgets, > > @@ -173,27 +202,73 @@ 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; > > + struct device_node *np_analog; > > + struct device_node *np_cpu; > > + struct device_node *np_hdmi_codec; > > + struct of_phandle_args args; > > > > /* register the soc card */ > > card->dev = &pdev->dev; > > > > - rk_dailink.codecs->of_node = of_parse_phandle(np, > > - "rockchip,audio-codec", 0); > > - if (!rk_dailink.codecs->of_node) { > > + np_analog = of_parse_phandle(np, "rockchip,audio-codec", 0); > > + if (!np_analog) { > > dev_err(&pdev->dev, > > "Property 'rockchip,audio-codec' missing or > invalid\n"); > > return -EINVAL; > > } > > + rk_dailinks[DAILINK_MAX98090].codecs->of_node = np_analog; > > + > > + ret = of_parse_phandle_with_fixed_args(np, "rockchip,audio-codec", > > + 0, 0, &args); > > + if (ret) { > > + dev_err(&pdev->dev, > > + "Unable to parse property 'rockchip,audio- > codec'\n"); > > + return ret; > > + } > > + > > + ret = snd_soc_get_dai_name( > > + &args, &rk_dailinks[DAILINK_MAX98090].codecs- > >dai_name); > > + if (ret) { > > + dev_err(&pdev->dev, "Unable to get codec dai_name\n"); > > + return ret; > > + } > > + > > + np_cpu = of_parse_phandle(np, "rockchip,i2s-controller", 0); > > > > - rk_dailink.cpus->of_node = of_parse_phandle(np, > > - "rockchip,i2s-controller", 0); > > - if (!rk_dailink.cpus->of_node) { > > + if (!np_cpu) { > > dev_err(&pdev->dev, > > "Property 'rockchip,i2s-controller' missing > or invalid\n"); > > return -EINVAL; > > } > > > > - rk_dailink.platforms->of_node = rk_dailink.cpus->of_node; > > + np_hdmi_codec = of_parse_phandle(np, "rockchip,hdmi-codec", 0); > > + if (!np_hdmi_codec) { > > + dev_err(&pdev->dev, > > + "Property 'rockchip,hdmi-codec' missing or > invalid\n"); > > + return -EINVAL; > > + } > > Property "rockchip,hdmi-codec" is added in this series, right? You can't make > it mandatory, because kernel must be backward compatible with old device tree > files and they don't have this property. > > Think about use case when user happily used this driver and after kernel > update, it suddenly stops working. You can't assume that board DTB file will be > updated along with kernel update. > > Just make it optional and don't expose jack functionality if it's not present. Hi Jernej, Thanks for the reply. I see. Yes I can make it optional. But it will become a little bit messy for two types of usage to share one machine driver. I think I will create two instances of structs for dapm widgets, dapm routes, kcontrols, dai_links for "max98090 only" and "max98090+hdmi" and set those fields in snd_soc_card depending on depending on the property. These two usages can still share most of the function calls. Hope this looks clean. If you have a cleaner way of sharing machine driver please let me know. I'll post an update probably next week. Thanks a lot! > > Best regards, > Jernej > > > + > > + rk_dailinks[DAILINK_HDMI].codecs->of_node = np_hdmi_codec; > > + > > + ret = of_parse_phandle_with_fixed_args(np, "rockchip,hdmi-codec", > > + 0, 0, &args); > > + if (ret) { > > + dev_err(&pdev->dev, > > + "Unable to parse property 'rockchip,hdmi- > codec'\n"); > > + return ret; > > + } > > + > > + ret = snd_soc_get_dai_name( > > + &args, &rk_dailinks[DAILINK_HDMI].codecs- > >dai_name); > > + if (ret) { > > + dev_err(&pdev->dev, "Unable to get hdmi codec > dai_name\n"); > > + return ret; > > + } > > + > > + rk_dailinks[DAILINK_MAX98090].cpus->of_node = np_cpu; > > + rk_dailinks[DAILINK_MAX98090].platforms->of_node = np_cpu; > > + rk_dailinks[DAILINK_HDMI].cpus->of_node = np_cpu; > > + rk_dailinks[DAILINK_HDMI].platforms->of_node = np_cpu; > > > > rk_98090_headset_dev.codec_of_node = of_parse_phandle(np, > > "rockchip,headset-codec", 0); > > > > _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel