From: Hailong Fan <hailong.fan@mediatek.com>
This patch adds mt8196 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: Hailong Fan <hailong.fan@mediatek.com>
---
.../bindings/sound/mediatek,mt8196-dsp.yaml | 95 +++++++++++++++++++
1 file changed, 95 insertions(+)
create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml
diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml
new file mode 100644
index 000000000000..68f594f476e8
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml
@@ -0,0 +1,95 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/mediatek,mt8196-dsp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek mt8196 DSP core
+
+maintainers:
+ - Hailong Fan <hailong.fan@mediatek.com>
+
+description: The MediaTek mt8196 SoC mt8196 contain a DSP core used for advanced
+ pre- and post- audio processing.
+
+properties:
+ compatible:
+ const: mediatek,mt8196-dsp
+
+ reg:
+ items:
+ - description: DSP configuration registers
+ - description: DSP SRAM
+ - description: DSP secure registers
+ - description: DSP bus registers
+
+ reg-names:
+ items:
+ - const: cfg
+ - const: sram
+ - const: sec
+ - const: bus
+
+ clocks:
+ items:
+ - description: mux for dsp clock
+ - description: 26M clock
+ - description: ADSP PLL clock
+
+ clock-names:
+ items:
+ - const: adsp_sel
+ - const: clk26m
+ - const: adsppll
+
+ 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
+ - mboxes
+ - mbox-names
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/mt8196-clk.h>
+ #include <dt-bindings/power/mt8196-power.h>
+ adsp@1a000000 {
+ compatible = "mediatek,mt8196-dsp";
+ reg = <0x1a000000 0x5000>,
+ <0x1a210000 0x80000>,
+ <0x1a345000 0x300>,
+ <0x1a00f000 0x1000>;
+ reg-names = "cfg", "sram", "sec", "bus";
+ clocks = <&cksys_clk CLK_CK_ADSP_SEL>,
+ <&cksys_clk CLK_CK_TCK_26M_MX9>,
+ <&cksys_clk CLK_CK_ADSPPLL>;
+ clock-names = "adsp_sel",
+ "clk26m",
+ "adsppll";
+ power-domains = <&scpsys MT8196_POWER_DOMAIN_ADSP_TOP_DORMANT>;
+ mboxes = <&adsp_mailbox0>, <&adsp_mailbox1>;
+ mbox-names = "rx", "tx";
+ };
--
2.45.2
On Thu, 03 Jul 2025 15:56:23 +0800, hailong.fan wrote:
> From: Hailong Fan <hailong.fan@mediatek.com>
>
> This patch adds mt8196 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: Hailong Fan <hailong.fan@mediatek.com>
> ---
> .../bindings/sound/mediatek,mt8196-dsp.yaml | 95 +++++++++++++++++++
> 1 file changed, 95 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.example.dts:18:18: fatal error: dt-bindings/clock/mt8196-clk.h: No such file or directory
18 | #include <dt-bindings/clock/mt8196-clk.h>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [scripts/Makefile.dtbs:131: Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1525: dt_binding_check] Error 2
make: *** [Makefile:248: __sub-make] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250703075632.20758-2-hailong.fan@mediatek.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
On 03/07/2025 09:56, hailong.fan wrote: > From: Hailong Fan <hailong.fan@mediatek.com> > > This patch adds mt8196 dsp document. The dsp is used for Sound Open Please do not use "This commit/patch/change", but imperative mood. See longer explanation here: https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95 > Firmware driver node. It includes registers, clocks, memory regions, > and mailbox for dsp. > > Signed-off-by: Hailong Fan <hailong.fan@mediatek.com> > --- > .../bindings/sound/mediatek,mt8196-dsp.yaml | 95 +++++++++++++++++++ > 1 file changed, 95 insertions(+) > create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml > > diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml > new file mode 100644 > index 000000000000..68f594f476e8 > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml > @@ -0,0 +1,95 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/sound/mediatek,mt8196-dsp.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: MediaTek mt8196 DSP core > + > +maintainers: > + - Hailong Fan <hailong.fan@mediatek.com> > + > +description: The MediaTek mt8196 SoC mt8196 contain a DSP core used for advanced Missing blank line. Look at other bindings. Also does not look wrapped at 80 (see Linux coding style doc). > + pre- and post- audio processing. > + > +properties: > + compatible: > + const: mediatek,mt8196-dsp > + > + reg: > + items: > + - description: DSP configuration registers > + - description: DSP SRAM > + - description: DSP secure registers > + - description: DSP bus registers > + > + reg-names: > + items: > + - const: cfg > + - const: sram > + - const: sec > + - const: bus > + > + clocks: > + items: > + - description: mux for dsp clock > + - description: 26M clock > + - description: ADSP PLL clock > + > + clock-names: > + items: > + - const: adsp_sel Isn't this called audiodsp in other bindings? > + - const: clk26m Don't use frequencies. How is the pin or input called in datasheet? How other devices call it? > + - const: adsppll > + > + power-domains: > + maxItems: 1 > + Best regards, Krzysztof
On Thu, 2025-07-03 at 10:08 +0200, Krzysztof Kozlowski wrote: > External email : Please do not click links or open attachments until > you have verified the sender or the content. > > > On 03/07/2025 09:56, hailong.fan wrote: > > From: Hailong Fan <hailong.fan@mediatek.com> > > > > This patch adds mt8196 dsp document. The dsp is used for Sound Open > > Please do not use "This commit/patch/change", but imperative mood. > See > longer explanation here: > https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst*L95__;Iw!!CTRNKA9wMg0ARbw!mhCB8cQ9NY4AztdErJWay3VtFNHSe89i00TgGeTd62W6m-Ios46XeDaYqM0QQ7m4SHjlNw69PvU7T9M$ > Okay, will be updated to: Add device tree binding documentation for the MediaTek MT8196 DSP. The DSP is used by the Sound Open Firmware driver node and includes registers, clocks, memory regions, and a mailbox for DSP communication. Thanks. > > Firmware driver node. It includes registers, clocks, memory > > regions, > > and mailbox for dsp. > > > > Signed-off-by: Hailong Fan <hailong.fan@mediatek.com> > > --- > > .../bindings/sound/mediatek,mt8196-dsp.yaml | 95 > > +++++++++++++++++++ > > 1 file changed, 95 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml > > b/Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml > > new file mode 100644 > > index 000000000000..68f594f476e8 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8196- > > dsp.yaml > > @@ -0,0 +1,95 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: > > https://urldefense.com/v3/__http://devicetree.org/schemas/sound/mediatek,mt8196-dsp.yaml*__;Iw!!CTRNKA9wMg0ARbw!mhCB8cQ9NY4AztdErJWay3VtFNHSe89i00TgGeTd62W6m-Ios46XeDaYqM0QQ7m4SHjlNw697qH1t6A$ > > +$schema: > > https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!mhCB8cQ9NY4AztdErJWay3VtFNHSe89i00TgGeTd62W6m-Ios46XeDaYqM0QQ7m4SHjlNw69EdDHgNY$ > > + > > +title: MediaTek mt8196 DSP core > > + > > +maintainers: > > + - Hailong Fan <hailong.fan@mediatek.com> > > + > > +description: The MediaTek mt8196 SoC mt8196 contain a DSP core > > used for advanced > > Missing blank line. Look at other bindings. Also does not look > wrapped > at 80 (see Linux coding style doc). > > Okay, will be update to: description: | The MediaTek mt8196 SoC contains a DSP core used for advanced pre- and post-audio processing. This DSP is typically used by the Sound Open Firmware (SOF) driver and requires registers, clocks, memory regions, and a mailbox for communication. Th anks. > > + pre- and post- audio processing. > > + > > +properties: > > + compatible: > > + const: mediatek,mt8196-dsp > > + > > + reg: > > + items: > > + - description: DSP configuration registers > > + - description: DSP SRAM > > + - description: DSP secure registers > > + - description: DSP bus registers > > + > > + reg-names: > > + items: > > + - const: cfg > > + - const: sram > > + - const: sec > > + - const: bus > > + > > + clocks: > > + items: > > + - description: mux for dsp clock > > + - description: 26M clock > > + - description: ADSP PLL clock > > + > > + clock-names: > > + items: > > + - const: adsp_sel > > Isn't this called audiodsp in other bindings? > Yes, but essentially it is the mux selection of the ADSP clock. So, would using "adsp_sel" make it clearer? > > + - const: clk26m > > Don't use frequencies. How is the pin or input called in datasheet? > How > other devices call it? > > > + - const: adsppll > > + > > + power-domains: > > + maxItems: 1 > > + > > Best regards, > Krzysztof
On 09/07/2025 04:52, Hailong Fan (范海龙) wrote: >>> + clocks: >>> + items: >>> + - description: mux for dsp clock >>> + - description: 26M clock >>> + - description: ADSP PLL clock >>> + >>> + clock-names: >>> + items: >>> + - const: adsp_sel >> >> Isn't this called audiodsp in other bindings? >> > Yes, but essentially it is the mux selection of the ADSP clock. > So, would using "adsp_sel" make it clearer? > So every new binding will come with new name? Stick to the same name if this is the same clock. Best regards, Krzysztof
On Wed, 2025-07-09 at 08:55 +0200, Krzysztof Kozlowski wrote: > External email : Please do not click links or open attachments until > you have verified the sender or the content. > > > On 09/07/2025 04:52, Hailong Fan (范海龙) wrote: > > > > + clocks: > > > > + items: > > > > + - description: mux for dsp clock > > > > + - description: 26M clock > > > > + - description: ADSP PLL clock > > > > + > > > > + clock-names: > > > > + items: > > > > + - const: adsp_sel > > > > > > Isn't this called audiodsp in other bindings? > > > > > > > Yes, but essentially it is the mux selection of the ADSP clock. > > So, would using "adsp_sel" make it clearer? > > > > So every new binding will come with new name? Stick to the same name > if > this is the same clock. > > Best regards, > Krzysztof Got it, we will keep the name consistent, thanks a lot.
Il 03/07/25 10:08, Krzysztof Kozlowski ha scritto: > On 03/07/2025 09:56, hailong.fan wrote: >> From: Hailong Fan <hailong.fan@mediatek.com> >> >> This patch adds mt8196 dsp document. The dsp is used for Sound Open > > Please do not use "This commit/patch/change", but imperative mood. See > longer explanation here: > https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95 > >> Firmware driver node. It includes registers, clocks, memory regions, >> and mailbox for dsp. >> >> Signed-off-by: Hailong Fan <hailong.fan@mediatek.com> >> --- >> .../bindings/sound/mediatek,mt8196-dsp.yaml | 95 +++++++++++++++++++ >> 1 file changed, 95 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml >> >> diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml >> new file mode 100644 >> index 000000000000..68f594f476e8 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml Wrong folder; this should go to dsp/mediatek,mt8196-dsp.yaml .... ...but then I don't get why MT8196 wasn't simply added to mediatek,mt8186-dsp.yaml. >> @@ -0,0 +1,95 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/sound/mediatek,mt8196-dsp.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: MediaTek mt8196 DSP core >> + >> +maintainers: >> + - Hailong Fan <hailong.fan@mediatek.com> >> + >> +description: The MediaTek mt8196 SoC mt8196 contain a DSP core used for advanced > > Missing blank line. Look at other bindings. Also does not look wrapped > at 80 (see Linux coding style doc). > There's also a typo, "mt8196 SoC mt8196" > >> + pre- and post- audio processing. >> + >> +properties: >> + compatible: >> + const: mediatek,mt8196-dsp >> + >> + reg: >> + items: >> + - description: DSP configuration registers >> + - description: DSP SRAM >> + - description: DSP secure registers >> + - description: DSP bus registers >> + >> + reg-names: >> + items: >> + - const: cfg >> + - const: sram >> + - const: sec >> + - const: bus >> + >> + clocks: >> + items: >> + - description: mux for dsp clock >> + - description: 26M clock >> + - description: ADSP PLL clock >> + >> + clock-names: >> + items: >> + - const: adsp_sel > > Isn't this called audiodsp in other bindings? > >> + - const: clk26m > > Don't use frequencies. How is the pin or input called in datasheet? How > other devices call it? In the datasheet, this is called.... CLK26M (really). Cheers, Angelo > >> + - const: adsppll >> + >> + power-domains: >> + maxItems: 1 >> + > Best regards, > Krzysztof
On Thu, 2025-07-03 at 11:05 +0200, AngeloGioacchino Del Regno wrote: > External email : Please do not click links or open attachments until > you have verified the sender or the content. > > > Il 03/07/25 10:08, Krzysztof Kozlowski ha scritto: > > On 03/07/2025 09:56, hailong.fan wrote: > > > From: Hailong Fan <hailong.fan@mediatek.com> > > > > > > This patch adds mt8196 dsp document. The dsp is used for Sound > > > Open > > > > Please do not use "This commit/patch/change", but imperative mood. > > See > > longer explanation here: > > https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst*L95__;Iw!!CTRNKA9wMg0ARbw!g39q48fUCsDJ5PETUtQ0PK0Bkwf0Ce1F_ZzJiEiWMzuybWQ0dSPzblvLlJ-4saWTSoJWuS5imsXbpdgLBvUy4wxw9C5w7B4m$ > > > > > Firmware driver node. It includes registers, clocks, memory > > > regions, > > > and mailbox for dsp. > > > > > > Signed-off-by: Hailong Fan <hailong.fan@mediatek.com> > > > --- > > > .../bindings/sound/mediatek,mt8196-dsp.yaml | 95 > > > +++++++++++++++++++ > > > 1 file changed, 95 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml > > > > > > diff --git > > > a/Documentation/devicetree/bindings/sound/mediatek,mt8196- > > > dsp.yaml > > > b/Documentation/devicetree/bindings/sound/mediatek,mt8196- > > > dsp.yaml > > > new file mode 100644 > > > index 000000000000..68f594f476e8 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8196- > > > dsp.yaml > > Wrong folder; this should go to dsp/mediatek,mt8196-dsp.yaml .... > > ...but then I don't get why MT8196 wasn't simply added to > mediatek,mt8186-dsp.yaml. > Yes, the first version was written this way, but the maintainer suggested the following, so I'm not sure which approach is more appropriate. Could you please provide a final recommendation? v1 link: https://lore.kernel.org/all/a72988212d0d95bfe76eb9c53bbdb8c57c989377.camel@mediatek.com/ comment: > > > --- > > .../bindings/dsp/mediatek,mt8196-dsp.yaml | 96 > > +++++++++++++++++++ > > Don't grow dsp directory. Either this is remoteproc or some sound > component, so place it accordingly. This is a reference to the approach used in a previous MediaTek project: https://web.git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/dsp Do we need to move all the files under the DSP directory to the sound directory? > > > @@ -0,0 +1,95 @@ > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: > > > https://urldefense.com/v3/__http://devicetree.org/schemas/sound/mediatek,mt8196-dsp.yaml*__;Iw!!CTRNKA9wMg0ARbw!g39q48fUCsDJ5PETUtQ0PK0Bkwf0Ce1F_ZzJiEiWMzuybWQ0dSPzblvLlJ-4saWTSoJWuS5imsXbpdgLBvUy4wxw9GN4ryBQ$ > > > +$schema: > > > https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!g39q48fUCsDJ5PETUtQ0PK0Bkwf0Ce1F_ZzJiEiWMzuybWQ0dSPzblvLlJ-4saWTSoJWuS5imsXbpdgLBvUy4wxw9NKbMuhN$ > > > + > > > +title: MediaTek mt8196 DSP core > > > + > > > +maintainers: > > > + - Hailong Fan <hailong.fan@mediatek.com> > > > + > > > +description: The MediaTek mt8196 SoC mt8196 contain a DSP core > > > used for advanced > > > > Missing blank line. Look at other bindings. Also does not look > > wrapped > > at 80 (see Linux coding style doc). > > > > There's also a typo, "mt8196 SoC mt8196" > Will update in next version, thanks. > > > > > + pre- and post- audio processing. > > > + > > > +properties: > > > + compatible: > > > + const: mediatek,mt8196-dsp > > > + > > > + reg: > > > + items: > > > + - description: DSP configuration registers > > > + - description: DSP SRAM > > > + - description: DSP secure registers > > > + - description: DSP bus registers > > > + > > > + reg-names: > > > + items: > > > + - const: cfg > > > + - const: sram > > > + - const: sec > > > + - const: bus > > > + > > > + clocks: > > > + items: > > > + - description: mux for dsp clock > > > + - description: 26M clock > > > + - description: ADSP PLL clock > > > + > > > + clock-names: > > > + items: > > > + - const: adsp_sel > > > > Isn't this called audiodsp in other bindings? > > > > > + - const: clk26m > > > > Don't use frequencies. How is the pin or input called in datasheet? > > How > > other devices call it? > > In the datasheet, this is called.... CLK26M (really). > > Cheers, > Angelo > > > > > > + - const: adsppll > > > + > > > + power-domains: > > > + maxItems: 1 > > > + > > > > Best regards, > > Krzysztof > >
On 09/07/2025 08:34, Hailong Fan (范海龙) wrote: > On Thu, 2025-07-03 at 11:05 +0200, AngeloGioacchino Del Regno wrote: >> External email : Please do not click links or open attachments until >> you have verified the sender or the content. >> >> >> Il 03/07/25 10:08, Krzysztof Kozlowski ha scritto: >>> On 03/07/2025 09:56, hailong.fan wrote: >>>> From: Hailong Fan <hailong.fan@mediatek.com> >>>> >>>> This patch adds mt8196 dsp document. The dsp is used for Sound >>>> Open >>> >>> Please do not use "This commit/patch/change", but imperative mood. >>> See >>> longer explanation here: >>> > https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst*L95__;Iw!!CTRNKA9wMg0ARbw!g39q48fUCsDJ5PETUtQ0PK0Bkwf0Ce1F_ZzJiEiWMzuybWQ0dSPzblvLlJ-4saWTSoJWuS5imsXbpdgLBvUy4wxw9C5w7B4m$ >>> Fix your email client not to produce such junk like above ^. >>>> Firmware driver node. It includes registers, clocks, memory >>>> regions, >>>> and mailbox for dsp. >>>> >>>> Signed-off-by: Hailong Fan <hailong.fan@mediatek.com> >>>> --- >>>> .../bindings/sound/mediatek,mt8196-dsp.yaml | 95 >>>> +++++++++++++++++++ >>>> 1 file changed, 95 insertions(+) >>>> create mode 100644 >>>> Documentation/devicetree/bindings/sound/mediatek,mt8196-dsp.yaml >>>> >>>> diff --git >>>> a/Documentation/devicetree/bindings/sound/mediatek,mt8196- >>>> dsp.yaml >>>> b/Documentation/devicetree/bindings/sound/mediatek,mt8196- >>>> dsp.yaml >>>> new file mode 100644 >>>> index 000000000000..68f594f476e8 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8196- >>>> dsp.yaml >> >> Wrong folder; this should go to dsp/mediatek,mt8196-dsp.yaml .... >> >> ...but then I don't get why MT8196 wasn't simply added to >> mediatek,mt8186-dsp.yaml. >> > Yes, the first version was written this way, but the maintainer > suggested the following, so I'm not sure which approach is more > appropriate. Could you please provide a final recommendation? > > v1 link: > https://lore.kernel.org/all/a72988212d0d95bfe76eb9c53bbdb8c57c989377.camel@mediatek.com/ You linked to your reply. What does it prove? You are the maintainer and you gave this recommendation for your own patch? > > comment: >> >>> --- >>> .../bindings/dsp/mediatek,mt8196-dsp.yaml | 96 >>> +++++++++++++++++++ >> >> Don't grow dsp directory. Either this is remoteproc or some sound >> component, so place it accordingly. > This is a reference to the approach used in a previous MediaTek > project: > > > https://web.git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/dsp > Do we need to move all the files under the DSP directory to the sound > directory? No. Best regards, Krzysztof
On Wed, 2025-07-09 at 08:59 +0200, Krzysztof Kozlowski wrote: > External email : Please do not click links or open attachments until > you have verified the sender or the content. > > > On 09/07/2025 08:34, Hailong Fan (范海龙) wrote: > > On Thu, 2025-07-03 at 11:05 +0200, AngeloGioacchino Del Regno > > wrote: > > > External email : Please do not click links or open attachments > > > until > > > you have verified the sender or the content. > > > > > > > > > Il 03/07/25 10:08, Krzysztof Kozlowski ha scritto: > > > > On 03/07/2025 09:56, hailong.fan wrote: > > > > > From: Hailong Fan <hailong.fan@mediatek.com> > > > > > > > > > > This patch adds mt8196 dsp document. The dsp is used for > > > > > Sound > > > > > Open > > > > > > > > Please do not use "This commit/patch/change", but imperative > > > > mood. > > > > See > > > > longer explanation here: > > > > > > > > https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst*L95__;Iw!!CTRNKA9wMg0ARbw!g39q48fUCsDJ5PETUtQ0PK0Bkwf0Ce1F_ZzJiEiWMzuybWQ0dSPzblvLlJ-4saWTSoJWuS5imsXbpdgLBvUy4wxw9C5w7B4m$ > > > > > > Fix your email client not to produce such junk like above ^. OK,thx. > > > > > > Firmware driver node. It includes registers, clocks, memory > > > > > regions, > > > > > and mailbox for dsp. > > > > > > > > > > Signed-off-by: Hailong Fan <hailong.fan@mediatek.com> > > > > > --- > > > > > .../bindings/sound/mediatek,mt8196-dsp.yaml | 95 > > > > > +++++++++++++++++++ > > > > > 1 file changed, 95 insertions(+) > > > > > create mode 100644 > > > > > Documentation/devicetree/bindings/sound/mediatek,mt8196- > > > > > dsp.yaml > > > > > > > > > > diff --git > > > > > a/Documentation/devicetree/bindings/sound/mediatek,mt8196- > > > > > dsp.yaml > > > > > b/Documentation/devicetree/bindings/sound/mediatek,mt8196- > > > > > dsp.yaml > > > > > new file mode 100644 > > > > > index 000000000000..68f594f476e8 > > > > > --- /dev/null > > > > > +++ > > > > > b/Documentation/devicetree/bindings/sound/mediatek,mt8196- > > > > > dsp.yaml > > > > > > Wrong folder; this should go to dsp/mediatek,mt8196-dsp.yaml .... > > > > > > ...but then I don't get why MT8196 wasn't simply added to > > > mediatek,mt8186-dsp.yaml. > > > > > > > Yes, the first version was written this way, but the maintainer > > suggested the following, so I'm not sure which approach is more > > appropriate. Could you please provide a final recommendation? > > > > v1 link: > > https://lore.kernel.org/all/a72988212d0d95bfe76eb9c53bbdb8c57c989377.camel@mediatek.com/ > > You linked to your reply. What does it prove? You are the maintainer > and > you gave this recommendation for your own patch? > > > > > comment: > > > > > > > --- > > > > .../bindings/dsp/mediatek,mt8196-dsp.yaml | 96 > > > > +++++++++++++++++++ > > > > > > Don't grow dsp directory. Either this is remoteproc or some sound > > > component, so place it accordingly. > > > > This is a reference to the approach used in a previous MediaTek > > project: > > > > > > https://urldefense.com/v3/__https://web.git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/dsp__;!!CTRNKA9wMg0ARbw!nYISmR0CRlwCkvCsX__KkrKPFCiGy1jjwq_rZjMMXmQ6RhzlUeLgBmUpxvAslqJZTvM7J-QDvq_hpD4$ > > Do we need to move all the files under the DSP directory to the > > sound > > directory? > > No. Got it, we will place mediatek,mt8196-dsp.yaml in the Documentation/devicetree/bindings/dsp directory. Thx. > > > Best regards, > Krzysztof
On 03/07/2025 11:05, AngeloGioacchino Del Regno wrote: >>> + >>> + clocks: >>> + items: >>> + - description: mux for dsp clock >>> + - description: 26M clock >>> + - description: ADSP PLL clock >>> + >>> + clock-names: >>> + items: >>> + - const: adsp_sel >> >> Isn't this called audiodsp in other bindings? >> >>> + - const: clk26m >> >> Don't use frequencies. How is the pin or input called in datasheet? How >> other devices call it? > > In the datasheet, this is called.... CLK26M (really). > OK, this is a valid argument, however we still try to unify the inputs so bindings can share such pieces. It is discouraged to have similar devices with different bindings in only one place: clk26m -> clk27m or whatever other number. Common is also to name the clock input based on the purpose (like bus, ref). Best regards, Krzysztof
On Thu, 2025-07-03 at 12:24 +0200, Krzysztof Kozlowski wrote: > External email : Please do not click links or open attachments until > you have verified the sender or the content. > > > On 03/07/2025 11:05, AngeloGioacchino Del Regno wrote: > > > > + > > > > + clocks: > > > > + items: > > > > + - description: mux for dsp clock > > > > + - description: 26M clock > > > > + - description: ADSP PLL clock > > > > + > > > > + clock-names: > > > > + items: > > > > + - const: adsp_sel > > > > > > Isn't this called audiodsp in other bindings? > > > > > > > + - const: clk26m > > > > > > Don't use frequencies. How is the pin or input called in > > > datasheet? How > > > other devices call it? > > > > In the datasheet, this is called.... CLK26M (really). > > > > OK, this is a valid argument, however we still try to unify the > inputs > so bindings can share such pieces. It is discouraged to have similar > devices with different bindings in only one place: clk26m -> clk27m > or > whatever other number. > > Common is also to name the clock input based on the purpose (like > bus, ref). > > > Best regards, > Krzysztof On the MediaTek platform, the main SoC clock sources are typically 32K, 13M, and 26M. This is why the terms clk32k, clk13m, and clk26m are used. Specifically, clk26m refers to the system’s 26 MHz clock source. The clkxxx naming convention is intended to indicate that these are SoC clock sources, while also distinguishing between different clock frequencies. Therefore, using other terms could potentially cause confusion. For example, the ADSP clock sources are ADSPPLL(800MHz) and clk26m(26MHz). On other platforms, the definition of clk26m in the Device Tree Source (DTS) is as follows: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/mediatek/mt8188.dtsi#n328 The mt8196 platform uses the same design. Best Regards, Hailong.Fan
On 09/07/2025 08:05, Hailong Fan (范海龙) wrote: >> >> OK, this is a valid argument, however we still try to unify the >> inputs >> so bindings can share such pieces. It is discouraged to have similar >> devices with different bindings in only one place: clk26m -> clk27m >> or >> whatever other number. >> >> Common is also to name the clock input based on the purpose (like >> bus, ref). >> >> >> Best regards, >> Krzysztof > > On the MediaTek platform, the main SoC clock sources are typically 32K, > 13M, and 26M. This is why the terms clk32k, clk13m, and clk26m are > used. > Specifically, clk26m refers to the system’s 26 MHz clock source. > The clkxxx naming convention is intended to indicate that these are SoC But it should nnot. > clock sources, while also distinguishing between different clock > frequencies. > > Therefore, using other terms could potentially cause confusion. No, you don't understand. This device receives some main PLL or ref clock. That's the only important information in the name, not its frequency.. > > For example, the ADSP clock sources are ADSPPLL(800MHz) and > clk26m(26MHz). > On other platforms, the definition of clk26m in the Device Tree Source > (DTS) is as follows: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/mediatek/mt8188.dtsi#n328 > The mt8196 platform uses the same design. So you add poor code, because previously Mediatek added more poor code? What sort of argument is that? Best regards, Krzysztof
On Wed, 2025-07-09 at 08:57 +0200, Krzysztof Kozlowski wrote: > External email : Please do not click links or open attachments until > you have verified the sender or the content. > > > On 09/07/2025 08:05, Hailong Fan (范海龙) wrote: > > > > > > OK, this is a valid argument, however we still try to unify the > > > inputs > > > so bindings can share such pieces. It is discouraged to have > > > similar > > > devices with different bindings in only one place: clk26m -> > > > clk27m > > > or > > > whatever other number. > > > > > > Common is also to name the clock input based on the purpose (like > > > bus, ref). > > > > > > > > > Best regards, > > > Krzysztof > > > > On the MediaTek platform, the main SoC clock sources are typically > > 32K, > > 13M, and 26M. This is why the terms clk32k, clk13m, and clk26m are > > used. > > Specifically, clk26m refers to the system’s 26 MHz clock source. > > The clkxxx naming convention is intended to indicate that these are > > SoC > > But it should nnot. > > > clock sources, while also distinguishing between different clock > > frequencies. > > > > Therefore, using other terms could potentially cause confusion. > > No, you don't understand. This device receives some main PLL or ref > clock. That's the only important information in the name, not its > frequency.. > > > > > For example, the ADSP clock sources are ADSPPLL(800MHz) and > > clk26m(26MHz). > > On other platforms, the definition of clk26m in the Device Tree > > Source > > (DTS) is as follows: > > > > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/mediatek/mt8188.dtsi*n328__;Iw!!CTRNKA9wMg0ARbw!ibScPB89tyVolnUhBscm6gkEwMebDiD8FozTY1H_-cMZ5_klBxmJxgWlOTOhboITEls4ef4w-Tce2qs$ > > The mt8196 platform uses the same design. > > So you add poor code, because previously Mediatek added more poor > code? > What sort of argument is that? Thank you for your suggestion. We are planning to update it to sys_clk in the next version. What do you think? > > > Best regards, > Krzysztof
© 2016 - 2025 Red Hat, Inc.