From: Marcel Holtmann <marcel@holtmann.org>
To: Bing Zhao <bzhao@marvell.com>
Cc: "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Subject: RE: [PATCH 1/4 v3] bluetooth: add btmrvl driver to support Marvell bluetooth devices
Date: Wed, 10 Jun 2009 09:07:23 +0200 [thread overview]
Message-ID: <1244617643.3068.26.camel@localhost.localdomain> (raw)
In-Reply-To: <477F20668A386D41ADCC57781B1F704301EDD86A59@SC-VEXCH1.marvell.com>
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.
I also have to compile this at least on one big endian machine. Will use
my Quad G5 once I get home.
> > I did fix the firmware non-sense you had in there, but I haven't had
> > time to test it with real hardware. So please do so.
>
> Thanks for the fix and it works well.
Good. I wasn't sure I did this right. Stupid driver_data is unsigned
long and that requires casting :(
> > Also I am considering to just remove all this Enter/Leave debug
> > non-sense since it makes the code really hard to read.
>
> I can remove those Enter/Leave debug, if you prefer.
Tell me what you need them for. I don't mind having the Enter part with
BT_DBG since we are using that anyway to print debug information about
the parameters we give to a function, but the Leave part is cluttering
the code. Can you just follow the style the other drivers are using.
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.
Regards
Marcel
next prev parent reply other threads:[~2009-06-10 7:07 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-06-02 21:29 [PATCH 1/4 v3] bluetooth: add btmrvl driver to support Marvell bluetooth devices Bing Zhao
2009-06-02 21:29 ` [PATCH 2/4 v3] bluetooth: btmrvl_sdio: Marvell BT-over-SDIO driver Bing Zhao
2009-06-02 21:29 ` [PATCH 3/4 v3] bluetooth: Add debugfs support to btmrvl driver Bing Zhao
2009-06-02 21:29 ` [PATCH 4/4 v3] bluetooth: Documentation for Marvell Bluetooth driver Bing Zhao
2009-06-09 15:20 ` [PATCH 1/4 v3] bluetooth: add btmrvl driver to support Marvell bluetooth devices Marcel Holtmann
2009-06-09 23:39 ` Bing Zhao
2009-06-10 7:07 ` Marcel Holtmann [this message]
2009-06-10 21:18 ` Bing Zhao
2009-06-13 6:24 ` Marcel Holtmann
2009-06-16 0:02 ` Bing Zhao
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=1244617643.3068.26.camel@localhost.localdomain \
--to=marcel@holtmann.org \
--cc=bzhao@marvell.com \
--cc=linux-bluetooth@vger.kernel.org \
/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