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

[Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>]

On 17/08/2017 17:21, Eric Anholt wrote:
> Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> writes:
>
>> Hi Loic,
>>
>>>>>> < 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.

Eric is correct - GPIOs 28-31 on the Pi 3B are connected to the PCM function on
the 43438. We haven't made use of those pins, and we are unlikely to. On the
Zero W, 30 and 31 are connected to the BT RTS and CTS, but on Pi 3B the flow control
pins are not connected, hence the baud rate restriction due to the risk of data
loss - particularly likely and fatal during firmware uploaded since at boot time
systemd is "aggresively parallel". This is the reason for the BlueZ patch that
defers switching to the higher baudrate until after firmware loading is complete.

Regards,

Phil