Re: [PATCH 1/4] staging: wilc1000: modify data type
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
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
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
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