Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751332AbdFFHok (ORCPT ); Tue, 6 Jun 2017 03:44:40 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:49206 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750953AbdFFHoi (ORCPT ); Tue, 6 Jun 2017 03:44:38 -0400 Date: Tue, 6 Jun 2017 09:44:32 +0200 From: Greg KH To: Felipe Balbi Cc: Ruslan Bilovol , Alan Stern , Daniel Mack , Jassi Brar , Clemens Ladisch , Jonathan Corbet , Peter Chen , Julian Scheel , "linux-usb@vger.kernel.org" , linux-doc@vger.kernel.org, "linux-kernel@vger.kernel.org" Subject: Re: [PATCH v4 0/3] USB Audio Gadget refactoring Message-ID: <20170606074432.GA10232@kroah.com> References: <1495060654-31126-1-git-send-email-ruslan.bilovol@gmail.com> <87inkevi87.fsf@linux.intel.com> <87h8zuu6uy.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87h8zuu6uy.fsf@linux.intel.com> User-Agent: Mutt/1.8.3 (2017-05-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4968 Lines: 115 On Mon, Jun 05, 2017 at 12:22:13PM +0300, Felipe Balbi wrote: > > Hi, > > Ruslan Bilovol writes: > >> Ruslan Bilovol writes: > >>> I came to this patch series when wanted to do two things: > >>> - use UAC1 as virtual ALSA sound card on gadget side, > >>> just like UAC2 is used so it's possible to do rate > >>> resampling > >>> - have both playback/capture support in UAC1 > >>> > >>> Since I wanted to have same behavior for both UAC1/UAC2, > >>> obviously I've got an utility part (u_audio.c) for > >>> virtual ALSA sound card handling like we have > >>> for ethernet(u_ether) or serial(u_serial) functions. > >>> Function-specific parts (f_uac1/f_uac2) became almost > >>> as storage for class-specific USB descriptors, some > >>> boilerplate for configfs, binding and few USB > >>> config request handling. > >>> > >>> Originally in RFC [1] I've posted before, there was > >>> major change to f_uac1 after that it couldn't do > >>> direct play to existing ALSA sound card anymore, > >>> representing audio on gadget side as virtual > >>> ALSA sound card where audio streams are simply > >>> sinked to and sourced from it, so it may break > >>> current usecase for some people (and that's why > >>> it was RFC). > >>> > >>> During RFC discussion, it was agreed to not touch > >>> existing f_uac1 implementation and create new one > >>> instead. This patchset (v4) introduced new function > >>> named f_uac1_acard and doesn't touch current f_uac1 > >>> implementation, so people still can use old behavior > >> > >> Do you have a pointer to the original RFC discussion where this was > >> discussed? If we really *must* keep the old implementation, I would > >> rather rename that to f_uac1_legacy. Still, I find it unlikely that > >> anybody will care about the old implementation. > > > > It is on LKML (which is down for me) [1] or alternative archive [2] > > > >> > >>> Now, it's possible to use existing user-space > >>> applications for audio routing between Audio Gadget > >>> and real sound card. I personally use alsaloop tool > >>> from alsautils and have ability to create PCM > >>> loopback between two different ALSA cards using > >>> rate resampling, which was not possible with previous > >>> "direct play to ALSA card" approach in f_uac1. > >> > >> this is really good result and will actually make it a lot easier for > >> testing things out. > >> > >>> While here, also dropped redundant platform > >>> driver/device creation in f_uac2 driver (as well as > >>> didn't add "never implemented" volume/mute functionality > >>> in f_uac1 to f_uac1_acard) that made this work even > >>> easier to do. > >>> > >>> This series is tested with both legacy g_audio.ko and > >>> modern configfs approaches under Ubuntu 14.04 (UAC1 and > >>> UAC2) and under Windows7 x64 (UAC1 only) having > >>> perfect results in all cases. > >>> > >>> Comments, testing are welcome. > >>> > >>> v4 changes: > >>> - renamed f_uac1_newapi to f_uac1_acard that is > >>> more meaningful > >> > >> I really don't get why you wanna keep both f_uac1 and f_uac1_acard. Why > >> do we need to maintain the old uac1 implementation? Why two separate > >> files? > > > > In first RFC ([1],[2]) I did exactly what you wrote here (removed > > old uac1 implementation and replaced it by new one) but got feedback > > that it will break things for existing f_uac1 legacy users and it's better to > > have separate implementation. > > > > I'm OK with dropping legacy f_uac1 implementation. > > > > Another idea I was thinking about is to implement simple in-kernel > > driver which will do the same as existing alsaloop tool userspace > > tool does (so legacy users will need to load two kernel modules > > and get same functionality). But this seems to be a wrong way, > > since It known that Linux kernel community doesn't like to take drivers > > with same functionality as existing userspace tools already have. > > > > So bottom line: since I'm not a legacy f_uac1 user, there is no > > difference for me how to handle it - remove legacy f_uac1 completely, > > rename it to f_uac1_legacy or add separate f_uac1_acard function. > > > > So if dropping of legacy f_uac1 implementation is OK for you, > > I can do it quickly in next patchset. > > Personally, I don't want duplicated functionality and I think the > virtual sound card approach is much better. Then again, removing > functionality we already support is kind of odd. > > Greg, Alan, what do you guys think? Do we keep a duplicated function > around or do we just tell people to rely on alsaloop? Personally, I > think we're better off with the flexibility of the virtual sound card, > what's your take? If the in-kernel driver will do more things, and we don't break the existing setups using alsaloop, then I don't see the problem, except that we now have to maintain two things :) If you don't mind the maintenance, fine with me... thanks, greg k-h