Re: [PATCH v3] ALSA: usb-audio: Add support for TASCAM US-144MKII

[Takashi Iwai <tiwai@suse.de>]

On Fri, 08 Aug 2025 13:14:06 +0200,
Šerif Rami wrote:
> 
> Hi Takashi,
> 
> Thank you for your reply and for taking the time to review my patch.
> 
> > - The sysfs file should be dropped as Greg suggested
> >
> > - Try to use guard() for spinlocks.  (I know other USB audio drivers
> >   don't do that yet, but I already have a bit patch set to convert
> >   those.)
> >
> >   Of course, there are cases where guard() and scoped_guard() don't
> >   fit well (e.g. the loop with temporary unlock/re-locking), but in
> >   most cases, you can use it well.  If it doesn't fit, it's a good
> >   chance to take a look at your code again and reconsider whether the
> >   code flow can be changed better.
> >
> > - Similarly, try to use __free(kfree) for temporary buffers.
> >
> > - snd_pcm_lib_preallocate_*() can be replaced with
> >   snd_pcm_set_managed_buffer().  Then you can drop
> >   snd_pcm_lib_malloc_pages() and snd_pcm_lib_free_pages() calls from
> >   hw_params and hw_free callbacks, too.
> >
> > - Most of enum info callbacks can be simplified with
> >   snd_ctl_enum_info() helper function.
> >
> > - It's a bit big code and it'd be great if you can split the patches
> >   in a logical manner.  But it'd be OK-ish to have a single patch if
> >   it's not easy, too.
> 
> I will implement the suggested changes.
> 
> Regarding patch splitting, since most of the driver code is new, the plan is to create two patches: one that removes the US-144MKII device binding from the existing `us122l` driver, and a second that adds the new driver files under `sound/usb/usx2y`. Please let me know if this sounds good or if you’d prefer a different approach.

I don't think this would bring so much -- the drop of the old driver
binding is just a few line of entry point, after all.
Rather splitting the new driver code in a few logical pieces might
help for reviewing.


thanks,

Takashi