Re: [PATCH] ALSA: Add rate defines for 352k8 and 384k

[Takashi Iwai <tiwai@suse.de>]

On Mon, 06 Jun 2016 23:22:20 +0200,
Clive Messer wrote:
> 
> On Mon, 2016-06-06 at 22:29 +0200, Takashi Iwai wrote:
> 
> Takashi,
> 
> > I'm asking about "the current tree".  In other words, after applying
> > your patch, how many codes in my current tree can be reduced?
> 
> Well, in the current tree, the pcm5102a codec isn't enabled for 384k.
> You want me to submit a patch for doing that, using constraints and
> then re-submit my patch to add the sample rate defines and then a 3rd
> patch, removing the constraint code from the pcm5102a codec, now it
> would result in a one line change with the sample rate defines?

That would be better than nothing, really.  You submitted only a patch
changing the core part without showing the end result.  It's
unacceptable.  At least, you should have submitted with the change of
pcm5102a driver using the new definition.

> > So, please prove the cleanup results as patches, and send together
> > with your patch as a complete patchset.  Then it'll become more
> > convincing.
> 
> There must be something I am missing. I don't understand what this has
> to do with "cleanup"? I never proposed anything other than adding a
> couple of defines, for what are now becoming more common high-res
> sample rates.

Well, I assumed that your patch was intended to reduce the redundant
code we already have.  If it's only for your new code, then it's even
less convincing...

> There has been resistance to adding these defines in the past. I didn't
> understand why then, and I don't understand why now. What is it you
> need convincing of? That DXD (352k8/24) is a standard resolution? That
> 192k used to be the max fs of most DAC chips.... Time marches on....
> Now many are max fs 384k? 
> 
> The last time, 20160127, someone from Cirrus tried submitting a patch
> to add a 384k define, the thread ended with you inviting them to re-
> submit it. AFAIK, it never was re-submitted. 
> 
> http://mailman.alsa-project.org/pipermail/alsa-devel/2016-January/103506.html
> 
> And again, I don't understand why you are talking about "cleanup". I am
> not proposing to cleanup any code, only add a couple of defines to a
> header, which would result in one line changes, at the point someone
> wanted to add 352k8/384k support to an existing codec driver, rather
> than adding multiple lines of methods/code to add 352k8/384k support
> via KNOT/constraints. The point I was trying to show, with the pcm5102a
> example.

The rule is simple: if you change a core code, it must be the benefit
allover the tree.  That is, if your patch helps other multiple
drivers, it's fine, let's take it.  If your patch is only for your
own, it needs more evaluation and it's often no-go.
How is your case?  Show it by the result of patches.


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel