From: Kalle Valo <kvalo@codeaurora.org>
To: <pkshih@realtek.com>
Cc: <Larry.Finger@lwfinger.net>, <linux-wireless@vger.kernel.org>,
<stable@vger.kernel.org>
Subject: Re: [PATCH v2] rtlwifi: cleanup 8723be ant_sel definition
Date: Fri, 20 Apr 2018 15:01:22 +0300 [thread overview]
Message-ID: <87zi1ygk8d.fsf@kamboji.qca.qualcomm.com> (raw)
In-Reply-To: <20180420023009.3182-1-pkshih@realtek.com> (pkshih@realtek.com's message of "Fri, 20 Apr 2018 10:30:09 +0800")
<pkshih@realtek.com> writes:
> From: Ping-Ke Shih <pkshih@realtek.com>
>
> The module parameter ant_sel is used to control antenna number and path.
> There is an existing enum ANT_{X2,X1} defined the antenna number, so
> add a new enum ANT_{MAIN,AUX} to make it readable. After this work,
> incorrect given values depend on ant_sel were exposed, so refill values
> according following definition:
> ant_sel ant_num ant_path print_label
> 1 ANT_X1 ANT_AUX #2
> 2 ANT_X2 ANT_MAIN #1
> Then, a workaround resulted from the incorrect values in halbtcoutsrc.c was
> removed. These is a existing bug in the workaround while ant_sel=2, but the
> case is rare use so user is hard to observe this bug.
>
> The experimental results with single antenna connected to specific path
> are in following:
> ant_sel ANT_MAIN(#1) ANT_AUX(#2)
> 0 -8 -62
> 1 -62 -10
> 2 -6 -60
>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> Cc: Stable <stable@vger.kernel.org> # 4.7+
> Reviewed-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
> v2: Add more description about fixed bugs in end of first paragraph.
Sorry, I still don't understand the bug you are fixing. It shouldn't
take more than one minute to understand a commit log.
I prose that you rewrite the commit log, or at least parts of if it.
When you are describing a bug don't talk about enums or source files,
that's an implementation detail, and instead talk how the bug looks like
from users point of view. For example:
"On HP laptop model 1234 with Realtex 4321 device users are supposed
to use ant_sel module parameter with value 77 to use the correct
antenna. But instead rtlwifi incorrectly chose antenna 88 with lower
transmit power and that caused packet loss. Fix it so that the user
gets better transmit power and..."
(that's of course all made up information as I don't know what the
actual bug is)
And after that you can write rtlwifi internals in the commit log as much
as you want :) But there needs to be a clear generic description of the
bug first and it needs to understandable in one read.
To make this faster I propose that you send the new commit log as a
reply to this mail and I can then comment faster.
--
Kalle Valo
next prev parent reply other threads:[~2018-04-20 12:01 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-20 2:30 [PATCH v2] rtlwifi: cleanup 8723be ant_sel definition pkshih
2018-04-20 12:01 ` Kalle Valo [this message]
2018-04-20 20:56 ` Larry Finger
2018-04-23 2:19 ` Pkshih
2018-04-23 13:47 ` Kalle Valo
2018-04-23 16:09 ` Larry Finger
2018-04-24 10:16 ` [v2] " Kalle Valo
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=87zi1ygk8d.fsf@kamboji.qca.qualcomm.com \
--to=kvalo@codeaurora.org \
--cc=Larry.Finger@lwfinger.net \
--cc=linux-wireless@vger.kernel.org \
--cc=pkshih@realtek.com \
--cc=stable@vger.kernel.org \
/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;
as well as URLs for NNTP newsgroup(s).