From: Jes Sorensen <Jes.Sorensen@redhat.com>
To: John Heenan <john@zgus.com>
Cc: Kalle Valo <kvalo@codeaurora.org>,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] rtl8xxxu: Fix for bogus data used to determine macpower
Date: Sun, 30 Oct 2016 08:00:44 -0400 [thread overview]
Message-ID: <wrfjpomidp1v.fsf@redhat.com> (raw)
In-Reply-To: <20161030102112.GA5789@cube> (John Heenan's message of "Sun, 30 Oct 2016 20:21:12 +1000")
John Heenan <john@zgus.com> writes:
> Code tests show data returned by rtl8xxxu_read8(priv, REG_CR), used to set
> macpower, is never 0xea. It is only ever 0x01 (first time after modprobe)
> using wpa_supplicant and 0x00 thereafter using wpa_supplicant. These results
> occurs with 'Fix for authentication failure' [PATCH 1/2] in place.
>
> Whatever was returned, code tests always showed that at least
> rtl8xxxu_init_queue_reserved_page(priv);
> is always required. Not called if macpower set to true.
>
> Please see cover letter, [PATCH 0/2], for more information from tests.
>
> For rtl8xxxu-devel branch of git.kernel.org/pub/scm/linux/kernel/git/jes/linux.git
>
> Signed-off-by: John Heenan <john@zgus.com>
> ---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index f25b4df..aae05f3 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -3904,6 +3904,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
> macpower = false;
> else
> macpower = true;
> + macpower = false; // Code testing shows macpower must always be set to false to avoid failure
>
> ret = fops->power_on(priv);
> if (ret < 0) {
Sorry but this patch is neither serious nor acceptable. First of all,
hardcoding macpower like this right after an if statement is plain
wrong, second your comments violate all kernel rules.
Second, you argue this was tested using code test - on which device? Did
you test it on all rtl8xxxu based devices or just rtl8723bu?
NACK
Jes
next prev parent reply other threads:[~2016-10-30 12:00 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <cover.1477769750.git.john@zgus.com>
2016-10-30 10:20 ` [PATCH 1/2] rtl8xxxu: Fix for authentication failure John Heenan
2016-11-03 1:00 ` Larry Finger
2016-11-03 7:10 ` John Heenan
2016-11-03 15:39 ` Larry Finger
2016-10-30 10:21 ` [PATCH 2/2] rtl8xxxu: Fix for bogus data used to determine macpower John Heenan
2016-10-30 12:00 ` Jes Sorensen [this message]
2016-10-30 13:56 ` John Heenan
2016-10-30 23:02 ` Jes Sorensen
2016-11-03 8:41 ` Joe Perches
2016-11-03 15:43 ` Larry Finger
2016-11-04 13:56 ` Jes Sorensen
2016-11-03 1:00 ` Larry Finger
2016-11-03 2:58 ` David Miller
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=wrfjpomidp1v.fsf@redhat.com \
--to=jes.sorensen@redhat.com \
--cc=john@zgus.com \
--cc=kvalo@codeaurora.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=netdev@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).