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

[Phil Elwell <phil@raspberrypi.org>]

On 17/08/2017 17:21, Eric Anholt wrote:
> Marcel Holtmann <marcel@holtmann.org> writes:
>
>> Hi Loic,
>>
>>>>>> < HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) ple=
n 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 018=
2
>>>>>> [  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 turn=
s 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] stu=
ff
>>>>> 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=
=2E
>>>> 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 co=
mpletely and only applies it for the ACPI based devices where we know tha=
t it works.
>>>>
>>>> With my patch and the right DT entries for uart0 it actually works w=
ith =93btattach -B /dev/ttyAMA0 -P bcm=94. It will load the firmware, con=
figure it and head towards the right path.
>>>>
>>>> Obviously btattach is only an interim step here. Loic=92s patches fo=
r serdev integration and changing the DT to expose uart0 as serial-slave =
for Bluetooth is the right approach. Once Loic=92s resends the patches we=
 can get them into bluetooth-next and start merging these towards upstrea=
m. After that, Bluetooth should work just out of the box like with any US=
B 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 UAR=
T wiring doesn=92t cause any bit errors, it is not worth it.
>>>>
>>>> That said, I do see a "Bluetooth: hci0: Frame reassembly failed (-84=
)=94 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 by=
te from the controller (unknown pkt type).
>>
>> that means adding something like this will silence it.
>>
>> +#define BCM_RECV_NULL \
>> +       .type =3D 0x00, \
>> +       .hlen =3D 0, \
>> +       .loff =3D 0, \
>> +       .lsize =3D 0, \
>> +       .maxlen =3D 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[] =3D {
>>         { H4_RECV_ACL,      .recv =3D hci_recv_frame },
>>         { H4_RECV_SCO,      .recv =3D hci_recv_frame },
>>         { H4_RECV_EVENT,    .recv =3D hci_recv_frame },
>>         { BCM_RECV_LM_DIAG, .recv =3D hci_recv_diag  },
>> +       { BCM_RECV_NULL,    .recv =3D 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=92t. However it seems with hciattach you can crank it up t=
o
>> 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 funct=
ion 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 f=
low control
pins are not connected, hence the baud rate restriction due to the risk o=
f data
loss - particularly likely and fatal during firmware uploaded since at bo=
ot 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 c=
omplete.

Regards,

Phil