Hi Marcel, > -----Original Message----- > From: Marcel Holtmann [mailto:marcel@xxxxxxxxxxxx] > Sent: Friday, June 12, 2009 11:24 PM > To: Bing Zhao > Cc: linux-bluetooth@xxxxxxxxxxxxxxx > Subject: RE: [PATCH 1/4 v3] bluetooth: add btmrvl driver to support Marvell bluetooth devices > > Hi Bing, > > > > > > > This driver provides basic definitions and library functions to > > > > > > support Marvell Bluetooth enabled devices, such as 88W8688 WLAN/BT > > > > > > combo chip. > > > > > > > > > > so I created a bluetooth-mrvl-2.6 tree with your patches and already a > > > > > major collection of cleanup patches. > > > > > > > > Thanks for the cleanups. > > > > > > > > > The biggest problem right now is that your driver spits out casting > > > > > warnings like crazy. This is not acceptable. So please send me a patch > > > > > to fix these based on bluetooth-mrvl-2.6. > > > > > > > > Could you please let me know how to catch these casting warnings? I'm using GCC 4.3.2 with > sparse > > > on x86 laptop (Fedora 8) but I couldn't get the warnings. > > > > > > let me guess, you are running a 32-bit kernel. Just run a 64-bit kernel > > > and you will see them. I am using Fedora 10. > > > > Yes, I'm running 32-bit kernel. I'll try to install a 64-bit system and fix those warnings. > > seriously don't do that. There is so many 32-bit stuff in the code that > I am afraid it will fall over on 64-bit machines. I have cleaned up > almost everything, but in btmrvl_sdio.c you have some nasty alignment > thing going on. Please fix that and test it on 64-bit systems since it > looks really scary. Thanks for the code cleanup. I'm ordering a new laptop for 64-bit development/debugging. I'll work on the alignment issues once my 64-bit system is ready. > Can you explain how the firmware loading process is suppose to work. The > code looks way too complicated for what I can make out of it. We have to > make this more readable. There are two steps involving firmware downloading. 1) helper downloading; 2) firmware downloading through helper. The helper is small size of image that needs to be downloaded first. The whole image data is cut to multiple trunks which will be written (CMD53) to card consecutively. Each trunk has fixed size of 124 bytes of helper image plus 4 bytes of header (totaling 128 bytes or 2 blocks with block size 64). After all image data is downloaded an EOF block (1 block) is written to card to finish the helper downloading. The helper gets initialized afterwards. After helper is downloaded, firmware downloading process starts. The firmware image data is also cut to multiple trunks for CMD53. But the size of every trunk is not fixed. Each trunk size is determined by the helper. The driver communicates with helper to get the trunk size and then download the requested image to the card. > > The whole btmrvl driver was ported from Marvell SD8688 Bluetooth reference driver. The Enter/Leave > debug messages have been in there since very beginning. Sometimes these Enter/Leave things are useful > for customers to debug when our reference driver is ported onto their own platform/OS. > > > > > > To make it easier to upstream kernel, I'm sending a patch to remove all Enter/Leave debug messages > except for an Enter message with parameters passed in. > > I removed them by myself since your patch was breaking with my cleanups. > It is pushed now and to please test the latest bluetooth-mrvl-2.6.git > tree. Thanks. I'll pull the latest code. > > > And again, after that you are not done since the whole driver needs more > > > cleanups. That is just in a shape I consider merging it upstream so we > > > can start fixing it there. I still consider it a little bit too > > > complicated for what is really needed to drive your hardware. > > > > That's understood. Several items are remaining: > > > > 1) Proper handling of vendor commands/events > > 2) "rmmod" flag in exit_module() > > 3) more cleanups > > > > Should you have any suggestions on any of these, I'd be happy to try. > > I will work with you on them, but first we have to fix this alignment > mess. So please go through your SDIO code and run it also on a 64-bit > system. Will do. Thanks, Bing > Regards > > Marcel > -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
