[PATCH v1] ARM: imx_v6_v7_defconfig: enable SND_SOC_SPDIF

[PATCH v1] ARM: imx_v6_v7_defconfig: enable SND_SOC_SPDIF

[Stefan Eichenberger]

From: Stefan Eichenberger <stefan.eichenberger@toradex.com>

Enable SND_SOC_SPDIF in imx_v6_v7_defconfig to support SPDIF audio. This
change will fix commit d469b771afe1 ("ARM: dts: imx6: update spdif sound
card node properties") which moves away from the old "spdif-controller"
property to the new "audio-codec" property.

Fixes: d469b771afe1 ("ARM: dts: imx6: update spdif sound card node properties")
Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
---
 arch/arm/configs/imx_v6_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig
index 333ef55476a30..c771d7dbcadcd 100644
--- a/arch/arm/configs/imx_v6_v7_defconfig
+++ b/arch/arm/configs/imx_v6_v7_defconfig
@@ -321,6 +321,7 @@ CONFIG_SND_SOC_IMX_SGTL5000=y
 CONFIG_SND_SOC_FSL_ASOC_CARD=y
 CONFIG_SND_SOC_AC97_CODEC=y
 CONFIG_SND_SOC_CS42XX8_I2C=y
+CONFIG_SND_SOC_SPDIF=y
 CONFIG_SND_SOC_TLV320AIC3X_I2C=y
 CONFIG_SND_SOC_WM8960=y
 CONFIG_SND_SOC_WM8962=y
-- 
2.43.0

Re: [PATCH v1] ARM: imx_v6_v7_defconfig: enable SND_SOC_SPDIF

[Shawn Guo]

On Wed, Oct 30, 2024 at 01:21:12PM +0100, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> 
> Enable SND_SOC_SPDIF in imx_v6_v7_defconfig to support SPDIF audio. This
> change will fix commit d469b771afe1 ("ARM: dts: imx6: update spdif sound
> card node properties") which moves away from the old "spdif-controller"
> property to the new "audio-codec" property.
> 
> Fixes: d469b771afe1 ("ARM: dts: imx6: update spdif sound card node properties")

It doesn't look a fix to me.

Shawn

> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> ---
>  arch/arm/configs/imx_v6_v7_defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig
> index 333ef55476a30..c771d7dbcadcd 100644
> --- a/arch/arm/configs/imx_v6_v7_defconfig
> +++ b/arch/arm/configs/imx_v6_v7_defconfig
> @@ -321,6 +321,7 @@ CONFIG_SND_SOC_IMX_SGTL5000=y
>  CONFIG_SND_SOC_FSL_ASOC_CARD=y
>  CONFIG_SND_SOC_AC97_CODEC=y
>  CONFIG_SND_SOC_CS42XX8_I2C=y
> +CONFIG_SND_SOC_SPDIF=y
>  CONFIG_SND_SOC_TLV320AIC3X_I2C=y
>  CONFIG_SND_SOC_WM8960=y
>  CONFIG_SND_SOC_WM8962=y
> -- 
> 2.43.0
> 

Re: [PATCH v1] ARM: imx_v6_v7_defconfig: enable SND_SOC_SPDIF

[Stefan Eichenberger]

Hi Shawn,

On Sat, Nov 02, 2024 at 03:21:58PM +0800, Shawn Guo wrote:
> On Wed, Oct 30, 2024 at 01:21:12PM +0100, Stefan Eichenberger wrote:
> > From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> > 
> > Enable SND_SOC_SPDIF in imx_v6_v7_defconfig to support SPDIF audio. This
> > change will fix commit d469b771afe1 ("ARM: dts: imx6: update spdif sound
> > card node properties") which moves away from the old "spdif-controller"
> > property to the new "audio-codec" property.
> > 
> > Fixes: d469b771afe1 ("ARM: dts: imx6: update spdif sound card node properties")
> 
> It doesn't look a fix to me.

I agree somehow, it was just that before the referenced commit our test
succeeds with the imx_v6_v7_defconfig and after that we get the
following error:
[   24.165534] platform sound-spdif: deferred probe pending: fsl-asoc-card: snd_soc_register_card failed

So maybe it is not a fix in the sense of a bug, but it fixes the error
message. However, I'm also fine with removing the Fixes tag.

Regards,
Stefan

Re: [PATCH v1] ARM: imx_v6_v7_defconfig: enable SND_SOC_SPDIF

[Stefan Wahren]

Hi Stefan,

Am 02.11.24 um 15:36 schrieb Stefan Eichenberger:
> Hi Shawn,
>
> On Sat, Nov 02, 2024 at 03:21:58PM +0800, Shawn Guo wrote:
>> On Wed, Oct 30, 2024 at 01:21:12PM +0100, Stefan Eichenberger wrote:
>>> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
>>>
>>> Enable SND_SOC_SPDIF in imx_v6_v7_defconfig to support SPDIF audio. This
>>> change will fix commit d469b771afe1 ("ARM: dts: imx6: update spdif sound
>>> card node properties") which moves away from the old "spdif-controller"
>>> property to the new "audio-codec" property.
>>>
>>> Fixes: d469b771afe1 ("ARM: dts: imx6: update spdif sound card node properties")
>> It doesn't look a fix to me.
> I agree somehow, it was just that before the referenced commit our test
> succeeds with the imx_v6_v7_defconfig and after that we get the
> following error:
> [   24.165534] platform sound-spdif: deferred probe pending: fsl-asoc-card: snd_soc_register_card failed
this error should have been in the commit message including the
information which platform/board is affected.

> So maybe it is not a fix in the sense of a bug, but it fixes the error
> message. However, I'm also fine with removing the Fixes tag.
But this patch doesn't look like the real approach.

Could you please clarify the impact of the regression?

Is it just this error message and audio works fine or is audio also broken?

Regards
>
> Regards,
> Stefan
>
>
>

Re: [PATCH v1] ARM: imx_v6_v7_defconfig: enable SND_SOC_SPDIF

[Stefan Eichenberger]

Hi Stefan,

On Sat, Nov 02, 2024 at 04:35:19PM +0100, Stefan Wahren wrote:
> Hi Stefan,
> 
> Am 02.11.24 um 15:36 schrieb Stefan Eichenberger:
> > Hi Shawn,
> > 
> > On Sat, Nov 02, 2024 at 03:21:58PM +0800, Shawn Guo wrote:
> > > On Wed, Oct 30, 2024 at 01:21:12PM +0100, Stefan Eichenberger wrote:
> > > > From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> > > > 
> > > > Enable SND_SOC_SPDIF in imx_v6_v7_defconfig to support SPDIF audio. This
> > > > change will fix commit d469b771afe1 ("ARM: dts: imx6: update spdif sound
> > > > card node properties") which moves away from the old "spdif-controller"
> > > > property to the new "audio-codec" property.
> > > > 
> > > > Fixes: d469b771afe1 ("ARM: dts: imx6: update spdif sound card node properties")
> > > It doesn't look a fix to me.
> > I agree somehow, it was just that before the referenced commit our test
> > succeeds with the imx_v6_v7_defconfig and after that we get the
> > following error:
> > [   24.165534] platform sound-spdif: deferred probe pending: fsl-asoc-card: snd_soc_register_card failed
> this error should have been in the commit message including the
> information which platform/board is affected.

Okay, I will add this information to the next version. We see this error
on an Apalis iMX6 which has in my variant an NXP i.MX6Q SoC.

> > So maybe it is not a fix in the sense of a bug, but it fixes the error
> > message. However, I'm also fine with removing the Fixes tag.
> But this patch doesn't look like the real approach.
> 
> Could you please clarify the impact of the regression?

So the problem is that before commit d469b771afe1 ("ARM: dts: imx6:
update spdif sound card node properties") the audio driver was
using an implementation of linux,spdif-dit and linux,spdif-dir which was
directly inside the fsl,imx-audio-spdif compatible driver. Now with the
referenced commit the idea is to use the more generic linux,spdif-dir
and linux,spdif-dit compatible drivers. That's why this driver must be
enabled in the kernel configuration.

> Is it just this error message and audio works fine or is audio also broken?

It is not just the error message, audio is not working because the
driver deferes and because it is not enabled it will never succeed to
load. I don't know if this is called a regression, because the driver is
there it is just not enabled in the imx6_v7_defconfig. I thought because
a lot of the i.MX6 based board use the generic driver, it makes sense to
enable it in the imx_v6_v7_defconfig.

Here the output of git stat to show which boards are most likely showing
the same behavior as the Apalis iMX6 with the current
imx_v6_v7_defconfig:
git show d469b771afe1 --stat
....
 arch/arm/boot/dts/nxp/imx/imx6q-cm-fx6.dts          | 15 ++++++++++++---
 arch/arm/boot/dts/nxp/imx/imx6q-prti6q.dts          | 15 ++++++++++++---
 arch/arm/boot/dts/nxp/imx/imx6q-tbs2910.dts         |  9 +++++++--
 arch/arm/boot/dts/nxp/imx/imx6qdl-apalis.dtsi       | 15 ++++++++++++---
 arch/arm/boot/dts/nxp/imx/imx6qdl-apf6dev.dtsi      |  9 +++++++--
 arch/arm/boot/dts/nxp/imx/imx6qdl-colibri.dtsi      | 15 ++++++++++++---
 arch/arm/boot/dts/nxp/imx/imx6qdl-cubox-i.dtsi      |  9 +++++++--
 arch/arm/boot/dts/nxp/imx/imx6qdl-hummingboard.dtsi |  9 +++++++--
 arch/arm/boot/dts/nxp/imx/imx6qdl-sabreauto.dtsi    |  9 +++++++--
 arch/arm/boot/dts/nxp/imx/imx6qdl-wandboard.dtsi    |  9 +++++++--
 arch/arm/boot/dts/nxp/imx/imx6sx-sabreauto.dts      |  9 +++++++--
 arch/arm/boot/dts/nxp/imx/imx6sx-sdb.dtsi           |  9 +++++++--

Regards,
Stefan

Re: [PATCH v1] ARM: imx_v6_v7_defconfig: enable SND_SOC_SPDIF

[Stefan Wahren]

Hi Stefan,

Am 04.11.24 um 09:05 schrieb Stefan Eichenberger:
> Hi Stefan,
>
> On Sat, Nov 02, 2024 at 04:35:19PM +0100, Stefan Wahren wrote:
>> Hi Stefan,
>>
>> Am 02.11.24 um 15:36 schrieb Stefan Eichenberger:
>>> Hi Shawn,
>>>
>>> On Sat, Nov 02, 2024 at 03:21:58PM +0800, Shawn Guo wrote:
>>>> On Wed, Oct 30, 2024 at 01:21:12PM +0100, Stefan Eichenberger wrote:
>>>>> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
>>>>>
>>>>> Enable SND_SOC_SPDIF in imx_v6_v7_defconfig to support SPDIF audio. This
>>>>> change will fix commit d469b771afe1 ("ARM: dts: imx6: update spdif sound
>>>>> card node properties") which moves away from the old "spdif-controller"
>>>>> property to the new "audio-codec" property.
>>>>>
>>>>> Fixes: d469b771afe1 ("ARM: dts: imx6: update spdif sound card node properties")
>>>> It doesn't look a fix to me.
>>> I agree somehow, it was just that before the referenced commit our test
>>> succeeds with the imx_v6_v7_defconfig and after that we get the
>>> following error:
>>> [   24.165534] platform sound-spdif: deferred probe pending: fsl-asoc-card: snd_soc_register_card failed
>> this error should have been in the commit message including the
>> information which platform/board is affected.
> Okay, I will add this information to the next version. We see this error
> on an Apalis iMX6 which has in my variant an NXP i.MX6Q SoC.
>
>>> So maybe it is not a fix in the sense of a bug, but it fixes the error
>>> message. However, I'm also fine with removing the Fixes tag.
>> But this patch doesn't look like the real approach.
>>
>> Could you please clarify the impact of the regression?
> So the problem is that before commit d469b771afe1 ("ARM: dts: imx6:
> update spdif sound card node properties") the audio driver was
> using an implementation of linux,spdif-dit and linux,spdif-dir which was
> directly inside the fsl,imx-audio-spdif compatible driver. Now with the
> referenced commit the idea is to use the more generic linux,spdif-dir
> and linux,spdif-dit compatible drivers. That's why this driver must be
> enabled in the kernel configuration.
>
>> Is it just this error message and audio works fine or is audio also broken?
> It is not just the error message, audio is not working because the
> driver deferes and because it is not enabled it will never succeed to
> load. I don't know if this is called a regression, because the driver is
> there it is just not enabled in the imx6_v7_defconfig. I thought because
> a lot of the i.MX6 based board use the generic driver, it makes sense to
> enable it in the imx_v6_v7_defconfig.
okay, thanks for the clarification. From my understanding
imx6_v7_defconfig is just an example config for testing. All possible
users of these boards might have their own configs and stumble across
the same issue. So I thought it would be better to add the dependency in
the Kconfig of the FSL audio driver.

I'm not that audio driver expert and don't know how the dependency
handling between the FSL audio driver and the required codecs like
SND_SOC_SPDIF. So it's possible that I'm completely wrong here and your
approach is the best we can do.

Best regards

Re: [PATCH v1] ARM: imx_v6_v7_defconfig: enable SND_SOC_SPDIF

[Stefan Eichenberger]

Hi Stefan,

On Mon, Nov 04, 2024 at 12:39:40PM +0100, Stefan Wahren wrote:
> Hi Stefan,
> 
> Am 04.11.24 um 09:05 schrieb Stefan Eichenberger:
> > Hi Stefan,
> > 
> > On Sat, Nov 02, 2024 at 04:35:19PM +0100, Stefan Wahren wrote:
> > > Hi Stefan,
> > > 
> > > Am 02.11.24 um 15:36 schrieb Stefan Eichenberger:
> > > > Hi Shawn,
> > > > 
> > > > On Sat, Nov 02, 2024 at 03:21:58PM +0800, Shawn Guo wrote:
> > > > > On Wed, Oct 30, 2024 at 01:21:12PM +0100, Stefan Eichenberger wrote:
> > > > > > From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> > > > > > 
> > > > > > Enable SND_SOC_SPDIF in imx_v6_v7_defconfig to support SPDIF audio. This
> > > > > > change will fix commit d469b771afe1 ("ARM: dts: imx6: update spdif sound
> > > > > > card node properties") which moves away from the old "spdif-controller"
> > > > > > property to the new "audio-codec" property.
> > > > > > 
> > > > > > Fixes: d469b771afe1 ("ARM: dts: imx6: update spdif sound card node properties")
> > > > > It doesn't look a fix to me.
> > > > I agree somehow, it was just that before the referenced commit our test
> > > > succeeds with the imx_v6_v7_defconfig and after that we get the
> > > > following error:
> > > > [   24.165534] platform sound-spdif: deferred probe pending: fsl-asoc-card: snd_soc_register_card failed
> > > this error should have been in the commit message including the
> > > information which platform/board is affected.
> > Okay, I will add this information to the next version. We see this error
> > on an Apalis iMX6 which has in my variant an NXP i.MX6Q SoC.
> > 
> > > > So maybe it is not a fix in the sense of a bug, but it fixes the error
> > > > message. However, I'm also fine with removing the Fixes tag.
> > > But this patch doesn't look like the real approach.
> > > 
> > > Could you please clarify the impact of the regression?
> > So the problem is that before commit d469b771afe1 ("ARM: dts: imx6:
> > update spdif sound card node properties") the audio driver was
> > using an implementation of linux,spdif-dit and linux,spdif-dir which was
> > directly inside the fsl,imx-audio-spdif compatible driver. Now with the
> > referenced commit the idea is to use the more generic linux,spdif-dir
> > and linux,spdif-dit compatible drivers. That's why this driver must be
> > enabled in the kernel configuration.
> > 
> > > Is it just this error message and audio works fine or is audio also broken?
> > It is not just the error message, audio is not working because the
> > driver deferes and because it is not enabled it will never succeed to
> > load. I don't know if this is called a regression, because the driver is
> > there it is just not enabled in the imx6_v7_defconfig. I thought because
> > a lot of the i.MX6 based board use the generic driver, it makes sense to
> > enable it in the imx_v6_v7_defconfig.
> okay, thanks for the clarification. From my understanding
> imx6_v7_defconfig is just an example config for testing. All possible
> users of these boards might have their own configs and stumble across
> the same issue. So I thought it would be better to add the dependency in
> the Kconfig of the FSL audio driver.
> 
> I'm not that audio driver expert and don't know how the dependency
> handling between the FSL audio driver and the required codecs like
> SND_SOC_SPDIF. So it's possible that I'm completely wrong here and your
> approach is the best we can do.

That might be a good point. I don't know how this is usually handled.
@Shawn and @Elinor, do you think this could be an approach to make
SND_SOC_FSL_ASOC_CARD select SND_SOC_SPDIF? It already seems to do this
for SND_SOC_WM8994 and SND_SOC_FSL_SPDIF.

Regards,
Stefan

Re: [PATCH v1] ARM: imx_v6_v7_defconfig: enable SND_SOC_SPDIF

[Elinor Montmasson]

Hi Stefan,

On Monday, 4 November, 2024 17:20:38, Stefan Eichenberger wrote:
> Hi Stefan,
> 
> On Mon, Nov 04, 2024 at 12:39:40PM +0100, Stefan Wahren wrote:
>> Hi Stefan,
>> 
>> Am 04.11.24 um 09:05 schrieb Stefan Eichenberger:
>> > Hi Stefan,
>> > 
>> > On Sat, Nov 02, 2024 at 04:35:19PM +0100, Stefan Wahren wrote:
>> > > Hi Stefan,
>> > > 
>> > > Am 02.11.24 um 15:36 schrieb Stefan Eichenberger:
>> > > > Hi Shawn,
>> > > > 
>> > > > On Sat, Nov 02, 2024 at 03:21:58PM +0800, Shawn Guo wrote:
>> > > > > On Wed, Oct 30, 2024 at 01:21:12PM +0100, Stefan Eichenberger wrote:
>> > > > > > From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
>> > > > > > 
>> > > > > > Enable SND_SOC_SPDIF in imx_v6_v7_defconfig to support SPDIF audio. This
>> > > > > > change will fix commit d469b771afe1 ("ARM: dts: imx6: update spdif sound
>> > > > > > card node properties") which moves away from the old "spdif-controller"
>> > > > > > property to the new "audio-codec" property.
>> > > > > > 
>> > > > > > Fixes: d469b771afe1 ("ARM: dts: imx6: update spdif sound card node properties")
>> > > > > It doesn't look a fix to me.
>> > > > I agree somehow, it was just that before the referenced commit our test
>> > > > succeeds with the imx_v6_v7_defconfig and after that we get the
>> > > > following error:
>> > > > [   24.165534] platform sound-spdif: deferred probe pending: fsl-asoc-card:
>> > > > snd_soc_register_card failed
>> > > this error should have been in the commit message including the
>> > > information which platform/board is affected.
>> > Okay, I will add this information to the next version. We see this error
>> > on an Apalis iMX6 which has in my variant an NXP i.MX6Q SoC.
>> > 
>> > > > So maybe it is not a fix in the sense of a bug, but it fixes the error
>> > > > message. However, I'm also fine with removing the Fixes tag.
>> > > But this patch doesn't look like the real approach.
>> > > 
>> > > Could you please clarify the impact of the regression?
>> > So the problem is that before commit d469b771afe1 ("ARM: dts: imx6:
>> > update spdif sound card node properties") the audio driver was
>> > using an implementation of linux,spdif-dit and linux,spdif-dir which was
>> > directly inside the fsl,imx-audio-spdif compatible driver. Now with the
>> > referenced commit the idea is to use the more generic linux,spdif-dir
>> > and linux,spdif-dit compatible drivers. That's why this driver must be
>> > enabled in the kernel configuration.
>> > 
>> > > Is it just this error message and audio works fine or is audio also broken?
>> > It is not just the error message, audio is not working because the
>> > driver deferes and because it is not enabled it will never succeed to
>> > load. I don't know if this is called a regression, because the driver is
>> > there it is just not enabled in the imx6_v7_defconfig. I thought because
>> > a lot of the i.MX6 based board use the generic driver, it makes sense to
>> > enable it in the imx_v6_v7_defconfig.
>> okay, thanks for the clarification. From my understanding
>> imx6_v7_defconfig is just an example config for testing. All possible
>> users of these boards might have their own configs and stumble across
>> the same issue. So I thought it would be better to add the dependency in
>> the Kconfig of the FSL audio driver.
>> 
>> I'm not that audio driver expert and don't know how the dependency
>> handling between the FSL audio driver and the required codecs like
>> SND_SOC_SPDIF. So it's possible that I'm completely wrong here and your
>> approach is the best we can do.
> 
> That might be a good point. I don't know how this is usually handled.
> @Shawn and @Elinor, do you think this could be an approach to make
> SND_SOC_FSL_ASOC_CARD select SND_SOC_SPDIF? It already seems to do this
> for SND_SOC_WM8994 and SND_SOC_FSL_SPDIF.

SND_SOC_FSL_ASOC_CARD will compile the machine driver fsl-asoc-card,
SND_SOC_FSL_SPDIF the CPU DAI driver fsl_spdif for the SPDIF
and SND_SOC_SPDIF the codec drivers spdif-rx and spdif-tx.

In my commit series I made SND_SOC_FSL_ASOC_CARD select SND_SOC_FSL_SPDIF
because the old machine driver previously compiled with SND_SOC_IMX_SPDIF
selected SND_SOC_FSL_SPDIF.
But because fsl-asoc-card is a generic driver, it could be used on a system
that doesn't have an SPDIF device, and therefore should not require
SND_SOC_SPDIF nor SND_SOC_FSL_SPDIF.
So maybe it is not a good idea to automatically select SND_SOC_FSL_SPDIF or SND_SOC_SPDIF.

On the other hand, if every imx6 or imx7 boards have an SPDIF device, then
I suppose SND_SOC_SPDIF can be put in imx_v6_v7_defconfig.

Regards,
Elinor

Re: [PATCH v1] ARM: imx_v6_v7_defconfig: enable SND_SOC_SPDIF

[Stefan Wahren]

Am 05.11.24 um 14:58 schrieb Elinor Montmasson:
> Hi Stefan,
>
> On Monday, 4 November, 2024 17:20:38, Stefan Eichenberger wrote:
>> Hi Stefan,
>>
>> On Mon, Nov 04, 2024 at 12:39:40PM +0100, Stefan Wahren wrote:
>>> Hi Stefan,
>>>
>>> Am 04.11.24 um 09:05 schrieb Stefan Eichenberger:
>>>> Hi Stefan,
>>>>
>>>> On Sat, Nov 02, 2024 at 04:35:19PM +0100, Stefan Wahren wrote:
>>>>> Hi Stefan,
>>>>>
>>>>> Am 02.11.24 um 15:36 schrieb Stefan Eichenberger:
>>>>>> Hi Shawn,
>>>>>>
>>>>>> On Sat, Nov 02, 2024 at 03:21:58PM +0800, Shawn Guo wrote:
>>>>>>> On Wed, Oct 30, 2024 at 01:21:12PM +0100, Stefan Eichenberger wrote:
>>>>>>>> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
>>>>>>>>
>>>>>>>> Enable SND_SOC_SPDIF in imx_v6_v7_defconfig to support SPDIF audio. This
>>>>>>>> change will fix commit d469b771afe1 ("ARM: dts: imx6: update spdif sound
>>>>>>>> card node properties") which moves away from the old "spdif-controller"
>>>>>>>> property to the new "audio-codec" property.
>>>>>>>>
>>>>>>>> Fixes: d469b771afe1 ("ARM: dts: imx6: update spdif sound card node properties")
>>>>>>> It doesn't look a fix to me.
>>>>>> I agree somehow, it was just that before the referenced commit our test
>>>>>> succeeds with the imx_v6_v7_defconfig and after that we get the
>>>>>> following error:
>>>>>> [   24.165534] platform sound-spdif: deferred probe pending: fsl-asoc-card:
>>>>>> snd_soc_register_card failed
>>>>> this error should have been in the commit message including the
>>>>> information which platform/board is affected.
>>>> Okay, I will add this information to the next version. We see this error
>>>> on an Apalis iMX6 which has in my variant an NXP i.MX6Q SoC.
>>>>
>>>>>> So maybe it is not a fix in the sense of a bug, but it fixes the error
>>>>>> message. However, I'm also fine with removing the Fixes tag.
>>>>> But this patch doesn't look like the real approach.
>>>>>
>>>>> Could you please clarify the impact of the regression?
>>>> So the problem is that before commit d469b771afe1 ("ARM: dts: imx6:
>>>> update spdif sound card node properties") the audio driver was
>>>> using an implementation of linux,spdif-dit and linux,spdif-dir which was
>>>> directly inside the fsl,imx-audio-spdif compatible driver. Now with the
>>>> referenced commit the idea is to use the more generic linux,spdif-dir
>>>> and linux,spdif-dit compatible drivers. That's why this driver must be
>>>> enabled in the kernel configuration.
>>>>
>>>>> Is it just this error message and audio works fine or is audio also broken?
>>>> It is not just the error message, audio is not working because the
>>>> driver deferes and because it is not enabled it will never succeed to
>>>> load. I don't know if this is called a regression, because the driver is
>>>> there it is just not enabled in the imx6_v7_defconfig. I thought because
>>>> a lot of the i.MX6 based board use the generic driver, it makes sense to
>>>> enable it in the imx_v6_v7_defconfig.
>>> okay, thanks for the clarification. From my understanding
>>> imx6_v7_defconfig is just an example config for testing. All possible
>>> users of these boards might have their own configs and stumble across
>>> the same issue. So I thought it would be better to add the dependency in
>>> the Kconfig of the FSL audio driver.
>>>
>>> I'm not that audio driver expert and don't know how the dependency
>>> handling between the FSL audio driver and the required codecs like
>>> SND_SOC_SPDIF. So it's possible that I'm completely wrong here and your
>>> approach is the best we can do.
>> That might be a good point. I don't know how this is usually handled.
>> @Shawn and @Elinor, do you think this could be an approach to make
>> SND_SOC_FSL_ASOC_CARD select SND_SOC_SPDIF? It already seems to do this
>> for SND_SOC_WM8994 and SND_SOC_FSL_SPDIF.
> SND_SOC_FSL_ASOC_CARD will compile the machine driver fsl-asoc-card,
> SND_SOC_FSL_SPDIF the CPU DAI driver fsl_spdif for the SPDIF
> and SND_SOC_SPDIF the codec drivers spdif-rx and spdif-tx.
>
> In my commit series I made SND_SOC_FSL_ASOC_CARD select SND_SOC_FSL_SPDIF
> because the old machine driver previously compiled with SND_SOC_IMX_SPDIF
> selected SND_SOC_FSL_SPDIF.
> But because fsl-asoc-card is a generic driver, it could be used on a system
> that doesn't have an SPDIF device, and therefore should not require
> SND_SOC_SPDIF nor SND_SOC_FSL_SPDIF.
> So maybe it is not a good idea to automatically select SND_SOC_FSL_SPDIF or SND_SOC_SPDIF.
>
> On the other hand, if every imx6 or imx7 boards have an SPDIF device, then
> I suppose SND_SOC_SPDIF can be put in imx_v6_v7_defconfig.
Okay, I'm fine with the original approach.
>
> Regards,
> Elinor
>

Re: [PATCH v1] ARM: imx_v6_v7_defconfig: enable SND_SOC_SPDIF

[Stefan Eichenberger]

On Tue, Nov 05, 2024 at 04:52:04PM +0100, Stefan Wahren wrote:
> Am 05.11.24 um 14:58 schrieb Elinor Montmasson:
> > Hi Stefan,
> > 
> > On Monday, 4 November, 2024 17:20:38, Stefan Eichenberger wrote:
> > > Hi Stefan,
> > > 
> > > On Mon, Nov 04, 2024 at 12:39:40PM +0100, Stefan Wahren wrote:
> > > > Hi Stefan,
> > > > 
> > > > Am 04.11.24 um 09:05 schrieb Stefan Eichenberger:
> > > > > Hi Stefan,
> > > > > 
> > > > > On Sat, Nov 02, 2024 at 04:35:19PM +0100, Stefan Wahren wrote:
> > > > > > Hi Stefan,
> > > > > > 
> > > > > > Am 02.11.24 um 15:36 schrieb Stefan Eichenberger:
> > > > > > > Hi Shawn,
> > > > > > > 
> > > > > > > On Sat, Nov 02, 2024 at 03:21:58PM +0800, Shawn Guo wrote:
> > > > > > > > On Wed, Oct 30, 2024 at 01:21:12PM +0100, Stefan Eichenberger wrote:
> > > > > > > > > From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> > > > > > > > > 
> > > > > > > > > Enable SND_SOC_SPDIF in imx_v6_v7_defconfig to support SPDIF audio. This
> > > > > > > > > change will fix commit d469b771afe1 ("ARM: dts: imx6: update spdif sound
> > > > > > > > > card node properties") which moves away from the old "spdif-controller"
> > > > > > > > > property to the new "audio-codec" property.
> > > > > > > > > 
> > > > > > > > > Fixes: d469b771afe1 ("ARM: dts: imx6: update spdif sound card node properties")
> > > > > > > > It doesn't look a fix to me.
> > > > > > > I agree somehow, it was just that before the referenced commit our test
> > > > > > > succeeds with the imx_v6_v7_defconfig and after that we get the
> > > > > > > following error:
> > > > > > > [   24.165534] platform sound-spdif: deferred probe pending: fsl-asoc-card:
> > > > > > > snd_soc_register_card failed
> > > > > > this error should have been in the commit message including the
> > > > > > information which platform/board is affected.
> > > > > Okay, I will add this information to the next version. We see this error
> > > > > on an Apalis iMX6 which has in my variant an NXP i.MX6Q SoC.
> > > > > 
> > > > > > > So maybe it is not a fix in the sense of a bug, but it fixes the error
> > > > > > > message. However, I'm also fine with removing the Fixes tag.
> > > > > > But this patch doesn't look like the real approach.
> > > > > > 
> > > > > > Could you please clarify the impact of the regression?
> > > > > So the problem is that before commit d469b771afe1 ("ARM: dts: imx6:
> > > > > update spdif sound card node properties") the audio driver was
> > > > > using an implementation of linux,spdif-dit and linux,spdif-dir which was
> > > > > directly inside the fsl,imx-audio-spdif compatible driver. Now with the
> > > > > referenced commit the idea is to use the more generic linux,spdif-dir
> > > > > and linux,spdif-dit compatible drivers. That's why this driver must be
> > > > > enabled in the kernel configuration.
> > > > > 
> > > > > > Is it just this error message and audio works fine or is audio also broken?
> > > > > It is not just the error message, audio is not working because the
> > > > > driver deferes and because it is not enabled it will never succeed to
> > > > > load. I don't know if this is called a regression, because the driver is
> > > > > there it is just not enabled in the imx6_v7_defconfig. I thought because
> > > > > a lot of the i.MX6 based board use the generic driver, it makes sense to
> > > > > enable it in the imx_v6_v7_defconfig.
> > > > okay, thanks for the clarification. From my understanding
> > > > imx6_v7_defconfig is just an example config for testing. All possible
> > > > users of these boards might have their own configs and stumble across
> > > > the same issue. So I thought it would be better to add the dependency in
> > > > the Kconfig of the FSL audio driver.
> > > > 
> > > > I'm not that audio driver expert and don't know how the dependency
> > > > handling between the FSL audio driver and the required codecs like
> > > > SND_SOC_SPDIF. So it's possible that I'm completely wrong here and your
> > > > approach is the best we can do.
> > > That might be a good point. I don't know how this is usually handled.
> > > @Shawn and @Elinor, do you think this could be an approach to make
> > > SND_SOC_FSL_ASOC_CARD select SND_SOC_SPDIF? It already seems to do this
> > > for SND_SOC_WM8994 and SND_SOC_FSL_SPDIF.
> > SND_SOC_FSL_ASOC_CARD will compile the machine driver fsl-asoc-card,
> > SND_SOC_FSL_SPDIF the CPU DAI driver fsl_spdif for the SPDIF
> > and SND_SOC_SPDIF the codec drivers spdif-rx and spdif-tx.
> > 
> > In my commit series I made SND_SOC_FSL_ASOC_CARD select SND_SOC_FSL_SPDIF
> > because the old machine driver previously compiled with SND_SOC_IMX_SPDIF
> > selected SND_SOC_FSL_SPDIF.
> > But because fsl-asoc-card is a generic driver, it could be used on a system
> > that doesn't have an SPDIF device, and therefore should not require
> > SND_SOC_SPDIF nor SND_SOC_FSL_SPDIF.
> > So maybe it is not a good idea to automatically select SND_SOC_FSL_SPDIF or SND_SOC_SPDIF.
> > 
> > On the other hand, if every imx6 or imx7 boards have an SPDIF device, then
> > I suppose SND_SOC_SPDIF can be put in imx_v6_v7_defconfig.
> Okay, I'm fine with the original approach.

Perfect thanks, then I will improve the commit message and send a v2.

Regards,
Stefan