Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave

[Eric Anholt <eric@anholt.net>]

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

Marcel Holtmann <marcel@holtmann.org> writes:

> Hi Eric,
>
>>>>>>> < HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) plen 1
>>>>>>>       01                                               .
>>>>>>>> HCI Event: Command Complete (0x0e) plen 4
>>>>>>>     Broadcom Write UART Clock Setting (0x3f|0x0045) ncmd 1
>>>>>>>       Status: Unknown HCI Command (0x01)
>>>>>>> 
>>>>>>> And I am seeing fun stuff like failed frame assembly.
>>>>>>> 
>>>>>>> [  888.687594] Bluetooth: hci0: BCM: chip id 94
>>>>>>> [  888.687821] Bluetooth: hci0: BCM43430A1 (001.002.009) build 0182
>>>>>>> [  892.059023] Bluetooth: hci0: Frame reassembly failed (-84)
>>>>>>> [  892.316936] Bluetooth: hci0: BCM: failed to write clock (-56)
>>>>>>> [  892.429478] Bluetooth: hci0: BCM (001.002.009) build 0182
>>>>>>> 
>>>>>>> Actually not providing the firmware makes the controller work. It however is stuck ad AA:AA:.. default address. Providing the firmware turns the address active. However then it never completes.
>>>>>> I've tried on and off to get the BT working, there seems to lots of
>>>>>> options and bits needed including some patches to the bluez [1] stuff
>>>>>> but between not quite upstream kernel bits and numerous distros all
>>>>>> doing it slightly differently I've never got it to work well.
>>>>>> 
>>>>>> The yocto [1] bits seem fairly representative of some the patches
>>>>>> flying around "to get it working" although I'm not sure how many of
>>>>>> these are actually required and how many are superfluous with this
>>>>>> patch set. There seems to be a firmware required that's not
>>>>>> distributed with linux-firmware which would also be nice to resolve.
>>>>> Non of these Yocto patches are actually needed. The culprit is the .oper_speed setting to be 4Mbps. Once you reduce that to 921600 thing will start to work smoothly. I sent a patch that takes the .oper_speed out completely and only applies it for the ACPI based devices where we know that it works.
>>>>> 
>>>>> With my patch and the right DT entries for uart0 it actually works with “btattach -B /dev/ttyAMA0 -P bcm”. It will load the firmware, configure it and head towards the right path.
>>>>> 
>>>>> Obviously btattach is only an interim step here. Loic’s patches for serdev integration and changing the DT to expose uart0 as serial-slave for Bluetooth is the right approach. Once Loic’s resends the patches we can get them into bluetooth-next and start merging these towards upstream. After that, Bluetooth should work just out of the box like with any USB dongle.
>>>>> 
>>>>> And the Yocto patches should be abandoned. If using H:5 (aka 3-Wire) instead of H:4 is possible, we could consider it, but as long as the UART wiring doesn’t cause any bit errors, it is not worth it.
>>>>> 
>>>>> That said, I do see a "Bluetooth: hci0: Frame reassembly failed (-84)” error. I need to figure out where that is. Frankly we really need to hexdump the packet when this happens.
>>>>> 
>>>> 
>>>> I also meet this Frame reassembly failure, Seems we receive a 0x00 byte from the controller (unknown pkt type).
>>> 
>>> that means adding something like this will silence it.
>>> 
>>> +#define BCM_RECV_NULL \
>>> +       .type = 0x00, \
>>> +       .hlen = 0, \
>>> +       .loff = 0, \
>>> +       .lsize = 0, \
>>> +       .maxlen = 0
>>> +
>>> +static int bcm_recv_null(struct hci_dev *hdev, struct sk_buff *skb)
>>> +{
>>> +       kfree_skb(skb);
>>> +       return 0;
>>> +}
>>> +
>>> static const struct h4_recv_pkt bcm_recv_pkts[] = {
>>>        { H4_RECV_ACL,      .recv = hci_recv_frame },
>>>        { H4_RECV_SCO,      .recv = hci_recv_frame },
>>>        { H4_RECV_EVENT,    .recv = hci_recv_frame },
>>>        { BCM_RECV_LM_DIAG, .recv = hci_recv_diag  },
>>> +       { BCM_RECV_NULL,    .recv = bcm_recv_null  },
>>> };
>>> 
>>> It does silence it indeed. Maybe this is some of their markers to
>>> ensure that the baud rate change worked. Or it is the indication that
>>> the reset completed. Do you happen to know between which commands we
>>> receive it?
>>> 
>>>> Regarding the speed, I'm unable to reach 3Mbps, I selected 921600
>>>> because this is the baudrate used by raspbian bt script.
>>> 
>>> That is the same I experienced. The 921600 works fine, but trying
>>> 3Mbps doesn’t. However it seems with hciattach you can crank it up to
>>> 3Mpbs somehow. Maybe the delay is just to short for the UART switching
>>> in that case.
>>> 
>>>> Maybe they had some issues at higher speed (empirical value ?),
>>>> However 2Mbps seems ok on my side (need to double check/adjust).
>>> 
>>> We have to make it a DT max-speed param anyway. So I am not that
>>> worried.
>>> 
>>>> In a second step, need to check If we can use hardware flow control,
>>>> I heard that pin 16&17 are routed to the bcm RTS/CTS.  Since there is
>>>> no DMA usage, it could help.
>> 
>> The BT's CTS/RTS are tied to 3v3/ground.
>> 
>>> Eric, any chance you can dig out the recommendation for the Bluetooth
>>> controller usage? I would also like to see the recommended PCM
>>> settings for this hardware. It bet it is somehow wired up to the sound
>>> controller.
>> 
>> BT's PCM is connected to gpio 28-31 on the pi3, which you can pinmux to
>> the i2s controller (pcm_gpio28 pin group).
>> 
>> I'm not clear on what you're asking for, or where I would go to find
>> anything else.  However, for integration stuff on the board, Phil, Dom,
>> and Gordon at RPi are likely to know more.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/bluetooth/btbcm.h
>
> We have to send Set Sleepmode, Set PCM Int Params and Set PCM Format
> Params commands via Bluetooth HCI that will configure the sleep
> behavior and PCM parameters. So even if you route it via GPIO 28-31,
> you still need to tell the controller what parameters are used there.

Yeah, no clue about those.

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