Return-Path: MIME-Version: 1.0 In-Reply-To: <698451580.263856.1520467358497@email.1und1.de> References: <1519567855-26105-1-git-send-email-stefan.wahren@i2se.com> <1519567855-26105-4-git-send-email-stefan.wahren@i2se.com> <84F7E645-59B4-4618-8C91-A5CD654E16A6@holtmann.org> <648118289.71086.1519593373795@email.1und1.de> <9D6209F2-5389-4F4E-844D-6A8683044F88@holtmann.org> <438185389.274708.1520422108782@email.1und1.de> <03B9ACBE-3ACA-40C4-9DA2-6A57D1AD96E0@holtmann.org> <2071508922.276811.1520426568577@email.1und1.de> <87sh9bvh3r.fsf@anholt.net> <87muzjfr8l.fsf@anholt.net> <698451580.263856.1520467358497@email.1und1.de> From: Loic Poulain Date: Thu, 8 Mar 2018 10:14:59 +0100 Message-ID: Subject: Re: [PATCH 3/4] ARM: dts: bcm2835-rpi-zero-w: Add bcm43438 serial slave To: Stefan Wahren Cc: Marcel Holtmann , Eric Anholt , Lukas Wunner , Rob Herring , Florian Fainelli , linux-rpi-kernel@lists.infradead.org, Mark Rutland , Phil Elwell , Johan Hedberg , =?UTF-8?B?RnLDqWTDqXJpYyBEYW5pcw==?= , devicetree , Linux Bluetooth mailing list , Loic Poulain , linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="UTF-8" List-ID: Hi Stefan, On 8 March 2018 at 01:02, Stefan Wahren wrote: > Hi, > >> Eric Anholt hat am 7. M=C3=A4rz 2018 um 23:36 geschrie= ben: >> >> >> Marcel Holtmann writes: >> >> > Hi Eric, >> > >> >>>>>>>>> Add BCM43438 (bluetooth) as a serdev slave device of uart0 (pl= 011/ttyAMA0). >> >>>>>>>>> This allows to automatically insert the bcm43438 to the blueto= oth >> >>>>>>>>> subsystem instead of relying on patched userspace helpers (hci= attach). >> >>>>>>>>> >> >>>>>>>>> In order to keep a debug UART we need to switch to uart1. >> >>>>>>>>> >> >>>>>>>>> Signed-off-by: Stefan Wahren >> >>>>>>>>> --- >> >>>>>>>>> arch/arm/boot/dts/bcm2835-rpi-zero-w.dts | 14 +++++++++++++- >> >>>>>>>>> 1 file changed, 13 insertions(+), 1 deletion(-) >> >>>>>>>>> >> >>>>>>>>> diff --git a/arch/arm/boot/dts/bcm2835-rpi-zero-w.dts b/arch/a= rm/boot/dts/bcm2835-rpi-zero-w.dts >> >>>>>>>>> index cf53436..b7f79f1 100644 >> >>>>>>>>> --- a/arch/arm/boot/dts/bcm2835-rpi-zero-w.dts >> >>>>>>>>> +++ b/arch/arm/boot/dts/bcm2835-rpi-zero-w.dts >> >>>>>>>>> @@ -131,6 +131,18 @@ >> >>>>>>>>> >> >>>>>>>>> &uart0 { >> >>>>>>>>> pinctrl-names =3D "default"; >> >>>>>>>>> - pinctrl-0 =3D <&uart0_gpio14>; >> >>>>>>>>> + pinctrl-0 =3D <&uart0_gpio32 &uart0_ctsrts_gpio30>; >> >>>>>>>>> + status =3D "okay"; >> >>>>>>>>> + >> >>>>>>>>> + bluetooth { >> >>>>>>>>> + compatible =3D "brcm,bcm43438-bt"; >> >>>>>>>>> + max-speed =3D <2000000>; >> >>>>>>>>> + shutdown-gpios =3D <&gpio 45 GPIO_ACTIVE_HIGH>; >> >>>>>>>>> + }; >> >>>>>>>>> +}; >> >>>>>>>> >> >>>>>>>> is the shutdown GPIO working as expected with this hardware. So= even module unload and reload works fine? >> >>>>>>> >> >>>>>>> Yes, unload and reload works fine. >> >>>>>>> >> >>>>>>>> Meaning we are getting back to the 115200 default baud rate on = the UART? >> >>>>>>> >> >>>>>>> I assume that, because reload works as expected. >> >>>>>> >> >>>>>> awesome. That is good news. >> >>>>>> >> >>>>>> Since you said that the GPIO expander driver for the RPi 3 has be= en accepted, did you test it there as well? If so, then it would be good to= get a patch that also provides shutdown-gpios for RPi 3. >> >>>>> >> >>>>> after applying Loic's patch and the necessary patch for the RPi 3 = dts file (see below), i will get this output: >> >>>>> >> >>>>> [ 4.873246] Bluetooth: HCI UART driver ver 2.3 >> >>>>> [ 4.873260] Bluetooth: HCI UART protocol H4 registered >> >>>>> [ 4.873265] Bluetooth: HCI UART protocol Three-wire (H5) regist= ered >> >>>>> [ 4.873751] Bluetooth: HCI UART protocol Broadcom registered >> >>>>> [ 4.877279] uart-pl011 3f201000.serial: no DMA platform data >> >>>>> [ 6.952382] Bluetooth: hci0: command 0xfc18 tx timeout >> >>>>> [ 15.192298] Bluetooth: hci0: BCM: failed to write update baudra= te (-110) >> >>>>> [ 15.192312] Bluetooth: hci0: Failed to set baudrate >> >>>>> [ 15.316415] Bluetooth: hci0: BCM: chip id 94 >> >>>>> [ 15.318567] Bluetooth: hci0: BCM: features 0x2e >> >>>>> [ 15.341538] Bluetooth: hci0: BCM43430A1 >> >>>>> [ 15.341560] Bluetooth: hci0: BCM43430A1 (001.002.009) build 000= 0 >> >>>>> [ 19.112670] Bluetooth: hci0: BCM (001.002.009) build 0360 >> >>>>> [ 274.713732] Bluetooth: hci0: command 0x0c14 tx timeout >> >>>>> [ 274.714085] Bluetooth: hci0: Frame reassembly failed (-84) >> >>>>> [ 317.275941] Bluetooth: hci0: last event is not cmd complete (0x= 0f) >> >>>>> >> >>>>> I don't see these errors on RPi Zero W. Maybe the reason for this = is the lack of hardware flowcontrol on RPi 3. Or some of the downstream pat= ches on BlueZ must be adapted for the kernel [1]. >> >>>>> >> >>>>> Btw the bcm43438 is detected even after unloading and reloading th= e driver. But the timeout occurs also on driver reload. Reducing the baudra= te to 115200 doesn't help here. >> >>>> >> >>>> maybe it needs some time after switching the hardware on. Have you = tried to sleep for a bit at the end of bcm_gpio_set_power? >> >>> >> >>> I will try, but this could also be an "issue" of the gpio expander >> >>> driver. AFAIK the expander is connected via I2C to the GPU. In case >> >>> the mailbox reply come before the I2C command has been handled by th= e >> >>> GPIO expander, we are running out of sync. >> >> >> >> The mailbox command for setting GPIO state is synchronous in terms of >> >> sending the i2c command to the GPIO expander HW. >> > >> > does that mean the driver does this correctly and really waits for the >> > completion. Or does the GPIO expander might return to quickly and >> > returns a false sense of GPIO state. I think it would be good a sleep >> > of 50ms after the GPIO makes a difference. Maybe the GPIO expander >> > driver needs to take this into account. >> >> It seems unlikely that you can get to I2C stop before the GPIO expander >> HW has changed the state of the output. >> >> > Reality is that once the Bluetooth driver toggled the GPIO, it goes >> > onto initializing the chip. One other reason might be that no device >> > wakeup GPIO is wired here, that the chip actually needs some time >> > before it is all good to go. >> >> This seems much more likely to me. The spec for bcm43438 is easily >> googleable and has a power-on timing diagram. > > after applying this patch [1] the RPi 3 specific probing errors disappear= . > > But this is only a quick hack. The proper solution would be to extend hci= _h5 in order to support the BCM43438. > > [1] - https://github.com/lategoodbye/rpi-zero/commit/ed5900296dfb7aec7f46= 7477440751e7367a1881 I think this patch is perfectly acceptable, this is an hardware timing the driver has to handle. For sure H5 would improve robustness... Regards, Loic