Re: [PATCH v2] Bluetooth: Add hci_h4p driver

[Sebastian Reichel <sre@ring0.de>]

[-- Attachment #1: Type: text/plain, Size: 3112 bytes --]

Hi,

On Mon, Dec 30, 2013 at 03:31:25PM +0100, Pali Rohár wrote:
> [...] I think that correct commit message is not needed now.

Please add patch descriptions as early as possible. Patches without
descriptions are anoying for misc. reasons. It's not too much work
to do and helps everyone to understand what's going on. Especially
if the patch gets forgotten for some reason and somebody wants to
pick it up at a later time.

Wolfram Sang gave a talk about that at 30C3 yesterday. Official
recordings of his talk are not yet ready, but the live stream
has been recorded and uploaded to youtube:

http://www.youtube.com/watch?v=DjI7IFbvU0s (starting at 14:30)

> > > diff --git a/drivers/bluetooth/Kconfig
> > > b/drivers/bluetooth/Kconfig index 11a6104..95155c3 100644
> > > --- a/drivers/bluetooth/Kconfig
> > > +++ b/drivers/bluetooth/Kconfig
> > > @@ -242,4 +242,14 @@ config BT_WILINK
> > > 
> > > 	  Say Y here to compile support for Texas Instrument's
> > > 	  WiLink7 driver into the kernel or say M to compile it as
> > > 	  module.
> > > 
> > > +
> > > +config BT_HCIH4P
> > > +	tristate "HCI driver with H4 Nokia extensions"
> > > +	depends on BT && ARCH_OMAP
> > 
> > Since then we moved away from doing hci_* prefix of drivers
> > since that is misleading. See btusb.ko, btmrvl_sdio.ko etc.
> > 
> > So this might be better named BT_NOK_H4P or BT_NOKIA_H4P and
> > the module named btnok_h4p.ko or btnokia_h4p.ko.
> > 
> > I still never understood what “p” was for.
> > 
> 
> I do not know too, I did not invent that name. I just copied
> kernel driver from nokia kernel and patched it to work with 3.12.
> 
> Maybe 'p' means plus (+) as H4+.

AFAIR there is a H4+ reference in the code, so I also guess H4P =
H4+.  Apart from that I assume, that it's meant as a short for "H4
plus some extensions"

> > Can we also make this just depend on some device tree
> > information and not on a specific architecture. I know that
> > this driver is pretty much OMAP specific, but if we want this
> > upstream, we should at least try to make it more generic.
> > 
> 
> Sebastian, can you look at code how hard is to add DT support?

I already have it on my TODO list.

> > > +MODULE_DESCRIPTION("Bluetooth h4 driver with nokia
> > > extensions"); +MODULE_LICENSE("GPL");
> > > +MODULE_AUTHOR("Ville Tervo");
> > > +MODULE_FIRMWARE(FW_NAME_TI1271_PRELE);
> > > +MODULE_FIRMWARE(FW_NAME_TI1271_LE);
> > > +MODULE_FIRMWARE(FW_NAME_TI1271);
> > > +MODULE_FIRMWARE(FW_NAME_BCM2048);
> > > +MODULE_FIRMWARE(FW_NAME_CSR);
> > 
> > Do we actually have all these firmware files still available.
> > If not, then focus on the ones we have.
> 
> Firmware files are available for download from nemo project:
> 
> https://api.merproject.org/public/source/nemo:devel:hw:ti:omap3:n900/bcm-bt-firmware/bcm-bt-firmware-0.21rc3.tar.bz2
> https://api.merproject.org/public/source/nemo:devel:hw:ti:omap3:n950-n9/ti-wl1273-bt-firmware/bt-firmware-ti1273_0.23+0m6.tar.gz

Would be nice to have them added to the linux-firmware.git.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]