[PATCH 0/3] snd-usb-6fire: firmware load and pulseaudio assumption

René Herman rene.herman at gmail.com
Tue Jul 21 10:29:29 CEST 2020


On 21-07-2020 10:02, Takashi Iwai wrote:

> The patch needs to point to the right path that is applicable with
> patch -p1 option, i.e. it should be like
>   diff -up a/sound/usb/6fire/xxx.c b/sound/usb/6fire/xxx.c
> 
> At best use git for creating / submitting a patch.

Blast. I did, but worked against an isolated (DKMS-ed) version of the
kernel driver. Pardon. Shall re-setup against the kernel as such and
resend after dealing with your other two comments. I.e.,

Re: [PATCH 1/3] snd-usb-6fire: Move DMA-buffer off of the stack

> Could you rather change return with goto out (with ret variable
> set)? In that way we can see what actually you changed more clearly.

I already did exactly that though and in fact, the original not doing so
is what makes the patch seem involved. With the added kmalloc() I change
it so that all returns goto out, which kfree()s again. The only one that
does not is when kmalloc() fails, i.e., when there's nothing to kfree().
I suppose you just misread and do not need to have that single one go
through a goto as well?

Re: [PATCH 3/3] snd-usb-6fire: Unmark struct snd_pcm_hardware const

> This must be superfluous.  usb6fire_pcm_open() changes the field of 
> the copied pcm_hw, not the original pcm_hw itself.  Otherwise we
> must have got already a compile warning / error.

Unfortunately no; it's as mentioned in the cover letter accessed via
pointer: usb6fire_pcm_open() sets "alsa_rt->hw = pcm_hw" and then
changes pcm_hw as e.g. "alsa_rt->hw.channels_max = OUT_N_CHANNELS;".
I.e., not a copy.

And yes, it had me wonder if this was "allowed" in the first place as
well (the same pcm_hw structure is also shared between playback and
capture) but it does seem to work fine, so just unmarking it const seems
a minimal fix or, better, "cleanup", since nothing complained, but the
const qualifier no doubt at least /conceptually/ means it could be an issue.

I'll re-setup against the kernel as such and wait for reply on the above
two comments to know what to re-submit.

Regards,
Rene


More information about the Alsa-devel mailing list