Re: [PATCH] Bluetooth: hci_nokia: select BT_BCM for btbcm_set_bdaddr()

[Sebastian Reichel <sebastian.reichel@collabora.co.uk>]

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

Hi,

On Mon, Jul 24, 2017 at 07:09:08PM +0200, Marcel Holtmann wrote:
> >>>>>> The Nokia devices require the setup of its Public Bluetooth Device
> >>>>>> Address and for that it is required to depend on vendor specific
> >>>>>> commands. For Broadcom based Nokia devices, that is part of btbcm
> >>>>>> module and can be selected via BT_BCM config option.
> >>>>>> 
> >>>>>> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> >>>>> 
> >>>>> Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> >>>> 
> >>>> btw. we need to also provide the set_bdaddr for TI chips.
> >>> 
> >>> The TI based chip in Nokia N950 has a proper address configured, so
> >>> it works without set_bdaddr (it would still be nice of course). It
> >>> also seems to be missing for hci_ll, though. So the required command
> >>> is completly unknown?
> >> 
> >> are these a TI address or a Nokia address? I doubt Nokia would
> >> have actually shipped any hardware using an IEEE assigned address
> >> to TI.
> > 
> > I can check this later.
> > 
> >> Where do we have the firmware files for the Nokia devices?
> > 
> > For N950 I extracted it out of the following rpm:
> > 
> > http://repo.merproject.org/obs/nemo:/devel:/hw:/ti:/omap3:/n950-n9/latest_armv7hl/src/ti-wl1273-bt-firmware-0.23-1.2.Nemo.Adaptation.N950.N9.src.rpm
> > 
> >> Don't they contain a command for setting the BD_ADDR.
> > 
> > I did not check, since the firmware file is not device
> > specific as far as I know.
> 
> doen't matter since the old Nokia written driver hacked it from a
> sysfs file and the address came from the secure flash area.

The sysfs "just" puts the address into platform_data for their h4p
driver. It does not take care of applying it to the hardware. That
is the job of their h4p driver. Here is the code for the sysfs file:

https://github.com/nemomobile/kernel-adaptation-n950-n9/blob/master/arch/arm/mach-omap2/board-omap-bt.c

> >> Also the original Nokia driver had code to re-write that command
> >> inline while loading the firmware.
> > 
> > The address injection happens in hardware specific code (stock driver
> > supports TI, Broadcom & CSR). In opposit to Broadcom and CSR the TI
> > specific one does not change the address:
> > 
> > https://github.com/nemomobile/kernel-adaptation-n950-n9/blob/master/drivers/bluetooth/hci_h4p/fw-ti1273.c
> 
> As far as I know they rewrite the Write BD_ADDR command of each
> vendor at the time before they send to the hardware.

You can clearly see that they modify the address in the hardware
specific firmware files for Broadcom and CSR:

https://github.com/nemomobile/kernel-adaptation-n950-n9/blob/master/drivers/bluetooth/hci_h4p/fw-bcm.c
https://github.com/nemomobile/kernel-adaptation-n950-n9/blob/master/drivers/bluetooth/hci_h4p/fw-csr.c

But there is no such code for TI (except for a comment):

/* Check if this is bd_address packet */

I think they just forgot to implement it and since the
comment has no XXX/TODO/FIXME prefix nobody noticed it.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]