DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 59E8320825
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 8F2A0601D1
From:   Kalle Valo <kvalo@codeaurora.org>
To:     Priit Laes <plaes@plaes.org>
Cc:     Michael =?utf-8?Q?B=C3=BCsch?= <m@bues.ch>,
        Kees Cook <keescook@chromium.org>,
        Jia-Ju Bai <baijiaju1990@gmail.com>,
        Larry Finger <Larry.Finger@lwfinger.net>,
        "Gustavo A. R. Silva" <gustavo@embeddedor.com>,
        Colin Ian King <colin.king@canonical.com>,
        Arend van Spriel <arend.vanspriel@broadcom.com>,
        Varsha Rao <rvarsha016@gmail.com>,
        linux-wireless@vger.kernel.org, b43-dev@lists.infradead.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 3/3] b43: Use cordic algorithm from kernel library
References: <cover.c0b1738f3f06bbf87f4ad3200ece93322a5f135f.1542219596.git-series.plaes@plaes.org>
        <d93962cdc5a32fe99e6e5a25172920d28583aa6a.1542219596.git-series.plaes@plaes.org>
        <20181114194628.0f485637@wiggum>
        <20181117083628.sm6juvz75kiargz4@plaes.org>
Date:   Sat, 17 Nov 2018 13:06:15 +0200
In-Reply-To: <20181117083628.sm6juvz75kiargz4@plaes.org> (Priit Laes's message
        of "Sat, 17 Nov 2018 08:36:28 +0000")
Message-ID: <878t1s55g8.fsf@codeaurora.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

Priit Laes <plaes@plaes.org> writes:

> On Wed, Nov 14, 2018 at 07:46:28PM +0100, Michael B=C3=BCsch wrote:
>> On Wed, 14 Nov 2018 20:27:52 +0200
>> Priit Laes <plaes@plaes.org> wrote:
>>=20
>> > Kernel library has a common cordic algorithm which is identical
>> > to internally implementatd one, so use it and drop the duplicate
>> > implementation.
>>=20
>>=20
>> In v2 of the series it has been said that:
>>=20
>> Arend van Spriel <arend.vanspriel@broadcom.com> wrote:
>> > I recall doing a comparison between the algorithms and thought I put=20
>> > that in the original commit message. However, it is not there. It is n=
ot=20
>> > exactly the same as in b43 so there are difference for certain angles,=
=20
>> > most results are the same however. This implementation is slightly mor=
e=20
>> > accurate on the full scale.
>>=20
>>=20
>> That's not my definition of "identical".
>>=20
>> Please do not apply this patch without doing a thorough regression test
>> on actual b43 LP hardware.
>
> Indeed, there's a big discrepancy in the results of both algorithms.
>
> Here's the test script:
> https://gist.github.com/plaes/284993a4fc65e0926d0628a11f0cf874
>
> So at current state, this is self-NAK from me too and this patch should
> be dropped.

Ok, I'll drop patch 3 but keep patches 1-2 still in review. Thanks for
verifying this!

But of course it would be better if somebody could test this properly on
a real device. When that happens, please resubmit patch 3.

--=20
Kalle Valo