[PATCH v2 0/3] ASoC: rockchip: Parse dai links from dts

[PATCH v2 0/3] ASoC: rockchip: Parse dai links from dts

[Jeffy Chen]

Currently we are using a fixed list of dai links in the driver.
This serial of patches would let the driver parse dai links from
dts, so that we can disable some of them for future boards in the
dts.

Tested on my chromebook bob(with cros 4.4 kernel), it still works
after disabled rt5514 codec in the dts.


Changes in v2:
Let rockchip,codec-names be a required property, because we plan to
add more supported codecs to the fixed dai link list in the driver.
Let rockchip,codec-names be a required property.

Jeffy Chen (3):
  ASoC: rockchip: Parse dai links from dts
  arm64: dts: rockchip: Add rockchip,codec-names property
  dt-bindings: ASoC: rockchip: Add rockchip,codec-names property

 .../bindings/sound/rockchip,rk3399-gru-sound.txt   |   2 +
 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi       |   1 +
 sound/soc/rockchip/rk3399_gru_sound.c              | 125 +++++++++++++--------
 3 files changed, 84 insertions(+), 44 deletions(-)

-- 
2.1.4

[PATCH v2 1/3] ASoC: rockchip: Parse dai links from dts

[Jeffy Chen]

Refactor rockchip_sound_probe, parse dai links from dts instead of
hard coding them.

Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---

Changes in v2:
Let rockchip,codec-names be a required property, because we plan to
add more supported codecs to the fixed dai link list in the driver.

 sound/soc/rockchip/rk3399_gru_sound.c | 125 ++++++++++++++++++++++------------
 1 file changed, 81 insertions(+), 44 deletions(-)

diff --git a/sound/soc/rockchip/rk3399_gru_sound.c b/sound/soc/rockchip/rk3399_gru_sound.c
index 3475c61..03b7fae 100644
--- a/sound/soc/rockchip/rk3399_gru_sound.c
+++ b/sound/soc/rockchip/rk3399_gru_sound.c
@@ -247,9 +247,7 @@ enum {
 	DAILINK_RT5514_DSP,
 };
 
-#define DAILINK_ENTITIES	(DAILINK_DA7219 + 1)
-
-static struct snd_soc_dai_link rockchip_dailinks[] = {
+static const struct snd_soc_dai_link rockchip_dais[] = {
 	[DAILINK_MAX98357A] = {
 		.name = "MAX98357A",
 		.stream_name = "MAX98357A PCM",
@@ -290,8 +288,6 @@ static struct snd_soc_dai_link rockchip_dailinks[] = {
 static struct snd_soc_card rockchip_sound_card = {
 	.name = "rk3399-gru-sound",
 	.owner = THIS_MODULE,
-	.dai_link = rockchip_dailinks,
-	.num_links =  ARRAY_SIZE(rockchip_dailinks),
 	.dapm_widgets = rockchip_dapm_widgets,
 	.num_dapm_widgets = ARRAY_SIZE(rockchip_dapm_widgets),
 	.dapm_routes = rockchip_dapm_routes,
@@ -305,71 +301,112 @@ static int rockchip_sound_match_stub(struct device *dev, void *data)
 	return 1;
 }
 
-static int rockchip_sound_probe(struct platform_device *pdev)
+static int rockchip_sound_of_parse_dais(struct device *dev,
+					struct snd_soc_card *card)
 {
-	struct snd_soc_card *card = &rockchip_sound_card;
+	struct device *rt5514_dev;
+	struct device_driver *rt5514_drv;
 	struct device_node *cpu_node;
-	struct device *dev;
-	struct device_driver *drv;
-	int i, ret;
-
-	cpu_node = of_parse_phandle(pdev->dev.of_node, "rockchip,cpu", 0);
-	if (!cpu_node) {
-		dev_err(&pdev->dev, "Property 'rockchip,cpu' missing or invalid\n");
-		return -EINVAL;
-	}
-
-	for (i = 0; i < DAILINK_ENTITIES; i++) {
-		rockchip_dailinks[i].platform_of_node = cpu_node;
-		rockchip_dailinks[i].cpu_of_node = cpu_node;
+	struct device_node *np_codec;
+	struct snd_soc_dai_link *dai;
+	bool has_rt5514 = false;
+	int i, index, ret;
+
+	card->dai_link = devm_kzalloc(dev, sizeof(rockchip_dais),
+				      GFP_KERNEL);
+	if (!card->dai_link)
+		return -ENOMEM;
+
+	cpu_node = of_parse_phandle(dev->of_node, "rockchip,cpu", 0);
+
+	card->num_links = 0;
+	for (i = 0; i < DAILINK_RT5514_DSP; i++) {
+		index = of_property_match_string(dev->of_node,
+				"rockchip,codec-names",
+				rockchip_dais[i].name);
+		if (index < 0)
+			continue;
+
+		np_codec = of_parse_phandle(dev->of_node,
+					    "rockchip,codec", index);
+		if (!np_codec) {
+			dev_err(dev, "Missing 'rockchip,codec' for %s\n",
+				rockchip_dais[i].name);
+			return -EINVAL;
+		}
+		if (!of_device_is_available(np_codec))
+			continue;
 
-		rockchip_dailinks[i].codec_of_node =
-			of_parse_phandle(pdev->dev.of_node, "rockchip,codec", i);
-		if (!rockchip_dailinks[i].codec_of_node) {
-			dev_err(&pdev->dev,
-				"Property[%d] 'rockchip,codec' missing or invalid\n", i);
+		if (!cpu_node) {
+			dev_err(dev, "Missing 'rockchip,cpu' for %s\n",
+				rockchip_dais[i].name);
 			return -EINVAL;
 		}
+
+		dai = &card->dai_link[card->num_links++];
+		*dai = rockchip_dais[i];
+
+		dai->codec_of_node = np_codec;
+		dai->platform_of_node = cpu_node;
+		dai->cpu_of_node = cpu_node;
+
+		if (i == DAILINK_RT5514)
+			has_rt5514 = true;
 	}
 
+	if (!has_rt5514)
+		return 0;
+
 	/**
 	 * To acquire the spi driver of the rt5514 and set the dai-links names
 	 * for soc_bind_dai_link
 	 */
-	drv = driver_find("rt5514", &spi_bus_type);
-	if (!drv) {
-		dev_err(&pdev->dev, "Can not find the rt5514 driver at the spi bus\n");
+	rt5514_drv = driver_find("rt5514", &spi_bus_type);
+	if (!rt5514_drv) {
+		dev_err(dev, "Can not find the rt5514 driver at the spi bus\n");
 		return -EINVAL;
 	}
 
-	dev = driver_find_device(drv, NULL, NULL, rockchip_sound_match_stub);
-	if (!dev) {
-		dev_err(&pdev->dev, "Can not find the rt5514 device\n");
+	rt5514_dev = driver_find_device(rt5514_drv, NULL, NULL,
+					rockchip_sound_match_stub);
+	if (!rt5514_dev) {
+		dev_err(dev, "Can not find the rt5514 device\n");
 		return -ENODEV;
 	}
 
 	/* Set DMIC delay */
-	ret = device_property_read_u32(&pdev->dev, "dmic-delay",
-					&rt5514_dmic_delay);
-	if (ret) {
+	ret = device_property_read_u32(dev, "dmic-delay", &rt5514_dmic_delay);
+	if (ret < 0) {
 		rt5514_dmic_delay = 0;
-		dev_dbg(&pdev->dev,
+		dev_dbg(dev,
 			"no optional property 'dmic-delay' found, default: no delay\n");
 	}
 
-	rockchip_dailinks[DAILINK_RT5514_DSP].cpu_name = kstrdup_const(dev_name(dev), GFP_KERNEL);
-	rockchip_dailinks[DAILINK_RT5514_DSP].cpu_dai_name = kstrdup_const(dev_name(dev), GFP_KERNEL);
-	rockchip_dailinks[DAILINK_RT5514_DSP].platform_name = kstrdup_const(dev_name(dev), GFP_KERNEL);
+	dai = &card->dai_link[card->num_links++];
+	*dai = rockchip_dais[DAILINK_RT5514_DSP];
+
+	dai->cpu_name = devm_kstrdup(dev, dev_name(rt5514_dev), GFP_KERNEL);
+	dai->cpu_dai_name = dai->cpu_name;
+	dai->platform_name = dai->cpu_name;
+
+	return 0;
+}
+
+static int rockchip_sound_probe(struct platform_device *pdev)
+{
+	struct snd_soc_card *card = &rockchip_sound_card;
+	int ret;
+
+	ret = rockchip_sound_of_parse_dais(&pdev->dev, card);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to parse dais: %d\n", ret);
+		return ret;
+	}
 
 	card->dev = &pdev->dev;
 	platform_set_drvdata(pdev, card);
 
-	ret = devm_snd_soc_register_card(&pdev->dev, card);
-	if (ret)
-		dev_err(&pdev->dev, "%s snd_soc_register_card fail %d\n",
-			__func__, ret);
-
-	return ret;
+	return devm_snd_soc_register_card(&pdev->dev, card);
 }
 
 static const struct of_device_id rockchip_sound_of_match[] = {
-- 
2.1.4

[PATCH v2 2/3] arm64: dts: rockchip: Add rockchip,codec-names property

[Jeffy Chen]

Add rockchip,codec-names property for codecs.

Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---

Changes in v2: None

 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
index d48e98b..c8f7f0c 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
@@ -515,6 +515,7 @@
 		compatible = "rockchip,rk3399-gru-sound";
 		rockchip,cpu = <&i2s0 &i2s2>;
 		rockchip,codec = <&max98357a &headsetcodec &codec>;
+		rockchip,codec-names = "MAX98357A", "RT5514", "DA7219";
 	};
 };
 
-- 
2.1.4

[PATCH v2 3/3] dt-bindings: ASoC: rockchip: Add rockchip,codec-names property

[Jeffy Chen]

Add a new rockchip,codec-names property, so that the driver can parse
the codecs by name.

Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---

Changes in v2:
Let rockchip,codec-names be a required property.

 Documentation/devicetree/bindings/sound/rockchip,rk3399-gru-sound.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/rockchip,rk3399-gru-sound.txt b/Documentation/devicetree/bindings/sound/rockchip,rk3399-gru-sound.txt
index eac91db..05351df 100644
--- a/Documentation/devicetree/bindings/sound/rockchip,rk3399-gru-sound.txt
+++ b/Documentation/devicetree/bindings/sound/rockchip,rk3399-gru-sound.txt
@@ -5,6 +5,7 @@ Required properties:
 - rockchip,cpu: The phandle of the Rockchip I2S controller that's
   connected to the codecs
 - rockchip,codec: The phandle of the MAX98357A/RT5514/DA7219 codecs
+- rockchip,codec-names: The names of the MAX98357A/RT5514/DA7219 codecs
 
 Optional properties:
 - dmic-wakeup-delay-ms : specify delay time (ms) for DMIC ready.
@@ -18,5 +19,6 @@ sound {
 	compatible = "rockchip,rk3399-gru-sound";
 	rockchip,cpu = <&i2s0>;
 	rockchip,codec = <&max98357a &rt5514 &da7219>;
+	rockchip,codec-names = "MAX98357A", "RT5514", "DA7219";
 	dmic-wakeup-delay-ms = <20>;
 };
-- 
2.1.4

Re: [PATCH v2 3/3] dt-bindings: ASoC: rockchip: Add rockchip,codec-names property

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 253 bytes --]

On Thu, Aug 10, 2017 at 12:54:58PM +0800, Jeffy Chen wrote:
> Add a new rockchip,codec-names property, so that the driver can parse
> the codecs by name.

Why?  You're already referencing the CODECs by phandle and these names
are not part of any ABI...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 3/3] dt-bindings: ASoC: rockchip: Add rockchip,codec-names property

[jeffy]

Hi Mark,

On 08/10/2017 10:56 PM, Mark Brown wrote:
> On Thu, Aug 10, 2017 at 12:54:58PM +0800, Jeffy Chen wrote:
>> Add a new rockchip,codec-names property, so that the driver can parse
>> the codecs by name.
>
> Why?  You're already referencing the CODECs by phandle and these names
> are not part of any ABI...
>

currently we are binding the phandles to a fixed codec list in the 
driver. but we want to make it dynamic, since some exist codecs could be 
optional, and some new codecs could be add for new board too.

and the support of this new property in the driver is added in 
https://patchwork.kernel.org/patch/9892737

Re: [PATCH v2 1/3] ASoC: rockchip: Parse dai links from dts

[Matthias Kaehlcke]

El Thu, Aug 10, 2017 at 12:54:56PM +0800 Jeffy Chen ha dit:

> Refactor rockchip_sound_probe, parse dai links from dts instead of
> hard coding them.

Mark doesn't seem to be overly convinced that 'rockchip,codec-names'
is a good idea (https://lkml.org/lkml/2017/8/10/511).

How about using something like this instead:

static const char *dailink_compat[] = {
	[DAILINK_MAX98357A] = "maxim,max98357a",
	[DAILINK_RT5514] = "realtek,rt5514",
	[DAILINK_DA7219] = "dlg,da7219",
};

...

static int rockchip_sound_probe(struct platform_device *pdev)
{
	...

	for (i = 0; ; i++) {
		struct device_node *codec_node;
		int j;

		codec_node = of_parse_phandle(pdev->dev.of_node, "rockchip,codec", i);
		if (!codec_node)
			break;

		for (j = 0; j < DAILINK_ENTITIES; j++) {
			if (of_device_is_compatible(codec_node, dailink_compat[j]))
				break;
		}

		if (j == DAILINK_ENTITIES) {
			dev_err(...);
			return -EINVAL;
		}

		rockchip_dailinks[j].codec_of_node = codec_node;
		rockchip_dailinks[j].platform_of_node = cpu_node;
		rockchip_dailinks[j].cpu_of_node = cpu_node;

		/* not strictly needed, could also check for
		   rockchip_dailinks[DAILINK_RT5514].cpu_of_node or so
		   */
		if (j == DAILINK_RT5514)
			has_rt5514 = true;
	}
	...
}

Matthias

> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> ---
> 
> Changes in v2:
> Let rockchip,codec-names be a required property, because we plan to
> add more supported codecs to the fixed dai link list in the driver.
> 
>  sound/soc/rockchip/rk3399_gru_sound.c | 125 ++++++++++++++++++++++------------
>  1 file changed, 81 insertions(+), 44 deletions(-)
> 
> diff --git a/sound/soc/rockchip/rk3399_gru_sound.c b/sound/soc/rockchip/rk3399_gru_sound.c
> index 3475c61..03b7fae 100644
> --- a/sound/soc/rockchip/rk3399_gru_sound.c
> +++ b/sound/soc/rockchip/rk3399_gru_sound.c
> @@ -247,9 +247,7 @@ enum {
>  	DAILINK_RT5514_DSP,
>  };
>  
> -#define DAILINK_ENTITIES	(DAILINK_DA7219 + 1)
> -
> -static struct snd_soc_dai_link rockchip_dailinks[] = {
> +static const struct snd_soc_dai_link rockchip_dais[] = {
>  	[DAILINK_MAX98357A] = {
>  		.name = "MAX98357A",
>  		.stream_name = "MAX98357A PCM",
> @@ -290,8 +288,6 @@ static struct snd_soc_dai_link rockchip_dailinks[] = {
>  static struct snd_soc_card rockchip_sound_card = {
>  	.name = "rk3399-gru-sound",
>  	.owner = THIS_MODULE,
> -	.dai_link = rockchip_dailinks,
> -	.num_links =  ARRAY_SIZE(rockchip_dailinks),
>  	.dapm_widgets = rockchip_dapm_widgets,
>  	.num_dapm_widgets = ARRAY_SIZE(rockchip_dapm_widgets),
>  	.dapm_routes = rockchip_dapm_routes,
> @@ -305,71 +301,112 @@ static int rockchip_sound_match_stub(struct device *dev, void *data)
>  	return 1;
>  }
>  
> -static int rockchip_sound_probe(struct platform_device *pdev)
> +static int rockchip_sound_of_parse_dais(struct device *dev,
> +					struct snd_soc_card *card)
>  {
> -	struct snd_soc_card *card = &rockchip_sound_card;
> +	struct device *rt5514_dev;
> +	struct device_driver *rt5514_drv;
>  	struct device_node *cpu_node;
> -	struct device *dev;
> -	struct device_driver *drv;
> -	int i, ret;
> -
> -	cpu_node = of_parse_phandle(pdev->dev.of_node, "rockchip,cpu", 0);
> -	if (!cpu_node) {
> -		dev_err(&pdev->dev, "Property 'rockchip,cpu' missing or invalid\n");
> -		return -EINVAL;
> -	}
> -
> -	for (i = 0; i < DAILINK_ENTITIES; i++) {
> -		rockchip_dailinks[i].platform_of_node = cpu_node;
> -		rockchip_dailinks[i].cpu_of_node = cpu_node;
> +	struct device_node *np_codec;
> +	struct snd_soc_dai_link *dai;
> +	bool has_rt5514 = false;
> +	int i, index, ret;
> +
> +	card->dai_link = devm_kzalloc(dev, sizeof(rockchip_dais),
> +				      GFP_KERNEL);
> +	if (!card->dai_link)
> +		return -ENOMEM;
> +
> +	cpu_node = of_parse_phandle(dev->of_node, "rockchip,cpu", 0);
> +
> +	card->num_links = 0;
> +	for (i = 0; i < DAILINK_RT5514_DSP; i++) {
> +		index = of_property_match_string(dev->of_node,
> +				"rockchip,codec-names",
> +				rockchip_dais[i].name);
> +		if (index < 0)
> +			continue;
> +
> +		np_codec = of_parse_phandle(dev->of_node,
> +					    "rockchip,codec", index);
> +		if (!np_codec) {
> +			dev_err(dev, "Missing 'rockchip,codec' for %s\n",
> +				rockchip_dais[i].name);
> +			return -EINVAL;
> +		}
> +		if (!of_device_is_available(np_codec))
> +			continue;
>  
> -		rockchip_dailinks[i].codec_of_node =
> -			of_parse_phandle(pdev->dev.of_node, "rockchip,codec", i);
> -		if (!rockchip_dailinks[i].codec_of_node) {
> -			dev_err(&pdev->dev,
> -				"Property[%d] 'rockchip,codec' missing or invalid\n", i);
> +		if (!cpu_node) {
> +			dev_err(dev, "Missing 'rockchip,cpu' for %s\n",
> +				rockchip_dais[i].name);
>  			return -EINVAL;
>  		}
> +
> +		dai = &card->dai_link[card->num_links++];
> +		*dai = rockchip_dais[i];
> +
> +		dai->codec_of_node = np_codec;
> +		dai->platform_of_node = cpu_node;
> +		dai->cpu_of_node = cpu_node;
> +
> +		if (i == DAILINK_RT5514)
> +			has_rt5514 = true;
>  	}
>  
> +	if (!has_rt5514)
> +		return 0;
> +
>  	/**
>  	 * To acquire the spi driver of the rt5514 and set the dai-links names
>  	 * for soc_bind_dai_link
>  	 */
> -	drv = driver_find("rt5514", &spi_bus_type);
> -	if (!drv) {
> -		dev_err(&pdev->dev, "Can not find the rt5514 driver at the spi bus\n");
> +	rt5514_drv = driver_find("rt5514", &spi_bus_type);
> +	if (!rt5514_drv) {
> +		dev_err(dev, "Can not find the rt5514 driver at the spi bus\n");
>  		return -EINVAL;
>  	}
>  
> -	dev = driver_find_device(drv, NULL, NULL, rockchip_sound_match_stub);
> -	if (!dev) {
> -		dev_err(&pdev->dev, "Can not find the rt5514 device\n");
> +	rt5514_dev = driver_find_device(rt5514_drv, NULL, NULL,
> +					rockchip_sound_match_stub);
> +	if (!rt5514_dev) {
> +		dev_err(dev, "Can not find the rt5514 device\n");
>  		return -ENODEV;
>  	}
>  
>  	/* Set DMIC delay */
> -	ret = device_property_read_u32(&pdev->dev, "dmic-delay",
> -					&rt5514_dmic_delay);
> -	if (ret) {
> +	ret = device_property_read_u32(dev, "dmic-delay", &rt5514_dmic_delay);
> +	if (ret < 0) {
>  		rt5514_dmic_delay = 0;
> -		dev_dbg(&pdev->dev,
> +		dev_dbg(dev,
>  			"no optional property 'dmic-delay' found, default: no delay\n");
>  	}
>  
> -	rockchip_dailinks[DAILINK_RT5514_DSP].cpu_name = kstrdup_const(dev_name(dev), GFP_KERNEL);
> -	rockchip_dailinks[DAILINK_RT5514_DSP].cpu_dai_name = kstrdup_const(dev_name(dev), GFP_KERNEL);
> -	rockchip_dailinks[DAILINK_RT5514_DSP].platform_name = kstrdup_const(dev_name(dev), GFP_KERNEL);
> +	dai = &card->dai_link[card->num_links++];
> +	*dai = rockchip_dais[DAILINK_RT5514_DSP];
> +
> +	dai->cpu_name = devm_kstrdup(dev, dev_name(rt5514_dev), GFP_KERNEL);
> +	dai->cpu_dai_name = dai->cpu_name;
> +	dai->platform_name = dai->cpu_name;
> +
> +	return 0;
> +}
> +
> +static int rockchip_sound_probe(struct platform_device *pdev)
> +{
> +	struct snd_soc_card *card = &rockchip_sound_card;
> +	int ret;
> +
> +	ret = rockchip_sound_of_parse_dais(&pdev->dev, card);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Failed to parse dais: %d\n", ret);
> +		return ret;
> +	}
>  
>  	card->dev = &pdev->dev;
>  	platform_set_drvdata(pdev, card);
>  
> -	ret = devm_snd_soc_register_card(&pdev->dev, card);
> -	if (ret)
> -		dev_err(&pdev->dev, "%s snd_soc_register_card fail %d\n",
> -			__func__, ret);
> -
> -	return ret;
> +	return devm_snd_soc_register_card(&pdev->dev, card);
>  }
>  
>  static const struct of_device_id rockchip_sound_of_match[] = {

Re: [PATCH v2 1/3] ASoC: rockchip: Parse dai links from dts

[jeffy]

hi matthias,

thanks for your suggestion.

On 08/17/2017 05:59 AM, Matthias Kaehlcke wrote:
> El Thu, Aug 10, 2017 at 12:54:56PM +0800 Jeffy Chen ha dit:
>
>> >Refactor rockchip_sound_probe, parse dai links from dts instead of
>> >hard coding them.
> Mark doesn't seem to be overly convinced that 'rockchip,codec-names'
> is a good idea (https://lkml.org/lkml/2017/8/10/511).
>
> How about using something like this instead:
>
> static const char *dailink_compat[] = {
> 	[DAILINK_MAX98357A] = "maxim,max98357a",
> 	[DAILINK_RT5514] = "realtek,rt5514",
> 	[DAILINK_DA7219] = "dlg,da7219",
> };
i've thought about this too, but i'm working on converting rt5514 
dsp(spi) from codec name matching to of_node, and it would have the same 
compatible with rt5514(i2c)

Re: [PATCH v2 1/3] ASoC: rockchip: Parse dai links from dts

[Matthias Kaehlcke]

El Thu, Aug 17, 2017 at 06:55:20AM +0800 jeffy ha dit:

> hi matthias,
> 
> thanks for your suggestion.
> 
> On 08/17/2017 05:59 AM, Matthias Kaehlcke wrote:
> >El Thu, Aug 10, 2017 at 12:54:56PM +0800 Jeffy Chen ha dit:
> >
> >>>Refactor rockchip_sound_probe, parse dai links from dts instead of
> >>>hard coding them.
> >Mark doesn't seem to be overly convinced that 'rockchip,codec-names'
> >is a good idea (https://lkml.org/lkml/2017/8/10/511).
> >
> >How about using something like this instead:
> >
> >static const char *dailink_compat[] = {
> >	[DAILINK_MAX98357A] = "maxim,max98357a",
> >	[DAILINK_RT5514] = "realtek,rt5514",
> >	[DAILINK_DA7219] = "dlg,da7219",
> >};
> i've thought about this too, but i'm working on converting rt5514
> dsp(spi) from codec name matching to of_node, and it would have the
> same compatible with rt5514(i2c)

Bummer!

I wonder if a relatively inoffensive DT hack would be an appropriate
solution in this case, since the conflicting compatible string is a
somewhat special case and this change only affects the DT and the
driver/glue of a specific device (family).

The hack would consist in adding an additional 'compatible' entry to
the DT entry of the codec, which is ignored by the rt5514 driver, and
only used by the sound glue to identify it:

--- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
@@ -671,7 +671,7 @@ ap_i2c_mic: &i2c1 {
        i2c-scl-rising-time-ns = <300>;
 
        headsetcodec: rt5514@57 {
-               compatible = "realtek,rt5514";
+               compatible = "realtek,rt5514", "realtek,rt5514-i2c";


And then use "realtek,rt5514-i2c" in dailink_compat.

Mark, would you prefer a hack like this over the list of codec names
or do you have any other suggestions?

Matthias

Re: [PATCH v2 1/3] ASoC: rockchip: Parse dai links from dts

[jeffy]

Hi Matthias,

On 08/17/2017 07:50 AM, Matthias Kaehlcke wrote:
> El Thu, Aug 17, 2017 at 06:55:20AM +0800 jeffy ha dit:
>
>> hi matthias,
>>
>> thanks for your suggestion.
>>
>> On 08/17/2017 05:59 AM, Matthias Kaehlcke wrote:
>>> El Thu, Aug 10, 2017 at 12:54:56PM +0800 Jeffy Chen ha dit:
>>>
>>>>> Refactor rockchip_sound_probe, parse dai links from dts instead of
>>>>> hard coding them.
>>> Mark doesn't seem to be overly convinced that 'rockchip,codec-names'
>>> is a good idea (https://lkml.org/lkml/2017/8/10/511).
>>>
>>> How about using something like this instead:
>>>
>>> static const char *dailink_compat[] = {
>>> 	[DAILINK_MAX98357A] = "maxim,max98357a",
>>> 	[DAILINK_RT5514] = "realtek,rt5514",
>>> 	[DAILINK_DA7219] = "dlg,da7219",
>>> };
>> i've thought about this too, but i'm working on converting rt5514
>> dsp(spi) from codec name matching to of_node, and it would have the
>> same compatible with rt5514(i2c)
>
> Bummer!
>
> I wonder if a relatively inoffensive DT hack would be an appropriate
> solution in this case, since the conflicting compatible string is a
> somewhat special case and this change only affects the DT and the
> driver/glue of a specific device (family).
>
> The hack would consist in adding an additional 'compatible' entry to
> the DT entry of the codec, which is ignored by the rt5514 driver, and
> only used by the sound glue to identify it:
>
> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> @@ -671,7 +671,7 @@ ap_i2c_mic: &i2c1 {
>          i2c-scl-rising-time-ns = <300>;
>
>          headsetcodec: rt5514@57 {
> -               compatible = "realtek,rt5514";
> +               compatible = "realtek,rt5514", "realtek,rt5514-i2c";
>
>
> And then use "realtek,rt5514-i2c" in dailink_compat.
this should work, i'll do that in new version, thanks.

>
> Mark, would you prefer a hack like this over the list of codec names
> or do you have any other suggestions?
>
> Matthias
>
>
>

Re: [PATCH v2 3/3] dt-bindings: ASoC: rockchip: Add rockchip,codec-names property

[Rob Herring]

On Thu, Aug 10, 2017 at 12:54:58PM +0800, Jeffy Chen wrote:
> Add a new rockchip,codec-names property, so that the driver can parse
> the codecs by name.
> 
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> ---
> 
> Changes in v2:
> Let rockchip,codec-names be a required property.
> 
>  Documentation/devicetree/bindings/sound/rockchip,rk3399-gru-sound.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/rockchip,rk3399-gru-sound.txt b/Documentation/devicetree/bindings/sound/rockchip,rk3399-gru-sound.txt
> index eac91db..05351df 100644
> --- a/Documentation/devicetree/bindings/sound/rockchip,rk3399-gru-sound.txt
> +++ b/Documentation/devicetree/bindings/sound/rockchip,rk3399-gru-sound.txt
> @@ -5,6 +5,7 @@ Required properties:
>  - rockchip,cpu: The phandle of the Rockchip I2S controller that's
>    connected to the codecs
>  - rockchip,codec: The phandle of the MAX98357A/RT5514/DA7219 codecs
> +- rockchip,codec-names: The names of the MAX98357A/RT5514/DA7219 codecs

No, just lookup the handle and get the compatible for the codec if you 
need a name.

>  
>  Optional properties:
>  - dmic-wakeup-delay-ms : specify delay time (ms) for DMIC ready.
> @@ -18,5 +19,6 @@ sound {
>  	compatible = "rockchip,rk3399-gru-sound";
>  	rockchip,cpu = <&i2s0>;
>  	rockchip,codec = <&max98357a &rt5514 &da7219>;
> +	rockchip,codec-names = "MAX98357A", "RT5514", "DA7219";
>  	dmic-wakeup-delay-ms = <20>;
>  };
> -- 
> 2.1.4
> 
>