[v7,00/16] Add audio support for the MediaTek Genio 350-evk board

Message ID 20240226-audio-i350-v7-0-6518d953a141@baylibre.com (mailing list archive)
Headers
Series Add audio support for the MediaTek Genio 350-evk board |

Message

Alexandre Mergnat July 22, 2024, 6:53 a.m. UTC
  This serie aim to add the following audio support for the Genio 350-evk:
- Playback
  - 2ch Headset Jack (Earphone)
  - 1ch Line-out Jack (Speaker)
  - 8ch HDMI Tx
- Capture
  - 1ch DMIC (On-board Digital Microphone)
  - 1ch AMIC (On-board Analogic Microphone)
  - 1ch Headset Jack (External Analogic Microphone)

Of course, HDMI playback need the MT8365 display patches [1] and a DTS
change documented in "mediatek,mt8365-mt6357.yaml".

Applied patch:
- mfd: mt6397-core: register mt6357 sound codec

Test passed:
- mixer-test log: [3]
- pcm-test log: [4]

[1]: https://lore.kernel.org/all/20231023-display-support-v1-0-5c860ed5c33b@baylibre.com/
[2]: https://lore.kernel.org/all/20240313110147.1267793-1-angelogioacchino.delregno@collabora.com/
[3]: https://pastebin.com/pc43AVrT
[4]: https://pastebin.com/cCtGhDpg
[5]: https://gitlab.baylibre.com/baylibre/mediatek/bsp/linux/-/commits/sound/for-next/add-i350-audio-support

Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>
---
Changes in v7:
- Rebase to "sound/for-6.11" branch.
- Move audio-codec properties to the parent node
- Remove gain values change at init to keep HW default values.
- Remove spurious function by inlining them directly.
- Use standard adaptators for regmap.
- Use "ARRAY_SIZE()" instead of defined value.
- Remove unused variable which breaks an x86 allmodconfig build.
- Link to v6: https://lore.kernel.org/r/20240226-audio-i350-v6-0-f754ec1a7634@baylibre.com

Changes in v6:
- Remove spurious defines
- all files: replace "Mediatek" by "MediaTek"
- dts: replace "pins" by "clk-dat-pins"
- dts: drive-strength: use integer instead of define
- Link to v5: https://lore.kernel.org/r/20240226-audio-i350-v5-0-e7e2569df481@baylibre.com

Changes in v5:
- Rebase to "next-20240523" branch.
- bindings: power supply property moved to the parent node
- Replace "SoC" by "ASoC" in the patch title (5/16)
- Move and rename DAI I2S's defines
- Improve code readability and cleanup
- Link to v4: https://lore.kernel.org/r/20240226-audio-i350-v4-0-082b22186d4c@baylibre.com

Changes in v4:
- Rebase to "next-20240422" branch.
- Re-pass dt_binding_check, functionnal tests, mixer test and pcm test.
- Remove copyright changes.
- Move mt6357 audio codec documention from mt6357.yaml
  to mediatek,mt6357.yaml
- Fix broken indentation in mt8365-evk.dts
- Remove empty node.
- Add more dai link name according to the HW capability.
- Remove spurious property (mediatek,topckgen)
  from mediatek,mt8365-afe.yaml
- Rename "afe" to "audio-controller" in the documentation.
- Link to v3: https://lore.kernel.org/r/20240226-audio-i350-v3-0-16bb2c974c55@baylibre.com

Changes in v3:
- Re-order documentation commit to fix dt_binding_check error.
- Remove $ref and add "mediatek," prefix to vaud28-supply property.
- Link to v2: https://lore.kernel.org/r/20240226-audio-i350-v2-0-3043d483de0d@baylibre.com

Changes in v2:
- Documentation fixed:
  - Remove spurious description.
  - Change property order to fit with dts coding style rules.
  - micbias property: use microvolt value instead of index.
  - mediatek,i2s-shared-clock property removed.
  - mediatek,dmic-iir-on property removed.
  - mediatek,dmic-irr-mode property removed.
  - Change dmic-two-wire-mode => dmic-mode to be aligned with another SoC
  - Remove the spurious 2nd reg of the afe.
- Manage IIR filter feature using audio controls.
- Fix audio controls to pass mixer-test and pcm-test.
- Refactor some const name according to feedbacks.
- Rework the codec to remove spurious driver data.
- Use the new common MTK probe functions for AFE PCM and sound card.
- Rework pinctrl probe in the soundcard driver.
- Remove spurious "const" variables in all files.
- Link to v1: https://lore.kernel.org/r/20240226-audio-i350-v1-0-4fa1cea1667f@baylibre.com

---
Alexandre Mergnat (14):
      ASoC: dt-bindings: mediatek,mt8365-afe: Add audio afe document
      ASoC: dt-bindings: mediatek,mt8365-mt6357: Add audio sound card document
      dt-bindings: mfd: mediatek: Add codec property for MT6357 PMIC
      ASoC: mediatek: mt8365: Add common header
      ASoC: mediatek: mt8365: Add audio clock control support
      ASoC: mediatek: mt8365: Add I2S DAI support
      ASoC: mediatek: mt8365: Add ADDA DAI support
      ASoC: mediatek: mt8365: Add DMIC DAI support
      ASoC: mediatek: mt8365: Add PCM DAI support
      ASoC: mediatek: mt8365: Add the AFE driver support
      ASoC: mediatek: Add MT8365 support
      arm64: defconfig: enable mt8365 sound
      arm64: dts: mediatek: add afe support for mt8365 SoC
      arm64: dts: mediatek: add audio support for mt8365-evk

Nicolas Belin (2):
      ASoc: mediatek: mt8365: Add a specific soundcard for EVK
      ASoC: codecs: add MT6357 support

 .../devicetree/bindings/mfd/mediatek,mt6357.yaml   |   21 +
 .../bindings/sound/mediatek,mt8365-afe.yaml        |  130 ++
 .../bindings/sound/mediatek,mt8365-mt6357.yaml     |  107 +
 arch/arm64/boot/dts/mediatek/mt8365-evk.dts        |   86 +
 arch/arm64/boot/dts/mediatek/mt8365.dtsi           |   43 +-
 arch/arm64/configs/defconfig                       |    2 +
 sound/soc/codecs/Kconfig                           |    7 +
 sound/soc/codecs/Makefile                          |    2 +
 sound/soc/codecs/mt6357.c                          | 1855 ++++++++++++++++
 sound/soc/codecs/mt6357.h                          |  660 ++++++
 sound/soc/mediatek/Kconfig                         |   20 +
 sound/soc/mediatek/Makefile                        |    1 +
 sound/soc/mediatek/mt8365/Makefile                 |   15 +
 sound/soc/mediatek/mt8365/mt8365-afe-clk.c         |  421 ++++
 sound/soc/mediatek/mt8365/mt8365-afe-clk.h         |   32 +
 sound/soc/mediatek/mt8365/mt8365-afe-common.h      |  449 ++++
 sound/soc/mediatek/mt8365/mt8365-afe-pcm.c         | 2275 ++++++++++++++++++++
 sound/soc/mediatek/mt8365/mt8365-dai-adda.c        |  311 +++
 sound/soc/mediatek/mt8365/mt8365-dai-dmic.c        |  340 +++
 sound/soc/mediatek/mt8365/mt8365-dai-i2s.c         |  850 ++++++++
 sound/soc/mediatek/mt8365/mt8365-dai-pcm.c         |  293 +++
 sound/soc/mediatek/mt8365/mt8365-mt6357.c          |  345 +++
 sound/soc/mediatek/mt8365/mt8365-reg.h             |  991 +++++++++
 23 files changed, 9254 insertions(+), 2 deletions(-)
---
base-commit: f2038c12e8133bf4c6bd4d1127a23310d55d9e21
change-id: 20240226-audio-i350-4e11da088e55

Best regards,
  

Comments

Alexandre Mergnat Aug. 14, 2024, 8:23 a.m. UTC | #1
Hi guys !

Simple gentle ping, the serie seems ready to be applied.
Thanks
  
Mark Brown Aug. 14, 2024, 2:05 p.m. UTC | #2
On Wed, Aug 14, 2024 at 10:23:12AM +0200, Alexandre Mergnat wrote:

> Simple gentle ping, the serie seems ready to be applied.

Please don't send content free pings and please allow a reasonable time
for review.  People get busy, go on holiday, attend conferences and so 
on so unless there is some reason for urgency (like critical bug fixes)
please allow at least a couple of weeks for review.  If there have been
review comments then people may be waiting for those to be addressed.

Sending content free pings adds to the mail volume (if they are seen at
all) which is often the problem and since they can't be reviewed
directly if something has gone wrong you'll have to resend the patches
anyway, so sending again is generally a better approach though there are
some other maintainers who like them - if in doubt look at how patches
for the subsystem are normally handled.
  
Mark Brown Sept. 4, 2024, 11:16 a.m. UTC | #3
On Mon, 22 Jul 2024 08:53:29 +0200, Alexandre Mergnat wrote:
> This serie aim to add the following audio support for the Genio 350-evk:
> - Playback
>   - 2ch Headset Jack (Earphone)
>   - 1ch Line-out Jack (Speaker)
>   - 8ch HDMI Tx
> - Capture
>   - 1ch DMIC (On-board Digital Microphone)
>   - 1ch AMIC (On-board Analogic Microphone)
>   - 1ch Headset Jack (External Analogic Microphone)
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[01/16] ASoC: dt-bindings: mediatek,mt8365-afe: Add audio afe document
        commit: ceb3ca2876243e3ea02f78b3d488b1f2d734de49
[02/16] ASoC: dt-bindings: mediatek,mt8365-mt6357: Add audio sound card document
        commit: 76d80dcdd55f70b28930edb97b96ee375e1cce5a
[03/16] dt-bindings: mfd: mediatek: Add codec property for MT6357 PMIC
        commit: 761cab667898d86c04867948f1b7aec1090be796
[04/16] ASoC: mediatek: mt8365: Add common header
        commit: 38c7c9ddc74033406461d64e541bbc8268e77f73
[05/16] ASoC: mediatek: mt8365: Add audio clock control support
        commit: ef307b40b7f0042d54f020bccb3e728ced292282
[06/16] ASoC: mediatek: mt8365: Add I2S DAI support
        commit: 402bbb13a195caa83b3279ebecdabfb11ddee084
[07/16] ASoC: mediatek: mt8365: Add ADDA DAI support
        commit: 7c58c88e524180e8439acdfc44872325e7f6d33d
[08/16] ASoC: mediatek: mt8365: Add DMIC DAI support
        commit: 1c50ec75ce6c0c6b5736499393e522f73e19d0cf
[09/16] ASoC: mediatek: mt8365: Add PCM DAI support
        commit: 5097c0c8634d703e3c59cfb89831b7db9dc46339
[10/16] ASoc: mediatek: mt8365: Add a specific soundcard for EVK
        commit: 1bf6dbd75f7603dd026660bebf324f812200dc1b
[11/16] ASoC: mediatek: mt8365: Add the AFE driver support
        commit: e1991d102bc2abb32331c462f8f3e77059c69578
[12/16] ASoC: codecs: add MT6357 support
        (no commit info)
[13/16] ASoC: mediatek: Add MT8365 support
        (no commit info)

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
  
Nathan Chancellor Sept. 6, 2024, 6:03 p.m. UTC | #4
On Wed, Sep 04, 2024 at 12:16:48PM +0100, Mark Brown wrote:
> [01/16] ASoC: dt-bindings: mediatek,mt8365-afe: Add audio afe document
>         commit: ceb3ca2876243e3ea02f78b3d488b1f2d734de49
> [02/16] ASoC: dt-bindings: mediatek,mt8365-mt6357: Add audio sound card document
>         commit: 76d80dcdd55f70b28930edb97b96ee375e1cce5a
> [03/16] dt-bindings: mfd: mediatek: Add codec property for MT6357 PMIC
>         commit: 761cab667898d86c04867948f1b7aec1090be796
> [04/16] ASoC: mediatek: mt8365: Add common header
>         commit: 38c7c9ddc74033406461d64e541bbc8268e77f73
> [05/16] ASoC: mediatek: mt8365: Add audio clock control support
>         commit: ef307b40b7f0042d54f020bccb3e728ced292282
> [06/16] ASoC: mediatek: mt8365: Add I2S DAI support
>         commit: 402bbb13a195caa83b3279ebecdabfb11ddee084
> [07/16] ASoC: mediatek: mt8365: Add ADDA DAI support
>         commit: 7c58c88e524180e8439acdfc44872325e7f6d33d
> [08/16] ASoC: mediatek: mt8365: Add DMIC DAI support
>         commit: 1c50ec75ce6c0c6b5736499393e522f73e19d0cf
> [09/16] ASoC: mediatek: mt8365: Add PCM DAI support
>         commit: 5097c0c8634d703e3c59cfb89831b7db9dc46339
> [10/16] ASoc: mediatek: mt8365: Add a specific soundcard for EVK
>         commit: 1bf6dbd75f7603dd026660bebf324f812200dc1b
> [11/16] ASoC: mediatek: mt8365: Add the AFE driver support
>         commit: e1991d102bc2abb32331c462f8f3e77059c69578

I am seeing several warnings/errors from both GCC and Clang with
ARCH=arm64 allmodconfig after this series appeared in next-20240906.
As far as I can tell, they appear to agree. I wondered how this was not
caught during the series development but perhaps it was written against
a development tree that did not have Arnd's extrawarn series from 6.10
in it yet? I was going to work on a series but I was not sure about the
best way to address the overflow errors, hence just the report.

Clang 19:

  sound/soc/mediatek/mt8365/mt8365-afe-clk.c:298:5: error: no previous prototype for function 'mt8365_afe_hd_engen_enable' [-Werror,-Wmissing-prototypes]
    298 | int mt8365_afe_hd_engen_enable(struct mtk_base_afe *afe, bool apll1)
        |     ^
  sound/soc/mediatek/mt8365/mt8365-afe-clk.c:298:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
    298 | int mt8365_afe_hd_engen_enable(struct mtk_base_afe *afe, bool apll1)
        | ^
        | static 
  sound/soc/mediatek/mt8365/mt8365-afe-clk.c:310:5: error: no previous prototype for function 'mt8365_afe_hd_engen_disable' [-Werror,-Wmissing-prototypes]
    310 | int mt8365_afe_hd_engen_disable(struct mtk_base_afe *afe, bool apll1)
        |     ^
  sound/soc/mediatek/mt8365/mt8365-afe-clk.c:310:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
    310 | int mt8365_afe_hd_engen_disable(struct mtk_base_afe *afe, bool apll1)
        | ^
        | static 
  sound/soc/mediatek/mt8365/mt8365-afe-clk.c:314:24: error: implicit conversion from 'unsigned long' to 'unsigned int' changes value from 18446744073709551614 to 4294967294 [-Werror,-Wconstant-conversion]
    313 |                 regmap_update_bits(afe->regmap, AFE_HD_ENGEN_ENABLE,
        |                 ~~~~~~~~~~~~~~~~~~
    314 |                                    AFE_22M_PLL_EN, ~AFE_22M_PLL_EN);
        |                                                    ^~~~~~~~~~~~~~~
  sound/soc/mediatek/mt8365/mt8365-afe-clk.c:317:24: error: implicit conversion from 'unsigned long' to 'unsigned int' changes value from 18446744073709551613 to 4294967293 [-Werror,-Wconstant-conversion]
    316 |                 regmap_update_bits(afe->regmap, AFE_HD_ENGEN_ENABLE,
        |                 ~~~~~~~~~~~~~~~~~~
    317 |                                    AFE_24M_PLL_EN, ~AFE_24M_PLL_EN);
        |                                                    ^~~~~~~~~~~~~~~
  4 errors generated.

  sound/soc/mediatek/mt8365/mt8365-dai-adda.c:93:8: error: implicit conversion from 'unsigned long' to 'unsigned int' changes value from 18446744073709551614 to 4294967294 [-Werror,-Wconstant-conversion]
     91 |                 regmap_update_bits(afe->regmap, AFE_ADDA_UL_DL_CON0,
        |                 ~~~~~~~~~~~~~~~~~~
     92 |                                    AFE_ADDA_UL_DL_ADDA_AFE_ON,
     93 |                                    ~AFE_ADDA_UL_DL_ADDA_AFE_ON);
        |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~
  1 error generated.

  sound/soc/mediatek/mt8365/mt8365-mt6357.c:293:22: error: unused variable 'platform_node' [-Werror,-Wunused-variable]
    293 |         struct device_node *platform_node;
        |                             ^~~~~~~~~~~~~
  sound/soc/mediatek/mt8365/mt8365-mt6357.c:295:6: error: unused variable 'i' [-Werror,-Wunused-variable]
    295 |         int i, ret;
        |             ^
  2 errors generated.

  sound/soc/mediatek/mt8365/mt8365-dai-dmic.c:64:7: error: implicit conversion from 'unsigned long' to 'unsigned int' changes value from 18446744073709551613 to 4294967293 [-Werror,-Wconstant-conversion]
     62 |         regmap_update_bits(afe->regmap, AFE_ADDA_UL_DL_CON0,
        |         ~~~~~~~~~~~~~~~~~~
     63 |                            AFE_ADDA_UL_DL_DMIC_CLKDIV_ON,
     64 |                            ~AFE_ADDA_UL_DL_DMIC_CLKDIV_ON);
        |                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  1 error generated.

  sound/soc/mediatek/mt8365/mt8365-dai-i2s.c:388:8: error: implicit conversion from 'unsigned long' to 'unsigned int' changes value from 18446744073709551613 to 4294967293 [-Werror,-Wconstant-conversion]
    386 |                 regmap_update_bits(afe->regmap, AFE_ASRC_2CH_CON0,
        |                 ~~~~~~~~~~~~~~~~~~
    387 |                                    COEFF_SRAM_CTRL,
    388 |                                    (unsigned long)~COEFF_SRAM_CTRL);
        |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  sound/soc/mediatek/mt8365/mt8365-dai-i2s.c:396:16: error: implicit conversion from 'unsigned long' to 'unsigned int' changes value from 18446744073709549567 to 4294965247 [-Werror,-Wconstant-conversion]
    395 |                 regmap_update_bits(afe->regmap, AFE_ASRC_2CH_CON2,
        |                 ~~~~~~~~~~~~~~~~~~
    396 |                                    IIR_EN, (unsigned long)~IIR_EN);
        |                                            ^~~~~~~~~~~~~~~~~~~~~~
  sound/soc/mediatek/mt8365/mt8365-dai-i2s.c:459:16: error: implicit conversion from 'unsigned long' to 'unsigned int' changes value from 18446744073709551614 to 4294967294 [-Werror,-Wconstant-conversion]
    458 |                 regmap_update_bits(afe->regmap, AFE_ASRC_2CH_CON0,
        |                 ~~~~~~~~~~~~~~~~~~
    459 |                                    ASM_ON, (unsigned long)~ASM_ON);
        |                                            ^~~~~~~~~~~~~~~~~~~~~~
  3 errors generated.

  sound/soc/mediatek/mt8365/mt8365-afe-pcm.c:173:6: error: no previous prototype for function 'mt8365_afe_clk_group_44k' [-Werror,-Wmissing-prototypes]
    173 | bool mt8365_afe_clk_group_44k(int sample_rate)
        |      ^
  sound/soc/mediatek/mt8365/mt8365-afe-pcm.c:173:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
    173 | bool mt8365_afe_clk_group_44k(int sample_rate)
        | ^
        | static 
  sound/soc/mediatek/mt8365/mt8365-afe-pcm.c:499:5: error: no previous prototype for function 'mt8365_afe_fe_startup' [-Werror,-Wmissing-prototypes]
    499 | int mt8365_afe_fe_startup(struct snd_pcm_substream *substream,
        |     ^
  sound/soc/mediatek/mt8365/mt8365-afe-pcm.c:499:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
    499 | int mt8365_afe_fe_startup(struct snd_pcm_substream *substream,
        | ^
        | static 
  sound/soc/mediatek/mt8365/mt8365-afe-pcm.c:575:9: error: implicit conversion from 'unsigned long' to 'unsigned int' changes value from 18446744073709547519 to 4294963199 [-Werror,-Wconstant-conversion]
    573 |                         regmap_update_bits(afe->regmap, AFE_CM2_CON0,
        |                         ~~~~~~~~~~~~~~~~~~
    574 |                                            CM_AFE_CM2_TDM_SEL,
    575 |                                            ~CM_AFE_CM2_TDM_SEL);
        |                                            ^~~~~~~~~~~~~~~~~~~
  sound/soc/mediatek/mt8365/mt8365-afe-pcm.c:654:29: error: unused variable 'memif' [-Werror,-Wunused-variable]
    654 |         struct mtk_base_afe_memif *memif = &afe->memif[dai_id];
        |                                    ^~~~~
  sound/soc/mediatek/mt8365/mt8365-afe-pcm.c:717:5: error: no previous prototype for function 'mt8365_afe_fe_trigger' [-Werror,-Wmissing-prototypes]
    717 | int mt8365_afe_fe_trigger(struct snd_pcm_substream *substream, int cmd,
        |     ^
  sound/soc/mediatek/mt8365/mt8365-afe-pcm.c:717:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
    717 | int mt8365_afe_fe_trigger(struct snd_pcm_substream *substream, int cmd,
        | ^
        | static 
  sound/soc/mediatek/mt8365/mt8365-afe-pcm.c:746:23: error: implicit conversion from 'unsigned long' to 'unsigned int' changes value from 18446744073709551599 to 4294967279 [-Werror,-Wconstant-conversion]
    745 |                         regmap_update_bits(afe->regmap, AFE_CM1_CON0,
        |                         ~~~~~~~~~~~~~~~~~~
    746 |                                            CM_AFE_CM_ON, ~CM_AFE_CM_ON);
        |                                                          ^~~~~~~~~~~~~
  sound/soc/mediatek/mt8365/mt8365-afe-pcm.c:750:23: error: implicit conversion from 'unsigned long' to 'unsigned int' changes value from 18446744073709551599 to 4294967279 [-Werror,-Wconstant-conversion]
    749 |                         regmap_update_bits(afe->regmap, AFE_CM2_CON0,
        |                         ~~~~~~~~~~~~~~~~~~
    750 |                                            CM_AFE_CM_ON, ~CM_AFE_CM_ON);
        |                                                          ^~~~~~~~~~~~~
  7 errors generated.

GCC 14:

  sound/soc/mediatek/mt8365/mt8365-mt6357.c: In function 'mt8365_mt6357_dev_probe':
  sound/soc/mediatek/mt8365/mt8365-mt6357.c:295:13: error: unused variable 'i' [-Werror=unused-variable]
    295 |         int i, ret;
        |             ^
  sound/soc/mediatek/mt8365/mt8365-mt6357.c:293:29: error: unused variable 'platform_node' [-Werror=unused-variable]
    293 |         struct device_node *platform_node;
        |                             ^~~~~~~~~~~~~
  cc1: all warnings being treated as errors

  sound/soc/mediatek/mt8365/mt8365-dai-dmic.c: In function 'audio_dmic_adda_disable':
  sound/soc/mediatek/mt8365/mt8365-dai-dmic.c:64:28: error: conversion from 'long unsigned int' to 'unsigned int' changes value from '18446744073709551613' to '4294967293' [-Werror=overflow]
     64 |                            ~AFE_ADDA_UL_DL_DMIC_CLKDIV_ON);
  sound/soc/mediatek/mt8365/mt8365-dai-dmic.c: At top level:
  sound/soc/mediatek/mt8365/mt8365-dai-dmic.c:134:12: error: 'mt8365_dai_load_dmic_iir_coeff_table' defined but not used [-Werror=unused-function]
    134 | static int mt8365_dai_load_dmic_iir_coeff_table(struct mtk_base_afe *afe)
        |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  cc1: all warnings being treated as errors

  sound/soc/mediatek/mt8365/mt8365-dai-adda.c: In function 'mt8365_dai_disable_adda_on':
  sound/soc/mediatek/mt8365/mt8365-dai-adda.c:93:36: error: conversion from 'long unsigned int' to 'unsigned int' changes value from '18446744073709551614' to '4294967294' [-Werror=overflow]
     93 |                                    ~AFE_ADDA_UL_DL_ADDA_AFE_ON);
  cc1: all warnings being treated as errors

  sound/soc/mediatek/mt8365/mt8365-afe-clk.c:298:5: error: no previous prototype for 'mt8365_afe_hd_engen_enable' [-Werror=missing-prototypes]
    298 | int mt8365_afe_hd_engen_enable(struct mtk_base_afe *afe, bool apll1)
        |     ^~~~~~~~~~~~~~~~~~~~~~~~~~
  sound/soc/mediatek/mt8365/mt8365-afe-clk.c:310:5: error: no previous prototype for 'mt8365_afe_hd_engen_disable' [-Werror=missing-prototypes]
    310 | int mt8365_afe_hd_engen_disable(struct mtk_base_afe *afe, bool apll1)
        |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~
  sound/soc/mediatek/mt8365/mt8365-afe-clk.c: In function 'mt8365_afe_hd_engen_disable':
  sound/soc/mediatek/mt8365/mt8365-afe-clk.c:314:52: error: conversion from 'long unsigned int' to 'unsigned int' changes value from '18446744073709551614' to '4294967294' [-Werror=overflow]
    314 |                                    AFE_22M_PLL_EN, ~AFE_22M_PLL_EN);
  sound/soc/mediatek/mt8365/mt8365-afe-clk.c:317:52: error: conversion from 'long unsigned int' to 'unsigned int' changes value from '18446744073709551613' to '4294967293' [-Werror=overflow]
    317 |                                    AFE_24M_PLL_EN, ~AFE_24M_PLL_EN);
  cc1: all warnings being treated as errors

  sound/soc/mediatek/mt8365/mt8365-dai-i2s.c: In function 'mt8365_afe_set_2nd_i2s_asrc':
  sound/soc/mediatek/mt8365/mt8365-dai-i2s.c:388:36: error: conversion from 'long unsigned int' to 'unsigned int' changes value from '18446744073709551613' to '4294967293' [-Werror=overflow]
    388 |                                    (unsigned long)~COEFF_SRAM_CTRL);
  sound/soc/mediatek/mt8365/mt8365-dai-i2s.c:396:44: error: conversion from 'long unsigned int' to 'unsigned int' changes value from '18446744073709549567' to '4294965247' [-Werror=overflow]
    396 |                                    IIR_EN, (unsigned long)~IIR_EN);
  sound/soc/mediatek/mt8365/mt8365-dai-i2s.c: In function 'mt8365_afe_set_2nd_i2s_asrc_enable':
  sound/soc/mediatek/mt8365/mt8365-dai-i2s.c:459:44: error: conversion from 'long unsigned int' to 'unsigned int' changes value from '18446744073709551614' to '4294967294' [-Werror=overflow]
    459 |                                    ASM_ON, (unsigned long)~ASM_ON);
  cc1: all warnings being treated as errors

  sound/soc/mediatek/mt8365/mt8365-afe-pcm.c:173:6: error: no previous prototype for 'mt8365_afe_clk_group_44k' [-Werror=missing-prototypes]
    173 | bool mt8365_afe_clk_group_44k(int sample_rate)
        |      ^~~~~~~~~~~~~~~~~~~~~~~~
  sound/soc/mediatek/mt8365/mt8365-afe-pcm.c:499:5: error: no previous prototype for 'mt8365_afe_fe_startup' [-Werror=missing-prototypes]
    499 | int mt8365_afe_fe_startup(struct snd_pcm_substream *substream,
        |     ^~~~~~~~~~~~~~~~~~~~~
  sound/soc/mediatek/mt8365/mt8365-afe-pcm.c: In function 'mt8365_afe_fe_hw_params':
  sound/soc/mediatek/mt8365/mt8365-afe-pcm.c:575:44: error: conversion from 'long unsigned int' to 'unsigned int' changes value from '18446744073709547519' to '4294963199' [-Werror=overflow]
    575 |                                            ~CM_AFE_CM2_TDM_SEL);
  sound/soc/mediatek/mt8365/mt8365-afe-pcm.c: In function 'mt8365_afe_fe_hw_free':
  sound/soc/mediatek/mt8365/mt8365-afe-pcm.c:654:36: error: unused variable 'memif' [-Werror=unused-variable]
    654 |         struct mtk_base_afe_memif *memif = &afe->memif[dai_id];
        |                                    ^~~~~
  sound/soc/mediatek/mt8365/mt8365-afe-pcm.c: At top level:
  sound/soc/mediatek/mt8365/mt8365-afe-pcm.c:717:5: error: no previous prototype for 'mt8365_afe_fe_trigger' [-Werror=missing-prototypes]
    717 | int mt8365_afe_fe_trigger(struct snd_pcm_substream *substream, int cmd,
        |     ^~~~~~~~~~~~~~~~~~~~~
  sound/soc/mediatek/mt8365/mt8365-afe-pcm.c: In function 'mt8365_afe_fe_trigger':
  sound/soc/mediatek/mt8365/mt8365-afe-pcm.c:746:58: error: conversion from 'long unsigned int' to 'unsigned int' changes value from '18446744073709551599' to '4294967279' [-Werror=overflow]
    746 |                                            CM_AFE_CM_ON, ~CM_AFE_CM_ON);
  sound/soc/mediatek/mt8365/mt8365-afe-pcm.c:750:58: error: conversion from 'long unsigned int' to 'unsigned int' changes value from '18446744073709551599' to '4294967279' [-Werror=overflow]
    750 |                                            CM_AFE_CM_ON, ~CM_AFE_CM_ON);
  cc1: all warnings being treated as errors

Cheers,
Nathan
  
Mark Brown Sept. 6, 2024, 6:27 p.m. UTC | #5
On Fri, Sep 06, 2024 at 11:03:48AM -0700, Nathan Chancellor wrote:

> I am seeing several warnings/errors from both GCC and Clang with
> ARCH=arm64 allmodconfig after this series appeared in next-20240906.
> As far as I can tell, they appear to agree. I wondered how this was not
> caught during the series development but perhaps it was written against
> a development tree that did not have Arnd's extrawarn series from 6.10
> in it yet? I was going to work on a series but I was not sure about the
> best way to address the overflow errors, hence just the report.

Are these just warnings introduced by recent versions of the toolchains?
These commits passed an x86 allmodconfig with GCC 12 at each step (I did
catch one warning there with another patch in the series that didn't get
applied) and 0day didn't say anything at any point.

> Clang 19:

That's relatively modern, though some of the warnings don't look
particularly new and exciting.

>   sound/soc/mediatek/mt8365/mt8365-dai-adda.c:93:8: error: implicit conversion from 'unsigned long' to 'unsigned int' changes value from 18446744073709551614 to 4294967294 [-Werror,-Wconstant-conversion]
>      91 |                 regmap_update_bits(afe->regmap, AFE_ADDA_UL_DL_CON0,
>         |                 ~~~~~~~~~~~~~~~~~~
>      92 |                                    AFE_ADDA_UL_DL_ADDA_AFE_ON,
>      93 |                                    ~AFE_ADDA_UL_DL_ADDA_AFE_ON);
>         |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>   1 error generated.

That's a bit surprising, regmap_update_bits() takes an unsigned long?  I
suspect the constants need to be defined as unsigned.
  
Mark Brown Sept. 6, 2024, 6:34 p.m. UTC | #6
On Fri, Sep 06, 2024 at 07:27:04PM +0100, Mark Brown wrote:

> These commits passed an x86 allmodconfig with GCC 12 at each step (I did
> catch one warning there with another patch in the series that didn't get
> applied) and 0day didn't say anything at any point.

Oh, actually right as I was writing that message 0day decided to report
that (and some other things that look rather preexisting), though only
with clang 20.
  
Nathan Chancellor Sept. 6, 2024, 7:23 p.m. UTC | #7
On Fri, Sep 06, 2024 at 07:27:01PM +0100, Mark Brown wrote:
> Are these just warnings introduced by recent versions of the toolchains?
> These commits passed an x86 allmodconfig with GCC 12 at each step (I did
> catch one warning there with another patch in the series that didn't get
> applied) and 0day didn't say anything at any point.

Not sure, I did not look too hard. At cursory glance, I am not sure x86
allmodconfig would catch these, as this code depends on ARCH_MEDIATEK
(not COMPILE_TEST), which only exists for arm and arm64.

> > Clang 19:
> 
> That's relatively modern, though some of the warnings don't look
> particularly new and exciting.

Fair although I still see some of them on old versions too:

https://github.com/ClangBuiltLinux/continuous-integration2/actions/runs/10738441894

> >   sound/soc/mediatek/mt8365/mt8365-dai-adda.c:93:8: error: implicit conversion from 'unsigned long' to 'unsigned int' changes value from 18446744073709551614 to 4294967294 [-Werror,-Wconstant-conversion]
> >      91 |                 regmap_update_bits(afe->regmap, AFE_ADDA_UL_DL_CON0,
> >         |                 ~~~~~~~~~~~~~~~~~~
> >      92 |                                    AFE_ADDA_UL_DL_ADDA_AFE_ON,
> >      93 |                                    ~AFE_ADDA_UL_DL_ADDA_AFE_ON);
> >         |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> >   1 error generated.
> 
> That's a bit surprising, regmap_update_bits() takes an unsigned long?  I
> suspect the constants need to be defined as unsigned.

Does it? I see it taking 'unsigned int' for all of its parameters.

$ sed -n '1242,1250p' include/linux/regmap.h
int regmap_update_bits_base(struct regmap *map, unsigned int reg,
                            unsigned int mask, unsigned int val,
                            bool *change, bool async, bool force);

static inline int regmap_update_bits(struct regmap *map, unsigned int reg,
                                     unsigned int mask, unsigned int val)
{
        return regmap_update_bits_base(map, reg, mask, val, NULL, false, false);
}

Cheers,
Nathan
  
Mark Brown Sept. 6, 2024, 9:04 p.m. UTC | #8
On Fri, Sep 06, 2024 at 12:23:23PM -0700, Nathan Chancellor wrote:
> On Fri, Sep 06, 2024 at 07:27:01PM +0100, Mark Brown wrote:
> > Are these just warnings introduced by recent versions of the toolchains?
> > These commits passed an x86 allmodconfig with GCC 12 at each step (I did
> > catch one warning there with another patch in the series that didn't get
> > applied) and 0day didn't say anything at any point.

> Not sure, I did not look too hard. At cursory glance, I am not sure x86
> allmodconfig would catch these, as this code depends on ARCH_MEDIATEK
> (not COMPILE_TEST), which only exists for arm and arm64.

Ah, I hadn't seen that (the other bits were building on x86).

> > > Clang 19:

> > That's relatively modern, though some of the warnings don't look
> > particularly new and exciting.

> Fair although I still see some of them on old versions too:

Yeah, like I say some of the warnings didn't look like they were new.

> https://github.com/ClangBuiltLinux/continuous-integration2/actions/runs/10738441894
> 
> > >   sound/soc/mediatek/mt8365/mt8365-dai-adda.c:93:8: error: implicit conversion from 'unsigned long' to 'unsigned int' changes value from 18446744073709551614 to 4294967294 [-Werror,-Wconstant-conversion]
> > >      91 |                 regmap_update_bits(afe->regmap, AFE_ADDA_UL_DL_CON0,
> > >         |                 ~~~~~~~~~~~~~~~~~~
> > >      92 |                                    AFE_ADDA_UL_DL_ADDA_AFE_ON,
> > >      93 |                                    ~AFE_ADDA_UL_DL_ADDA_AFE_ON);
> > >         |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >   1 error generated.
> > 
> > That's a bit surprising, regmap_update_bits() takes an unsigned long?  I
> > suspect the constants need to be defined as unsigned.

> Does it? I see it taking 'unsigned int' for all of its parameters.

Sorry, I misread the warning there (or was perhaps looking at another
one) and for some reason though it was about dropping signs but it's
rather due to BIT() being defined for longs which is a rather bad
landmine given how common a pattern negation is.  I do see that
synclink.h has some convenient BITn macros for some reason which would
do the trick but really I think it's just a case of open coding the
BIT() usage, or defining a BITI() or something but that seems ugly.
Probably just open code the definitions.
  
Lee Jones Sept. 12, 2024, 2:51 p.m. UTC | #9
On Wed, 04 Sep 2024, Mark Brown wrote:

> On Mon, 22 Jul 2024 08:53:29 +0200, Alexandre Mergnat wrote:
> > This serie aim to add the following audio support for the Genio 350-evk:
> > - Playback
> >   - 2ch Headset Jack (Earphone)
> >   - 1ch Line-out Jack (Speaker)
> >   - 8ch HDMI Tx
> > - Capture
> >   - 1ch DMIC (On-board Digital Microphone)
> >   - 1ch AMIC (On-board Analogic Microphone)
> >   - 1ch Headset Jack (External Analogic Microphone)
> > 
> > [...]
> 
> Applied to
> 
>    https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
> 
> Thanks!
> 
> [01/16] ASoC: dt-bindings: mediatek,mt8365-afe: Add audio afe document
>         commit: ceb3ca2876243e3ea02f78b3d488b1f2d734de49
> [02/16] ASoC: dt-bindings: mediatek,mt8365-mt6357: Add audio sound card document
>         commit: 76d80dcdd55f70b28930edb97b96ee375e1cce5a
> [03/16] dt-bindings: mfd: mediatek: Add codec property for MT6357 PMIC
>         commit: 761cab667898d86c04867948f1b7aec1090be796

Did you mean to hoover this up?

> [04/16] ASoC: mediatek: mt8365: Add common header
>         commit: 38c7c9ddc74033406461d64e541bbc8268e77f73
> [05/16] ASoC: mediatek: mt8365: Add audio clock control support
>         commit: ef307b40b7f0042d54f020bccb3e728ced292282
> [06/16] ASoC: mediatek: mt8365: Add I2S DAI support
>         commit: 402bbb13a195caa83b3279ebecdabfb11ddee084
> [07/16] ASoC: mediatek: mt8365: Add ADDA DAI support
>         commit: 7c58c88e524180e8439acdfc44872325e7f6d33d
> [08/16] ASoC: mediatek: mt8365: Add DMIC DAI support
>         commit: 1c50ec75ce6c0c6b5736499393e522f73e19d0cf
> [09/16] ASoC: mediatek: mt8365: Add PCM DAI support
>         commit: 5097c0c8634d703e3c59cfb89831b7db9dc46339
> [10/16] ASoc: mediatek: mt8365: Add a specific soundcard for EVK
>         commit: 1bf6dbd75f7603dd026660bebf324f812200dc1b
> [11/16] ASoC: mediatek: mt8365: Add the AFE driver support
>         commit: e1991d102bc2abb32331c462f8f3e77059c69578
> [12/16] ASoC: codecs: add MT6357 support
>         (no commit info)
> [13/16] ASoC: mediatek: Add MT8365 support
>         (no commit info)
> 
> All being well this means that it will be integrated into the linux-next
> tree (usually sometime in the next 24 hours) and sent to Linus during
> the next merge window (or sooner if it is a bug fix), however if
> problems are discovered then the patch may be dropped or reverted.
> 
> You may get further e-mails resulting from automated or manual testing
> and review of the tree, please engage with people reporting problems and
> send followup patches addressing any issues that are reported if needed.
> 
> If any updates are required or you are submitting further changes they
> should be sent as incremental updates against current git, existing
> patches will not be replaced.
> 
> Please add any relevant lists and maintainers to the CCs when replying
> to this mail.
> 
> Thanks,
> Mark
>
  
Mark Brown Sept. 12, 2024, 3:17 p.m. UTC | #10
On Thu, Sep 12, 2024 at 03:51:00PM +0100, Lee Jones wrote:
> On Wed, 04 Sep 2024, Mark Brown wrote:

> > [03/16] dt-bindings: mfd: mediatek: Add codec property for MT6357 PMIC
> >         commit: 761cab667898d86c04867948f1b7aec1090be796

> Did you mean to hoover this up?

It seemed to go along with the series and had a DT review so it looked
like you'd just left it to the DT people to review, there wasn't any
other MFD content in the series.
  
Lee Jones Sept. 12, 2024, 4:54 p.m. UTC | #11
On Thu, 12 Sep 2024, Mark Brown wrote:

> On Thu, Sep 12, 2024 at 03:51:00PM +0100, Lee Jones wrote:
> > On Wed, 04 Sep 2024, Mark Brown wrote:
> 
> > > [03/16] dt-bindings: mfd: mediatek: Add codec property for MT6357 PMIC
> > >         commit: 761cab667898d86c04867948f1b7aec1090be796
> 
> > Did you mean to hoover this up?
> 
> It seemed to go along with the series and had a DT review so it looked
> like you'd just left it to the DT people to review, there wasn't any
> other MFD content in the series.

I applied it from this set 6 weeks ago! :)