[PATCH] [inline assembly] fix pcm_dmix_i386.h assembly chunk interfaces
Takashi Iwai
tiwai at suse.de
Tue May 5 15:52:19 CEST 2020
On Mon, 04 May 2020 21:25:16 +0200,
FRÉDÉRIC RECOULES wrote:
>
> Hi Takashi,
>
> I would like an update on the review process ([PATCH */6 V2] [pcm_dmix
> assembly])
>
> As a reminder, I split the changes in 6 distinct patches.
> The 3 first patches produce exactly the same binary output, so they do not
> need testing.
> The 4th one has just some minor change due to the fact that I added an
> instruction -- I am highly confident it breaks nothing.
The compile tests passed with a few different compiler set, so that's
good. But there were some issues with the patch format. IIRC, the
patch 2 couldn't be applied to the latest git tree cleanly (some space
letter issues?), so I had to manually modify it.
Also, some style issues:
- Please avoid a prefix like "[configure]" in the subject.
The prefix with "[PATCH xxx]" is good, and this should remain, but
the next prefix should be "configure:" instead. Otherwise the
prefix with the brackets are pruned at applying a patch via git-am.
- Please give more texts about why the change is done.
In all your patches, there are no explanations why you change it.
It's often more important than describing what you're changing.
For example, the patch 2 "change the token by symbolic names". Why
is this needed to be symbolic names? Write some more information in
each patch description.
- We usually use #ifdef without space between "#" and "ifdef".
Let's keep that style consistently.
> If you need I test the 2 last ones (that reduce the size of the produced
> binary), could you point me out what test I should run?
We need at least some build tests with different compiler versions and
check whether dmix actually works (not necessarily on all of them but
some of those compiled results).
> Meanwhile, my deadline comes and I would really appreciate to see the patches
> applied by wednesday night.
If you can work on the above and resubmit v3 patchset, I'll happily
apply them.
Thanks!
Takashi
More information about the Alsa-devel
mailing list