Re: [PATCH v3 4/9] ASoC: dt-bindings: loongson,ls-audio-card: Use common sound card

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Global state corruption via in-place modification of static DAI links array and codec string buffer.
- [High] Sysfs and ASoC component name collision via illegal `dev_set_name` override during probe.
- [High] Bitwise read-modify-write bug without clearing target bitfields causes incorrect APB DMA routing.
--

commit 864c9404304a7ef17062fbe81f6d13b229c50a0d
Author: Binbin Zhou <zhoubinbin@loongson.cn>
Subject: ASoC: dt-bindings: loongson,ls-audio-card: Use common sound card

This commit updates the loongson,ls-audio-card binding to reference the common
sound card schema. It removes the user specified model property and
transitions to evaluating additional properties via unevaluatedProperties.

> diff --git a/Documentation/devicetree/bindings/sound/loongson,ls-audio-card.yaml b/Documentation/devicetree/bindings/sound/loongson,ls-audio-card.yaml
> index 61e8babed402f..e1b7445a8b22f 100644
> --- a/Documentation/devicetree/bindings/sound/loongson,ls-audio-card.yaml
> +++ b/Documentation/devicetree/bindings/sound/loongson,ls-audio-card.yaml

[ ... ]

While reviewing the related sound driver for this binding, I noticed a few
pre-existing items in the driver code.

[Severity: High]
This is a pre-existing issue, but I noticed a potential problem in the
sound card driver when handling multiple instances.

In sound/soc/loongson/loongson_card.c:loongson_card_parse_acpi():
    loongson_dai_links[i].platforms->name = dev_name(phy_dev);
    loongson_dai_links[i].codecs->name = codec_name;
    loongson_dai_links[i].codecs->dai_name = codec_dai_name;

And similarly in sound/soc/loongson/loongson_card.c:loongson_card_parse_of():
    ret = snd_soc_of_get_dlc(cpu, NULL, loongson_dai_links[i].cpus, 0);
    ...
    loongson_dai_links[i].platforms->of_node =
        loongson_dai_links[i].cpus->of_node;
    ret = snd_soc_of_get_dlc(codec, NULL, loongson_dai_links[i].codecs, 0);

Will this corrupt the shared static loongson_dai_links array if multiple
sound cards are instantiated via the device tree or ACPI tables?

If multiple instances are probed, they might overwrite each other's
DAI links and codec strings. Should these structures be dynamically
allocated instead of mutating shared static memory?

[Severity: High]
This is a pre-existing issue, but there might be a sysfs naming collision
in the I2S platform driver.

In sound/soc/loongson/loongson_i2s_plat.c:loongson_i2s_plat_probe():
    dev_set_name(dev, LS_I2S_DRVNAME);

Could this override cause duplicate ASoC component names if multiple I2S
controllers are instantiated in the device tree?

Calling dev_set_name during probe after the device is added to sysfs might
break internal kernel tracking and cause probe failures for subsequent
instances.

[Severity: High]
This is a pre-existing issue, but there appears to be a bitwise
read-modify-write problem in the APB DMA configuration.

In sound/soc/loongson/loongson_i2s_plat.c:loongson_i2s_apbdma_config():
    val |= LOONGSON_DMA2_CONF << LOONGSON_I2S_TX_DMA_OFFSET;
    val |= LOONGSON_DMA3_CONF << LOONGSON_I2S_RX_DMA_OFFSET;

Does this code correctly handle cases where the bootloader or previous
firmware left the DMA configuration register in a non-zero state?

Since LOONGSON_DMA2_CONF occupies multiple bits, if other bits are
already set, the bitwise OR will blend the values instead of strictly
setting the field.

Should the target bitfields be cleared via a bitwise AND mask before
applying the new configuration?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1782439646.git.zhoubinbin@loongson.cn?part=4