[PATCH v3 0/4] Add MT8186 ADSP dt-binding

[PATCH v3 0/4] Add MT8186 ADSP dt-binding

[Tinghan Shen]

v2 -> v3:
1. Change mbox-names to rx/tx for both mt8186 and mt8195.
2. Update description of mbox-names
3. Use static string array instead of kasprintf
4. Align clock names in dsp driver with dt-bindings

v1 -> v2:
1. Change mbox-names from mbox0/mbox1 to rep/req for both mt8186 and mt8195.
2. rename clock-names and remove unused headers

Tinghan Shen (4):
  dt-bindings: dsp: mediatek: Use meaningful names for mbox
  firmware: mediatek: Use meaningful names for mbox
  dt-bindings: dsp: mediatek: Add mt8186 dsp document
  ASoC: SOF: mediatek: Align mt8186 clock names with dt-bindings

 .../bindings/dsp/mediatek,mt8186-dsp.yaml     | 91 +++++++++++++++++++
 .../bindings/dsp/mediatek,mt8195-dsp.yaml     | 10 +-
 drivers/firmware/mtk-adsp-ipc.c               | 36 +++-----
 sound/soc/sof/mediatek/mt8186/mt8186-clk.c    |  4 +-
 4 files changed, 110 insertions(+), 31 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/dsp/mediatek,mt8186-dsp.yaml

-- 
2.18.0

[PATCH v3 1/4] dt-bindings: dsp: mediatek: Use meaningful names for mbox

[Tinghan Shen]

Rename mbox according to actions instead of 'mbox0' and 'mbox1'.

Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 .../devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml   | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml b/Documentation/devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml
index b7e68b0dfa13..ca8d8661f872 100644
--- a/Documentation/devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml
+++ b/Documentation/devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml
@@ -50,13 +50,13 @@ properties:
 
   mboxes:
     items:
-      - description: ipc reply between host and audio DSP.
-      - description: ipc request between host and audio DSP.
+      - description: mailbox for receiving audio DSP requests.
+      - description: mailbox for transmitting requests to audio DSP.
 
   mbox-names:
     items:
-      - const: mbox0
-      - const: mbox1
+      - const: rx
+      - const: tx
 
   memory-region:
     items:
@@ -100,6 +100,6 @@ examples:
        memory-region = <&adsp_dma_mem_reserved>,
                        <&adsp_mem_reserved>;
        power-domains = <&spm 6>; //MT8195_POWER_DOMAIN_ADSP
-       mbox-names = "mbox0", "mbox1";
+       mbox-names = "rx", "tx";
        mboxes = <&adsp_mailbox0>, <&adsp_mailbox1>;
     };
-- 
2.18.0

[PATCH v3 2/4] firmware: mediatek: Use meaningful names for mbox

[Tinghan Shen]

Rename mbox according to actions instead of 'mbox0' and 'mbox1'

Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 drivers/firmware/mtk-adsp-ipc.c | 36 +++++++++++----------------------
 1 file changed, 12 insertions(+), 24 deletions(-)

diff --git a/drivers/firmware/mtk-adsp-ipc.c b/drivers/firmware/mtk-adsp-ipc.c
index cb255a99170c..3c071f814455 100644
--- a/drivers/firmware/mtk-adsp-ipc.c
+++ b/drivers/firmware/mtk-adsp-ipc.c
@@ -12,6 +12,8 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 
+static const char * const adsp_mbox_ch_names[MTK_ADSP_MBOX_NUM] = { "rx", "tx" };
+
 /*
  * mtk_adsp_ipc_send - send ipc cmd to MTK ADSP
  *
@@ -72,7 +74,6 @@ static int mtk_adsp_ipc_probe(struct platform_device *pdev)
 	struct mtk_adsp_ipc *adsp_ipc;
 	struct mtk_adsp_chan *adsp_chan;
 	struct mbox_client *cl;
-	char *chan_name;
 	int ret;
 	int i, j;
 
@@ -83,12 +84,6 @@ static int mtk_adsp_ipc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	for (i = 0; i < MTK_ADSP_MBOX_NUM; i++) {
-		chan_name = kasprintf(GFP_KERNEL, "mbox%d", i);
-		if (!chan_name) {
-			ret = -ENOMEM;
-			goto out;
-		}
-
 		adsp_chan = &adsp_ipc->chans[i];
 		cl = &adsp_chan->cl;
 		cl->dev = dev->parent;
@@ -99,17 +94,20 @@ static int mtk_adsp_ipc_probe(struct platform_device *pdev)
 
 		adsp_chan->ipc = adsp_ipc;
 		adsp_chan->idx = i;
-		adsp_chan->ch = mbox_request_channel_byname(cl, chan_name);
+		adsp_chan->ch = mbox_request_channel_byname(cl, adsp_mbox_ch_names[i]);
 		if (IS_ERR(adsp_chan->ch)) {
 			ret = PTR_ERR(adsp_chan->ch);
 			if (ret != -EPROBE_DEFER)
-				dev_err(dev, "Failed to request mbox chan %d ret %d\n",
-					i, ret);
-			goto out_free;
-		}
+				dev_err(dev, "Failed to request mbox chan %s ret %d\n",
+					adsp_mbox_ch_names[i], ret);
+
+			for (j = 0; j < i; j++) {
+				adsp_chan = &adsp_ipc->chans[j];
+				mbox_free_channel(adsp_chan->ch);
+			}
 
-		dev_dbg(dev, "request mbox chan %s\n", chan_name);
-		kfree(chan_name);
+			return ret;
+		}
 	}
 
 	adsp_ipc->dev = dev;
@@ -117,16 +115,6 @@ static int mtk_adsp_ipc_probe(struct platform_device *pdev)
 	dev_dbg(dev, "MTK ADSP IPC initialized\n");
 
 	return 0;
-
-out_free:
-	kfree(chan_name);
-out:
-	for (j = 0; j < i; j++) {
-		adsp_chan = &adsp_ipc->chans[j];
-		mbox_free_channel(adsp_chan->ch);
-	}
-
-	return ret;
 }
 
 static int mtk_adsp_ipc_remove(struct platform_device *pdev)
-- 
2.18.0

[PATCH v3 3/4] dt-bindings: dsp: mediatek: Add mt8186 dsp document

[Tinghan Shen]

This patch adds mt8186 dsp document. The dsp is used for Sound Open
Firmware driver node. It includes registers, clocks, memory regions,
and mailbox for dsp.

Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 .../bindings/dsp/mediatek,mt8186-dsp.yaml     | 91 +++++++++++++++++++
 1 file changed, 91 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dsp/mediatek,mt8186-dsp.yaml

diff --git a/Documentation/devicetree/bindings/dsp/mediatek,mt8186-dsp.yaml b/Documentation/devicetree/bindings/dsp/mediatek,mt8186-dsp.yaml
new file mode 100644
index 000000000000..3e63f79890b4
--- /dev/null
+++ b/Documentation/devicetree/bindings/dsp/mediatek,mt8186-dsp.yaml
@@ -0,0 +1,91 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dsp/mediatek,mt8186-dsp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek mt8186 DSP core
+
+maintainers:
+  - Tinghan Shen <tinghan.shen@mediatek.com>
+
+description: |
+  MediaTek mt8186 SoC contains a DSP core used for
+  advanced pre- and post- audio processing.
+
+properties:
+  compatible:
+    const: mediatek,mt8186-dsp
+
+  reg:
+    items:
+      - description: Address and size of the DSP config registers
+      - description: Address and size of the DSP SRAM
+      - description: Address and size of the DSP secure registers
+      - description: Address and size of the DSP bus registers
+
+  reg-names:
+    items:
+      - const: cfg
+      - const: sram
+      - const: sec
+      - const: bus
+
+  clocks:
+    items:
+      - description: mux for audio dsp clock
+      - description: mux for audio dsp local bus
+
+  clock-names:
+    items:
+      - const: audiodsp
+      - const: adsp_bus
+
+  power-domains:
+    maxItems: 1
+
+  mboxes:
+    items:
+      - description: mailbox for receiving audio DSP requests.
+      - description: mailbox for transmitting requests to audio DSP.
+
+  mbox-names:
+    items:
+      - const: rx
+      - const: tx
+
+  memory-region:
+    items:
+      - description: dma buffer between host and DSP.
+      - description: DSP system memory.
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - clocks
+  - clock-names
+  - power-domains
+  - mbox-names
+  - mboxes
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/mt8186-clk.h>
+    dsp@10680000 {
+        compatible = "mediatek,mt8186-dsp";
+        reg = <0x10680000 0x2000>,
+              <0x10800000 0x100000>,
+              <0x1068b000 0x100>,
+              <0x1068f000 0x1000>;
+        reg-names = "cfg", "sram", "sec", "bus";
+        clocks = <&topckgen CLK_TOP_AUDIODSP>,
+                 <&topckgen CLK_TOP_ADSP_BUS>;
+        clock-names = "audiodsp",
+                      "adsp_bus";
+        power-domains = <&spm 6>;
+        mbox-names = "rx", "tx";
+        mboxes = <&adsp_mailbox0>, <&adsp_mailbox1>;
+    };
-- 
2.18.0

[PATCH v3 4/4] ASoC: SOF: mediatek: Align mt8186 clock names with dt-bindings

[Tinghan Shen]

Align clock names in mt8186 dsp driver with dt-bindings.

Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
 sound/soc/sof/mediatek/mt8186/mt8186-clk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sof/mediatek/mt8186/mt8186-clk.c b/sound/soc/sof/mediatek/mt8186/mt8186-clk.c
index 22220fd50b62..2df3b7ae1c6f 100644
--- a/sound/soc/sof/mediatek/mt8186/mt8186-clk.c
+++ b/sound/soc/sof/mediatek/mt8186/mt8186-clk.c
@@ -18,8 +18,8 @@
 #include "mt8186-clk.h"
 
 static const char *adsp_clks[ADSP_CLK_MAX] = {
-	[CLK_TOP_AUDIODSP] = "audiodsp_sel",
-	[CLK_TOP_ADSP_BUS] = "adsp_bus_sel",
+	[CLK_TOP_AUDIODSP] = "audiodsp",
+	[CLK_TOP_ADSP_BUS] = "adsp_bus",
 };
 
 int mt8186_adsp_init_clock(struct snd_sof_dev *sdev)
-- 
2.18.0

Re: [PATCH v3 1/4] dt-bindings: dsp: mediatek: Use meaningful names for mbox

[Krzysztof Kozlowski]

On 16/06/2022 00:30, Tinghan Shen wrote:
> Rename mbox according to actions instead of 'mbox0' and 'mbox1'.
> 
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> ---
>  .../devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml   | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml b/Documentation/devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml
> index b7e68b0dfa13..ca8d8661f872 100644
> --- a/Documentation/devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml
> +++ b/Documentation/devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml
> @@ -50,13 +50,13 @@ properties:
>  
>    mboxes:
>      items:
> -      - description: ipc reply between host and audio DSP.
> -      - description: ipc request between host and audio DSP.
> +      - description: mailbox for receiving audio DSP requests.
> +      - description: mailbox for transmitting requests to audio DSP.
>  
>    mbox-names:
>      items:
> -      - const: mbox0
> -      - const: mbox1
> +      - const: rx
> +      - const: tx
>  

Commit msg lacks important piece - do you break any users with this? Do
you have any users of this binding?


Best regards,
Krzysztof

Re: [PATCH v3 3/4] dt-bindings: dsp: mediatek: Add mt8186 dsp document

[Krzysztof Kozlowski]

On 16/06/2022 00:30, Tinghan Shen wrote:
> This patch adds mt8186 dsp document. The dsp is used for Sound Open
> Firmware driver node. It includes registers, clocks, memory regions,
> and mailbox for dsp.
> 
> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH v3 1/4] dt-bindings: dsp: mediatek: Use meaningful names for mbox

[Tinghan Shen]

Hi Krzysztof,

On Thu, 2022-06-16 at 06:55 -0700, Krzysztof Kozlowski wrote:
> On 16/06/2022 00:30, Tinghan Shen wrote:
> > Rename mbox according to actions instead of 'mbox0' and 'mbox1'.
> > 
> > Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> > ---
> >  .../devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml   | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml
> > b/Documentation/devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml
> > index b7e68b0dfa13..ca8d8661f872 100644
> > --- a/Documentation/devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml
> > +++ b/Documentation/devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml
> > @@ -50,13 +50,13 @@ properties:
> >  
> >    mboxes:
> >      items:
> > -      - description: ipc reply between host and audio DSP.
> > -      - description: ipc request between host and audio DSP.
> > +      - description: mailbox for receiving audio DSP requests.
> > +      - description: mailbox for transmitting requests to audio DSP.
> >  
> >    mbox-names:
> >      items:
> > -      - const: mbox0
> > -      - const: mbox1
> > +      - const: rx
> > +      - const: tx
> >  
> 
> Commit msg lacks important piece - do you break any users with this? Do
> you have any users of this binding?
> 
> 
> Best regards,
> Krzysztof

The 8195 dsp node, which uses this binding, has not yet been added to the 8195 devicetree.


Thanks,
TingHan

Re: [PATCH v3 1/4] dt-bindings: dsp: mediatek: Use meaningful names for mbox

[Krzysztof Kozlowski]

On 20/06/2022 08:40, Tinghan Shen wrote:
> Hi Krzysztof,
> 
> On Thu, 2022-06-16 at 06:55 -0700, Krzysztof Kozlowski wrote:
>> On 16/06/2022 00:30, Tinghan Shen wrote:
>>> Rename mbox according to actions instead of 'mbox0' and 'mbox1'.
>>>
>>> Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
>>> ---
>>>  .../devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml   | 10 +++++-----
>>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml
>>> b/Documentation/devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml
>>> index b7e68b0dfa13..ca8d8661f872 100644
>>> --- a/Documentation/devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml
>>> +++ b/Documentation/devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml
>>> @@ -50,13 +50,13 @@ properties:
>>>  
>>>    mboxes:
>>>      items:
>>> -      - description: ipc reply between host and audio DSP.
>>> -      - description: ipc request between host and audio DSP.
>>> +      - description: mailbox for receiving audio DSP requests.
>>> +      - description: mailbox for transmitting requests to audio DSP.
>>>  
>>>    mbox-names:
>>>      items:
>>> -      - const: mbox0
>>> -      - const: mbox1
>>> +      - const: rx
>>> +      - const: tx
>>>  
>>
>> Commit msg lacks important piece - do you break any users with this? Do
>> you have any users of this binding?
>>
>>
>> Best regards,
>> Krzysztof
> 
> The 8195 dsp node, which uses this binding, has not yet been added to the 8195 devicetree.

Could you mention it in the commit msg? Such one sentence is enough.

With commit msg extended:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH v3 1/4] dt-bindings: dsp: mediatek: Use meaningful names for mbox

[Tinghan Shen]

Hi Krzysztof,

On Mon, 2022-06-20 at 10:51 +0200, Krzysztof Kozlowski wrote:
> On 20/06/2022 08:40, Tinghan Shen wrote:
> > Hi Krzysztof,
> > 
> > On Thu, 2022-06-16 at 06:55 -0700, Krzysztof Kozlowski wrote:
> > > On 16/06/2022 00:30, Tinghan Shen wrote:
> > > > Rename mbox according to actions instead of 'mbox0' and 'mbox1'.
> > > > 
> > > > Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
> > > > ---
> > > >  .../devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml   | 10 +++++-----
> > > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml
> > > > b/Documentation/devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml
> > > > index b7e68b0dfa13..ca8d8661f872 100644
> > > > --- a/Documentation/devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml
> > > > +++ b/Documentation/devicetree/bindings/dsp/mediatek,mt8195-dsp.yaml
> > > > @@ -50,13 +50,13 @@ properties:
> > > >  
> > > >    mboxes:
> > > >      items:
> > > > -      - description: ipc reply between host and audio DSP.
> > > > -      - description: ipc request between host and audio DSP.
> > > > +      - description: mailbox for receiving audio DSP requests.
> > > > +      - description: mailbox for transmitting requests to audio DSP.
> > > >  
> > > >    mbox-names:
> > > >      items:
> > > > -      - const: mbox0
> > > > -      - const: mbox1
> > > > +      - const: rx
> > > > +      - const: tx
> > > >  
> > > 
> > > Commit msg lacks important piece - do you break any users with this? Do
> > > you have any users of this binding?
> > > 
> > > 
> > > Best regards,
> > > Krzysztof
> > 
> > The 8195 dsp node, which uses this binding, has not yet been added to the 8195 devicetree.
> 
> Could you mention it in the commit msg? Such one sentence is enough.
> 
> With commit msg extended:
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> 
> Best regards,
> Krzysztof

No problem. I'll udpate commit message at next version.


Thanks,
TingHan