Re: [PATCH v2 1/2] dt-bindings: dsp: mediatek: add mt8196 dsp document

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/submit...

...

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