| Message ID | 20240409113310.303261-1-angelogioacchino.delregno@collabora.com |
|---|---|
| Headers | show |
| Series | SoC: Cleanup MediaTek soundcard machine drivers | expand |
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> On 09/04/2024 13:32, AngeloGioacchino Del Regno wrote: > Add a common machine soundcard driver probe function that supports both > DSP and AFE-direct usecases and also provides a hook for legacy machine > soundcard driver probe mechanisms. > > Note that the hook is there because, even for legacy probe, a lot of the > actual code can still be commonized, hence still reducing duplication > for the legacy devicetree retrocompatibility cases. > > This common probe function deprecates all of the inconsistent previous > probe mechanisms and aims to settle all of the MediaTek card drivers on > consistent and common devicetree properties describing wanted DAIs, > device specific DAI configuration and DAI links to codecs found on > each device/board.
On 09/04/2024 13:32, AngeloGioacchino Del Regno wrote: > @@ -29,6 +30,13 @@ > #define RT1019_SPEAKER_AMP_PRESENT BIT(1) > #define MAX98390_SPEAKER_AMP_PRESENT BIT(2) > > +#define DUMB_CODEC_INIT BIT(0) > +#define MT6359_CODEC_INIT BIT(1) > +#define RT1011_CODEC_INIT BIT(2) > +#define RT1019_CODEC_INIT BIT(3) > +#define MAX98390_CODEC_INIT BIT(4) > +#define RT5682_CODEC_INIT BIT(5) > + Why are you using defines+single variable to track inited parts in the probe function but do it in the different way for legacy_probe using bool: is5682s, init6359 ? AFAII, both can use the same method with the defines above. > #define RT1011_CODEC_DAI "rt1011-aif" > #define RT1011_DEV0_NAME "rt1011.2-0038" > #define RT1011_DEV1_NAME "rt1011.2-0039"
On 09/04/2024 13:32, AngeloGioacchino Del Regno wrote: > @@ -1211,52 +1196,85 @@ static int mt8192_mt6359_dev_probe(struct platform_device *pdev) > if (dai_link->num_codecs && dai_link->codecs[0].dai_name && > strcmp(dai_link->codecs[0].dai_name, RT1015_CODEC_DAI) == 0) > dai_link->ops = &mt8192_rt1015_i2s_ops; > - > - if (!dai_link->platforms->name) > - dai_link->platforms->of_node = platform_node; > - } > - > - priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > - if (!priv) { > - ret = -ENOMEM; > - goto err_probe; > - } > - snd_soc_card_set_drvdata(card, priv); > - > - ret = mt8192_afe_gpio_init(&pdev->dev); > - if (ret) { > - dev_err_probe(&pdev->dev, ret, "%s init gpio error\n", __func__); I don't think __func__ is necessary. > - goto err_probe; > } > > - ret = devm_snd_soc_register_card(&pdev->dev, card); > - if (ret) > - dev_err_probe(&pdev->dev, ret, "%s snd_soc_register_card fail\n", __func__); I don't think __func__ is necessary. > - > -err_probe: > of_node_put(headset_codec); > err_headset_codec: > of_node_put(speaker_codec); > err_speaker_codec: > - of_node_put(platform_node); > -err_platform_node: > - of_node_put(hdmi_codec); > + if (hdmi_codec) > + of_node_put(hdmi_codec); > + > return ret; > } > > +static int mt8192_mt6359_soc_card_probe(struct mtk_soc_card_data *soc_card_data, bool legacy) > +{ > + struct mtk_platform_card_data *card_data = soc_card_data->card_data; > + struct snd_soc_card *card = card_data->card; > + int ret; > + > + if (legacy) { > + ret = mt8192_mt6359_legacy_probe(soc_card_data); > + if (ret) > + return ret; > + } else { > + struct snd_soc_dai_link *dai_link; > + int i; > + > + for_each_card_prelinks(card, i, dai_link) > + if (dai_link->num_codecs && dai_link->codecs[0].dai_name && > + strcmp(dai_link->codecs[0].dai_name, RT1015_CODEC_DAI) == 0) > + dai_link->ops = &mt8192_rt1015_i2s_ops; > + } > + > + ret = mt8192_afe_gpio_init(card->dev); > + if (ret) > + return dev_err_probe(card->dev, ret, "%s init gpio error\n", __func__); I don't think __func__ is necessary. > + > + return 0; > +} Beside that, Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> On 09/04/2024 13:32, AngeloGioacchino Del Regno wrote: > Add mtk_soundcard_pdata platform data for the MediaTek common sound card > probe mechanism, including a driver/soc-specific probe extension (used > for bits that cannot be commonized hence specific to this driver), and > change the probe function to mtk_soundcard_common_probe. > > This is also adding the possibility of specifying the links and routing > with the audio-routing property and (x)-dai-link nodes in device trees > to stop hardcoding machine specific links in the card driver assupported > by the common probe function, but support for legacy device trees is > retained with a legacy_probe function, which is used only in case the > new properties are not found.
Way better with common stuff ! :p Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> On 09/04/2024 13:33, AngeloGioacchino Del Regno wrote: > Add a const mtk_pcm_constraints_data struct array with all of the > (again, constant) constraints for all of the supported usecases, > remove the duplicated functions and call mtk_soundcard_startup() > instead in all of the .startup() callbacks.
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> On 09/04/2024 13:33, AngeloGioacchino Del Regno wrote: > Add a const mtk_pcm_constraints_data struct array with all of the > (again, constant) constraints for all of the supported usecases, > remove the duplicated functions and call mtk_soundcard_startup() > instead in all of the .startup() callbacks.
Changes in v4: - Rebased over next-20240409 - Dropped the first 4 patches from v3 as were already picked by Mark Changes in v3: - Added audio-routing names in enum in all yaml files - Added mention of disallowing old and new properties together in commit message of bindings patches - Fixed validation errors with sound-card-common.yaml inclusion due to missing model property in examples - Removed `else` enforcing headset-codec/speaker-codecs requirement if xxx-dai-link not present to avoid future commit noise as the deprecated statement will disallow deprecated properties as required Changes in v2: - Bindings: Changed link-name/codec/clk-provider to remove `items` and leave just the enum - Moved .*-dai-link pattern additionalProperties after `type: object` - Added ref to sound-card-common.yaml - Fixed dai-link-xxx -> xxx-dai-link typo in example comment This series performs a cleanup of most of the MediaTek AFE drivers and soundcard machine drivers, reducing code duplication and setting a base to be consistent with their devicetree bindings, as many of those are using different properties and nodes for no good reason. Summarizing: - Commonizes functions and ops where possible - Adds a common probe mechanism, increasing maintainability of soundcard drivers for older MediaTek SoCs - Migrates all drivers to support the new bindings - Obviously retains compatibility with old device trees - Reduces machine-specific parameters hardcoding in drivers - Can now set machine-specific params in device tree - Uses the `audio-routing` and `dai-link` nodes like some other non-MediaTek SoC sound drivers - Imposes consistency between MediaTek ASoC machine soundcard drivers bindings - Reduces code size and greatly reduces the amount of code that will be required for newer drivers (retaining compatibility with the old bindings was costly in terms of code size, otherwise this series would've removed ~1000 more lines, or something along that line). This series was (manually) tested on MT8173, MT8192, MT8195 and MT8186 Chromebooks. AngeloGioacchino Del Regno (18): ASoC: mediatek: Add common machine soundcard driver probe mechanism ASoC: mediatek: common: Constify struct mtk_sof_priv ASoC: mediatek: mt8188: Migrate to mtk_soundcard_common_probe ASoC: mediatek: mt8195: Migrate to mtk_soundcard_common_probe ASoC: mediatek: mt8192: Migrate to mtk_soundcard_common_probe ASoC: mediatek: mt8186: Migrate to mtk_soundcard_common_probe ASoC: mediatek: Add common snd_soc_ops .startup() callback ASoC: mediatek: mt8195: Migrate to the common mtk_soundcard_startup ASoC: mediatek: mt8192: Migrate to the common mtk_soundcard_startup ASoC: mediatek: mt8186-rt1019: Migrate to the common mtk_soundcard_startup ASoC: mediatek: Add common mtk_afe_component_probe callback ASoC: mediatek: Use common mtk_afe_pcm_platform with common probe cb ASoC: mediatek: mt8186: Unify mt8186-mt6366 machine drivers ASoC: dt-bindings: mt8195: Document audio-routing and dai-link subnode ASoC: dt-bindings: mt8192: Document audio-routing and dai-link subnode ASoC: dt-bindings: mt8186: Document audio-routing and dai-link subnode arm64: dts: mediatek: mt8195-cherry: Specify sound DAI links and routing arm64: dts: mediatek: mt8186-corsola: Specify sound DAI links and routing .../sound/mt8186-mt6366-da7219-max98357.yaml | 131 +- .../sound/mt8186-mt6366-rt1019-rt5682s.yaml | 120 +- .../sound/mt8192-mt6359-rt1015-rt5682.yaml | 139 +- .../bindings/sound/mt8195-mt6359.yaml | 134 ++ .../boot/dts/mediatek/mt8186-corsola.dtsi | 42 +- .../boot/dts/mediatek/mt8195-cherry.dtsi | 45 + sound/soc/mediatek/Kconfig | 24 +- .../mediatek/common/mtk-afe-platform-driver.c | 18 + .../soc/mediatek/common/mtk-dsp-sof-common.c | 15 +- .../soc/mediatek/common/mtk-dsp-sof-common.h | 1 - sound/soc/mediatek/common/mtk-soc-card.h | 7 +- .../mediatek/common/mtk-soundcard-driver.c | 199 +++ .../mediatek/common/mtk-soundcard-driver.h | 42 + sound/soc/mediatek/mt6797/mt6797-afe-pcm.c | 14 +- sound/soc/mediatek/mt7986/mt7986-afe-pcm.c | 14 +- sound/soc/mediatek/mt8183/mt8183-afe-pcm.c | 14 +- sound/soc/mediatek/mt8186/Makefile | 3 +- sound/soc/mediatek/mt8186/mt8186-afe-pcm.c | 19 +- sound/soc/mediatek/mt8186/mt8186-dai-adda.c | 2 +- .../mt8186/mt8186-mt6366-da7219-max98357.c | 1189 ----------------- ...t6366-rt1019-rt5682s.c => mt8186-mt6366.c} | 578 ++++---- sound/soc/mediatek/mt8188/mt8188-afe-pcm.c | 21 +- sound/soc/mediatek/mt8188/mt8188-mt6359.c | 203 +-- sound/soc/mediatek/mt8192/mt8192-afe-pcm.c | 25 +- .../mt8192/mt8192-mt6359-rt1015-rt5682.c | 301 ++--- sound/soc/mediatek/mt8195/mt8195-afe-pcm.c | 21 +- sound/soc/mediatek/mt8195/mt8195-mt6359.c | 487 +++---- 27 files changed, 1613 insertions(+), 2195 deletions(-) delete mode 100644 sound/soc/mediatek/mt8186/mt8186-mt6366-da7219-max98357.c rename sound/soc/mediatek/mt8186/{mt8186-mt6366-rt1019-rt5682s.c => mt8186-mt6366.c} (72%)