Re: [RFC][PATCH 0/8] ALSA: dice: constrain PCM substreams to current sampling transfer frequency

[Takashi Iwai <tiwai@suse.de>]

On Thu, 19 Nov 2015 04:34:56 +0100,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> On Nov 18 2015 23:13, Takashi Iwai wrote:
> > On Sun, 15 Nov 2015 10:25:28 +0100,
> > Takashi Sakamoto wrote:
> >>
> >> Hi,
> >>
> >> This patchset adds a constrain to ALSA dice driver to start PCM
> >> substreams and AMDTP packet transferring just at current sampling
> >> transfer frequency.
> >>
> >> Dice hardware doesn't allow drivers to get supported combinations
> >> between sampling rate and PCM channels. ALSA dice driver should follow
> >> to the hardware design, though current ALSA driver has some
> >> over-specifications. As a result, the driver has several issue and
> >> brings inconvenience to users.
> >>
> >> This patchset consists of two parts:
> >>   * 01-05: to add constrain to current sampling transfer frequency and related
> >> 	  code cleanup
> >>   * 06-08: to ensure and stabilize AMDTP packet transmission
> >>
> >> As a result, userspace applications can request PCM substreams at current
> >> sampling transfer frequency. Therefore, when users want to start PCM
> >> substreams at different rate, they should set the rate in advance by the
> >> other ways (i.e. ffado-dbus-server/ffado-mixer).
> >
> > This sounds rather like a step backward.  Why can't the driver set the
> > rate if it's the only first and exclusive user?
> 
> I think you misunderstanding about the main reason of this patch.
> 
> Users are allowed to set their favorite sampling rate to Dice device. 
> But it should be achieved by ways except for PCM functionality in ALSA 
> dice driver, due to some reasons.

By which reasons?  And may this be seen as a regression?

> Which part of this patchset should I explain? I described all of the 
> reasons in each patch. Due to Dice framewotk, due to the design of unit 
> drivers in Linux FireWire subsystem, due to basic principle for driver 
> developers, and so on.

Well, the biggest missing piece is the information about the usage.
Most of commit messages are spent for the technical details in the
code you're changing.  It's good, per se, but it alone is not enough;
why this patchset is mandatory and how it changes the world aren't
explained enough.

More specifically, although this patchset can be seen as a "bug fix",
the bug you're trying to address isn't clear.  And then, what impact
this patchset has for users isn't described enough.  If there is no
usage change, it should be mentioned clearly.  If any usage change is
expected, it must be written.


thanks,

Takashi