Re: [PATCH v3 3/3] b43: Use cordic algorithm from kernel library

[Larry Finger]

This is a multi-part message in MIME format.
On 11/19/18 5:27 AM, Priit Laes wrote:
> On Sun, Nov 18, 2018 at 01:35:57PM -0600, Larry Finger wrote:
> > On 11/18/18 2:23 AM, Priit Laes wrote:
> > > On Sat, Nov 17, 2018 at 09:31:35PM -0600, Larry Finger wrote:
> > > > On 11/14/18 12:27 PM, Priit Laes wrote:
> > > > > Kernel library has a common cordic algorithm which is identical
> > > > > to internally implementatd one, so use it and drop the duplicate
> > > > > implementation.
> >
> > My test setup has a hardware failure, thus I cannot test your patch, but I
> > now believe it to be correct. Thus your first and third patches may be
> > annotated with
> > ACKed-by: Larry Finger <Larry.Finger@xxxxxxxxxxxx>
> >
> > One thing that should be done is to fix the error in the b43 code for stable
> > as it was introduced in 2.6.34. I propose adding the attached patched to
> > your series placed between your current 2nd and 3rd patches so that the old
> > kernels get fixed. Of course, your 3rd patch will need to be revised. If all
> > 4 of the patches get submitted together there will be no problems with the
> > timing. My change will exist for seconds in the mainline kernel, but it will
> > get propagated back through stable.
>
> Thanks!
>
> >  From b42ae73ef7505de93e4c66fb9f66930e3f3d969a Mon Sep 17 00:00:00 2001
> > From: Larry Finger <Larry.Finger@xxxxxxxxxxxx>
> > Date: Sun, 18 Nov 2018 13:15:07 -0600
> > Subject: [PATCH] b43: Fix error in cordic routine
> > MIME-Version: 1.0
> > Content-Type: text/plain; charset=UTF-8
> > Content-Transfer-Encoding: 8bit
> > To: kvalo@xxxxxxxxxxxxxx
> > Cc: linux-wireless@xxxxxxxxxxxxxxx
> >
> > The cordic routine for calculating sines and cosines that was added in
> > commit 986504540306 ("b43: make cordic common (LP-PHY and N-PHY need it)")
> > contains an error whereby a quantity declared u32 can in fact go negative.
>
> It seems to be different commit though:
> commit 6f98e62a9 ("b43: update cordic code to match current specs")

Thanks for catching that mistake. I must have gotten one line off in my copy and 
paste. The respun version of my patch is attached.

I have now been able to test b43 on an LP-PHY device. I do not see any major 
changes, but there has to be some effect.

Larry
Larry