| Message ID | 20240924081237.50046-1-andrei.simion@microchip.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ASoC: atmel: mchp-pdmc: Skip ALSA restoration if substream runtime is uninitialized | expand |
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?
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
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.
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
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;
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(+) base-commit: 4d0326b60bb753627437fff0f76bf1525bcda422