Re: [PATCH v3] ALSA: usb-audio: Add support for TASCAM US-144MKII
From: Takashi Iwai
Date: Fri Aug 08 2025 - 07:48:16 EST
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