[PATCH] ASoC: atmel: mchp-pdmc: Skip ALSA restoration if substream runtime is uninitialized

[PATCH] ASoC: atmel: mchp-pdmc: Skip ALSA restoration if substream runtime is uninitialized

[Andrei Simion]

Update the driver to prevent alsa-restore.service from failing when
reading data from /var/lib/alsa/asound.state at boot. Ensure that the
restoration of ALSA mixer configurations is skipped if substream->runtime
is NULL.

Fixes: 50291652af52 ("ASoC: atmel: mchp-pdmc: add PDMC driver")
Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
---
 sound/soc/atmel/mchp-pdmc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/atmel/mchp-pdmc.c b/sound/soc/atmel/mchp-pdmc.c
index 939cd44ebc8a..06dc3c48e7e8 100644
--- a/sound/soc/atmel/mchp-pdmc.c
+++ b/sound/soc/atmel/mchp-pdmc.c
@@ -302,6 +302,9 @@ static int mchp_pdmc_chmap_ctl_put(struct snd_kcontrol *kcontrol,
 	if (!substream)
 		return -ENODEV;
 
+	if (!substream->runtime)
+		return 0; /* just for avoiding error from alsactl restore */
+
 	map = mchp_pdmc_chmap_get(substream, info);
 	if (!map)
 		return -EINVAL;

base-commit: 4d0326b60bb753627437fff0f76bf1525bcda422
-- 
2.34.1

Re: [PATCH] ASoC: atmel: mchp-pdmc: Skip ALSA restoration if substream runtime is uninitialized

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 856 bytes --]

On Tue, Sep 24, 2024 at 11:12:38AM +0300, Andrei Simion wrote:

> Update the driver to prevent alsa-restore.service from failing when
> reading data from /var/lib/alsa/asound.state at boot. Ensure that the
> restoration of ALSA mixer configurations is skipped if substream->runtime
> is NULL.

> +++ b/sound/soc/atmel/mchp-pdmc.c
> @@ -302,6 +302,9 @@ static int mchp_pdmc_chmap_ctl_put(struct snd_kcontrol *kcontrol,
>  	if (!substream)
>  		return -ENODEV;
>  
> +	if (!substream->runtime)
> +		return 0; /* just for avoiding error from alsactl restore */
> +

This then means that control writes are just discarded which presumably
is going to upset things if they actually saved a value here.  Why is
that a good choice, rather than either fixing the race so the card
doesn't come up too early or removing the need for the runtime?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: atmel: mchp-pdmc: Skip ALSA restoration if substream runtime is uninitialized

[Andrei Simion]

On 24.09.2024 11:59, Mark Brown wrote:
> 
> 
> On Tue, Sep 24, 2024 at 11:12:38AM +0300, Andrei Simion wrote:
> 
>> Update the driver to prevent alsa-restore.service from failing when
>> reading data from /var/lib/alsa/asound.state at boot. Ensure that the
>> restoration of ALSA mixer configurations is skipped if substream->runtime
>> is NULL.
>> +++ b/sound/soc/atmel/mchp-pdmc.c
>> @@ -302,6 +302,9 @@ static int mchp_pdmc_chmap_ctl_put(struct snd_kcontrol *kcontrol,
>>	if (!substream)
>>		return -ENODEV;
>>
>> +	if (!substream->runtime)
>> +		return 0; /* just for avoiding error from alsactl restore */
>> +
> This then means that control writes are just discarded which presumably
> is going to upset things if they actually saved a value here.  Why is
> that a good choice, rather than either fixing the race so the card
> doesn't come up too early or removing the need for the runtime?

Ok. I understand. My first intention was to follow the
https://github.com/torvalds/linux/blob/master/sound/hda/hdmi_chmap.c#L794
but after your point of view, I intend to return -EAGAIN in V2
to specify the substream->runtime is not ready.

I retested: configured pdmc, then reboot the board and the configuration:
as a result the configuration kept.

alsa-restore.service status success.

Do you think this solution is enough?

Thank you and best regards,
Andrei Simion

Re: [PATCH] ASoC: atmel: mchp-pdmc: Skip ALSA restoration if substream runtime is uninitialized

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1334 bytes --]

On Tue, Sep 24, 2024 at 02:25:44PM +0300, Andrei Simion wrote:
> On 24.09.2024 11:59, Mark Brown wrote:
> > On Tue, Sep 24, 2024 at 11:12:38AM +0300, Andrei Simion wrote:

> > This then means that control writes are just discarded which presumably
> > is going to upset things if they actually saved a value here.  Why is
> > that a good choice, rather than either fixing the race so the card
> > doesn't come up too early or removing the need for the runtime?

> Ok. I understand. My first intention was to follow the
> https://github.com/torvalds/linux/blob/master/sound/hda/hdmi_chmap.c#L794
> but after your point of view, I intend to return -EAGAIN in V2
> to specify the substream->runtime is not ready.

> I retested: configured pdmc, then reboot the board and the configuration:
> as a result the configuration kept.

> alsa-restore.service status success.

> Do you think this solution is enough?

Ah, I'm a little surprised that we baked retries like that into the API
but if userspace already understands that then that'd be fine.  Though
now I see that the HDA code already does what your existing patch does
I'm happy to just queue this version, HDA is so widely deployed that I'd
guess everything will DTRT and it's probably best to be consistent.
Sorry, I hadn't seen these nuances of the channel map interface before.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: atmel: mchp-pdmc: Skip ALSA restoration if substream runtime is uninitialized

[Mark Brown]

On Tue, 24 Sep 2024 11:12:38 +0300, Andrei Simion wrote:
> Update the driver to prevent alsa-restore.service from failing when
> reading data from /var/lib/alsa/asound.state at boot. Ensure that the
> restoration of ALSA mixer configurations is skipped if substream->runtime
> is NULL.
> 
> 

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: atmel: mchp-pdmc: Skip ALSA restoration if substream runtime is uninitialized
      commit: 09cfc6a532d249a51d3af5022d37ebbe9c3d31f6

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark