Re: [PATCH v1 05/13] ASoC: qcom: support bitclk and osrclk per i2s port

From: Srinivas Kandagatla
Date: Fri May 15 2015 - 04:44:44 EST

Next message: Sylwester Nawrocki: "Re: [PATCH v4] clk: Show correct information when fail to set clock rate"
Previous message: Wangnan (F): "Re: [RFC PATCH v2 00/37] perf tools: introduce 'perf bpf' command to load eBPF programs."
In reply to: Kenneth Westfield: "Re: [PATCH v1 05/13] ASoC: qcom: support bitclk and osrclk per i2s port"
Next in thread: Srinivas Kandagatla: "[PATCH v1 06/13] ASoC: qcom: make osr clock optional"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 15/05/15 06:23, Kenneth Westfield wrote:

On Wed, May 13, 2015 at 05:00:52AM -0700, Srinivas Kandagatla wrote:

This patch adds support to allow bitclk and osrclk per i2s dai port.
on APQ8016 there are 4 i2s ports each one has its own bit clks.

Without this patch its not possible to support multiple i2s ports in the
lpass driver.

diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
index 5965667..0d28ea7 100644
--- a/sound/soc/qcom/lpass-cpu.c
+++ b/sound/soc/qcom/lpass-cpu.c
@@ -33,7 +33,7 @@ static int lpass_cpu_daiops_set_sysclk(struct
snd_soc_dai *dai, int clk_id,
struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
int ret;

- ret = clk_set_rate(drvdata->mi2s_osr_clk, freq);
+ ret = clk_set_rate(drvdata->mi2s_osr_clk[dai->driver->id], freq);
if (ret)
dev_err(dai->dev, "%s() error setting mi2s osrclk to %u:
%d\n",
__func__, freq, ret);

Audio was broken on the Storm board with this patch series. The issue
has to do with the mismatch of the clock position in the array (which
was 0) and the dai->driver->id (which was 4). Basically, the position of
the bit/osr clocks in their respective arrays need to match the MI2S
port number, even if the port number doesn't start at the 0 position.

I realize there are multiple ways to address this. The quick solution I
came up with (to get audio functioning again) was to change the DT clock
entries for the ipq806x (see changes below for your reference). The
downside to the way I did this is, that now, there is no error-checking
for clocks that should be in the DT but aren't there.

Suggestions are welcome on how to best address this issue.

I think all we need to do is populate the correct index in the clk array. TBH I don't want to change any of ipq806x bindings.

I think the below fix should be the right one, actually I did think of doing this in v1 when I changed usage of dai->id to dai->driver->id , but some how I missed it.

Let me know your thoughts.
-----------------------><---------------------------------------------
diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
index 33e28370..64620ce 100644
--- a/sound/soc/qcom/lpass-cpu.c
+++ b/sound/soc/qcom/lpass-cpu.c
@@ -365,7 +365,7 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
const struct of_device_id *match;
char clk_name[16];
- int ret, i;
+ int ret, i, dai_id;

dsp_of_node = of_parse_phandle(pdev->dev.of_node, "qcom,adsp", 0);
if (dsp_of_node) {
@@ -412,34 +412,36 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
variant->init(pdev);

for (i = 0; i < variant->num_dai; i++) {
+ dai_id = variant->dai_driver[i].id;
if (variant->num_dai > 1)
sprintf(clk_name, "mi2s-osr-clk%d", i);
else
sprintf(clk_name, "mi2s-osr-clk");

- drvdata->mi2s_osr_clk[i] = devm_clk_get(&pdev->dev,
+ drvdata->mi2s_osr_clk[dai_id] = devm_clk_get(&pdev->dev,
clk_name);
- if (IS_ERR(drvdata->mi2s_osr_clk[i])) {
+ if (IS_ERR(drvdata->mi2s_osr_clk[dai_id])) {
dev_err(&pdev->dev,
"%s() error getting mi2s-osr-clk: %ld\n",
__func__,
- PTR_ERR(drvdata->mi2s_osr_clk[i]));
+ PTR_ERR(drvdata->mi2s_osr_clk[dai_id]));
}
}

for (i = 0; i < variant->num_dai; i++) {
+ dai_id = variant->dai_driver[i].id;

if (variant->num_dai > 1)
sprintf(clk_name, "mi2s-bit-clk%d", i);
else
sprintf(clk_name, "mi2s-bit-clk");

- drvdata->mi2s_bit_clk[i] = devm_clk_get(&pdev->dev, clk_name);
- if (IS_ERR(drvdata->mi2s_bit_clk[i])) {
+ drvdata->mi2s_bit_clk[dai_id] = devm_clk_get(&pdev->dev, clk_name);
+ if (IS_ERR(drvdata->mi2s_bit_clk[dai_id])) {
dev_err(&pdev->dev,
"%s() error getting mi2s-bit-clk: %ld\n",
__func__, PTR_ERR(drvdata->mi2s_bit_clk[i]));
- return PTR_ERR(drvdata->mi2s_bit_clk[i]);
+ return PTR_ERR(drvdata->mi2s_bit_clk[dai_id]);
}
}

-----------------------><---------------------------------------------

--srini

-----------------------><---------------------------------------------
diff --git a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
index 21c6483..2684a4f 100644
--- a/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
+++ b/Documentation/devicetree/bindings/sound/qcom,lpass-cpu.txt
@@ -8,8 +8,8 @@ Required properties:
- clocks : Must contain an entry for each entry in clock-names.
- clock-names : A list which must include the following entries:
* "ahbix-clk"
- * "mi2s-osr-clk"
- * "mi2s-bit-clk"
+ * "mi2s-osr-clk4"
+ * "mi2s-bit-clk4"
: required clocks for "qcom,lpass-cpu-apq8016"
* "ahbix-clk"
* "mi2s-bit-clk0"
@@ -42,7 +42,7 @@ Example:
lpass@28100000 {
compatible = "qcom,lpass-cpu";
clocks = <&lcc AHBIX_CLK>, <&lcc MI2S_OSR_CLK>, <&lcc MI2S_BIT_CLK>;
- clock-names = "ahbix-clk", "mi2s-osr-clk", "mi2s-bit-clk";
+ clock-names = "ahbix-clk", "mi2s-osr-clk4", "mi2s-bit-clk4";
interrupts = <0 85 1>;
interrupt-names = "lpass-irq-lpaif";
pinctrl-names = "default", "idle";
diff --git a/arch/arm/boot/dts/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom-ipq8064.dtsi
index 5a13366..090984f 100644
--- a/arch/arm/boot/dts/qcom-ipq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-ipq8064.dtsi
@@ -189,8 +189,8 @@
<&lcc MI2S_OSR_CLK>,
<&lcc MI2S_BIT_CLK>;
clock-names = "ahbix-clk",
- "mi2s-osr-clk",
- "mi2s-bit-clk";
+ "mi2s-osr-clk4",
+ "mi2s-bit-clk4";
interrupts = <0 85 1>;
interrupt-names = "lpass-irq-lpaif";
reg = <0x28100000 0x10000>;
diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
index 5053629..7b66e52 100644
--- a/sound/soc/qcom/lpass-cpu.c
+++ b/sound/soc/qcom/lpass-cpu.c
@@ -411,11 +411,8 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
if (variant->init)
variant->init(pdev);

- for (i = 0; i < variant->num_dai; i++) {
- if (variant->num_dai > 1)
- sprintf(clk_name, "mi2s-osr-clk%d", i);
- else
- sprintf(clk_name, "mi2s-osr-clk");
+ for (i = 0; i < LPASS_MAX_MI2S_PORTS; i++) {
+ sprintf(clk_name, "mi2s-osr-clk%d", i);

drvdata->mi2s_osr_clk[i] = devm_clk_get(&pdev->dev,
clk_name);
@@ -427,19 +424,14 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
}
}

- for (i = 0; i < variant->num_dai; i++) {
-
- if (variant->num_dai > 1)
- sprintf(clk_name, "mi2s-bit-clk%d", i);
- else
- sprintf(clk_name, "mi2s-bit-clk");
+ for (i = 0; i < LPASS_MAX_MI2S_PORTS; i++) {
+ sprintf(clk_name, "mi2s-bit-clk%d", i);

drvdata->mi2s_bit_clk[i] = devm_clk_get(&pdev->dev, clk_name);
if (IS_ERR(drvdata->mi2s_bit_clk[i])) {
dev_err(&pdev->dev,
"%s() error getting mi2s-bit-clk: %ld\n",
__func__, PTR_ERR(drvdata->mi2s_bit_clk[i]));
- return PTR_ERR(drvdata->mi2s_bit_clk[i]);
}
}
-----------------------><---------------------------------------------

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Sylwester Nawrocki: "Re: [PATCH v4] clk: Show correct information when fail to set clock rate"
Previous message: Wangnan (F): "Re: [RFC PATCH v2 00/37] perf tools: introduce 'perf bpf' command to load eBPF programs."
In reply to: Kenneth Westfield: "Re: [PATCH v1 05/13] ASoC: qcom: support bitclk and osrclk per i2s port"
Next in thread: Srinivas Kandagatla: "[PATCH v1 06/13] ASoC: qcom: make osr clock optional"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]