From: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
To: Takashi Iwai <tiwai@suse.de>
Cc: alsa-devel@alsa-project.org
Subject: Re: [PATCH 2/2] ALSA: emu10k1: use high-level I/O in set_filterQ()
Date: Sun, 23 Apr 2023 10:51:38 +0200 [thread overview]
Message-ID: <ZETxmveuGo8cBqBI@ugly> (raw)
In-Reply-To: <87y1mjnj8t.wl-tiwai@suse.de> <878rejoya4.wl-tiwai@suse.de>
On Sun, Apr 23, 2023 at 09:25:39AM +0200, Takashi Iwai wrote:
>Again, you must have a bit more say here...
>For example, you didn't write why this change is needed.
>You thought it obvious? No, readers don't know.
>
it is obvious from the patch - the code becomes much shorter and more
legible. and someone who just reads the log/blame wouldn't care, because
it doesn't actually explain anything. but whatever.
On Sun, Apr 23, 2023 at 09:35:46AM +0200, Takashi Iwai wrote:
>On Sat, 22 Apr 2023 18:10:20 +0200,
>Oswald Buddenhagen wrote:
>>
>> ... and also use more pre-defined constants on the way (some of which
>> required adjustment).
>>
>> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
>
>Applied this one, but skipped the patch 2.
>
which is funny, because that commit message misses the obvious "why"
part as well - it just mentions an additional thing that is unique to
this patch.
so to be consistent, you should reject both patches and wait for an
update.
>BTW, it would be really better if we define some macro for the
>highlevel I/O definition. It's cumbersome to decode and check
>manually at review whether the conversion is correct, and it's
>error-prone.
>
yes, i considered that.
i also considered many more refactorings, and had to hold myself back -
there are enough nice-to-have patches in this series as-is.
i mean, 15 years ago it would have made sense to go crazy, but now the
hardware is a bit too obsolete to go much beyond what i actually need
for my project. i'm assuming some people outside the western sphere are
still using our scrap with linux, but we rarely hear from them, so it's
hard to know ...
regards
next prev parent reply other threads:[~2023-04-23 8:53 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-22 16:10 [PATCH 1/2] ALSA: emu10k1: use high-level I/O functions also during init Oswald Buddenhagen
2023-04-22 16:10 ` [PATCH 2/2] ALSA: emu10k1: use high-level I/O in set_filterQ() Oswald Buddenhagen
2023-04-23 7:25 ` Takashi Iwai
2023-04-23 8:51 ` Oswald Buddenhagen [this message]
2023-04-23 14:30 ` Takashi Iwai
2023-04-23 7:35 ` [PATCH 1/2] ALSA: emu10k1: use high-level I/O functions also during init Takashi Iwai
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZETxmveuGo8cBqBI@ugly \
--to=oswald.buddenhagen@gmx.de \
--cc=alsa-devel@alsa-project.org \
--cc=tiwai@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox