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

[Takashi Iwai <tiwai@suse.de>]

On Tue, 21 Jul 2020 10:29:29 +0200,
Ren9 Herman wrote:
> 
> 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

Submit a complete set freshly as a v2 patch set.

  [PATCH v2 1/3] ...

> > 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?

The first return for kmalloc error should remain so, but the rest can
be replaced with goto out.  That I meant.

> 
> 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.

Note that it copies the whole instance, not the pointer.  So the
current code is correct.


thanks,

Takashi