Re: [PATCH v2 5/6] ASoC: qcom: sc8280xp: Add RubikPi 3 sound card support

[Hongyang Zhao <hongyang.zhao@thundersoft.com>]

Hi Rafi,

Thank you for the review.

> On 7/15/2026 7:32 PM, Hongyang Zhao wrote:
> > RubikPi 3 connects an ES8316 codec to the primary MI2S interface for
> > headset playback and capture, and an LT9611 bridge to the quaternary
> > MI2S interface for HDMI audio.
> >
> > Add board-specific DAI data for these links. Configure both directions
> > of the primary interface for I2S with normal clock polarity and the
> > codec as bit and frame clock consumer. Program the LPASS MCLK and BCLK,
> > drive the ES8316 with its required 19.2 MHz MCLK, and enable the generic
> > codec jack-detection setup on the playback link.
> >
> > Configure the quaternary playback link with the same I2S clock format.
> > Do not request a codec sysclk for this link because the LT9611 HDMI
> > codec DAI does not implement set_sysclk().
> >
> > Select this configuration through the RubikPi 3 sound-card compatible.
> >
> > Signed-off-by: Hongyang Zhao <hongyang.zhao@thundersoft.com>
> > ---
> >   sound/soc/qcom/sc8280xp.c | 39 +++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 39 insertions(+)
> >
> > diff --git a/sound/soc/qcom/sc8280xp.c b/sound/soc/qcom/sc8280xp.c
> > index 885ec1e3e575..1354eecb56c9 100644
> > --- a/sound/soc/qcom/sc8280xp.c
> > +++ b/sound/soc/qcom/sc8280xp.c
> > @@ -410,6 +410,41 @@ static const struct snd_soc_common qcs6490_priv_data = {
> >   	}),
> >   };
> >
> > +static const struct snd_soc_common qcs6490_rubikpi3_priv_data = {
> > +	.driver_name = "qcs6490",
> > +	.dapm_widgets = sc8280xp_dapm_widgets,
> > +	.num_dapm_widgets = ARRAY_SIZE(sc8280xp_dapm_widgets),
> > +	SC8280XP_DAI_DATA(
>
> Where is SC8280XP_DAI_DATA() defined?

It is introduced by the preceding patch 4/6.

https://lore.kernel.org/linux-arm-msm/20260715-rubikpi-next-20260605-v2-4-67601d7a8581@thundersoft.com/

It initializes snd_soc_common::dai_data with an array of
struct sc8280xp_dai_data and sets num_dai_data.

Patch 4/6 reached the mailing list after your reply, so the definition
was not visible when you reviewed this patch.

>
> > +		{
> > +			.id = PRIMARY_MI2S_RX,
> > +			.codec_dai_fmt = SND_SOC_DAIFMT_I2S |
> > +					 SND_SOC_DAIFMT_NB_NF |
> > +					 SND_SOC_DAIFMT_BC_FC,
>
> codec_dai_fmt appears to be identical for all MI2S DAIs. Can this be
> moved into the common initialization path instead of being specified per
> DAI.
>
> > +			.mclk_rate = 19200000,
>
> Is this mclk frequency fixed ?
>
> Typically, audio MCLK is expected to be 12.288 MHz or 24.576 MHz.

Yes. The RubikPi 3 machine configuration intentionally sets the ES8316
sysclk to 19.2 MHz. At the fixed 48 kHz backend rate this gives an
MCLK/LRCK ratio of 400, which is explicitly supported and documented by
the ES8316 driver.

>
> > +			.codec_sysclk_set = true,
> > +			.mi2s_mclk_enable = true,
>
> Drop this mclk_enable flag; your already enabled from the codec node.
>
> is not ?

Agreed. The ES8316 codec node is now the only MCLK consumer. The Primary
MI2S RX and TX CPU backends only configure BCLK. The machine driver
still calls the ES8316 set_sysclk() operation with 19.2 MHz to set the
clock rate and sample-rate constraints, but it no longer enables MCLK
through the CPU DAI.

>
> +&i2c0 {
> +	status = "okay";
> +
> +	es8316: audio-codec@11 {
> +		compatible = "everest,es8316";
> +		reg = <0x11>;
> +		#sound-dai-cells = <0>;
> +
> +		clocks = <&q6prmcc LPASS_CLK_ID_MCLK_1 LPASS_CLK_ATTRIBUTE_COUPLE_NO>;
> +		clock-names = "mclk";
> +
>
> > +			.mi2s_bclk_enable = true,
> > +			.jack_setup = SC8280XP_JACK_SETUP_CODEC,
>
> .jack_setup is not part of snd_soc_common

jack_setup is a member of struct sc8280xp_dai_data, introduced in 4/6.
SC8280XP_DAI_DATA() places these entries in snd_soc_common::dai_data.
This was also obscured by patch 4/6 arriving after your review.

>
> Best regards,
> Rafi
>
> > +		},
> > +		{
> > +			.id = PRIMARY_MI2S_TX,
> > +			.codec_dai_fmt = SND_SOC_DAIFMT_I2S |
> > +					 SND_SOC_DAIFMT_NB_NF |
> > +					 SND_SOC_DAIFMT_BC_FC,
> > +			.mclk_rate = 19200000,
> > +			.codec_sysclk_set = true,
> > +			.mi2s_mclk_enable = true,
> > +			.mi2s_bclk_enable = true,
> > +		},
> > +		{
> > +			.id = QUATERNARY_MI2S_RX,
> > +			.codec_dai_fmt = SND_SOC_DAIFMT_I2S |
> > +					 SND_SOC_DAIFMT_NB_NF |
> > +					 SND_SOC_DAIFMT_BC_FC,
> > +		}
> > +	),
> > +};
> > +
> >   static const struct snd_soc_common qcs8275_priv_data = {
> >   	.driver_name = "qcs8300",
> >   	.dapm_widgets = sc8280xp_dapm_widgets,
> > @@ -480,6 +515,10 @@ static const struct of_device_id snd_sc8280xp_dt_match[] = {
> >   	{ .compatible = "qcom,sm8550-sndcard", .data = &sm8550_priv_data },
> >   	{ .compatible = "qcom,sm8650-sndcard", .data = &sm8650_priv_data },
> >   	{ .compatible = "qcom,sm8750-sndcard", .data = &sm8750_priv_data },
> > +	{
> > +		.compatible = "thundercomm,qcs6490-rubikpi3-sndcard",
> > +		.data = &qcs6490_rubikpi3_priv_data,
> > +	},
> >   	{}
> >   };
> >
> >

Best regards,
Hongyang