2017-08-01 08:38:22

by Loic Poulain

[permalink] [raw]
Subject: [PATCH v2 1/3] dt-bindings: net: bluetooth: Add broadcom-bluetooth

Add binding document for serial bluetooth chips using
Broadcom protocol.

Signed-off-by: Loic Poulain <[email protected]>
---
v2: dt-bindings as separate patch
rebase on upcoming pi3 dts changes

.../devicetree/bindings/net/broadcom-bluetooth.txt | 29 ++++++++++++++++++++++
1 file changed, 29 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/broadcom-bluetooth.txt

diff --git a/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
new file mode 100644
index 0000000..c51ea1b
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
@@ -0,0 +1,29 @@
+Broadcom Bluetooth Chips
+---------------------
+
+This documents the binding structure and common properties for serial
+attached Broadcom devices.
+
+Serial attached Broadcom devices shall be a child node of the host UART
+device the slave device is attached to.
+
+Required properties:
+
+ - compatible: should contain one of the following:
+ * "brcm,bcm43438-bt"
+
+Optional properties:
+
+ - max-speed: see Documentation/devicetree/bindings/serial/slave-device.txt
+
+Example:
+
+&uart2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart2_pins>;
+
+ bluetooth {
+ compatible = "brcm,bcm43438-bt";
+ max-speed = <921600>;
+ };
+};
--
1.9.1


2017-08-31 19:32:56

by Frédéric Danis

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: net: bluetooth: Add broadcom-bluetooth

Hi Marcel and Loïc,

Le 23/08/2017 à 16:19, Marcel Holtmann a écrit :
>> This is already partially supported in the hci_bcm driver.
>> Today this driver registers a platform_driver(legacy/ACPI) and a
>> serdev_device_driver (new/DT).
>>
>> The platform driver retrieves the gpios and mainly uses them in pm ops.
>> Once the ACPI for serdev will be supported, this plat driver should be
>> removed.
>>
>> The serdev driver does no support this yet because I used the RPi3
>> as dev platform. But this is something we want to have as well.
> Making ACPI serdev aware and turning BTH0 into serdev nodes is really the next step. That would allow us to simplify the drivers and remove duplicate code. Do you still have access to ACPI based tablets with Broadcom chips? It would be great if you can take a stab at this.

I have access to an Asus T100TA with Broadcom chipset and I recently
started to work on the ACPI part of SerDev.
I hope to send patches for it soon.

Regards,

Frédéric Danis

2017-08-27 10:40:30

by Stefan Wahren

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: net: bluetooth: Add broadcom-bluetooth

Hi Marcel,

> Marcel Holtmann <[email protected]> hat am 27. August 2017 um 12:23 ges=
chrieben:
>=20
>=20
> Hi Stefan,
>=20
> >>>>> Thanks a lot for working on that, I've made a similar attempt a few
> >>>>> weeks ago but didn't manage to get it to work.
> >>>>>=20
> >>>>> The way it's hooked in our boards is a bit more complex though, eve=
n
> >>>>> if it could be because we're using a different part.
> >>>>>=20
> >>>>> In order to get it running we need:
> >>>>> - two clocks, called in the broadcom datasheets lpo and tcxo.
> >>>>> - three GPIOs, device wakeup, host wakeup and a shutdown GPIO (whi=
ch
> >>>>> might be the BT_ON you were discussing about)
> >>>>> - two regulators called vbat and reg-en for us (I guess they're
> >>>>> meant to power the chip, and its registers >>
> >>>>> Do you know if you're also using those? Or could it be that it's ju=
st
> >>>>> hardwired to some non-gatable crystal / regulator on the RPI?
> >>>=20
> >>> Not on Pi3, but the three gpios and the clock are pretty common for
> >>> Broadcom bt controller (cf v4 of dt-bindings patch).
> >>=20
> >> once we get the GPIO expander driver upstream, I think we also need th=
is for RPI 3 and Zero W. Right now we can just not do anything about this.
> >=20
> > AFAIK the Zero W doesn't have this GPIO expander. Would it be easier fo=
r you?
>=20
> I don=E2=80=99t understand the question. In general you want the BT_ON an=
d wakeup GPIOs to be available so that you can have good power savings. If =
they are not there, then it is always powered. It works of course as shown =
with RPI 3 where the boot loader enables the GPIOs. Which means it is like =
the current support that we have in net-next.

the question was related to the point that all SoC GPIOs should be directly=
connected to the 43438. So using Zero W for development could be easier, b=
ecause you don't need the GPIO expander driver. But generally i agree that =
it should work with RPI 3.=20

>=20
> On the other hand, the hci_bcm.c support for RPI 3 is essentially a suppo=
rt for all Broadcom devices using DT. It should be extended to support all =
sorts of boards. And once ACPI becomes serdev support, we can unify ACPI, p=
latform and DT devices into a single code path.
>=20
> Regards
>=20
> Marcel
>

2017-08-27 10:23:02

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: net: bluetooth: Add broadcom-bluetooth

Hi Stefan,

>>>>> Thanks a lot for working on that, I've made a similar attempt a few
>>>>> weeks ago but didn't manage to get it to work.
>>>>>
>>>>> The way it's hooked in our boards is a bit more complex though, even
>>>>> if it could be because we're using a different part.
>>>>>
>>>>> In order to get it running we need:
>>>>> - two clocks, called in the broadcom datasheets lpo and tcxo.
>>>>> - three GPIOs, device wakeup, host wakeup and a shutdown GPIO (which
>>>>> might be the BT_ON you were discussing about)
>>>>> - two regulators called vbat and reg-en for us (I guess they're
>>>>> meant to power the chip, and its registers >>
>>>>> Do you know if you're also using those? Or could it be that it's just
>>>>> hardwired to some non-gatable crystal / regulator on the RPI?
>>>
>>> Not on Pi3, but the three gpios and the clock are pretty common for
>>> Broadcom bt controller (cf v4 of dt-bindings patch).
>>
>> once we get the GPIO expander driver upstream, I think we also need this for RPI 3 and Zero W. Right now we can just not do anything about this.
>
> AFAIK the Zero W doesn't have this GPIO expander. Would it be easier for you?

I don’t understand the question. In general you want the BT_ON and wakeup GPIOs to be available so that you can have good power savings. If they are not there, then it is always powered. It works of course as shown with RPI 3 where the boot loader enables the GPIOs. Which means it is like the current support that we have in net-next.

On the other hand, the hci_bcm.c support for RPI 3 is essentially a support for all Broadcom devices using DT. It should be extended to support all sorts of boards. And once ACPI becomes serdev support, we can unify ACPI, platform and DT devices into a single code path.

Regards

Marcel


2017-08-27 09:54:24

by Stefan Wahren

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: net: bluetooth: Add broadcom-bluetooth

Hi Marcel,

> Marcel Holtmann <[email protected]> hat am 23. August 2017 um 16:19 geschrieben:
>
>
> Hi Loic,
>
> >>> Thanks a lot for working on that, I've made a similar attempt a few
> >>> weeks ago but didn't manage to get it to work.
> >>>
> >>> The way it's hooked in our boards is a bit more complex though, even
> >>> if it could be because we're using a different part.
> >>>
> >>> In order to get it running we need:
> >>> - two clocks, called in the broadcom datasheets lpo and tcxo.
> >>> - three GPIOs, device wakeup, host wakeup and a shutdown GPIO (which
> >>> might be the BT_ON you were discussing about)
> >>> - two regulators called vbat and reg-en for us (I guess they're
> >>> meant to power the chip, and its registers >>
> >>> Do you know if you're also using those? Or could it be that it's just
> >>> hardwired to some non-gatable crystal / regulator on the RPI?
> >
> > Not on Pi3, but the three gpios and the clock are pretty common for
> > Broadcom bt controller (cf v4 of dt-bindings patch).
>
> once we get the GPIO expander driver upstream, I think we also need this for RPI 3 and Zero W. Right now we can just not do anything about this.

AFAIK the Zero W doesn't have this GPIO expander. Would it be easier for you?

2017-08-23 14:19:56

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: net: bluetooth: Add broadcom-bluetooth

Hi Loic,

>>> Thanks a lot for working on that, I've made a similar attempt a few
>>> weeks ago but didn't manage to get it to work.
>>>
>>> The way it's hooked in our boards is a bit more complex though, even
>>> if it could be because we're using a different part.
>>>
>>> In order to get it running we need:
>>> - two clocks, called in the broadcom datasheets lpo and tcxo.
>>> - three GPIOs, device wakeup, host wakeup and a shutdown GPIO (which
>>> might be the BT_ON you were discussing about)
>>> - two regulators called vbat and reg-en for us (I guess they're
>>> meant to power the chip, and its registers >>
>>> Do you know if you're also using those? Or could it be that it's just
>>> hardwired to some non-gatable crystal / regulator on the RPI?
>
> Not on Pi3, but the three gpios and the clock are pretty common for
> Broadcom bt controller (cf v4 of dt-bindings patch).

once we get the GPIO expander driver upstream, I think we also need this for RPI 3 and Zero W. Right now we can just not do anything about this.

However we can start looking at the PCM setup and see how that one is wired up and see if we can feed that into ALSA or wherever they terminate that.

> This is already partially supported in the hci_bcm driver.
> Today this driver registers a platform_driver(legacy/ACPI) and a
> serdev_device_driver (new/DT).
>
> The platform driver retrieves the gpios and mainly uses them in pm ops.
> Once the ACPI for serdev will be supported, this plat driver should be
> removed.
>
> The serdev driver does no support this yet because I used the RPi3
> as dev platform. But this is something we want to have as well.

Making ACPI serdev aware and turning BTH0 into serdev nodes is really the next step. That would allow us to simplify the drivers and remove duplicate code. Do you still have access to ACPI based tablets with Broadcom chips? It would be great if you can take a stab at this.

Regards

Marcel


2017-08-23 14:12:15

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: net: bluetooth: Add broadcom-bluetooth

Hi Loic,

On Tue, Aug 22, 2017 at 10:27:43PM +0200, Loic Poulain wrote:
> Hi Maxime,
>
> > > Thanks a lot for working on that, I've made a similar attempt a few
> > > weeks ago but didn't manage to get it to work.
> > >
> > > The way it's hooked in our boards is a bit more complex though, even
> > > if it could be because we're using a different part.
> > >
> > > In order to get it running we need:
> > > - two clocks, called in the broadcom datasheets lpo and tcxo.
> > > - three GPIOs, device wakeup, host wakeup and a shutdown GPIO (which
> > > might be the BT_ON you were discussing about)
> > > - two regulators called vbat and reg-en for us (I guess they're
> > > meant to power the chip, and its registers >>
> > > Do you know if you're also using those? Or could it be that it's just
> > > hardwired to some non-gatable crystal / regulator on the RPI?
>
> Not on Pi3, but the three gpios and the clock are pretty common for
> Broadcom bt controller (cf v4 of dt-bindings patch).
>
> This is already partially supported in the hci_bcm driver.
> Today this driver registers a platform_driver(legacy/ACPI) and a
> serdev_device_driver (new/DT).
>
> The platform driver retrieves the gpios and mainly uses them in pm ops.
> Once the ACPI for serdev will be supported, this plat driver should be
> removed.
>
> The serdev driver does no support this yet because I used the RPi3
> as dev platform. But this is something we want to have as well.

I'll give it a shot then. Thanks!
Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


Attachments:
(No filename) (1.58 kB)
signature.asc (801.00 B)
Download all attachments

2017-08-22 20:27:43

by Loic Poulain

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: net: bluetooth: Add broadcom-bluetooth

Hi Maxime,

>> Thanks a lot for working on that, I've made a similar attempt a few
>> weeks ago but didn't manage to get it to work.
>>
>> The way it's hooked in our boards is a bit more complex though, even
>> if it could be because we're using a different part.
>>
>> In order to get it running we need:
>> - two clocks, called in the broadcom datasheets lpo and tcxo.
>> - three GPIOs, device wakeup, host wakeup and a shutdown GPIO (which
>> might be the BT_ON you were discussing about)
>> - two regulators called vbat and reg-en for us (I guess they're
>> meant to power the chip, and its registers >>
>> Do you know if you're also using those? Or could it be that it's just
>> hardwired to some non-gatable crystal / regulator on the RPI?

Not on Pi3, but the three gpios and the clock are pretty common for
Broadcom bt controller (cf v4 of dt-bindings patch).

This is already partially supported in the hci_bcm driver.
Today this driver registers a platform_driver(legacy/ACPI) and a
serdev_device_driver (new/DT).

The platform driver retrieves the gpios and mainly uses them in pm ops.
Once the ACPI for serdev will be supported, this plat driver should be
removed.

The serdev driver does no support this yet because I used the RPi3
as dev platform. But this is something we want to have as well.

>
> unfortunately the Foundation doesn't provide the full schematics for RPI
> 3 or RPI Zero W (which uses the same chip).
>
> Please also take a look at the RPI Zero W device tree sources, which
> describe the wifi part of the chip [1]. It contains a line called WL_ON
> on gpio41 and the low power clock on gpio43.

Regards,
Loic

2017-08-22 11:01:20

by Stefan Wahren

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: net: bluetooth: Add broadcom-bluetooth

Hi Maxime,


Am 22.08.2017 um 09:47 schrieb Maxime Ripard:
> Hi Loic,
>
> On Tue, Aug 01, 2017 at 10:38:22AM +0200, Loic Poulain wrote:
>> Add binding document for serial bluetooth chips using
>> Broadcom protocol.
>>
>> Signed-off-by: Loic Poulain <[email protected]>
>> ---
>> v2: dt-bindings as separate patch
>> rebase on upcoming pi3 dts changes
>>
>> .../devicetree/bindings/net/broadcom-bluetooth.txt | 29 ++++++++++++++++++++++
>> 1 file changed, 29 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
>> new file mode 100644
>> index 0000000..c51ea1b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
>> @@ -0,0 +1,29 @@
>> +Broadcom Bluetooth Chips
>> +---------------------
>> +
>> +This documents the binding structure and common properties for serial
>> +attached Broadcom devices.
>> +
>> +Serial attached Broadcom devices shall be a child node of the host UART
>> +device the slave device is attached to.
>> +
>> +Required properties:
>> +
>> + - compatible: should contain one of the following:
>> + * "brcm,bcm43438-bt"
>> +
>> +Optional properties:
>> +
>> + - max-speed: see Documentation/devicetree/bindings/serial/slave-device.txt
>> +
>> +Example:
>> +
>> +&uart2 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&uart2_pins>;
>> +
>> + bluetooth {
>> + compatible = "brcm,bcm43438-bt";
>> + max-speed = <921600>;
>> + };
>> +};
> Thanks a lot for working on that, I've made a similar attempt a few
> weeks ago but didn't manage to get it to work.
>
> The way it's hooked in our boards is a bit more complex though, even
> if it could be because we're using a different part.
>
> In order to get it running we need:
> - two clocks, called in the broadcom datasheets lpo and tcxo.
> - three GPIOs, device wakeup, host wakeup and a shutdown GPIO (which
> might be the BT_ON you were discussing about)
> - two regulators called vbat and reg-en for us (I guess they're
> meant to power the chip, and its registers)
>
> Do you know if you're also using those? Or could it be that it's just
> hardwired to some non-gatable crystal / regulator on the RPI?

unfortunately the Foundation doesn't provide the full schematics for RPI
3 or RPI Zero W (which uses the same chip).

Please also take a look at the RPI Zero W device tree sources, which
describe the wifi part of the chip [1]. It contains a line called WL_ON
on gpio41 and the low power clock on gpio43.

Stefan

[1] -
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm/boot/dts/bcm2835-rpi-zero-w.dts?h=next-20170822

>
> Thanks!
> Maxime
>

2017-08-22 07:47:26

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: net: bluetooth: Add broadcom-bluetooth

Hi Loic,

On Tue, Aug 01, 2017 at 10:38:22AM +0200, Loic Poulain wrote:
> Add binding document for serial bluetooth chips using
> Broadcom protocol.
>
> Signed-off-by: Loic Poulain <[email protected]>
> ---
> v2: dt-bindings as separate patch
> rebase on upcoming pi3 dts changes
>
> .../devicetree/bindings/net/broadcom-bluetooth.txt | 29 ++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
>
> diff --git a/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
> new file mode 100644
> index 0000000..c51ea1b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
> @@ -0,0 +1,29 @@
> +Broadcom Bluetooth Chips
> +---------------------
> +
> +This documents the binding structure and common properties for serial
> +attached Broadcom devices.
> +
> +Serial attached Broadcom devices shall be a child node of the host UART
> +device the slave device is attached to.
> +
> +Required properties:
> +
> + - compatible: should contain one of the following:
> + * "brcm,bcm43438-bt"
> +
> +Optional properties:
> +
> + - max-speed: see Documentation/devicetree/bindings/serial/slave-device.txt
> +
> +Example:
> +
> +&uart2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart2_pins>;
> +
> + bluetooth {
> + compatible = "brcm,bcm43438-bt";
> + max-speed = <921600>;
> + };
> +};

Thanks a lot for working on that, I've made a similar attempt a few
weeks ago but didn't manage to get it to work.

The way it's hooked in our boards is a bit more complex though, even
if it could be because we're using a different part.

In order to get it running we need:
- two clocks, called in the broadcom datasheets lpo and tcxo.
- three GPIOs, device wakeup, host wakeup and a shutdown GPIO (which
might be the BT_ON you were discussing about)
- two regulators called vbat and reg-en for us (I guess they're
meant to power the chip, and its registers)

Do you know if you're also using those? Or could it be that it's just
hardwired to some non-gatable crystal / regulator on the RPI?

Thanks!
Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


Attachments:
(No filename) (2.30 kB)
signature.asc (801.00 B)
Download all attachments

2017-08-08 04:47:18

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] Bluetooth: hci_bcm: Add serdev support

Hi Eric,

>>>>>>>>> Add basic support for Broadcom serial slave devices.
>>>>>>>>> Probe the serial device, retrieve its maximum speed and
>>>>>>>>> register a new hci uart device.
>>>>>>>>>
>>>>>>>>> Tested/compatible with bcm43438 (RPi3).
>>>>>>>>>
>>>>>>>>> Signed-off-by: Loic Poulain <[email protected]>
>>>>>>>>> ...
>>>>>>>>>
>>>>>>>>> +static int bcm_serdev_probe(struct serdev_device *serdev)
>>>>>>>>> +{
>>>>>>>>> + struct bcm_bt_device *bcmdev;
>>>>>>>>> + u32 speed;
>>>>>>>>> + int err;
>>>>>>>>> +
>>>>>>>>> + bcmdev = devm_kzalloc(&serdev->dev, sizeof(*bcmdev), GFP_KERNEL);
>>>>>>>>> + if (!bcmdev)
>>>>>>>>> + return -ENOMEM;
>>>>>>>>> +
>>>>>>>>> + bcmdev->hu.serdev = serdev;
>>>>>>>>> + serdev_device_set_drvdata(serdev, bcmdev);
>>>>>>>>> +
>>>>>>>>> + err = of_property_read_u32(serdev->dev.of_node, "max-speed", &speed);
>>>>>>>>> + if (!err)
>>>>>>>>> + bcmdev->hu.oper_speed = speed;
>>>>>>>>> +
>>>>>>>>> + return hci_uart_register_device(&bcmdev->hu, &bcm_proto);
>>>>>>>>> +}
>>>>>>>>
>>>>>>>> We do not need any GPIO for reset lines or anything else for the rPI3?
>>>>>>>>
>>>>>>>
>>>>>>> unfortunately we don't have full schematics for RPI3, but according to firmware dt-blob.dts [1] there is a GPIO called BT_ON. This GPIO is controlled by the GPIO expander on the RPI3 board. The necessary driver for this expander is still out of tree (last mainlining attempt [2]).
>>>>>>
>>>>>> so who handles this GPIO then right now (or any other GPIO for that matter)? I have seen Bluetooth being enabled and operational, but I really want to get this working in a plain upstream Fedora and without using hciattach or btattach.
>>>>>
>>>>> The GPU firmware enables the GPIO and keeps this state. The mentioned expander driver should provide the ARM core (Linux) the necessary control.
>>>>
>>>> what is the default behavior after reboot. BT_ON enabled or not?
>>>
>>> AFAIK: enabled
>
> Correct: it's enabled by the 2nd stage bootloader and never modified
> From there.

so I do not have Bluetooth working with a Fedora 26 upstream kernel. I can not figure out what extra is needed to make the Bluetooth chip respond. Is there a simple way to check the status of the BT_ON GPIO (with the lacking GPIO expander code).

>> that would be a funny default since it means that it consumes power
>> even if Bluetooth is never used. We really want to hook this up into
>> the Bluetooth power control of the controller. So that on power down
>> it also disables it and saves as much power as possible.
>
> We would need a driver to talk to the firmware for the GPIO expander to
> turn it off. I wrote one last year, but my motivation got killed by DT
> bikeshedding.

If we get the GPIO expander code upstream, I wonder if we can make BT_ON disabled by default. The hci_bcm could in theory turn it back off if the Bluetooth controller doesn’t get enabled, but it would require the Bluetooth driver to be at least loaded. While for now it is easiest to have BT_ON enabled, but if you really don’t want Bluetooth it seems kinda wasteful.

In addition we would need to know if the Bluetooth firmware needs to be reloaded after every BT_ON toggle. Normally there is an additional BT_RST reset line, but that seems to be missing in the rPI3 design.

Also all these hacks with miniuart and switching the console from ttyAMA0 to ttyS0 is totally weird to me. The Raspbian side of things seems to have hacked a bunch of things to make this work, but we really need to use serdev for Bluetooth devices like this and also have the power control of these done by the Bluetooth driver.

So what would it take to get this all working upstream so it can trickle down into a Fedora kernel?

Regards

Marcel


2017-08-07 22:17:25

by Eric Anholt

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] Bluetooth: hci_bcm: Add serdev support

Marcel Holtmann <[email protected]> writes:

> Hi Stefan,
>
>>>>>>>> Add basic support for Broadcom serial slave devices.
>>>>>>>> Probe the serial device, retrieve its maximum speed and
>>>>>>>> register a new hci uart device.
>>>>>>>>
>>>>>>>> Tested/compatible with bcm43438 (RPi3).
>>>>>>>>
>>>>>>>> Signed-off-by: Loic Poulain <[email protected]>
>>>>>>>> ...
>>>>>>>>
>>>>>>>> +static int bcm_serdev_probe(struct serdev_device *serdev)
>>>>>>>> +{
>>>>>>>> + struct bcm_bt_device *bcmdev;
>>>>>>>> + u32 speed;
>>>>>>>> + int err;
>>>>>>>> +
>>>>>>>> + bcmdev = devm_kzalloc(&serdev->dev, sizeof(*bcmdev), GFP_KERNEL);
>>>>>>>> + if (!bcmdev)
>>>>>>>> + return -ENOMEM;
>>>>>>>> +
>>>>>>>> + bcmdev->hu.serdev = serdev;
>>>>>>>> + serdev_device_set_drvdata(serdev, bcmdev);
>>>>>>>> +
>>>>>>>> + err = of_property_read_u32(serdev->dev.of_node, "max-speed", &speed);
>>>>>>>> + if (!err)
>>>>>>>> + bcmdev->hu.oper_speed = speed;
>>>>>>>> +
>>>>>>>> + return hci_uart_register_device(&bcmdev->hu, &bcm_proto);
>>>>>>>> +}
>>>>>>>
>>>>>>> We do not need any GPIO for reset lines or anything else for the rPI3?
>>>>>>>
>>>>>>
>>>>>> unfortunately we don't have full schematics for RPI3, but according to firmware dt-blob.dts [1] there is a GPIO called BT_ON. This GPIO is controlled by the GPIO expander on the RPI3 board. The necessary driver for this expander is still out of tree (last mainlining attempt [2]).
>>>>>
>>>>> so who handles this GPIO then right now (or any other GPIO for that matter)? I have seen Bluetooth being enabled and operational, but I really want to get this working in a plain upstream Fedora and without using hciattach or btattach.
>>>>
>>>> The GPU firmware enables the GPIO and keeps this state. The mentioned expander driver should provide the ARM core (Linux) the necessary control.
>>>
>>> what is the default behavior after reboot. BT_ON enabled or not?
>>
>> AFAIK: enabled

Correct: it's enabled by the 2nd stage bootloader and never modified
From there.

> that would be a funny default since it means that it consumes power
> even if Bluetooth is never used. We really want to hook this up into
> the Bluetooth power control of the controller. So that on power down
> it also disables it and saves as much power as possible.

We would need a driver to talk to the firmware for the GPIO expander to
turn it off. I wrote one last year, but my motivation got killed by DT
bikeshedding.


Attachments:
signature.asc (832.00 B)

2017-08-04 20:58:02

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] Bluetooth: hci_bcm: Add serdev support

Hi Stefan,

>>>>>>> Add basic support for Broadcom serial slave devices.
>>>>>>> Probe the serial device, retrieve its maximum speed and
>>>>>>> register a new hci uart device.
>>>>>>>
>>>>>>> Tested/compatible with bcm43438 (RPi3).
>>>>>>>
>>>>>>> Signed-off-by: Loic Poulain <[email protected]>
>>>>>>> ...
>>>>>>>
>>>>>>> +static int bcm_serdev_probe(struct serdev_device *serdev)
>>>>>>> +{
>>>>>>> + struct bcm_bt_device *bcmdev;
>>>>>>> + u32 speed;
>>>>>>> + int err;
>>>>>>> +
>>>>>>> + bcmdev = devm_kzalloc(&serdev->dev, sizeof(*bcmdev), GFP_KERNEL);
>>>>>>> + if (!bcmdev)
>>>>>>> + return -ENOMEM;
>>>>>>> +
>>>>>>> + bcmdev->hu.serdev = serdev;
>>>>>>> + serdev_device_set_drvdata(serdev, bcmdev);
>>>>>>> +
>>>>>>> + err = of_property_read_u32(serdev->dev.of_node, "max-speed", &speed);
>>>>>>> + if (!err)
>>>>>>> + bcmdev->hu.oper_speed = speed;
>>>>>>> +
>>>>>>> + return hci_uart_register_device(&bcmdev->hu, &bcm_proto);
>>>>>>> +}
>>>>>>
>>>>>> We do not need any GPIO for reset lines or anything else for the rPI3?
>>>>>>
>>>>>
>>>>> unfortunately we don't have full schematics for RPI3, but according to firmware dt-blob.dts [1] there is a GPIO called BT_ON. This GPIO is controlled by the GPIO expander on the RPI3 board. The necessary driver for this expander is still out of tree (last mainlining attempt [2]).
>>>>
>>>> so who handles this GPIO then right now (or any other GPIO for that matter)? I have seen Bluetooth being enabled and operational, but I really want to get this working in a plain upstream Fedora and without using hciattach or btattach.
>>>
>>> The GPU firmware enables the GPIO and keeps this state. The mentioned expander driver should provide the ARM core (Linux) the necessary control.
>>
>> what is the default behavior after reboot. BT_ON enabled or not?
>
> AFAIK: enabled

that would be a funny default since it means that it consumes power even if Bluetooth is never used. We really want to hook this up into the Bluetooth power control of the controller. So that on power down it also disables it and saves as much power as possible.

Regards

Marcel


2017-08-04 19:50:03

by Stefan Wahren

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] Bluetooth: hci_bcm: Add serdev support


> Marcel Holtmann <[email protected]> hat am 4. August 2017 um 21:45 geschrieben:
>
>
> Hi Stefan,
>
> >>>>> Add basic support for Broadcom serial slave devices.
> >>>>> Probe the serial device, retrieve its maximum speed and
> >>>>> register a new hci uart device.
> >>>>>
> >>>>> Tested/compatible with bcm43438 (RPi3).
> >>>>>
> >>>>> Signed-off-by: Loic Poulain <[email protected]>
> >>>>> ...
> >>>>>
> >>>>> +static int bcm_serdev_probe(struct serdev_device *serdev)
> >>>>> +{
> >>>>> + struct bcm_bt_device *bcmdev;
> >>>>> + u32 speed;
> >>>>> + int err;
> >>>>> +
> >>>>> + bcmdev = devm_kzalloc(&serdev->dev, sizeof(*bcmdev), GFP_KERNEL);
> >>>>> + if (!bcmdev)
> >>>>> + return -ENOMEM;
> >>>>> +
> >>>>> + bcmdev->hu.serdev = serdev;
> >>>>> + serdev_device_set_drvdata(serdev, bcmdev);
> >>>>> +
> >>>>> + err = of_property_read_u32(serdev->dev.of_node, "max-speed", &speed);
> >>>>> + if (!err)
> >>>>> + bcmdev->hu.oper_speed = speed;
> >>>>> +
> >>>>> + return hci_uart_register_device(&bcmdev->hu, &bcm_proto);
> >>>>> +}
> >>>>
> >>>> We do not need any GPIO for reset lines or anything else for the rPI3?
> >>>>
> >>>
> >>> unfortunately we don't have full schematics for RPI3, but according to firmware dt-blob.dts [1] there is a GPIO called BT_ON. This GPIO is controlled by the GPIO expander on the RPI3 board. The necessary driver for this expander is still out of tree (last mainlining attempt [2]).
> >>
> >> so who handles this GPIO then right now (or any other GPIO for that matter)? I have seen Bluetooth being enabled and operational, but I really want to get this working in a plain upstream Fedora and without using hciattach or btattach.
> >
> > The GPU firmware enables the GPIO and keeps this state. The mentioned expander driver should provide the ARM core (Linux) the necessary control.
>
> what is the default behavior after reboot. BT_ON enabled or not?

AFAIK: enabled

>
> Regards
>
> Marcel
>

2017-08-04 19:45:16

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] Bluetooth: hci_bcm: Add serdev support

Hi Stefan,

>>>>> Add basic support for Broadcom serial slave devices.
>>>>> Probe the serial device, retrieve its maximum speed and
>>>>> register a new hci uart device.
>>>>>
>>>>> Tested/compatible with bcm43438 (RPi3).
>>>>>
>>>>> Signed-off-by: Loic Poulain <[email protected]>
>>>>> ...
>>>>>
>>>>> +static int bcm_serdev_probe(struct serdev_device *serdev)
>>>>> +{
>>>>> + struct bcm_bt_device *bcmdev;
>>>>> + u32 speed;
>>>>> + int err;
>>>>> +
>>>>> + bcmdev = devm_kzalloc(&serdev->dev, sizeof(*bcmdev), GFP_KERNEL);
>>>>> + if (!bcmdev)
>>>>> + return -ENOMEM;
>>>>> +
>>>>> + bcmdev->hu.serdev = serdev;
>>>>> + serdev_device_set_drvdata(serdev, bcmdev);
>>>>> +
>>>>> + err = of_property_read_u32(serdev->dev.of_node, "max-speed", &speed);
>>>>> + if (!err)
>>>>> + bcmdev->hu.oper_speed = speed;
>>>>> +
>>>>> + return hci_uart_register_device(&bcmdev->hu, &bcm_proto);
>>>>> +}
>>>>
>>>> We do not need any GPIO for reset lines or anything else for the rPI3?
>>>>
>>>
>>> unfortunately we don't have full schematics for RPI3, but according to firmware dt-blob.dts [1] there is a GPIO called BT_ON. This GPIO is controlled by the GPIO expander on the RPI3 board. The necessary driver for this expander is still out of tree (last mainlining attempt [2]).
>>
>> so who handles this GPIO then right now (or any other GPIO for that matter)? I have seen Bluetooth being enabled and operational, but I really want to get this working in a plain upstream Fedora and without using hciattach or btattach.
>
> The GPU firmware enables the GPIO and keeps this state. The mentioned expander driver should provide the ARM core (Linux) the necessary control.

what is the default behavior after reboot. BT_ON enabled or not?

Regards

Marcel


2017-08-04 19:44:01

by Stefan Wahren

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] Bluetooth: hci_bcm: Add serdev support


> Marcel Holtmann <[email protected]> hat am 4. August 2017 um 21:37 geschrieben:
>
>
> Hi Stefan,
>
> >>> Add basic support for Broadcom serial slave devices.
> >>> Probe the serial device, retrieve its maximum speed and
> >>> register a new hci uart device.
> >>>
> >>> Tested/compatible with bcm43438 (RPi3).
> >>>
> >>> Signed-off-by: Loic Poulain <[email protected]>
> >>> ...
> >>>
> >>> +static int bcm_serdev_probe(struct serdev_device *serdev)
> >>> +{
> >>> + struct bcm_bt_device *bcmdev;
> >>> + u32 speed;
> >>> + int err;
> >>> +
> >>> + bcmdev = devm_kzalloc(&serdev->dev, sizeof(*bcmdev), GFP_KERNEL);
> >>> + if (!bcmdev)
> >>> + return -ENOMEM;
> >>> +
> >>> + bcmdev->hu.serdev = serdev;
> >>> + serdev_device_set_drvdata(serdev, bcmdev);
> >>> +
> >>> + err = of_property_read_u32(serdev->dev.of_node, "max-speed", &speed);
> >>> + if (!err)
> >>> + bcmdev->hu.oper_speed = speed;
> >>> +
> >>> + return hci_uart_register_device(&bcmdev->hu, &bcm_proto);
> >>> +}
> >>
> >> We do not need any GPIO for reset lines or anything else for the rPI3?
> >>
> >
> > unfortunately we don't have full schematics for RPI3, but according to firmware dt-blob.dts [1] there is a GPIO called BT_ON. This GPIO is controlled by the GPIO expander on the RPI3 board. The necessary driver for this expander is still out of tree (last mainlining attempt [2]).
>
> so who handles this GPIO then right now (or any other GPIO for that matter)? I have seen Bluetooth being enabled and operational, but I really want to get this working in a plain upstream Fedora and without using hciattach or btattach.

The GPU firmware enables the GPIO and keeps this state. The mentioned expander driver should provide the ARM core (Linux) the necessary control.

>
> Regards
>
> Marcel
>

2017-08-04 19:37:24

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] Bluetooth: hci_bcm: Add serdev support

Hi Stefan,

>>> Add basic support for Broadcom serial slave devices.
>>> Probe the serial device, retrieve its maximum speed and
>>> register a new hci uart device.
>>>
>>> Tested/compatible with bcm43438 (RPi3).
>>>
>>> Signed-off-by: Loic Poulain <[email protected]>
>>> ...
>>>
>>> +static int bcm_serdev_probe(struct serdev_device *serdev)
>>> +{
>>> + struct bcm_bt_device *bcmdev;
>>> + u32 speed;
>>> + int err;
>>> +
>>> + bcmdev = devm_kzalloc(&serdev->dev, sizeof(*bcmdev), GFP_KERNEL);
>>> + if (!bcmdev)
>>> + return -ENOMEM;
>>> +
>>> + bcmdev->hu.serdev = serdev;
>>> + serdev_device_set_drvdata(serdev, bcmdev);
>>> +
>>> + err = of_property_read_u32(serdev->dev.of_node, "max-speed", &speed);
>>> + if (!err)
>>> + bcmdev->hu.oper_speed = speed;
>>> +
>>> + return hci_uart_register_device(&bcmdev->hu, &bcm_proto);
>>> +}
>>
>> We do not need any GPIO for reset lines or anything else for the rPI3?
>>
>
> unfortunately we don't have full schematics for RPI3, but according to firmware dt-blob.dts [1] there is a GPIO called BT_ON. This GPIO is controlled by the GPIO expander on the RPI3 board. The necessary driver for this expander is still out of tree (last mainlining attempt [2]).

so who handles this GPIO then right now (or any other GPIO for that matter)? I have seen Bluetooth being enabled and operational, but I really want to get this working in a plain upstream Fedora and without using hciattach or btattach.

Regards

Marcel


2017-08-04 19:26:25

by Stefan Wahren

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] Bluetooth: hci_bcm: Add serdev support

Hi Marcel,

> Marcel Holtmann <[email protected]> hat am 4. August 2017 um 16:36 geschrieben:
>
>
> Hi Loic,
>
> > Add basic support for Broadcom serial slave devices.
> > Probe the serial device, retrieve its maximum speed and
> > register a new hci uart device.
> >
> > Tested/compatible with bcm43438 (RPi3).
> >
> > Signed-off-by: Loic Poulain <[email protected]>
> > ...
> >
> > +static int bcm_serdev_probe(struct serdev_device *serdev)
> > +{
> > + struct bcm_bt_device *bcmdev;
> > + u32 speed;
> > + int err;
> > +
> > + bcmdev = devm_kzalloc(&serdev->dev, sizeof(*bcmdev), GFP_KERNEL);
> > + if (!bcmdev)
> > + return -ENOMEM;
> > +
> > + bcmdev->hu.serdev = serdev;
> > + serdev_device_set_drvdata(serdev, bcmdev);
> > +
> > + err = of_property_read_u32(serdev->dev.of_node, "max-speed", &speed);
> > + if (!err)
> > + bcmdev->hu.oper_speed = speed;
> > +
> > + return hci_uart_register_device(&bcmdev->hu, &bcm_proto);
> > +}
>
> We do not need any GPIO for reset lines or anything else for the rPI3?
>

unfortunately we don't have full schematics for RPI3, but according to firmware dt-blob.dts [1] there is a GPIO called BT_ON. This GPIO is controlled by the GPIO expander on the RPI3 board. The necessary driver for this expander is still out of tree (last mainlining attempt [2]).

Regards
Stefan

[1] - https://github.com/raspberrypi/firmware/blob/master/extra/dt-blob.dts
[2] - http://lists.infradead.org/pipermail/linux-rpi-kernel/2017-March/005801.html

2017-08-04 14:36:02

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] Bluetooth: hci_bcm: Add serdev support

Hi Loic,

> Add basic support for Broadcom serial slave devices.
> Probe the serial device, retrieve its maximum speed and
> register a new hci uart device.
>
> Tested/compatible with bcm43438 (RPi3).
>
> Signed-off-by: Loic Poulain <[email protected]>
> ---
> v2: dt-bindings as separate patch
> rebase on upcoming pi3 dts changes
>
> drivers/bluetooth/hci_bcm.c | 82 ++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 78 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
> index 6b42372..f824738 100644
> --- a/drivers/bluetooth/hci_bcm.c
> +++ b/drivers/bluetooth/hci_bcm.c
> @@ -27,6 +27,7 @@
> #include <linux/firmware.h>
> #include <linux/module.h>
> #include <linux/acpi.h>
> +#include <linux/of.h>
> #include <linux/platform_device.h>
> #include <linux/clk.h>
> #include <linux/gpio/consumer.h>
> @@ -34,6 +35,7 @@
> #include <linux/interrupt.h>
> #include <linux/dmi.h>
> #include <linux/pm_runtime.h>
> +#include <linux/serdev.h>
>
> #include <net/bluetooth/bluetooth.h>
> #include <net/bluetooth/hci_core.h>
> @@ -46,6 +48,7 @@
>
> #define BCM_AUTOSUSPEND_DELAY 5000 /* default autosleep delay */
>
> +/* platform device driver resources */
> struct bcm_device {
> struct list_head list;
>
> @@ -68,6 +71,12 @@ struct bcm_device {
> #endif
> };
>
> +/* serdev driver resources */
> +struct bcm_bt_device {
> + struct hci_uart hu;
> +};

why not call this bcm_serdev for now. I think that keeps it a bit simpler.

> +
> +/* generic bcm uart resources */
> struct bcm_data {
> struct sk_buff *rx_skb;
> struct sk_buff_head txq;
> @@ -289,6 +298,11 @@ static int bcm_open(struct hci_uart *hu)
>
> hu->priv = bcm;
>
> + if (hu->serdev) {
> + serdev_device_open(hu->serdev);
> + goto out;
> + }
> +

Lets prefix these with comments saying that if this is a serdev / DT defined device, then we only use serdev open primitive and skip the rest.

> if (!hu->tty->dev)
> goto out;
>
> @@ -323,6 +337,9 @@ static int bcm_close(struct hci_uart *hu)
>
> bt_dev_dbg(hu->hdev, "hu %p", hu);
>
> + if (hu->serdev)
> + serdev_device_close(hu->serdev);
> +

Similar here, lets have a comment that says we are closing serdev and then continue as usual.

> /* Protect bcm->dev against removal of the device or driver */
> mutex_lock(&bcm_device_lock);
> if (bcm_device_exists(bdev)) {
> @@ -397,8 +414,12 @@ static int bcm_setup(struct hci_uart *hu)
> else
> speed = 0;
>
> - if (speed)
> - hci_uart_set_baudrate(hu, speed);
> + if (speed) {
> + if (hu->serdev)
> + serdev_device_set_baudrate(hu->serdev, speed);
> + else
> + hci_uart_set_baudrate(hu, speed);
> + }
>

Here I would also add a small comment that when serdev is used a different baud rate primitive needs to be used.

> /* Operational speed if any */
> if (hu->oper_speed)
> @@ -410,8 +431,12 @@ static int bcm_setup(struct hci_uart *hu)
>
> if (speed) {
> err = bcm_set_baudrate(hu, speed);
> - if (!err)
> - hci_uart_set_baudrate(hu, speed);
> + if (!err) {
> + if (hu->serdev)
> + serdev_device_set_baudrate(hu->serdev, speed);
> + else
> + hci_uart_set_baudrate(hu, speed);
> + }

Maybe we should split this out into a helper. Maybe call it it device_set_baudrate.

> }
>
> finalize:
> @@ -903,9 +928,57 @@ static int bcm_remove(struct platform_device *pdev)
> },
> };
>
> +static int bcm_serdev_probe(struct serdev_device *serdev)
> +{
> + struct bcm_bt_device *bcmdev;
> + u32 speed;
> + int err;
> +
> + bcmdev = devm_kzalloc(&serdev->dev, sizeof(*bcmdev), GFP_KERNEL);
> + if (!bcmdev)
> + return -ENOMEM;
> +
> + bcmdev->hu.serdev = serdev;
> + serdev_device_set_drvdata(serdev, bcmdev);
> +
> + err = of_property_read_u32(serdev->dev.of_node, "max-speed", &speed);
> + if (!err)
> + bcmdev->hu.oper_speed = speed;
> +
> + return hci_uart_register_device(&bcmdev->hu, &bcm_proto);
> +}

We do not need any GPIO for reset lines or anything else for the rPI3?

> +
> +static void bcm_serdev_remove(struct serdev_device *serdev)
> +{
> + struct bcm_bt_device *bcmdev = serdev_device_get_drvdata(serdev);
> +
> + hci_uart_unregister_device(&bcmdev->hu);
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id bcm_bluetooth_of_match[] = {
> + { .compatible = "brcm,bcm43438-bt" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, bcm_bluetooth_of_match);
> +#endif
> +
> +static struct serdev_device_driver bcm_serdev_driver = {
> + .probe = bcm_serdev_probe,
> + .remove = bcm_serdev_remove,
> + .driver = {
> + .name = "hci_uart_bcm",
> + .of_match_table = of_match_ptr(bcm_bluetooth_of_match),
> + },
> +};
> +
> int __init bcm_init(void)
> {
> + /* For now, we need to keep both platform device
> + * driver (ACPI generated) and serdev driver (DT).
> + */
> platform_driver_register(&bcm_driver);
> + serdev_device_driver_register(&bcm_serdev_driver);
>
> return hci_uart_register_proto(&bcm_proto);
> }
> @@ -913,6 +986,7 @@ int __init bcm_init(void)
> int __exit bcm_deinit(void)
> {
> platform_driver_unregister(&bcm_driver);
> + serdev_device_driver_unregister(&bcm_serdev_driver);
>
> return hci_uart_unregister_proto(&bcm_proto);
> }

Regards

Marcel


2017-08-04 07:24:31

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave

Hi Rob,

> Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
> This allows to automatically insert the bcm43438 to the bluetooth
> subsystem instead of relying on userspace helpers (hciattach).
>
> Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
> of pl011/ttyAMA0.
>
> Signed-off-by: Loic Poulain <[email protected]>
> ---
> v2: dt-bindings as separate patch
> rebase on upcoming pi3 dts changes
>
> arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> index 20725ca..5abc1df 100644
> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> @@ -8,6 +8,11 @@
> compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
> model = "Raspberry Pi 3 Model B";
>
> + chosen {
> + /* 8250 auxiliar UART instead of pl011 */
> + bootargs = "earlyprintk console=ttyS0,115200";
> + };
> +
> memory {
> reg = <0 0x40000000>;
> };
> @@ -24,6 +29,11 @@
> pinctrl-names = "default";
> pinctrl-0 = <&uart0_gpio32 &gpclk2_gpio43>;
> status = "okay";
> +
> + bluetooth {
> + compatible = "brcm,bcm43438-bt";
> + max-speed = <921600>;
> + };
> };
>

can I get an ack on these patches? I like to take them through bluetooth-next tree.

Regards

Marcel


2017-08-01 08:51:32

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] Bluetooth: hci_bcm: Add serdev support

Hi,

On Tue, Aug 01, 2017 at 10:38:24AM +0200, Loic Poulain wrote:
> [...]
> + err = of_property_read_u32(serdev->dev.of_node, "max-speed", &speed);
> + if (!err)
> + bcmdev->hu.oper_speed = speed;
> [...]

I suggest to use device_property_read_u32() here and include
<linux/property.h> instead of <linux/of.h>. This way the
serdev support is not DT specific.

-- Sebastian


Attachments:
(No filename) (375.00 B)
signature.asc (833.00 B)
Download all attachments

2017-08-01 08:38:24

by Loic Poulain

[permalink] [raw]
Subject: [PATCH v2 3/3] Bluetooth: hci_bcm: Add serdev support

Add basic support for Broadcom serial slave devices.
Probe the serial device, retrieve its maximum speed and
register a new hci uart device.

Tested/compatible with bcm43438 (RPi3).

Signed-off-by: Loic Poulain <[email protected]>
---
v2: dt-bindings as separate patch
rebase on upcoming pi3 dts changes

drivers/bluetooth/hci_bcm.c | 82 ++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 78 insertions(+), 4 deletions(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 6b42372..f824738 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -27,6 +27,7 @@
#include <linux/firmware.h>
#include <linux/module.h>
#include <linux/acpi.h>
+#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/clk.h>
#include <linux/gpio/consumer.h>
@@ -34,6 +35,7 @@
#include <linux/interrupt.h>
#include <linux/dmi.h>
#include <linux/pm_runtime.h>
+#include <linux/serdev.h>

#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
@@ -46,6 +48,7 @@

#define BCM_AUTOSUSPEND_DELAY 5000 /* default autosleep delay */

+/* platform device driver resources */
struct bcm_device {
struct list_head list;

@@ -68,6 +71,12 @@ struct bcm_device {
#endif
};

+/* serdev driver resources */
+struct bcm_bt_device {
+ struct hci_uart hu;
+};
+
+/* generic bcm uart resources */
struct bcm_data {
struct sk_buff *rx_skb;
struct sk_buff_head txq;
@@ -289,6 +298,11 @@ static int bcm_open(struct hci_uart *hu)

hu->priv = bcm;

+ if (hu->serdev) {
+ serdev_device_open(hu->serdev);
+ goto out;
+ }
+
if (!hu->tty->dev)
goto out;

@@ -323,6 +337,9 @@ static int bcm_close(struct hci_uart *hu)

bt_dev_dbg(hu->hdev, "hu %p", hu);

+ if (hu->serdev)
+ serdev_device_close(hu->serdev);
+
/* Protect bcm->dev against removal of the device or driver */
mutex_lock(&bcm_device_lock);
if (bcm_device_exists(bdev)) {
@@ -397,8 +414,12 @@ static int bcm_setup(struct hci_uart *hu)
else
speed = 0;

- if (speed)
- hci_uart_set_baudrate(hu, speed);
+ if (speed) {
+ if (hu->serdev)
+ serdev_device_set_baudrate(hu->serdev, speed);
+ else
+ hci_uart_set_baudrate(hu, speed);
+ }

/* Operational speed if any */
if (hu->oper_speed)
@@ -410,8 +431,12 @@ static int bcm_setup(struct hci_uart *hu)

if (speed) {
err = bcm_set_baudrate(hu, speed);
- if (!err)
- hci_uart_set_baudrate(hu, speed);
+ if (!err) {
+ if (hu->serdev)
+ serdev_device_set_baudrate(hu->serdev, speed);
+ else
+ hci_uart_set_baudrate(hu, speed);
+ }
}

finalize:
@@ -903,9 +928,57 @@ static int bcm_remove(struct platform_device *pdev)
},
};

+static int bcm_serdev_probe(struct serdev_device *serdev)
+{
+ struct bcm_bt_device *bcmdev;
+ u32 speed;
+ int err;
+
+ bcmdev = devm_kzalloc(&serdev->dev, sizeof(*bcmdev), GFP_KERNEL);
+ if (!bcmdev)
+ return -ENOMEM;
+
+ bcmdev->hu.serdev = serdev;
+ serdev_device_set_drvdata(serdev, bcmdev);
+
+ err = of_property_read_u32(serdev->dev.of_node, "max-speed", &speed);
+ if (!err)
+ bcmdev->hu.oper_speed = speed;
+
+ return hci_uart_register_device(&bcmdev->hu, &bcm_proto);
+}
+
+static void bcm_serdev_remove(struct serdev_device *serdev)
+{
+ struct bcm_bt_device *bcmdev = serdev_device_get_drvdata(serdev);
+
+ hci_uart_unregister_device(&bcmdev->hu);
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id bcm_bluetooth_of_match[] = {
+ { .compatible = "brcm,bcm43438-bt" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, bcm_bluetooth_of_match);
+#endif
+
+static struct serdev_device_driver bcm_serdev_driver = {
+ .probe = bcm_serdev_probe,
+ .remove = bcm_serdev_remove,
+ .driver = {
+ .name = "hci_uart_bcm",
+ .of_match_table = of_match_ptr(bcm_bluetooth_of_match),
+ },
+};
+
int __init bcm_init(void)
{
+ /* For now, we need to keep both platform device
+ * driver (ACPI generated) and serdev driver (DT).
+ */
platform_driver_register(&bcm_driver);
+ serdev_device_driver_register(&bcm_serdev_driver);

return hci_uart_register_proto(&bcm_proto);
}
@@ -913,6 +986,7 @@ int __init bcm_init(void)
int __exit bcm_deinit(void)
{
platform_driver_unregister(&bcm_driver);
+ serdev_device_driver_unregister(&bcm_serdev_driver);

return hci_uart_unregister_proto(&bcm_proto);
}
--
1.9.1

2017-08-01 08:38:23

by Loic Poulain

[permalink] [raw]
Subject: [PATCH v2 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave

Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
This allows to automatically insert the bcm43438 to the bluetooth
subsystem instead of relying on userspace helpers (hciattach).

Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
of pl011/ttyAMA0.

Signed-off-by: Loic Poulain <[email protected]>
---
v2: dt-bindings as separate patch
rebase on upcoming pi3 dts changes

arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
index 20725ca..5abc1df 100644
--- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
+++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
@@ -8,6 +8,11 @@
compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
model = "Raspberry Pi 3 Model B";

+ chosen {
+ /* 8250 auxiliar UART instead of pl011 */
+ bootargs = "earlyprintk console=ttyS0,115200";
+ };
+
memory {
reg = <0 0x40000000>;
};
@@ -24,6 +29,11 @@
pinctrl-names = "default";
pinctrl-0 = <&uart0_gpio32 &gpclk2_gpio43>;
status = "okay";
+
+ bluetooth {
+ compatible = "brcm,bcm43438-bt";
+ max-speed = <921600>;
+ };
};

/* uart1 is mapped to the pin header */
--
1.9.1