| Message ID | 20250307124841.23777-1-darren.ye@mediatek.com |
|---|---|
| Headers | show |
| Series | ASoC: mediatek: Add support for MT8196 SoC | expand |
On Fri, 07 Mar 2025 20:47:40 +0800, Darren.Ye wrote: > From: Darren Ye <darren.ye@mediatek.com> > > Add document for mt8196 board with mt6681. > > Signed-off-by: Darren Ye <darren.ye@mediatek.com> > --- > .../sound/mediatek,mt8196-mt6681.yaml | 122 ++++++++++++++++++ > 1 file changed, 122 insertions(+) > create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt8196-mt6681.yaml > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250307124841.23777-15-darren.ye@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 07/03/2025 13:47, Darren.Ye wrote: > From: Darren Ye <darren.ye@mediatek.com> > > Add mt8196 audio AFE document. > > Signed-off-by: Darren Ye <darren.ye@mediatek.com> Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching. For bindings, the preferred subjects are explained here: https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters > --- > .../bindings/sound/mediatek,mt8196-afe.yaml | 259 ++++++++++++++++++ > 1 file changed, 259 insertions(+) > create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt8196-afe.yaml > > diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8196-afe.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8196-afe.yaml > new file mode 100644 > index 000000000000..59f8fdf3167c > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/mediatek,mt8196-afe.yaml > @@ -0,0 +1,259 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/sound/mediatek,mt8196-afe.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: MediaTek Audio Front End PCM controller for MT8196 > + > +maintainers: > + - Darren Ye <darren.ye@mediatek.com> > + > +properties: > + compatible: > + const: mediatek,mt8196-afe-pcm > + > + reg: > + maxItems: 1 > + > + "#sound-dai-cells": > + const: 0 > + > + clocks: > + items: > + - description: audio hopping clock > + - description: audio f26m clock > + - description: audio ul0 adc clock > + - description: audio ul0 adc hires clock > + - description: audio ul1 adc clock > + - description: audio ul1 adc hires clock > + - description: audio apll1 clock > + - description: audio apll2 clock > + - description: audio apll tuner1 clock > + - description: audio apll tuner2 clock > + - description: vlp mux audio int > + - description: vlp mux aud engen1 > + - description: vlp mux aud engen2 > + - description: vlp mux audio h > + - description: vlp clock 26m > + - description: ck mainpll d4 d4 > + - description: ck mux aud 1 > + - description: ck apll1 > + - description: ck mux aud 2 > + - description: ck apll2 > + - description: ck apll1 d4 > + - description: ck apll2 d4 > + - description: ck i2sin0 m sel > + - description: ck i2sin1 m sel > + - description: ck fmi2s m sel > + - description: ck tdmout m sel > + - description: ck apll12 div i2sin0 > + - description: ck apll12 div i2sin1 > + - description: ck apll12 div fmi2s > + - description: ck apll12 div tdmout m > + - description: ck apll12 div tdmout b > + - description: ck adsp sel > + - description: ck clock 26m > + > + clock-names: > + items: > + - const: aud_hopping_clk Look how other bindings call it. s/_clk// > + - const: aud_f26m_clk > + - const: aud_ul0_adc_clk > + - const: aud_ul0_adc_hires_clk > + - const: aud_ul1_adc_clk > + - const: aud_ul1_adc_hires_clk > + - const: aud_apll1_clk > + - const: aud_apll2_clk > + - const: aud_apll_tuner1_clk > + - const: aud_apll_tuner2_clk > + - const: vlp_mux_audio_int > + - const: vlp_mux_aud_eng1 > + - const: vlp_mux_aud_eng2 > + - const: vlp_mux_audio_h > + - const: vlp_clk26m_clk > + - const: ck_mainpll_d4_d4 What does ck stand for? You should name and explain the clocks based on this block, not the source. > + - const: ck_mux_aud_1 > + - const: ck_apll1_ck > + - const: ck_mux_aud_2 > + - const: ck_apll2_ck > + - const: ck_apll1_d4 > + - const: ck_apll2_d4 > + - const: ck_i2sin0_m_sel > + - const: ck_i2sin1_m_sel > + - const: ck_fmi2s_m_sel > + - const: ck_tdmout_m_sel > + - const: ck_apll12_div_i2sin0 > + - const: ck_apll12_div_i2sin1 > + - const: ck_apll12_div_fmi2s > + - const: ck_apll12_div_tdmout_m > + - const: ck_apll12_div_tdmout_b > + - const: ck_adsp_sel > + - const: ck_clk26m_clk s/ck// s/clk// and this goes probably first. Look at other bindings. > + > + interrupts: > + maxItems: 1 > + > + power-domains: > + maxItems: 1 > + > + cksys: Again, open existing bindings. > + $ref: /schemas/types.yaml#/definitions/phandle > + description: Phandle to the cksys clock controller. This tell me not much. Why do you need it? Drop redundant 'Phandle to' and explain how it is used. > + > + vlpcksys: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: Phandle to the vlpcksys clock controller. No, because you keep encoding clock information via non-clock API. > + > + memory-region: > + $ref: /schemas/types.yaml#/definitions/phandle Drop, see other bindings. > + description: Phandle to the reserved memory region for AFE DMA. > + > + pinctrl-names: Drop > + items: > + - const: default > + > + pinctrl-0: Drop > + $ref: /schemas/types.yaml#/definitions/phandle > + description: Phandle to the pin control group for default state. > + > + mediatek,etdm-out-ch: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: Number of ETDM output channels. > + enum: [2] That's pointless. > + > + mediatek,etdm-in-ch: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: Number of ETDM input channels. > + enum: [2] > + > + mediatek,etdm-out-sync: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: ETDM output synchronization. > + enum: [0, 1] > + > + mediatek,etdm-in-sync: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: ETDM input synchronization. > + enum: [0, 1] > + > + mediatek,etdm-ip-mode: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: ETDM IP mode. > + enum: [0, 1] Drop all above properties or explain why they make any sense in the terms of board configuration. > + > +required: > + - compatible > + - reg > + - clocks > + - clock-names > + - interrupts > + - power-domains > + - cksys > + - vlpcksys > + - memory-region > + - pinctrl-names > + - pinctrl-0 Why? > + - mediatek,etdm-out-ch > + - mediatek,etdm-in-ch > + - mediatek,etdm-out-sync > + - mediatek,etdm-in-sync > + - mediatek,etdm-ip-mode > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/mt8196-clk.h> > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + #include <dt-bindings/power/mt8196-power.h> > + > + soc { > + #address-cells = <2>; > + #size-cells = <2>; > + > + afe: mt8196-afe-pcm@1a110000 { Again... look at other bindings. Best regards, Krzysztof
On 07/03/2025 13:47, Darren.Ye wrote: > From: Darren Ye <darren.ye@mediatek.com> > > Export register read and write interface, add sample reate interface, and > update the mtk_memif_set_channel interface for the mt8196 platform. > > Signed-off-by: Darren Ye <darren.ye@mediatek.com> > --- > sound/soc/mediatek/common/mtk-afe-fe-dai.c | 30 ++++++++++++++-------- > sound/soc/mediatek/common/mtk-afe-fe-dai.h | 6 +++++ > sound/soc/mediatek/common/mtk-base-afe.h | 13 ++++++++++ > 3 files changed, 38 insertions(+), 11 deletions(-) > > diff --git a/sound/soc/mediatek/common/mtk-afe-fe-dai.c b/sound/soc/mediatek/common/mtk-afe-fe-dai.c > index 3809068f5620..c36dae520f04 100644 > --- a/sound/soc/mediatek/common/mtk-afe-fe-dai.c > +++ b/sound/soc/mediatek/common/mtk-afe-fe-dai.c > @@ -18,7 +18,7 @@ > > #define AFE_BASE_END_OFFSET 8 > > -static int mtk_regmap_update_bits(struct regmap *map, int reg, > +int mtk_regmap_update_bits(struct regmap *map, int reg, You need kerneldoc for all exported functions. > unsigned int mask, > unsigned int val, int shift) > { > @@ -26,13 +26,16 @@ static int mtk_regmap_update_bits(struct regmap *map, int reg, > return 0; > return regmap_update_bits(map, reg, mask << shift, val << shift); > } > +EXPORT_SYMBOL(mtk_regmap_update_bits); GPL > + > +int mtk_regmap_write(struct regmap *map, int reg, unsigned int val) > Why blank line? > -static int mtk_regmap_write(struct regmap *map, int reg, unsigned int val) Missing kerneldoc > { > if (reg < 0) > return 0; > return regmap_write(map, reg, val); > } > +EXPORT_SYMBOL(mtk_regmap_write); GPL Best regards, Krzysztof
On 07/03/2025 13:47, Darren.Ye wrote: > From: Darren Ye <darren.ye@mediatek.com> > > Implement sample rate conversion and set private data for mt8196. > > Signed-off-by: Darren Ye <darren.ye@mediatek.com> > --- > .../soc/mediatek/mt8196/mt8196-afe-control.c | 109 ++++++++++++++++++ > 1 file changed, 109 insertions(+) > create mode 100644 sound/soc/mediatek/mt8196/mt8196-afe-control.c > > diff --git a/sound/soc/mediatek/mt8196/mt8196-afe-control.c b/sound/soc/mediatek/mt8196/mt8196-afe-control.c > new file mode 100644 > index 000000000000..bb85f4ad8585 > --- /dev/null > +++ b/sound/soc/mediatek/mt8196/mt8196-afe-control.c > @@ -0,0 +1,109 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * MediaTek ALSA SoC Audio Control > + * > + * Copyright (c) 2024 MediaTek Inc. > + * Author: Darren Ye <darren.ye@mediatek.com> > + */ > + > +#include "mt8196-afe-common.h" > +#include <linux/pm_runtime.h> > + > +unsigned int mt8196_general_rate_transform(struct device *dev, > + unsigned int rate) These are not static, so you miss header for all of these changes. Best regards, Krzysztof
On 07/03/2025 13:47, Darren.Ye wrote: > + > +int mt8196_init_clock(struct mtk_base_afe *afe) > +{ > + struct mt8196_afe_private *afe_priv = afe->platform_priv; > + int i = 0; > + > + afe_priv->clk = devm_kcalloc(afe->dev, CLK_NUM, sizeof(*afe_priv->clk), > + GFP_KERNEL); > + if (!afe_priv->clk) > + return -ENOMEM; > + > + for (i = 0; i < CLK_NUM; i++) { > + if (!aud_clks[i]) { > + dev_err(afe->dev, "%s(), clk id %d not define!!!\n", > + __func__, i); > + } > + > + afe_priv->clk[i] = devm_clk_get(afe->dev, aud_clks[i]); > + if (IS_ERR(afe_priv->clk[i])) { > + dev_err(afe->dev, "%s devm_clk_get %s fail, ret %ld\n", > + __func__, dev_err_probe() and drop __func__ and ret. > + aud_clks[i], PTR_ERR(afe_priv->clk[i])); > + afe_priv->clk[i] = NULL; > + } > + } > + > + afe_priv->vlp_ck = syscon_regmap_lookup_by_phandle(afe->dev->of_node, > + "vlpcksys"); > + if (IS_ERR(afe_priv->vlp_ck)) { > + dev_err(afe->dev, "%s() Cannot find vlpcksys: %ld\n", > + __func__, PTR_ERR(afe_priv->vlp_ck)); > + afe_priv->vlp_ck = NULL; > + } > + > + afe_priv->cksys_ck = syscon_regmap_lookup_by_phandle(afe->dev->of_node, > + "cksys"); > + if (IS_ERR(afe_priv->cksys_ck)) { > + dev_err(afe->dev, "%s() Cannot find cksys controller: %ld\n", > + __func__, PTR_ERR(afe_priv->cksys_ck)); Eeach of my comments apply to entier code to all patches. You keep repeating same patterns from downstream. You should rather take upstream drivers as your base. Best regards, Krzysztof
From: Darren Ye <darren.ye@mediatek.com> This series of patches adds support for Mediatek AFE of MT8196 SoC. Patches are based on broonie tree "for-next" branch. --- This series patches dependent on: [1] https://lore.kernel.org/all/20250307032942.10447-1-guangjie.song@mediatek.com/ Darren Ye (14): ASoC: mediatek: common: modify mtk afe common driver for mt8196 ASoC: mediatek: common: modify mtk afe platform driver for mt8196 ASoC: mediatek: mt8196: add common header ASoC: mediatek: mt8196: add common interface for mt8196 DAI driver ASoC: mediatek: mt8196: support audio clock control ASoC: mediatek: mt8196: support audio GPIO control ASoC: mediatek: mt8196: support ADDA in platform driver ASoC: mediatek: mt8196: support I2S in platform driver ASoC: mediatek: mt8196: support TDM in platform driver ASoC: mediatek: mt8196: support CM in platform driver ASoC: mediatek: mt8196: add platform driver dt-bindings: mediatek: mt8196: add audio AFE document ASoC: mediatek: mt8196: add machine driver with mt6681 dt-bindings: mediatek: mt8196: add mt8196-mt6681 document .../bindings/sound/mediatek,mt8196-afe.yaml | 259 + .../sound/mediatek,mt8196-mt6681.yaml | 122 + sound/soc/mediatek/Kconfig | 30 + sound/soc/mediatek/Makefile | 1 + sound/soc/mediatek/common/mtk-afe-fe-dai.c | 30 +- sound/soc/mediatek/common/mtk-afe-fe-dai.h | 6 + .../mediatek/common/mtk-afe-platform-driver.c | 63 +- .../mediatek/common/mtk-afe-platform-driver.h | 5 + sound/soc/mediatek/common/mtk-base-afe.h | 13 + sound/soc/mediatek/mt8196/Makefile | 19 + sound/soc/mediatek/mt8196/mt8196-afe-clk.c | 698 + sound/soc/mediatek/mt8196/mt8196-afe-clk.h | 313 + sound/soc/mediatek/mt8196/mt8196-afe-cm.c | 94 + sound/soc/mediatek/mt8196/mt8196-afe-cm.h | 23 + sound/soc/mediatek/mt8196/mt8196-afe-common.h | 290 + .../soc/mediatek/mt8196/mt8196-afe-control.c | 109 + sound/soc/mediatek/mt8196/mt8196-afe-gpio.c | 239 + sound/soc/mediatek/mt8196/mt8196-afe-gpio.h | 58 + sound/soc/mediatek/mt8196/mt8196-afe-pcm.c | 5134 +++++++ sound/soc/mediatek/mt8196/mt8196-dai-adda.c | 2207 +++ sound/soc/mediatek/mt8196/mt8196-dai-i2s.c | 4399 ++++++ sound/soc/mediatek/mt8196/mt8196-dai-tdm.c | 837 ++ .../mediatek/mt8196/mt8196-interconnection.h | 121 + sound/soc/mediatek/mt8196/mt8196-mt6681.c | 879 ++ sound/soc/mediatek/mt8196/mt8196-reg.h | 12133 ++++++++++++++++ 25 files changed, 28055 insertions(+), 27 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt8196-afe.yaml create mode 100644 Documentation/devicetree/bindings/sound/mediatek,mt8196-mt6681.yaml create mode 100644 sound/soc/mediatek/mt8196/Makefile create mode 100644 sound/soc/mediatek/mt8196/mt8196-afe-clk.c create mode 100644 sound/soc/mediatek/mt8196/mt8196-afe-clk.h create mode 100644 sound/soc/mediatek/mt8196/mt8196-afe-cm.c create mode 100644 sound/soc/mediatek/mt8196/mt8196-afe-cm.h create mode 100644 sound/soc/mediatek/mt8196/mt8196-afe-common.h create mode 100644 sound/soc/mediatek/mt8196/mt8196-afe-control.c create mode 100644 sound/soc/mediatek/mt8196/mt8196-afe-gpio.c create mode 100644 sound/soc/mediatek/mt8196/mt8196-afe-gpio.h create mode 100644 sound/soc/mediatek/mt8196/mt8196-afe-pcm.c create mode 100644 sound/soc/mediatek/mt8196/mt8196-dai-adda.c create mode 100644 sound/soc/mediatek/mt8196/mt8196-dai-i2s.c create mode 100644 sound/soc/mediatek/mt8196/mt8196-dai-tdm.c create mode 100644 sound/soc/mediatek/mt8196/mt8196-interconnection.h create mode 100644 sound/soc/mediatek/mt8196/mt8196-mt6681.c create mode 100644 sound/soc/mediatek/mt8196/mt8196-reg.h