Return-Path: Date: Mon, 5 Mar 2018 07:28:24 +0100 (CET) From: Stefan Wahren To: Marcel Holtmann Cc: Lukas Wunner , Rob Herring , Eric Anholt , Johan Hedberg , devicetree , Loic Poulain , Florian Fainelli , linux-rpi-kernel@lists.infradead.org, Mark Rutland , Phil Elwell , =?UTF-8?Q?Fr=C3=A9d=C3=A9ric_Danis?= , Linux Bluetooth mailing list , linux-arm-kernel@lists.infradead.org Message-ID: <1219888621.198216.1520231304685@email.1und1.de> In-Reply-To: <423BE2B6-2209-4390-B420-FA7267318A09@holtmann.org> 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> <395166283.280598.1520200683559@email.1und1.de> <423BE2B6-2209-4390-B420-FA7267318A09@holtmann.org> Subject: Re: [PATCH 3/4] ARM: dts: bcm2835-rpi-zero-w: Add bcm43438 serial slave MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 List-ID: Hi Marcel, > Marcel Holtmann hat am 5. M=C3=A4rz 2018 um 06:19 g= eschrieben: >=20 >=20 > Hi Stefan, >=20 > >>>>> Add BCM43438 (bluetooth) as a serdev slave device of uart0 (pl011/t= tyAMA0). > >>>>> This allows to automatically insert the bcm43438 to the bluetooth > >>>>> subsystem instead of relying on patched userspace helpers (hciattac= h). > >>>>>=20 > >>>>> In order to keep a debug UART we need to switch to uart1. > >>>>>=20 > >>>>> Signed-off-by: Stefan Wahren > >>>>> --- > >>>>> arch/arm/boot/dts/bcm2835-rpi-zero-w.dts | 14 +++++++++++++- > >>>>> 1 file changed, 13 insertions(+), 1 deletion(-) > >>>>>=20 > >>>>> diff --git a/arch/arm/boot/dts/bcm2835-rpi-zero-w.dts b/arch/arm/bo= ot/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 @@ > >>>>>=20 > >>>>> &uart0 { > >>>>> =09pinctrl-names =3D "default"; > >>>>> -=09pinctrl-0 =3D <&uart0_gpio14>; > >>>>> +=09pinctrl-0 =3D <&uart0_gpio32 &uart0_ctsrts_gpio30>; > >>>>> +=09status =3D "okay"; > >>>>> + > >>>>> +=09bluetooth { > >>>>> +=09=09compatible =3D "brcm,bcm43438-bt"; > >>>>> +=09=09max-speed =3D <2000000>; > >>>>> +=09=09shutdown-gpios =3D <&gpio 45 GPIO_ACTIVE_HIGH>; > >>>>> +=09}; > >>>>> +}; > >>>>=20 > >>>> is the shutdown GPIO working as expected with this hardware. So even= module unload and reload works fine? > >>>=20 > >>> Yes, unload and reload works fine.=20 > >>>=20 > >>>> Meaning we are getting back to the 115200 default baud rate on the U= ART? > >>>=20 > >>> I assume that, because reload works as expected.=20 > >>=20 > >> awesome. That is good news. > >>=20 > >> Since you said that the GPIO expander driver for the RPi 3 has been ac= cepted, 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. > >=20 > > so here comes the bad news. hci_bcm uses gpiod_set_value() which is for= interrupt context, but the gpio expander can sleep. So we will get this wa= rning: > >=20 > > [ 4.802447] ------------[ cut here ]------------ > > [ 4.802485] WARNING: CPU: 3 PID: 91 at drivers/gpio/gpiolib.c:2986 g= piod_set_value+0x50/0x58 > > [ 4.802489] Modules linked in: hci_uart ac97_bus btbcm snd_pcm_dmaen= gine bluetooth snd_pcm snd_timer ecdh_generic snd soundcore bcm2835_thermal= crc32_arm_ce > > [ 4.802543] CPU: 3 PID: 91 Comm: kworker/u8:2 Not tainted 4.16.0-rc3= -next-20180302-dirty #4 > > [ 4.802547] Hardware name: BCM2835 > > [ 4.802562] Workqueue: events_unbound async_run_entry_fn > > [ 4.802593] [] (unwind_backtrace) from [] (show_= stack+0x10/0x14) > > [ 4.802612] [] (show_stack) from [] (dump_stack+= 0x8c/0xa0) > > [ 4.802628] [] (dump_stack) from [] (__warn+0xe0= /0xf8) > > [ 4.802640] [] (__warn) from [] (warn_slowpath_n= ull+0x40/0x48) > > [ 4.802652] [] (warn_slowpath_null) from [] (gpi= od_set_value+0x50/0x58) > > [ 4.802693] [] (gpiod_set_value) from [] (bcm_gp= io_set_shutdown+0xc/0x14 [hci_uart]) > > [ 4.802757] [] (bcm_gpio_set_shutdown [hci_uart]) from [] (bcm_gpio_set_power+0x18/0x140 [hci_uart]) > > [ 4.802806] [] (bcm_gpio_set_power [hci_uart]) from [] (bcm_serdev_probe+0x64/0x9c [hci_uart]) > > [ 4.802848] [] (bcm_serdev_probe [hci_uart]) from [] (driver_probe_device+0x254/0x32c) > > [ 4.802863] [] (driver_probe_device) from [] (__= driver_attach+0xa4/0xa8) > > [ 4.802878] [] (__driver_attach) from [] (bus_fo= r_each_dev+0x74/0xb4) > > [ 4.802895] [] (bus_for_each_dev) from [] (async= _run_entry_fn+0x44/0x108) > > [ 4.802910] [] (async_run_entry_fn) from [] (pro= cess_one_work+0x200/0x4f8) > > [ 4.802923] [] (process_one_work) from [] (worke= r_thread+0x4c/0x5d4) > > [ 4.802938] [] (worker_thread) from [] (kthread+= 0x150/0x158) > > [ 4.802952] [] (kthread) from [] (ret_from_fork+= 0x14/0x2c) > > [ 4.802957] Exception stack(0xed959fb0 to 0xed959ff8) > > [ 4.802966] 9fa0: 00000000 00000= 000 00000000 00000000 > > [ 4.802977] 9fc0: 00000000 00000000 00000000 00000000 00000000 00000= 000 00000000 00000000 > > [ 4.802986] 9fe0: 00000000 00000000 00000000 00000000 00000013 00000= 000 > > [ 4.802993] ---[ end trace b947f9d4c40fa261 ]=E2=80=94 >=20 > that is weird since we should be either in a probe context or in hdev->op= en context and both can sleep as far as I know. okay in this case we can replace gpiod_set_value with gpiod_set_value_cansl= eep. Stefan >=20 > Regards >=20 > Marcel >