All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Arend van Spriel" <arend@broadcom.com>
To: "Larry Finger" <Larry.Finger@lwfinger.net>
Cc: "Andrew Morton" <akpm@linux-foundation.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"John W. Linville" <linville@tuxdriver.com>,
	"Greg Kroah-Hartman" <gregkh@suse.de>,
	"Dan Carpenter" <error27@gmail.com>
Subject: Re: [RFC V1] lib: cordic: add library module for cordic angle calculation
Date: Thu, 26 May 2011 11:30:56 +0200	[thread overview]
Message-ID: <4DDE1DD0.4000404@broadcom.com> (raw)
In-Reply-To: <4DDD6801.4010100@lwfinger.net>

On 05/25/2011 10:35 PM, Larry Finger wrote:
> On 05/25/2011 02:40 PM, Arend van Spriel wrote:
>> The brcm80211 driver in the staging tree has a cordic function to
>> determine cosine and sine for a given angle. Feedback received from
>> John Linville suggested that these kind of functions should be made
>> available to others as a library function in the kernel tree.
> There is a similar routine in b43, thus this library function will likely have
> at least 2 users.

I know (forgot to mention it in the commit message). Henry Ptasinski 
(broadcom colleague) actually verified both functions against Matlab 
implementation (or Scilab, if you prefer open-source ;-) ). The b43 
version has a more limited range of the angle input in which the 
calculation is accurate.

>> +static const s32 AtanTbl[] = {
> In Documentation/CodingStyle, variables with mixed-case names are frownid upon.

No frowning needed. I will change it ;-)

>
>> +/*
>> + * cordic_calc_iq() - calculates the i/q coordinate for given angle
>> + *
>> + * theta: angle in degrees for which i/q coordinate is to be calculated
>> + * coord: function output parameter holding the i/q coordinate
>> + */
>> +void cordic_calc_iq(s32 theta, struct cordic_iq *coord)
> As "coord" is only for output, why not have this be a function returning a
> struct cordic_iq?

Only to avoid passing the structure over the stack. Given the fact that 
it is only 8 bytes (4 bytes extra) I think it is ok to change it.

Gr. AvS

-- 
Almost nobody dances sober, unless they happen to be insane.
-- H.P. Lovecraft --



      reply	other threads:[~2011-05-26  9:31 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-25 19:40 [RFC V1] lib: cordic: add library module for cordic angle calculation Arend van Spriel
2011-05-25 20:04 ` Randy Dunlap
     [not found]   ` <4DDE0AB4.5020102@broadcom.com>
2011-05-26  8:59     ` Arend van Spriel
2011-05-26 15:28     ` Randy Dunlap
2011-05-25 20:35 ` Larry Finger
2011-05-26  9:30   ` Arend van Spriel [this message]

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=4DDE1DD0.4000404@broadcom.com \
    --to=arend@broadcom.com \
    --cc=Larry.Finger@lwfinger.net \
    --cc=akpm@linux-foundation.org \
    --cc=error27@gmail.com \
    --cc=gregkh@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linville@tuxdriver.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.