Re: [PATCH 1/2] ALSA: sscape: Cache per-card resources for board reinitialization

Cássio Gabriel Monteiro Pires
2026-04-11T14:34:00Z

[-- Attachment #1.1: Type: text/plain, Size: 2548 bytes --]

On 4/11/26 05:07, Takashi Iwai wrote:
> On Sat, 11 Apr 2026 06:54:29 +0200,
> Cássio Gabriel wrote:
>> +/*
>> + * Restore the SoundScape's MIDI control state after the firmware
>> + * upload has made the host interface available again.
>> + */
>> +static int sscape_restore_midi_state(struct soundscape *sscape)
>> +{
>> + int err = 1;
>> +
>> + guard(spinlock_irqsave)(&sscape->lock);
>> + set_host_mode_unsafe(sscape->io_base);
>> + err &= host_write_ctrl_unsafe(sscape->io_base, CMD_SET_MIDI_VOL, 100);
>> + err &= host_write_ctrl_unsafe(sscape->io_base, sscape->midi_vol, 100);
>> + err &= host_write_ctrl_unsafe(sscape->io_base, CMD_XXX_MIDI_VOL, 100);
>> + err &= host_write_ctrl_unsafe(sscape->io_base, sscape->midi_vol, 100);
>> + err &= host_write_ctrl_unsafe(sscape->io_base, CMD_SET_EXTMIDI, 100);
>> + err &= host_write_ctrl_unsafe(sscape->io_base, 0, 100);
>> + err &= host_write_ctrl_unsafe(sscape->io_base, CMD_ACK, 100);
>> + set_midi_mode_unsafe(sscape->io_base);
>> +
>> + return err ? 0 : -EIO;
>
> The above means it checks only the error of the last write.
> Is it intentional?

It was meant to accumulate failures across the whole sequence, since
host_write_ctrl_unsafe() returns 0/1 and the accumulator starts at 1.

So an earlier failed write is meant to keep the final result failed, not
only the last write. But I agree the current form is kinda easy to misread.

I can rewrite to make the cumulative error handling explicit:

static int sscape_restore_midi_state(struct soundscape *sscape)
{
bool ok = true;

guard(spinlock_irqsave)(&sscape->lock);
set_host_mode_unsafe(sscape->io_base);

if (!host_write_ctrl_unsafe(sscape->io_base, CMD_SET_MIDI_VOL, 100))
ok = false;
if (!host_write_ctrl_unsafe(sscape->io_base, sscape->midi_vol, 100))
ok = false;
if (!host_write_ctrl_unsafe(sscape->io_base, CMD_XXX_MIDI_VOL, 100))
ok = false;
if (!host_write_ctrl_unsafe(sscape->io_base, sscape->midi_vol, 100))
ok = false;
if (!host_write_ctrl_unsafe(sscape->io_base, CMD_SET_EXTMIDI, 100))
ok = false;
if (!host_write_ctrl_unsafe(sscape->io_base, 0, 100))
ok = false;
if (!host_write_ctrl_unsafe(sscape->io_base, CMD_ACK, 100))
ok = false;

set_midi_mode_unsafe(sscape->io_base);

return ok ? 0 : -EIO;
}

What you think?

--
Thanks,
Cássio

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]