linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Tony Cho <tony.cho@atmel.com>
Cc: devel@driverdev.osuosl.org, linux-wireless@vger.kernel.org,
	johnny.kim@atmel.com, chris.park@atmel.com, rachel.kim@atmel.com,
	glen.lee@atmel.com, leo.kim@atmel.com, jude.lee@atmel.com,
	robin.hwang@atmel.com, austin.shin@atmel.com,
	adel.noureldin@atmel.com, adham.abozaeid@atmel.com,
	Nicolas.FERRE@atmel.com
Subject: Re: [PATCH 3/5] staging: wilc1000: use id value as argument
Date: Wed, 2 Sep 2015 18:33:31 -0700	[thread overview]
Message-ID: <20150903013331.GA19804@kroah.com> (raw)
In-Reply-To: <1440055974-9998-4-git-send-email-tony.cho@atmel.com>

On Thu, Aug 20, 2015 at 04:32:52PM +0900, Tony Cho wrote:
> From: Johnny Kim <johnny.kim@atmel.com>
> 
> The driver communicates with the chipset via the address of handlers
> to distinguish async data frame. The SendConfigPkt function gets the
> pointer address indicating the handlers as the last argument, but this
> requires redundant typecasting and does not support the 64 bit machine.
> 
> This patch adds the function which assigns ID values instead of pointer
> representing the driver handler to the address and then uses the ID
> instead of pointer as the last argument of SendConfigPkt. The driver
> also gets the handler's address from the ID in the data frame when it
> receives them.
> 

I don't understand this code at all.  You are randomly adding values to
a list, and then assuming that you can use the index into that list for
some type of representation?  As this is a local list, why not just use
the real variables instead of having a list and dealing with them in
this very ackward manner?

In other words, I don't see the need for the list at all, just use the
real types here, you have all the needed information (hint, if you know
the index, you really know the data as well...)

thanks,

greg k-h

  reply	other threads:[~2015-09-03  1:33 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-20  7:32 [PATCH 0/5] staging: wilc1000: support 64bit machine and remove warnings Tony Cho
2015-08-20  7:32 ` [PATCH 1/5] staging: wilc1000: use the real data type Tony Cho
2015-08-20  7:32 ` [PATCH 2/5] staging: wilc1000: add an argument for Handle_SetWfiDrvHandler Tony Cho
2015-08-20  7:32 ` [PATCH 3/5] staging: wilc1000: use id value as argument Tony Cho
2015-09-03  1:33   ` Greg KH [this message]
     [not found]     ` <55E7EFF8.2060705@atmel.com>
2015-09-03 15:47       ` Greg KH
     [not found]         ` <55E90EED.5010502@atmel.com>
2015-09-04  3:51           ` Greg KH
     [not found]             ` <55E933D3.4060302@atmel.com>
2015-09-04 16:24               ` Greg KH
2015-09-10 11:34                 ` Shin, Austin
2015-08-20  7:32 ` [PATCH 4/5] staging: wilc1000: change address to fixed value Tony Cho
2015-08-20  7:32 ` [PATCH 5/5] staging: wilc1000: define undefined operation mode Tony Cho

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=20150903013331.GA19804@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=Nicolas.FERRE@atmel.com \
    --cc=adel.noureldin@atmel.com \
    --cc=adham.abozaeid@atmel.com \
    --cc=austin.shin@atmel.com \
    --cc=chris.park@atmel.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=glen.lee@atmel.com \
    --cc=johnny.kim@atmel.com \
    --cc=jude.lee@atmel.com \
    --cc=leo.kim@atmel.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=rachel.kim@atmel.com \
    --cc=robin.hwang@atmel.com \
    --cc=tony.cho@atmel.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 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).