Re: [PATCH 1/4] staging: wilc1000: modify data type

2015-06-11 Thread Greg KH
On Thu, Jun 11, 2015 at 02:33:39PM +0900, Johnny Kim wrote:
 
 On 2015년 06월 11일 09:40, Greg KH wrote:
 On Wed, Jun 10, 2015 at 05:06:44PM +0900, Johnny Kim wrote:
 -static int wilc_wlan_cfg_commit(int type, uint32_t drvHandler)
 +static int wilc_wlan_cfg_commit(int type, size_t drvHandler)
 Shouldn't this just be a void *?  And if so, why not the real function
 pointer instead?
 
 Also, lots of these seem to be just pointers, please use a pointer
 instead of hiding it in a size_t as that's the most portable, and
 correct, way to do it.  No need to hide any function pointers here, this
 isn't Windows :)
 
 thanks,
 
 greg k-h
 
 To replace the integer to the pointer as your counsel, I need a lot of
 discussion internally.

Why internally?  Why not here in the development community?

 But I will fix the type and the related things like your thinking.

thank you.

 I know there is the build warning for 64-bit machine and this driver stays
 in BROKEN status on linux-next.
 I want to know if the reason is 64bit build warning.

Probably, fix them up and then we can see about getting that broken
status removed.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] staging: wilc1000: modify data type

2015-06-10 Thread Dan Carpenter
On Wed, Jun 10, 2015 at 05:06:44PM +0900, Johnny Kim wrote:
 This replace the argument of the function to the independent data type
 in system.
 
 Signed-off-by: Johnny Kim johnny@atmel.com

This changelog says what you are doing but not why.

What is the point of -u32Address?  Now that it's not 32 bits, the name
makes no sense.  Why not just make it a pointer?

There was someone changing all the datatypes with sed.  In some ways,
those patches were easier to review because they were mindless and not
really expected to make sense.  After we just sed the code to make it
look more normal then we can think about 64 bit bugs (like this patch
and the next).

regards,
dan carpenter

--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] staging: wilc1000: modify data type

2015-06-10 Thread Greg KH
On Wed, Jun 10, 2015 at 05:06:44PM +0900, Johnny Kim wrote:
 -static int wilc_wlan_cfg_commit(int type, uint32_t drvHandler)
 +static int wilc_wlan_cfg_commit(int type, size_t drvHandler)

Shouldn't this just be a void *?  And if so, why not the real function
pointer instead?

Also, lots of these seem to be just pointers, please use a pointer
instead of hiding it in a size_t as that's the most portable, and
correct, way to do it.  No need to hide any function pointers here, this
isn't Windows :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] staging: wilc1000: modify data type

2015-06-10 Thread Johnny Kim


On 2015년 06월 11일 09:40, Greg KH wrote:

On Wed, Jun 10, 2015 at 05:06:44PM +0900, Johnny Kim wrote:

-static int wilc_wlan_cfg_commit(int type, uint32_t drvHandler)
+static int wilc_wlan_cfg_commit(int type, size_t drvHandler)

Shouldn't this just be a void *?  And if so, why not the real function
pointer instead?

Also, lots of these seem to be just pointers, please use a pointer
instead of hiding it in a size_t as that's the most portable, and
correct, way to do it.  No need to hide any function pointers here, this
isn't Windows :)

thanks,

greg k-h


To replace the integer to the pointer as your counsel, I need a lot of
discussion internally. But I will fix the type and the related things
like your thinking.

I know there is the build warning for 64-bit machine and this driver stays
in BROKEN status on linux-next.
I want to know if the reason is 64bit build warning.

Thanks for your help.

Regards
Johnny
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html