Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Linus Walleij" <linusw@kernel.org>
Cc: devicetree@vger.kernel.org, conor+dt@kernel.org, robh@kernel.org,
	mfd@lists.linux.dev, lee@kernel.org
Subject: Re: [PATCH v2 7/9] ARM: dts: ux500: Convert HREF audio to audio-graph-card2
Date: Thu, 17 Sep 2026 23:06:46 +0000	[thread overview]
Message-ID: <20260917230646.AFDD81F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260918-ux500-simple-sound-v2-7-88d29416e3c3@kernel.org>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] sound: ux500: hardcoded TDM slots break dynamic channel support
- [High] sound: ux500: ABI regression causing loss of DAPM pin switches
--

commit 3bce354bbefb3531701f651af445996e9f8ba398
Author: Linus Walleij <linusw@kernel.org>

ARM: dts: ux500: Convert HREF audio to audio-graph-card2

This commit converts the HREF500 and HREF520 sound hardware to use
audio-graph-card2 in their device trees, replacing the legacy machine driver.
It establishes links between MSP1/MSP3 and the audio codec, along with
defining the TDM frame properties and endpoints.

> diff --git a/arch/arm/boot/dts/st/ste-href-ab8500.dtsi b/arch/arm/boot/dts/st/ste-href-ab8500.dtsi
> index 5eeb44c5e9323..ff72f5f6d6525 100644
> --- a/arch/arm/boot/dts/st/ste-href-ab8500.dtsi
> +++ b/arch/arm/boot/dts/st/ste-href-ab8500.dtsi
> @@ -6,6 +6,48 @@
>  #include "ste-ab8500.dtsi"
>  
>  / {
> +	href_sound: sound {
> +		compatible = "audio-graph-card2";
[Severity: High]
Does this transition to audio-graph-card2 omit the creation of DAPM pin
switches that were explicitly created by the legacy machine driver?

The audio-graph-card2 driver fundamentally lacks support for generating these
switches because it does not parse a 'pin-switches' DT property (unlike
simple-audio-card), nor are they specified in the DT.

Could this lead to the loss of 20 userspace ALSA mixer controls (such as
'Headset Left Switch' and 'Mic 1 Switch'), breaking ALSA UCM profiles,
PulseAudio, and scripts that attempt to route audio or toggle endpoints?

> +		label = "HREF-500";
> +		widgets =

[ ... ]

> +&codec {
> +	assigned-clocks = <&ab8500_clock AB8500_SYSCLK_INT>;
> +	assigned-clock-parents = <&ab8500_clock AB8500_SYSCLK_ULP>;
> +
> +	ports {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		port@0 {
> +			reg = <0>;
> +
> +			href_codec_playback: endpoint {
> +				remote-endpoint = <&href_msp1_endpoint>;
> +				bitclock-master;
> +				frame-master;
> +				dai-tdm-slot-num = <16>;
> +				dai-tdm-slot-width = <16>;
> +				dai-tdm-slot-tx-mask = <0 1 0 1>;
[Severity: High]
Can statically setting dai-tdm-slot-tx-mask limit playback to exactly 2
channels (bits 1 and 3), permanently forcing stereo playback and breaking
previously supported mono playback?

Looking at ux500_msp_dai_hw_params(), the backend CPU DAI driver rigidly
enforces that the runtime ALSA channel count matches the active slots defined
statically in the DT:

sound/soc/ux500/ux500_msp_dai.c:ux500_msp_dai_hw_params() {
    ...
    if (!slots_active || params_channels(params) != slots_active)
        return -EINVAL;
    ...
}

Will userspace audio applications playing mono or capturing stereo directly via
hw: fail with -EINVAL, causing a functional regression from the legacy machine
driver?

> +			};
> +		};
> +
> +		port@1 {
> +			reg = <1>;
> +
> +			href_codec_capture: endpoint {
> +				remote-endpoint = <&href_msp3_endpoint>;
> +				bitclock-master;
> +				frame-master;
> +				dai-tdm-slot-num = <16>;
> +				dai-tdm-slot-width = <16>;
> +				dai-tdm-slot-rx-mask = <1>;
> +			};
> +		};
> +	};
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918-ux500-simple-sound-v2-0-88d29416e3c3@kernel.org?part=7

  reply	other threads:[~2026-09-17 23:06 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-17 22:50 [PATCH v2 0/9] ASoC: ux500: Migrate HREF and Samsung audio to audio-graph-card2 Linus Walleij
2026-09-17 22:50 ` [PATCH v2 1/9] ASoC: dt-bindings: sound: Prepare Ux500 audio graph links Linus Walleij
2026-09-17 22:59   ` sashiko-bot
2026-09-17 22:51 ` [PATCH v2 2/9] mfd: dt-bindings: ab8500: Describe codec graph and microphone wiring Linus Walleij
2026-09-17 22:57   ` sashiko-bot
2026-09-24 14:21   ` (subset) " Lee Jones
2026-09-17 22:51 ` [PATCH v2 3/9] ASoC: dt-bindings: ux500: Remove MOP500 sound card binding Linus Walleij
2026-09-17 22:56   ` sashiko-bot
2026-09-17 22:51 ` [PATCH v2 4/9] ASoC: ab8500: Skip digital microphone support on AB8505 Linus Walleij
2026-09-17 23:07   ` sashiko-bot
2026-09-17 22:51 ` [PATCH v2 5/9] ASoC: ux500: Remove the MOP500 machine driver Linus Walleij
2026-09-17 23:03   ` sashiko-bot
2026-09-18  8:36     ` Linus Walleij
2026-09-17 22:51 ` [PATCH v2 6/9] ARM: dts: ux500: Add sound DAI provider cells Linus Walleij
2026-09-17 22:55   ` sashiko-bot
2026-09-23 17:27   ` Linus Walleij
2026-09-24  6:54     ` Lee Jones
2026-09-24  7:00       ` Linus Walleij
2026-09-24  9:44         ` Lee Jones
2026-09-24 11:04           ` Linus Walleij
2026-09-24 14:22             ` Lee Jones
2026-09-17 22:51 ` [PATCH v2 7/9] ARM: dts: ux500: Convert HREF audio to audio-graph-card2 Linus Walleij
2026-09-17 23:06   ` sashiko-bot [this message]
2026-09-18  8:37     ` Linus Walleij
2026-09-17 22:51 ` [PATCH v2 8/9] ARM: dts: ux500: Add Samsung phone audio graphs Linus Walleij
2026-09-17 23:06   ` sashiko-bot
2026-09-17 22:51 ` [PATCH v2 9/9] ARM: config: u8500: Enable audio graph card2 Linus Walleij
2026-09-17 22:57   ` sashiko-bot
2026-09-18 17:17 ` (subset) [PATCH v2 0/9] ASoC: ux500: Migrate HREF and Samsung audio to audio-graph-card2 Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260917230646.AFDD81F000FF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=lee@kernel.org \
    --cc=linusw@kernel.org \
    --cc=mfd@lists.linux.dev \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox